From 96cd5197e3b5b0804ccfc28d27edca6c332b3377 Mon Sep 17 00:00:00 2001
From: gaba <gabelula@gmail.com>
Date: Mon, 8 May 2017 16:05:40 -0700
Subject: [PATCH] Test was failing as I was not returning an error.

---
 errors.js                                 |  4 ++--
 models/comment.js                         |  9 +++++++
 services/comments.js                      | 29 +++++++++++++++++------
 test/server/graph/mutations/ignoreUser.js |  1 -
 test/server/services/comments.js          | 11 +++++----
 5 files changed, 40 insertions(+), 14 deletions(-)

diff --git a/errors.js b/errors.js
index 9f12c7835..5c2b05f92 100644
--- a/errors.js
+++ b/errors.js
@@ -82,7 +82,7 @@ const ErrNoCommentFound = new APIError('comment does not exist', {
 });
 
 // ErrNoCommentFound is returned when trying to add a tag to a comment that does not exist.
-const ErrorTagNotAllowed = new APIError('tag not allowed', {
+const ErrTagNotAllowed = new APIError('tag not allowed', {
   translation_key: 'TAG_NOT_ALLOWED',
   status: 400
 });
@@ -174,7 +174,7 @@ module.exports = {
   ErrMissingPassword,
   ErrMissingToken,
   ErrNoCommentFound,
-  ErrorTagNotAllowed,
+  ErrTagNotAllowed,
   ErrEmailTaken,
   ErrSpecialChars,
   ErrMissingUsername,
diff --git a/models/comment.js b/models/comment.js
index f3e239080..219ec055d 100644
--- a/models/comment.js
+++ b/models/comment.js
@@ -92,6 +92,15 @@ const CommentSchema = new Schema({
   }
 });
 
+// Add the indexes on the comment tag.
+CommentSchema.index({
+  'id': 1,
+  'tags.id': 1
+}, {
+  unique: true,
+  background: false
+});
+
 // Comment model.
 const Comment = mongoose.model('Comment', CommentSchema);
 
diff --git a/services/comments.js b/services/comments.js
index db4dd18c1..e1873ba02 100644
--- a/services/comments.js
+++ b/services/comments.js
@@ -76,21 +76,26 @@ module.exports = class CommentsService {
           });
         }
         else if (!ALLOWED_TAGS.includes(name) || settings.tags.findIndex((t) => {return t.id === name & t.models.include('COMMENTS');}) === -1) {
-          return Promise.reject(errors.ErrorTagNotAllowed);
+          return Promise.reject(errors.ErrTagNotAllowed);
         }
       });
 
-      return CommentModel.findOneAndUpdate({id}, {
+      return CommentModel.findOneAndUpdate({id, 'tags.id': {$ne: name}}, {
         $push: {
           tags: {
             id: name,
             added_by: added_by
           }
         },
-      },
-        {
-          new: false,
-          upsert: false
+      })
+        .then(({nModified}) => {
+          switch (nModified) {
+          case 0:
+            return Promise.reject(errors.ErrNoCommentFound);
+          case 1:
+            return;
+          default:
+          }
         });
     });
   }
@@ -102,12 +107,22 @@ module.exports = class CommentsService {
    * @param {String} tag_id the id of the tag to remove
    */
   static removeTag(id, tag_id) {
-    return CommentModel.findOneAndUpdate({id}, {
+    return CommentModel.findOneAndUpdate({id, 'tags.id': tag_id}, {
       $pull: {
         tags: {
           id: tag_id
         }
       }
+    }
+    )
+    .then(({nModified}) => {
+      switch(nModified) {
+      case 0:
+        return Promise.reject(errors.ErrNoCommentFound);
+      case 1:
+        return;
+      default:
+      }
     });
   }
 
diff --git a/test/server/graph/mutations/ignoreUser.js b/test/server/graph/mutations/ignoreUser.js
index 8de54a41d..54d6305a1 100644
--- a/test/server/graph/mutations/ignoreUser.js
+++ b/test/server/graph/mutations/ignoreUser.js
@@ -32,7 +32,6 @@ describe('graph.mutations.ignoreUser', () => {
 
   // @TODO (bengo) - test a user can't ignore themselves
   it('users can ignoreUser', async () => {
-    UsersService.findLocalUser('usernameB@example.com');
     const user = await UsersService.createLocalUser('usernameA@example.com', 'password', 'usernameA');
     const userToIgnore = await UsersService.createLocalUser('usernameB@example.com', 'password', 'usernameB');
     const context = new Context({user});
diff --git a/test/server/services/comments.js b/test/server/services/comments.js
index e01fc7bad..27657efa5 100644
--- a/test/server/services/comments.js
+++ b/test/server/services/comments.js
@@ -236,7 +236,10 @@ describe('services.CommentsService', () => {
       const tagName = 'BEST';
       const userId = users[0].id;
 
-      await expect(CommentsService.addTag(commentId, tagName, userId, 'PUBLIC')).to.be.rejected;
+      CommentsService.addTag(commentId, tagName, userId)
+      .catch((error) => {
+        expect(error).to.not.be.null;
+      });
     });
     it('can\'t add same tag.id twice', async () => {
       const commentId = comments[0].id;
@@ -244,10 +247,10 @@ describe('services.CommentsService', () => {
       const userId = users[0].id;
 
       // first time
-      await CommentsService.addTag(commentId, tagName, userId, 'PUBLIC');
+      await CommentsService.addTag(commentId, tagName, userId);
 
       // second time should fail
-      await expect(CommentsService.addTag(commentId, tagName, userId, 'PUBLIC')).to.be.rejected;
+      await expect(CommentsService.addTag(commentId, tagName, userId)).to.be.rejected;
     });
   });
 
@@ -255,7 +258,7 @@ describe('services.CommentsService', () => {
     it('removes a tag', async () => {
       const commentId = comments[0].id;
       const tagName = 'BEST';
-      await CommentsService.addTag(commentId, tagName, users[0].id, 'PUBLIC');
+      await CommentsService.addTag(commentId, tagName, users[0].id);
       const {tags} = await CommentsService.findById(commentId);
       expect(tags.length).to.equal(1);