fixes to tests and impl edit hooks

services/comments.js

@@ -1,6 +1,6 @@
 const CommentModel = require('../models/comment');
 
-const ActionModel = require('../models/action');
+const debug = require('debug')('talk:services:comments');
 const ActionsService = require('./actions');
 const SettingsService = require('./settings');
 
@@ -63,7 +63,7 @@ module.exports = class CommentsService {
       id,
       author_id,
       status: {
-        $in: ['NONE', 'PREMOD'],
+        $in: ['NONE', 'PREMOD', 'ACCEPTED'],
       },
     };
 
@@ -100,21 +100,25 @@ module.exports = class CommentsService {
       // Try to get the comment.
       const comment = await CommentsService.findById(id);
       if (comment === null) {
+        debug('rejecting comment edit because comment was not found');
         throw errors.ErrNotFound;
       }
 
       // Check to see if the user was't allowed to edit it.
       if (comment.author_id !== author_id) {
+        debug('rejecting comment edit because author id does not match editing user');
         throw errors.ErrNotAuthorized;
       }
 
       // Check to see if the comment had a status that was editable.
-      if (!['NONE', 'PREMOD'].includes(comment.status)) {
+      if (!['NONE', 'PREMOD', 'ACCEPTED'].includes(comment.status)) {
+        debug('rejecting comment edit because original comment has a non-editable status');
         throw errors.ErrNotAuthorized;
       }
 
       // Check to see if the edit window expired.
       if (!ignoreEditWindow && comment.created_at <= lastEditableCommentCreatedAt) {
+        debug('rejecting comment edit because outside edit time window');
         throw errors.ErrEditWindowHasEnded;
       }
 
@@ -351,7 +355,7 @@ const incrReplyCount = async (comment, value) => {
 events.on(COMMENTS_NEW, async (comment) => {
   if (
     !comment || // Check that the comment is defined.
-    (comment.parent_id === null || comment.parent_id.length === 0) || // Check that the comment has a parent (is a reply).
+    (!comment.parent_id || comment.parent_id.length === 0) || // Check that the comment has a parent (is a reply).
     !(comment.status === 'NONE' || comment.status === 'APPROVED') // Check that the comment is visible.
   ) {
     return;
@@ -365,7 +369,7 @@ events.on(COMMENTS_NEW, async (comment) => {
 events.on(COMMENTS_EDIT, async (originalComment, editedComment) => {
   if (
     !editedComment || // Check that the comment is defined.
-    (editedComment.parent_id === null || editedComment.parent_id.length === 0) // Check that the comment has a parent (is a reply).
+    (!editedComment.parent_id || editedComment.parent_id.length === 0) // Check that the comment has a parent (is a reply).
   ) {
     return;
   }

test/server/graph/mutations/editComment.js

@@ -11,20 +11,12 @@ const CommentsService = require('../../../../services/comments');
 const {expect} = require('chai');
 
 describe('graph.mutations.editComment', () => {
-  let asset;
-  let user;
-  let settings;
+  let asset, user;
   beforeEach(async () => {
     timekeeper.reset();
-    settings = await SettingsService.init();
+    await SettingsService.init();
     asset = await AssetModel.create({});
-    user = await UsersService.createLocalUser(
-      'usernameA@example.com', 'password', 'usernameA');
-  });
-  afterEach(async () => {
-    await asset.remove();
-    await user.remove();
-    await settings.remove();
+    user = await UsersService.createLocalUser('usernameA@example.com', 'password', 'usernameA');
   });
 
   const editCommentMutation = `
@@ -120,8 +112,7 @@ describe('graph.mutations.editComment', () => {
       body: `hello there! ${String(Math.random()).slice(2)}`,
     });
 
-    const userB = await UsersService.createLocalUser(
-      'usernameB@example.com', 'password', 'usernameB');
+    const userB = await UsersService.createLocalUser('usernameB@example.com', 'password', 'usernameB');
     const newBody = 'This body should never be set';
     const context = new Context({user: userB});
     const response = await graphql(schema, editCommentMutation, {}, context, {
@@ -161,7 +152,7 @@ describe('graph.mutations.editComment', () => {
   const bannedWord = 'BANNED_WORD';
   [
     {
-      description: 'premod: editing a REJECTED comment sets back to PREMOD',
+      description: 'premod: editing a REJECTED comment is rejected',
       settings: {
         moderation: 'PRE',
       },
@@ -172,9 +163,7 @@ describe('graph.mutations.editComment', () => {
       edit: {
         body: 'I have been edited to be less offensive',
       },
-      afterEdit: {
-        status: 'PREMOD',
-      },
+      error: true
     },
     {
       description: 'editing an ACCEPTED comment to add a bad word sets status to REJECTED',
@@ -196,7 +185,7 @@ describe('graph.mutations.editComment', () => {
       },
     },
     {
-      description: 'postmod: editing a REJECTED comment with banned word to remove banned word sets status to NONE',
+      description: 'postmod: editing a REJECTED comment with banned word be rejected',
       settings: {
         moderation: 'POST',
         wordlist: {
@@ -210,9 +199,7 @@ describe('graph.mutations.editComment', () => {
       edit: {
         body: 'I have been edited to remove the bad word'
       },
-      afterEdit: {
-        status: 'NONE',
-      },
+      error: true
     },
     {
       description: 'postmod + premodLinksEnable: editing an ACCEPTED comment to add a link sets status to PREMOD',
@@ -231,9 +218,8 @@ describe('graph.mutations.editComment', () => {
         status: 'PREMOD',
       },
     },
-  ].forEach(({description, settings, beforeEdit, edit, afterEdit, only}) => {
-    const test = only ? it.only : it;
-    test(description, async () => {
+  ].forEach(({description, settings, beforeEdit, edit, afterEdit, error}) => {
+    it(description, async () => {
       await SettingsService.update(settings);
       const context = new Context({user});
       const comment = await CommentsService.publicCreate(Object.assign(
@@ -253,11 +239,18 @@ describe('graph.mutations.editComment', () => {
           body: newBody
         }
       });
-      if (response.errors && response.errors.length) {console.error(response.errors);}
-      expect(response.errors).to.be.empty;
-      const commentAfterEdit = await CommentsService.findById(comment.id);
-      expect(commentAfterEdit.body).to.equal(newBody);
-      expect(commentAfterEdit.status).to.equal(afterEdit.status);
+
+      if (error) {
+        expect(response.data.editComment.errors).to.not.be.empty;
+      } else {
+        if (response.data.editComment.errors && response.data.editComment.errors.length) {
+          console.error(response.data.editComment.errors);
+        }
+        expect(response.data.editComment.errors).to.be.null;
+        const commentAfterEdit = await CommentsService.findById(comment.id);
+        expect(commentAfterEdit.body).to.equal(newBody);
+        expect(commentAfterEdit.status).to.equal(afterEdit.status);
+      }
     });
   });
 });

test/server/services/comments.js

@@ -201,7 +201,7 @@ describe('services.CommentsService', () => {
       expect(c.status).to.be.equal('NONE');
 
       const spy = sinon.spy();
-      events.once(COMMENTS_EDIT, () => spy());
+      events.once(COMMENTS_EDIT, spy);
 
       let c2 = await CommentsService.pushStatus(comment_id, 'REJECTED', '123');
       expect(c2).to.have.property('status');