From 61a7e5eccb95f616741bdc5c5da1cb72c651c808 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tao=20Bojl=C3=A9n?= Date: Tue, 14 Jul 2026 10:44:28 +0100 Subject: [PATCH] test: migrate DB-backed tests to transactional harness (#821) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: close express-session store on api shutdown connect-pg-simple keeps a recurring pruneSessions timer that calls pool.query on its pool. makeApiServer created the store with the shared KyselyPgPool but never closed it, so the timer kept running after shutdown. In tests this leaked a setInterval per makeMockedServer() call that fired pool.query on the harness's already-closed pinned connection after each test, logging "Failed to prune sessions: Client was closed and is not queryable" indefinitely and hanging the test worker — the loop that forced the manual cancellation of CI run 27951870325. Hold a reference to the store instance and call its close() in the shutdown path. ownsPg is false (we pass in our own pool), so close() only stops the prune timer and won't end the shared pool. Co-Authored-By: Claude Opus 4.8 * test: migrate DB-backed tests to transactional harness Move the remaining DB-backed server tests onto the transaction-rollback harness from #732 so they get per-test isolation with no hand-written cleanup. - Convert the makeMockedServer/getBottle-based tests to makeTransactionalTestWithFixture, dropping manual org/user/queue/action deletes and KyselyPg.destroy() teardown: userKyselyPersistenceFindByEmailAndOrg, resolveSamlUser, and the MRT module tests (CommentOperations, JobRouting, QueueOperations, ReporterInvalidation). - Rewrite moderationConfigService and manualReviewToolService so every test is self-contained: each creates its own fresh org (rolled back automatically) rather than sharing a suite-scoped org. This removes the cross-test ordering/accumulator dependency in moderationConfigService and the hardcoded staging-seed-data dependency in manualReviewToolService. Scylla-touching tests (itemInvestigationService) keep uid-based isolation, since the Postgres-only harness can't roll those back. Co-Authored-By: Claude Opus 4.8 (1M context) * test: migrate UserReportSweep and userStrikeService to transactional harness Two DB-backed test suites were missed by the initial migration commit: - manualReviewToolService/modules/UserReportSweep.test.ts: a sibling of the four MRT module tests already migrated (CommentOperations, JobRouting, QueueOperations, ReporterInvalidation). It used makeTestWithFixture + getBottle with hand-written cleanup (createOrg/createUser/createMrtQueue teardown + KyselyPg/KyselyPgReadReplica.destroy()). Converted to makeTransactionalTestWithFixture; cleanup is now automatic rollback. - userStrikeService/userStrikeService.test.ts: used beforeAll/afterAll with a shared getBottle container and uid-based isolation. Converted to makeTransactionalTestWithFixture for true per-test rollback isolation. Also fixes a copy-paste describe block name ('Item Investigation Service' -> 'User Strike Service'). Co-Authored-By: Claude Opus 4.8 (1M context) * fix: resolve typecheck errors in migrated test files Two files migrated by the prior commit had TypeScript errors that broke `tsc` (and therefore `check_api_server` and the e2e server start): - manualReviewToolService.test.ts: the 'records an AUTOMATIC_CLOSE decision with no human reviewer' test was left as a bare `it(...)` referencing an out-of-scope `mrtService` and hardcoded staging `orgId`/'queueId' ('e7c89ce7729'/'1') — the very seed-data dependency the migration was meant to remove. Convert it to `testWithQueue()` like its siblings, using the fixture's `mrtService`/`org.id`/`queue.id`. - moderationConfigService.test.ts: the 'should return actions for a rule scoped to the caller org' snapshot read `it.id` from `getActionsForRuleId`, which returns `{ action, parameters }[]` — should be `it.action.id` (as on main). Also add the now-present `email` field role to the #createUserType inline snapshot. Co-Authored-By: Claude Opus 4.8 (1M context) * fix: create real user in migrated JobRouting fixture The transactional JobRouting fixture accidentally replaced the pre-migration createUser() call with a bare uid(). createManualReviewQueue validates queue users, so setup failed after creating org/item-type rows but before the fixture returned. Because makeTestWithFixture cannot run cleanup when setup throws, the outer transaction stayed idle-in-transaction and later tests blocked on the item_type_versions materialized-view refresh, timing out check_api_server. Keep the transactional harness, but create a real user and pass user.id to the queue setup. Co-Authored-By: Claude Opus 4.8 (1M context) * fix: restore QueueOperations org-scoping tests The rebase conflict resolution for QueueOperations.test.ts accidentally kept the pre-#872 branch side and dropped the two-org org-scoping regression tests that now exist on main. Restore testWithTwoOrgs and its cross-org access tests, using the transactional harness. Co-Authored-By: Claude Opus 4.8 (1M context) * test: configure decision-reason settings through service API Replace the decision-reason test helpers' direct writes to manual_review_tool_settings with a behavior-shaped helper that uses the ManualReviewToolService update API. The tests still exercise persisted org settings through submitDecision, but no longer couple setup to table and column names. Co-Authored-By: Claude Opus 4.8 (1M context) * style: remove api shutdown explanatory comments Remove the comments added around the express-session store shutdown while keeping the shutdown behavior unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) --------- Co-authored-by: Claude Opus 4.8 --- ...KyselyPersistenceFindByEmailAndOrg.test.ts | 46 +- server/graphql/utils/resolveSamlUser.test.ts | 68 +- .../manualReviewToolService.test.ts | 1389 +++++----- .../modules/CommentOperations.test.ts | 157 +- .../modules/JobRouting.test.ts | 336 ++- .../modules/QueueOperations.test.ts | 67 +- .../modules/ReporterInvalidation.test.ts | 44 +- .../modules/UserReportSweep.test.ts | 44 +- .../moderationConfigService.test.ts | 2236 ++++++++--------- .../userStrikeService.test.ts | 450 ++-- 10 files changed, 2355 insertions(+), 2482 deletions(-) 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); + }, + ); }); -- 2.51.2