From d79254eeda66839db9ebb4ae290881c395d91c06 Mon Sep 17 00:00:00 2001 From: Zach Schneider Date: Mon, 13 Jan 2014 09:29:40 -0500 Subject: [PATCH] Fixed issue with adding multiple new tags to a post Fixes #1907 Refactored `updateTags` to correct a loop issue where the `insert` method was mistakingly being passed rather than `update`, triggering a duplicate PK SQL error. --- core/server/models/post.js | 48 ++++++++--- .../test/integration/model/model_tags_spec.js | 85 ++++++++++++++++++- 2 files changed, 117 insertions(+), 16 deletions(-) diff --git a/core/server/models/post.js b/core/server/models/post.js index fb24db0d045..75b6123841c 100644 --- a/core/server/models/post.js +++ b/core/server/models/post.js @@ -115,7 +115,8 @@ Post = ghostBookshelf.Model.extend({ var existingTags = thisPostWithTags.related('tags').toJSON(), tagOperations = [], tagsToDetach = [], - tagsToAttach = []; + tagsToAttach = [], + createdTagsToAttach = []; // First find any tags which have been removed _.each(existingTags, function (existingTag) { @@ -143,23 +144,45 @@ Post = ghostBookshelf.Model.extend({ tagsToAttach = _.reject(tagsToAttach, function (tagToAttach) { return tagToAttach.name === matchingTag.name; }); + }); + // Return if no tags to add + if (tagsToAttach.length === 0) { + return; + } + + // Set method to insert, so each tag gets inserted with the appropriate options + var opt = options.method; + options.method = 'insert'; + + // Create each tag that doesn't yet exist _.each(tagsToAttach, function (tagToCreateAndAttach) { - var createAndAttachOperation, - opt = options.method; - //TODO: remove when refactor; ugly fix to overcome bookshelf - options.method = 'insert'; - createAndAttachOperation = Tag.add({name: tagToCreateAndAttach.name}, options).then(function (createdTag) { - options.method = opt; - return self.tags().attach(createdTag.id, createdTag.name, options); - }); + var createAndAttachOperation = Tag.add({name: tagToCreateAndAttach.name}, options).then(function (createdTag) { + createdTagsToAttach.push(createdTag); + + // If the tags are all inserted, process them + if (tagsToAttach.length === createdTagsToAttach.length) { + + // Set method back to whatever it was, for tag attachment + options.method = opt; + // Attach each newly created tag + _.each(createdTagsToAttach, function (tagToAttach) { + self.tags().attach(tagToAttach.id, tagToAttach.name, options); + }); + + } + + }); tagOperations.push(createAndAttachOperation); + }); + // Return when all tags attached return when.all(tagOperations); + }); } @@ -381,10 +404,7 @@ Post = ghostBookshelf.Model.extend({ var self = this; return ghostBookshelf.Model.edit.call(this, editedPost, options).then(function (editedObj) { - return when(editedObj.updateTags(editedPost.tags, null, options)).then(function () { - return self.findOne({status: 'all', id: editedObj.id}, options); - }); - //return self.findOne({status: 'all', id: editedObj.id}, options); + return self.findOne({status: 'all', id: editedObj.id}, options); }); }, destroy: function (_identifier, options) { @@ -411,4 +431,4 @@ Posts = ghostBookshelf.Collection.extend({ module.exports = { Post: Post, Posts: Posts -}; +}; \ No newline at end of file diff --git a/core/test/integration/model/model_tags_spec.js b/core/test/integration/model/model_tags_spec.js index 359825eddbe..84677abe518 100644 --- a/core/test/integration/model/model_tags_spec.js +++ b/core/test/integration/model/model_tags_spec.js @@ -185,7 +185,7 @@ describe('Tag Model', function () { }).then(null, done); }); - it('creates and attaches tags that are new to the Tags table', function (done) { + it('creates and attaches a tag that is new to the Tags table', function (done) { var seededTagNames = ['tag1', 'tag2']; seedTags(seededTagNames).then(function (postModel) { @@ -205,6 +205,87 @@ describe('Tag Model', function () { }).then(null, done); }); + it('creates and attaches multiple tags that are new to the Tags table', function (done) { + var seededTagNames = ['tag1']; + + seedTags(seededTagNames).then(function (postModel) { + // the tag API expects tags to be provided like {id: 1, name: 'draft'} + var tagData = seededTagNames.map(function (tagName, i) { return {id: i + 1, name: tagName}; }); + + // add the additional tags, and save + tagData.push({id: null, name: 'tag2'}); + tagData.push({id: null, name: 'tag3'}); + return postModel.set('tags', tagData).save(); + }).then(function (postModel) { + return PostModel.read({id: postModel.id, status: 'all'}, { withRelated: ['tags']}); + }).then(function (reloadedPost) { + var tagNames = reloadedPost.related('tags').models.map(function (t) { return t.attributes.name; }); + tagNames.sort().should.eql(['tag1', 'tag2', 'tag3']); + + done(); + }).then(null, done); + }); + + it('attaches one tag that exists in the Tags database and one tag that is new to the Tags database', function (done) { + var seededTagNames = ['tag1'], + postModel; + + seedTags(seededTagNames).then(function (_postModel) { + postModel = _postModel; + return TagModel.add({name: 'tag2'}); + }).then(function () { + // the tag API expects tags to be provided like {id: 1, name: 'draft'} + var tagData = seededTagNames.map(function (tagName, i) { return {id: i + 1, name: tagName}; }); + + // Add the tag that exists in the database + tagData.push({id: 2, name: 'tag2'}); + + // Add the tag that doesn't exist in the database + tagData.push({id: 3, name: 'tag3'}); + + return postModel.set('tags', tagData).save(); + }).then(function () { + return PostModel.read({id: postModel.id, status: 'all'}, { withRelated: ['tags']}); + }).then(function (reloadedPost) { + var tagModels = reloadedPost.related('tags').models, + tagNames = tagModels.map(function (t) { return t.attributes.name; }); + tagNames.sort().should.eql(['tag1', 'tag2', 'tag3']); + tagModels[2].id.should.eql(4); // make sure it hasn't just added a new tag with the same name + + done(); + }).then(null, done); + }); + + it('attaches one tag that exists in the Tags database and two tags that are new to the Tags database', function (done) { + var seededTagNames = ['tag1'], + postModel; + + seedTags(seededTagNames).then(function (_postModel) { + postModel = _postModel; + return TagModel.add({name: 'tag2'}); + }).then(function () { + // the tag API expects tags to be provided like {id: 1, name: 'draft'} + var tagData = seededTagNames.map(function (tagName, i) { return {id: i + 1, name: tagName}; }); + + // Add the tag that exists in the database + tagData.push({id: 2, name: 'tag2'}); + + // Add the tags that doesn't exist in the database + tagData.push({id: 3, name: 'tag3'}); + tagData.push({id: 4, name: 'tag4'}); + + return postModel.set('tags', tagData).save(); + }).then(function () { + return PostModel.read({id: postModel.id, status: 'all'}, { withRelated: ['tags']}); + }).then(function (reloadedPost) { + var tagModels = reloadedPost.related('tags').models, + tagNames = tagModels.map(function (t) { return t.attributes.name; }); + tagNames.sort().should.eql(['tag1', 'tag2', 'tag3', 'tag4']); + tagModels[2].id.should.eql(4); // make sure it hasn't just added a new tag with the same name + + done(); + }).then(null, done); + }); it('can add a tag to a post on creation', function (done) { var newPost = _.extend(testUtils.DataGenerator.forModel.posts[0], {tags: [{name: 'test_tag_1'}]}) @@ -221,4 +302,4 @@ describe('Tag Model', function () { }); -}); +}); \ No newline at end of file