From 81338d1386a2970c62c9f6ff02dfe4a690ef3636 Mon Sep 17 00:00:00 2001 From: Nick Funk Date: Wed, 8 Jan 2020 10:32:24 -0700 Subject: [PATCH] [CORL-825-cp] Allow sending of moderation decisions to Google (#2783) * Allow sending of moderation decisions to Google If enabled in the config, moderation decisions can be sent to Google. Simplify OnOffField test id's Consolidate guard clauses Reconstruct tenant url's when sending perspective feedback Use the comment revision id when sending perspective feedback Use retrieveStory instead of findStory Clarify logging warning levels on perspective feedback Clean html tags from sent comment body Fix include to match new graph context refactors CORL-825 * Handle asynchronous perspective operation more cleanly CORL-825 --- .../admin/routes/Configure/OnOffField.tsx | 13 +- .../sections/Moderation/PerspectiveConfig.tsx | 38 ++++ .../__snapshots__/moderation.spec.tsx.snap | 71 ++++++++ .../admin/test/configure/moderation.spec.tsx | 80 ++++++++- src/core/client/admin/test/fixtures.ts | 1 + src/core/server/graph/mutators/Actions.ts | 2 + src/core/server/graph/mutators/Comments.ts | 1 + src/core/server/graph/schema/schema.graphql | 11 ++ src/core/server/models/tenant/tenant.ts | 1 + .../comments/pipeline/phases/toxic.ts | 7 +- src/core/server/services/perspective/index.ts | 162 ++++++++++++++++++ src/core/server/stacks/approveComment.ts | 15 ++ src/core/server/stacks/rejectComment.ts | 15 ++ src/locales/en-US/admin.ftl | 5 + 14 files changed, 417 insertions(+), 5 deletions(-) create mode 100644 src/core/server/services/perspective/index.ts diff --git a/src/core/client/admin/routes/Configure/OnOffField.tsx b/src/core/client/admin/routes/Configure/OnOffField.tsx index 4657854c5..f0e426c3f 100644 --- a/src/core/client/admin/routes/Configure/OnOffField.tsx +++ b/src/core/client/admin/routes/Configure/OnOffField.tsx @@ -15,6 +15,10 @@ interface Props { offLabel?: React.ReactNode; format?: (value: any, name: string) => any; parse?: (value: any, name: string) => any; + testIDs?: { + on: string; + off: string; + }; className?: string; } @@ -26,6 +30,7 @@ const OnOffField: FunctionComponent = ({ invert = false, parse = parseStringBool, format = formatBool, + testIDs, className, }) => (
@@ -37,7 +42,12 @@ const OnOffField: FunctionComponent = ({ format={format} > {({ input }) => ( - + {onLabel || ( On @@ -52,6 +62,7 @@ const OnOffField: FunctionComponent = ({ parse={parse} format={format} value={JSON.stringify(invert)} + data-testid={testIDs ? testIDs.off : undefined} > {({ input }) => ( diff --git a/src/core/client/admin/routes/Configure/sections/Moderation/PerspectiveConfig.tsx b/src/core/client/admin/routes/Configure/sections/Moderation/PerspectiveConfig.tsx index cacdaee7a..9d874ce29 100644 --- a/src/core/client/admin/routes/Configure/sections/Moderation/PerspectiveConfig.tsx +++ b/src/core/client/admin/routes/Configure/sections/Moderation/PerspectiveConfig.tsx @@ -52,6 +52,7 @@ graphql` model threshold doNotStore + sendFeedback } } } @@ -200,6 +201,10 @@ const PerspectiveConfig: FunctionComponent = ({ disabled }) => { Allow @@ -213,6 +218,39 @@ const PerspectiveConfig: FunctionComponent = ({ disabled }) => { invert /> + }> + + + + + + + Sent moderation actions will be used for future research and + community model building purposes to improve the API over time. + + + + + Allow + + } + offLabel={ + + Don't Allow + + } + /> + }> Configuration diff --git a/src/core/client/admin/test/configure/__snapshots__/moderation.spec.tsx.snap b/src/core/client/admin/test/configure/__snapshots__/moderation.spec.tsx.snap index d4a90365c..3a305043b 100644 --- a/src/core/client/admin/test/configure/__snapshots__/moderation.spec.tsx.snap +++ b/src/core/client/admin/test/configure/__snapshots__/moderation.spec.tsx.snap @@ -819,6 +819,7 @@ improve the API over time.
+
+
+ + Allow Coral to send moderation actions to Google + +

+ Sent moderation actions will be used for future research and +community model building purposes to improve the API over time. +

+
+
+
+ + +
+
+ + +
+
+
diff --git a/src/core/client/admin/test/configure/moderation.spec.tsx b/src/core/client/admin/test/configure/moderation.spec.tsx index d4b5ae330..7f388b8b9 100644 --- a/src/core/client/admin/test/configure/moderation.spec.tsx +++ b/src/core/client/admin/test/configure/moderation.spec.tsx @@ -21,14 +21,15 @@ beforeEach(() => { const viewer = users.admins[0]; async function createTestRenderer( - params: CreateTestRendererParams = {} + params: CreateTestRendererParams = {}, + settingsOverride?: any ) { const { testRenderer } = create({ ...params, resolvers: pureMerge( createResolversStub({ Query: { - settings: () => settings, + settings: () => (settingsOverride ? settingsOverride : settings), viewer: () => viewer, }, }), @@ -239,6 +240,7 @@ it("change perspective settings", async () => { key: "my api key", model: null, threshold: 0.1, + sendFeedback: false, } ); break; @@ -262,7 +264,9 @@ it("change perspective settings", async () => { ); const onField = within(perspectiveContainer).getByLabelText("On"); - const allowField = within(perspectiveContainer).getByLabelText("Allow"); + const allowField = within(perspectiveContainer).getByTestID( + "test-allowStoreCommentData" + ); const keyField = within(perspectiveContainer).getByLabelText("API key"); const thresholdField = within(perspectiveContainer).getByLabelText( "Toxicity threshold" @@ -361,3 +365,73 @@ it("change perspective settings", async () => { // Should have successfully sent with server. expect(resolvers.Mutation!.updateSettings!.calledTwice).toBe(true); }); + +it("change perspective send feedback setting", async () => { + const settingsOverride = settings; + settingsOverride.integrations.perspective = { + doNotStore: false, + enabled: true, + endpoint: "https://custom-endpoint.net", + key: "api key", + model: "TOXIC_MODEL", + threshold: 0.1, + sendFeedback: false, + }; + + const resolvers = createResolversStub({ + Mutation: { + updateSettings: ({ variables }) => { + expectAndFail(variables.settings.integrations!.perspective).toEqual({ + doNotStore: false, + enabled: true, + endpoint: "https://custom-endpoint.net", + key: "api key", + model: "TOXIC_MODEL", + threshold: 0.1, + sendFeedback: true, + }); + + return { + settings: pureMerge(settings, variables.settings), + }; + }, + }, + }); + const { moderationContainer, saveChangesButton } = await createTestRenderer( + { + resolvers, + }, + settingsOverride + ); + + const perspectiveContainer = within(moderationContainer).getByTestID( + "perspective-container" + ); + const onField = within(perspectiveContainer).getByLabelText("On"); + const allowField = within(perspectiveContainer).getByTestID( + "test-allowSendFeedback" + ); + const form = findParentWithType(perspectiveContainer, "form")!; + + // Let's turn it on. + act(() => allowField.props.onChange(allowField.props.value.toString())); + expect(saveChangesButton.props.disabled).toBe(false); + + // Send form + act(() => { + form.props.onSubmit(); + }); + + // Wait for submission to be finished + await act(async () => { + await wait(() => { + expect(onField.props.disabled).toBe(false); + }); + }); + + // Submit button and text field should be disabled. + expect(saveChangesButton.props.disabled).toBe(true); + + // Should have successfully sent with server. + expect(resolvers.Mutation!.updateSettings!.calledOnce).toBe(true); +}); diff --git a/src/core/client/admin/test/fixtures.ts b/src/core/client/admin/test/fixtures.ts index 200317f50..e77a613fb 100644 --- a/src/core/client/admin/test/fixtures.ts +++ b/src/core/client/admin/test/fixtures.ts @@ -90,6 +90,7 @@ export const settings = createFixture({ perspective: { enabled: false, threshold: TOXICITY_THRESHOLD_DEFAULT / 100, + sendFeedback: false, }, }, auth: { diff --git a/src/core/server/graph/mutators/Actions.ts b/src/core/server/graph/mutators/Actions.ts index dcedd783d..7cd3faee6 100644 --- a/src/core/server/graph/mutators/Actions.ts +++ b/src/core/server/graph/mutators/Actions.ts @@ -11,6 +11,7 @@ export const Actions = (ctx: GraphContext) => ({ approveComment( ctx.mongo, ctx.redis, + ctx.config, ctx.publisher, ctx.tenant, input.commentID, @@ -22,6 +23,7 @@ export const Actions = (ctx: GraphContext) => ({ rejectComment( ctx.mongo, ctx.redis, + ctx.config, ctx.publisher, ctx.tenant, input.commentID, diff --git a/src/core/server/graph/mutators/Comments.ts b/src/core/server/graph/mutators/Comments.ts index 70f1704fe..e118916cf 100644 --- a/src/core/server/graph/mutators/Comments.ts +++ b/src/core/server/graph/mutators/Comments.ts @@ -173,6 +173,7 @@ export const Comments = (ctx: GraphContext) => ({ ? approveComment( ctx.mongo, ctx.redis, + ctx.config, ctx.publisher, ctx.tenant, commentID, diff --git a/src/core/server/graph/schema/schema.graphql b/src/core/server/graph/schema/schema.graphql index bec03d482..e57fa3ec3 100644 --- a/src/core/server/graph/schema/schema.graphql +++ b/src/core/server/graph/schema/schema.graphql @@ -827,6 +827,12 @@ type PerspectiveExternalIntegration { When True, comments sent will not be stored by the Google Perspective API. """ doNotStore: Boolean @auth(roles: [ADMIN]) + + """ + When True, comment moderation decisions will be sent to the Google + Perspective API to help improve the comment analysis algorithms. + """ + sendFeedback: Boolean @auth(roles: [ADMIN]) } type ExternalIntegrations { @@ -3275,6 +3281,11 @@ input SettingsPerspectiveExternalIntegrationInput { When True, comments sent will not be stored by the Google Perspective API. """ doNotStore: Boolean + + """ + When True, moderation actions will be sent to the Google Perspective API. + """ + sendFeedback: Boolean } input SettingsExternalIntegrationsInput { diff --git a/src/core/server/models/tenant/tenant.ts b/src/core/server/models/tenant/tenant.ts index 77a9451c0..935c429ea 100644 --- a/src/core/server/models/tenant/tenant.ts +++ b/src/core/server/models/tenant/tenant.ts @@ -182,6 +182,7 @@ export async function createTenant( perspective: { enabled: false, doNotStore: true, + sendFeedback: false, }, }, reaction: getDefaultReactionConfiguration(bundle), diff --git a/src/core/server/services/comments/pipeline/phases/toxic.ts b/src/core/server/services/comments/pipeline/phases/toxic.ts index a2303520e..afc02896f 100644 --- a/src/core/server/services/comments/pipeline/phases/toxic.ts +++ b/src/core/server/services/comments/pipeline/phases/toxic.ts @@ -210,7 +210,12 @@ async function getScore( endpoint, model, doNotStore, - }: Required>, + }: Required< + Omit< + GQLPerspectiveExternalIntegration, + "enabled" | "threshold" | "sendFeedback" + > + >, language: PerspectiveLanguage, timeout: number ): Promise { diff --git a/src/core/server/services/perspective/index.ts b/src/core/server/services/perspective/index.ts new file mode 100644 index 000000000..1153cf543 --- /dev/null +++ b/src/core/server/services/perspective/index.ts @@ -0,0 +1,162 @@ +import { Db } from "mongodb"; +import striptags from "striptags"; + +import { TOXICITY_ENDPOINT_DEFAULT } from "coral-common/constants"; +import { reconstructTenantURL } from "coral-server/app/url"; +import { Config } from "coral-server/config"; +import logger from "coral-server/logger"; +import { Comment } from "coral-server/models/comment"; +import { getURLWithCommentID, retrieveStory } from "coral-server/models/story"; +import { Tenant } from "coral-server/models/tenant"; + +import { + GQLCOMMENT_STATUS, + GQLPerspectiveExternalIntegration, +} from "coral-server/graph/schema/__generated__/types"; + +interface SendResult { + ok: boolean; + status: number; + data: any; +} + +async function send( + endpoint: string, + apiKey: string, + method: string, + body: any +): Promise { + const result = await fetch(`${endpoint}/${method}?key=${apiKey}`, { + method: "POST", + headers: { + "Content-Type": "application/json", + }, + body: JSON.stringify(body, null, 2), + }); + + if (!result.ok) { + return { + ok: result.ok, + status: result.status, + data: null, + }; + } + + const data = await result.json(); + + return { + ok: result.ok, + status: result.status, + data, + }; +} + +function computeStatus(status: GQLCOMMENT_STATUS) { + if (status === GQLCOMMENT_STATUS.APPROVED) { + return "APPROVED"; + } + if (status === GQLCOMMENT_STATUS.REJECTED) { + return "DELETED"; + } + + return null; +} + +export async function notifyPerspectiveModerationDecision( + mongo: Db, + tenant: Tenant, + config: Config, + perspectiveConfig: GQLPerspectiveExternalIntegration, + comment: Comment, + commentRevisionID: string, + status: GQLCOMMENT_STATUS +) { + if ( + !perspectiveConfig.enabled || + !perspectiveConfig.key || + !perspectiveConfig.sendFeedback + ) { + return; + } + + const commentStatus = computeStatus(status); + if (!commentStatus) { + return; + } + + const revision = comment.revisions.find(c => c.id === commentRevisionID); + if (!revision) { + logger.warn( + { commentID: comment.id, commentRevisionID }, + "unable to find comment revision ID in comment revision history" + ); + return; + } + + const endpoint = perspectiveConfig.endpoint + ? perspectiveConfig.endpoint + : TOXICITY_ENDPOINT_DEFAULT; + const apiKey = perspectiveConfig.key; + + const tenantUrl = reconstructTenantURL(config, tenant, undefined, "/"); + const communityId = `Coral:${tenantUrl}`; + const clientToken = `comment:${comment.id}`; + + try { + const story = await retrieveStory(mongo, comment.tenantID, comment.storyID); + if (!story) { + logger.warn({ storyID: comment.storyID }, "could not find story"); + return; + } + + const url = getURLWithCommentID(story.url, comment.id); + + const body = { + comment: { + text: striptags(revision.body), + }, + context: { + entries: [ + { + text: JSON.stringify({ + url, + reply_to_id_Coral_comment_id: comment.parentID, + Coral_comment_id: comment.id, + }), + }, + ], + }, + attributeScores: { + [commentStatus]: { + summaryScore: { + value: 1, + }, + }, + }, + languages: ["EN"], + communityId, + clientToken, + }; + + const result = await send(endpoint, apiKey, "comments:suggestscore", body); + + if (result.ok) { + logger.debug( + { commentID: comment.id, commentRevisionID }, + "successfully sent moderation feedback to perspective" + ); + } else if (!result.ok) { + logger.error( + { status: result.status }, + "unable to send moderation feedback to perspective" + ); + } else if (!result.data || result.data.clientToken !== clientToken) { + logger.error( + { data: result.data }, + "result data from perspective did not contain the clientToken we expected" + ); + } + } catch (err) { + logger.error({ err }, "unable to send moderation feedback to perspective"); + } +} diff --git a/src/core/server/stacks/approveComment.ts b/src/core/server/stacks/approveComment.ts index f41fb56c6..d4dd1f16b 100644 --- a/src/core/server/stacks/approveComment.ts +++ b/src/core/server/stacks/approveComment.ts @@ -1,8 +1,10 @@ import { Db } from "mongodb"; +import { Config } from "coral-server/config"; import { Publisher } from "coral-server/graph/subscriptions/publisher"; import { Tenant } from "coral-server/models/tenant"; import { moderate } from "coral-server/services/comments/moderation"; +import { notifyPerspectiveModerationDecision } from "coral-server/services/perspective"; import { AugmentedRedis } from "coral-server/services/redis"; import { GQLCOMMENT_STATUS } from "coral-server/graph/schema/__generated__/types"; @@ -12,6 +14,7 @@ import { publishChanges, updateAllCounts } from "./helpers"; const approveComment = async ( mongo: Db, redis: AugmentedRedis, + config: Config, publisher: Publisher, tenant: Tenant, commentID: string, @@ -45,6 +48,18 @@ const approveComment = async ( moderatorID, }); + // We don't want to await on this so that + // we don't hold up the moderation flow and response + notifyPerspectiveModerationDecision( + mongo, + tenant, + config, + tenant.integrations.perspective, + result.after, + commentRevisionID, + GQLCOMMENT_STATUS.APPROVED + ); + // Return the resulting comment. return result.after; }; diff --git a/src/core/server/stacks/rejectComment.ts b/src/core/server/stacks/rejectComment.ts index b96bda49d..84e246797 100644 --- a/src/core/server/stacks/rejectComment.ts +++ b/src/core/server/stacks/rejectComment.ts @@ -1,10 +1,12 @@ import { Db } from "mongodb"; +import { Config } from "coral-server/config"; import { Publisher } from "coral-server/graph/subscriptions/publisher"; import { hasTag } from "coral-server/models/comment"; import { Tenant } from "coral-server/models/tenant"; import { removeTag } from "coral-server/services/comments"; import { moderate } from "coral-server/services/comments/moderation"; +import { notifyPerspectiveModerationDecision } from "coral-server/services/perspective"; import { AugmentedRedis } from "coral-server/services/redis"; import { @@ -17,6 +19,7 @@ import { publishChanges, updateAllCounts } from "./helpers"; const rejectComment = async ( mongo: Db, redis: AugmentedRedis, + config: Config, publisher: Publisher, tenant: Tenant, commentID: string, @@ -55,6 +58,18 @@ const rejectComment = async ( return removeTag(mongo, tenant, result.after.id, GQLTAG.FEATURED); } + // We don't want to await on this so that + // we don't hold up the moderation flow and response + notifyPerspectiveModerationDecision( + mongo, + tenant, + config, + tenant.integrations.perspective, + result.after, + commentRevisionID, + GQLCOMMENT_STATUS.REJECTED + ); + // Return the resulting comment. return result.after; }; diff --git a/src/locales/en-US/admin.ftl b/src/locales/en-US/admin.ftl index e717808b9..a6e41d7b0 100644 --- a/src/locales/en-US/admin.ftl +++ b/src/locales/en-US/admin.ftl @@ -316,6 +316,11 @@ configure-moderation-perspective-allowStoreCommentData = Allow Google to store c configure-moderation-perspective-allowStoreCommentDataDescription = Stored comments will be used for future research and community model building purposes to improve the API over time. +configure-moderation-perspective-allowSendFeedback = + Allow Coral to send moderation actions to Google +configure-moderation-perspective-allowSendFeedbackDescription = + Sent moderation actions will be used for future research and + community model building purposes to improve the API over time. configure-moderation-perspective-customEndpoint = Custom endpoint configure-moderation-perspective-defaultEndpoint = By default the endpoint is set to { $default }. You may override this here.