From 3a75984654db888a95d657c181e4e2a1c3a46b2d Mon Sep 17 00:00:00 2001 From: Juan Mrad Date: Wed, 22 Apr 2026 22:15:54 -0500 Subject: [PATCH] [Kysely] Migrate Action mutations and lookups to Kysely (#275) * [Kysely] Migrate Action mutations and lookups to Kysely * add org match for rules and tests to cover missing bugs * attempt to make lint happy * code review fixes * fix tests using bad org as we create a new org now. --- codegen.yaml | 8 +- server/graphql/datasources/ActionApi.ts | 142 ++--- server/graphql/datasources/RuleApi.ts | 7 +- .../datasources/buildGraphqlRuleParent.ts | 5 +- server/graphql/generated.ts | 12 +- server/graphql/modules/action.ts | 6 + server/graphql/modules/contentType.ts | 13 +- server/graphql/modules/org.ts | 5 +- server/graphql/modules/policy.ts | 2 +- server/graphql/modules/rule.ts | 10 +- server/rule_engine/ActionPublisher.test.ts | 2 + server/rule_engine/RuleEngine.ts | 7 +- server/rule_engine/ruleEngineQueries.ts | 13 +- .../moderationConfigService/dbTypes.ts | 3 +- .../moderationConfigService.test.ts | 592 ++++++++++++++++++ .../moderationConfigService.ts | 111 ++-- .../modules/ActionOperations.ts | 276 +++++++- .../moderationConfigService/types/actions.ts | 10 +- .../aggregation/AggregationSignal.test.ts | 11 +- .../userStrikeService.test.ts | 3 + server/test/fixtureHelpers/createActions.ts | 2 +- 21 files changed, 1048 insertions(+), 192 deletions(-) diff --git a/codegen.yaml b/codegen.yaml index 309e53b..82929b1 100644 --- a/codegen.yaml +++ b/codegen.yaml @@ -91,10 +91,10 @@ generates: # Don't change this; it makes type checking too permissive. allowParentTypeOverride: false mappers: - CustomAction: ../models/rules/ActionModel.js#CustomAction - EnqueueToMrtAction: ../models/rules/ActionModel.js#EnqueueToMrtAction - EnqueueToNcmecAction: ../models/rules/ActionModel.js#EnqueueToNcmecAction - EnqueueAuthorToMrtAction: ../models/rules/ActionModel.js#EnqueueAuthorToMrtAction + CustomAction: ../services/moderationConfigService/types/actions.js#CustomAction + EnqueueToMrtAction: ../services/moderationConfigService/types/actions.js#EnqueueToMrtAction + EnqueueToNcmecAction: ../services/moderationConfigService/types/actions.js#EnqueueToNcmecAction + EnqueueAuthorToMrtAction: ../services/moderationConfigService/types/actions.js#EnqueueAuthorToMrtAction Backtest: ../models/rules/BacktestModel.js#Backtest ContentType: ../models/rules/ItemTypeModel.js#ItemType DerivedFieldSource: ../services/derivedFieldsService/helpers.js#DerivedFieldSpecSource diff --git a/server/graphql/datasources/ActionApi.ts b/server/graphql/datasources/ActionApi.ts index 666d94d..f2d9677 100644 --- a/server/graphql/datasources/ActionApi.ts +++ b/server/graphql/datasources/ActionApi.ts @@ -1,26 +1,10 @@ import { type Exception } from '@opentelemetry/api'; import pLimit from 'p-limit'; -import { uid } from 'uid'; import { v1 as uuidv1 } from 'uuid'; import { inject, type Dependencies } from '../../iocContainer/index.js'; -import { isUniqueConstraintError } from '../../models/errors.js'; -import { - type CollapsedSequelizeAction, - type CustomAction, - type SequelizeAction, -} from '../../models/rules/ActionModel.js'; -import { - ActionType, - type Action, -} from '../../services/moderationConfigService/index.js'; -// TODO: delete the import below when we move the action mutation logic into the -// moderation config service, which is where it should be. -// eslint-disable-next-line import/no-restricted-paths -import { makeActionNameExistsError } from '../../services/moderationConfigService/modules/ActionOperations.js'; import { toCorrelationId } from '../../utils/correlationIds.js'; -import { patchInPlace } from '../../utils/misc.js'; -import { type CollapseCases } from '../../utils/typescript-types.js'; +import { makeNotFoundError } from '../../utils/errors.js'; import { type GQLCreateActionInput, type GQLUpdateActionInput, @@ -32,27 +16,35 @@ import { class ActionAPI { constructor( private readonly actionPublisher: Dependencies['ActionPublisher'], - private readonly sequelize: Dependencies['Sequelize'], + private readonly moderationConfigService: Dependencies['ModerationConfigService'], private readonly tracer: Dependencies['Tracer'], private readonly itemInvestigationService: Dependencies['ItemInvestigationService'], private readonly getItemTypeEventuallyConsistent: Dependencies['getItemTypeEventuallyConsistent'], - ) { - } + ) {} async getGraphQLActionFromId(opts: { id: string; orgId: string }) { const { id, orgId } = opts; - const action = await this.sequelize.Action.findOne({ - where: { id, orgId }, - rejectOnEmpty: true, + const actions = await this.moderationConfigService.getActions({ + orgId, + ids: [id], + readFromReplica: false, }); - - return action satisfies CollapsedSequelizeAction as SequelizeAction; + const action = actions.at(0); + if (action === undefined) { + throw makeNotFoundError('Action not found', { shouldErrorSpan: true }); + } + return action; } async getGraphQLActionsFromIds(orgId: string, ids: readonly string[]) { - return (await this.sequelize.Action.findAll({ - where: { orgId, id: ids }, - })) satisfies CollapsedSequelizeAction[] as SequelizeAction[]; + if (ids.length === 0) { + return []; + } + return this.moderationConfigService.getActions({ + orgId, + ids, + readFromReplica: false, + }); } async createAction(input: GQLCreateActionInput, orgId: string) { @@ -65,33 +57,17 @@ class ActionAPI { callbackUrlBody, applyUserStrikes, } = input; - const action = this.sequelize.Action.build({ - id: uid(), + + return this.moderationConfigService.createAction(orgId, { name, - description, + description: description ?? null, + type: 'CUSTOM_ACTION', callbackUrl, - callbackUrlHeaders, - callbackUrlBody, - orgId, - penalty: 'NONE', - applyUserStrikes: applyUserStrikes ?? false, - actionType: ActionType.CUSTOM_ACTION, - appliesToAllItemsOfKind: [], - }) as CustomAction; - - try { - await this.sequelize.transactionWithRetry(async () => { - await action.save(); - await action.addContentTypes([...itemTypeIds]); - await action.save(); - }); - } catch (e: unknown) { - throw isUniqueConstraintError(e) - ? makeActionNameExistsError({ shouldErrorSpan: true }) - : e; - } - - return action; + callbackUrlHeaders: callbackUrlHeaders ?? null, + callbackUrlBody: callbackUrlBody ?? null, + applyUserStrikes: applyUserStrikes ?? undefined, + itemTypeIds, + }); } async updateAction(input: GQLUpdateActionInput, orgId: string) { @@ -106,41 +82,26 @@ class ActionAPI { applyUserStrikes, } = input; - const action = (await this.sequelize.Action.findOne({ - where: { id, orgId, actionType: ActionType.CUSTOM_ACTION }, - rejectOnEmpty: true, - })) as CustomAction; - patchInPlace(action, { - name: name ?? undefined, - description, - callbackUrl: callbackUrl ?? undefined, - callbackUrlHeaders, - callbackUrlBody, - applyUserStrikes: applyUserStrikes ?? undefined, + return this.moderationConfigService.updateCustomAction(orgId, { + actionId: id, + patch: { + name: name ?? undefined, + description, + callbackUrl: callbackUrl ?? undefined, + callbackUrlHeaders, + callbackUrlBody, + applyUserStrikes: applyUserStrikes ?? undefined, + }, + itemTypeIds: itemTypeIds ?? undefined, }); - - try { - await this.sequelize.transactionWithRetry(async () => { - if (itemTypeIds) { - await action.setContentTypes([...itemTypeIds]); - } - await action.save(); - }); - } catch (e: unknown) { - throw isUniqueConstraintError(e) - ? makeActionNameExistsError({ shouldErrorSpan: true }) - : e; - } - - return action; } async deleteAction(orgId: string, id: string) { try { - const action = await this.sequelize.Action.findOne({ - where: { id, orgId, actionType: ActionType.CUSTOM_ACTION }, + return await this.moderationConfigService.deleteCustomAction({ + orgId, + actionId: id, }); - await action?.destroy(); } catch (exception) { const activeSpan = this.tracer.getActiveSpan(); if (activeSpan?.isRecording()) { @@ -149,7 +110,6 @@ class ActionAPI { return false; } - return true; } async bulkExecuteActions( @@ -162,10 +122,16 @@ class ActionAPI { actorEmail: string, ) { const [actions, policies, itemType] = await Promise.all([ - this.sequelize.Action.findAll({ - where: { id: actionIds, orgId }, - }) satisfies Promise[]> as Promise, - this.sequelize.Policy.findAll({ where: { id: policyIds, orgId } }), + this.moderationConfigService.getActions({ + orgId, + ids: actionIds, + readFromReplica: false, + }), + this.moderationConfigService.getPoliciesByIds({ + orgId, + ids: policyIds, + readFromReplica: false, + }), this.getItemTypeEventuallyConsistent({ orgId, typeSelector: { id: itemTypeId }, @@ -246,7 +212,7 @@ class ActionAPI { export default inject( [ 'ActionPublisher', - 'Sequelize', + 'ModerationConfigService', 'Tracer', 'ItemInvestigationService', 'getItemTypeEventuallyConsistent', diff --git a/server/graphql/datasources/RuleApi.ts b/server/graphql/datasources/RuleApi.ts index d8de0af..129d3f8 100644 --- a/server/graphql/datasources/RuleApi.ts +++ b/server/graphql/datasources/RuleApi.ts @@ -501,7 +501,12 @@ class RuleAPI { // that are restricted to routing rules only. const willHaveActions = actionIds ? actionIds.length > 0 - : (await this.moderationConfigService.getActionsForRuleId(id)).length > 0; + : ( + await this.moderationConfigService.getActionsForRuleId({ + orgId, + ruleId: id, + }) + ).length > 0; if (willHaveActions && conditionSet) { await this.validateSignalsAllowedInAutomatedRules(conditionSet, orgId); diff --git a/server/graphql/datasources/buildGraphqlRuleParent.ts b/server/graphql/datasources/buildGraphqlRuleParent.ts index 674d552..786ffb1 100644 --- a/server/graphql/datasources/buildGraphqlRuleParent.ts +++ b/server/graphql/datasources/buildGraphqlRuleParent.ts @@ -42,7 +42,10 @@ export function buildGraphqlRuleParent( return user; }, async getActions() { - return deps.moderationConfigService.getActionsForRuleId(plain.id); + return deps.moderationConfigService.getActionsForRuleId({ + orgId: plain.orgId, + ruleId: plain.id, + }); }, async getPolicies() { const byRule = await deps.moderationConfigService.getPoliciesByRuleIds([ diff --git a/server/graphql/generated.ts b/server/graphql/generated.ts index ba793c1..15f3f5f 100644 --- a/server/graphql/generated.ts +++ b/server/graphql/generated.ts @@ -18,12 +18,6 @@ import type { import type { ReportingInsights } from '../graphql/modules/reporting.js'; import type { HashBank } from '../models/HashBankModel.js'; import type { Org } from '../models/OrgModel.js'; -import type { - CustomAction, - EnqueueAuthorToMrtAction, - EnqueueToMrtAction, - EnqueueToNcmecAction, -} from '../models/rules/ActionModel.js'; import type { Backtest } from '../models/rules/BacktestModel.js'; import type { ItemType } from '../models/rules/ItemTypeModel.js'; import type { @@ -54,6 +48,12 @@ import type { ConditionSet, LeafCondition, } from '../services/moderationConfigService/index.js'; +import type { + CustomAction, + EnqueueAuthorToMrtAction, + EnqueueToMrtAction, + EnqueueToNcmecAction, +} from '../services/moderationConfigService/types/actions.js'; import type { Notification } from '../services/notificationsService/notificationsService.js'; import type { ReportingRuleWithoutVersion } from '../services/reportingService/ReportingRules.js'; import type { Signal } from '../services/signalsService/index.js'; diff --git a/server/graphql/modules/action.ts b/server/graphql/modules/action.ts index 94e3038..cee5cb2 100644 --- a/server/graphql/modules/action.ts +++ b/server/graphql/modules/action.ts @@ -3,6 +3,7 @@ import { assertUnreachable } from '../../utils/misc.js'; import { type GQLActionResolvers, type GQLCustomActionResolvers, + type GQLCustomMrtApiParamSpec, type GQLEnqueueAuthorToMrtActionResolvers, type GQLEnqueueToMrtActionResolvers, type GQLEnqueueToNcmecActionResolvers, @@ -179,6 +180,11 @@ const Action: GQLActionResolvers = { }; const CustomAction: GQLCustomActionResolvers = { + customMrtApiParams(parent) { + return Array.isArray(parent.customMrtApiParams) + ? (parent.customMrtApiParams as readonly GQLCustomMrtApiParamSpec[]) + : []; + }, async itemTypes(action, _, context) { const user = context.getUser(); if (user == null) { diff --git a/server/graphql/modules/contentType.ts b/server/graphql/modules/contentType.ts index f13bc0b..dc6e963 100644 --- a/server/graphql/modules/contentType.ts +++ b/server/graphql/modules/contentType.ts @@ -1,4 +1,5 @@ import { type GQLContentTypeResolvers } from '../generated.js'; +import { unauthenticatedError } from '../utils/errors.js'; const typeDefs = /* GraphQL */ ` type ContentType { @@ -12,8 +13,16 @@ const typeDefs = /* GraphQL */ ` `; const ContentType: GQLContentTypeResolvers = { - async actions(contentType) { - return contentType.getActions(); + async actions(contentType, _, context) { + const user = context.getUser(); + if (user == null || user.orgId !== contentType.orgId) { + throw unauthenticatedError('User required.'); + } + return context.services.ModerationConfigService.getActionsForItemType({ + orgId: contentType.orgId, + itemTypeId: contentType.id, + itemTypeKind: contentType.kind, + }); }, baseFields(contentType) { return contentType.fields; diff --git a/server/graphql/modules/org.ts b/server/graphql/modules/org.ts index 4c89a5a..ca74290 100644 --- a/server/graphql/modules/org.ts +++ b/server/graphql/modules/org.ts @@ -206,7 +206,10 @@ const Org: GQLOrgResolvers = { throw unauthenticatedError('User required.'); } - return org.getActions(); + return context.services.ModerationConfigService.getActions({ + orgId: org.id, + readFromReplica: true, + }); }, async contentTypes(org, _, context) { const user = context.getUser(); diff --git a/server/graphql/modules/policy.ts b/server/graphql/modules/policy.ts index 05b939a..d78da61 100644 --- a/server/graphql/modules/policy.ts +++ b/server/graphql/modules/policy.ts @@ -1,6 +1,6 @@ import _ from 'lodash'; -import { type Policy } from '../../models/PolicyModel.js'; +import { type Policy } from '../../services/moderationConfigService/index.js'; import { isCoopErrorOfType } from '../../utils/errors.js'; import { type GQLMutationDeletePolicyArgs, diff --git a/server/graphql/modules/rule.ts b/server/graphql/modules/rule.ts index ac5d62f..b8d34ca 100644 --- a/server/graphql/modules/rule.ts +++ b/server/graphql/modules/rule.ts @@ -659,7 +659,10 @@ const ContentRule: GQLContentRuleResolvers = { throw unauthenticatedError('Authenticated user required'); } - return rule.getActions(); + return context.services.ModerationConfigService.getActionsForRuleId({ + orgId: user.orgId, + ruleId: rule.id, + }); }, async policies(rule, _, context) { const user = context.getUser(); @@ -709,7 +712,10 @@ const UserRule: GQLUserRuleResolvers = { throw unauthenticatedError('Authenticated user required'); } - return rule.getActions(); + return context.services.ModerationConfigService.getActionsForRuleId({ + orgId: user.orgId, + ruleId: rule.id, + }); }, async policies(rule, _, context) { const user = context.getUser(); diff --git a/server/rule_engine/ActionPublisher.test.ts b/server/rule_engine/ActionPublisher.test.ts index 36487e6..dd14b93 100644 --- a/server/rule_engine/ActionPublisher.test.ts +++ b/server/rule_engine/ActionPublisher.test.ts @@ -41,6 +41,7 @@ describe('ActionPublisher', () => { id: 'action-1', orgId: 'org-123', name: 'Action 1', + description: null, applyUserStrikes: false, penalty: 'NONE' as const, actionType: ActionType.CUSTOM_ACTION, @@ -73,6 +74,7 @@ describe('ActionPublisher', () => { id: 'action-2', orgId: 'org-123', name: 'Action 2', + description: null, applyUserStrikes: false, penalty: 'NONE' as const, actionType: ActionType.CUSTOM_ACTION, diff --git a/server/rule_engine/RuleEngine.ts b/server/rule_engine/RuleEngine.ts index c796242..6f2bb32 100644 --- a/server/rule_engine/RuleEngine.ts +++ b/server/rule_engine/RuleEngine.ts @@ -233,9 +233,10 @@ class RuleEngine { await Promise.all( actionableRules.map( async (rule) => { - const actions = (await this.getRuleActionsEventuallyConsistent( - rule.id, - )) satisfies readonly ReadonlyDeep[] as readonly Action[]; + const actions = (await this.getRuleActionsEventuallyConsistent({ + orgId: evaluationContext.org.id, + ruleId: rule.id, + })) satisfies readonly ReadonlyDeep[] as readonly Action[]; return [rule, actions] as const; diff --git a/server/rule_engine/ruleEngineQueries.ts b/server/rule_engine/ruleEngineQueries.ts index c1a9d52..133aff2 100644 --- a/server/rule_engine/ruleEngineQueries.ts +++ b/server/rule_engine/ruleEngineQueries.ts @@ -69,8 +69,17 @@ export const makeGetActionsForRuleEventuallyConsistent = inject( ['ModerationConfigService'], (moderationConfigService) => { return cached({ - async producer(ruleId: string) { - return moderationConfigService.getActionsForRuleId(ruleId); + keyGeneration: { + toString: (key: { orgId: string; ruleId: string }) => + jsonStringify(key), + fromString: (it) => jsonParse(it), + }, + async producer(key: { orgId: string; ruleId: string }) { + return moderationConfigService.getActionsForRuleId({ + orgId: key.orgId, + ruleId: key.ruleId, + readFromReplica: true, + }); }, directives: { freshUntilAge: 30 }, }); diff --git a/server/services/moderationConfigService/dbTypes.ts b/server/services/moderationConfigService/dbTypes.ts index 2c217cd..13fc62b 100644 --- a/server/services/moderationConfigService/dbTypes.ts +++ b/server/services/moderationConfigService/dbTypes.ts @@ -1,6 +1,6 @@ import { type ItemTypeKind } from '@roostorg/types'; import { type Generated, type GeneratedAlways } from 'kysely'; -import { type JsonObject } from 'type-fest'; +import { type JsonObject, type JsonValue } from 'type-fest'; import { type TaggedUnionFromCases } from '../../utils/typescript-types.js'; import { type ActionType } from './types/actions.js'; @@ -103,6 +103,7 @@ export type ModerationConfigServicePg = { updated_at: Generated; applies_to_all_items_of_kind: Generated; apply_user_strikes: boolean; + custom_mrt_api_params: JsonValue[] | null; } & TaggedUnionFromCases< { action_type: ActionType }, { diff --git a/server/services/moderationConfigService/moderationConfigService.test.ts b/server/services/moderationConfigService/moderationConfigService.test.ts index 5acf370..4df9c06 100644 --- a/server/services/moderationConfigService/moderationConfigService.test.ts +++ b/server/services/moderationConfigService/moderationConfigService.test.ts @@ -6,6 +6,7 @@ import { uid } from 'uid'; 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 { makeMockPgDialect, @@ -540,6 +541,8 @@ describe('ModerationConfigService', () => { "callbackUrl": "https://example.com", "callbackUrlBody": null, "callbackUrlHeaders": null, + "customMrtApiParams": null, + "description": "Test description", "id": Any, "name": "Test Action", "orgId": Any, @@ -565,8 +568,597 @@ describe('ModerationConfigService', () => { expect(res).toHaveLength(createdActions.length); expect(res).toEqual(expect.arrayContaining(createdActions)); }); + + it('should round-trip a non-null customMrtApiParams value', async () => { + const action = await sutWithPrimary.createAction(dummyOrgId, { + name: faker.random.alphaNumeric(), + description: null, + type: 'CUSTOM_ACTION', + callbackUrl: 'https://example.com', + callbackUrlHeaders: null, + callbackUrlBody: null, + }); + + // The service create methods don't expose customMrtApiParams, + // so set it via raw Kysely to exercise the read mapping. + 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(); + + try { + const [fetched] = await sutWithPrimary.getActions({ + orgId: dummyOrgId, + ids: [action.id], + }); + expect(fetched).toBeDefined(); + expect(fetched.actionType).toBe('CUSTOM_ACTION'); + // The narrowed CustomAction shape exposes customMrtApiParams. + expect( + (fetched as { customMrtApiParams: unknown }).customMrtApiParams, + ).toEqual(params); + } finally { + await sutWithPrimary.deleteCustomAction({ + orgId: dummyOrgId, + actionId: action.id, + }); + } + }); + }); + }); + + describe('Update methods', () => { + describe('#updateCustomAction', () => { + const testWithAction = makeTestWithFixture(async () => { + const action = await sutWithPrimary.createAction(dummyOrgId, { + name: faker.random.alphaNumeric(), + 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, + }); + }, + }; + }); + + testWithAction( + 'should update user-editable fields and bump updated_at', + async ({ action }) => { + const before = await container.KyselyPg.selectFrom('public.actions') + .select(['updated_at']) + .where('id', '=', action.id) + .executeTakeFirstOrThrow(); + + // 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, + }, + }, + ); + + 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') + .select(['updated_at', 'description']) + .where('id', '=', action.id) + .executeTakeFirstOrThrow(); + expect(after.description).toBe('after'); + expect(after.updated_at.getTime()).toBeGreaterThan( + before.updated_at.getTime(), + ); + }, + ); + + testWithAction( + 'should not bump updated_at for an empty patch with no itemTypeIds', + async ({ action }) => { + const before = await container.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, + { actionId: action.id, patch: {} }, + ); + + const after = await container.KyselyPg.selectFrom('public.actions') + .select(['updated_at']) + .where('id', '=', action.id) + .executeTakeFirstOrThrow(); + expect(after.updated_at.getTime()).toBe(before.updated_at.getTime()); + expect(result.id).toBe(action.id); + }, + ); + + testWithAction( + 'should throw NotFound when called with the wrong org', + async ({ action }) => { + const otherOrg = await createOrg( + { Org: container.Sequelize.Org }, + container.ModerationConfigService, + container.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(); + } + }, + ); + + testWithAction( + 'should reject renaming onto an existing action name', + async ({ action }) => { + const other = await sutWithPrimary.createAction(dummyOrgId, { + name: faker.random.alphaNumeric(), + description: null, + type: 'CUSTOM_ACTION', + callbackUrl: 'https://example.com', + 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, + }); + } + }, + ); + + 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(), + schemaFieldRoles: { displayName: 'fakeField' }, + }, + ); + const itemTypeB = await sutWithPrimary.createContentType( + dummyOrgId, + { + schema: dummySchema, + description: null, + name: faker.random.alphaNumeric(), + 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, + }); + } + }, + ); }); }); + + describe('Delete methods', () => { + describe('#deleteCustomAction', () => { + const testWithAction = makeTestWithFixture(async () => { + const action = await sutWithPrimary.createAction(dummyOrgId, { + name: faker.random.alphaNumeric(), + 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(() => {}); + }, + }; + }); + + testWithAction( + 'should return true and delete the action on success', + async ({ action }) => { + const result = await sutWithPrimary.deleteCustomAction({ + orgId: dummyOrgId, + actionId: action.id, + }); + expect(result).toBe(true); + expect( + await sutWithPrimary.getActions({ + orgId: dummyOrgId, + 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); + }); + + testWithAction( + 'should return false when called with the wrong org and leave the row intact', + async ({ action }) => { + const otherOrg = await createOrg( + { Org: container.Sequelize.Org }, + container.ModerationConfigService, + container.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(); + } + }, + ); + + 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(), + schemaFieldRoles: { displayName: 'fakeField' }, + }, + ); + const rule = await createRule(container.Sequelize, dummyOrgId); + + await container.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') + .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, + }); + } + }, + ); + }); + }); + + describe('#getActionsForItemType', () => { + const testWithItemTypeAndActions = makeTestWithFixture(async () => { + const itemType = await sutWithPrimary.createContentType(dummyOrgId, { + schema: dummySchema, + description: null, + name: faker.random.alphaNumeric(), + schemaFieldRoles: { displayName: 'fakeField' }, + }); + + const viaJunctionAction = await sutWithPrimary.createAction( + dummyOrgId, + { + name: faker.random.alphaNumeric(), + description: null, + type: 'CUSTOM_ACTION', + callbackUrl: 'https://example.com', + callbackUrlHeaders: null, + callbackUrlBody: null, + itemTypeIds: [itemType.id], + }, + ); + + const viaAppliesAllAction = await sutWithPrimary.createAction( + dummyOrgId, + { + name: faker.random.alphaNumeric(), + 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(), + 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(); + + 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, + }); + }, + }; + }); + + testWithItemTypeAndActions( + 'should return actions from both branches, deduped, scoped to the org', + async ({ + itemType, + viaJunctionAction, + viaAppliesAllAction, + viaBothAction, + }) => { + const result = await sutWithPrimary.getActionsForItemType({ + orgId: dummyOrgId, + itemTypeId: itemType.id, + itemTypeKind: 'CONTENT', + readFromReplica: false, + }); + + const ids = result.map((it) => it.id).sort(); + expect(ids).toEqual( + [ + viaJunctionAction.id, + viaAppliesAllAction.id, + viaBothAction.id, + ].sort(), + ); + + // 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( + { Org: container.Sequelize.Org }, + container.ModerationConfigService, + container.ApiKeyService, + uid(), + ); + try { + const otherResult = await sutWithPrimary.getActionsForItemType({ + orgId: otherOrg.org.id, + itemTypeId: itemType.id, + itemTypeKind: 'CONTENT', + readFromReplica: false, + }); + expect(otherResult).toEqual([]); + } finally { + await otherOrg.cleanup(); + } + }, + ); + }); + + describe('#getActionsForRuleId', () => { + const testWithRuleAndAction = makeTestWithFixture(async () => { + const rule = await createRule(container.Sequelize, dummyOrgId); + const action = await sutWithPrimary.createAction(dummyOrgId, { + name: faker.random.alphaNumeric(), + 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(); + }, + }; + }); + + testWithRuleAndAction( + 'should return actions for a rule scoped to the caller org', + async ({ rule, action }) => { + const result = await sutWithPrimary.getActionsForRuleId({ + orgId: dummyOrgId, + ruleId: rule.id, + readFromReplica: false, + }); + expect(result.map((it) => it.id)).toEqual([action.id]); + }, + ); + + testWithRuleAndAction( + 'should not return actions when called with a different org', + async ({ rule }) => { + const otherOrg = await createOrg( + { Org: container.Sequelize.Org }, + container.ModerationConfigService, + container.ApiKeyService, + uid(), + ); + try { + const result = await sutWithPrimary.getActionsForRuleId({ + orgId: otherOrg.org.id, + ruleId: rule.id, + readFromReplica: false, + }); + expect(result).toEqual([]); + } finally { + await otherOrg.cleanup(); + } + }, + ); + }); }); describe('Policy returning methods', () => { diff --git a/server/services/moderationConfigService/moderationConfigService.ts b/server/services/moderationConfigService/moderationConfigService.ts index 560f468..5f5920c 100644 --- a/server/services/moderationConfigService/moderationConfigService.ts +++ b/server/services/moderationConfigService/moderationConfigService.ts @@ -6,7 +6,7 @@ import { type ConsumerDirectives } from '../../lib/cache/index.js'; import type { Invoker } from '../../models/types/permissioning.js'; import { type RuleErrorType, type LocationBankErrorType } from './errors.js'; import { type ModerationConfigServicePg } from './dbTypes.js'; -import { type Action, type Policy } from './index.js'; +import { type Action, type CustomAction, type Policy } from './index.js'; import ActionOperations, { type ActionErrorType, } from './modules/ActionOperations.js'; @@ -59,6 +59,27 @@ type ArrayOrPromiseOf = | Promise[]> | Promise>; +type ContentTypeSchemaFieldRoles = { + creatorId?: string | null; + threadId?: string | null; + parentId?: string | null; + createdAt?: string | null; + displayName?: string | null; +}; + +type ThreadTypeSchemaFieldRoles = { + createdAt?: string | null; + displayName?: string | null; + creatorId?: string | null; +}; + +type UserTypeSchemaFieldRoles = { + profileIcon?: string | null; + backgroundImage?: string | null; + createdAt?: string | null; + displayName?: string | null; +}; + /** * This service will eventually manage all CRUD operations on entities that are * part of an organization's defined moderation policy, including: rules, @@ -137,13 +158,7 @@ export class ModerationConfigService implements ReturnsModerationConfigTypes { name: string; schema: ItemSchema; description?: string | null; - schemaFieldRoles: { - creatorId?: string | null; - threadId?: string | null; - parentId?: string | null; - createdAt?: string | null; - displayName?: string | null; - }; + schemaFieldRoles: ContentTypeSchemaFieldRoles; }, ): Promise> { return this.itemTypeOps.createContentType(orgId, input); @@ -156,13 +171,7 @@ export class ModerationConfigService implements ReturnsModerationConfigTypes { name?: string; schema?: ItemSchema; description?: string | null; - schemaFieldRoles: { - creatorId?: string | null; - threadId?: string | null; - parentId?: string | null; - createdAt?: string | null; - displayName?: string | null; - }; + schemaFieldRoles: ContentTypeSchemaFieldRoles; }, ): Promise> { return this.itemTypeOps.updateContentType(orgId, input); @@ -174,11 +183,7 @@ export class ModerationConfigService implements ReturnsModerationConfigTypes { name: string; schema: ItemSchema; description?: string | null; - schemaFieldRoles: { - createdAt?: string | null; - displayName?: string | null; - creatorId?: string | null; - }; + schemaFieldRoles: ThreadTypeSchemaFieldRoles; }, ): Promise> { return this.itemTypeOps.createThreadType(orgId, input); @@ -191,11 +196,7 @@ export class ModerationConfigService implements ReturnsModerationConfigTypes { name?: string; schema?: ItemSchema; description?: string | null; - schemaFieldRoles: { - createdAt?: string | null; - displayName?: string | null; - creatorId?: string | null; - }; + schemaFieldRoles: ThreadTypeSchemaFieldRoles; }, ): Promise> { return this.itemTypeOps.updateThreadType(orgId, input); @@ -207,12 +208,7 @@ export class ModerationConfigService implements ReturnsModerationConfigTypes { name: string; schema: ItemSchema; description?: string | null; - schemaFieldRoles: { - profileIcon?: string | null; - backgroundImage?: string | null; - createdAt?: string | null; - displayName?: string | null; - }; + schemaFieldRoles: UserTypeSchemaFieldRoles; }, ): Promise> { return this.itemTypeOps.createUserType(orgId, input); @@ -225,12 +221,7 @@ export class ModerationConfigService implements ReturnsModerationConfigTypes { name?: string; schema?: ItemSchema; description?: string | null; - schemaFieldRoles: { - profileIcon?: string | null; - backgroundImage?: string | null; - createdAt?: string | null; - displayName?: string | null; - }; + schemaFieldRoles: UserTypeSchemaFieldRoles; }, ): Promise> { return this.itemTypeOps.updateUserType(orgId, input); @@ -268,13 +259,35 @@ export class ModerationConfigService implements ReturnsModerationConfigTypes { callbackUrl: string; callbackUrlHeaders: JsonObject | null; callbackUrlBody: JsonObject | null; - // TODO: linking specific item types not yet supported. applyUserStrikes?: boolean; + itemTypeIds?: readonly string[]; }, - ) { + ): Promise { return this.actionOps.createAction(orgId, input); } + async updateCustomAction( + orgId: string, + opts: { + actionId: string; + patch: { + name?: string; + description?: string | null; + callbackUrl?: string; + callbackUrlHeaders?: JsonObject | null; + callbackUrlBody?: JsonObject | null; + applyUserStrikes?: boolean; + }; + itemTypeIds?: readonly string[] | undefined; + }, + ): Promise { + return this.actionOps.updateCustomAction({ orgId, ...opts }); + } + + async deleteCustomAction(opts: { orgId: string; actionId: string }) { + return this.actionOps.deleteCustomAction(opts); + } + async getActions(opts: { orgId: string; ids?: readonly string[]; @@ -283,11 +296,21 @@ export class ModerationConfigService implements ReturnsModerationConfigTypes { return this.actionOps.getActions(opts); } - async getActionsForRuleId(ruleId: string) { - return this.actionOps.getActionsForRuleId({ - ruleId, - readFromReplica: true, - }); + async getActionsForItemType(opts: { + orgId: string; + itemTypeId: string; + itemTypeKind: ItemTypeKind; + readFromReplica?: boolean; + }) { + return this.actionOps.getActionsForItemType(opts); + } + + async getActionsForRuleId(opts: { + orgId: string; + ruleId: string; + readFromReplica?: boolean; + }) { + return this.actionOps.getActionsForRuleId(opts); } async getPoliciesByRuleIds(ruleIds: readonly string[]) { diff --git a/server/services/moderationConfigService/modules/ActionOperations.ts b/server/services/moderationConfigService/modules/ActionOperations.ts index 5cc42bc..08c6c50 100644 --- a/server/services/moderationConfigService/modules/ActionOperations.ts +++ b/server/services/moderationConfigService/modules/ActionOperations.ts @@ -1,19 +1,30 @@ -import { type Kysely } from 'kysely'; -import { type JsonObject, type Writable } from 'type-fest'; +import { type Kysely, sql } from 'kysely'; +import { type JsonObject, type JsonValue, type Writable } from 'type-fest'; import { uid } from 'uid'; import { CoopError, ErrorType, + makeNotFoundError, type ErrorInstanceData, } from '../../../utils/errors.js'; import { + isUniqueViolationError, type FixKyselyRowCorrelation, - } from '../../../utils/kysely.js'; -import { assertUnreachable } from '../../../utils/misc.js'; +import { makeKyselyTransactionWithRetry } from '../../../utils/kyselyTransactionWithRetry.js'; +import { assertUnreachable, removeUndefinedKeys } from '../../../utils/misc.js'; import { type ModerationConfigServicePg } from '../dbTypes.js'; -import { type Action } from '../index.js'; +import { type Action, type CustomAction } from '../index.js'; +import { type ItemTypeKind } from '../types/itemTypes.js'; + +function assertCustomAction(action: Action): asserts action is CustomAction { + if (action.actionType !== 'CUSTOM_ACTION') { + throw new Error( + `Expected CUSTOM_ACTION but received ${action.actionType}`, + ); + } +} const actionDbSelection = [ 'id', @@ -27,6 +38,7 @@ const actionDbSelection = [ 'action_type as actionType', 'applies_to_all_items_of_kind as appliesToAllItemsOfKind', 'apply_user_strikes as applyUserStrikes', + 'custom_mrt_api_params as customMrtApiParams', ] as const; const actionJoinDbSelection = [ @@ -41,6 +53,7 @@ const actionJoinDbSelection = [ 'a.action_type as actionType', 'a.applies_to_all_items_of_kind as appliesToAllItemsOfKind', 'a.apply_user_strikes as applyUserStrikes', + 'a.custom_mrt_api_params as customMrtApiParams', ] as const; type ActionDbResult = FixKyselyRowCorrelation< @@ -49,10 +62,16 @@ type ActionDbResult = FixKyselyRowCorrelation< >; export default class ActionOperations { + private readonly transactionWithRetry: ReturnType< + typeof makeKyselyTransactionWithRetry + >; + constructor( private readonly pgQuery: Kysely, private readonly pgQueryReplica: Kysely, - ) {} + ) { + this.transactionWithRetry = makeKyselyTransactionWithRetry(this.pgQuery); + } async createAction( orgId: string, @@ -67,40 +86,219 @@ export default class ActionOperations { callbackUrlHeaders: JsonObject | null; callbackUrlBody: JsonObject | null; applyUserStrikes?: boolean; - // TODO: linking specific item types not yet supported. + itemTypeIds?: readonly string[]; }, - ) { - return this.pgQuery.transaction().execute(async (trx) => { - const query = trx - .insertInto('public.actions') - .values({ - id: uid(), - name: input.name, - description: input.description, - org_id: orgId, - action_type: input.type, - callback_url: input.callbackUrl, - callback_url_headers: input.callbackUrlHeaders, - callback_url_body: input.callbackUrlBody, - penalty: 'NONE', - apply_user_strikes: input.applyUserStrikes ?? false, - }) - .returning(actionDbSelection); - - // eslint-disable-next-line no-useless-catch + ): Promise { + return this.transactionWithRetry(async (trx) => { try { + const query = trx + .insertInto('public.actions') + .values({ + id: uid(), + name: input.name, + description: input.description, + org_id: orgId, + action_type: input.type, + callback_url: input.callbackUrl, + callback_url_headers: input.callbackUrlHeaders, + callback_url_body: input.callbackUrlBody, + penalty: 'NONE', + apply_user_strikes: input.applyUserStrikes ?? false, + updated_at: new Date(), + }) + .returning(actionDbSelection); + const actionRow = (await query.executeTakeFirstOrThrow()) as ActionDbResult; - return this.#dbResultToAction(actionRow); - } catch (e) { - // TODO: catch specific error for duplicate action name and call - // makeActionNameExistsError and throw that error instead. + if (input.itemTypeIds !== undefined && input.itemTypeIds.length > 0) { + await trx + .insertInto('public.actions_and_item_types') + .values( + input.itemTypeIds.map((item_type_id) => ({ + action_id: actionRow.id, + item_type_id, + })), + ) + .execute(); + } + + const action = this.#dbResultToAction(actionRow); + assertCustomAction(action); + return action; + } catch (e: unknown) { + if (isUniqueViolationError(e)) { + throw makeActionNameExistsError({ shouldErrorSpan: true }); + } + throw e; + } + }); + } + + async updateCustomAction(opts: { + orgId: string; + actionId: string; + patch: { + name?: string; + description?: string | null; + callbackUrl?: string; + callbackUrlHeaders?: JsonObject | null; + callbackUrlBody?: JsonObject | null; + applyUserStrikes?: boolean; + }; + itemTypeIds?: readonly string[] | undefined; + }): Promise { + const { orgId, actionId, patch, itemTypeIds } = opts; + return this.transactionWithRetry(async (trx) => { + const existing = (await trx + .selectFrom('public.actions') + .select(actionDbSelection) + .where('id', '=', actionId) + .where('org_id', '=', orgId) + .where('action_type', '=', 'CUSTOM_ACTION') + .executeTakeFirst()) as ActionDbResult | undefined; + + if (existing == null) { + throw makeNotFoundError('Action not found', { shouldErrorSpan: true }); + } + + const setPayload = removeUndefinedKeys({ + name: patch.name, + description: patch.description, + callback_url: patch.callbackUrl, + callback_url_headers: patch.callbackUrlHeaders, + callback_url_body: patch.callbackUrlBody, + apply_user_strikes: patch.applyUserStrikes, + }); + const hasUserFields = Object.keys(setPayload).length > 0; + const touchesJunction = itemTypeIds !== undefined; + + if (!hasUserFields && !touchesJunction) { + const action = this.#dbResultToAction(existing); + assertCustomAction(action); + return action; + } + + try { + if (hasUserFields) { + await trx + .updateTable('public.actions') + .set({ + ...setPayload, + updated_at: new Date(), + }) + .where('id', '=', actionId) + .where('org_id', '=', orgId) + .execute(); + } + + if (itemTypeIds !== undefined) { + await trx + .deleteFrom('public.actions_and_item_types') + .where('action_id', '=', actionId) + .execute(); + if (itemTypeIds.length > 0) { + await trx + .insertInto('public.actions_and_item_types') + .values( + itemTypeIds.map((item_type_id) => ({ + action_id: actionId, + item_type_id, + })), + ) + .execute(); + } + } + + const refreshed = (await trx + .selectFrom('public.actions') + .select(actionDbSelection) + .where('id', '=', actionId) + .where('org_id', '=', orgId) + .executeTakeFirstOrThrow()) as ActionDbResult; + + const action = this.#dbResultToAction(refreshed); + assertCustomAction(action); + return action; + } catch (e: unknown) { + if (isUniqueViolationError(e)) { + throw makeActionNameExistsError({ shouldErrorSpan: true }); + } throw e; } }); } + async deleteCustomAction(opts: { orgId: string; actionId: string }) { + const { orgId, actionId } = opts; + return this.transactionWithRetry(async (trx) => { + const row = await trx + .selectFrom('public.actions') + .select('id') + .where('id', '=', actionId) + .where('org_id', '=', orgId) + .where('action_type', '=', 'CUSTOM_ACTION') + .executeTakeFirst(); + + if (row == null) { + return false; + } + + await trx + .deleteFrom('public.rules_and_actions') + .where('action_id', '=', actionId) + .execute(); + await trx + .deleteFrom('public.actions_and_item_types') + .where('action_id', '=', actionId) + .execute(); + await trx + .deleteFrom('public.actions') + .where('id', '=', actionId) + .where('org_id', '=', orgId) + .execute(); + + return true; + }); + } + + async getActionsForItemType(opts: { + orgId: string; + itemTypeId: string; + itemTypeKind: ItemTypeKind; + readFromReplica?: boolean; + }) { + const { orgId, itemTypeId, itemTypeKind, readFromReplica } = opts; + const pgQuery = this.#getPgQuery(readFromReplica); + + const [viaJunction, viaAppliesAll] = await Promise.all([ + pgQuery + .selectFrom('public.actions_and_item_types as ait') + .innerJoin('public.actions as a', 'a.id', 'ait.action_id') + .select(actionJoinDbSelection) + .where('ait.item_type_id', '=', itemTypeId) + .where('a.org_id', '=', orgId) + .execute(), + pgQuery + .selectFrom('public.actions as a') + .select(actionJoinDbSelection) + .where('a.org_id', '=', orgId) + .where( + sql`${itemTypeKind}::text = ANY(a.applies_to_all_items_of_kind::text[])`, + ) + .execute(), + ]); + + const junctionRows = viaJunction as ActionDbResult[]; + const appliesAllRows = viaAppliesAll as ActionDbResult[]; + + const byId = new Map(); + for (const row of [...junctionRows, ...appliesAllRows]) { + byId.set(row.id, row); + } + return [...byId.values()].map((it) => this.#dbResultToAction(it)); + } + async getActions(opts: { orgId: string; ids?: readonly string[]; @@ -120,25 +318,37 @@ export default class ActionOperations { } async getActionsForRuleId(opts: { + orgId: string; ruleId: string; readFromReplica?: boolean; }) { - const { ruleId, readFromReplica } = opts; - const pgQuery = this.#getPgQuery(readFromReplica ?? true); + const { orgId, ruleId, readFromReplica } = opts; + const pgQuery = this.#getPgQuery(readFromReplica); const results = (await pgQuery .selectFrom('public.rules_and_actions as raa') .innerJoin('public.actions as a', 'a.id', 'raa.action_id') .select(actionJoinDbSelection) .where('raa.rule_id', '=', ruleId) + .where('a.org_id', '=', orgId) .execute()) as ActionDbResult[]; return results.map((it) => this.#dbResultToAction(it)); } + private static customMrtApiParamsFromDb( + value: JsonValue[] | null, + ): JsonValue | null { + if (value == null || value.length === 0) { + return null; + } + return value; + } + #dbResultToAction(it: ActionDbResult) { return { id: it.id, name: it.name, + description: it.description, orgId: it.orgId, applyUserStrikes: it.applyUserStrikes, penalty: it.penalty, @@ -150,6 +360,8 @@ export default class ActionOperations { callbackUrl: it.callbackUrl, callbackUrlBody: it.callbackUrlBody, callbackUrlHeaders: it.callbackUrlHeaders, + customMrtApiParams: + ActionOperations.customMrtApiParamsFromDb(it.customMrtApiParams), }; case 'ENQUEUE_TO_MRT': case 'ENQUEUE_TO_NCMEC': diff --git a/server/services/moderationConfigService/types/actions.ts b/server/services/moderationConfigService/types/actions.ts index a0bdfb0..da89ef2 100644 --- a/server/services/moderationConfigService/types/actions.ts +++ b/server/services/moderationConfigService/types/actions.ts @@ -2,7 +2,12 @@ // since this service should not have any dependencies on the model instances' import { makeEnumLike } from '@roostorg/types'; -import { type JsonObject, type ReadonlyDeep, type Simplify } from 'type-fest'; +import { + type JsonObject, + type JsonValue, + type ReadonlyDeep, + type Simplify, +} from 'type-fest'; import { type TaggedUnionFromCases } from '../../../utils/typescript-types.js'; @@ -29,6 +34,7 @@ type AnyAction = ReadonlyDeep< id: string; orgId: string; name: string; + description: string | null; applyUserStrikes: boolean; penalty: UserPenaltySeverity; } & TaggedUnionFromCases< @@ -41,7 +47,7 @@ type AnyAction = ReadonlyDeep< callbackUrl: string; callbackUrlHeaders: JsonObject | null; callbackUrlBody: JsonObject | null; - customMrtApiParams: JsonObject | null; + customMrtApiParams: JsonValue | null; }; } > diff --git a/server/services/signalsService/signals/aggregation/AggregationSignal.test.ts b/server/services/signalsService/signals/aggregation/AggregationSignal.test.ts index 6538ecd..018e27d 100644 --- a/server/services/signalsService/signals/aggregation/AggregationSignal.test.ts +++ b/server/services/signalsService/signals/aggregation/AggregationSignal.test.ts @@ -2,6 +2,7 @@ import { uid } from 'uid'; import { v1 as uuidv1 } from 'uuid'; import { TestDateProvider } from '../../../../test/dateProvider.js'; +import createActions from '../../../../test/fixtureHelpers/createActions.js'; import createContentItemTypes from '../../../../test/fixtureHelpers/createContentItemTypes.js'; import createOrg from '../../../../test/fixtureHelpers/createOrg.js'; import createUser from '../../../../test/fixtureHelpers/createUser.js'; @@ -24,6 +25,7 @@ describe('AggregationSignal', () => { ModerationConfigService, AggregationsService, RuleAPIDataSource, + ActionAPIDataSource, ApiKeyService, } = deps; @@ -44,6 +46,12 @@ describe('AggregationSignal', () => { extra: {}, }); + const { actions, cleanup: actionsCleanup } = await createActions({ + actionAPI: ActionAPIDataSource, + itemTypeIds: [itemTypes[0].id], + orgId: org.id, + }); + // Spy on aggregation service functions. const aggregationsServiceSpy = AggregationsService; // eslint-disable-next-line functional/immutable-data @@ -117,7 +125,7 @@ describe('AggregationSignal', () => { }, ], }, - actionIds: ['73b2f15cc91'], + actionIds: [actions[0].id], policyIds: [], tags: [], maxDailyActions: null, @@ -135,6 +143,7 @@ describe('AggregationSignal', () => { dateProvider, async cleanup() { await RuleAPIDataSource.deleteRule({ id: rule.id, orgId: org.id }); + await actionsCleanup(); await itemTypesCleanup(); await userCleanup(); await orgCleanup(); diff --git a/server/services/userStrikeService/userStrikeService.test.ts b/server/services/userStrikeService/userStrikeService.test.ts index 01e3ac6..3dbb5cb 100644 --- a/server/services/userStrikeService/userStrikeService.test.ts +++ b/server/services/userStrikeService/userStrikeService.test.ts @@ -63,6 +63,7 @@ describe('Item Investigation Service', () => { action: { id: 'fakeActionId1', name: 'testAction1', + description: null, applyUserStrikes: true, orgId: 'fakeOrgId', penalty: 'NONE' as const, @@ -105,6 +106,7 @@ describe('Item Investigation Service', () => { action: { id: 'fakeActionId1', name: 'testAction1', + description: null, applyUserStrikes: false, orgId: 'fakeOrgId', penalty: 'NONE' as const, @@ -137,6 +139,7 @@ describe('Item Investigation Service', () => { action: { id: 'fakeActionId1', name: 'testAction1', + description: null, applyUserStrikes: false, orgId: 'fakeOrgId', penalty: 'NONE' as const, diff --git a/server/test/fixtureHelpers/createActions.ts b/server/test/fixtureHelpers/createActions.ts index 12e4f8b..3ceaf8c 100644 --- a/server/test/fixtureHelpers/createActions.ts +++ b/server/test/fixtureHelpers/createActions.ts @@ -27,7 +27,7 @@ export default async function (opts: { actions, async cleanup() { await Promise.all( - actions.map(async (it) => actionAPI.deleteAction(it.id, orgId)), + actions.map(async (it) => actionAPI.deleteAction(orgId, it.id)), ); }, }; -- 2.51.2