From 626d46d8131fdf94a2c94f1951778d587f938e15 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Mon, 21 Oct 2019 18:34:41 +0000 Subject: [PATCH] [CORL-726] Repeat Comment Fix (#2655) * fix: strip tags when comparing for repeat - share the stripped tag version of the comment - share the logger to improve pipeline clarity * fix: renamed variables based on review --- src/core/server/services/comments/comments.ts | 2 + .../comments/pipeline/phases/commentLength.ts | 34 ++++++-------- .../comments/pipeline/phases/repeatPost.ts | 45 ++++++++++--------- .../services/comments/pipeline/phases/spam.ts | 18 +++----- .../comments/pipeline/phases/toxic.ts | 14 +++--- .../services/comments/pipeline/pipeline.ts | 22 +++++++-- 6 files changed, 70 insertions(+), 65 deletions(-) diff --git a/src/core/server/services/comments/comments.ts b/src/core/server/services/comments/comments.ts index 8e1abca98..6b7ed3643 100644 --- a/src/core/server/services/comments/comments.ts +++ b/src/core/server/services/comments/comments.ts @@ -124,6 +124,7 @@ export async function create( // Run the comment through the moderation phases. result = await processForModeration({ action: "NEW", + log, mongo, redis, config, @@ -323,6 +324,7 @@ export async function edit( // Run the comment through the moderation phases. const { body, status, metadata, actions } = await processForModeration({ action: "EDIT", + log, mongo, redis, config, diff --git a/src/core/server/services/comments/pipeline/phases/commentLength.ts b/src/core/server/services/comments/pipeline/phases/commentLength.ts index 8fb76026c..2510962ea 100644 --- a/src/core/server/services/comments/pipeline/phases/commentLength.ts +++ b/src/core/server/services/comments/pipeline/phases/commentLength.ts @@ -1,37 +1,29 @@ -import striptags from "striptags"; +import { isNil } from "lodash"; import { CommentBodyExceedsMaxLengthError, CommentBodyTooShortError, } from "coral-server/errors"; -import { Settings } from "coral-server/models/settings"; import { IntermediateModerationPhase, IntermediatePhaseResult, } from "coral-server/services/comments/pipeline"; -import { isNil } from "lodash"; -const testCharCount = (settings: Partial, length: number) => { - if (settings.charCount && settings.charCount.enabled) { - if (!isNil(settings.charCount.min)) { - if (length < settings.charCount.min) { - throw new CommentBodyTooShortError(settings.charCount.min); +export const commentLength: IntermediateModerationPhase = ({ + tenant, + htmlStripped, +}): IntermediatePhaseResult | void => { + const length = htmlStripped.trim().length; + if (tenant.charCount && tenant.charCount.enabled) { + if (!isNil(tenant.charCount.min)) { + if (length < tenant.charCount.min) { + throw new CommentBodyTooShortError(tenant.charCount.min); } } - if (!isNil(settings.charCount.max)) { - if (length > settings.charCount.max) { - throw new CommentBodyExceedsMaxLengthError(settings.charCount.max); + if (!isNil(tenant.charCount.max)) { + if (length > tenant.charCount.max) { + throw new CommentBodyExceedsMaxLengthError(tenant.charCount.max); } } } }; - -export const commentLength: IntermediateModerationPhase = ({ - tenant, - comment, -}): IntermediatePhaseResult | void => { - const length = striptags(comment.body).length; - - // Reject if the comment is too long or too short. - testCharCount(tenant, length); -}; diff --git a/src/core/server/services/comments/pipeline/phases/repeatPost.ts b/src/core/server/services/comments/pipeline/phases/repeatPost.ts index 6bd39ed77..d000eafc3 100644 --- a/src/core/server/services/comments/pipeline/phases/repeatPost.ts +++ b/src/core/server/services/comments/pipeline/phases/repeatPost.ts @@ -1,9 +1,6 @@ +import striptags from "striptags"; + import { RepeatPostCommentError } from "coral-server/errors"; -import { - GQLCOMMENT_FLAG_REASON, - GQLCOMMENT_STATUS, -} from "coral-server/graph/tenant/schema/__generated__/types"; -import logger from "coral-server/logger"; import { ACTION_TYPE } from "coral-server/models/action/comment"; import { getLatestRevision } from "coral-server/models/comment/helpers"; import { @@ -12,46 +9,50 @@ import { } from "coral-server/services/comments/pipeline"; import { retrieveUserLastComment } from "coral-server/services/users"; +import { + GQLCOMMENT_FLAG_REASON, + GQLCOMMENT_STATUS, +} from "coral-server/graph/tenant/schema/__generated__/types"; + export const repeatPost: IntermediateModerationPhase = async ({ - story, mongo, + htmlStripped, tenant, - comment, author, - req, nudge, redis, + log, }): Promise => { - const log = logger.child( - { - tenantID: tenant.id, - }, - true - ); - - if (!comment.body) { + if (!htmlStripped) { return; } try { log.trace("checking comment for repeat content"); + // Get the last comment (if it exists). const lastComment = await retrieveUserLastComment( mongo, redis, tenant, author ); - if (!lastComment) { + // The last comment can't been found or none was written within the + // time frame. return; } - const revision = getLatestRevision(lastComment); - const isRepeatComment = revision.body === comment.body; + const revision = striptags(getLatestRevision(lastComment).body); - if (isRepeatComment) { - log.trace({ isRepeatComment }, "comment contains repeat content"); + // Calculate the comment similarity. At the moment, we only do a string + // comparison, so it's either completely equal (they match) or the + // similarity can't be determined (null). This gives us room in the future + // to include a percentage matching. + const similarity = revision.trim() === htmlStripped.trim() ? 1 : null; + + if (similarity) { + log.trace({ similarity }, "comment contains repeat content"); // Throw an error if we're nudging instead of recording. if (nudge) { @@ -71,7 +72,7 @@ export const repeatPost: IntermediateModerationPhase = async ({ }; } - log.trace({ isRepeatComment }, "comment is not repeated"); + log.trace({ similarity }, "comment is not repeated"); } catch (err) { // Rethrow any RepeatPostError. if (err instanceof RepeatPostCommentError) { diff --git a/src/core/server/services/comments/pipeline/phases/spam.ts b/src/core/server/services/comments/pipeline/phases/spam.ts index 558221ab7..6e96e01d4 100644 --- a/src/core/server/services/comments/pipeline/phases/spam.ts +++ b/src/core/server/services/comments/pipeline/phases/spam.ts @@ -1,17 +1,17 @@ import { Client } from "akismet-api"; import { SpamCommentError } from "coral-server/errors"; -import { - GQLCOMMENT_FLAG_REASON, - GQLCOMMENT_STATUS, -} from "coral-server/graph/tenant/schema/__generated__/types"; -import logger from "coral-server/logger"; import { ACTION_TYPE } from "coral-server/models/action/comment"; import { IntermediateModerationPhase, IntermediatePhaseResult, } from "coral-server/services/comments/pipeline"; +import { + GQLCOMMENT_FLAG_REASON, + GQLCOMMENT_STATUS, +} from "coral-server/graph/tenant/schema/__generated__/types"; + export const spam: IntermediateModerationPhase = async ({ story, tenant, @@ -19,16 +19,10 @@ export const spam: IntermediateModerationPhase = async ({ author, req, nudge, + log, }): Promise => { const integration = tenant.integrations.akismet; - const log = logger.child( - { - tenantID: tenant.id, - }, - true - ); - // We can only check for spam if this comment originated from a graphql // request via an HTTP call. if (!req) { diff --git a/src/core/server/services/comments/pipeline/phases/toxic.ts b/src/core/server/services/comments/pipeline/phases/toxic.ts index 4320381ad..e2a5b76e8 100644 --- a/src/core/server/services/comments/pipeline/phases/toxic.ts +++ b/src/core/server/services/comments/pipeline/phases/toxic.ts @@ -11,11 +11,6 @@ import { } from "coral-common/constants"; import { Omit } from "coral-common/types"; import { ToxicCommentError } from "coral-server/errors"; -import { - GQLCOMMENT_FLAG_REASON, - GQLCOMMENT_STATUS, - GQLPerspectiveExternalIntegration, -} from "coral-server/graph/tenant/schema/__generated__/types"; import logger from "coral-server/logger"; import { ACTION_TYPE } from "coral-server/models/action/comment"; import { @@ -23,17 +18,22 @@ import { IntermediatePhaseResult, } from "coral-server/services/comments/pipeline"; +import { + GQLCOMMENT_FLAG_REASON, + GQLCOMMENT_STATUS, + GQLPerspectiveExternalIntegration, +} from "coral-server/graph/tenant/schema/__generated__/types"; + export const toxic: IntermediateModerationPhase = async ({ tenant, comment, nudge, + log, }): Promise => { if (!comment.body) { return; } - const log = logger.child({ tenantID: tenant.id }, true); - const integration = tenant.integrations.perspective; if (!integration.enabled) { diff --git a/src/core/server/services/comments/pipeline/pipeline.ts b/src/core/server/services/comments/pipeline/pipeline.ts index fd1f8e8e8..062a12a26 100644 --- a/src/core/server/services/comments/pipeline/pipeline.ts +++ b/src/core/server/services/comments/pipeline/pipeline.ts @@ -1,8 +1,9 @@ import { Db } from "mongodb"; +import striptags from "striptags"; import { Omit, Promiseable, RequireProperty } from "coral-common/types"; import { Config } from "coral-server/config"; -import { GQLCOMMENT_STATUS } from "coral-server/graph/tenant/schema/__generated__/types"; +import { Logger } from "coral-server/logger"; import { CreateActionInput } from "coral-server/models/action/comment"; import { EditCommentInput, @@ -15,6 +16,8 @@ import { User } from "coral-server/models/user"; import { AugmentedRedis } from "coral-server/services/redis"; import { Request } from "coral-server/types/express"; +import { GQLCOMMENT_STATUS } from "coral-server/graph/tenant/schema/__generated__/types"; + import { moderationPhases } from "./phases"; export type ModerationAction = Omit< @@ -30,10 +33,11 @@ export interface PhaseResult { tags: CommentTag[]; } -export interface ModerationPhaseContext { +export interface ModerationPhaseContextInput { mongo: Db; redis: AugmentedRedis; config: Config; + log: Logger; story: Story; tenant: Tenant; comment: RequireProperty, "body">; @@ -44,8 +48,15 @@ export interface ModerationPhaseContext { req?: Request; } +export interface ModerationPhaseContext extends ModerationPhaseContextInput { + /** + * htmlStripped is the HTML stripped version of the comment body. + */ + htmlStripped: string; +} + export type RootModerationPhase = ( - context: ModerationPhaseContext + context: ModerationPhaseContextInput ) => Promiseable; export type IntermediatePhaseResult = Partial | void; @@ -74,6 +85,10 @@ export const compose = ( tags: [], }; + // Strip the tags from the comment body so that filters that can't process + // HTML can reuse it. + const htmlStripped = striptags(final.body); + // Loop over all the moderation phases and see if we've resolved the status. for (const phase of phases) { const result = await phase({ @@ -82,6 +97,7 @@ export const compose = ( ...context.comment, body: final.body, }, + htmlStripped, metadata: final.metadata, }); if (result) {