From 5b589bfe2be57bdad76c93b1b89d581b475400df Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Thu, 16 Jul 2020 19:59:51 +0000 Subject: [PATCH] [CORL-1180] Moderation + Editing Improvements (#3020) * fix: handle moderating edited comments better * fix: add back deleted audit trail entry Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> --- .../admin/mutations/ApproveCommentMutation.ts | 19 ++++++- .../admin/mutations/RejectCommentMutation.ts | 19 ++++++- .../admin/test/moderate/regularQueue.spec.tsx | 4 +- .../test/moderate/rejectedQueue.spec.tsx | 2 +- .../test/moderate/singleComment.spec.tsx | 5 +- src/core/server/models/comment/helpers.ts | 7 +++ .../services/comments/moderation/moderate.ts | 53 +++++++++++++++---- src/core/server/stacks/approveComment.ts | 5 ++ src/core/server/stacks/rejectComment.ts | 5 ++ 9 files changed, 98 insertions(+), 21 deletions(-) diff --git a/src/core/client/admin/mutations/ApproveCommentMutation.ts b/src/core/client/admin/mutations/ApproveCommentMutation.ts index a4f7b6be3..773587399 100644 --- a/src/core/client/admin/mutations/ApproveCommentMutation.ts +++ b/src/core/client/admin/mutations/ApproveCommentMutation.ts @@ -47,7 +47,7 @@ const ApproveCommentMutation = createMutation( } } } - ...ModeratedByContainer_comment + ...ModerateCardContainer_comment } moderationQueues( storyID: $storyID @@ -83,7 +83,22 @@ const ApproveCommentMutation = createMutation( proxy.setValue("APPROVED", "status"); proxy.setValue(true, "viewerDidModerate"); }, - updater: (store) => { + updater: (store, data) => { + // If no comment came back or the returned comment status was the same, + // don't remove it from the connections! + if ( + !data.approveComment || + !data.approveComment.comment || + data.approveComment.comment.status !== "APPROVED" + ) { + return; + } + + // Ensure that the comment retains the viewerDidModerate state after it + // comes back from the update. + const proxy = store.get(input.commentID)!; + proxy.setValue(true, "viewerDidModerate"); + const connections = [ getQueueConnection( store, diff --git a/src/core/client/admin/mutations/RejectCommentMutation.ts b/src/core/client/admin/mutations/RejectCommentMutation.ts index 04e3858a0..cf74b1dbb 100644 --- a/src/core/client/admin/mutations/RejectCommentMutation.ts +++ b/src/core/client/admin/mutations/RejectCommentMutation.ts @@ -47,7 +47,7 @@ const RejectCommentMutation = createMutation( } } } - ...ModeratedByContainer_comment + ...ModerateCardContainer_comment } moderationQueues( storyID: $storyID @@ -83,7 +83,22 @@ const RejectCommentMutation = createMutation( proxy.setValue("REJECTED", "status"); proxy.setValue(true, "viewerDidModerate"); }, - updater: (store) => { + updater: (store, data) => { + // If no comment came back or the returned comment status was the same, + // don't remove it from the connections! + if ( + !data.rejectComment || + !data.rejectComment.comment || + data.rejectComment.comment.status !== "REJECTED" + ) { + return; + } + + // Ensure that the comment retains the viewerDidModerate state after it + // comes back from the update. + const proxy = store.get(input.commentID)!; + proxy.setValue(true, "viewerDidModerate"); + const connections = [ getQueueConnection(store, "REPORTED", input.storyID), getQueueConnection(store, "PENDING", input.storyID), diff --git a/src/core/client/admin/test/moderate/regularQueue.spec.tsx b/src/core/client/admin/test/moderate/regularQueue.spec.tsx index 1b2c57c5b..10ff6ca42 100644 --- a/src/core/client/admin/test/moderate/regularQueue.spec.tsx +++ b/src/core/client/admin/test/moderate/regularQueue.spec.tsx @@ -375,7 +375,7 @@ it("approves comment in reported queue", async () => { }); return { comment: { - id: reportedComments[0].id, + ...reportedComments[0], status: GQLCOMMENT_STATUS.APPROVED, statusHistory: { edges: [ @@ -473,7 +473,7 @@ it("rejects comment in reported queue", async () => { }); return { comment: { - id: reportedComments[0].id, + ...reportedComments[0], status: GQLCOMMENT_STATUS.REJECTED, statusHistory: { edges: [ diff --git a/src/core/client/admin/test/moderate/rejectedQueue.spec.tsx b/src/core/client/admin/test/moderate/rejectedQueue.spec.tsx index a845aa9c0..b5d9ce145 100644 --- a/src/core/client/admin/test/moderate/rejectedQueue.spec.tsx +++ b/src/core/client/admin/test/moderate/rejectedQueue.spec.tsx @@ -245,7 +245,7 @@ it("approves comment in rejected queue", async () => { }); return { comment: { - id: rejectedComments[0].id, + ...rejectedComments[0], status: GQLCOMMENT_STATUS.APPROVED, statusHistory: { edges: [ diff --git a/src/core/client/admin/test/moderate/singleComment.spec.tsx b/src/core/client/admin/test/moderate/singleComment.spec.tsx index 9752f892b..76d20a4ab 100644 --- a/src/core/client/admin/test/moderate/singleComment.spec.tsx +++ b/src/core/client/admin/test/moderate/singleComment.spec.tsx @@ -95,7 +95,7 @@ it("approves single comment", async () => { }); return { comment: { - id: comment.id, + ...comment, status: GQLCOMMENT_STATUS.APPROVED, statusHistory: { edges: [ @@ -148,9 +148,8 @@ it("rejects single comment", async () => { }); return { comment: { - id: comment.id, + ...comment, status: GQLCOMMENT_STATUS.REJECTED, - author: comment.author, statusHistory: { edges: [ { diff --git a/src/core/server/models/comment/helpers.ts b/src/core/server/models/comment/helpers.ts index fc6fea031..0bb08c859 100644 --- a/src/core/server/models/comment/helpers.ts +++ b/src/core/server/models/comment/helpers.ts @@ -47,6 +47,13 @@ export function getLatestRevision( return comment.revisions[comment.revisions.length - 1]; } +export function hasRevision( + comment: Pick, + revisionID: string +): boolean { + return comment.revisions.some((revision) => revision.id === revisionID); +} + export function calculateRejectionRate(counts: CommentStatusCounts): number { const published = calculateTotalPublishedCommentCount(counts); const rejected = counts[GQLCOMMENT_STATUS.REJECTED]; diff --git a/src/core/server/services/comments/moderation/moderate.ts b/src/core/server/services/comments/moderation/moderate.ts index 89f47429b..248362994 100644 --- a/src/core/server/services/comments/moderation/moderate.ts +++ b/src/core/server/services/comments/moderation/moderate.ts @@ -5,7 +5,12 @@ import { createCommentModerationAction, CreateCommentModerationActionInput, } from "coral-server/models/action/moderation/comment"; -import { updateCommentStatus } from "coral-server/models/comment"; +import { + getLatestRevision, + hasRevision, + retrieveComment, + updateCommentStatus, +} from "coral-server/models/comment"; import { Tenant } from "coral-server/models/tenant"; export type Moderate = CreateCommentModerationActionInput; @@ -18,18 +23,32 @@ export default async function moderate( ) { // TODO: wrap these operations in a transaction? - // Create the moderation action in the audit log. - const action = await createCommentModerationAction( - mongo, - tenant.id, - input, - now - ); - if (!action) { - // TODO: wrap in better error? - throw new Error("could not create moderation action"); + // Get the comment that we're moderating. + const comment = await retrieveComment(mongo, tenant.id, input.commentID); + if (!comment) { + throw new CommentNotFoundError(input.commentID, input.commentRevisionID); } + // Get the latest revision on that comment. + const revision = getLatestRevision(comment); + + // Ensure that the latest revision is the same revision that we're moderating. + if (revision.id !== input.commentRevisionID) { + // The revision has been updated since then! Ensure that this revision ID + // does exist. + if (!hasRevision(comment, input.commentRevisionID)) { + throw new CommentNotFoundError(input.commentID, input.commentRevisionID); + } + + // The comment has this revision, it just isn't the latest one. Return the + // same comment back because we didn't modify anything. + return { before: comment, after: null }; + } + + // TODO: (wyattjoh) this is a pretty race condition prone check here, replace + // with a more concrete query that can prevent the comment being edited in the + // time it takes to go from the above block to the next block. + // Update the Comment's status. const result = await updateCommentStatus( mongo, @@ -42,5 +61,17 @@ export default async function moderate( throw new CommentNotFoundError(input.commentID, input.commentRevisionID); } + // Create the moderation action in the audit log. + const action = await createCommentModerationAction( + mongo, + tenant.id, + input, + now + ); + if (!action) { + // TODO: wrap in better error? + throw new Error("could not create moderation action"); + } + return result; } diff --git a/src/core/server/stacks/approveComment.ts b/src/core/server/stacks/approveComment.ts index 15740bdf6..fa1174cff 100644 --- a/src/core/server/stacks/approveComment.ts +++ b/src/core/server/stacks/approveComment.ts @@ -32,6 +32,11 @@ const approveComment = async ( now ); + // If the comment hasn't been updated, skip the rest of the steps. + if (!result.after) { + return result.before; + } + // Update all the comment counts on stories and users. const counts = await updateAllCommentCounts(mongo, redis, { ...result, diff --git a/src/core/server/stacks/rejectComment.ts b/src/core/server/stacks/rejectComment.ts index 7eced5118..5487545ae 100644 --- a/src/core/server/stacks/rejectComment.ts +++ b/src/core/server/stacks/rejectComment.ts @@ -37,6 +37,11 @@ const rejectComment = async ( now ); + // If the comment hasn't been updated, skip the rest of the steps. + if (!result.after) { + return result.before; + } + // Update all the comment counts on stories and users. const counts = await updateAllCommentCounts(mongo, redis, { ...result,