diff --git a/client/coral-embed-stream/src/components/Comment.js b/client/coral-embed-stream/src/components/Comment.js index b80a62b28..000e0caad 100644 --- a/client/coral-embed-stream/src/components/Comment.js +++ b/client/coral-embed-stream/src/components/Comment.js @@ -116,7 +116,7 @@ class Comment extends React.Component { // dispatch action to ignore another user ignoreUser: React.PropTypes.func, - // edit a comment, passed (id, { body }) + // edit a comment, passed (id, asset_id, { body }) editComment: React.PropTypes.func, } @@ -280,7 +280,7 @@ class Comment extends React.Component { { this.state.isEditing ? { return { - editComment: (id, edit) => { + editComment: (id, asset_id, edit) => { return mutate({ variables: { id, + asset_id, edit, }, refetchQueries: [ diff --git a/graph/mutators/comment.js b/graph/mutators/comment.js index 283d66738..17c75a94b 100644 --- a/graph/mutators/comment.js +++ b/graph/mutators/comment.js @@ -233,19 +233,8 @@ const removeCommentTag = ({user, loaders: {Comments}}, {id, tag}) => { * @param {Object} edit describes how to edit the comment * @param {String} edit.body the new Comment body */ -const editComment = async ({user, loaders: {Comments}}, {id, edit}) => { +const editComment = async ({user, loaders: {Comments}}, {id, asset_id, edit}) => { const {body} = edit; - const comment = await CommentsService.findById(id); - if ( ! comment) { - throw new errors.APIError('Comment not found', { - status: 404, - translation_key: 'NOT_FOUND', - }); - } - if ( ! (user && user.id && (user.id === comment.author_id))) { - throw errors.ErrNotAuthorized; - } - const {asset_id} = comment; const determineStatusForComment = async ({body, asset_id}) => { const [wordlist, settings] = await filterNewComment({asset_id, body}); const status = await resolveNewCommentStatus({asset_id, body}, wordlist, settings); @@ -253,14 +242,21 @@ const editComment = async ({user, loaders: {Comments}}, {id, edit}) => { }; const status = await determineStatusForComment({body, asset_id}); try { - await CommentsService.edit(comment, Object.assign({status}, edit)); + await CommentsService.edit(id, asset_id, user.id, Object.assign({status}, edit)); } catch (error) { switch (error.name) { + case 'CommentNotFound': + throw new errors.APIError('Comment not found', { + status: 404, + translation_key: 'NOT_FOUND', + }); case 'EditWindowExpired': throw new errors.APIError('You can no longer edit this comment. The window to do so has expired.', { status: 401, translation_key: 'error.editWindowExpired', }); + case 'NotAuthorizedToEdit': + throw errors.ErrNotAuthorized; default: throw error; } diff --git a/graph/typeDefs.graphql b/graph/typeDefs.graphql index a16f4bc1c..26e7349ec 100644 --- a/graph/typeDefs.graphql +++ b/graph/typeDefs.graphql @@ -762,7 +762,7 @@ type RootMutation { deleteAction(id: ID!): DeleteActionResponse # Edit a comment - editComment(id: ID!, edit: EditCommentInput): EditCommentResponse + editComment(id: ID!, asset_id: ID!, edit: EditCommentInput): EditCommentResponse # Sets User status. Requires the `ADMIN` role. setUserStatus(id: ID!, status: USER_STATUS!): SetUserStatusResponse diff --git a/services/comments.js b/services/comments.js index 28216381f..6e3d79edd 100644 --- a/services/comments.js +++ b/services/comments.js @@ -51,50 +51,76 @@ module.exports = class CommentsService { /** * Edit a Comment - * @param {CommentModel|String} comment you want to edit (or its ID) - * @param {String} body the new Comment body - * @param {String} status the new Comment status + * @param {String} id comment.id you want to edit (or its ID) + * @param {String} asset_id asset_id of the comment + * @param {String} editor user.id of the user trying to edit the comment (will err if not comment author) + * @param {String} body the new Comment body + * @param {String} status the new Comment status */ - static async edit(comment, {body, status, ignoreEditWindow}) { - if (typeof comment === 'string') { - - // it's an id - comment = await this.findById(comment); - } - const editWindowExpired = (comment) => { - const now = new Date; - const editableUntil = this.getEditableUntilDate(comment); - return now > editableUntil; - }; - if (( ! ignoreEditWindow) && editWindowExpired(comment)) { - throw Object.assign(new Error('Edit window is over.'), { - name: 'EditWindowExpired' - }); - } + static async edit(id, asset_id, editor, {body, status, ignoreEditWindow}) { if (status && ! STATUSES.includes(status)) { throw new Error(`status ${status} is not supported`); } - - const {id} = comment; - const {nModified} = await CommentModel.update({id}, { - $set: { - body, - status, + const lastEditableCommentCreatedAt = new Date((new Date()).getTime() - EDIT_WINDOW_MS); + const filter = Object.assign( + { + id, + asset_id, + author_id: editor, }, - $push: { - body_history: { - body, - created_at: new Date(), + ignoreEditWindow ? {} : { + created_at: { + $gt: lastEditableCommentCreatedAt, }, - status_history: { - type: status, - created_at: new Date(), - } - }, - }); + } + ); + const {nModified} = await CommentModel.update( + filter, + { + $set: { + body, + status, + }, + $push: { + body_history: { + body, + created_at: new Date(), + }, + status_history: { + type: status, + created_at: new Date(), + } + }, + } + ); switch (nModified) { - case 0: - throw new Error(`Couldn't edit comment. There is no Comment with id "${id}"`); + case 0: { + + // disambiguate possible error cases + const comment = await this.findById(id); + + // return whether the comment should no longer be editable + // because its edit window expired + const editWindowExpired = (comment) => { + const now = new Date; + const editableUntil = this.getEditableUntilDate(comment); + return now > editableUntil; + }; + if ( ! comment || (comment.asset_id !== asset_id)) { + throw Object.assign(new Error('Comment not found'), { + name: 'CommentNotFound' + }); + } else if (comment.author_id !== editor) { + throw Object.assign(new Error('You aren\'t allowed to edit that comment'), { + name: 'NotAuthorizedToEdit' + }); + } else if (( ! ignoreEditWindow) && editWindowExpired(comment)) { + throw Object.assign(new Error('Edit window is over.'), { + name: 'EditWindowExpired' + }); + } + throw new Error('Failed to edit comment. This could be because it can\'t be found, the edit window expired, or because you\'re not allowed to edit it.'); + } } } diff --git a/test/server/graph/mutations/editComment.js b/test/server/graph/mutations/editComment.js index 1375157ad..f449e37a8 100644 --- a/test/server/graph/mutations/editComment.js +++ b/test/server/graph/mutations/editComment.js @@ -27,8 +27,8 @@ describe('graph.mutations.editComment', () => { }); const editCommentMutation = ` - mutation EditComment ($id: ID!, $edit: EditCommentInput) { - editComment(id:$id, edit:$edit) { + mutation EditComment ($id: ID!, $asset_id: ID!, $edit: EditCommentInput) { + editComment(id:$id, asset_id:$asset_id, edit:$edit) { errors { translation_key } @@ -55,6 +55,7 @@ describe('graph.mutations.editComment', () => { const newBody = 'I have been edited.'; const response = await graphql(schema, editCommentMutation, {}, context, { id: comment.id, + asset_id: asset.id, edit: { body: newBody } @@ -91,6 +92,7 @@ describe('graph.mutations.editComment', () => { const context = new Context({user}); const response = await graphql(schema, editCommentMutation, {}, context, { id: comment.id, + asset_id: asset.id, edit: { body: newBody } @@ -117,11 +119,13 @@ describe('graph.mutations.editComment', () => { const context = new Context({user: userB}); const response = await graphql(schema, editCommentMutation, {}, context, { id: comment.id, + asset_id: asset.id, edit: { body: newBody } }); expect(response.errors).to.be.empty; + expect(response.data.editComment.errors).to.not.be.empty; expect(response.data.editComment.errors[0].translation_key).to.equal('NOT_AUTHORIZED'); const commentAfterEdit = await CommentsService.findById(comment.id); @@ -135,6 +139,7 @@ describe('graph.mutations.editComment', () => { const context = new Context({user}); const response = await graphql(schema, editCommentMutation, {}, context, { id: fakeCommentId, + asset_id: asset.id, edit: { body: newBody } @@ -233,6 +238,7 @@ describe('graph.mutations.editComment', () => { const newBody = edit.body; const response = await graphql(schema, editCommentMutation, {}, context, { id: comment.id, + asset_id: asset.id, edit: { body: newBody }