From f4676c5e090ecb40fc787950212b15e7d2ce9f57 Mon Sep 17 00:00:00 2001 From: Marcus Haddon Date: Fri, 20 Oct 2023 10:26:52 -0700 Subject: [PATCH 1/9] WIP: distinguishing between commentActions and moderationActions --- .../pipeline/phases/wordList/phase.ts | 3 ++- .../services/comments/pipeline/pipeline.ts | 20 +++++++++++++++---- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/server/src/core/server/services/comments/pipeline/phases/wordList/phase.ts b/server/src/core/server/services/comments/pipeline/phases/wordList/phase.ts index 34e8174b49..00ef61e1d5 100644 --- a/server/src/core/server/services/comments/pipeline/phases/wordList/phase.ts +++ b/server/src/core/server/services/comments/pipeline/phases/wordList/phase.ts @@ -31,8 +31,9 @@ export const wordListPhase: IntermediateModerationPhase = async ({ if (banned.isMatched) { return { status: GQLCOMMENT_STATUS.REJECTED, - actions: [ + moderationActions: [ { + // BOOKMARK: (marchaddon) update this to match moderation action input after pulling in model updates actionType: ACTION_TYPE.FLAG, reason: GQLCOMMENT_FLAG_REASON.COMMENT_DETECTED_BANNED_WORD, }, diff --git a/server/src/core/server/services/comments/pipeline/pipeline.ts b/server/src/core/server/services/comments/pipeline/pipeline.ts index d9e5e4fa61..4095cd28c2 100644 --- a/server/src/core/server/services/comments/pipeline/pipeline.ts +++ b/server/src/core/server/services/comments/pipeline/pipeline.ts @@ -4,6 +4,7 @@ import { Config } from "coral-server/config"; import { MongoContext } from "coral-server/data/context"; import { Logger } from "coral-server/logger"; import { CreateActionInput } from "coral-server/models/action/comment"; +import { CreateCommentModerationActionInput } from "coral-server/models/action/moderation/comment"; import { Comment, CreateCommentInput, @@ -26,16 +27,26 @@ import { mergePhaseResult } from "./helpers"; import { moderationPhases } from "./phases"; import { WordListService } from "./phases/wordList/service"; -export type ModerationAction = Omit< +export type CommentAction = Omit< CreateActionInput, "commentID" | "commentRevisionID" | "storyID" | "siteID" | "userID" >; +export type ModerationAction = Omit< + CreateCommentModerationActionInput, + "commentID" | "commentRevisionID" | "storyID" | "siteID" | "userID" +>; + export interface PhaseResult { /** - * actions are moderation actions that are added to the comment revision. + * moderationActions are moderation actions that are added to the comment revision. + */ + moderationActions: ModerationAction[]; + + /** + * commentActions are comment actions that are added to the comment revision. */ - actions: ModerationAction[]; + commentActions: CommentAction[]; /** * status when provided decides and terminates the moderation process by @@ -122,7 +133,8 @@ export const compose = const final: PhaseResult = { status: GQLCOMMENT_STATUS.NONE, body: context.comment.body, - actions: [], + commentActions: [], + moderationActions: [], metadata: { // Merge in the passed comment metadata. ...(context.comment.metadata || {}), From 36fe11621146eb4324faa468fea3f6514011ea06 Mon Sep 17 00:00:00 2001 From: Marcus Haddon Date: Fri, 20 Oct 2023 11:31:19 -0700 Subject: [PATCH 2/9] emit moderationAction during wordlist phase --- .../services/comments/pipeline/helpers.ts | 10 ++++-- .../comments/pipeline/phases/detectLinks.ts | 2 +- .../comments/pipeline/phases/external.ts | 6 ++-- .../pipeline/phases/recentCommentHistory.ts | 2 +- .../comments/pipeline/phases/repeatPost.ts | 2 +- .../services/comments/pipeline/phases/spam.ts | 2 +- .../phases/statusPreModerateNewCommenter.ts | 2 +- .../comments/pipeline/phases/toxic.ts | 2 +- .../pipeline/phases/wordList/phase.ts | 13 ++++---- .../comments/pipeline/pipeline.spec.ts | 8 ++--- .../services/comments/pipeline/pipeline.ts | 3 +- .../src/core/server/stacks/createComment.ts | 33 ++++++++++++++++--- 12 files changed, 56 insertions(+), 29 deletions(-) diff --git a/server/src/core/server/services/comments/pipeline/helpers.ts b/server/src/core/server/services/comments/pipeline/helpers.ts index 3c9738ca13..2f859b04b1 100644 --- a/server/src/core/server/services/comments/pipeline/helpers.ts +++ b/server/src/core/server/services/comments/pipeline/helpers.ts @@ -6,12 +6,16 @@ export function mergePhaseResult( result: Partial, final: Partial ) { - const { actions = [], tags = [], metadata = {} } = final; + const { commentActions = [], tags = [], metadata = {} } = final; // If this result contained actions, then we should push it into the // other actions. - if (result.actions) { - final.actions = [...actions, ...result.actions]; + if (result.commentActions) { + final.commentActions = [...commentActions, ...result.commentActions]; + } + + if (result.moderationAction) { + final.moderationAction = result.moderationAction; } // If this result contained metadata, then we should merge it into the diff --git a/server/src/core/server/services/comments/pipeline/phases/detectLinks.ts b/server/src/core/server/services/comments/pipeline/phases/detectLinks.ts index 8833307ed6..dc0914e00f 100755 --- a/server/src/core/server/services/comments/pipeline/phases/detectLinks.ts +++ b/server/src/core/server/services/comments/pipeline/phases/detectLinks.ts @@ -31,7 +31,7 @@ export const detectLinks: IntermediateModerationPhase = ({ // Add the flag related to Trust to the comment. return { status: GQLCOMMENT_STATUS.SYSTEM_WITHHELD, - actions: [ + commentActions: [ { actionType: ACTION_TYPE.FLAG, reason: GQLCOMMENT_FLAG_REASON.COMMENT_DETECTED_LINKS, diff --git a/server/src/core/server/services/comments/pipeline/phases/external.ts b/server/src/core/server/services/comments/pipeline/phases/external.ts index ed1f7c5b7a..a93ca998da 100644 --- a/server/src/core/server/services/comments/pipeline/phases/external.ts +++ b/server/src/core/server/services/comments/pipeline/phases/external.ts @@ -102,8 +102,9 @@ export interface ExternalModerationRequest { tenantDomain: string; } +// BOOKMARK: (marcushaddon) do we need a new type to maintain type safety with joi? export type ExternalModerationResponse = Partial< - Pick + Pick >; const ExternalModerationResponseSchema = Joi.object().keys({ @@ -311,7 +312,8 @@ export const external: IntermediateModerationPhase = async (ctx) => { name: phase.name, analyzedAt: ctx.now, result: { - actions: response.actions, + // NOTE: these will be split into comment/moderation actions later + actions: response.commentActions, status: response.status, tags: response.tags, }, diff --git a/server/src/core/server/services/comments/pipeline/phases/recentCommentHistory.ts b/server/src/core/server/services/comments/pipeline/phases/recentCommentHistory.ts index b199917129..74d9a88d66 100644 --- a/server/src/core/server/services/comments/pipeline/phases/recentCommentHistory.ts +++ b/server/src/core/server/services/comments/pipeline/phases/recentCommentHistory.ts @@ -47,7 +47,7 @@ export const recentCommentHistory = async ({ if (rate >= tenant.recentCommentHistory.triggerRejectionRate) { return { status: GQLCOMMENT_STATUS.SYSTEM_WITHHELD, - actions: [ + commentActions: [ { actionType: ACTION_TYPE.FLAG, reason: GQLCOMMENT_FLAG_REASON.COMMENT_DETECTED_RECENT_HISTORY, diff --git a/server/src/core/server/services/comments/pipeline/phases/repeatPost.ts b/server/src/core/server/services/comments/pipeline/phases/repeatPost.ts index 00245be9ce..3dfeb1ba87 100644 --- a/server/src/core/server/services/comments/pipeline/phases/repeatPost.ts +++ b/server/src/core/server/services/comments/pipeline/phases/repeatPost.ts @@ -106,7 +106,7 @@ export const repeatPost: IntermediateModerationPhase = async ({ return { status: GQLCOMMENT_STATUS.SYSTEM_WITHHELD, - actions: [ + commentActions: [ { actionType: ACTION_TYPE.FLAG, reason: GQLCOMMENT_FLAG_REASON.COMMENT_DETECTED_REPEAT_POST, diff --git a/server/src/core/server/services/comments/pipeline/phases/spam.ts b/server/src/core/server/services/comments/pipeline/phases/spam.ts index e1c381cbef..e8d7a38014 100644 --- a/server/src/core/server/services/comments/pipeline/phases/spam.ts +++ b/server/src/core/server/services/comments/pipeline/phases/spam.ts @@ -114,7 +114,7 @@ export const spam: IntermediateModerationPhase = async ({ return { status: GQLCOMMENT_STATUS.SYSTEM_WITHHELD, - actions: [ + commentActions: [ { actionType: ACTION_TYPE.FLAG, reason: GQLCOMMENT_FLAG_REASON.COMMENT_DETECTED_SPAM, diff --git a/server/src/core/server/services/comments/pipeline/phases/statusPreModerateNewCommenter.ts b/server/src/core/server/services/comments/pipeline/phases/statusPreModerateNewCommenter.ts index 41d80b11af..1b164b1dfe 100644 --- a/server/src/core/server/services/comments/pipeline/phases/statusPreModerateNewCommenter.ts +++ b/server/src/core/server/services/comments/pipeline/phases/statusPreModerateNewCommenter.ts @@ -72,7 +72,7 @@ export const statusPreModerateNewCommenter = async ({ return { status: GQLCOMMENT_STATUS.SYSTEM_WITHHELD, - actions: [ + commentActions: [ { actionType: ACTION_TYPE.FLAG, reason: GQLCOMMENT_FLAG_REASON.COMMENT_DETECTED_NEW_COMMENTER, diff --git a/server/src/core/server/services/comments/pipeline/phases/toxic.ts b/server/src/core/server/services/comments/pipeline/phases/toxic.ts index 2897ed08a3..ef39f49f9d 100644 --- a/server/src/core/server/services/comments/pipeline/phases/toxic.ts +++ b/server/src/core/server/services/comments/pipeline/phases/toxic.ts @@ -128,7 +128,7 @@ export const toxic: IntermediateModerationPhase = async ({ return { status: GQLCOMMENT_STATUS.SYSTEM_WITHHELD, - actions: [ + commentActions: [ { actionType: ACTION_TYPE.FLAG, reason: GQLCOMMENT_FLAG_REASON.COMMENT_DETECTED_TOXIC, diff --git a/server/src/core/server/services/comments/pipeline/phases/wordList/phase.ts b/server/src/core/server/services/comments/pipeline/phases/wordList/phase.ts index 00ef61e1d5..31925581a3 100644 --- a/server/src/core/server/services/comments/pipeline/phases/wordList/phase.ts +++ b/server/src/core/server/services/comments/pipeline/phases/wordList/phase.ts @@ -33,9 +33,8 @@ export const wordListPhase: IntermediateModerationPhase = async ({ status: GQLCOMMENT_STATUS.REJECTED, moderationActions: [ { - // BOOKMARK: (marchaddon) update this to match moderation action input after pulling in model updates - actionType: ACTION_TYPE.FLAG, - reason: GQLCOMMENT_FLAG_REASON.COMMENT_DETECTED_BANNED_WORD, + status: GQLCOMMENT_STATUS.REJECTED, + moderatorID: null, }, ], metadata: { @@ -46,7 +45,7 @@ export const wordListPhase: IntermediateModerationPhase = async ({ }; } else if (banned.timedOut) { return { - actions: [ + commentActions: [ { actionType: ACTION_TYPE.FLAG, reason: GQLCOMMENT_FLAG_REASON.COMMENT_DETECTED_BANNED_WORD, @@ -69,7 +68,7 @@ export const wordListPhase: IntermediateModerationPhase = async ({ if (tenant.premoderateSuspectWords && suspect.isMatched) { return { status: GQLCOMMENT_STATUS.SYSTEM_WITHHELD, - actions: [ + commentActions: [ { actionType: ACTION_TYPE.FLAG, reason: GQLCOMMENT_FLAG_REASON.COMMENT_DETECTED_SUSPECT_WORD, @@ -83,7 +82,7 @@ export const wordListPhase: IntermediateModerationPhase = async ({ }; } else if (suspect.isMatched) { return { - actions: [ + commentActions: [ { actionType: ACTION_TYPE.FLAG, reason: GQLCOMMENT_FLAG_REASON.COMMENT_DETECTED_SUSPECT_WORD, @@ -97,7 +96,7 @@ export const wordListPhase: IntermediateModerationPhase = async ({ }; } else if (suspect.timedOut) { return { - actions: [ + commentActions: [ { actionType: ACTION_TYPE.FLAG, reason: GQLCOMMENT_FLAG_REASON.COMMENT_DETECTED_SUSPECT_WORD, diff --git a/server/src/core/server/services/comments/pipeline/pipeline.spec.ts b/server/src/core/server/services/comments/pipeline/pipeline.spec.ts index 26abc5d25d..d79b923680 100644 --- a/server/src/core/server/services/comments/pipeline/pipeline.spec.ts +++ b/server/src/core/server/services/comments/pipeline/pipeline.spec.ts @@ -71,14 +71,14 @@ describe("compose", () => { const enhanced = compose([ () => ({ - actions: [flags[0]], + commentActions: [flags[0]], }), () => ({ status, actions: [flags[1]], }), () => ({ - actions: [ + commentActions: [ { userID: null, actionType: ACTION_TYPE.FLAG, @@ -91,10 +91,10 @@ describe("compose", () => { const final = await enhanced(context); for (const flag of flags) { - expect(final.actions).toContainEqual(flag); + expect(final.commentActions).toContainEqual(flag); } - expect(final.actions).not.toContainEqual({ + expect(final.commentActions).not.toContainEqual({ body: context.comment.body, actionType: ACTION_TYPE.FLAG, reason: GQLCOMMENT_FLAG_REASON.COMMENT_DETECTED_LINKS, diff --git a/server/src/core/server/services/comments/pipeline/pipeline.ts b/server/src/core/server/services/comments/pipeline/pipeline.ts index 4095cd28c2..66f6e24abe 100644 --- a/server/src/core/server/services/comments/pipeline/pipeline.ts +++ b/server/src/core/server/services/comments/pipeline/pipeline.ts @@ -41,7 +41,7 @@ export interface PhaseResult { /** * moderationActions are moderation actions that are added to the comment revision. */ - moderationActions: ModerationAction[]; + moderationAction?: ModerationAction; /** * commentActions are comment actions that are added to the comment revision. @@ -134,7 +134,6 @@ export const compose = status: GQLCOMMENT_STATUS.NONE, body: context.comment.body, commentActions: [], - moderationActions: [], metadata: { // Merge in the passed comment metadata. ...(context.comment.metadata || {}), diff --git a/server/src/core/server/stacks/createComment.ts b/server/src/core/server/stacks/createComment.ts index 65aab158da..4db5806b7b 100644 --- a/server/src/core/server/stacks/createComment.ts +++ b/server/src/core/server/stacks/createComment.ts @@ -43,7 +43,7 @@ import { import { ensureFeatureFlag, Tenant } from "coral-server/models/tenant"; import { User } from "coral-server/models/user"; import { isSiteBanned } from "coral-server/models/user/helpers"; -import { removeTag } from "coral-server/services/comments"; +import { moderate, removeTag } from "coral-server/services/comments"; import { addCommentActions, CreateAction, @@ -347,11 +347,11 @@ export default async function create( // is added, that it can already know that the comment is already in the // queue. let actionCounts: EncodedCommentActionCounts = {}; - if (result.actions.length > 0) { + if (result.commentActions.length > 0) { // Determine the unique actions, we will use this to compute the comment // action counts. This should match what is added below. actionCounts = encodeActionCounts( - ...filterDuplicateActions(result.actions) + ...filterDuplicateActions(result.commentActions) ); } @@ -424,13 +424,13 @@ export default async function create( log.trace("pushed child comment id onto parent"); } - if (result.actions.length > 0) { + if (result.commentActions.length > 0) { // Actually add the actions to the database. This will not interact with the // counts at all. await addCommentActions( mongo, tenant, - result.actions.map( + result.commentActions.map( (action): CreateAction => ({ ...action, commentID: comment.id, @@ -447,6 +447,29 @@ export default async function create( ); } + if (result.moderationAction) { + // Actually add the actions to the database. This will not interact with the + // counts at all. + await moderate( + mongo, + redis, + config, + i18n, + tenant, + { + ...result.moderationAction, + commentID: comment.id, + commentRevisionID: revision.id, + }, + now, + false, + { + actionCounts, + // BOOKMARK (marcushaddon): need more clarity on what these options do + } + ); + } + // Update all the comment counts on stories and users. const counts = await updateAllCommentCounts(mongo, redis, config, i18n, { tenant, From dd4309510080338299da1db73db54b64a86525ff Mon Sep 17 00:00:00 2001 From: Marcus Haddon Date: Fri, 20 Oct 2023 12:25:17 -0700 Subject: [PATCH 3/9] WIP: correcting types and fixing tests --- .../comments/pipeline/phases/wordList/phase.ts | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/server/src/core/server/services/comments/pipeline/phases/wordList/phase.ts b/server/src/core/server/services/comments/pipeline/phases/wordList/phase.ts index 31925581a3..0bb102380e 100644 --- a/server/src/core/server/services/comments/pipeline/phases/wordList/phase.ts +++ b/server/src/core/server/services/comments/pipeline/phases/wordList/phase.ts @@ -31,12 +31,10 @@ export const wordListPhase: IntermediateModerationPhase = async ({ if (banned.isMatched) { return { status: GQLCOMMENT_STATUS.REJECTED, - moderationActions: [ - { - status: GQLCOMMENT_STATUS.REJECTED, - moderatorID: null, - }, - ], + moderationAction: { + status: GQLCOMMENT_STATUS.REJECTED, + moderatorID: null, + }, metadata: { wordList: { bannedWords: banned.matches, From 9f32e941fa5a6f5f9c51889b573eef22df855b8a Mon Sep 17 00:00:00 2001 From: Marcus Haddon Date: Sun, 22 Oct 2023 09:07:22 -0700 Subject: [PATCH 4/9] WIP: updating pipeline tests --- .../server/services/comments/pipeline/phases/external.ts | 2 +- .../server/services/comments/pipeline/pipeline.spec.ts | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/server/src/core/server/services/comments/pipeline/phases/external.ts b/server/src/core/server/services/comments/pipeline/phases/external.ts index a93ca998da..9e45ccfbb8 100644 --- a/server/src/core/server/services/comments/pipeline/phases/external.ts +++ b/server/src/core/server/services/comments/pipeline/phases/external.ts @@ -104,7 +104,7 @@ export interface ExternalModerationRequest { // BOOKMARK: (marcushaddon) do we need a new type to maintain type safety with joi? export type ExternalModerationResponse = Partial< - Pick + Pick >; const ExternalModerationResponseSchema = Joi.object().keys({ diff --git a/server/src/core/server/services/comments/pipeline/pipeline.spec.ts b/server/src/core/server/services/comments/pipeline/pipeline.spec.ts index d79b923680..9fee91504b 100644 --- a/server/src/core/server/services/comments/pipeline/pipeline.spec.ts +++ b/server/src/core/server/services/comments/pipeline/pipeline.spec.ts @@ -31,7 +31,7 @@ describe("compose", () => { body: context.comment.body, status, metadata: {}, - actions: [], + commentActions: [], tags: [], }); }); @@ -48,11 +48,12 @@ describe("compose", () => { body: context.comment.body, status, metadata: { akismet: false, linkCount: 1 }, - actions: [], + commentActions: [], tags: [], }); }); + // BOOKMARK (marcushaddon): how was this passing before? it("merges actions", async () => { const status = GQLCOMMENT_STATUS.APPROVED; @@ -111,7 +112,7 @@ describe("compose", () => { body: context.comment.body, status: GQLCOMMENT_STATUS.NONE, metadata: { akismet: false }, - actions: [], + commentActions: [], tags: [], }); }); From 00f3340903d7438f02eda2b25c9ea1b9a0135825 Mon Sep 17 00:00:00 2001 From: Marcus Haddon Date: Tue, 24 Oct 2023 14:37:21 -0700 Subject: [PATCH 5/9] remove comment --- .../core/server/services/comments/pipeline/pipeline.spec.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/server/src/core/server/services/comments/pipeline/pipeline.spec.ts b/server/src/core/server/services/comments/pipeline/pipeline.spec.ts index 9fee91504b..fba3d9068a 100644 --- a/server/src/core/server/services/comments/pipeline/pipeline.spec.ts +++ b/server/src/core/server/services/comments/pipeline/pipeline.spec.ts @@ -53,7 +53,6 @@ describe("compose", () => { }); }); - // BOOKMARK (marcushaddon): how was this passing before? it("merges actions", async () => { const status = GQLCOMMENT_STATUS.APPROVED; @@ -76,7 +75,7 @@ describe("compose", () => { }), () => ({ status, - actions: [flags[1]], + commentActions: [flags[1]], }), () => ({ commentActions: [ From 21491ac97b110b90366d4a615e624106c7bf2ced Mon Sep 17 00:00:00 2001 From: Marcus Haddon Date: Wed, 25 Oct 2023 15:45:16 -0700 Subject: [PATCH 6/9] WIP: adapting external moderation phase --- .../comments/pipeline/phases/external.ts | 18 +++- server/src/core/server/stacks/editComment.ts | 82 +++++++++++-------- server/src/core/server/utils/partition.ts | 19 +++++ 3 files changed, 79 insertions(+), 40 deletions(-) create mode 100644 server/src/core/server/utils/partition.ts diff --git a/server/src/core/server/services/comments/pipeline/phases/external.ts b/server/src/core/server/services/comments/pipeline/phases/external.ts index 9e45ccfbb8..ff69a7ee0a 100644 --- a/server/src/core/server/services/comments/pipeline/phases/external.ts +++ b/server/src/core/server/services/comments/pipeline/phases/external.ts @@ -12,6 +12,7 @@ import { PhaseResult, } from "coral-server/services/comments/pipeline"; import { createFetch, generateFetchOptions } from "coral-server/services/fetch"; +import { partition } from "coral-server/utils/partition"; import { GQLCOMMENT_BODY_FORMAT, @@ -104,8 +105,8 @@ export interface ExternalModerationRequest { // BOOKMARK: (marcushaddon) do we need a new type to maintain type safety with joi? export type ExternalModerationResponse = Partial< - Pick ->; + Pick +> & { actions: PhaseResult["commentActions"] }; const ExternalModerationResponseSchema = Joi.object().keys({ actions: Joi.array().items( @@ -268,6 +269,14 @@ async function processPhase( return validateResponse(body); } +/** + * Our external API still just has a concept of "actions", while + * internally we distinguish beteween "moderationActions" and "commentActions" + */ +const mapActions = (response: ExternalModerationResponse): PhaseResult => { + // TODO: marcushaddon - implement after deciding what to do +}; + export const external: IntermediateModerationPhase = async (ctx) => { // Check to see if any custom moderation phases have been defined, if there is // none, exit now. @@ -313,14 +322,15 @@ export const external: IntermediateModerationPhase = async (ctx) => { analyzedAt: ctx.now, result: { // NOTE: these will be split into comment/moderation actions later - actions: response.commentActions, + actions: response.actions, status: response.status, tags: response.tags, }, }); // Merge the results in. If we're finished, return now! - const finished = mergePhaseResult(response, result); + const mappedResult = mapActions(result); + const finished = mergePhaseResult(mappedResult, result); if (finished) { return result; } diff --git a/server/src/core/server/stacks/editComment.ts b/server/src/core/server/stacks/editComment.ts index 0981d178c4..d360b3c2c2 100644 --- a/server/src/core/server/stacks/editComment.ts +++ b/server/src/core/server/stacks/editComment.ts @@ -17,7 +17,7 @@ import { EncodedCommentActionCounts, filterDuplicateActions, } from "coral-server/models/action/comment"; -import { createCommentModerationAction } from "coral-server/models/action/moderation/comment"; + import { editComment, EditCommentInput, @@ -34,6 +34,7 @@ import { resolveStoryMode, retrieveStory } from "coral-server/models/story"; import { Tenant } from "coral-server/models/tenant"; import { User } from "coral-server/models/user"; import { isSiteBanned } from "coral-server/models/user/helpers"; +import { moderate } from "coral-server/services/comments"; import { addCommentActions, CreateAction, @@ -181,33 +182,36 @@ export default async function edit( } // Run the comment through the moderation phases. - const { body, status, metadata, actions } = await processForModeration({ - action: "EDIT", - log, - mongo, - redis, - config, - wordList, - tenant, - story, - storyMode, - parent, - comment: { - ...originalStaleComment, - ...input, - authorID: author.id, - }, - media, - author, - req, - now, - }); + const { body, status, metadata, commentActions, moderationAction } = + await processForModeration({ + action: "EDIT", + log, + mongo, + redis, + config, + wordList, + tenant, + story, + storyMode, + parent, + comment: { + ...originalStaleComment, + ...input, + authorID: author.id, + }, + media, + author, + req, + now, + }); let actionCounts: EncodedCommentActionCounts = {}; - if (actions.length > 0) { + if (commentActions.length > 0) { // Encode the new action counts that are going to be added to the new // revision. - actionCounts = encodeActionCounts(...filterDuplicateActions(actions)); + actionCounts = encodeActionCounts( + ...filterDuplicateActions(commentActions) + ); } // Perform the edit. @@ -234,11 +238,11 @@ export default async function edit( // If there were actions to insert, then insert them into the commentActions // collection. - if (actions.length > 0) { + if (commentActions.length > 0) { await addCommentActions( mongo, tenant, - actions.map( + commentActions.map( (action): CreateAction => ({ ...action, commentID: result.after.id, @@ -255,20 +259,26 @@ export default async function edit( ); } - // If the comment status changed as a result of a pipeline operation, create a - // moderation action (but don't associate it with a moderator). - if (result.before.status !== result.after.status) { - await createCommentModerationAction( + if (moderationAction) { + // Actually add the actions to the database. This will not interact with the + // counts at all. + await moderate( mongo, - tenant.id, + redis, + config, + i18n, + tenant, { - storyID: story.id, + ...moderationAction, commentID: result.after.id, - commentRevisionID: result.revision.id, - status: result.after.status, - moderatorID: null, + commentRevisionID: lastRevision.id, }, - now + now, + false, + { + actionCounts, + // BOOKMARK (marcushaddon): need more clarity on what these options do + } ); } diff --git a/server/src/core/server/utils/partition.ts b/server/src/core/server/utils/partition.ts new file mode 100644 index 0000000000..650f3ecd84 --- /dev/null +++ b/server/src/core/server/utils/partition.ts @@ -0,0 +1,19 @@ +export const partition = ( + items: T[], + predicate: (item: T) => boolean +): { + passed: T[]; + failed: T[]; +} => { + const passed: T[] = []; + const failed: T[] = []; + for (const item of items) { + if (predicate(item)) { + passed.push(item); + } else { + failed.push(item); + } + } + + return { passed, failed }; +}; From 11895b73cd7031622b497740934f1bb5517e059d Mon Sep 17 00:00:00 2001 From: Marcus Haddon Date: Fri, 27 Oct 2023 10:25:36 -0700 Subject: [PATCH 7/9] map external moderation phase api to new internal model --- .../services/comments/pipeline/phases/external.ts | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/server/src/core/server/services/comments/pipeline/phases/external.ts b/server/src/core/server/services/comments/pipeline/phases/external.ts index ff69a7ee0a..899cbcc2c8 100644 --- a/server/src/core/server/services/comments/pipeline/phases/external.ts +++ b/server/src/core/server/services/comments/pipeline/phases/external.ts @@ -12,7 +12,6 @@ import { PhaseResult, } from "coral-server/services/comments/pipeline"; import { createFetch, generateFetchOptions } from "coral-server/services/fetch"; -import { partition } from "coral-server/utils/partition"; import { GQLCOMMENT_BODY_FORMAT, @@ -273,8 +272,14 @@ async function processPhase( * Our external API still just has a concept of "actions", while * internally we distinguish beteween "moderationActions" and "commentActions" */ -const mapActions = (response: ExternalModerationResponse): PhaseResult => { +const mapActions = ( + response: ExternalModerationResponse +): Partial => { // TODO: marcushaddon - implement after deciding what to do + return { + ...response, + commentActions: response.actions, + }; }; export const external: IntermediateModerationPhase = async (ctx) => { @@ -328,9 +333,10 @@ export const external: IntermediateModerationPhase = async (ctx) => { }, }); + const mappedResponse = mapActions(response); + // Merge the results in. If we're finished, return now! - const mappedResult = mapActions(result); - const finished = mergePhaseResult(mappedResult, result); + const finished = mergePhaseResult(mappedResponse, result); if (finished) { return result; } From 9504b97b8e7b1cf7064a1c442987ba41a3985dd6 Mon Sep 17 00:00:00 2001 From: Marcus Haddon Date: Fri, 27 Oct 2023 10:34:32 -0700 Subject: [PATCH 8/9] clean up --- .../comments/pipeline/phases/external.ts | 2 -- .../src/core/server/stacks/createComment.ts | 1 - server/src/core/server/stacks/editComment.ts | 1 - server/src/core/server/utils/partition.ts | 19 ------------------- 4 files changed, 23 deletions(-) delete mode 100644 server/src/core/server/utils/partition.ts diff --git a/server/src/core/server/services/comments/pipeline/phases/external.ts b/server/src/core/server/services/comments/pipeline/phases/external.ts index 899cbcc2c8..c0315cac30 100644 --- a/server/src/core/server/services/comments/pipeline/phases/external.ts +++ b/server/src/core/server/services/comments/pipeline/phases/external.ts @@ -275,7 +275,6 @@ async function processPhase( const mapActions = ( response: ExternalModerationResponse ): Partial => { - // TODO: marcushaddon - implement after deciding what to do return { ...response, commentActions: response.actions, @@ -326,7 +325,6 @@ export const external: IntermediateModerationPhase = async (ctx) => { name: phase.name, analyzedAt: ctx.now, result: { - // NOTE: these will be split into comment/moderation actions later actions: response.actions, status: response.status, tags: response.tags, diff --git a/server/src/core/server/stacks/createComment.ts b/server/src/core/server/stacks/createComment.ts index 4db5806b7b..5c15b08366 100644 --- a/server/src/core/server/stacks/createComment.ts +++ b/server/src/core/server/stacks/createComment.ts @@ -465,7 +465,6 @@ export default async function create( false, { actionCounts, - // BOOKMARK (marcushaddon): need more clarity on what these options do } ); } diff --git a/server/src/core/server/stacks/editComment.ts b/server/src/core/server/stacks/editComment.ts index d360b3c2c2..dd45455dc5 100644 --- a/server/src/core/server/stacks/editComment.ts +++ b/server/src/core/server/stacks/editComment.ts @@ -277,7 +277,6 @@ export default async function edit( false, { actionCounts, - // BOOKMARK (marcushaddon): need more clarity on what these options do } ); } diff --git a/server/src/core/server/utils/partition.ts b/server/src/core/server/utils/partition.ts deleted file mode 100644 index 650f3ecd84..0000000000 --- a/server/src/core/server/utils/partition.ts +++ /dev/null @@ -1,19 +0,0 @@ -export const partition = ( - items: T[], - predicate: (item: T) => boolean -): { - passed: T[]; - failed: T[]; -} => { - const passed: T[] = []; - const failed: T[] = []; - for (const item of items) { - if (predicate(item)) { - passed.push(item); - } else { - failed.push(item); - } - } - - return { passed, failed }; -}; From 6e9bb230716e9a98c77014b6d2e7302d5986edf5 Mon Sep 17 00:00:00 2001 From: Marcus Haddon Date: Tue, 7 Nov 2023 14:09:13 -0800 Subject: [PATCH 9/9] remove comment also flag comment before rejecting for banned word --- .../server/services/comments/pipeline/phases/external.ts | 1 - .../services/comments/pipeline/phases/wordList/phase.ts | 6 ++++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/server/src/core/server/services/comments/pipeline/phases/external.ts b/server/src/core/server/services/comments/pipeline/phases/external.ts index c0315cac30..1aaa64faea 100644 --- a/server/src/core/server/services/comments/pipeline/phases/external.ts +++ b/server/src/core/server/services/comments/pipeline/phases/external.ts @@ -102,7 +102,6 @@ export interface ExternalModerationRequest { tenantDomain: string; } -// BOOKMARK: (marcushaddon) do we need a new type to maintain type safety with joi? export type ExternalModerationResponse = Partial< Pick > & { actions: PhaseResult["commentActions"] }; diff --git a/server/src/core/server/services/comments/pipeline/phases/wordList/phase.ts b/server/src/core/server/services/comments/pipeline/phases/wordList/phase.ts index 0bb102380e..358606d939 100644 --- a/server/src/core/server/services/comments/pipeline/phases/wordList/phase.ts +++ b/server/src/core/server/services/comments/pipeline/phases/wordList/phase.ts @@ -31,6 +31,12 @@ export const wordListPhase: IntermediateModerationPhase = async ({ if (banned.isMatched) { return { status: GQLCOMMENT_STATUS.REJECTED, + commentActions: [ + { + actionType: ACTION_TYPE.FLAG, + reason: GQLCOMMENT_FLAG_REASON.COMMENT_DETECTED_BANNED_WORD, + }, + ], moderationAction: { status: GQLCOMMENT_STATUS.REJECTED, moderatorID: null,