From bd72bfc6ecd5a5f5b888aff7b97f8f069fada01e Mon Sep 17 00:00:00 2001 From: gaba Date: Thu, 20 Apr 2017 12:24:08 -0700 Subject: [PATCH] Fix tests related to tags on comments. --- graph/loaders/index.js | 2 + graph/loaders/tags.js | 37 ++++++++++++ graph/mutators/comment.js | 14 ++--- models/tag.js | 10 +++- services/comments.js | 27 ++++++--- services/tags.js | 59 +++++++++---------- test/server/graph/mutations/addCommentTag.js | 13 ++-- test/server/graph/mutations/createComment.js | 14 +++-- .../graph/mutations/removeCommentTag.js | 16 +++-- test/server/services/comments.js | 28 +++++---- 10 files changed, 139 insertions(+), 81 deletions(-) create mode 100644 graph/loaders/tags.js diff --git a/graph/loaders/index.js b/graph/loaders/index.js index 26727940f..8bc170c6d 100644 --- a/graph/loaders/index.js +++ b/graph/loaders/index.js @@ -6,6 +6,7 @@ const Assets = require('./assets'); const Comments = require('./comments'); const Metrics = require('./metrics'); const Settings = require('./settings'); +const Tags = require('./tags'); const Users = require('./users'); const plugins = require('../../services/plugins'); @@ -18,6 +19,7 @@ let loaders = [ Comments, Metrics, Settings, + Tags, Users, // Load the plugin loaders from the manager. diff --git a/graph/loaders/tags.js b/graph/loaders/tags.js new file mode 100644 index 000000000..b76bc36bd --- /dev/null +++ b/graph/loaders/tags.js @@ -0,0 +1,37 @@ +const DataLoader = require('dataloader'); + +const util = require('./util'); + +const TagsService = require('../../services/tags'); +const TagModel = require('../../models/tag'); + +/** + * Gets tags based on their item id's. + */ +const genTagsByItemID = (_, item_ids) => { + return TagsService + .findByItemIdArray(item_ids) + .then(util.arrayJoinBy(item_ids, 'item_id')); +}; + +/** + * Search for tags based on their item_type and ensures that + * the tags returned have unique item id's. + * @param {String} item_type the item id to search by + * @return {Promise} resolves to distinct items tags + */ +const getItemIdsByItemType = (_, item_type) => { + return TagModel.distinct('item_id', {item_type}); +}; + +/** + * Creates a set of loaders based on a GraphQL context. + * @param {Object} context the context of the GraphQL request + * @return {Object} object of loaders + */ +module.exports = (context) => ({ + Tags: { + getByID: new DataLoader((ids) => genTagsByItemID(context, ids)), + getByTypes: ({item_type}) => getItemIdsByItemType(context, item_type) + } +}); diff --git a/graph/mutators/comment.js b/graph/mutators/comment.js index 728603ae2..930417c5c 100644 --- a/graph/mutators/comment.js +++ b/graph/mutators/comment.js @@ -5,6 +5,8 @@ const ActionsService = require('../../services/actions'); const CommentsService = require('../../services/comments'); const linkify = require('linkify-it')(); +const TagModel = require('../../models/tag'); + const Wordlist = require('../../services/wordlist'); /** @@ -18,20 +20,18 @@ const Wordlist = require('../../services/wordlist'); */ const createComment = ({user, loaders: {Comments}}, {body, asset_id, parent_id = null}, status = 'NONE') => { - let tags = []; - if (user.hasRoles('ADMIN') || user.hasRoles('MODERATOR')) { - tags = [{name: 'STAFF'}]; - } - return CommentsService.publicCreate({ body, asset_id, parent_id, status, - tags, author_id: user.id }) - .then((comment) => { + .then(async (comment) => { + + if (user.hasRoles('ADMIN') || user.hasRoles('MODERATOR')) { + await CommentsService.addTag(comment.id, 'STAFF', user.id); + } // If the loaders are present, clear the caches for these values because we // just added a new comment, hence the counts should be updated. We should diff --git a/models/tag.js b/models/tag.js index 6991f3abb..155e57dbb 100644 --- a/models/tag.js +++ b/models/tag.js @@ -50,13 +50,17 @@ const TagSchema = new Schema({ privacy_type: { type: String, - enum: PRIVACY_TYPES + enum: PRIVACY_TYPES, + default: 'SELF' }, // Additional metadata stored on the field. metadata: Schema.Types.Mixed -}, { - _id: false +}, { + timestamps: { + createdAt: 'created_at', + updatedAt: 'updated_at' + } }); const Tag = mongoose.model('Tag', TagSchema); diff --git a/services/comments.js b/services/comments.js index a7f483d2c..d61b6ca25 100644 --- a/services/comments.js +++ b/services/comments.js @@ -3,8 +3,8 @@ const CommentModel = require('../models/comment'); const ActionModel = require('../models/action'); const ActionsService = require('./actions'); -const TagModel = require('../models/tag'); const TagsService = require('./tags'); +const TagModel = require('../models/tag'); const STATUSES = [ 'ACCEPTED', @@ -51,14 +51,18 @@ module.exports = class CommentsService { */ static addTag(id, name, assigned_by) { - return TagsService.insertCommentTag({ - name, - item_id: id, - item_type: 'COMMENTS', - user_id: assigned_by, + return CommentModel.findOne({id}) + .then((comment) => { + if (comment == null) { + return Promise.reject(new Error('tag not allowed')); + } + return TagsService.insertCommentTag({ + name, + item_id: id, + item_type: 'COMMENTS', + user_id: assigned_by, + }); }); - - // Add the ID to the comment } /** @@ -68,10 +72,15 @@ module.exports = class CommentsService { * @param {String} name the name of the tag to add */ static removeTag(id, name) { - return TagModel.remove({ + return TagModel.findOneAndRemove({ item_type: 'COMMENTS', item_id: id, name + }) + .then((tag) => { + if (tag == null) { + return Promise.reject(new Error('tag does not exist')); + } }); } diff --git a/services/tags.js b/services/tags.js index caeb4e761..172148727 100644 --- a/services/tags.js +++ b/services/tags.js @@ -8,13 +8,26 @@ const ALLOWED_COMMENT_TAGS = [ module.exports = class TagsService { /** - * Finds an action by the id. + * Finds a tag by the id. * @param {String} id identifier of the tag (uuid) */ static findById(id) { return TagModel.findOne({id}); } + /** + * Finds atag by the item_id and name. + * @param {String} item_id identifier of the item that the tag was applied into(uuid) + * @param {string} name name of the tag + */ + static findByItemIdAndName(item_id, name, item_type) { + return TagModel.find({ + item_id, + item_type, + name + }); + } + /** * Add a tag. * @param {string} name the actual tag @@ -30,43 +43,25 @@ module.exports = class TagsService { return Promise.reject(new Error('tag not allowed')); } - // Tags are made unique by using a query that can be reproducable, i.e., - // not containing user inputable values. - let query = { + // // Tags are made unique by using a query that can be reproducable, i.e., + // // not containing user inputable values. + // let query = { + // name: tag.name, + // item_id: tag.item_id, + // item_type: tag.item_type, + // assigned_by: tag.user_id, + // privacy_type: tag.privacy_type + // }; + + // Create/Update the tag. + let newtag = new TagModel({ name: tag.name, item_id: tag.item_id, item_type: tag.item_type, assigned_by: tag.user_id, privacy_type: tag.privacy_type - }; - - // Create/Update the tag. - return TagModel.findOneAndUpdate(query, tag, { - - // Ensure that if it's new, we return the new object created. - new: true, - - // Perform an upsert in the event that this doesn't exist. - upsert: true, - - // Set the default values if not provided based on the mongoose models. - setDefaultsOnInsert: true - }) - .then(({nModified}) => { - switch (nModified) { - case 0: - - // either the tag was already there, or the comment doesn't exist with that id... - throw new Error('Could not add tag to comment. Either the comment doesn\'t exist or the tag is already present.'); - case 1: - - // tag added - return; - default: - - // this should never happen because no multi parameter and unique index on id - } }); + return newtag.save(); } }; diff --git a/test/server/graph/mutations/addCommentTag.js b/test/server/graph/mutations/addCommentTag.js index fbb7fe8a8..063537004 100644 --- a/test/server/graph/mutations/addCommentTag.js +++ b/test/server/graph/mutations/addCommentTag.js @@ -4,7 +4,7 @@ const {graphql} = require('graphql'); const schema = require('../../../../graph/schema'); const Context = require('../../../../graph/context'); const UserModel = require('../../../../models/user'); -const TagModel = require('../../../../models/tag'); +const TagService = require('../../../../services/tags'); const SettingsService = require('../../../../services/settings'); const CommentsService = require('../../../../services/comments'); @@ -19,9 +19,7 @@ describe('graph.mutations.addCommentTag', () => { mutation AddCommentTag ($id: ID!, $tag: String!) { addCommentTag(id:$id, tag:$tag) { comment { - tags { - name - } + id } errors { translation_key @@ -38,10 +36,9 @@ describe('graph.mutations.addCommentTag', () => { console.error(response.errors); } expect(response.errors).to.be.empty; - TagModel.find({ - item_id: response.data.addCommentTag.comment.id, - name: 'BEST' - }).then((tags) => { + + return TagService.findByItemIdAndName(response.data.addCommentTag.comment.id, 'BEST', 'COMMENTS') + .then((tags) => { expect(tags).to.have.length(1); }); }); diff --git a/test/server/graph/mutations/createComment.js b/test/server/graph/mutations/createComment.js index b3bdd459a..11fa666d0 100644 --- a/test/server/graph/mutations/createComment.js +++ b/test/server/graph/mutations/createComment.js @@ -3,11 +3,14 @@ const {graphql} = require('graphql'); const schema = require('../../../../graph/schema'); const Context = require('../../../../graph/context'); + const UserModel = require('../../../../models/user'); const AssetModel = require('../../../../models/asset'); -const SettingsService = require('../../../../services/settings'); const ActionModel = require('../../../../models/action'); +const SettingsService = require('../../../../services/settings'); +const TagService = require('../../../../services/tags'); + describe('graph.mutations.createComment', () => { beforeEach(() => SettingsService.init()); @@ -217,11 +220,14 @@ describe('graph.mutations.createComment', () => { expect(data.createComment).to.have.property('comment').not.null; expect(data.createComment).to.have.property('errors').null; + return TagService.findByItemIdAndName(data.createComment.comment.id, tag, 'COMMENTS'); + }) + .then((tags) => { if (tag) { - expect(data.createComment.comment).to.have.property('tags').length(1); - expect(data.createComment.comment.tags[0]).to.have.property('name', tag); + expect(tags).to.have.length(1); + expect(tags[0]).to.have.property('name', tag); } else { - expect(data.createComment.comment).to.have.property('tags').length(0); + expect(tags).length(0); } }); }); diff --git a/test/server/graph/mutations/removeCommentTag.js b/test/server/graph/mutations/removeCommentTag.js index c0d452f37..4455a0108 100644 --- a/test/server/graph/mutations/removeCommentTag.js +++ b/test/server/graph/mutations/removeCommentTag.js @@ -4,8 +4,10 @@ const {graphql} = require('graphql'); const schema = require('../../../../graph/schema'); const Context = require('../../../../graph/context'); const UserModel = require('../../../../models/user'); + const SettingsService = require('../../../../services/settings'); const CommentsService = require('../../../../services/comments'); +const TagService = require('../../../../services/tags'); describe('graph.mutations.removeCommentTag', () => { let comment; @@ -18,9 +20,7 @@ describe('graph.mutations.removeCommentTag', () => { mutation RemoveCommentTag ($id: ID!, $tag: String!) { removeCommentTag(id:$id, tag:$tag) { comment { - tags { - name - } + id } errors { translation_key @@ -41,7 +41,12 @@ describe('graph.mutations.removeCommentTag', () => { } expect(response.errors).to.be.empty; expect(response.data.removeCommentTag.errors).to.be.null; - expect(response.data.removeCommentTag.comment.tags).to.deep.equal([]); + + TagService.findByItemIdAndName(response.data.removeCommentTag.comment.id, 'BEST') + .then((tags) => { + expect(tags).to.deep.equal([]); + }); + }); describe('users who cant remove tags', () => { @@ -60,8 +65,9 @@ describe('graph.mutations.removeCommentTag', () => { console.error(response.errors); } expect(response.errors).to.be.empty; + expect(response.data.removeCommentTag.errors).to.deep.equal([{'translation_key':'NOT_AUTHORIZED'}]); - expect(response.data.removeCommentTag.comment).to.be.null; + expect(response.data.removeCommentTag.comment).to.be.null; }); }); }); diff --git a/test/server/services/comments.js b/test/server/services/comments.js index 5b9cbf4d1..952e071a3 100644 --- a/test/server/services/comments.js +++ b/test/server/services/comments.js @@ -5,6 +5,7 @@ const ActionsService = require('../../../services/actions'); const UsersService = require('../../../services/users'); const SettingsService = require('../../../services/settings'); const CommentsService = require('../../../services/comments'); +const TagService = require('../../../services/tags'); const settings = {id: '1', moderation: 'PRE', wordlist: {banned: ['bad words'], suspect: ['suspect words']}}; @@ -225,16 +226,17 @@ describe('services.CommentsService', () => { const tagName = 'BEST'; const userId = users[0].id; await CommentsService.addTag(commentId, tagName, userId); - const updatedComment = await CommentsService.findById(commentId); - expect(updatedComment.tags.length).to.equal(1); - expect(updatedComment.tags[0].name).to.equal(tagName); - expect(updatedComment.tags[0].assigned_by).to.equal(userId); - expect(updatedComment.tags[0].created_at).to.be.an.instanceof(Date); + const tags = await TagService.findByItemIdAndName(commentId, 'BEST', 'COMMENTS'); + expect(tags.length).to.equal(1); + expect(tags[0].name).to.equal(tagName); + expect(tags[0].assigned_by).to.equal(userId); + expect(tags[0].created_at).to.be.an.instanceof(Date); }); it('can\'t add a tag to comment id that doesn\'t exist', async () => { const commentId = 'fakenews'; const tagName = 'BEST'; const userId = users[0].id; + await expect(CommentsService.addTag(commentId, tagName, userId)).to.be.rejected; }); it('can\'t add same tag.name twice', async () => { @@ -255,23 +257,23 @@ describe('services.CommentsService', () => { const commentId = comments[0].id; const tagName = 'BEST'; await CommentsService.addTag(commentId, tagName, users[0].id); - const updatedComment = await CommentsService.findById(commentId); - expect(updatedComment.tags.length).to.equal(1); + const tags = await TagService.findByItemIdAndName(commentId, tagName, 'COMMENTS'); + expect(tags.length).to.equal(1); // ok now to remove it await CommentsService.removeTag(commentId, tagName); - const updatedComment2 = await CommentsService.findById(commentId); - expect(updatedComment2.tags.length).to.equal(0); + const tags2 = await TagService.findByItemIdAndName(commentId, tagName, 'COMMENTS'); + expect(tags2.length).to.equal(0); }); it('throws if removing a tag that isn\'t there', async () => { const commentId = comments[0].id; - // just make sure it has no tags to start - const updatedComment2 = await CommentsService.findById(commentId); - expect(updatedComment2.tags.length).to.equal(0); - const tagName = 'BEST'; + // just make sure it has no tags to start + const tags = await TagService.findByItemIdAndName(commentId, tagName, 'COMMENTS'); + expect(tags.length).to.equal(0); + // ok now to remove it await expect(CommentsService.removeTag(commentId, tagName)).to.be.rejected; });