From 49f09b87a06a1ac42b126ef63d02bde1ef46e3f4 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Mon, 11 Sep 2017 11:09:21 -0600 Subject: [PATCH 1/9] Added new SYSTEM_WITHHELD comment status --- client/coral-admin/src/actions/userDetail.js | 6 ++---- client/coral-admin/src/reducers/userDetail.js | 2 +- client/coral-embed-stream/src/containers/Stream.js | 2 +- client/coral-embed-stream/src/graphql/index.js | 4 ++-- client/talk-plugin-commentbox/CommentBox.js | 2 +- graph/mutators/comment.js | 12 +++++++++--- graph/typeDefs.graphql | 4 ++++ models/enum/comment_status.js | 1 + plugins/talk-plugin-toxic-comments/server/hooks.js | 4 +--- services/comments.js | 6 ++++-- test/server/graph/mutations/editComment.js | 2 +- 11 files changed, 27 insertions(+), 18 deletions(-) diff --git a/client/coral-admin/src/actions/userDetail.js b/client/coral-admin/src/actions/userDetail.js index fe9aa2df5..504183f97 100644 --- a/client/coral-admin/src/actions/userDetail.js +++ b/client/coral-admin/src/actions/userDetail.js @@ -4,10 +4,8 @@ export const viewUserDetail = (userId) => ({type: actions.VIEW_USER_DETAIL, user export const hideUserDetail = () => ({type: actions.HIDE_USER_DETAIL}); export const changeUserDetailStatuses = (tab) => { - let statuses; - if (tab === 'all') { - statuses = ['NONE', 'ACCEPTED', 'REJECTED', 'PREMOD']; - } else if (tab === 'rejected') { + let statuses = []; + if (tab === 'rejected') { statuses = ['REJECTED']; } return {type: actions.CHANGE_USER_DETAIL_STATUSES, tab, statuses}; diff --git a/client/coral-admin/src/reducers/userDetail.js b/client/coral-admin/src/reducers/userDetail.js index 8e1d35885..524dcfa31 100644 --- a/client/coral-admin/src/reducers/userDetail.js +++ b/client/coral-admin/src/reducers/userDetail.js @@ -3,7 +3,7 @@ import * as actions from '../constants/userDetail'; const initialState = { userId: null, activeTab: 'all', - statuses: ['NONE', 'ACCEPTED', 'REJECTED', 'PREMOD'], + statuses: [], selectedCommentIds: [], }; diff --git a/client/coral-embed-stream/src/containers/Stream.js b/client/coral-embed-stream/src/containers/Stream.js index 273d39589..0a701a029 100644 --- a/client/coral-embed-stream/src/containers/Stream.js +++ b/client/coral-embed-stream/src/containers/Stream.js @@ -51,7 +51,7 @@ class StreamContainer extends React.Component { return prev; } - if (['PREMOD', 'REJECTED'].includes(commentEdited.status)) { + if (['PREMOD', 'REJECTED', 'SYSTEM_WITHHELD'].includes(commentEdited.status)) { return removeCommentFromEmbedQuery(prev, commentEdited.id); } }, diff --git a/client/coral-embed-stream/src/graphql/index.js b/client/coral-embed-stream/src/graphql/index.js index ad281ce0c..884a9c86a 100644 --- a/client/coral-embed-stream/src/graphql/index.js +++ b/client/coral-embed-stream/src/graphql/index.js @@ -166,7 +166,7 @@ export default { }, updateQueries: { CoralEmbedStream_Embed: (prev, {mutationResult: {data: {createComment: {comment}}}}) => { - if (prev.asset.settings.moderation === 'PRE' || comment.status === 'PREMOD' || comment.status === 'REJECTED') { + if (prev.asset.settings.moderation === 'PRE' || comment.status === 'PREMOD' || comment.status === 'REJECTED' || comment.status === 'SYSTEM_WITHHELD') { return prev; } return insertCommentIntoEmbedQuery(prev, comment); @@ -185,7 +185,7 @@ export default { EditComment: () => ({ updateQueries: { CoralEmbedStream_Embed: (prev, {mutationResult: {data: {editComment: {comment}}}}) => { - if (!['PREMOD', 'REJECTED'].includes(comment.status)) { + if (!['PREMOD', 'REJECTED', 'SYSTEM_WITHHELD'].includes(comment.status)) { return null; } return removeCommentFromEmbedQuery(prev, comment.id); diff --git a/client/talk-plugin-commentbox/CommentBox.js b/client/talk-plugin-commentbox/CommentBox.js index 8bdfb9a24..33b46e245 100644 --- a/client/talk-plugin-commentbox/CommentBox.js +++ b/client/talk-plugin-commentbox/CommentBox.js @@ -16,7 +16,7 @@ export const name = 'talk-plugin-commentbox'; export const notifyForNewCommentStatus = (notify, status) => { if (status === 'REJECTED') { notify('error', t('comment_box.comment_post_banned_word')); - } else if (status === 'PREMOD') { + } else if (status === 'PREMOD' || status === 'SYSTEM_WITHHELD') { notify('success', t('comment_box.comment_post_notif_premod')); } }; diff --git a/graph/mutators/comment.js b/graph/mutators/comment.js index c6e335018..16a736d36 100644 --- a/graph/mutators/comment.js +++ b/graph/mutators/comment.js @@ -239,6 +239,12 @@ const resolveNewCommentStatus = async (context, {asset_id, body, status}, wordli throw errors.ErrCommentTooShort; } + // If the status was already defined, don't redefine it. It's only defined + // when specific external conditions exist, we don't want to override that. + if (status && status.length > 0) { + return status; + } + // Decide the status based on whether or not the current asset/settings // has pre-mod enabled or not. If the comment was rejected based on the // wordlist, then reject it, otherwise if the moderation setting is @@ -248,7 +254,7 @@ const resolveNewCommentStatus = async (context, {asset_id, body, status}, wordli } if (settings.premodLinksEnable && linkify.test(body)) { - return 'PREMOD'; + return 'SYSTEM_WITHHELD'; } let asset = await AssetsService.findById(asset_id); @@ -282,11 +288,11 @@ const resolveNewCommentStatus = async (context, {asset_id, body, status}, wordli // Update the response from the comment creation to add the PREMOD so that // that user's UI will reflect the fact that their comment is in pre-mod. - return 'PREMOD'; + return 'SYSTEM_WITHHELD'; } } - return (moderation === 'PRE' || status === 'PREMOD') ? 'PREMOD' : 'NONE'; + return moderation === 'PRE' ? 'PREMOD' : 'NONE'; }; /** diff --git a/graph/typeDefs.graphql b/graph/typeDefs.graphql index 916c0ade5..0ffb6e755 100644 --- a/graph/typeDefs.graphql +++ b/graph/typeDefs.graphql @@ -242,6 +242,10 @@ enum COMMENT_STATUS { # new comments that haven't been moderated yet are referred to as # "premoderated" or "premod" comments. PREMOD + + # SYSTEM_WITHHELD represents a comment that was withheld by the system because + # it was flagged by an internal process for further review. + SYSTEM_WITHHELD } # The types of action there are as enums. diff --git a/models/enum/comment_status.js b/models/enum/comment_status.js index 64633e9de..a0648f72e 100644 --- a/models/enum/comment_status.js +++ b/models/enum/comment_status.js @@ -2,5 +2,6 @@ module.exports = [ 'ACCEPTED', 'REJECTED', 'PREMOD', + 'SYSTEM_WITHHELD', 'NONE' ]; diff --git a/plugins/talk-plugin-toxic-comments/server/hooks.js b/plugins/talk-plugin-toxic-comments/server/hooks.js index 7fd138641..931ca0f60 100644 --- a/plugins/talk-plugin-toxic-comments/server/hooks.js +++ b/plugins/talk-plugin-toxic-comments/server/hooks.js @@ -37,9 +37,7 @@ module.exports = { }); if (commentIsToxic) { - - // TODO: this should have a different status than Premod. - input.status = 'PREMOD'; + input.status = 'SYSTEM_WITHHELD'; } }, async post(_, _input, _context, _info, result) { diff --git a/services/comments.js b/services/comments.js index 82914bcd0..ec55cfb4d 100644 --- a/services/comments.js +++ b/services/comments.js @@ -81,11 +81,13 @@ module.exports = class CommentsService { * @param {String} status the new Comment status */ static async edit({id, author_id, body, status}) { + const EDITABLE_STATUSES = ['NONE', 'PREMOD', 'ACCEPTED']; + const query = { id, author_id, status: { - $in: ['NONE', 'PREMOD', 'ACCEPTED'], + $in: EDITABLE_STATUSES, }, }; @@ -130,7 +132,7 @@ module.exports = class CommentsService { } // Check to see if the comment had a status that was editable. - if (!['NONE', 'PREMOD', 'ACCEPTED'].includes(comment.status)) { + if (!EDITABLE_STATUSES.includes(comment.status)) { debug('rejecting comment edit because original comment has a non-editable status'); throw errors.ErrNotAuthorized; } diff --git a/test/server/graph/mutations/editComment.js b/test/server/graph/mutations/editComment.js index 8507fdc36..234bb308e 100644 --- a/test/server/graph/mutations/editComment.js +++ b/test/server/graph/mutations/editComment.js @@ -215,7 +215,7 @@ describe('graph.mutations.editComment', () => { body: 'I have been edited to add a link: https://coralproject.net/' }, afterEdit: { - status: 'PREMOD', + status: 'SYSTEM_WITHHELD', }, }, ].forEach(({description, settings, beforeEdit, edit, afterEdit, error}) => { From ca411a3227b50d947e70a2a2d4b4249a62a910a8 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Mon, 11 Sep 2017 18:01:01 -0600 Subject: [PATCH 2/9] added support for deep comment counts --- .eslintignore | 1 + .gitignore | 1 + graph/typeDefs.graphql | 3 +- plugins/talk-plugin-deep-reply-count/index.js | 74 +++++++++++++++++++ 4 files changed, 78 insertions(+), 1 deletion(-) create mode 100644 plugins/talk-plugin-deep-reply-count/index.js diff --git a/.eslintignore b/.eslintignore index 21af2df92..fc0214dd2 100644 --- a/.eslintignore +++ b/.eslintignore @@ -25,5 +25,6 @@ plugins/* !plugins/talk-plugin-moderation-actions !plugins/talk-plugin-toxic-comments !plugins/talk-plugin-remember-sort +!plugins/talk-plugin-deep-reply-count node_modules diff --git a/.gitignore b/.gitignore index bba75a377..bcc7c89b1 100644 --- a/.gitignore +++ b/.gitignore @@ -42,5 +42,6 @@ plugins/* !plugins/talk-plugin-moderation-actions !plugins/talk-plugin-toxic-comments !plugins/talk-plugin-remember-sort +!plugins/talk-plugin-deep-reply-count **/node_modules/* diff --git a/graph/typeDefs.graphql b/graph/typeDefs.graphql index 916c0ade5..1c7f349d8 100644 --- a/graph/typeDefs.graphql +++ b/graph/typeDefs.graphql @@ -369,7 +369,8 @@ type Comment { # the replies that were made to the comment. replies(query: RepliesQuery = {}): CommentConnection! - # The count of replies on a comment. + # replyCount is the number of replies with a depth of 1. Only direct replies + # to this comment are counted. replyCount: Int # Actions completed on the parent. Requires the `ADMIN` role. diff --git a/plugins/talk-plugin-deep-reply-count/index.js b/plugins/talk-plugin-deep-reply-count/index.js new file mode 100644 index 000000000..92c7a06ef --- /dev/null +++ b/plugins/talk-plugin-deep-reply-count/index.js @@ -0,0 +1,74 @@ +const _ = require('lodash'); +const DataLoader = require('dataloader'); + +const CommentModel = require('../../models/comment'); + +console.warn('Enabling the talk-plugin-deep-reply-count plugin introduces a signifigant performance impact on larger sites, use with care.'); + +// genDeepCommentCount will return the deep comment count for a given parent id. +const genDeepCommentCount = async (context, parent_ids) => { + + // Get all the replies to the parent comments. + const replies = await CommentModel + .find({ + parent_id: { + $in: _.uniq(parent_ids), + }, + }, { + id: 1, + reply_count: 1, + parent_id: 1, + }); + + // Get all the replies that have comments on them. + const commentedOnReplies = replies.filter(({reply_count}) => { + return reply_count && reply_count > 0; + }); + + let deepReplyCount = []; + + // And if there were any.. + if (commentedOnReplies.length > 0) { + + // Load the reply count for each of them. + deepReplyCount = await context.loaders.Comments.getDeepCount.loadMany(_.uniq(commentedOnReplies.map(({id}) => { + return id; + }))); + } + + // Get all the direct replies to the parent comments. + const allDirectReplies = _.groupBy(replies, 'parent_id'); + + // Collect all the ancestor replies. + const allAncestorReplies = _.groupBy(_.zip(commentedOnReplies, deepReplyCount), ([{parent_id}]) => { + return parent_id; + }); + + // Return the replies in an array matching that of the input parent_ids array. + return parent_ids.map((parent_id) => { + + // Get the direct replies to this comment. + const directReplies = parent_id in allDirectReplies ? allDirectReplies[parent_id] : []; + const ancestorReplies = parent_id in allAncestorReplies ? allAncestorReplies[parent_id] : []; + + // Reduce this array. + return ancestorReplies.reduce((acc, [, count]) => { + return acc + count; + }, directReplies.length); + }); +}; + +module.exports = { + loaders: (context) => ({ + Comments: { + getDeepCount: new DataLoader((parent_ids) => genDeepCommentCount(context, parent_ids)), + } + }), + resolvers: { + Comment: { + replyCount({id}, args, {loaders: {Comments}}) { + return Comments.getDeepCount.load(id); + } + } + } +}; From d820d2d2c370abd8f63b37e5f5d91767191b9f61 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Thu, 14 Sep 2017 13:08:01 -0600 Subject: [PATCH 3/9] moved support for deepReplyCount to seperate field --- plugins/talk-plugin-deep-reply-count/index.js | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/plugins/talk-plugin-deep-reply-count/index.js b/plugins/talk-plugin-deep-reply-count/index.js index 92c7a06ef..f46386c62 100644 --- a/plugins/talk-plugin-deep-reply-count/index.js +++ b/plugins/talk-plugin-deep-reply-count/index.js @@ -59,6 +59,13 @@ const genDeepCommentCount = async (context, parent_ids) => { }; module.exports = { + typeDefs: ` + type Comment { + + # deepReplyCount is the count of all decendant replies. + deepReplyCount: Int + } + `, loaders: (context) => ({ Comments: { getDeepCount: new DataLoader((parent_ids) => genDeepCommentCount(context, parent_ids)), @@ -66,7 +73,7 @@ module.exports = { }), resolvers: { Comment: { - replyCount({id}, args, {loaders: {Comments}}) { + deepReplyCount({id}, args, {loaders: {Comments}}) { return Comments.getDeepCount.load(id); } } From e31cdebc12dec1664c71b3c2d3fc7b0d3c808b9f Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Fri, 15 Sep 2017 22:52:19 +0700 Subject: [PATCH 4/9] Make it appear in reported queue --- client/coral-admin/src/routes/Moderation/queueConfig.js | 2 +- client/coral-embed-stream/src/graphql/index.js | 8 ++++++++ client/talk-plugin-commentbox/CommentBox.js | 8 ++++---- graph/mutators/comment.js | 4 ++-- 4 files changed, 15 insertions(+), 7 deletions(-) diff --git a/client/coral-admin/src/routes/Moderation/queueConfig.js b/client/coral-admin/src/routes/Moderation/queueConfig.js index 86a622177..b90979efe 100644 --- a/client/coral-admin/src/routes/Moderation/queueConfig.js +++ b/client/coral-admin/src/routes/Moderation/queueConfig.js @@ -13,7 +13,7 @@ export default { }, reported: { action_type: 'FLAG', - statuses: ['NONE', 'PREMOD'], + statuses: ['NONE', 'PREMOD', 'SYSTEM_WITHHELD'], icon: 'flag', name: t('modqueue.reported'), }, diff --git a/client/coral-embed-stream/src/graphql/index.js b/client/coral-embed-stream/src/graphql/index.js index 884a9c86a..dc81b56a4 100644 --- a/client/coral-embed-stream/src/graphql/index.js +++ b/client/coral-embed-stream/src/graphql/index.js @@ -77,6 +77,14 @@ export default { title url } + actions { + __typename + id + ... on FlagAction { + reason + message + } + } tags { tag { name diff --git a/client/talk-plugin-commentbox/CommentBox.js b/client/talk-plugin-commentbox/CommentBox.js index 33b46e245..c85dfb306 100644 --- a/client/talk-plugin-commentbox/CommentBox.js +++ b/client/talk-plugin-commentbox/CommentBox.js @@ -13,10 +13,10 @@ export const name = 'talk-plugin-commentbox'; // Given a newly posted comment's status, show a notification to the user // if needed -export const notifyForNewCommentStatus = (notify, status) => { - if (status === 'REJECTED') { +export const notifyForNewCommentStatus = (notify, comment) => { + if (comment.status === 'REJECTED') { notify('error', t('comment_box.comment_post_banned_word')); - } else if (status === 'PREMOD' || status === 'SYSTEM_WITHHELD') { + } else if (comment.status === 'PREMOD' || comment.status === 'SYSTEM_WITHHELD') { notify('success', t('comment_box.comment_post_notif_premod')); } }; @@ -74,7 +74,7 @@ class CommentBox extends React.Component { // Execute postSubmit Hooks this.state.hooks.postSubmit.forEach((hook) => hook(data)); - notifyForNewCommentStatus(notify, postedComment.status); + notifyForNewCommentStatus(notify, postedComment); if (commentPostedHandler) { commentPostedHandler(); diff --git a/graph/mutators/comment.js b/graph/mutators/comment.js index c79634c99..2aa6f3128 100644 --- a/graph/mutators/comment.js +++ b/graph/mutators/comment.js @@ -282,8 +282,8 @@ const resolveNewCommentStatus = async (context, {asset_id, body, status}, wordli // If the user is not a reliable commenter (passed the unreliability // threshold by having too many rejected comments) then we can change the - // status of the comment to `PREMOD`, therefore pushing the user's comments - // away from the public eye until a moderator can manage them. This of + // status of the comment to `SYSTEM_WITHHELD`, therefore pushing the user's + // comments away from the public eye until a moderator can manage them. This of // course can only be applied if the comment's current status is `NONE`, // we don't want to interfere if the comment was rejected. if (KarmaService.isReliable('comment', user.metadata.trust) === false) { From 313392210055b5a5b4a9685c5374119dd4000cd2 Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Sat, 16 Sep 2017 01:02:19 +0700 Subject: [PATCH 5/9] Fix config parsing bug --- client/coral-embed-stream/src/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/coral-embed-stream/src/index.js b/client/coral-embed-stream/src/index.js index c8d3101ab..841b7b3d0 100644 --- a/client/coral-embed-stream/src/index.js +++ b/client/coral-embed-stream/src/index.js @@ -42,7 +42,7 @@ function preInit({store, pym}) { return new Promise((resolve) => { pym.sendMessage('getConfig'); pym.onMessage('config', (config) => { - store.dispatch(addExternalConfig(config)); + store.dispatch(addExternalConfig(JSON.parse(config))); store.dispatch(checkLogin()); resolve(); }); From 4567b4565f2170e9c60a52b999b61f0b18583d0b Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Sat, 16 Sep 2017 01:19:59 +0700 Subject: [PATCH 6/9] Detect and display stream errors --- client/coral-embed-stream/src/components/Stream.js | 5 ++++- client/coral-embed-stream/src/components/StreamError.js | 5 ++--- client/coral-embed-stream/src/containers/Stream.js | 2 +- locales/en.yml | 1 + locales/es.yml | 1 + 5 files changed, 9 insertions(+), 5 deletions(-) diff --git a/client/coral-embed-stream/src/components/Stream.js b/client/coral-embed-stream/src/components/Stream.js index 43176353f..5a7579567 100644 --- a/client/coral-embed-stream/src/components/Stream.js +++ b/client/coral-embed-stream/src/components/Stream.js @@ -223,8 +223,11 @@ class Stream extends React.Component { const slotQueryData = {root, asset}; if (!highlightedComment && !comments) { + if (highlightedComment === null) { + return {t('stream.comment_not_found')}; + } console.error('Talk: No comments came back from the graph given that query. Please, check the query params.'); - return ; + return {t('common.error')}; } return ( diff --git a/client/coral-embed-stream/src/components/StreamError.js b/client/coral-embed-stream/src/components/StreamError.js index 2e8b8ff96..2c381c720 100644 --- a/client/coral-embed-stream/src/components/StreamError.js +++ b/client/coral-embed-stream/src/components/StreamError.js @@ -1,9 +1,8 @@ import React from 'react'; import styles from './StreamError.css'; -import t from 'coral-framework/services/i18n'; -export const StreamError = () => ( +export const StreamError = ({children}) => (
- {t('common.error')} + {children}
); diff --git a/client/coral-embed-stream/src/containers/Stream.js b/client/coral-embed-stream/src/containers/Stream.js index 273d39589..bb41211bd 100644 --- a/client/coral-embed-stream/src/containers/Stream.js +++ b/client/coral-embed-stream/src/containers/Stream.js @@ -178,7 +178,7 @@ class StreamContainer extends React.Component { render() { if (!this.props.root.asset - || !this.props.root.asset.comment + || this.props.root.asset.comment === undefined && !this.props.root.asset.comments ) { return ; diff --git a/locales/en.yml b/locales/en.yml index 6d94393b2..9e1bf1056 100644 --- a/locales/en.yml +++ b/locales/en.yml @@ -323,6 +323,7 @@ en: user_no_comment: "You've never left a comment. Join the conversation!" stream: temporarily_suspended: "In accordance with {0}'s community guidelines, your account has been temporarily suspended. Please rejoin the conversation {1}." + comment_not_found: "Comment was not found" step_1_header: "Report an issue" step_2_header: "Help us understand" step_3_header: "Thank you for your input" diff --git a/locales/es.yml b/locales/es.yml index db25ab527..83edac80b 100644 --- a/locales/es.yml +++ b/locales/es.yml @@ -349,6 +349,7 @@ es: step_3_header: "Gracias por tu participación" stream: temporarily_suspended: "De acuerdo con la guía de la comunidad de {0}, su cuenta ha sido temporalmente suspendida. Por favor unirse a la conversación {1}." + comment_not_found: "Comentario no encontrado" streams: all: "Todos" article: "Artículo" From aa4f781d25271a19480f7f6237548c0c0b255ef9 Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Sat, 16 Sep 2017 01:27:50 +0700 Subject: [PATCH 7/9] Better error detection --- client/coral-embed-stream/src/components/Stream.js | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/client/coral-embed-stream/src/components/Stream.js b/client/coral-embed-stream/src/components/Stream.js index 5a7579567..af2ec698b 100644 --- a/client/coral-embed-stream/src/components/Stream.js +++ b/client/coral-embed-stream/src/components/Stream.js @@ -201,7 +201,7 @@ class Stream extends React.Component { data, root, appendItemArray, - root: {asset, asset: {comment: highlightedComment, comments}}, + root: {asset, asset: {comment: highlightedComment}}, postComment, notify, updateItem, @@ -222,12 +222,8 @@ class Stream extends React.Component { const slotProps = {data}; const slotQueryData = {root, asset}; - if (!highlightedComment && !comments) { - if (highlightedComment === null) { - return {t('stream.comment_not_found')}; - } - console.error('Talk: No comments came back from the graph given that query. Please, check the query params.'); - return {t('common.error')}; + if (highlightedComment === null) { + return {t('stream.comment_not_found')}; } return ( From f97094177a75b110fd336c02c201d3979be7d019 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Fri, 15 Sep 2017 14:23:18 -0600 Subject: [PATCH 8/9] revamped configuration for moderation phases --- graph/mutators/comment.js | 297 +++++++++++++----- .../server/hooks.js | 38 +-- services/wordlist.js | 2 + test/server/graph/mutations/createComment.js | 5 +- test/server/services/wordlist.js | 2 +- 5 files changed, 230 insertions(+), 114 deletions(-) diff --git a/graph/mutators/comment.js b/graph/mutators/comment.js index 2aa6f3128..f559bebdf 100644 --- a/graph/mutators/comment.js +++ b/graph/mutators/comment.js @@ -7,6 +7,7 @@ const TagsService = require('../../services/tags'); const CommentsService = require('../../services/comments'); const KarmaService = require('../../services/karma'); const tlds = require('tlds'); +const merge = require('lodash/merge'); const linkify = require('linkify-it')() .tlds(tlds); const Wordlist = require('../../services/wordlist'); @@ -156,7 +157,7 @@ const adjustKarma = (Comments, id, status) => async () => { * @param {String} [status='NONE'] the status of the new comment * @return {Promise} resolves to the created comment */ -const createComment = async (context, {tags = [], body, asset_id, parent_id = null, metadata = {}}, status = 'NONE') => { +const createComment = async (context, {tags = [], body, asset_id, parent_id = null, status = 'NONE', metadata = {}}) => { const {user, loaders: {Comments}, pubsub} = context; // Resolve the tags for the comment. @@ -224,6 +225,169 @@ const filterNewComment = async (context, {body, asset_id}) => { ]; }; +const moderationPhases = [ + + // This phase checks to see if the comment is long enough. + (context, comment) => { + + // Check to see if the body is too short, if it is, then complain about it! + if (comment.body.length < 2) { + throw errors.ErrCommentTooShort; + } + }, + + // This phase checks to see if the asset being processed is closed or not. + (context, comment, {asset}) => { + + // Check to see if the asset has closed commenting... + if (asset.isClosed) { + throw new errors.ErrAssetCommentingClosed(asset.closedMessage); + } + }, + + // This phase checks the comment against the wordlist. + (context, comment, {wordlist}) => { + + // Decide the status based on whether or not the current asset/settings + // has pre-mod enabled or not. If the comment was rejected based on the + // wordlist, then reject it, otherwise if the moderation setting is + // premod, set it to `premod`. + if (wordlist.banned) { + + // Add the flag related to Trust to the comment. + return { + status: 'REJECTED', + actions: [{ + action_type: 'FLAG', + user_id: null, + group_id: 'BANNED_WORD', + metadata: {} + }] + }; + } + + // If the comment has a suspect word or a link, we need to add a + // flag to it to indicate that it needs to be looked at. + // Otherwise just return the new comment. + + // If the wordlist has matched the suspect word filter and we haven't disabled + // auto-flagging suspect words, then we should flag the comment! + if (wordlist.suspect && !DISABLE_AUTOFLAG_SUSPECT_WORDS) { + + // TODO: this is kind of fragile, we should refactor this to resolve + // all these const's that we're using like 'COMMENTS', 'FLAG' to be + // defined in a checkable schema. + return { + actions: [{ + action_type: 'FLAG', + user_id: null, + group_id: 'Matched suspect word filter', + metadata: {} + }], + }; + } + }, + + // This phase checks to see if the comment's length exeeds maximum. + (context, comment, {assetSettings: {charCountEnable, charCount}}) => { + + // Reject if the comment is too long + if (charCountEnable && comment.body.length > charCount) { + + // Add the flag related to Trust to the comment. + return { + status: 'REJECTED', + actions: [{ + action_type: 'FLAG', + user_id: null, + group_id: 'BODY_COUNT', + metadata: { + count: comment.body.length, + } + }] + }; + } + }, + + // This phase checks the comment if it has any links in it if the check is + // enabled. + (context, comment, {assetSettings: {premodLinksEnable}}) => { + if (premodLinksEnable && linkify.test(comment.body)) { + + // Add the flag related to Trust to the comment. + return { + status:'SYSTEM_WITHHELD', + actions: [{ + action_type: 'FLAG', + user_id: null, + group_id: 'LINKS', + metadata: { + links: comment.body, + } + }], + }; + } + }, + + // This phase checks to see if the user making the comment is allowed to do so + // considering their reliability (Trust) status. + (context) => { + if (context.user && context.user.metadata) { + + // If the user is not a reliable commenter (passed the unreliability + // threshold by having too many rejected comments) then we can change the + // status of the comment to `SYSTEM_WITHHELD`, therefore pushing the user's + // comments away from the public eye until a moderator can manage them. This of + // course can only be applied if the comment's current status is `NONE`, + // we don't want to interfere if the comment was rejected. + if (KarmaService.isReliable('comment', context.user.metadata.trust) === false) { + + // Add the flag related to Trust to the comment. + return { + status: 'SYSTEM_WITHHELD', + actions: [{ + action_type: 'FLAG', + user_id: null, + group_id: 'TRUST', + metadata: { + trust: context.user.metadata.trust, + } + }], + }; + } + } + }, + + // This phase checks to see if the comment was already perscribed a status. + (context, comment) => { + + // If the status was already defined, don't redefine it. It's only defined + // when specific external conditions exist, we don't want to override that. + if (comment.status && comment.status.length > 0) { + return { + status: comment.status, + }; + } + }, + + // This phase checks to see if the settings have premod enabled, if they do, + // the comment is premod, otherwise, it's just none. + (context, comment, {assetSettings: {moderation}}) => { + + // If the settings say that we're in premod mode, then the comment is in + // premod status. + if (moderation === 'PRE') { + return { + status: 'PREMOD', + }; + } + + return { + status: 'NONE', + }; + } +]; + /** * This resolves a given comment's status to take into account moderator actions * are applied. @@ -233,68 +397,44 @@ const filterNewComment = async (context, {body, asset_id}) => { * @param {Object} [wordlist={}] the results of the wordlist scan * @return {Promise} resolves to the comment's status */ -const resolveNewCommentStatus = async (context, {asset_id, body, status}, wordlist = {}, settings = {}) => { - let {user} = context; +const resolveCommentModeration = async (context, comment) => { - // Check to see if the body is too short, if it is, then complain about it! - if (body.length < 2) { - throw errors.ErrCommentTooShort; - } + // First we filter the comment contents to ensure that we note any validation + // issues. + let [wordlist, settings] = await filterNewComment(context, comment); - // If the status was already defined, don't redefine it. It's only defined - // when specific external conditions exist, we don't want to override that. - if (status && status.length > 0) { - return status; - } - - // Decide the status based on whether or not the current asset/settings - // has pre-mod enabled or not. If the comment was rejected based on the - // wordlist, then reject it, otherwise if the moderation setting is - // premod, set it to `premod`. - if (wordlist.banned) { - return 'REJECTED'; - } - - if (settings.premodLinksEnable && linkify.test(body)) { - return 'SYSTEM_WITHHELD'; - } - - let asset = await AssetsService.findById(asset_id); + // Get the asset from the loader. + const asset = await context.loaders.Assets.getByID.load(comment.asset_id); if (!asset) { + + // And leave now if this asset wasn't found. throw errors.ErrNotFound; } - // Check to see if the asset has closed commenting... - if (asset.isClosed) { - throw new errors.ErrAssetCommentingClosed(asset.closedMessage); - } + // Combine the asset and the settings to get the asset settings. + const assetSettings = await AssetsService.rectifySettings(asset, settings); - // Return `premod` if pre-moderation is enabled and an empty "new" status - // in the event that it is not in pre-moderation mode. - let {moderation, charCountEnable, charCount} = await AssetsService.rectifySettings(asset, settings); + // Loop over all the moderation phases and see if we've resolved the status. + for (const phase of moderationPhases) { + const result = await phase(context, comment, { + asset, + assetSettings, + settings, + wordlist, + }); - // Reject if the comment is too long - if (charCountEnable && body.length > charCount) { - return 'REJECTED'; - } + if (result) { - if (user && user.metadata) { + // Merge the comment and the result together. + comment = merge(comment, result); - // If the user is not a reliable commenter (passed the unreliability - // threshold by having too many rejected comments) then we can change the - // status of the comment to `SYSTEM_WITHHELD`, therefore pushing the user's - // comments away from the public eye until a moderator can manage them. This of - // course can only be applied if the comment's current status is `NONE`, - // we don't want to interfere if the comment was rejected. - if (KarmaService.isReliable('comment', user.metadata.trust) === false) { - - // Update the response from the comment creation to add the PREMOD so that - // that user's UI will reflect the fact that their comment is in pre-mod. - return 'SYSTEM_WITHHELD'; + // If this result contained a status, then we've finished resolving + // phases! + if (result.status) { + return comment.actions; + } } } - - return moderation === 'PRE' ? 'PREMOD' : 'NONE'; }; /** @@ -305,47 +445,31 @@ const resolveNewCommentStatus = async (context, {asset_id, body, status}, wordli * @param {Object} commentInput the new comment to be created * @return {Promise} resolves to a new comment */ -const createPublicComment = async (context, commentInput) => { - - // First we filter the comment contents to ensure that we note any validation - // issues. - let [wordlist, settings] = await filterNewComment(context, commentInput); +const createPublicComment = async (context, comment) => { // We then take the wordlist and the comment into consideration when // considering what status to assign the new comment, and resolve the new // status to set the comment to. - let status = await resolveNewCommentStatus(context, commentInput, wordlist, settings); + let actions = await resolveCommentModeration(context, comment); // Then we actually create the comment with the new status. - let comment = await createComment(context, commentInput, status); + comment = await createComment(context, comment); - // If the comment has a suspect word or a link, we need to add a - // flag to it to indicate that it needs to be looked at. - // Otherwise just return the new comment. - - // TODO: Check why the wordlist is undefined - - // If the wordlist has matched the suspect word filter and we haven't disabled - // auto-flagging suspect words, then we should flag the comment! - if (wordlist != null && wordlist.suspect != null && !DISABLE_AUTOFLAG_SUSPECT_WORDS) { - - // TODO: this is kind of fragile, we should refactor this to resolve - // all these const's that we're using like 'COMMENTS', 'FLAG' to be - // defined in a checkable schema. - await ActionsService.create({ - item_id: comment.id, - item_type: 'COMMENTS', - action_type: 'FLAG', - user_id: null, - group_id: 'Matched suspect word filter', - metadata: {} - }); - } + // Create all the actions that were determined during the moderation check + // phase. + await createActions(comment.id, actions); // Finally, we return the comment. return comment; }; +// createActions will for each of the provided actions, create the given action +// on the comment at the same time using Promise.all. +const createActions = async (item_id, actions = []) => Promise.all(actions.map((action) => merge(action, { + item_id, + item_type: 'COMMENTS', +})).map((action) => ActionsService.create(action))); + /** * Sets the status of a comment * @param {Object} context graphql context @@ -382,14 +506,19 @@ const setStatus = async ({user, loaders: {Comments}}, {id, status}) => { */ const edit = async (context, {id, asset_id, edit: {body}}) => { - // Get the wordlist and the settings object. - const [wordlist, settings] = await filterNewComment(context, {asset_id, body}); + // Build up the new comment we're setting. We need to check this with + // moderation now. + let comment = {id, asset_id, body}; // Determine the new status of the comment. - const status = await resolveNewCommentStatus(context, {asset_id, body}, wordlist, settings); + const actions = await resolveCommentModeration(context, comment); // Execute the edit. - const comment = await CommentsService.edit({id, author_id: context.user.id, body, status}); + comment = await CommentsService.edit({id, author_id: context.user.id, body, status: comment.status}); + + // Create all the actions that were determined during the moderation check + // phase. + await createActions(comment.id, actions); // Publish the edited comment via the subscription. context.pubsub.publish('commentEdited', comment); diff --git a/plugins/talk-plugin-toxic-comments/server/hooks.js b/plugins/talk-plugin-toxic-comments/server/hooks.js index 931ca0f60..29ab4fe54 100644 --- a/plugins/talk-plugin-toxic-comments/server/hooks.js +++ b/plugins/talk-plugin-toxic-comments/server/hooks.js @@ -1,6 +1,5 @@ const {getScores, isToxic} = require('./perspective'); const {ErrToxic} = require('./errors'); -const ActionsService = require('../../../services/actions'); // We don't add the hooks during _test_ as the perspective API is not available. if (process.env.NODE_ENV === 'test') { @@ -12,53 +11,36 @@ module.exports = { createComment: { async pre(_, {input}, _context, _info) { - let scores; - // Try getting scores. + let scores; try { scores = await getScores(input.body); - } - catch(err) { + } catch(err) { // Warn and let mutation pass. console.trace(err); return; } - const commentIsToxic = isToxic(scores); - - if (input.checkToxicity && commentIsToxic) { - throw ErrToxic; - } - - // attach scores to metadata. + // Attach scores to metadata. input.metadata = Object.assign({}, input.metadata, { perspective: scores, }); - if (commentIsToxic) { + if (isToxic(scores)) { + if (input.checkToxicity) { + throw ErrToxic; + } + input.status = 'SYSTEM_WITHHELD'; - } - }, - async post(_, _input, _context, _info, result) { - const metadata = result.comment.metadata; - if (metadata.perspective && isToxic(metadata.perspective)) { - - // TODO: this is kind of fragile, we should refactor this to resolve - // all these const's that we're using like 'COMMENTS', 'FLAG' to be - // defined in a checkable schema. - - // Add a flag to the comment. - await ActionsService.create({ - item_id: result.comment.id, - item_type: 'COMMENTS', + input.actions = input.actions && input.actions.length >= 0 ? input.actions : []; + input.actions.push({ action_type: 'FLAG', user_id: null, group_id: 'Comment contains toxic language', metadata: {} }); } - return result; }, }, }, diff --git a/services/wordlist.js b/services/wordlist.js index e3aad5789..8d9ee95bb 100644 --- a/services/wordlist.js +++ b/services/wordlist.js @@ -118,6 +118,8 @@ class Wordlist { // word (suspect). return errors; } + + return errors; } /** diff --git a/test/server/graph/mutations/createComment.js b/test/server/graph/mutations/createComment.js index 72c163d35..9583c2066 100644 --- a/test/server/graph/mutations/createComment.js +++ b/test/server/graph/mutations/createComment.js @@ -167,7 +167,7 @@ describe('graph.mutations.createComment', () => { [ {message: 'comment does not contain banned/suspect words', body: 'This is such a nice comment!', status: 'NONE', flagged: false}, - {message: 'comment contains banned words', body: 'This is the WORST comment!', status: 'REJECTED', flagged: false}, + {message: 'comment contains banned words', body: 'This is the WORST comment!', status: 'REJECTED', flagged: true}, {message: 'comment contains suspect words', body: 'This is the EH comment!', status: 'NONE', flagged: true} ].forEach(({message, body, status, flagged}) => { describe(message, () => { @@ -222,6 +222,9 @@ describe('graph.mutations.createComment', () => { return graphql(schema, query, {}, context) .then(({data, errors}) => { + if (errors) { + console.error(errors); + } expect(errors).to.be.undefined; expect(data.createComment).to.have.property('comment').not.null; expect(data.createComment).to.have.property('errors').null; diff --git a/test/server/services/wordlist.js b/test/server/services/wordlist.js index b43dd2bec..5a2422b89 100644 --- a/test/server/services/wordlist.js +++ b/test/server/services/wordlist.js @@ -89,7 +89,7 @@ describe('services.Wordlist', () => { 'I have bad $ hit lling', 'That\'s a p***ch!', ].forEach((word) => { - expect(wordlist.scan('body', word)).to.be.undefined; + expect(wordlist.scan('body', word)).to.be.deep.equal({}); }); }); From 384c126d9e9e0d0b1c80aedbc7448d3ce13dfb4f Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Fri, 15 Sep 2017 15:29:20 -0600 Subject: [PATCH 9/9] added docs --- graph/mutators/comment.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/graph/mutators/comment.js b/graph/mutators/comment.js index f559bebdf..75cad99b7 100644 --- a/graph/mutators/comment.js +++ b/graph/mutators/comment.js @@ -225,6 +225,10 @@ const filterNewComment = async (context, {body, asset_id}) => { ]; }; +/** + * moderationPhases is an array of phases carried out in order until a status is + * returned. + */ const moderationPhases = [ // This phase checks to see if the comment is long enough.