diff --git a/server/graphql/datasources/userKyselyPersistenceFindByEmailAndOrg.test.ts b/server/graphql/datasources/userKyselyPersistenceFindByEmailAndOrg.test.ts index 7fdcc69..be77195 100644 --- a/server/graphql/datasources/userKyselyPersistenceFindByEmailAndOrg.test.ts +++ b/server/graphql/datasources/userKyselyPersistenceFindByEmailAndOrg.test.ts @@ -3,10 +3,8 @@ import { uid } from 'uid'; import { UserRole } from '../../services/userManagementService/index.js'; import createOrg from '../../test/fixtureHelpers/createOrg.js'; -import { makeMockedServer } from '../../test/setupMockedServer.js'; -import { makeTestWithFixture } from '../../test/utils.js'; +import { makeTransactionalTestWithFixture } from '../../test/harness/transactionalTest.js'; import { - kyselyUserDeleteById, kyselyUserFindByEmailAndOrg, kyselyUserInsert, } from './userKyselyPersistence.js'; @@ -25,9 +23,8 @@ function samlUserInput(orgId: string) { } describe('kyselyUserFindByEmailAndOrg', () => { - const testWithFixture = makeTestWithFixture(async () => { - const { deps, shutdown } = await makeMockedServer(); - const { org, cleanup: orgCleanup } = await createOrg( + const testWithFixture = makeTransactionalTestWithFixture(async ({ deps }) => { + const { org } = await createOrg( { KyselyPg: deps.KyselyPg, ModerationConfigService: deps.ModerationConfigService, @@ -35,14 +32,7 @@ describe('kyselyUserFindByEmailAndOrg', () => { }, uid(), ); - return { - deps, - org, - async cleanup() { - await orgCleanup(); - await shutdown(); - }, - }; + return { org }; }); testWithFixture( @@ -50,15 +40,11 @@ describe('kyselyUserFindByEmailAndOrg', () => { async ({ deps, org }) => { const input = samlUserInput(org.id); await kyselyUserInsert({ db: deps.KyselyPg, ...input }); - try { - const result = await kyselyUserFindByEmailAndOrg(deps.KyselyPg, { - email: input.email, - orgId: org.id, - }); - expect(result).toMatchObject({ id: input.id, orgId: org.id }); - } finally { - await kyselyUserDeleteById(deps.KyselyPg, input.id); - } + const result = await kyselyUserFindByEmailAndOrg(deps.KyselyPg, { + email: input.email, + orgId: org.id, + }); + expect(result).toMatchObject({ id: input.id, orgId: org.id }); }, ); @@ -69,15 +55,11 @@ describe('kyselyUserFindByEmailAndOrg', () => { async ({ deps, org }) => { const input = samlUserInput(org.id); await kyselyUserInsert({ db: deps.KyselyPg, ...input }); - try { - const result = await kyselyUserFindByEmailAndOrg(deps.KyselyPg, { - email: input.email, - orgId: `different-org-${uid()}`, - }); - expect(result).toBeUndefined(); - } finally { - await kyselyUserDeleteById(deps.KyselyPg, input.id); - } + const result = await kyselyUserFindByEmailAndOrg(deps.KyselyPg, { + email: input.email, + orgId: `different-org-${uid()}`, + }); + expect(result).toBeUndefined(); }, ); diff --git a/server/graphql/utils/resolveSamlUser.test.ts b/server/graphql/utils/resolveSamlUser.test.ts index c28154c..3c7d017 100644 --- a/server/graphql/utils/resolveSamlUser.test.ts +++ b/server/graphql/utils/resolveSamlUser.test.ts @@ -5,11 +5,9 @@ import { uid } from 'uid'; import { UserRole } from '../../services/userManagementService/index.js'; import createOrg from '../../test/fixtureHelpers/createOrg.js'; -import { makeMockedServer } from '../../test/setupMockedServer.js'; -import { makeTestWithFixture } from '../../test/utils.js'; +import { makeTransactionalTestWithFixture } from '../../test/harness/transactionalTest.js'; import { type default as SafeTracer } from '../../utils/SafeTracer.js'; import { - kyselyUserDeleteById, kyselyUserInsert, type UsersDb, } from '../datasources/userKyselyPersistence.js'; @@ -33,9 +31,8 @@ function samlUserInput(orgId: string) { } describe('resolveSamlUser', () => { - const testWithFixture = makeTestWithFixture(async () => { - const { deps, shutdown } = await makeMockedServer(); - const { org, cleanup: orgCleanup } = await createOrg( + const testWithFixture = makeTransactionalTestWithFixture(async ({ deps }) => { + const { org } = await createOrg( { KyselyPg: deps.KyselyPg, ModerationConfigService: deps.ModerationConfigService, @@ -43,14 +40,7 @@ describe('resolveSamlUser', () => { }, uid(), ); - return { - deps, - org, - async cleanup() { - await orgCleanup(); - await shutdown(); - }, - }; + return { org }; }); testWithFixture( @@ -59,21 +49,17 @@ describe('resolveSamlUser', () => { const input = samlUserInput(org.id); await kyselyUserInsert({ db: deps.KyselyPg, ...input }); const done = jest.fn(); - try { - await resolveSamlUser( - deps.KyselyPg, - deps.Tracer, - makeReq(org.id), - { email: input.email }, - done, - ); - expect(done).toHaveBeenCalledTimes(1); - const [err, user] = done.mock.calls[0]; - expect(err).toBeNull(); - expect(user).toMatchObject({ id: input.id, orgId: org.id }); - } finally { - await kyselyUserDeleteById(deps.KyselyPg, input.id); - } + await resolveSamlUser( + deps.KyselyPg, + deps.Tracer, + makeReq(org.id), + { email: input.email }, + done, + ); + expect(done).toHaveBeenCalledTimes(1); + const [err, user] = done.mock.calls[0]; + expect(err).toBeNull(); + expect(user).toMatchObject({ id: input.id, orgId: org.id }); }, ); @@ -85,20 +71,16 @@ describe('resolveSamlUser', () => { const input = samlUserInput(org.id); await kyselyUserInsert({ db: deps.KyselyPg, ...input }); const done = jest.fn(); - try { - await resolveSamlUser( - deps.KyselyPg, - deps.Tracer, - makeReq(`different-org-${uid()}`), - { email: input.email }, - done, - ); - const [err, user] = done.mock.calls[0]; - expect(err).toBeInstanceOf(Error); - expect(user).toBeUndefined(); - } finally { - await kyselyUserDeleteById(deps.KyselyPg, input.id); - } + await resolveSamlUser( + deps.KyselyPg, + deps.Tracer, + makeReq(`different-org-${uid()}`), + { email: input.email }, + done, + ); + const [err, user] = done.mock.calls[0]; + expect(err).toBeInstanceOf(Error); + expect(user).toBeUndefined(); }, ); diff --git a/server/services/manualReviewToolService/manualReviewToolService.test.ts b/server/services/manualReviewToolService/manualReviewToolService.test.ts index 7329309..8c100b1 100644 --- a/server/services/manualReviewToolService/manualReviewToolService.test.ts +++ b/server/services/manualReviewToolService/manualReviewToolService.test.ts @@ -1,7 +1,12 @@ /* eslint-disable max-lines */ +import { uid } from 'uid'; import { v1 as uuidv1 } from 'uuid'; -import getBottle, { type Dependencies } from '../../iocContainer/index.js'; +import createMrtQueue from '../../test/fixtureHelpers/createMrtQueue.js'; +import createOrg from '../../test/fixtureHelpers/createOrg.js'; +import createUser from '../../test/fixtureHelpers/createUser.js'; +import { makeTransactionalTestWithFixture } from '../../test/harness/transactionalTest.js'; +import { type MockedServer } from '../../test/setupMockedServer.js'; import { instantiateOpaqueType } from '../../utils/typescript-types.js'; import { makeSubmissionId, @@ -16,6 +21,8 @@ import { import { AUTOMATED_DECISION_REVIEWER_ID } from './modules/JobDecisioning.js'; import { jobIdToGuid } from './modules/QueueOperations.js'; +type TestDeps = MockedServer['deps']; + function makeDummyJob() { return { createdAt: new Date(), @@ -73,145 +80,128 @@ function makeDummyNcmecJob() { }; } -describe('Manual Review Tool Service', () => { - let mrtService: ManualReviewToolService; - let container: Dependencies; - - beforeAll(async () => { - // The mutation should be ok here since this is initial setup in a - // beforeAll; it doesn't involve reset state for each test in the suite - - ({ container } = await getBottle()); - mrtService = container.ManualReviewToolService; - }); - - afterAll(async () => { - await container.closeSharedResourcesForShutdown(); - }); +async function configureDecisionReasonRequirements( + mrtService: ManualReviewToolService, + orgId: string, + opts: { + onAction?: boolean; + onIgnore?: boolean; + }, +) { + if (opts.onAction !== undefined) { + await mrtService.updateRequiresDecisionReason(orgId, opts.onAction); + } + if (opts.onIgnore !== undefined) { + await mrtService.updateRequiresDecisionReasonOnIgnore(orgId, opts.onIgnore); + } +} - // Test that we can start the stalled jobs checker for manual job processing - test('should be able to start stalled jobs checker', async () => { - const worker = await mrtService['queueOps']['getBullWorker']({ - orgId: 'dummyOrg', - queueId: 'dummyQueue', - }); - // The startStalledCheckTimer method should be available and not throw - expect(worker).toBeDefined(); - }); +async function setRequiresPolicyForDecisions( + mrtService: ManualReviewToolService, + db: TestDeps['KyselyPg'], + orgId: string, + value: boolean, +) { + await mrtService.upsertDefaultSettings({ orgId }); + await db + .updateTable('manual_review_tool.manual_review_tool_settings') + .set({ requires_policy_for_decisions: value }) + .where('org_id', '=', orgId) + .execute(); +} - // TODO: rework when we rework the MRT error handling - test.skip('MRT throws for submitting a job that has already been moved to completed', async () => { - const orgId = 'e7c89ce7729', - queueId = '1', - reviewerId = uuidv1(), - reviewerEmail = 'test@test.com', - itemId = uuidv1(), - itemTypeId = uuidv1(); - - await mrtService['queueOps']['addJob']({ - queueId, - enqueueSourceInfo: { kind: 'REPORT' }, - jobPayload: { - createdAt: new Date(), - payload: { - kind: 'DEFAULT', - reportHistory: [], - item: instantiateOpaqueType({ - submissionId: makeSubmissionId(), - // eslint-disable-next-line @typescript-eslint/consistent-type-assertions - data: {} as NormalizedItemData, - itemTypeIdentifier: { - id: itemTypeId, - version: new Date().toISOString(), - schemaVariant: 'original', - }, - creator: { - id: uuidv1(), - typeId: uuidv1(), - }, - itemId, - }), - reportedForReason: undefined, - reportedForReasons: [], - enqueueSourceInfo: { kind: 'REPORT' }, - }, - policyIds: [], +describe('Manual Review Tool Service', () => { + // Just the service — for cases that don't need any org-scoped fixtures. + const testWithService = makeTransactionalTestWithFixture( + async ({ deps }) => ({ + mrtService: deps.ManualReviewToolService, + }), + ); + + // A fresh org with a queue and a CUSTOM_ACTION, so decision tests can enqueue + // a job and submit a real (validatable) action without relying on seed data. + const testWithQueue = makeTransactionalTestWithFixture(async ({ deps }) => { + const mrtService = deps.ManualReviewToolService; + const { org } = await createOrg( + { + KyselyPg: deps.KyselyPg, + ModerationConfigService: deps.ModerationConfigService, + ApiKeyService: deps.ApiKeyService, }, - orgId, - }); - - const dequeuedJob = await mrtService.dequeueNextJob({ - orgId, - queueId, - userId: reviewerId, + uid(), + ); + const { user } = await createUser(deps.KyselyPg, org.id); + const { queue } = await createMrtQueue({ + orgId: org.id, + mrtService, + userId: user.id, }); - - if (!dequeuedJob) { - throw new Error('should have dequeued successfully.'); - } - - await mrtService.submitDecision({ - queueId, - reportHistory: [], - jobId: dequeuedJob.job.id, - lockToken: dequeuedJob.lockToken, - decisionComponents: [ - { - type: 'CUSTOM_ACTION', - actions: [{ id: '8481310e8c4' }], - policies: [], - itemIds: [itemId], - itemTypeId, - }, - ], - relatedActions: [], - reviewerId, - reviewerEmail, - orgId, + const action = await deps.ModerationConfigService.createAction(org.id, { + name: `mrt-test-action-${uid()}`, + description: null, + type: 'CUSTOM_ACTION', + callbackUrl: 'https://example.com', + callbackUrlHeaders: null, + callbackUrlBody: null, }); - const duplicativeDecision = async () => { - return mrtService.submitDecision({ - queueId, - reportHistory: [], - jobId: dequeuedJob.job.id, - lockToken: dequeuedJob.lockToken, - decisionComponents: [ - { - type: 'CUSTOM_ACTION', - actions: [{ id: '8481310e8c4' }], - policies: [], - itemIds: [itemId], - itemTypeId, - }, - ], - relatedActions: [], - reviewerId, - reviewerEmail, - orgId, - }); - }; - - await expect(duplicativeDecision()).rejects.toThrow( - `No job with ID ${dequeuedJob.job.id} in queue with ID ${queueId}`, - ); + return { mrtService, org, user, queue, actionId: action.id }; }); - describe('duplicate decision handling', () => { - it('should reject duplicate decisions with the same lock token', async () => { - const orgId = 'e7c89ce7729', + // Test that we can start the stalled jobs checker for manual job processing + testWithService( + 'should be able to start stalled jobs checker', + async ({ mrtService }) => { + const worker = await mrtService['queueOps']['getBullWorker']({ + orgId: 'dummyOrg', + queueId: 'dummyQueue', + }); + // The startStalledCheckTimer method should be available and not throw + expect(worker).toBeDefined(); + }, + ); + + // TODO: rework when we rework the MRT error handling + testWithService.skip( + 'MRT throws for submitting a job that has already been moved to completed', + async ({ mrtService }) => { + const orgId = uid(), queueId = '1', reviewerId = uuidv1(), reviewerEmail = 'test@test.com', - jobPayload = makeDummyJob(); - const itemId = jobPayload.payload.item.itemId, - itemTypeId = jobPayload.payload.item.itemTypeIdentifier.id; + itemId = uuidv1(), + itemTypeId = uuidv1(); await mrtService['queueOps']['addJob']({ - jobPayload, - orgId, queueId, enqueueSourceInfo: { kind: 'REPORT' }, + jobPayload: { + createdAt: new Date(), + payload: { + kind: 'DEFAULT', + reportHistory: [], + item: instantiateOpaqueType({ + submissionId: makeSubmissionId(), + // eslint-disable-next-line @typescript-eslint/consistent-type-assertions + data: {} as NormalizedItemData, + itemTypeIdentifier: { + id: itemTypeId, + version: new Date().toISOString(), + schemaVariant: 'original', + }, + creator: { + id: uuidv1(), + typeId: uuidv1(), + }, + itemId, + }), + reportedForReason: undefined, + reportedForReasons: [], + enqueueSourceInfo: { kind: 'REPORT' }, + }, + policyIds: [], + }, + orgId, }); const dequeuedJob = await mrtService.dequeueNextJob({ @@ -221,7 +211,7 @@ describe('Manual Review Tool Service', () => { }); if (!dequeuedJob) { - throw new Error("should've returned a job"); + throw new Error('should have dequeued successfully.'); } await mrtService.submitDecision({ @@ -245,7 +235,7 @@ describe('Manual Review Tool Service', () => { }); const duplicativeDecision = async () => { - await mrtService.submitDecision({ + return mrtService.submitDecision({ queueId, reportHistory: [], jobId: dequeuedJob.job.id, @@ -266,8 +256,86 @@ describe('Manual Review Tool Service', () => { }); }; - await expect(duplicativeDecision()).rejects.toThrow(); - }); + await expect(duplicativeDecision()).rejects.toThrow( + `No job with ID ${dequeuedJob.job.id} in queue with ID ${queueId}`, + ); + }, + ); + + describe('duplicate decision handling', () => { + testWithQueue( + 'should reject duplicate decisions with the same lock token', + async ({ mrtService, org, queue, actionId }) => { + const orgId = org.id, + queueId = queue.id, + reviewerId = uuidv1(), + reviewerEmail = 'test@test.com', + jobPayload = makeDummyJob(); + const itemId = jobPayload.payload.item.itemId, + itemTypeId = jobPayload.payload.item.itemTypeIdentifier.id; + + await mrtService['queueOps']['addJob']({ + jobPayload, + orgId, + queueId, + enqueueSourceInfo: { kind: 'REPORT' }, + }); + + const dequeuedJob = await mrtService.dequeueNextJob({ + orgId, + queueId, + userId: reviewerId, + }); + + if (!dequeuedJob) { + throw new Error("should've returned a job"); + } + + await mrtService.submitDecision({ + queueId, + reportHistory: [], + jobId: dequeuedJob.job.id, + lockToken: dequeuedJob.lockToken, + decisionComponents: [ + { + type: 'CUSTOM_ACTION', + actions: [{ id: actionId }], + policies: [], + itemIds: [itemId], + itemTypeId, + }, + ], + relatedActions: [], + reviewerId, + reviewerEmail, + orgId, + }); + + const duplicativeDecision = async () => { + await mrtService.submitDecision({ + queueId, + reportHistory: [], + jobId: dequeuedJob.job.id, + lockToken: dequeuedJob.lockToken, + decisionComponents: [ + { + type: 'CUSTOM_ACTION', + actions: [{ id: actionId }], + policies: [], + itemIds: [itemId], + itemTypeId, + }, + ], + relatedActions: [], + reviewerId, + reviewerEmail, + orgId, + }); + }; + + await expect(duplicativeDecision()).rejects.toThrow(); + }, + ); it.skip('should reject duplicate decisions on jobs dequeued again after the lock expires', async () => {}); }); @@ -278,51 +346,54 @@ describe('Manual Review Tool Service', () => { // stuck in a retry loop. The decision must record the empty-string // reviewer id (rendered as "Automatic" client-side) and not throw. describe('automatic close decisions', () => { - it('records an AUTOMATIC_CLOSE decision with no human reviewer', async () => { - const orgId = 'e7c89ce7729'; - const queueId = '1'; - const jobPayload = makeDummyJob(); - - await mrtService['queueOps']['addJob']({ - jobPayload, - orgId, - queueId, - enqueueSourceInfo: { kind: 'REPORT' }, - }); + testWithQueue( + 'records an AUTOMATIC_CLOSE decision with no human reviewer', + async ({ mrtService, org, queue }) => { + const orgId = org.id, + queueId = queue.id, + jobPayload = makeDummyJob(); + + await mrtService['queueOps']['addJob']({ + jobPayload, + orgId, + queueId, + enqueueSourceInfo: { kind: 'REPORT' }, + }); - const dequeuedJob = await mrtService.dequeueNextJob({ - orgId, - queueId, - userId: uuidv1(), - }); + const dequeuedJob = await mrtService.dequeueNextJob({ + orgId, + queueId, + userId: uuidv1(), + }); - if (!dequeuedJob) { - throw new Error("should've returned a job"); - } + if (!dequeuedJob) { + throw new Error("should've returned a job"); + } - // Used to throw 23502 (null reviewer_id) before the sentinel fix. - await mrtService.submitDecision({ - queueId, - reportHistory: [], - jobId: dequeuedJob.job.id, - lockToken: dequeuedJob.lockToken, - relatedActions: [], - orgId, - automaticCloseDecision: { - type: 'AUTOMATIC_CLOSE', - reason: 'ITEM_DELETED_BEFORE_REVIEW', - }, - }); + // Used to throw 23502 (null reviewer_id) before the sentinel fix. + await mrtService.submitDecision({ + queueId, + reportHistory: [], + jobId: dequeuedJob.job.id, + lockToken: dequeuedJob.lockToken, + relatedActions: [], + orgId, + automaticCloseDecision: { + type: 'AUTOMATIC_CLOSE', + reason: 'ITEM_DELETED_BEFORE_REVIEW', + }, + }); - const row = await mrtService['pgQuery'] - .selectFrom('manual_review_tool.manual_review_decisions') - .where('id', '=', jobIdToGuid(dequeuedJob.job.id)) - .where('org_id', '=', orgId) - .select(['reviewer_id']) - .executeTakeFirst(); + const row = await mrtService['pgQuery'] + .selectFrom('manual_review_tool.manual_review_decisions') + .where('id', '=', jobIdToGuid(dequeuedJob.job.id)) + .where('org_id', '=', orgId) + .select(['reviewer_id']) + .executeTakeFirst(); - expect(row?.reviewer_id).toBe(AUTOMATED_DECISION_REVIEWER_ID); - }); + expect(row?.reviewer_id).toBe(AUTOMATED_DECISION_REVIEWER_ID); + }, + ); }); // Issue #616: when an org sets `mrt_requires_decision_reason_on_action`, @@ -335,91 +406,100 @@ describe('Manual Review Tool Service', () => { // (`..._on_ignore`) — so the cases below also cover that an IGNORE decision is // gated by the ignore flag, not the action flag. describe('requires_decision_reason enforcement', () => { - const orgId = 'e7c89ce7729'; - const queueId = '1'; - // Pulled from the staging seed data — any CUSTOM_ACTION row on this org - // will do; the action-id validation runs before our reason check. - const seededActionId = '1873b2f15cc'; - - const setRequiresDecisionReason = async (value: boolean) => { - await mrtService.upsertDefaultSettings({ orgId }); - await mrtService['pgQuery'] - .updateTable('manual_review_tool.manual_review_tool_settings') - .set({ mrt_requires_decision_reason_on_action: value }) - .where('org_id', '=', orgId) - .execute(); - }; - - const setRequiresDecisionReasonOnIgnore = async (value: boolean) => { - await mrtService.upsertDefaultSettings({ orgId }); - await mrtService['pgQuery'] - .updateTable('manual_review_tool.manual_review_tool_settings') - .set({ mrt_requires_decision_reason_on_ignore: value }) - .where('org_id', '=', orgId) - .execute(); - }; - - beforeAll(async () => { - // The queue row must exist before addJob, but no other test in this - // file owns its lifecycle, so we seed it here idempotently. - await mrtService['pgQuery'] - .insertInto('manual_review_tool.manual_review_queues') - .values({ - id: queueId, - name: 'integ-test-queue', - description: null, - org_id: orgId, - is_default_queue: false, - is_appeals_queue: false, - auto_close_jobs: false, - }) - .onConflict((oc) => oc.doNothing()) - .execute(); - }); + testWithQueue( + 'rejects a decision with no reason when the flag is on', + async ({ mrtService, org, queue, actionId }) => { + await configureDecisionReasonRequirements(mrtService, org.id, { + onAction: true, + }); - afterEach(async () => { - // Reset so the flags don't leak into other tests in this file or - // subsequent runs that reuse the seeded org. - await setRequiresDecisionReason(false); - await setRequiresDecisionReasonOnIgnore(false); - }); + const reviewerId = uuidv1(); + const reviewerEmail = 'test@test.com'; + const jobPayload = makeDummyJob(); + const itemId = jobPayload.payload.item.itemId; + const itemTypeId = jobPayload.payload.item.itemTypeIdentifier.id; - it('rejects a decision with no reason when the flag is on', async () => { - await setRequiresDecisionReason(true); + await mrtService['queueOps']['addJob']({ + jobPayload, + orgId: org.id, + queueId: queue.id, + enqueueSourceInfo: { kind: 'REPORT' }, + }); - const reviewerId = uuidv1(); - const reviewerEmail = 'test@test.com'; - const jobPayload = makeDummyJob(); - const itemId = jobPayload.payload.item.itemId; - const itemTypeId = jobPayload.payload.item.itemTypeIdentifier.id; + const dequeuedJob = await mrtService.dequeueNextJob({ + orgId: org.id, + queueId: queue.id, + userId: reviewerId, + }); - await mrtService['queueOps']['addJob']({ - jobPayload, - orgId, - queueId, - enqueueSourceInfo: { kind: 'REPORT' }, - }); + if (!dequeuedJob) { + throw new Error("should've returned a job"); + } + + await expect( + mrtService.submitDecision({ + queueId: queue.id, + reportHistory: [], + jobId: dequeuedJob.job.id, + lockToken: dequeuedJob.lockToken, + decisionComponents: [ + { + type: 'CUSTOM_ACTION', + actions: [{ id: actionId }], + policies: [{ id: uuidv1() }], + itemIds: [itemId], + itemTypeId, + }, + ], + relatedActions: [], + reviewerId, + reviewerEmail, + orgId: org.id, + // decisionReason intentionally omitted + }), + ).rejects.toThrow(/requires every decision to include a reason/i); + }, + ); - const dequeuedJob = await mrtService.dequeueNextJob({ - orgId, - queueId, - userId: reviewerId, - }); + testWithQueue( + 'allows a decision with a reason when the flag is on', + async ({ mrtService, org, queue, actionId }) => { + await configureDecisionReasonRequirements(mrtService, org.id, { + onAction: true, + }); - if (!dequeuedJob) { - throw new Error("should've returned a job"); - } + const reviewerId = uuidv1(); + const reviewerEmail = 'test@test.com'; + const jobPayload = makeDummyJob(); + const itemId = jobPayload.payload.item.itemId; + const itemTypeId = jobPayload.payload.item.itemTypeIdentifier.id; - await expect( - mrtService.submitDecision({ - queueId, + await mrtService['queueOps']['addJob']({ + jobPayload, + orgId: org.id, + queueId: queue.id, + enqueueSourceInfo: { kind: 'REPORT' }, + }); + + const dequeuedJob = await mrtService.dequeueNextJob({ + orgId: org.id, + queueId: queue.id, + userId: reviewerId, + }); + + if (!dequeuedJob) { + throw new Error("should've returned a job"); + } + + await mrtService.submitDecision({ + queueId: queue.id, reportHistory: [], jobId: dequeuedJob.job.id, lockToken: dequeuedJob.lockToken, decisionComponents: [ { type: 'CUSTOM_ACTION', - actions: [{ id: seededActionId }], + actions: [{ id: actionId }], policies: [{ id: uuidv1() }], itemIds: [itemId], itemTypeId, @@ -428,140 +508,148 @@ describe('Manual Review Tool Service', () => { relatedActions: [], reviewerId, reviewerEmail, - orgId, - // decisionReason intentionally omitted - }), - ).rejects.toThrow(/requires every decision to include a reason/i); - }); + orgId: org.id, + decisionReason: 'Repeat offender', + }); + }, + ); - it('allows a decision with a reason when the flag is on', async () => { - await setRequiresDecisionReason(true); + testWithQueue( + 'allows a decision with no reason when the flag is off', + async ({ mrtService, org, queue, actionId }) => { + // Control case: default-off behavior must remain unchanged so orgs that + // never opt in see no difference from this PR. + await configureDecisionReasonRequirements(mrtService, org.id, { + onAction: false, + }); - const reviewerId = uuidv1(); - const reviewerEmail = 'test@test.com'; - const jobPayload = makeDummyJob(); - const itemId = jobPayload.payload.item.itemId; - const itemTypeId = jobPayload.payload.item.itemTypeIdentifier.id; + const reviewerId = uuidv1(); + const reviewerEmail = 'test@test.com'; + const jobPayload = makeDummyJob(); + const itemId = jobPayload.payload.item.itemId; + const itemTypeId = jobPayload.payload.item.itemTypeIdentifier.id; - await mrtService['queueOps']['addJob']({ - jobPayload, - orgId, - queueId, - enqueueSourceInfo: { kind: 'REPORT' }, - }); + await mrtService['queueOps']['addJob']({ + jobPayload, + orgId: org.id, + queueId: queue.id, + enqueueSourceInfo: { kind: 'REPORT' }, + }); - const dequeuedJob = await mrtService.dequeueNextJob({ - orgId, - queueId, - userId: reviewerId, - }); + const dequeuedJob = await mrtService.dequeueNextJob({ + orgId: org.id, + queueId: queue.id, + userId: reviewerId, + }); - if (!dequeuedJob) { - throw new Error("should've returned a job"); - } + if (!dequeuedJob) { + throw new Error("should've returned a job"); + } - await mrtService.submitDecision({ - queueId, - reportHistory: [], - jobId: dequeuedJob.job.id, - lockToken: dequeuedJob.lockToken, - decisionComponents: [ - { - type: 'CUSTOM_ACTION', - actions: [{ id: seededActionId }], - policies: [{ id: uuidv1() }], - itemIds: [itemId], - itemTypeId, - }, - ], - relatedActions: [], - reviewerId, - reviewerEmail, - orgId, - decisionReason: 'Repeat offender', - }); - }); + await mrtService.submitDecision({ + queueId: queue.id, + reportHistory: [], + jobId: dequeuedJob.job.id, + lockToken: dequeuedJob.lockToken, + decisionComponents: [ + { + type: 'CUSTOM_ACTION', + actions: [{ id: actionId }], + policies: [{ id: uuidv1() }], + itemIds: [itemId], + itemTypeId, + }, + ], + relatedActions: [], + reviewerId, + reviewerEmail, + orgId: org.id, + }); + }, + ); - it('allows a decision with no reason when the flag is off', async () => { - // Control case: default-off behavior must remain unchanged so orgs that - // never opt in see no difference from this PR. - await setRequiresDecisionReason(false); + // Issue #757: an IGNORE decision is gated by the ignore flag, not the + // action flag. With only the ignore flag on, an IGNORE with no reason is + // rejected. + testWithQueue( + 'rejects an IGNORE decision with no reason when only the ignore flag is on', + async ({ mrtService, org, queue }) => { + await configureDecisionReasonRequirements(mrtService, org.id, { + onAction: false, + onIgnore: true, + }); - const reviewerId = uuidv1(); - const reviewerEmail = 'test@test.com'; - const jobPayload = makeDummyJob(); - const itemId = jobPayload.payload.item.itemId; - const itemTypeId = jobPayload.payload.item.itemTypeIdentifier.id; + const reviewerId = uuidv1(); + const reviewerEmail = 'test@test.com'; + const jobPayload = makeDummyJob(); - await mrtService['queueOps']['addJob']({ - jobPayload, - orgId, - queueId, - enqueueSourceInfo: { kind: 'REPORT' }, - }); + await mrtService['queueOps']['addJob']({ + jobPayload, + orgId: org.id, + queueId: queue.id, + enqueueSourceInfo: { kind: 'REPORT' }, + }); - const dequeuedJob = await mrtService.dequeueNextJob({ - orgId, - queueId, - userId: reviewerId, - }); + const dequeuedJob = await mrtService.dequeueNextJob({ + orgId: org.id, + queueId: queue.id, + userId: reviewerId, + }); - if (!dequeuedJob) { - throw new Error("should've returned a job"); - } + if (!dequeuedJob) { + throw new Error("should've returned a job"); + } + + await expect( + mrtService.submitDecision({ + queueId: queue.id, + reportHistory: [], + jobId: dequeuedJob.job.id, + lockToken: dequeuedJob.lockToken, + decisionComponents: [{ type: 'IGNORE' }], + relatedActions: [], + reviewerId, + reviewerEmail, + orgId: org.id, + // decisionReason intentionally omitted + }), + ).rejects.toThrow(/requires every decision to include a reason/i); + }, + ); - await mrtService.submitDecision({ - queueId, - reportHistory: [], - jobId: dequeuedJob.job.id, - lockToken: dequeuedJob.lockToken, - decisionComponents: [ - { - type: 'CUSTOM_ACTION', - actions: [{ id: seededActionId }], - policies: [{ id: uuidv1() }], - itemIds: [itemId], - itemTypeId, - }, - ], - relatedActions: [], - reviewerId, - reviewerEmail, - orgId, - }); - }); - - // Issue #757: an IGNORE decision is gated by the ignore flag, not the - // action flag. With only the ignore flag on, an IGNORE with no reason is - // rejected. - it('rejects an IGNORE decision with no reason when only the ignore flag is on', async () => { - await setRequiresDecisionReason(false); - await setRequiresDecisionReasonOnIgnore(true); + // Issue #757: with only the action flag on, ignoring a job must NOT require + // a reason — this is the bug from the issue. + testWithQueue( + 'allows an IGNORE decision with no reason when only the action flag is on', + async ({ mrtService, org, queue }) => { + await configureDecisionReasonRequirements(mrtService, org.id, { + onAction: true, + onIgnore: false, + }); - const reviewerId = uuidv1(); - const reviewerEmail = 'test@test.com'; - const jobPayload = makeDummyJob(); + const reviewerId = uuidv1(); + const reviewerEmail = 'test@test.com'; + const jobPayload = makeDummyJob(); - await mrtService['queueOps']['addJob']({ - jobPayload, - orgId, - queueId, - enqueueSourceInfo: { kind: 'REPORT' }, - }); + await mrtService['queueOps']['addJob']({ + jobPayload, + orgId: org.id, + queueId: queue.id, + enqueueSourceInfo: { kind: 'REPORT' }, + }); - const dequeuedJob = await mrtService.dequeueNextJob({ - orgId, - queueId, - userId: reviewerId, - }); + const dequeuedJob = await mrtService.dequeueNextJob({ + orgId: org.id, + queueId: queue.id, + userId: reviewerId, + }); - if (!dequeuedJob) { - throw new Error("should've returned a job"); - } + if (!dequeuedJob) { + throw new Error("should've returned a job"); + } - await expect( - mrtService.submitDecision({ - queueId, + await mrtService.submitDecision({ + queueId: queue.id, reportHistory: [], jobId: dequeuedJob.job.id, lockToken: dequeuedJob.lockToken, @@ -569,224 +657,223 @@ describe('Manual Review Tool Service', () => { relatedActions: [], reviewerId, reviewerEmail, - orgId, + orgId: org.id, // decisionReason intentionally omitted - }), - ).rejects.toThrow(/requires every decision to include a reason/i); - }); - - // Issue #757: with only the action flag on, ignoring a job must NOT require - // a reason — this is the bug from the issue. - it('allows an IGNORE decision with no reason when only the action flag is on', async () => { - await setRequiresDecisionReason(true); - await setRequiresDecisionReasonOnIgnore(false); - - const reviewerId = uuidv1(); - const reviewerEmail = 'test@test.com'; - const jobPayload = makeDummyJob(); - - await mrtService['queueOps']['addJob']({ - jobPayload, - orgId, - queueId, - enqueueSourceInfo: { kind: 'REPORT' }, - }); - - const dequeuedJob = await mrtService.dequeueNextJob({ - orgId, - queueId, - userId: reviewerId, - }); - - if (!dequeuedJob) { - throw new Error("should've returned a job"); - } - - await mrtService.submitDecision({ - queueId, - reportHistory: [], - jobId: dequeuedJob.job.id, - lockToken: dequeuedJob.lockToken, - decisionComponents: [{ type: 'IGNORE' }], - relatedActions: [], - reviewerId, - reviewerEmail, - orgId, - // decisionReason intentionally omitted - }); - }); + }); + }, + ); // Issue #757: with only the ignore flag on, acting on a violating job must // NOT require a reason. - it('allows a CUSTOM_ACTION decision with no reason when only the ignore flag is on', async () => { - await setRequiresDecisionReason(false); - await setRequiresDecisionReasonOnIgnore(true); + testWithQueue( + 'allows a CUSTOM_ACTION decision with no reason when only the ignore flag is on', + async ({ mrtService, org, queue, actionId }) => { + await configureDecisionReasonRequirements(mrtService, org.id, { + onAction: false, + onIgnore: true, + }); - const reviewerId = uuidv1(); - const reviewerEmail = 'test@test.com'; - const jobPayload = makeDummyJob(); - const itemId = jobPayload.payload.item.itemId; - const itemTypeId = jobPayload.payload.item.itemTypeIdentifier.id; + const reviewerId = uuidv1(); + const reviewerEmail = 'test@test.com'; + const jobPayload = makeDummyJob(); + const itemId = jobPayload.payload.item.itemId; + const itemTypeId = jobPayload.payload.item.itemTypeIdentifier.id; - await mrtService['queueOps']['addJob']({ - jobPayload, - orgId, - queueId, - enqueueSourceInfo: { kind: 'REPORT' }, - }); + await mrtService['queueOps']['addJob']({ + jobPayload, + orgId: org.id, + queueId: queue.id, + enqueueSourceInfo: { kind: 'REPORT' }, + }); - const dequeuedJob = await mrtService.dequeueNextJob({ - orgId, - queueId, - userId: reviewerId, - }); + const dequeuedJob = await mrtService.dequeueNextJob({ + orgId: org.id, + queueId: queue.id, + userId: reviewerId, + }); - if (!dequeuedJob) { - throw new Error("should've returned a job"); - } + if (!dequeuedJob) { + throw new Error("should've returned a job"); + } - await mrtService.submitDecision({ - queueId, - reportHistory: [], - jobId: dequeuedJob.job.id, - lockToken: dequeuedJob.lockToken, - decisionComponents: [ - { - type: 'CUSTOM_ACTION', - actions: [{ id: seededActionId }], - policies: [{ id: uuidv1() }], - itemIds: [itemId], - itemTypeId, - }, - ], - relatedActions: [], - reviewerId, - reviewerEmail, - orgId, - // decisionReason intentionally omitted - }); - }); + await mrtService.submitDecision({ + queueId: queue.id, + reportHistory: [], + jobId: dequeuedJob.job.id, + lockToken: dequeuedJob.lockToken, + decisionComponents: [ + { + type: 'CUSTOM_ACTION', + actions: [{ id: actionId }], + policies: [{ id: uuidv1() }], + itemIds: [itemId], + itemTypeId, + }, + ], + relatedActions: [], + reviewerId, + reviewerEmail, + orgId: org.id, + // decisionReason intentionally omitted + }); + }, + ); // Issue #736: NCMEC review uses Submit NCMEC Report or Ignore, neither of // which carries a written decision reason. The require-reason flag is for // moderation decisions on standard MRT jobs and should not block the // NCMEC path. - it('allows an IGNORE decision on an NCMEC job with no reason when the flag is on', async () => { - await setRequiresDecisionReason(true); - await setRequiresDecisionReasonOnIgnore(true); + testWithQueue( + 'allows an IGNORE decision on an NCMEC job with no reason when the flag is on', + async ({ mrtService, org, queue }) => { + await configureDecisionReasonRequirements(mrtService, org.id, { + onAction: true, + onIgnore: true, + }); - const reviewerId = uuidv1(); - const reviewerEmail = 'test@test.com'; - const jobPayload = makeDummyNcmecJob(); + const reviewerId = uuidv1(); + const reviewerEmail = 'test@test.com'; + const jobPayload = makeDummyNcmecJob(); - await mrtService['queueOps']['addJob']({ - jobPayload, - orgId, - queueId, - enqueueSourceInfo: { kind: 'REPORT' }, - }); + await mrtService['queueOps']['addJob']({ + jobPayload, + orgId: org.id, + queueId: queue.id, + enqueueSourceInfo: { kind: 'REPORT' }, + }); - const dequeuedJob = await mrtService.dequeueNextJob({ - orgId, - queueId, - userId: reviewerId, - }); + const dequeuedJob = await mrtService.dequeueNextJob({ + orgId: org.id, + queueId: queue.id, + userId: reviewerId, + }); - if (!dequeuedJob) { - throw new Error("should've returned a job"); - } + if (!dequeuedJob) { + throw new Error("should've returned a job"); + } - await mrtService.submitDecision({ - queueId, - reportHistory: [], - jobId: dequeuedJob.job.id, - lockToken: dequeuedJob.lockToken, - decisionComponents: [{ type: 'IGNORE' }], - relatedActions: [], - reviewerId, - reviewerEmail, - orgId, - // decisionReason intentionally omitted - }); - }); + await mrtService.submitDecision({ + queueId: queue.id, + reportHistory: [], + jobId: dequeuedJob.job.id, + lockToken: dequeuedJob.lockToken, + decisionComponents: [{ type: 'IGNORE' }], + relatedActions: [], + reviewerId, + reviewerEmail, + orgId: org.id, + // decisionReason intentionally omitted + }); + }, + ); }); // Issue #389: when an org sets `requires_policy_for_decisions`, submitDecision // must reject CUSTOM_ACTION decisions with no policies. The UI already blocks // this; the server-side check closes the API-bypass gap. describe('requires_policy_for_decisions enforcement', () => { - const orgId = 'e7c89ce7729'; - const queueId = '1'; - // Pulled from the staging seed data — any CUSTOM_ACTION row on this org - // will do; the action-id validation runs before our flag check. - const seededActionId = '1873b2f15cc'; - - const setRequiresPolicyForDecisions = async (value: boolean) => { - await mrtService.upsertDefaultSettings({ orgId }); - await mrtService['pgQuery'] - .updateTable('manual_review_tool.manual_review_tool_settings') - .set({ requires_policy_for_decisions: value }) - .where('org_id', '=', orgId) - .execute(); - }; - - beforeAll(async () => { - await mrtService['pgQuery'] - .insertInto('manual_review_tool.manual_review_queues') - .values({ - id: queueId, - name: 'integ-test-queue', - description: null, - org_id: orgId, - is_default_queue: false, - is_appeals_queue: false, - auto_close_jobs: false, - }) - .onConflict((oc) => oc.doNothing()) - .execute(); - }); - - afterEach(async () => { - await setRequiresPolicyForDecisions(false); - }); + testWithQueue( + 'rejects a CUSTOM_ACTION decision with no policies when the flag is on', + async ({ mrtService, deps, org, queue, actionId }) => { + await setRequiresPolicyForDecisions( + mrtService, + deps.KyselyPg, + org.id, + true, + ); + + const reviewerId = uuidv1(); + const reviewerEmail = 'test@test.com'; + const jobPayload = makeDummyJob(); + const itemId = jobPayload.payload.item.itemId; + const itemTypeId = jobPayload.payload.item.itemTypeIdentifier.id; + + await mrtService['queueOps']['addJob']({ + jobPayload, + orgId: org.id, + queueId: queue.id, + enqueueSourceInfo: { kind: 'REPORT' }, + }); - it('rejects a CUSTOM_ACTION decision with no policies when the flag is on', async () => { - await setRequiresPolicyForDecisions(true); + const dequeuedJob = await mrtService.dequeueNextJob({ + orgId: org.id, + queueId: queue.id, + userId: reviewerId, + }); - const reviewerId = uuidv1(); - const reviewerEmail = 'test@test.com'; - const jobPayload = makeDummyJob(); - const itemId = jobPayload.payload.item.itemId; - const itemTypeId = jobPayload.payload.item.itemTypeIdentifier.id; + if (!dequeuedJob) { + throw new Error("should've returned a job"); + } + + await expect( + mrtService.submitDecision({ + queueId: queue.id, + reportHistory: [], + jobId: dequeuedJob.job.id, + lockToken: dequeuedJob.lockToken, + decisionComponents: [ + { + type: 'CUSTOM_ACTION', + actions: [{ id: actionId }], + policies: [], + itemIds: [itemId], + itemTypeId, + }, + ], + relatedActions: [], + reviewerId, + reviewerEmail, + orgId: org.id, + }), + ).rejects.toThrow( + /requires every decision to include at least one policy/i, + ); + }, + ); - await mrtService['queueOps']['addJob']({ - jobPayload, - orgId, - queueId, - enqueueSourceInfo: { kind: 'REPORT' }, - }); + testWithQueue( + 'allows a CUSTOM_ACTION decision with policies when the flag is on', + async ({ mrtService, deps, org, queue, actionId }) => { + await setRequiresPolicyForDecisions( + mrtService, + deps.KyselyPg, + org.id, + true, + ); + + const reviewerId = uuidv1(); + const reviewerEmail = 'test@test.com'; + const jobPayload = makeDummyJob(); + const itemId = jobPayload.payload.item.itemId; + const itemTypeId = jobPayload.payload.item.itemTypeIdentifier.id; + + await mrtService['queueOps']['addJob']({ + jobPayload, + orgId: org.id, + queueId: queue.id, + enqueueSourceInfo: { kind: 'REPORT' }, + }); - const dequeuedJob = await mrtService.dequeueNextJob({ - orgId, - queueId, - userId: reviewerId, - }); + const dequeuedJob = await mrtService.dequeueNextJob({ + orgId: org.id, + queueId: queue.id, + userId: reviewerId, + }); - if (!dequeuedJob) { - throw new Error("should've returned a job"); - } + if (!dequeuedJob) { + throw new Error("should've returned a job"); + } - await expect( - mrtService.submitDecision({ - queueId, + await mrtService.submitDecision({ + queueId: queue.id, reportHistory: [], jobId: dequeuedJob.job.id, lockToken: dequeuedJob.lockToken, decisionComponents: [ { type: 'CUSTOM_ACTION', - actions: [{ id: seededActionId }], - policies: [], + actions: [{ id: actionId }], + policies: [{ id: uuidv1() }], itemIds: [itemId], itemTypeId, }, @@ -794,145 +881,113 @@ describe('Manual Review Tool Service', () => { relatedActions: [], reviewerId, reviewerEmail, - orgId, - }), - ).rejects.toThrow( - /requires every decision to include at least one policy/i, - ); - }); - - it('allows a CUSTOM_ACTION decision with policies when the flag is on', async () => { - await setRequiresPolicyForDecisions(true); - - const reviewerId = uuidv1(); - const reviewerEmail = 'test@test.com'; - const jobPayload = makeDummyJob(); - const itemId = jobPayload.payload.item.itemId; - const itemTypeId = jobPayload.payload.item.itemTypeIdentifier.id; - - await mrtService['queueOps']['addJob']({ - jobPayload, - orgId, - queueId, - enqueueSourceInfo: { kind: 'REPORT' }, - }); - - const dequeuedJob = await mrtService.dequeueNextJob({ - orgId, - queueId, - userId: reviewerId, - }); - - if (!dequeuedJob) { - throw new Error("should've returned a job"); - } - - await mrtService.submitDecision({ - queueId, - reportHistory: [], - jobId: dequeuedJob.job.id, - lockToken: dequeuedJob.lockToken, - decisionComponents: [ - { - type: 'CUSTOM_ACTION', - actions: [{ id: seededActionId }], - policies: [{ id: uuidv1() }], - itemIds: [itemId], - itemTypeId, - }, - ], - relatedActions: [], - reviewerId, - reviewerEmail, - orgId, - }); - }); - - it('allows a CUSTOM_ACTION decision without policies when the flag is off', async () => { - await setRequiresPolicyForDecisions(false); - - const reviewerId = uuidv1(); - const reviewerEmail = 'test@test.com'; - const jobPayload = makeDummyJob(); - const itemId = jobPayload.payload.item.itemId; - const itemTypeId = jobPayload.payload.item.itemTypeIdentifier.id; + orgId: org.id, + }); + }, + ); - await mrtService['queueOps']['addJob']({ - jobPayload, - orgId, - queueId, - enqueueSourceInfo: { kind: 'REPORT' }, - }); + testWithQueue( + 'allows a CUSTOM_ACTION decision without policies when the flag is off', + async ({ mrtService, deps, org, queue, actionId }) => { + await setRequiresPolicyForDecisions( + mrtService, + deps.KyselyPg, + org.id, + false, + ); + + const reviewerId = uuidv1(); + const reviewerEmail = 'test@test.com'; + const jobPayload = makeDummyJob(); + const itemId = jobPayload.payload.item.itemId; + const itemTypeId = jobPayload.payload.item.itemTypeIdentifier.id; + + await mrtService['queueOps']['addJob']({ + jobPayload, + orgId: org.id, + queueId: queue.id, + enqueueSourceInfo: { kind: 'REPORT' }, + }); - const dequeuedJob = await mrtService.dequeueNextJob({ - orgId, - queueId, - userId: reviewerId, - }); + const dequeuedJob = await mrtService.dequeueNextJob({ + orgId: org.id, + queueId: queue.id, + userId: reviewerId, + }); - if (!dequeuedJob) { - throw new Error("should've returned a job"); - } + if (!dequeuedJob) { + throw new Error("should've returned a job"); + } - await mrtService.submitDecision({ - queueId, - reportHistory: [], - jobId: dequeuedJob.job.id, - lockToken: dequeuedJob.lockToken, - decisionComponents: [ - { - type: 'CUSTOM_ACTION', - actions: [{ id: seededActionId }], - policies: [], - itemIds: [itemId], - itemTypeId, - }, - ], - relatedActions: [], - reviewerId, - reviewerEmail, - orgId, - }); - }); + await mrtService.submitDecision({ + queueId: queue.id, + reportHistory: [], + jobId: dequeuedJob.job.id, + lockToken: dequeuedJob.lockToken, + decisionComponents: [ + { + type: 'CUSTOM_ACTION', + actions: [{ id: actionId }], + policies: [], + itemIds: [itemId], + itemTypeId, + }, + ], + relatedActions: [], + reviewerId, + reviewerEmail, + orgId: org.id, + }); + }, + ); }); // Issue #615: orgs created before manual_review_tool_settings existed have no // row, so a save against them used to UPDATE zero rows and silently no-op. describe('settings persistence without a pre-existing row', () => { - const orgId = `no-row-${uuidv1()}`; - - afterEach(async () => { - await mrtService['pgQuery'] - .deleteFrom('manual_review_tool.manual_review_tool_settings') - .where('org_id', '=', orgId) - .execute(); - }); - - it('persists a boolean toggle when the org has no settings row', async () => { - expect(await mrtService.getHideSkipButtonForNonAdmins(orgId)).toBe(false); - - await mrtService.updateHideSkipButtonForNonAdmins(orgId, true); - - expect(await mrtService.getHideSkipButtonForNonAdmins(orgId)).toBe(true); - }); - - it('persists the ignore callback url when the org has no settings row', async () => { - await mrtService.updateIgnoreCallbackUrl( - orgId, - 'https://example.com/webhook/ignore', - ); + testWithService( + 'persists a boolean toggle when the org has no settings row', + async ({ mrtService }) => { + const orgId = `no-row-${uid()}`; + expect(await mrtService.getHideSkipButtonForNonAdmins(orgId)).toBe( + false, + ); + + await mrtService.updateHideSkipButtonForNonAdmins(orgId, true); + + expect(await mrtService.getHideSkipButtonForNonAdmins(orgId)).toBe( + true, + ); + }, + ); - expect(await mrtService.getIgnoreCallbackUrl(orgId)).toBe( - 'https://example.com/webhook/ignore', - ); - }); + testWithService( + 'persists the ignore callback url when the org has no settings row', + async ({ mrtService }) => { + const orgId = `no-row-${uid()}`; + await mrtService.updateIgnoreCallbackUrl( + orgId, + 'https://example.com/webhook/ignore', + ); - it('leaves other columns at their defaults when upserting one setting', async () => { - await mrtService.updatePreviewJobsViewEnabled(orgId, true); + expect(await mrtService.getIgnoreCallbackUrl(orgId)).toBe( + 'https://example.com/webhook/ignore', + ); + }, + ); - expect(await mrtService.getPreviewJobsViewEnabled(orgId)).toBe(true); - expect(await mrtService.getRequiresPolicyForDecisions(orgId)).toBe(false); - expect(await mrtService.getRequiresDecisionReason(orgId)).toBe(false); - }); + testWithService( + 'leaves other columns at their defaults when upserting one setting', + async ({ mrtService }) => { + const orgId = `no-row-${uid()}`; + await mrtService.updatePreviewJobsViewEnabled(orgId, true); + + expect(await mrtService.getPreviewJobsViewEnabled(orgId)).toBe(true); + expect(await mrtService.getRequiresPolicyForDecisions(orgId)).toBe( + false, + ); + expect(await mrtService.getRequiresDecisionReason(orgId)).toBe(false); + }, + ); }); }); diff --git a/server/services/manualReviewToolService/modules/CommentOperations.test.ts b/server/services/manualReviewToolService/modules/CommentOperations.test.ts index dd17ef9..7e1801e 100644 --- a/server/services/manualReviewToolService/modules/CommentOperations.test.ts +++ b/server/services/manualReviewToolService/modules/CommentOperations.test.ts @@ -1,39 +1,34 @@ import { v1 as uuidv1 } from 'uuid'; -import getBottle from '../../../iocContainer/index.js'; import createOrg from '../../../test/fixtureHelpers/createOrg.js'; import createUser from '../../../test/fixtureHelpers/createUser.js'; -import { makeTestWithFixture } from '../../../test/utils.js'; +import { makeTransactionalTestWithFixture } from '../../../test/harness/transactionalTest.js'; import { UserPermission } from '../../userManagementService/index.js'; import { type JobId } from '../manualReviewToolService.js'; import CommentOperations from './CommentOperations.js'; describe('CommentOperations', () => { - const testWithFixtures = makeTestWithFixture(async () => { - const container = (await getBottle()).container; - const pgQuery = container.KyselyPg; - const commentOps = new CommentOperations(pgQuery); - - // Create test org - const orgId = uuidv1(); - const { cleanup: orgCleanup } = await createOrg( - { - KyselyPg: container.KyselyPg, - ModerationConfigService: container.ModerationConfigService, - ApiKeyService: container.ApiKeyService, - }, - orgId, - ); + const testWithFixtures = makeTransactionalTestWithFixture( + async ({ deps }) => { + const pgQuery = deps.KyselyPg; + const commentOps = new CommentOperations(pgQuery); + + // Create test org + const orgId = uuidv1(); + await createOrg( + { + KyselyPg: deps.KyselyPg, + ModerationConfigService: deps.ModerationConfigService, + ApiKeyService: deps.ApiKeyService, + }, + orgId, + ); - // Create test user - const { user, cleanup: userCleanup } = await createUser( - container.KyselyPg, - orgId, - ); + // Create test user + const { user } = await createUser(deps.KyselyPg, orgId); - // Create a queue (required for job_creations foreign key) - const queue = - await container.ManualReviewToolService.createManualReviewQueue({ + // Create a queue (required for job_creations foreign key) + const queue = await deps.ManualReviewToolService.createManualReviewQueue({ name: 'Test Queue', description: null, userIds: [user.id], @@ -46,75 +41,49 @@ describe('CommentOperations', () => { }, }); - // Create test item identifiers and jobs - const itemId = uuidv1(); - const itemTypeId = uuidv1(); - const jobId1 = uuidv1(); - const jobId2 = uuidv1(); - - await pgQuery - .insertInto('manual_review_tool.job_creations') - .values([ - { - id: jobId1 as JobId, - org_id: orgId, - item_id: itemId, - item_type_id: itemTypeId, - queue_id: queue.id, - created_at: new Date('2023-01-01'), - enqueue_source_info: {}, - }, - { - id: jobId2 as JobId, - org_id: orgId, - item_id: itemId, - item_type_id: itemTypeId, - queue_id: queue.id, - created_at: new Date('2023-01-02'), - enqueue_source_info: {}, - }, - ]) - .execute(); - - return { - commentOps, - pgQuery, - orgId, - userId: user.id, - itemId, - itemTypeId, - jobId1, - jobId2, - queueId: queue.id, - async cleanup() { - // Clean up comments - await pgQuery - .deleteFrom('manual_review_tool.job_comments') - .where('org_id', '=', orgId) - .execute(); - - // Clean up job_creations - await pgQuery - .deleteFrom('manual_review_tool.job_creations') - .where('org_id', '=', orgId) - .execute(); - - // Clean up queue - await container.ManualReviewToolService.deleteManualReviewQueueForTestsDO_NOT_USE( - orgId, - queue.id, - ); - - // Clean up user and org - await userCleanup(); - await orgCleanup(); - - // Close database connections - await container.KyselyPg.destroy(); - await container.KyselyPgReadReplica.destroy(); - }, - }; - }); + // Create test item identifiers and jobs + const itemId = uuidv1(); + const itemTypeId = uuidv1(); + const jobId1 = uuidv1(); + const jobId2 = uuidv1(); + + await pgQuery + .insertInto('manual_review_tool.job_creations') + .values([ + { + id: jobId1 as JobId, + org_id: orgId, + item_id: itemId, + item_type_id: itemTypeId, + queue_id: queue.id, + created_at: new Date('2023-01-01'), + enqueue_source_info: {}, + }, + { + id: jobId2 as JobId, + org_id: orgId, + item_id: itemId, + item_type_id: itemTypeId, + queue_id: queue.id, + created_at: new Date('2023-01-02'), + enqueue_source_info: {}, + }, + ]) + .execute(); + + return { + commentOps, + pgQuery, + orgId, + userId: user.id, + itemId, + itemTypeId, + jobId1, + jobId2, + queueId: queue.id, + }; + }, + ); describe('getRelatedJobIds', () => { testWithFixtures( diff --git a/server/services/manualReviewToolService/modules/JobRouting.test.ts b/server/services/manualReviewToolService/modules/JobRouting.test.ts index 30c01d0..6b41930 100644 --- a/server/services/manualReviewToolService/modules/JobRouting.test.ts +++ b/server/services/manualReviewToolService/modules/JobRouting.test.ts @@ -2,11 +2,10 @@ import { ScalarTypes } from '@roostorg/coop-types'; import { uid } from 'uid'; -import getBottle from '../../../iocContainer/index.js'; import createContentItemTypes from '../../../test/fixtureHelpers/createContentItemTypes.js'; import createOrg from '../../../test/fixtureHelpers/createOrg.js'; import createUser from '../../../test/fixtureHelpers/createUser.js'; -import { makeTestWithFixture } from '../../../test/utils.js'; +import { makeTransactionalTestWithFixture } from '../../../test/harness/transactionalTest.js'; import { toCorrelationId } from '../../../utils/correlationIds.js'; import { jsonStringify } from '../../../utils/encoding.js'; import { type NonEmptyString } from '../../../utils/typescript-types.js'; @@ -20,25 +19,21 @@ import { SignalType } from '../../signalsService/index.js'; import { UserPermission } from '../../userManagementService/index.js'; describe('JobRouting tests', () => { - const jobRoutingTestWithFixtures = makeTestWithFixture(async () => { - const { container } = await getBottle(); - const manualReviewToolService = container.ManualReviewToolService; - const { org, cleanup: orgCleanup } = await createOrg( - { - KyselyPg: container.KyselyPg, - ModerationConfigService: container.ModerationConfigService, - ApiKeyService: container.ApiKeyService, - }, - uid(), - ); - const { user, cleanup: userCleanup } = await createUser( - container.KyselyPg, - org.id, - ); - const userId = user.id; - const { itemTypes, cleanup: itemTypesCleanup } = - await createContentItemTypes({ - moderationConfigService: container.ModerationConfigService, + const jobRoutingTestWithFixtures = makeTransactionalTestWithFixture( + async ({ deps }) => { + const manualReviewToolService = deps.ManualReviewToolService; + const { org } = await createOrg( + { + KyselyPg: deps.KyselyPg, + ModerationConfigService: deps.ModerationConfigService, + ApiKeyService: deps.ApiKeyService, + }, + uid(), + ); + const { user } = await createUser(deps.KyselyPg, org.id); + const userId = user.id; + const { itemTypes } = await createContentItemTypes({ + moderationConfigService: deps.ModerationConfigService, orgId: org.id, extra: { fields: [ @@ -51,110 +46,112 @@ describe('JobRouting tests', () => { ], }, }); - const itemType = itemTypes[0]; - - const defaultQueue = await manualReviewToolService.createManualReviewQueue({ - name: 'Default Queue', - description: null, - userIds: [userId], - hiddenActionIds: [], - isAppealsQueue: false, - invokedBy: { - userId, - permissions: [UserPermission.EDIT_MRT_QUEUES], - orgId: org.id, - }, - }); - const anotherQueue = await manualReviewToolService.createManualReviewQueue({ - name: 'Another Queue', - description: null, - userIds: [userId], - hiddenActionIds: [], - isAppealsQueue: false, - invokedBy: { - userId, - permissions: [UserPermission.EDIT_MRT_QUEUES], - orgId: org.id, - }, - }); - const policyQueue = await manualReviewToolService.createManualReviewQueue({ - name: 'Policy Queue', - description: null, - userIds: [userId], - hiddenActionIds: [], - isAppealsQueue: false, - invokedBy: { - userId, - permissions: [UserPermission.EDIT_MRT_QUEUES], - orgId: org.id, - }, - }); - const noPolicyQueue = await manualReviewToolService.createManualReviewQueue( - { - name: 'No Policy Queue', - description: null, - userIds: [userId], - hiddenActionIds: [], - isAppealsQueue: false, - invokedBy: { - userId, - permissions: [UserPermission.EDIT_MRT_QUEUES], - orgId: org.id, + const itemType = itemTypes[0]; + + const defaultQueue = + await manualReviewToolService.createManualReviewQueue({ + name: 'Default Queue', + description: null, + userIds: [userId], + hiddenActionIds: [], + isAppealsQueue: false, + invokedBy: { + userId, + permissions: [UserPermission.EDIT_MRT_QUEUES], + orgId: org.id, + }, + }); + const anotherQueue = + await manualReviewToolService.createManualReviewQueue({ + name: 'Another Queue', + description: null, + userIds: [userId], + hiddenActionIds: [], + isAppealsQueue: false, + invokedBy: { + userId, + permissions: [UserPermission.EDIT_MRT_QUEUES], + orgId: org.id, + }, + }); + const policyQueue = await manualReviewToolService.createManualReviewQueue( + { + name: 'Policy Queue', + description: null, + userIds: [userId], + hiddenActionIds: [], + isAppealsQueue: false, + invokedBy: { + userId, + permissions: [UserPermission.EDIT_MRT_QUEUES], + orgId: org.id, + }, }, - }, - ); - - const rule = await manualReviewToolService.createRoutingRule({ - orgId: org.id, - name: 'Some rule', - status: 'LIVE', - itemTypeIds: [itemType.id as NonEmptyString], - creatorId: '', - conditionSet: { - conjunction: 'AND', - conditions: [ - { - input: { - type: 'CONTENT_FIELD', - name: 'text', - contentTypeId: itemType.id, - }, - signal: { - id: jsonStringify({ + ); + const noPolicyQueue = + await manualReviewToolService.createManualReviewQueue({ + name: 'No Policy Queue', + description: null, + userIds: [userId], + hiddenActionIds: [], + isAppealsQueue: false, + invokedBy: { + userId, + permissions: [UserPermission.EDIT_MRT_QUEUES], + orgId: org.id, + }, + }); + + await manualReviewToolService.createRoutingRule({ + orgId: org.id, + name: 'Some rule', + status: 'LIVE', + itemTypeIds: [itemType.id as NonEmptyString], + creatorId: '', + conditionSet: { + conjunction: 'AND', + conditions: [ + { + input: { + type: 'CONTENT_FIELD', + name: 'text', + contentTypeId: itemType.id, + }, + signal: { + id: jsonStringify({ + type: SignalType.TEXT_MATCHING_CONTAINS_TEXT, + }), type: SignalType.TEXT_MATCHING_CONTAINS_TEXT, - }), - type: SignalType.TEXT_MATCHING_CONTAINS_TEXT, + }, + matchingValues: { strings: ['test'] }, }, - matchingValues: { strings: ['test'] }, - }, - ], - }, - destinationQueueId: anotherQueue.id, - }); - - const policyRule = await manualReviewToolService.createRoutingRule({ - orgId: org.id, - name: 'Policy ID rule', - status: 'LIVE', - itemTypeIds: [itemType.id as NonEmptyString], - creatorId: '', - conditionSet: { - conjunction: 'OR', - conditions: [ - { - input: { - type: 'CONTENT_COOP_INPUT', - name: 'Relevant Policy', + ], + }, + destinationQueueId: anotherQueue.id, + }); + + await manualReviewToolService.createRoutingRule({ + orgId: org.id, + name: 'Policy ID rule', + status: 'LIVE', + itemTypeIds: [itemType.id as NonEmptyString], + creatorId: '', + conditionSet: { + conjunction: 'OR', + conditions: [ + { + input: { + type: 'CONTENT_COOP_INPUT', + name: 'Relevant Policy', + }, + threshold: 'testPolicyId', + comparator: 'EQUALS', }, - threshold: 'testPolicyId', - comparator: 'EQUALS', - }, - ], - }, - destinationQueueId: policyQueue.id, - }); + ], + }, + destinationQueueId: policyQueue.id, + }); - const policyNotProvidedRule = await manualReviewToolService.createRoutingRule({ orgId: org.id, name: 'Policy ID not provided rule', @@ -190,76 +187,39 @@ describe('JobRouting tests', () => { destinationQueueId: noPolicyQueue.id, }); - const sourceTypeRule = await manualReviewToolService.createRoutingRule({ - orgId: org.id, - name: 'Source Type rule', - status: 'LIVE', - itemTypeIds: [itemType.id as NonEmptyString], - creatorId: '', - conditionSet: { - conjunction: 'OR', - conditions: [ - { - input: { - type: 'CONTENT_COOP_INPUT', - name: 'Source', + await manualReviewToolService.createRoutingRule({ + orgId: org.id, + name: 'Source Type rule', + status: 'LIVE', + itemTypeIds: [itemType.id as NonEmptyString], + creatorId: '', + conditionSet: { + conjunction: 'OR', + conditions: [ + { + input: { + type: 'CONTENT_COOP_INPUT', + name: 'Source', + }, + threshold: 'post-actions', + comparator: 'EQUALS', }, - threshold: 'post-actions', - comparator: 'EQUALS', - }, - ], - }, - destinationQueueId: anotherQueue.id, - }); + ], + }, + destinationQueueId: anotherQueue.id, + }); - return { - manualReviewToolService, - org, - itemType, - defaultQueue, - anotherQueue, - policyQueue, - noPolicyQueue, - async cleanup() { - await manualReviewToolService.deleteRoutingRule({ - id: rule.id, - orgId: org.id, - }); - await manualReviewToolService.deleteRoutingRule({ - id: policyRule.id, - orgId: org.id, - }); - await manualReviewToolService.deleteRoutingRule({ - id: policyNotProvidedRule.id, - orgId: org.id, - }); - await manualReviewToolService.deleteRoutingRule({ - id: sourceTypeRule.id, - orgId: org.id, - }); - await manualReviewToolService.deleteManualReviewQueueForTestsDO_NOT_USE( - org.id, - anotherQueue.id, - ); - await manualReviewToolService.deleteManualReviewQueueForTestsDO_NOT_USE( - org.id, - policyQueue.id, - ); - await manualReviewToolService.deleteManualReviewQueueForTestsDO_NOT_USE( - org.id, - defaultQueue.id, - ); - await manualReviewToolService.deleteManualReviewQueueForTestsDO_NOT_USE( - org.id, - noPolicyQueue.id, - ); - await itemTypesCleanup(); - await userCleanup(); - await orgCleanup(); - await container.closeSharedResourcesForShutdown(); - }, - }; - }); + return { + manualReviewToolService, + org, + itemType, + defaultQueue, + anotherQueue, + policyQueue, + noPolicyQueue, + }; + }, + ); jobRoutingTestWithFixtures( 'Should enqueue based off of routing rule', diff --git a/server/services/manualReviewToolService/modules/QueueOperations.test.ts b/server/services/manualReviewToolService/modules/QueueOperations.test.ts index 24dd887..182d4ed 100644 --- a/server/services/manualReviewToolService/modules/QueueOperations.test.ts +++ b/server/services/manualReviewToolService/modules/QueueOperations.test.ts @@ -1,14 +1,12 @@ import fc from 'fast-check'; import { uid } from 'uid'; -import getBottle from '../../../iocContainer/index.js'; import createActions from '../../../test/fixtureHelpers/createActions.js'; import createContentItemTypes from '../../../test/fixtureHelpers/createContentItemTypes.js'; import createMrtQueue from '../../../test/fixtureHelpers/createMrtQueue.js'; import createOrg from '../../../test/fixtureHelpers/createOrg.js'; import createUser from '../../../test/fixtureHelpers/createUser.js'; import { makeTransactionalTestWithFixture } from '../../../test/harness/transactionalTest.js'; -import { makeTestWithFixture } from '../../../test/utils.js'; import { UserPermission } from '../../userManagementService/index.js'; import { bullJobIdtoExternalJobId, @@ -34,48 +32,42 @@ describe('QueueOperations', () => { }); const testWithQueueAndActions = () => - makeTestWithFixture(async () => { - const container = (await getBottle()).container; - - const { org, cleanup: orgCleanup } = await createOrg( + makeTransactionalTestWithFixture(async ({ deps }) => { + const { org } = await createOrg( { - KyselyPg: container.KyselyPg, - ModerationConfigService: container.ModerationConfigService, - ApiKeyService: container.ApiKeyService, + KyselyPg: deps.KyselyPg, + ModerationConfigService: deps.ModerationConfigService, + ApiKeyService: deps.ApiKeyService, }, uid(), ); - const { user, cleanup: userCleanup } = await createUser( - container.KyselyPg, - org.id, - ); - const { itemTypes, cleanup: itemTypesCleanup } = - await createContentItemTypes({ - moderationConfigService: container.ModerationConfigService, - orgId: org.id, - extra: { - fields: [ - { - name: 'someField', - type: 'NUMBER', - required: false, - container: null, - }, - ], - }, - }); + const { user } = await createUser(deps.KyselyPg, org.id); + const { itemTypes } = await createContentItemTypes({ + moderationConfigService: deps.ModerationConfigService, + orgId: org.id, + extra: { + fields: [ + { + name: 'someField', + type: 'NUMBER', + required: false, + container: null, + }, + ], + }, + }); - const { actions, cleanup: actionsCleanup } = await createActions({ - actionAPI: container.ActionAPIDataSource, + const { actions } = await createActions({ + actionAPI: deps.ActionAPIDataSource, itemTypeIds: itemTypes.map((it) => it.id), orgId: org.id, numActions: 3, }); - const { queue, cleanup: queuesCleanup } = await createMrtQueue({ + const { queue } = await createMrtQueue({ orgId: org.id, - mrtService: container.ManualReviewToolService, + mrtService: deps.ManualReviewToolService, userId: user.id, }); @@ -83,16 +75,7 @@ describe('QueueOperations', () => { org, actions, queue, - mrtService: container.ManualReviewToolService, - cleanup: async () => { - await queuesCleanup(); - await actionsCleanup(); - await itemTypesCleanup(); - await userCleanup(); - await orgCleanup(); - await container.KyselyPg.destroy(); - await container.KyselyPgReadReplica.destroy(); - }, + mrtService: deps.ManualReviewToolService, }; }); diff --git a/server/services/manualReviewToolService/modules/ReporterInvalidation.test.ts b/server/services/manualReviewToolService/modules/ReporterInvalidation.test.ts index 1bc4008..df8d7e2 100644 --- a/server/services/manualReviewToolService/modules/ReporterInvalidation.test.ts +++ b/server/services/manualReviewToolService/modules/ReporterInvalidation.test.ts @@ -2,12 +2,11 @@ import { uid } from 'uid'; import { v1 as uuidv1 } from 'uuid'; -import getBottle from '../../../iocContainer/index.js'; import createContentItemTypes from '../../../test/fixtureHelpers/createContentItemTypes.js'; import createMrtQueue from '../../../test/fixtureHelpers/createMrtQueue.js'; import createOrg from '../../../test/fixtureHelpers/createOrg.js'; import createUser from '../../../test/fixtureHelpers/createUser.js'; -import { makeTestWithFixture } from '../../../test/utils.js'; +import { makeTransactionalTestWithFixture } from '../../../test/harness/transactionalTest.js'; import { instantiateOpaqueType } from '../../../utils/typescript-types.js'; import { makeSubmissionId, @@ -274,33 +273,28 @@ describe('scrubPayloadForReporter (pure)', () => { // Integration tests below mirror the pattern in `QueueOperations.test.ts`. const testWithQueue = () => - makeTestWithFixture(async () => { - const container = (await getBottle()).container; - const { org, cleanup: orgCleanup } = await createOrg( + makeTransactionalTestWithFixture(async ({ deps }) => { + const { org } = await createOrg( { - KyselyPg: container.KyselyPg, - ModerationConfigService: container.ModerationConfigService, - ApiKeyService: container.ApiKeyService, + KyselyPg: deps.KyselyPg, + ModerationConfigService: deps.ModerationConfigService, + ApiKeyService: deps.ApiKeyService, }, uid(), ); - const { user, cleanup: userCleanup } = await createUser( - container.KyselyPg, - org.id, - ); - const { itemTypes, cleanup: itemTypesCleanup } = - await createContentItemTypes({ - moderationConfigService: container.ModerationConfigService, - orgId: org.id, - extra: {}, - }); - const { queue, cleanup: queueCleanup } = await createMrtQueue({ + const { user } = await createUser(deps.KyselyPg, org.id); + const { itemTypes } = await createContentItemTypes({ + moderationConfigService: deps.ModerationConfigService, orgId: org.id, - mrtService: container.ManualReviewToolService, + extra: {}, + }); + const { queue } = await createMrtQueue({ + orgId: org.id, + mrtService: deps.ManualReviewToolService, userId: user.id, }); - const mrtService = container.ManualReviewToolService; + const mrtService = deps.ManualReviewToolService; // Bracket-index into the private QueueOperations to seed jobs directly, // matching the pattern in manualReviewToolService.test.ts. @@ -349,14 +343,6 @@ const testWithQueue = () => queue, mrtService, addJob, - cleanup: async () => { - await queueCleanup(); - await itemTypesCleanup(); - await userCleanup(); - await orgCleanup(); - await container.KyselyPg.destroy(); - await container.KyselyPgReadReplica.destroy(); - }, }; }); diff --git a/server/services/manualReviewToolService/modules/UserReportSweep.test.ts b/server/services/manualReviewToolService/modules/UserReportSweep.test.ts index 0e95cb1..68a8ded 100644 --- a/server/services/manualReviewToolService/modules/UserReportSweep.test.ts +++ b/server/services/manualReviewToolService/modules/UserReportSweep.test.ts @@ -2,12 +2,11 @@ import { type ItemIdentifier } from '@roostorg/types'; import { uid } from 'uid'; import { v1 as uuidv1 } from 'uuid'; -import getBottle from '../../../iocContainer/index.js'; import createContentItemTypes from '../../../test/fixtureHelpers/createContentItemTypes.js'; import createMrtQueue from '../../../test/fixtureHelpers/createMrtQueue.js'; import createOrg from '../../../test/fixtureHelpers/createOrg.js'; import createUser from '../../../test/fixtureHelpers/createUser.js'; -import { makeTestWithFixture } from '../../../test/utils.js'; +import { makeTransactionalTestWithFixture } from '../../../test/harness/transactionalTest.js'; import { instantiateOpaqueType } from '../../../utils/typescript-types.js'; import { makeSubmissionId, @@ -19,33 +18,28 @@ import { type CustomActionDecisionComponent } from './JobDecisioning.js'; const TRIGGER_ACTION_ID = 'ban-action'; const testWithQueue = () => - makeTestWithFixture(async () => { - const container = (await getBottle()).container; - const { org, cleanup: orgCleanup } = await createOrg( + makeTransactionalTestWithFixture(async ({ deps }) => { + const { org } = await createOrg( { - KyselyPg: container.KyselyPg, - ModerationConfigService: container.ModerationConfigService, - ApiKeyService: container.ApiKeyService, + KyselyPg: deps.KyselyPg, + ModerationConfigService: deps.ModerationConfigService, + ApiKeyService: deps.ApiKeyService, }, uid(), ); - const { user, cleanup: userCleanup } = await createUser( - container.KyselyPg, - org.id, - ); - const { itemTypes, cleanup: itemTypesCleanup } = - await createContentItemTypes({ - moderationConfigService: container.ModerationConfigService, - orgId: org.id, - extra: {}, - }); - const { queue, cleanup: queueCleanup } = await createMrtQueue({ + const { user } = await createUser(deps.KyselyPg, org.id); + const { itemTypes } = await createContentItemTypes({ + moderationConfigService: deps.ModerationConfigService, orgId: org.id, - mrtService: container.ManualReviewToolService, + extra: {}, + }); + const { queue } = await createMrtQueue({ + orgId: org.id, + mrtService: deps.ManualReviewToolService, userId: user.id, }); - const mrtService = container.ManualReviewToolService; + const mrtService = deps.ManualReviewToolService; const queueOps = mrtService['queueOps']; const addJob = async (opts: { @@ -122,14 +116,6 @@ const testWithQueue = () => addJob, configureQueue, pendingJobIds, - cleanup: async () => { - await queueCleanup(); - await itemTypesCleanup(); - await userCleanup(); - await orgCleanup(); - await container.KyselyPg.destroy(); - await container.KyselyPgReadReplica.destroy(); - }, }; }); diff --git a/server/services/moderationConfigService/moderationConfigService.test.ts b/server/services/moderationConfigService/moderationConfigService.test.ts index b425296..bcaa952 100644 --- a/server/services/moderationConfigService/moderationConfigService.test.ts +++ b/server/services/moderationConfigService/moderationConfigService.test.ts @@ -8,6 +8,8 @@ import getBottle from '../../iocContainer/index.js'; import createOrg from '../../test/fixtureHelpers/createOrg.js'; import createRule from '../../test/fixtureHelpers/createRule.js'; import createUser from '../../test/fixtureHelpers/createUser.js'; +import { makeTransactionalTestWithFixture } from '../../test/harness/transactionalTest.js'; +import { type MockedServer } from '../../test/setupMockedServer.js'; import { makeMockPgDialect, type MockPgExecute, @@ -19,215 +21,145 @@ import { type ModerationConfigServicePg } from './dbTypes.js'; import { RuleStatus, RuleType, - type Action, type ConditionSet, - type ItemType, type Policy, - type UserItemType, } from './index.js'; import { ModerationConfigService } from './moderationConfigService.js'; import { PolicyType } from './types/policies.js'; -describe('ModerationConfigService', () => { - let container: Awaited>['container']; - let sutWithPrimary: ModerationConfigService; - let sutWithReadReplica: ModerationConfigService; - let defaultUserItemType: UserItemType; - - // NB: because we don't create a new org for each tests (that feels like - // overkill), we have to track entities added in each write test, by adding - // them to the variables below, so that we can assert on the results when - // reading. - let allCreatedItemTypes = [] as ItemType[]; - const createdItemTypes = { - get ALL() { - return allCreatedItemTypes; - }, - get USER() { - return allCreatedItemTypes.filter((it) => it.kind === 'USER'); - }, - get CONTENT() { - return allCreatedItemTypes.filter((it) => it.kind === 'CONTENT'); - }, - get THREAD() { - return allCreatedItemTypes.filter((it) => it.kind === 'THREAD'); - }, - }; - - let createdActions = [] as Action[]; - - const createdPolicies = [ - { - id: '1', - name: 'Example policy', - orgId: 'orgId', - parentId: 'parentId', - createdAt: new Date(), - updatedAt: new Date(), - semanticVersion: 1, - policyText: '', - policyType: PolicyType.DRUG_SALES, - userStrikeCount: 1, - applyUserStrikeCountConfigToChildren: false, - penalty: 'NONE', - }, - ] satisfies Policy[]; - - const dummyOrgId = uid(); - let dummyOrgCleanup: () => Promise; - const dummySchema = [ - { name: 'fakeField', type: 'STRING', required: false, container: null }, - ] as const; - - // Every time we'll run these tests, we'll generate a new org from scratch, - // and then delete it at the end (which should hopefully do a cascading delete - // of most/all of its relevant data). Testing this way let's us truly test the - // moderationConfigService as a black box -- inserting data only using the - // public methods, and then verifying that we can retrieve it or delete it - // with only the public methods. Any other approach would require our tests to - // memorize exactly what queries the service is issuing and the schema of the - // underlying db tables, which makes the tests more brittle/harder to maintain - // than I'd like if the service is refactored. - beforeAll(async () => { - container = (await getBottle()).container; - - // An instance of kysely that will throw if any queries are run through it; - // used to test that the moderationConfigService is querying the correct db. - const kyselyShouldBeUnused = new Kysely({ - dialect: makeMockPgDialect( - jest.fn().mockImplementation(async () => { - throw new Error('Did not expect this kysely instance to be used!'); - }), - ), - }); - - // In order to test that the correct db is queried (i.e., replicas vs the - // primary), we'll just use different instances of the service, where each - // only has access to the db we expect to be hit. +type TestDeps = MockedServer['deps']; + +type Sut = ConstructorParameters[0]; + +// We test the moderationConfigService as a black box: every test gets a fresh +// org and seeds data only through the public methods, then verifies it can read +// or delete that data with only the public methods. The transactional harness +// rolls everything back after each test, so there's no cleanup and no state +// leaking between tests (and therefore no ordering dependency between them). +// +// To test that the correct db is queried (i.e., replicas vs the primary), we +// use two service instances, each with access only to the db we expect to be +// hit; the other db is a kysely instance that throws if it's ever used. +function makeSuts(primary: Sut, replica: Sut) { + const kyselyShouldBeUnused = new Kysely({ + dialect: makeMockPgDialect( + jest.fn().mockImplementation(async () => { + throw new Error('Did not expect this kysely instance to be used!'); + }), + ), + }); - sutWithPrimary = new ModerationConfigService( - container.KyselyPg, + return { + sutWithPrimary: new ModerationConfigService( + primary, kyselyShouldBeUnused, async () => {}, - ); - - sutWithReadReplica = new ModerationConfigService( + ), + sutWithReadReplica: new ModerationConfigService( kyselyShouldBeUnused, - container.KyselyPgReadReplica, + replica, async () => {}, - ); + ), + }; +} - const createOrgResult = await createOrg( - { - KyselyPg: container.KyselyPg, - ModerationConfigService: container.ModerationConfigService, - ApiKeyService: container.ApiKeyService, - }, - dummyOrgId, - ); +async function setupOrg(deps: TestDeps) { + const suts = makeSuts(deps.KyselyPg, deps.KyselyPgReadReplica); + const { org, defaultUserItemType } = await createOrg( + { + KyselyPg: deps.KyselyPg, + ModerationConfigService: deps.ModerationConfigService, + ApiKeyService: deps.ApiKeyService, + }, + uid(), + ); + return { ...suts, org, defaultUserItemType }; +} + +type SupportsReplicaMethod = Satisfies< + | 'getItemTypes' + | 'getItemType' + | 'getItemTypesByKind' + | 'getDefaultUserType' + | 'getItemTypesForAction' + | 'getItemTypesForRule' + | 'getActions' + | 'getPolicies', + keyof ModerationConfigService +>; + +async function expectReadReplicaUse( + suts: { + sutWithPrimary: ModerationConfigService; + sutWithReadReplica: ModerationConfigService; + }, + method: T, + baseFilter: Parameters[0], +) { + // cast baseFilter to prevent TS errors that would arise because TS can't + // verify that the particular string that `method` takes on at runtime + // corresponsds to the particular binding for baseFilter. + const filters = baseFilter as UnionToIntersection< + Parameters[0] + >; - defaultUserItemType = createOrgResult.defaultUserItemType; - dummyOrgCleanup = createOrgResult.cleanup; - allCreatedItemTypes = [...allCreatedItemTypes, defaultUserItemType]; - }); + // We're calling these to test that none of them throw, which'll only be + // true if the proper db is used. + await suts.sutWithPrimary[method](filters); + await suts.sutWithPrimary[method]({ ...filters, readFromReplica: false }); + await suts.sutWithReadReplica[method]({ ...filters, readFromReplica: true }); +} - afterAll(async () => { - await dummyOrgCleanup(); +const dummySchema = [ + { name: 'fakeField', type: 'STRING', required: false, container: null }, +] as const; - await Promise.all([ - container.KyselyPg.destroy(), - container.KyselyPgReadReplica.destroy(), - ]); - }); +const minimalRuleConditionSet = { + conjunction: 'AND' as const, + conditions: [ + { + input: { type: 'FULL_ITEM' as const }, + comparator: 'IS_NOT_PROVIDED' as const, + }, + ], +} satisfies ConditionSet; - const itemTypeSnapshotMatchers = { - id: expect.any(String), - version: expect.any(String), - orgId: expect.any(String), - }; +const itemTypeSnapshotMatchers = { + id: expect.any(String), + version: expect.any(String), + orgId: expect.any(String), +}; - const actionSnapshotMatchers = { - id: expect.any(String), - orgId: expect.any(String), - }; +const actionSnapshotMatchers = { + id: expect.any(String), + orgId: expect.any(String), +}; - type SupportsReplicaMethod = Satisfies< - | 'getItemTypes' - | 'getItemType' - | 'getItemTypesByKind' - | 'getDefaultUserType' - | 'getItemTypesForAction' - | 'getItemTypesForRule' - | 'getActions' - | 'getPolicies', - keyof ModerationConfigService - >; - - async function testReadReplicaUse( - method: T, - baseFilter: Parameters[0], - ) { - // cast baseFilter to prevent TS errors that would arise because TS can't - // verify that the particular string that `method` takes on at runtime - // corresponsds to the particular binding for baseFilter. - const filters = baseFilter as UnionToIntersection< - Parameters[0] - >; - - // We're calling these to test that none of them throw, which'll only be - // true if the proper db is used. - await sutWithPrimary[method](filters); - await sutWithPrimary[method]({ ...filters, readFromReplica: false }); - await sutWithReadReplica[method]({ ...filters, readFromReplica: true }); - } - - const minimalRuleConditionSet = { - conjunction: 'AND' as const, - conditions: [ - { - input: { type: 'FULL_ITEM' as const }, - comparator: 'IS_NOT_PROVIDED' as const, - }, - ], - } satisfies ConditionSet; +describe('ModerationConfigService', () => { + const testWithOrg = makeTransactionalTestWithFixture(async ({ deps }) => + setupOrg(deps), + ); describe('#getRuleByIdAndOrg', () => { - const testWithRuleRow = makeTestWithFixture(async () => { - const { org, cleanup: orgCleanup } = await createOrg( - { - KyselyPg: container.KyselyPg, - ModerationConfigService: container.ModerationConfigService, - ApiKeyService: container.ApiKeyService, - }, - uid(), - ); - const { user, cleanup: userCleanup } = await createUser( - container.KyselyPg, - org.id, - ); - const rule = await createRule(container.KyselyPg, org.id, { - creator: user, - name: 'getRuleByIdAndOrg fixture rule', - ruleType: RuleType.USER, - status: RuleStatus.DRAFT, - conditionSet: minimalRuleConditionSet, - }); + const testWithRuleRow = makeTransactionalTestWithFixture( + async ({ deps }) => { + const base = await setupOrg(deps); + const { user } = await createUser(deps.KyselyPg, base.org.id); + const rule = await createRule(deps.KyselyPg, base.org.id, { + creator: user, + name: 'getRuleByIdAndOrg fixture rule', + ruleType: RuleType.USER, + status: RuleStatus.DRAFT, + conditionSet: minimalRuleConditionSet, + }); - return { - org, - user, - ruleId: rule.id, - async cleanup() { - await rule.destroy(); - await userCleanup(); - await orgCleanup(); - }, - }; - }); + return { ...base, user, ruleId: rule.id }; + }, + ); testWithRuleRow( 'returns the rule when the org id matches the rule row', - async ({ org, ruleId }) => { + async ({ sutWithPrimary, org, ruleId }) => { const row = await sutWithPrimary.getRuleByIdAndOrg(ruleId, org.id, { readFromReplica: false, }); @@ -239,273 +171,322 @@ describe('ModerationConfigService', () => { testWithRuleRow( 'returns null when the org id does not match (IDOR guard)', - async ({ ruleId }) => { - const { org: otherOrg, cleanup: otherOrgCleanup } = await createOrg( + async ({ sutWithPrimary, deps, ruleId }) => { + const { org: otherOrg } = await createOrg( { - KyselyPg: container.KyselyPg, - ModerationConfigService: container.ModerationConfigService, - ApiKeyService: container.ApiKeyService, + KyselyPg: deps.KyselyPg, + ModerationConfigService: deps.ModerationConfigService, + ApiKeyService: deps.ApiKeyService, }, uid(), ); - try { - const row = await sutWithPrimary.getRuleByIdAndOrg( - ruleId, - otherOrg.id, - { readFromReplica: false }, - ); - expect(row).toBeNull(); - } finally { - await otherOrgCleanup(); - } + const row = await sutWithPrimary.getRuleByIdAndOrg( + ruleId, + otherOrg.id, + { + readFromReplica: false, + }, + ); + expect(row).toBeNull(); }, ); }); - // NB: there is an ordering dependency between these tests, as the creation - // tests run first and then the read tests assert on the presence of their - // writes. describe('ItemType-Returning methods', () => { describe('Creation methods', () => { describe('#createContentType', () => { - it('should return and durably save the new item type', async () => { - const saved = await sutWithPrimary.createContentType(dummyOrgId, { - schema: dummySchema, - description: null, - name: 'Content Item Type', - schemaFieldRoles: { - displayName: 'fakeField', - }, - }); + testWithOrg( + 'should return and durably save the new item type', + async ({ sutWithPrimary, org }) => { + const saved = await sutWithPrimary.createContentType(org.id, { + schema: dummySchema, + description: null, + name: 'Content Item Type', + schemaFieldRoles: { + displayName: 'fakeField', + }, + }); - const fetched = await sutWithPrimary.getItemType({ - orgId: dummyOrgId, - itemTypeSelector: { id: saved.id }, - }); + const fetched = await sutWithPrimary.getItemType({ + orgId: org.id, + itemTypeSelector: { id: saved.id }, + }); - expect(saved).toMatchInlineSnapshot( - itemTypeSnapshotMatchers, - ` - { - "description": null, - "id": Any, - "kind": "CONTENT", - "name": "Content Item Type", - "orgId": Any, - "schema": [ - { - "container": null, - "name": "fakeField", - "required": false, - "type": "STRING", + expect(saved).toMatchInlineSnapshot( + itemTypeSnapshotMatchers, + ` + { + "description": null, + "id": Any, + "kind": "CONTENT", + "name": "Content Item Type", + "orgId": Any, + "schema": [ + { + "container": null, + "name": "fakeField", + "required": false, + "type": "STRING", + }, + ], + "schemaFieldRoles": { + "createdAt": undefined, + "creatorId": undefined, + "displayName": "fakeField", + "ipAddress": undefined, + "isDeleted": undefined, + "parentId": undefined, + "threadId": undefined, }, - ], - "schemaFieldRoles": { - "createdAt": undefined, - "creatorId": undefined, - "displayName": "fakeField", - "ipAddress": undefined, - "isDeleted": undefined, - "parentId": undefined, - "threadId": undefined, - }, - "schemaVariant": "original", - "version": Any, - } - `, - ); - expect(saved.orgId).toBe(dummyOrgId); - expect(saved).toEqual(fetched); - allCreatedItemTypes = [...allCreatedItemTypes, saved]; - }); + "schemaVariant": "original", + "version": Any, + } + `, + ); + expect(saved.orgId).toBe(org.id); + expect(saved).toEqual(fetched); + }, + ); }); describe('#createThreadType', () => { - it('should return and durably save the new item type', async () => { - const saved = await sutWithPrimary.createThreadType(dummyOrgId, { - schema: dummySchema, - description: 'Test description', - name: 'Thread Item Type', - schemaFieldRoles: { - displayName: 'fakeField', - }, - }); + testWithOrg( + 'should return and durably save the new item type', + async ({ sutWithPrimary, org }) => { + const saved = await sutWithPrimary.createThreadType(org.id, { + schema: dummySchema, + description: 'Test description', + name: 'Thread Item Type', + schemaFieldRoles: { + displayName: 'fakeField', + }, + }); - const fetched = await sutWithPrimary.getItemType({ - orgId: dummyOrgId, - itemTypeSelector: { id: saved.id }, - }); + const fetched = await sutWithPrimary.getItemType({ + orgId: org.id, + itemTypeSelector: { id: saved.id }, + }); - expect(saved).toMatchInlineSnapshot( - itemTypeSnapshotMatchers, - ` - { - "description": "Test description", - "id": Any, - "kind": "THREAD", - "name": "Thread Item Type", - "orgId": Any, - "schema": [ - { - "container": null, - "name": "fakeField", - "required": false, - "type": "STRING", + expect(saved).toMatchInlineSnapshot( + itemTypeSnapshotMatchers, + ` + { + "description": "Test description", + "id": Any, + "kind": "THREAD", + "name": "Thread Item Type", + "orgId": Any, + "schema": [ + { + "container": null, + "name": "fakeField", + "required": false, + "type": "STRING", + }, + ], + "schemaFieldRoles": { + "createdAt": undefined, + "creatorId": undefined, + "displayName": "fakeField", + "ipAddress": undefined, + "isDeleted": undefined, }, - ], - "schemaFieldRoles": { - "createdAt": undefined, - "creatorId": undefined, - "displayName": "fakeField", - "ipAddress": undefined, - "isDeleted": undefined, - }, - "schemaVariant": "original", - "version": Any, - } - `, - ); - expect(saved.orgId).toBe(dummyOrgId); - expect(saved).toEqual(fetched); - allCreatedItemTypes = [...allCreatedItemTypes, saved]; - }); + "schemaVariant": "original", + "version": Any, + } + `, + ); + expect(saved.orgId).toBe(org.id); + expect(saved).toEqual(fetched); + }, + ); }); describe('#createUserType', () => { - it('should return and durably save the new item type', async () => { - const saved = await sutWithPrimary.createUserType(dummyOrgId, { - schema: dummySchema, - description: null, - name: 'User Item Type', - schemaFieldRoles: { - displayName: 'fakeField', - }, - }); + testWithOrg( + 'should return and durably save the new item type', + async ({ sutWithPrimary, org }) => { + const saved = await sutWithPrimary.createUserType(org.id, { + schema: dummySchema, + description: null, + name: 'User Item Type', + schemaFieldRoles: { + displayName: 'fakeField', + }, + }); - const fetched = await sutWithPrimary.getItemType({ - orgId: dummyOrgId, - itemTypeSelector: { id: saved.id }, - }); + const fetched = await sutWithPrimary.getItemType({ + orgId: org.id, + itemTypeSelector: { id: saved.id }, + }); - expect(saved).toMatchInlineSnapshot( - itemTypeSnapshotMatchers, - ` - { - "description": null, - "id": Any, - "isDefaultUserType": false, - "kind": "USER", - "name": "User Item Type", - "orgId": Any, - "schema": [ - { - "container": null, - "name": "fakeField", - "required": false, - "type": "STRING", + expect(saved).toMatchInlineSnapshot( + itemTypeSnapshotMatchers, + ` + { + "description": null, + "id": Any, + "isDefaultUserType": false, + "kind": "USER", + "name": "User Item Type", + "orgId": Any, + "schema": [ + { + "container": null, + "name": "fakeField", + "required": false, + "type": "STRING", + }, + ], + "schemaFieldRoles": { + "backgroundImage": undefined, + "createdAt": undefined, + "displayName": "fakeField", + "email": undefined, + "ipAddress": undefined, + "isDeleted": undefined, + "profileIcon": undefined, }, - ], - "schemaFieldRoles": { - "backgroundImage": undefined, - "createdAt": undefined, - "displayName": "fakeField", - "email": undefined, - "ipAddress": undefined, - "isDeleted": undefined, - "profileIcon": undefined, - }, - "schemaVariant": "original", - "version": Any, - } - `, - ); - expect(saved.orgId).toBe(dummyOrgId); - expect(saved).toEqual(fetched); - allCreatedItemTypes = [...allCreatedItemTypes, saved]; - }); + "schemaVariant": "original", + "version": Any, + } + `, + ); + expect(saved.orgId).toBe(org.id); + expect(saved).toEqual(fetched); + }, + ); }); }); describe('Read methods', () => { describe('#getItemTypes', () => { - it('should return all item types, properly formatted', async () => { - const res = await sutWithPrimary.getItemTypes({ orgId: dummyOrgId }); - expect(res).toHaveLength(createdItemTypes.ALL.length); - expect(res).toEqual(expect.arrayContaining(createdItemTypes.ALL)); - }); + testWithOrg( + 'should return all item types, properly formatted', + async ({ sutWithPrimary, org, defaultUserItemType }) => { + const contentType = await sutWithPrimary.createContentType(org.id, { + schema: dummySchema, + description: null, + name: 'Content Item Type', + schemaFieldRoles: { displayName: 'fakeField' }, + }); + const threadType = await sutWithPrimary.createThreadType(org.id, { + schema: dummySchema, + description: null, + name: 'Thread Item Type', + schemaFieldRoles: { displayName: 'fakeField' }, + }); + const userType = await sutWithPrimary.createUserType(org.id, { + schema: dummySchema, + description: null, + name: 'User Item Type', + schemaFieldRoles: { displayName: 'fakeField' }, + }); + + const expected = [ + defaultUserItemType, + contentType, + threadType, + userType, + ]; + const res = await sutWithPrimary.getItemTypes({ orgId: org.id }); + expect(res).toHaveLength(expected.length); + expect(res).toEqual(expect.arrayContaining(expected)); + }, + ); }); describe('#getItemTypesByKind', () => { - it('should filter by kind', async () => { - const [userItemTypes, contentItemTypes, threadItemTypes] = - await Promise.all([ - sutWithPrimary.getItemTypesByKind({ - orgId: dummyOrgId, - kind: 'USER', - }), - sutWithPrimary.getItemTypesByKind({ - orgId: dummyOrgId, - kind: 'CONTENT', - }), - sutWithPrimary.getItemTypesByKind({ - orgId: dummyOrgId, - kind: 'THREAD', - }), - ]); + testWithOrg( + 'should filter by kind', + async ({ sutWithPrimary, org, defaultUserItemType }) => { + const contentType = await sutWithPrimary.createContentType(org.id, { + schema: dummySchema, + description: null, + name: 'Content Item Type', + schemaFieldRoles: { displayName: 'fakeField' }, + }); + const threadType = await sutWithPrimary.createThreadType(org.id, { + schema: dummySchema, + description: null, + name: 'Thread Item Type', + schemaFieldRoles: { displayName: 'fakeField' }, + }); + const userType = await sutWithPrimary.createUserType(org.id, { + schema: dummySchema, + description: null, + name: 'User Item Type', + schemaFieldRoles: { displayName: 'fakeField' }, + }); - expect(userItemTypes).toHaveLength(createdItemTypes.USER.length); - expect(userItemTypes).toEqual( - expect.arrayContaining(createdItemTypes.USER), - ); + const userItemTypes = await sutWithPrimary.getItemTypesByKind({ + orgId: org.id, + kind: 'USER', + }); + const contentItemTypes = await sutWithPrimary.getItemTypesByKind({ + orgId: org.id, + kind: 'CONTENT', + }); + const threadItemTypes = await sutWithPrimary.getItemTypesByKind({ + orgId: org.id, + kind: 'THREAD', + }); - expect(contentItemTypes).toHaveLength( - createdItemTypes.CONTENT.length, - ); - expect(contentItemTypes).toEqual( - expect.arrayContaining(createdItemTypes.CONTENT), - ); + expect(userItemTypes).toHaveLength(2); + expect(userItemTypes).toEqual( + expect.arrayContaining([defaultUserItemType, userType]), + ); - expect(threadItemTypes).toHaveLength(createdItemTypes.THREAD.length); - expect(threadItemTypes).toEqual( - expect.arrayContaining(createdItemTypes.THREAD), - ); - }); + expect(contentItemTypes).toEqual([contentType]); + expect(threadItemTypes).toEqual([threadType]); + }, + ); }); describe('#getDefaultUserType', () => { - it('should return the defualt user type, properly formatted', async () => { - const res = await sutWithPrimary.getDefaultUserType({ - orgId: dummyOrgId, - }); - expect(res).toEqual(defaultUserItemType); - }); + testWithOrg( + 'should return the default user type, properly formatted', + async ({ sutWithPrimary, org, defaultUserItemType }) => { + const res = await sutWithPrimary.getDefaultUserType({ + orgId: org.id, + }); + expect(res).toEqual(defaultUserItemType); + }, + ); }); describe('#getItemTypesForAction', () => { - it('should query from the proper db', async () => { - // These tests will throw if the wrong db is used (see kyselyShouldBeUnused) - await sutWithPrimary.getItemTypesForAction({ - orgId: dummyOrgId, - actionId: 'someId', - directives: { maxAge: 0 }, - }); - await sutWithReadReplica.getItemTypesForAction({ - orgId: dummyOrgId, - actionId: 'someId', - directives: { maxAge: 10 }, - }); - }); + testWithOrg( + 'should query from the proper db', + async ({ sutWithPrimary, sutWithReadReplica, org }) => { + // These tests will throw if the wrong db is used (see kyselyShouldBeUnused) + await sutWithPrimary.getItemTypesForAction({ + orgId: org.id, + actionId: 'someId', + directives: { maxAge: 0 }, + }); + await sutWithReadReplica.getItemTypesForAction({ + orgId: org.id, + actionId: 'someId', + directives: { maxAge: 10 }, + }); + }, + ); it.skip('should return the right results', () => {}); }); describe('#getItemTypesForRule', () => { - it('should query from the proper db', async () => { - await testReadReplicaUse('getItemTypesForRule', { - orgId: dummyOrgId, - ruleId: 'sasts', - }); - }); + testWithOrg( + 'should query from the proper db', + async ({ sutWithPrimary, sutWithReadReplica, org }) => { + await expectReadReplicaUse( + { sutWithPrimary, sutWithReadReplica }, + 'getItemTypesForRule', + { orgId: org.id, ruleId: 'sasts' }, + ); + }, + ); it.skip('should return the right results', () => {}); }); @@ -515,67 +496,63 @@ describe('ModerationConfigService', () => { describe('Action-returning methods', () => { describe('Creation methods', () => { describe('#upsertBuiltInActions', () => { - it('seeds the three built-in (non-CUSTOM_ACTION) rows for the org', async () => { - const all = await sutWithPrimary.getActions({ orgId: dummyOrgId }); - const builtIns = all.filter( - (it) => it.actionType !== 'CUSTOM_ACTION', - ); - const types = builtIns.map((it) => it.actionType).sort(); - expect(types).toEqual( - [ - 'ENQUEUE_AUTHOR_TO_MRT', - 'ENQUEUE_TO_MRT', - 'ENQUEUE_TO_NCMEC', - ].sort(), - ); - for (const action of builtIns) { - expect(action.orgId).toBe(dummyOrgId); - expect(action).not.toHaveProperty('callbackUrl'); - } - }); + testWithOrg( + 'seeds the three built-in (non-CUSTOM_ACTION) rows for the org', + async ({ sutWithPrimary, org }) => { + const all = await sutWithPrimary.getActions({ orgId: org.id }); + const builtIns = all.filter( + (it) => it.actionType !== 'CUSTOM_ACTION', + ); + const types = builtIns.map((it) => it.actionType).sort(); + expect(types).toEqual( + [ + 'ENQUEUE_AUTHOR_TO_MRT', + 'ENQUEUE_TO_MRT', + 'ENQUEUE_TO_NCMEC', + ].sort(), + ); + for (const action of builtIns) { + expect(action.orgId).toBe(org.id); + expect(action).not.toHaveProperty('callbackUrl'); + } + }, + ); - it('is idempotent: calling twice does not create duplicates', async () => { - const before = await sutWithPrimary.getActions({ - orgId: dummyOrgId, - }); - const beforeBuiltIns = before - .filter((it) => it.actionType !== 'CUSTOM_ACTION') - .map((it) => it.id) - .sort(); - await sutWithPrimary.upsertBuiltInActions(dummyOrgId); - const after = await sutWithPrimary.getActions({ - orgId: dummyOrgId, - }); - const afterBuiltIns = after - .filter((it) => it.actionType !== 'CUSTOM_ACTION') - .map((it) => it.id) - .sort(); - expect(afterBuiltIns).toEqual(beforeBuiltIns); - }); + testWithOrg( + 'is idempotent: calling twice does not create duplicates', + async ({ sutWithPrimary, org }) => { + const before = await sutWithPrimary.getActions({ + orgId: org.id, + }); + const beforeBuiltIns = before + .filter((it) => it.actionType !== 'CUSTOM_ACTION') + .map((it) => it.id) + .sort(); + await sutWithPrimary.upsertBuiltInActions(org.id); + const after = await sutWithPrimary.getActions({ + orgId: org.id, + }); + const afterBuiltIns = after + .filter((it) => it.actionType !== 'CUSTOM_ACTION') + .map((it) => it.id) + .sort(); + expect(afterBuiltIns).toEqual(beforeBuiltIns); + }, + ); - it('built-ins surface for the appropriate item type kinds', async () => { - const fresh = await createOrg( - { - KyselyPg: container.KyselyPg, - ModerationConfigService: container.ModerationConfigService, - ApiKeyService: container.ApiKeyService, - }, - uid(), - ); - try { - const contentType = await sutWithPrimary.createContentType( - fresh.org.id, - { - schema: dummySchema, - description: null, - name: faker.random.alphaNumeric(16), - schemaFieldRoles: { displayName: 'fakeField' }, - }, - ); + testWithOrg( + 'built-ins surface for the appropriate item type kinds', + async ({ sutWithPrimary, org, defaultUserItemType }) => { + const contentType = await sutWithPrimary.createContentType(org.id, { + schema: dummySchema, + description: null, + name: faker.random.alphaNumeric(16), + schemaFieldRoles: { displayName: 'fakeField' }, + }); const forUser = await sutWithPrimary.getActionsForItemType({ - orgId: fresh.org.id, - itemTypeId: fresh.defaultUserItemType.id, + orgId: org.id, + itemTypeId: defaultUserItemType.id, itemTypeKind: 'USER', }); expect(forUser.map((it) => it.actionType).sort()).toEqual( @@ -583,7 +560,7 @@ describe('ModerationConfigService', () => { ); const forContent = await sutWithPrimary.getActionsForItemType({ - orgId: fresh.org.id, + orgId: org.id, itemTypeId: contentType.id, itemTypeKind: 'CONTENT', }); @@ -594,95 +571,129 @@ describe('ModerationConfigService', () => { 'ENQUEUE_TO_NCMEC', ].sort(), ); - } finally { - await fresh.cleanup(); - } - }); + }, + ); }); describe('#createAction', () => { - it('should return and durably save the new action', async () => { - const saved = await sutWithPrimary.createAction(dummyOrgId, { - name: 'Test Action', - description: 'Test description', - type: 'CUSTOM_ACTION', - callbackUrl: 'https://example.com', - callbackUrlHeaders: null, - callbackUrlBody: null, - applyUserStrikes: false, - }); + testWithOrg( + 'should return and durably save the new action', + async ({ sutWithPrimary, org }) => { + const saved = await sutWithPrimary.createAction(org.id, { + name: 'Test Action', + description: 'Test description', + type: 'CUSTOM_ACTION', + callbackUrl: 'https://example.com', + callbackUrlHeaders: null, + callbackUrlBody: null, + applyUserStrikes: false, + }); - const [fetched] = await sutWithPrimary.getActions({ - orgId: dummyOrgId, - ids: [saved.id], - }); + const [fetched] = await sutWithPrimary.getActions({ + orgId: org.id, + ids: [saved.id], + }); - expect(saved).toMatchInlineSnapshot( - actionSnapshotMatchers, - ` - { - "actionType": "CUSTOM_ACTION", - "applyUserStrikes": false, - "callbackUrl": "https://example.com", - "callbackUrlBody": null, - "callbackUrlHeaders": null, - "customMrtApiParams": null, - "description": "Test description", - "id": Any, - "name": "Test Action", - "orgId": Any, - "penalty": "NONE", - } - `, - ); - expect(saved.orgId).toBe(dummyOrgId); - expect(saved).toEqual(fetched); - createdActions = [...createdActions, saved]; - }); + expect(saved).toMatchInlineSnapshot( + actionSnapshotMatchers, + ` + { + "actionType": "CUSTOM_ACTION", + "applyUserStrikes": false, + "callbackUrl": "https://example.com", + "callbackUrlBody": null, + "callbackUrlHeaders": null, + "customMrtApiParams": null, + "description": "Test description", + "id": Any, + "name": "Test Action", + "orgId": Any, + "penalty": "NONE", + } + `, + ); + expect(saved.orgId).toBe(org.id); + expect(saved).toEqual(fetched); + }, + ); }); }); describe('Read methods', () => { describe('#getActions', () => { - it('should query from the proper db', async () => { - await testReadReplicaUse('getActions', { orgId: dummyOrgId }); - }); + testWithOrg( + 'should query from the proper db', + async ({ sutWithPrimary, sutWithReadReplica, org }) => { + await expectReadReplicaUse( + { sutWithPrimary, sutWithReadReplica }, + 'getActions', + { orgId: org.id }, + ); + }, + ); - it('should return all actions, properly formatted', async () => { - const res = await sutWithPrimary.getActions({ orgId: dummyOrgId }); - const customActions = res.filter( - (it) => it.actionType === 'CUSTOM_ACTION', - ); - expect(customActions).toHaveLength(createdActions.length); - expect(customActions).toEqual(expect.arrayContaining(createdActions)); - }); + testWithOrg( + 'should return all custom actions, properly formatted', + async ({ sutWithPrimary, org }) => { + const createdActions = [ + await sutWithPrimary.createAction(org.id, { + name: faker.random.alphaNumeric(16), + description: 'Test description', + type: 'CUSTOM_ACTION', + callbackUrl: 'https://example.com', + callbackUrlHeaders: null, + callbackUrlBody: null, + applyUserStrikes: false, + }), + await sutWithPrimary.createAction(org.id, { + name: faker.random.alphaNumeric(16), + description: null, + type: 'CUSTOM_ACTION', + callbackUrl: 'https://example.com', + callbackUrlHeaders: null, + callbackUrlBody: null, + applyUserStrikes: false, + }), + ]; - it('should round-trip a non-null customMrtApiParams value', async () => { - const action = await sutWithPrimary.createAction(dummyOrgId, { - name: faker.random.alphaNumeric(16), - description: null, - type: 'CUSTOM_ACTION', - callbackUrl: 'https://example.com', - callbackUrlHeaders: null, - callbackUrlBody: null, - }); + const res = await sutWithPrimary.getActions({ orgId: org.id }); + const customActions = res.filter( + (it) => it.actionType === 'CUSTOM_ACTION', + ); + expect(customActions).toHaveLength(createdActions.length); + expect(customActions).toEqual( + expect.arrayContaining(createdActions), + ); + }, + ); - // Legacy shape pre-dating the typed parameter spec — set it via raw - // Kysely to verify the read mapping still surfaces older rows - // unchanged for back-compat. - const params = [ - { key: 'foo', value: 'bar' }, - { key: 'baz', value: 'qux' }, - ]; - await container.KyselyPg.updateTable('public.actions') - .set({ custom_mrt_api_params: params }) - .where('id', '=', action.id) - .where('org_id', '=', dummyOrgId) - .execute(); + testWithOrg( + 'should round-trip a non-null customMrtApiParams value', + async ({ sutWithPrimary, deps, org }) => { + const action = await sutWithPrimary.createAction(org.id, { + name: faker.random.alphaNumeric(16), + description: null, + type: 'CUSTOM_ACTION', + callbackUrl: 'https://example.com', + callbackUrlHeaders: null, + callbackUrlBody: null, + }); + + // Legacy shape pre-dating the typed parameter spec — set it via raw + // Kysely to verify the read mapping still surfaces older rows + // unchanged for back-compat. + const params = [ + { key: 'foo', value: 'bar' }, + { key: 'baz', value: 'qux' }, + ]; + await deps.KyselyPg.updateTable('public.actions') + .set({ custom_mrt_api_params: params }) + .where('id', '=', action.id) + .where('org_id', '=', org.id) + .execute(); - try { const [fetched] = await sutWithPrimary.getActions({ - orgId: dummyOrgId, + orgId: org.id, ids: [action.id], }); expect(fetched).toBeDefined(); @@ -691,121 +702,110 @@ describe('ModerationConfigService', () => { expect( (fetched as { customMrtApiParams: unknown }).customMrtApiParams, ).toEqual(params); - } finally { - await sutWithPrimary.deleteCustomAction({ - orgId: dummyOrgId, - actionId: action.id, - }); - } - }); + }, + ); - it('round-trips typed parameters through createAction', async () => { - const parameters = [ - { - name: 'num_days_banned', - displayName: 'Days to ban', - type: 'NUMBER', - required: true, - min: 1, - max: 365, - defaultValue: 7, - }, - { - name: 'reason', - displayName: 'Reason', - type: 'SELECT', - required: true, - options: [ - { value: 'spam', label: 'Spam' }, - { value: 'abuse', label: 'Abuse' }, - ], - }, - { - name: 'notify_user', - displayName: 'Notify user', - type: 'BOOLEAN', - required: false, - defaultValue: false, - }, - ]; + testWithOrg( + 'round-trips typed parameters through createAction', + async ({ sutWithPrimary, org }) => { + const parameters = [ + { + name: 'num_days_banned', + displayName: 'Days to ban', + type: 'NUMBER', + required: true, + min: 1, + max: 365, + defaultValue: 7, + }, + { + name: 'reason', + displayName: 'Reason', + type: 'SELECT', + required: true, + options: [ + { value: 'spam', label: 'Spam' }, + { value: 'abuse', label: 'Abuse' }, + ], + }, + { + name: 'notify_user', + displayName: 'Notify user', + type: 'BOOLEAN', + required: false, + defaultValue: false, + }, + ]; - const created = await sutWithPrimary.createAction(dummyOrgId, { - name: faker.random.alphaNumeric(16), - description: null, - type: 'CUSTOM_ACTION', - callbackUrl: 'https://example.com', - callbackUrlHeaders: null, - callbackUrlBody: null, - parameters, - }); + const created = await sutWithPrimary.createAction(org.id, { + name: faker.random.alphaNumeric(16), + description: null, + type: 'CUSTOM_ACTION', + callbackUrl: 'https://example.com', + callbackUrlHeaders: null, + callbackUrlBody: null, + parameters, + }); - try { const [fetched] = await sutWithPrimary.getActions({ - orgId: dummyOrgId, + orgId: org.id, ids: [created.id], }); expect(fetched.actionType).toBe('CUSTOM_ACTION'); const stored = (fetched as { customMrtApiParams: unknown }) .customMrtApiParams; expect(stored).toEqual(parameters); - } finally { - await sutWithPrimary.deleteCustomAction({ - orgId: dummyOrgId, - actionId: created.id, - }); - } - }); + }, + ); - it('rejects invalid parameters at create time', async () => { - await expect( - sutWithPrimary.createAction(dummyOrgId, { - name: faker.random.alphaNumeric(16), - description: null, - type: 'CUSTOM_ACTION', - callbackUrl: 'https://example.com', - callbackUrlHeaders: null, - callbackUrlBody: null, - parameters: [ - { - name: 'invalid name with spaces', - displayName: 'X', - type: 'STRING', - required: false, - }, - ], - }), - ).rejects.toMatchObject({ status: 400 }); - }); + testWithOrg( + 'rejects invalid parameters at create time', + async ({ sutWithPrimary, org }) => { + await expect( + sutWithPrimary.createAction(org.id, { + name: faker.random.alphaNumeric(16), + description: null, + type: 'CUSTOM_ACTION', + callbackUrl: 'https://example.com', + callbackUrlHeaders: null, + callbackUrlBody: null, + parameters: [ + { + name: 'invalid name with spaces', + displayName: 'X', + type: 'STRING', + required: false, + }, + ], + }), + ).rejects.toMatchObject({ status: 400 }); + }, + ); }); }); describe('Update methods', () => { describe('#updateCustomAction', () => { - const testWithAction = makeTestWithFixture(async () => { - const action = await sutWithPrimary.createAction(dummyOrgId, { - name: faker.random.alphaNumeric(16), - description: 'before', - type: 'CUSTOM_ACTION', - callbackUrl: 'https://before.example.com', - callbackUrlHeaders: null, - callbackUrlBody: null, - applyUserStrikes: false, - }); - return { - action, - async cleanup() { - await sutWithPrimary.deleteCustomAction({ - orgId: dummyOrgId, - actionId: action.id, - }); - }, - }; - }); + const testWithAction = makeTransactionalTestWithFixture( + async ({ deps }) => { + const base = await setupOrg(deps); + const action = await base.sutWithPrimary.createAction(base.org.id, { + name: faker.random.alphaNumeric(16), + description: 'before', + type: 'CUSTOM_ACTION', + callbackUrl: 'https://before.example.com', + callbackUrlHeaders: null, + callbackUrlBody: null, + applyUserStrikes: false, + }); + return { ...base, action }; + }, + ); testWithAction( 'should update user-editable fields and bump updated_at', - async ({ action }) => { - const before = await container.KyselyPg.selectFrom('public.actions') + async ({ sutWithPrimary, deps, org, action }) => { + const before = await deps.KyselyPg.selectFrom('public.actions') .select(['updated_at']) .where('id', '=', action.id) .executeTakeFirstOrThrow(); @@ -813,24 +813,21 @@ describe('ModerationConfigService', () => { // Wait briefly so updated_at can advance even on fast clocks. await new Promise((resolve) => setTimeout(resolve, 5)); - const updated = await sutWithPrimary.updateCustomAction( - dummyOrgId, - { - actionId: action.id, - patch: { - description: 'after', - callbackUrl: 'https://after.example.com', - applyUserStrikes: true, - }, + const updated = await sutWithPrimary.updateCustomAction(org.id, { + actionId: action.id, + patch: { + description: 'after', + callbackUrl: 'https://after.example.com', + applyUserStrikes: true, }, - ); + }); expect(updated.actionType).toBe('CUSTOM_ACTION'); expect(updated.description).toBe('after'); expect(updated.callbackUrl).toBe('https://after.example.com'); expect(updated.applyUserStrikes).toBe(true); - const after = await container.KyselyPg.selectFrom('public.actions') + const after = await deps.KyselyPg.selectFrom('public.actions') .select(['updated_at', 'description']) .where('id', '=', action.id) .executeTakeFirstOrThrow(); @@ -843,20 +840,20 @@ describe('ModerationConfigService', () => { testWithAction( 'should not bump updated_at for an empty patch with no itemTypeIds', - async ({ action }) => { - const before = await container.KyselyPg.selectFrom('public.actions') + async ({ sutWithPrimary, deps, org, action }) => { + const before = await deps.KyselyPg.selectFrom('public.actions') .select(['updated_at']) .where('id', '=', action.id) .executeTakeFirstOrThrow(); await new Promise((resolve) => setTimeout(resolve, 5)); - const result = await sutWithPrimary.updateCustomAction(dummyOrgId, { + const result = await sutWithPrimary.updateCustomAction(org.id, { actionId: action.id, patch: {}, }); - const after = await container.KyselyPg.selectFrom('public.actions') + const after = await deps.KyselyPg.selectFrom('public.actions') .select(['updated_at']) .where('id', '=', action.id) .executeTakeFirstOrThrow(); @@ -869,41 +866,37 @@ describe('ModerationConfigService', () => { testWithAction( 'should throw NotFound when called with the wrong org', - async ({ action }) => { - const otherOrg = await createOrg( + async ({ sutWithPrimary, deps, action }) => { + const { org: otherOrg } = await createOrg( { - KyselyPg: container.KyselyPg, - ModerationConfigService: container.ModerationConfigService, - ApiKeyService: container.ApiKeyService, + KyselyPg: deps.KyselyPg, + ModerationConfigService: deps.ModerationConfigService, + ApiKeyService: deps.ApiKeyService, }, uid(), ); - try { - await expect( - sutWithPrimary.updateCustomAction(otherOrg.org.id, { - actionId: action.id, - patch: { description: 'leaked' }, - }), - ).rejects.toThrow( - expect.objectContaining({ type: [ErrorType.NotFound] }), - ); - - // The action's row in the original org must be untouched. - const row = await container.KyselyPg.selectFrom('public.actions') - .select(['description']) - .where('id', '=', action.id) - .executeTakeFirstOrThrow(); - expect(row.description).toBe('before'); - } finally { - await otherOrg.cleanup(); - } + await expect( + sutWithPrimary.updateCustomAction(otherOrg.id, { + actionId: action.id, + patch: { description: 'leaked' }, + }), + ).rejects.toThrow( + expect.objectContaining({ type: [ErrorType.NotFound] }), + ); + + // The action's row in the original org must be untouched. + const row = await deps.KyselyPg.selectFrom('public.actions') + .select(['description']) + .where('id', '=', action.id) + .executeTakeFirstOrThrow(); + expect(row.description).toBe('before'); }, ); testWithAction( 'updates parameters when patch.parameters is supplied', - async ({ action }) => { - await sutWithPrimary.updateCustomAction(dummyOrgId, { + async ({ sutWithPrimary, org, action }) => { + await sutWithPrimary.updateCustomAction(org.id, { actionId: action.id, patch: { parameters: [ @@ -918,7 +911,7 @@ describe('ModerationConfigService', () => { }); const [afterSet] = await sutWithPrimary.getActions({ - orgId: dummyOrgId, + orgId: org.id, ids: [action.id], }); expect( @@ -933,12 +926,12 @@ describe('ModerationConfigService', () => { ]); // Passing `[]` should clear, not leave the existing list in place. - await sutWithPrimary.updateCustomAction(dummyOrgId, { + await sutWithPrimary.updateCustomAction(org.id, { actionId: action.id, patch: { parameters: [] }, }); const [afterClear] = await sutWithPrimary.getActions({ - orgId: dummyOrgId, + orgId: org.id, ids: [action.id], }); expect( @@ -950,8 +943,8 @@ describe('ModerationConfigService', () => { testWithAction( 'leaves parameters unchanged when patch.parameters is omitted', - async ({ action }) => { - await sutWithPrimary.updateCustomAction(dummyOrgId, { + async ({ sutWithPrimary, org, action }) => { + await sutWithPrimary.updateCustomAction(org.id, { actionId: action.id, patch: { parameters: [ @@ -964,12 +957,12 @@ describe('ModerationConfigService', () => { ], }, }); - await sutWithPrimary.updateCustomAction(dummyOrgId, { + await sutWithPrimary.updateCustomAction(org.id, { actionId: action.id, patch: { description: 'after' }, }); const [fetched] = await sutWithPrimary.getActions({ - orgId: dummyOrgId, + orgId: org.id, ids: [action.id], }); expect(fetched.description).toBe('after'); @@ -988,8 +981,8 @@ describe('ModerationConfigService', () => { testWithAction( 'should reject renaming onto an existing action name', - async ({ action }) => { - const other = await sutWithPrimary.createAction(dummyOrgId, { + async ({ sutWithPrimary, org, action }) => { + const other = await sutWithPrimary.createAction(org.id, { name: faker.random.alphaNumeric(16), description: null, type: 'CUSTOM_ACTION', @@ -997,100 +990,70 @@ describe('ModerationConfigService', () => { callbackUrlHeaders: null, callbackUrlBody: null, }); - try { - await expect( - sutWithPrimary.updateCustomAction(dummyOrgId, { - actionId: action.id, - patch: { name: other.name }, - }), - ).rejects.toThrow( - expect.objectContaining({ - type: [ErrorType.UniqueViolation], - }), - ); - } finally { - await sutWithPrimary.deleteCustomAction({ - orgId: dummyOrgId, - actionId: other.id, - }); - } + await expect( + sutWithPrimary.updateCustomAction(org.id, { + actionId: action.id, + patch: { name: other.name }, + }), + ).rejects.toThrow( + expect.objectContaining({ + type: [ErrorType.UniqueViolation], + }), + ); }, ); testWithAction( 'should replace the item-type junction when itemTypeIds is provided', - async ({ action }) => { - const itemTypeA = await sutWithPrimary.createContentType( - dummyOrgId, - { - schema: dummySchema, - description: null, - name: faker.random.alphaNumeric(16), - schemaFieldRoles: { displayName: 'fakeField' }, - }, - ); - const itemTypeB = await sutWithPrimary.createContentType( - dummyOrgId, - { - schema: dummySchema, - description: null, - name: faker.random.alphaNumeric(16), - schemaFieldRoles: { displayName: 'fakeField' }, - }, - ); + async ({ sutWithPrimary, deps, org, action }) => { + const itemTypeA = await sutWithPrimary.createContentType(org.id, { + schema: dummySchema, + description: null, + name: faker.random.alphaNumeric(16), + schemaFieldRoles: { displayName: 'fakeField' }, + }); + const itemTypeB = await sutWithPrimary.createContentType(org.id, { + schema: dummySchema, + description: null, + name: faker.random.alphaNumeric(16), + schemaFieldRoles: { displayName: 'fakeField' }, + }); - try { - await sutWithPrimary.updateCustomAction(dummyOrgId, { - actionId: action.id, - patch: {}, - itemTypeIds: [itemTypeA.id], - }); - expect( - await container.KyselyPg.selectFrom( - 'public.actions_and_item_types', - ) - .select(['item_type_id']) - .where('action_id', '=', action.id) - .execute(), - ).toEqual([{ item_type_id: itemTypeA.id }]); - - await sutWithPrimary.updateCustomAction(dummyOrgId, { - actionId: action.id, - patch: {}, - itemTypeIds: [itemTypeB.id], - }); - expect( - await container.KyselyPg.selectFrom( - 'public.actions_and_item_types', - ) - .select(['item_type_id']) - .where('action_id', '=', action.id) - .execute(), - ).toEqual([{ item_type_id: itemTypeB.id }]); - - await sutWithPrimary.updateCustomAction(dummyOrgId, { - actionId: action.id, - patch: {}, - itemTypeIds: [], - }); - expect( - await container.KyselyPg.selectFrom( - 'public.actions_and_item_types', - ) - .select(['item_type_id']) - .where('action_id', '=', action.id) - .execute(), - ).toEqual([]); - } finally { - await sutWithPrimary.deleteItemType({ - orgId: dummyOrgId, - itemTypeId: itemTypeA.id, - }); - await sutWithPrimary.deleteItemType({ - orgId: dummyOrgId, - itemTypeId: itemTypeB.id, - }); - } + await sutWithPrimary.updateCustomAction(org.id, { + actionId: action.id, + patch: {}, + itemTypeIds: [itemTypeA.id], + }); + expect( + await deps.KyselyPg.selectFrom('public.actions_and_item_types') + .select(['item_type_id']) + .where('action_id', '=', action.id) + .execute(), + ).toEqual([{ item_type_id: itemTypeA.id }]); + + await sutWithPrimary.updateCustomAction(org.id, { + actionId: action.id, + patch: {}, + itemTypeIds: [itemTypeB.id], + }); + expect( + await deps.KyselyPg.selectFrom('public.actions_and_item_types') + .select(['item_type_id']) + .where('action_id', '=', action.id) + .execute(), + ).toEqual([{ item_type_id: itemTypeB.id }]); + + await sutWithPrimary.updateCustomAction(org.id, { + actionId: action.id, + patch: {}, + itemTypeIds: [], + }); + expect( + await deps.KyselyPg.selectFrom('public.actions_and_item_types') + .select(['item_type_id']) + .where('action_id', '=', action.id) + .execute(), + ).toEqual([]); }, ); }); @@ -1098,148 +1061,127 @@ describe('ModerationConfigService', () => { describe('Delete methods', () => { describe('#deleteCustomAction', () => { - const testWithAction = makeTestWithFixture(async () => { - const action = await sutWithPrimary.createAction(dummyOrgId, { - name: faker.random.alphaNumeric(16), - description: null, - type: 'CUSTOM_ACTION', - callbackUrl: 'https://example.com', - callbackUrlHeaders: null, - callbackUrlBody: null, - }); - return { - action, - // Best-effort cleanup; the test under assertion may have already - // removed the row. - async cleanup() { - await sutWithPrimary - .deleteCustomAction({ - orgId: dummyOrgId, - actionId: action.id, - }) - .catch(() => {}); - }, - }; - }); + const testWithAction = makeTransactionalTestWithFixture( + async ({ deps }) => { + const base = await setupOrg(deps); + const action = await base.sutWithPrimary.createAction(base.org.id, { + name: faker.random.alphaNumeric(16), + description: null, + type: 'CUSTOM_ACTION', + callbackUrl: 'https://example.com', + callbackUrlHeaders: null, + callbackUrlBody: null, + }); + return { ...base, action }; + }, + ); testWithAction( 'should return true and delete the action on success', - async ({ action }) => { + async ({ sutWithPrimary, org, action }) => { const result = await sutWithPrimary.deleteCustomAction({ - orgId: dummyOrgId, + orgId: org.id, actionId: action.id, }); expect(result).toBe(true); expect( await sutWithPrimary.getActions({ - orgId: dummyOrgId, + orgId: org.id, ids: [action.id], }), ).toEqual([]); }, ); - it('should return false when the action does not exist', async () => { - const result = await sutWithPrimary.deleteCustomAction({ - orgId: dummyOrgId, - actionId: uid(), - }); - expect(result).toBe(false); - }); + testWithOrg( + 'should return false when the action does not exist', + async ({ sutWithPrimary, org }) => { + const result = await sutWithPrimary.deleteCustomAction({ + orgId: org.id, + actionId: uid(), + }); + expect(result).toBe(false); + }, + ); testWithAction( 'should return false when called with the wrong org and leave the row intact', - async ({ action }) => { - const otherOrg = await createOrg( + async ({ sutWithPrimary, deps, org, action }) => { + const { org: otherOrg } = await createOrg( { - KyselyPg: container.KyselyPg, - ModerationConfigService: container.ModerationConfigService, - ApiKeyService: container.ApiKeyService, + KyselyPg: deps.KyselyPg, + ModerationConfigService: deps.ModerationConfigService, + ApiKeyService: deps.ApiKeyService, }, uid(), ); - try { - const result = await sutWithPrimary.deleteCustomAction({ - orgId: otherOrg.org.id, - actionId: action.id, - }); - expect(result).toBe(false); - const [stillThere] = await sutWithPrimary.getActions({ - orgId: dummyOrgId, - ids: [action.id], - }); - expect(stillThere.id).toBe(action.id); - } finally { - await otherOrg.cleanup(); - } + const result = await sutWithPrimary.deleteCustomAction({ + orgId: otherOrg.id, + actionId: action.id, + }); + expect(result).toBe(false); + const [stillThere] = await sutWithPrimary.getActions({ + orgId: org.id, + ids: [action.id], + }); + expect(stillThere.id).toBe(action.id); }, ); testWithAction( 'should clean up rules_and_actions and actions_and_item_types junction rows', - async ({ action }) => { - const itemType = await sutWithPrimary.createContentType( - dummyOrgId, - { - schema: dummySchema, - description: null, - name: faker.random.alphaNumeric(16), - schemaFieldRoles: { displayName: 'fakeField' }, - }, - ); - const rule = await createRule(container.KyselyPg, dummyOrgId); + async ({ sutWithPrimary, deps, org, action }) => { + const itemType = await sutWithPrimary.createContentType(org.id, { + schema: dummySchema, + description: null, + name: faker.random.alphaNumeric(16), + schemaFieldRoles: { displayName: 'fakeField' }, + }); + const rule = await createRule(deps.KyselyPg, org.id); - await container.KyselyPg.insertInto('public.actions_and_item_types') + await deps.KyselyPg.insertInto('public.actions_and_item_types') .values({ action_id: action.id, item_type_id: itemType.id }) .execute(); - await container.KyselyPg.insertInto('public.rules_and_actions') + await deps.KyselyPg.insertInto('public.rules_and_actions') .values({ action_id: action.id, rule_id: rule.id }) .execute(); - try { - const result = await sutWithPrimary.deleteCustomAction({ - orgId: dummyOrgId, - actionId: action.id, - }); - expect(result).toBe(true); - expect( - await container.KyselyPg.selectFrom( - 'public.actions_and_item_types', - ) - .select(['action_id']) - .where('action_id', '=', action.id) - .execute(), - ).toEqual([]); - expect( - await container.KyselyPg.selectFrom('public.rules_and_actions') - .select(['action_id']) - .where('action_id', '=', action.id) - .execute(), - ).toEqual([]); - } finally { - await rule.destroy(); - await sutWithPrimary.deleteItemType({ - orgId: dummyOrgId, - itemTypeId: itemType.id, - }); - } + const result = await sutWithPrimary.deleteCustomAction({ + orgId: org.id, + actionId: action.id, + }); + expect(result).toBe(true); + expect( + await deps.KyselyPg.selectFrom('public.actions_and_item_types') + .select(['action_id']) + .where('action_id', '=', action.id) + .execute(), + ).toEqual([]); + expect( + await deps.KyselyPg.selectFrom('public.rules_and_actions') + .select(['action_id']) + .where('action_id', '=', action.id) + .execute(), + ).toEqual([]); }, ); }); }); describe('#getActionsForItemType', () => { - const testWithItemTypeAndActions = makeTestWithFixture(async () => { - const itemType = await sutWithPrimary.createContentType(dummyOrgId, { - schema: dummySchema, - description: null, - name: faker.random.alphaNumeric(16), - schemaFieldRoles: { displayName: 'fakeField' }, - }); + const testWithItemTypeAndActions = makeTransactionalTestWithFixture( + async ({ deps }) => { + const base = await setupOrg(deps); + const { sutWithPrimary, org } = base; - const viaJunctionAction = await sutWithPrimary.createAction( - dummyOrgId, - { + const itemType = await sutWithPrimary.createContentType(org.id, { + schema: dummySchema, + description: null, + name: faker.random.alphaNumeric(16), + schemaFieldRoles: { displayName: 'fakeField' }, + }); + + const viaJunctionAction = await sutWithPrimary.createAction(org.id, { name: faker.random.alphaNumeric(16), description: null, type: 'CUSTOM_ACTION', @@ -1247,76 +1189,62 @@ describe('ModerationConfigService', () => { callbackUrlHeaders: null, callbackUrlBody: null, itemTypeIds: [itemType.id], - }, - ); + }); - const viaAppliesAllAction = await sutWithPrimary.createAction( - dummyOrgId, - { + const viaAppliesAllAction = await sutWithPrimary.createAction( + org.id, + { + name: faker.random.alphaNumeric(16), + description: null, + type: 'CUSTOM_ACTION', + callbackUrl: 'https://example.com', + callbackUrlHeaders: null, + callbackUrlBody: null, + }, + ); + await deps.KyselyPg.updateTable('public.actions') + .set({ applies_to_all_items_of_kind: ['CONTENT'] }) + .where('id', '=', viaAppliesAllAction.id) + .execute(); + + // Action satisfying both branches; result should still include it once. + const viaBothAction = await sutWithPrimary.createAction(org.id, { name: faker.random.alphaNumeric(16), description: null, type: 'CUSTOM_ACTION', callbackUrl: 'https://example.com', callbackUrlHeaders: null, callbackUrlBody: null, - }, - ); - await container.KyselyPg.updateTable('public.actions') - .set({ applies_to_all_items_of_kind: ['CONTENT'] }) - .where('id', '=', viaAppliesAllAction.id) - .execute(); - - // Action satisfying both branches; result should still include it once. - const viaBothAction = await sutWithPrimary.createAction(dummyOrgId, { - name: faker.random.alphaNumeric(16), - description: null, - type: 'CUSTOM_ACTION', - callbackUrl: 'https://example.com', - callbackUrlHeaders: null, - callbackUrlBody: null, - itemTypeIds: [itemType.id], - }); - await container.KyselyPg.updateTable('public.actions') - .set({ applies_to_all_items_of_kind: ['CONTENT'] }) - .where('id', '=', viaBothAction.id) - .execute(); + itemTypeIds: [itemType.id], + }); + await deps.KyselyPg.updateTable('public.actions') + .set({ applies_to_all_items_of_kind: ['CONTENT'] }) + .where('id', '=', viaBothAction.id) + .execute(); - return { - itemType, - viaJunctionAction, - viaAppliesAllAction, - viaBothAction, - async cleanup() { - await Promise.all( - [ - viaJunctionAction.id, - viaAppliesAllAction.id, - viaBothAction.id, - ].map(async (id) => - sutWithPrimary.deleteCustomAction({ - orgId: dummyOrgId, - actionId: id, - }), - ), - ); - await sutWithPrimary.deleteItemType({ - orgId: dummyOrgId, - itemTypeId: itemType.id, - }); - }, - }; - }); + return { + ...base, + itemType, + viaJunctionAction, + viaAppliesAllAction, + viaBothAction, + }; + }, + ); testWithItemTypeAndActions( 'should return actions from both branches, deduped, scoped to the org', async ({ + sutWithPrimary, + deps, + org, itemType, viaJunctionAction, viaAppliesAllAction, viaBothAction, }) => { const result = await sutWithPrimary.getActionsForItemType({ - orgId: dummyOrgId, + orgId: org.id, itemTypeId: itemType.id, itemTypeKind: 'CONTENT', readFromReplica: false, @@ -1337,63 +1265,54 @@ describe('ModerationConfigService', () => { // Calling with a different org should never surface this org's // applies-to-all rows (they'd otherwise leak across orgs since the // ANY(...) predicate alone has no tenant scope). - const otherOrg = await createOrg( + const { org: otherOrg } = await createOrg( { - KyselyPg: container.KyselyPg, - ModerationConfigService: container.ModerationConfigService, - ApiKeyService: container.ApiKeyService, + KyselyPg: deps.KyselyPg, + ModerationConfigService: deps.ModerationConfigService, + ApiKeyService: deps.ApiKeyService, }, uid(), ); - try { - const otherResult = await sutWithPrimary.getActionsForItemType({ - orgId: otherOrg.org.id, - itemTypeId: itemType.id, - itemTypeKind: 'CONTENT', - readFromReplica: false, - }); - expect( - otherResult.filter((it) => it.actionType === 'CUSTOM_ACTION'), - ).toEqual([]); - } finally { - await otherOrg.cleanup(); - } + const otherResult = await sutWithPrimary.getActionsForItemType({ + orgId: otherOrg.id, + itemTypeId: itemType.id, + itemTypeKind: 'CONTENT', + readFromReplica: false, + }); + expect( + otherResult.filter((it) => it.actionType === 'CUSTOM_ACTION'), + ).toEqual([]); }, ); }); describe('#getActionsForRuleId', () => { - const testWithRuleAndAction = makeTestWithFixture(async () => { - const rule = await createRule(container.KyselyPg, dummyOrgId); - const action = await sutWithPrimary.createAction(dummyOrgId, { - name: faker.random.alphaNumeric(16), - description: null, - type: 'CUSTOM_ACTION', - callbackUrl: 'https://example.com', - callbackUrlHeaders: null, - callbackUrlBody: null, - }); - await container.KyselyPg.insertInto('public.rules_and_actions') - .values({ action_id: action.id, rule_id: rule.id }) - .execute(); - return { - rule, - action, - async cleanup() { - await sutWithPrimary.deleteCustomAction({ - orgId: dummyOrgId, - actionId: action.id, - }); - await rule.destroy(); - }, - }; - }); + const testWithRuleAndAction = makeTransactionalTestWithFixture( + async ({ deps }) => { + const base = await setupOrg(deps); + const { sutWithPrimary, org } = base; + + const rule = await createRule(deps.KyselyPg, org.id); + const action = await sutWithPrimary.createAction(org.id, { + name: faker.random.alphaNumeric(16), + description: null, + type: 'CUSTOM_ACTION', + callbackUrl: 'https://example.com', + callbackUrlHeaders: null, + callbackUrlBody: null, + }); + await deps.KyselyPg.insertInto('public.rules_and_actions') + .values({ action_id: action.id, rule_id: rule.id }) + .execute(); + return { ...base, rule, action }; + }, + ); testWithRuleAndAction( 'should return actions for a rule scoped to the caller org', - async ({ rule, action }) => { + async ({ sutWithPrimary, org, rule, action }) => { const result = await sutWithPrimary.getActionsForRuleId({ - orgId: dummyOrgId, + orgId: org.id, ruleId: rule.id, readFromReplica: false, }); @@ -1403,25 +1322,21 @@ describe('ModerationConfigService', () => { testWithRuleAndAction( 'should not return actions when called with a different org', - async ({ rule }) => { - const otherOrg = await createOrg( + async ({ sutWithPrimary, deps, rule }) => { + const { org: otherOrg } = await createOrg( { - KyselyPg: container.KyselyPg, - ModerationConfigService: container.ModerationConfigService, - ApiKeyService: container.ApiKeyService, + KyselyPg: deps.KyselyPg, + ModerationConfigService: deps.ModerationConfigService, + ApiKeyService: deps.ApiKeyService, }, uid(), ); - try { - const result = await sutWithPrimary.getActionsForRuleId({ - orgId: otherOrg.org.id, - ruleId: rule.id, - readFromReplica: false, - }); - expect(result).toEqual([]); - } finally { - await otherOrg.cleanup(); - } + const result = await sutWithPrimary.getActionsForRuleId({ + orgId: otherOrg.id, + ruleId: rule.id, + readFromReplica: false, + }); + expect(result).toEqual([]); }, ); }); @@ -1429,46 +1344,40 @@ describe('ModerationConfigService', () => { describe('Policy returning methods', () => { describe('Read methods', () => { - it('should query from the proper db', async () => { - await testReadReplicaUse('getPolicies', { orgId: dummyOrgId }); - }); + testWithOrg( + 'should query from the proper db', + async ({ sutWithPrimary, sutWithReadReplica, org }) => { + await expectReadReplicaUse( + { sutWithPrimary, sutWithReadReplica }, + 'getPolicies', + { orgId: org.id }, + ); + }, + ); // TODO: Fill in this test once we've implemented the policy mutations - it.skip('should return all policies, properly formatted', async () => { - const res = await sutWithPrimary.getPolicies({ orgId: dummyOrgId }); - expect(res).toHaveLength(createdPolicies.length); - expect(res).toEqual(expect.arrayContaining(createdPolicies)); - }); + testWithOrg.skip( + 'should return all policies, properly formatted', + async ({ sutWithPrimary, org }) => { + const createdPolicies = [] as Policy[]; + const res = await sutWithPrimary.getPolicies({ orgId: org.id }); + expect(res).toHaveLength(createdPolicies.length); + expect(res).toEqual(expect.arrayContaining(createdPolicies)); + }, + ); }); describe('Mutations', () => { - const testWithUserAndOrg = makeTestWithFixture(async () => { - const { org, cleanup: orgCleanup } = await createOrg( - { - KyselyPg: container.KyselyPg, - ModerationConfigService: container.ModerationConfigService, - ApiKeyService: container.ApiKeyService, - }, - uid(), - ); - - const { user, cleanup: userCleanup } = await createUser( - container.KyselyPg, - org.id, - ); - - return { - org, - user, - async cleanup() { - await userCleanup(); - await orgCleanup(); - }, - }; - }); + const testWithUserAndOrg = makeTransactionalTestWithFixture( + async ({ deps }) => { + const base = await setupOrg(deps); + const { user } = await createUser(deps.KyselyPg, base.org.id); + return { ...base, user }; + }, + ); testWithUserAndOrg( 'should create a root policy', - async ({ org, user }) => { + async ({ sutWithPrimary, org, user }) => { const policy = await sutWithPrimary.createPolicy({ orgId: org.id, policy: { @@ -1493,7 +1402,7 @@ describe('ModerationConfigService', () => { testWithUserAndOrg( 'should create parent and child policies', - async ({ org, user }) => { + async ({ sutWithPrimary, org, user }) => { const parentPolicy = await sutWithPrimary.createPolicy({ orgId: org.id, policy: { @@ -1536,7 +1445,7 @@ describe('ModerationConfigService', () => { testWithUserAndOrg( 'should update an existing policy', - async ({ org, user }) => { + async ({ sutWithPrimary, org, user }) => { const policy = await sutWithPrimary.createPolicy({ orgId: org.id, policy: { @@ -1580,7 +1489,7 @@ describe('ModerationConfigService', () => { testWithUserAndOrg( 'Prevent creation of policy with the same name as an existing policy', - async ({ org, user }) => { + async ({ sutWithPrimary, org, user }) => { await sutWithPrimary.createPolicy({ orgId: org.id, policy: { @@ -1621,135 +1530,153 @@ describe('ModerationConfigService', () => { }); }); describe('TextBank-returning methods', () => { - let createdTextBanks = [] as { - id: string; - orgId: string; - name: string; - description: string | null; - type: 'STRING' | 'REGEX'; - createdAt: Date; - updatedAt: Date; - ownerId: string | null; - strings: string[]; - }[]; - describe('Mutations', () => { describe('#createTextBank', () => { - it('should create a text bank', async () => { - const textBank = await sutWithPrimary.createTextBank(dummyOrgId, { - name: 'Test Text Bank', - description: 'Test description', - type: 'STRING' as const, - strings: ['test entry 1', 'test entry 2'], - }); - - expect(textBank).toEqual( - expect.objectContaining({ - createdAt: expect.any(Date), - updatedAt: expect.any(Date), - description: 'Test description', - id: expect.any(String), + testWithOrg( + 'should create a text bank', + async ({ sutWithPrimary, org }) => { + const textBank = await sutWithPrimary.createTextBank(org.id, { name: 'Test Text Bank', - orgId: expect.any(String), - ownerId: null, + description: 'Test description', + type: 'STRING' as const, strings: ['test entry 1', 'test entry 2'], - type: 'STRING', - }), - ); + }); - expect(textBank.orgId).toBe(dummyOrgId); - createdTextBanks = [...createdTextBanks, textBank]; - }); + expect(textBank).toEqual( + expect.objectContaining({ + createdAt: expect.any(Date), + updatedAt: expect.any(Date), + description: 'Test description', + id: expect.any(String), + name: 'Test Text Bank', + orgId: expect.any(String), + ownerId: null, + strings: ['test entry 1', 'test entry 2'], + type: 'STRING', + }), + ); + + expect(textBank.orgId).toBe(org.id); + }, + ); }); }); describe('Read methods', () => { describe('#getTextBanks', () => { - it('should return all text banks, properly formatted', async () => { - const res = await sutWithPrimary.getTextBanks({ orgId: dummyOrgId }); - expect(res).toHaveLength(createdTextBanks.length); - expect(res).toEqual(expect.arrayContaining(createdTextBanks)); - }); + testWithOrg( + 'should return all text banks, properly formatted', + async ({ sutWithPrimary, org }) => { + const createdTextBanks = [ + await sutWithPrimary.createTextBank(org.id, { + name: 'Test Text Bank 1', + description: 'Test description', + type: 'STRING' as const, + strings: ['test entry 1', 'test entry 2'], + }), + await sutWithPrimary.createTextBank(org.id, { + name: 'Test Text Bank 2', + description: null, + type: 'REGEX' as const, + strings: ['.*'], + }), + ]; + + const res = await sutWithPrimary.getTextBanks({ orgId: org.id }); + expect(res).toHaveLength(createdTextBanks.length); + expect(res).toEqual(expect.arrayContaining(createdTextBanks)); + }, + ); }); describe('#getTextBank', () => { - it('should return a specific text bank, properly formatted', async () => { - const textBank = createdTextBanks[0]; - const res = await sutWithPrimary.getTextBank({ - orgId: dummyOrgId, - id: textBank.id, - }); - expect(res).toEqual(textBank); - }); + testWithOrg( + 'should return a specific text bank, properly formatted', + async ({ sutWithPrimary, org }) => { + const textBank = await sutWithPrimary.createTextBank(org.id, { + name: 'Test Text Bank', + description: 'Test description', + type: 'STRING' as const, + strings: ['test entry 1', 'test entry 2'], + }); + + const res = await sutWithPrimary.getTextBank({ + orgId: org.id, + id: textBank.id, + }); + expect(res).toEqual(textBank); + }, + ); }); }); }); describe('#getItemType', () => { - const testWithOneItemTypeFixture = makeTestWithFixture(async () => { - const itemType = await sutWithPrimary.createContentType(dummyOrgId, { - schema: dummySchema, - description: null, - name: faker.random.alphaNumeric(16), - schemaFieldRoles: { - displayName: 'fakeField', - }, - }); + const testWithOneItemTypeFixture = makeTransactionalTestWithFixture( + async ({ deps }) => { + const base = await setupOrg(deps); + const itemType = await base.sutWithPrimary.createContentType( + base.org.id, + { + schema: dummySchema, + description: null, + name: faker.random.alphaNumeric(16), + schemaFieldRoles: { + displayName: 'fakeField', + }, + }, + ); - return { - itemType, - async cleanup() { - await sutWithPrimary.deleteItemType({ - orgId: dummyOrgId, - itemTypeId: itemType.id, - }); - }, - }; - }); + return { ...base, itemType }; + }, + ); - const testWithTwoItemTypesFixture = makeTestWithFixture(async () => { - const itemType = await sutWithPrimary.createContentType(dummyOrgId, { - schema: dummySchema, - description: null, - name: faker.random.alphaNumeric(16), - schemaFieldRoles: { - displayName: 'fakeField', - }, - }); + const testWithTwoItemTypesFixture = makeTransactionalTestWithFixture( + async ({ deps }) => { + const base = await setupOrg(deps); + const itemType = await base.sutWithPrimary.createContentType( + base.org.id, + { + schema: dummySchema, + description: null, + name: faker.random.alphaNumeric(16), + schemaFieldRoles: { + displayName: 'fakeField', + }, + }, + ); - const newItemType = await sutWithPrimary.updateContentType(dummyOrgId, { - id: itemType.id, - name: faker.random.alphaNumeric(16), - schemaFieldRoles: { - creatorId: undefined, - }, - }); + const newItemType = await base.sutWithPrimary.updateContentType( + base.org.id, + { + id: itemType.id, + name: faker.random.alphaNumeric(16), + schemaFieldRoles: { + creatorId: undefined, + }, + }, + ); - return { - itemType, - newItemType, - async cleanup() { - await sutWithPrimary.deleteItemType({ - orgId: dummyOrgId, - itemTypeId: itemType.id, - }); - }, - }; - }); + return { ...base, itemType, newItemType }; + }, + ); - it("Should return undefined if an item type with the given ID doesn't exist", async () => { - const itemType = await sutWithPrimary.getItemType({ - orgId: dummyOrgId, - itemTypeSelector: { id: 'fakeId' }, - }); + testWithOrg( + "Should return undefined if an item type with the given ID doesn't exist", + async ({ sutWithPrimary, org }) => { + const itemType = await sutWithPrimary.getItemType({ + orgId: org.id, + itemTypeSelector: { id: 'fakeId' }, + }); - expect(itemType).toBeUndefined(); - }); + expect(itemType).toBeUndefined(); + }, + ); testWithOneItemTypeFixture( 'Should return a partial item type if requested for a selector without a version', - async ({ itemType }) => { + async ({ sutWithPrimary, org, itemType }) => { const fetched = await sutWithPrimary.getItemType({ - orgId: dummyOrgId, + orgId: org.id, itemTypeSelector: { id: itemType.id, schemaVariant: 'partial' }, }); @@ -1759,9 +1686,9 @@ describe('ModerationConfigService', () => { ); testWithTwoItemTypesFixture( 'Should return a partial item type if requested for a selector with a version', - async ({ itemType, newItemType }) => { + async ({ sutWithPrimary, org, itemType, newItemType }) => { const fetched = await sutWithPrimary.getItemType({ - orgId: dummyOrgId, + orgId: org.id, itemTypeSelector: { id: itemType.id, schemaVariant: 'partial', @@ -1772,18 +1699,13 @@ describe('ModerationConfigService', () => { expect(fetched).not.toBeNull(); expect(fetched!.name).toEqual(newItemType.name); fetched!.schema.forEach((it) => expect(it.required).toEqual(false)); - - await sutWithPrimary.deleteItemType({ - itemTypeId: itemType.id, - orgId: dummyOrgId, - }); }, ); testWithTwoItemTypesFixture( 'Should return latest item type if only an ID is provided', - async ({ itemType, newItemType }) => { + async ({ sutWithPrimary, org, itemType, newItemType }) => { const fetched = await sutWithPrimary.getItemType({ - orgId: dummyOrgId, + orgId: org.id, itemTypeSelector: { id: itemType.id }, }); @@ -1791,10 +1713,54 @@ describe('ModerationConfigService', () => { expect(fetched!.name).toEqual(newItemType.name); }, ); - testWithOneItemTypeFixture( + // Fetching a *historical* version needs two versions with distinct + // timestamps. `item_type_versions` is a view over the system-versioned + // `item_types` table, whose `version` is `transaction_timestamp()`. The + // rollback harness runs the whole test in one transaction, so a create + + // update there share a timestamp and collapse into a single version — there + // is no older version left to fetch. This test therefore commits its two + // versions through the real container (separate transactions => distinct + // timestamps) and cleans up after itself; it's still self-contained. + const testWithHistoricalItemType = makeTestWithFixture(async () => { + const { container } = await getBottle(); + const sut = new ModerationConfigService( + container.KyselyPg, + container.KyselyPgReadReplica, + async () => {}, + ); + const { org, cleanup: orgCleanup } = await createOrg( + { + KyselyPg: container.KyselyPg, + ModerationConfigService: container.ModerationConfigService, + ApiKeyService: container.ApiKeyService, + }, + uid(), + ); + const itemType = await sut.createContentType(org.id, { + schema: dummySchema, + description: null, + name: faker.random.alphaNumeric(16), + schemaFieldRoles: { displayName: 'fakeField' }, + }); + + return { + sut, + org, + itemType, + async cleanup() { + await orgCleanup(); + await Promise.all([ + container.KyselyPg.destroy(), + container.KyselyPgReadReplica.destroy(), + ]); + }, + }; + }); + + testWithHistoricalItemType( 'Should return requested item type version', - async ({ itemType }) => { - await sutWithPrimary.updateContentType(dummyOrgId, { + async ({ sut, org, itemType }) => { + await sut.updateContentType(org.id, { id: itemType.id, name: faker.random.alphaNumeric(16), schemaFieldRoles: { @@ -1802,8 +1768,8 @@ describe('ModerationConfigService', () => { }, }); - const fetched = await sutWithPrimary.getItemType({ - orgId: dummyOrgId, + const fetched = await sut.getItemType({ + orgId: org.id, itemTypeSelector: { id: itemType.id, version: itemType.version }, }); diff --git a/server/services/userStrikeService/userStrikeService.test.ts b/server/services/userStrikeService/userStrikeService.test.ts index 3dbb5cb..c82aedd 100644 --- a/server/services/userStrikeService/userStrikeService.test.ts +++ b/server/services/userStrikeService/userStrikeService.test.ts @@ -1,237 +1,241 @@ import { uid } from 'uid'; -import getBottle, { type Dependencies } from '../../iocContainer/index.js'; -import { type UserStrikeService } from './index.js'; +import { makeTransactionalTestWithFixture } from '../../test/harness/transactionalTest.js'; -describe('Item Investigation Service', () => { - let container: Dependencies; - let userStrikeService: UserStrikeService; +describe('User Strike Service', () => { + const testWithStrikes = makeTransactionalTestWithFixture( + async ({ deps }) => ({ + userStrikeService: deps.UserStrikeService, + }), + ); - beforeAll(async () => { - // The mutation should be ok here since this is initial setup in a - // beforeAll; it doesn't involve reset state for each test in the suite + testWithStrikes( + 'Should properly calculate strike counts for a given user', + async ({ userStrikeService }) => { + const fakeUserId = { id: uid(), typeId: uid() }; + const fakeOrgId = uid(); + await userStrikeService.applyUserStrike( + fakeOrgId, + fakeUserId, + 'fakePolicyId', + 1, + ); + const strikeCount1 = await userStrikeService.getUserStrikeValue( + fakeOrgId, + fakeUserId, + ); + expect(strikeCount1).toEqual(1); + await userStrikeService.applyUserStrike( + fakeOrgId, + fakeUserId, + 'fakePolicyId1', + 1, + ); + const strikeCount2 = await userStrikeService.getUserStrikeValue( + fakeOrgId, + fakeUserId, + ); + expect(strikeCount2).toEqual(2); + await userStrikeService.applyUserStrike( + fakeOrgId, + fakeUserId, + 'fakePolicyId2', + 10, + ); + const strikeCount3 = await userStrikeService.getUserStrikeValue( + fakeOrgId, + fakeUserId, + ); + expect(strikeCount3).toEqual(12); + }, + ); - ({ container } = await getBottle()); - userStrikeService = container.UserStrikeService; - }); - afterAll(async () => { - await container.closeSharedResourcesForShutdown(); - }); - - test('Should properly calculate strike counts for a given user', async () => { - const fakeUserId = { id: uid(), typeId: uid() }; - const fakeOrgId = uid(); - await userStrikeService.applyUserStrike( - fakeOrgId, - fakeUserId, - 'fakePolicyId', - 1, - ); - const strikeCount1 = await userStrikeService.getUserStrikeValue( - fakeOrgId, - fakeUserId, - ); - expect(strikeCount1).toEqual(1); - await userStrikeService.applyUserStrike( - fakeOrgId, - fakeUserId, - 'fakePolicyId1', - 1, - ); - const strikeCount2 = await userStrikeService.getUserStrikeValue( - fakeOrgId, - fakeUserId, - ); - expect(strikeCount2).toEqual(2); - await userStrikeService.applyUserStrike( - fakeOrgId, - fakeUserId, - 'fakePolicyId2', - 10, - ); - const strikeCount3 = await userStrikeService.getUserStrikeValue( - fakeOrgId, - fakeUserId, - ); - expect(strikeCount3).toEqual(12); - }); - - test('Should only apply strike for most severe policy violation', async () => { - const testActions = [ - { - orgId: 'fakeOrgId', - action: { - id: 'fakeActionId1', - name: 'testAction1', - description: null, - applyUserStrikes: true, + testWithStrikes( + 'Should only apply strike for most severe policy violation', + async ({ userStrikeService }) => { + const testActions = [ + { orgId: 'fakeOrgId', - penalty: 'NONE' as const, - callbackUrl: 'fakeCallbackUrl1', - callbackUrlHeaders: null, - callbackUrlBody: null, - customMrtApiParams: null, - actionType: 'CUSTOM_ACTION' as const, - }, - targetItem: { itemId: 'fakeItemId1', itemType: 'fakeItemType1' }, - matchingRules: undefined, - ruleEnvironment: undefined, - policies: [ - { - id: 'fakePolicyId1', - name: 'testPolicy1', - userStrikeCount: 1, - penalty: 'LOW' as const, + action: { + id: 'fakeActionId1', + name: 'testAction1', + description: null, + applyUserStrikes: true, + orgId: 'fakeOrgId', + penalty: 'NONE' as const, + callbackUrl: 'fakeCallbackUrl1', + callbackUrlHeaders: null, + callbackUrlBody: null, + customMrtApiParams: null, + actionType: 'CUSTOM_ACTION' as const, }, - { - id: 'severePolicyId', - name: 'testPolicy2', - userStrikeCount: 2, - penalty: 'LOW' as const, - }, - ], - }, - ]; - const mostSeverePolicyViolation = - userStrikeService.findMostSeverePolicyViolationFromActions(testActions); - if (mostSeverePolicyViolation === undefined) { - throw new Error('mostSeverePolicyViolation is undefined'); - } - expect(mostSeverePolicyViolation.id).toEqual('severePolicyId'); - }); - test('findMostSeverePolicyViolationFromActions should return undefined if no actions apply user strikes', async () => { - const testActions = [ - { - orgId: 'fakeOrgId', - action: { - id: 'fakeActionId1', - name: 'testAction1', - description: null, - applyUserStrikes: false, - orgId: 'fakeOrgId', - penalty: 'NONE' as const, - callbackUrl: 'fakeCallbackUrl1', - callbackUrlHeaders: null, - callbackUrlBody: null, - customMrtApiParams: null, - actionType: 'CUSTOM_ACTION' as const, + targetItem: { itemId: 'fakeItemId1', itemType: 'fakeItemType1' }, + matchingRules: undefined, + ruleEnvironment: undefined, + policies: [ + { + id: 'fakePolicyId1', + name: 'testPolicy1', + userStrikeCount: 1, + penalty: 'LOW' as const, + }, + { + id: 'severePolicyId', + name: 'testPolicy2', + userStrikeCount: 2, + penalty: 'LOW' as const, + }, + ], }, - targetItem: { itemId: 'fakeItemId1', itemType: 'fakeItemType1' }, - matchingRules: undefined, - ruleEnvironment: undefined, - policies: [ - { - id: 'fakePolicyId1', - name: 'testPolicy1', - userStrikeCount: 1, - penalty: 'LOW' as const, - }, - { - id: 'severePolicyId', - name: 'testPolicy2', - userStrikeCount: 2, - penalty: 'LOW' as const, - }, - ], - }, - { - orgId: 'fakeOrgId', - action: { - id: 'fakeActionId1', - name: 'testAction1', - description: null, - applyUserStrikes: false, + ]; + const mostSeverePolicyViolation = + userStrikeService.findMostSeverePolicyViolationFromActions(testActions); + if (mostSeverePolicyViolation === undefined) { + throw new Error('mostSeverePolicyViolation is undefined'); + } + expect(mostSeverePolicyViolation.id).toEqual('severePolicyId'); + }, + ); + + testWithStrikes( + 'findMostSeverePolicyViolationFromActions should return undefined if no actions apply user strikes', + async ({ userStrikeService }) => { + const testActions = [ + { orgId: 'fakeOrgId', - penalty: 'NONE' as const, - callbackUrl: 'fakeCallbackUrl1', - callbackUrlHeaders: null, - callbackUrlBody: null, - customMrtApiParams: null, - actionType: 'CUSTOM_ACTION' as const, - }, - targetItem: { itemId: 'fakeItemId1', itemType: 'fakeItemType1' }, - matchingRules: undefined, - ruleEnvironment: undefined, - policies: [ - { - id: 'fakePolicyId1', - name: 'testPolicy1', - userStrikeCount: 1, - penalty: 'LOW' as const, + action: { + id: 'fakeActionId1', + name: 'testAction1', + description: null, + applyUserStrikes: false, + orgId: 'fakeOrgId', + penalty: 'NONE' as const, + callbackUrl: 'fakeCallbackUrl1', + callbackUrlHeaders: null, + callbackUrlBody: null, + customMrtApiParams: null, + actionType: 'CUSTOM_ACTION' as const, }, - { - id: 'severePolicyId', - name: 'testPolicy2', - userStrikeCount: 2, - penalty: 'LOW' as const, + targetItem: { itemId: 'fakeItemId1', itemType: 'fakeItemType1' }, + matchingRules: undefined, + ruleEnvironment: undefined, + policies: [ + { + id: 'fakePolicyId1', + name: 'testPolicy1', + userStrikeCount: 1, + penalty: 'LOW' as const, + }, + { + id: 'severePolicyId', + name: 'testPolicy2', + userStrikeCount: 2, + penalty: 'LOW' as const, + }, + ], + }, + { + orgId: 'fakeOrgId', + action: { + id: 'fakeActionId1', + name: 'testAction1', + description: null, + applyUserStrikes: false, + orgId: 'fakeOrgId', + penalty: 'NONE' as const, + callbackUrl: 'fakeCallbackUrl1', + callbackUrlHeaders: null, + callbackUrlBody: null, + customMrtApiParams: null, + actionType: 'CUSTOM_ACTION' as const, }, - ], - }, - ]; - const mostSeverePolicyViolation = - userStrikeService.findMostSeverePolicyViolationFromActions(testActions); - expect(mostSeverePolicyViolation).toBeUndefined(); - }); + targetItem: { itemId: 'fakeItemId1', itemType: 'fakeItemType1' }, + matchingRules: undefined, + ruleEnvironment: undefined, + policies: [ + { + id: 'fakePolicyId1', + name: 'testPolicy1', + userStrikeCount: 1, + penalty: 'LOW' as const, + }, + { + id: 'severePolicyId', + name: 'testPolicy2', + userStrikeCount: 2, + penalty: 'LOW' as const, + }, + ], + }, + ]; + const mostSeverePolicyViolation = + userStrikeService.findMostSeverePolicyViolationFromActions(testActions); + expect(mostSeverePolicyViolation).toBeUndefined(); + }, + ); - test('Should properly calculate strike values using getAllUserStrikeCountsForOrg', async () => { - const fakeTypeId = uid(); - const fakeUserId1 = { id: uid(), typeId: fakeTypeId }; - const fakeUserId2 = { id: uid(), typeId: fakeTypeId }; - const fakeUserId3 = { id: uid(), typeId: fakeTypeId }; - const fakeOrgId = uid(); - // 1 strike for user 1 - await userStrikeService.applyUserStrike( - fakeOrgId, - fakeUserId1, - 'fakePolicyId', - 1, - ); - // 2 strikes for user 2 - await userStrikeService.applyUserStrike( - fakeOrgId, - fakeUserId2, - 'fakePolicyId', - 1, - ); - await userStrikeService.applyUserStrike( - fakeOrgId, - fakeUserId2, - 'fakePolicyId1', - 1, - ); - // 3 strikes for user 3 - await userStrikeService.applyUserStrike( - fakeOrgId, - fakeUserId3, - 'fakePolicyId1', - 1, - ); - await userStrikeService.applyUserStrike( - fakeOrgId, - fakeUserId3, - 'fakePolicyId2', - 1, - ); - await userStrikeService.applyUserStrike( - fakeOrgId, - fakeUserId3, - 'fakePolicyId3', - 1, - ); + testWithStrikes( + 'Should properly calculate strike values using getAllUserStrikeCountsForOrg', + async ({ userStrikeService }) => { + const fakeTypeId = uid(); + const fakeUserId1 = { id: uid(), typeId: fakeTypeId }; + const fakeUserId2 = { id: uid(), typeId: fakeTypeId }; + const fakeUserId3 = { id: uid(), typeId: fakeTypeId }; + const fakeOrgId = uid(); + // 1 strike for user 1 + await userStrikeService.applyUserStrike( + fakeOrgId, + fakeUserId1, + 'fakePolicyId', + 1, + ); + // 2 strikes for user 2 + await userStrikeService.applyUserStrike( + fakeOrgId, + fakeUserId2, + 'fakePolicyId', + 1, + ); + await userStrikeService.applyUserStrike( + fakeOrgId, + fakeUserId2, + 'fakePolicyId1', + 1, + ); + // 3 strikes for user 3 + await userStrikeService.applyUserStrike( + fakeOrgId, + fakeUserId3, + 'fakePolicyId1', + 1, + ); + await userStrikeService.applyUserStrike( + fakeOrgId, + fakeUserId3, + 'fakePolicyId2', + 1, + ); + await userStrikeService.applyUserStrike( + fakeOrgId, + fakeUserId3, + 'fakePolicyId3', + 1, + ); - const userStrikesForOrg = - await userStrikeService.getAllUserStrikeCountsForOrg(fakeOrgId); - const user1 = userStrikesForOrg.find( - (it) => it.user_identifier.id === fakeUserId1.id, - ); - expect(user1?.strike_count).toEqual(1); - const user2 = userStrikesForOrg.find( - (it) => it.user_identifier.id === fakeUserId2.id, - ); - expect(user2?.strike_count).toEqual(2); - const user3 = userStrikesForOrg.find( - (it) => it.user_identifier.id === fakeUserId3.id, - ); - expect(user3?.strike_count).toEqual(3); - }); + const userStrikesForOrg = + await userStrikeService.getAllUserStrikeCountsForOrg(fakeOrgId); + const user1 = userStrikesForOrg.find( + (it) => it.user_identifier.id === fakeUserId1.id, + ); + expect(user1?.strike_count).toEqual(1); + const user2 = userStrikesForOrg.find( + (it) => it.user_identifier.id === fakeUserId2.id, + ); + expect(user2?.strike_count).toEqual(2); + const user3 = userStrikesForOrg.find( + (it) => it.user_identifier.id === fakeUserId3.id, + ); + expect(user3?.strike_count).toEqual(3); + }, + ); });