Skip to content

Commit

Permalink
Fixed issue with adding multiple new tags to a post
Browse files Browse the repository at this point in the history 
Fixes TryGhost#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.
  • Loading branch information
@schneidmaster @oluseyi
schneidmaster authored and oluseyi committed Feb 1, 2014
1 parent 91873f8 commit d79254e
Show file tree
Hide file tree
Showing 2 changed files with 117 additions and 16 deletions.
48 changes: 34 additions & 14 deletions core/server/models/post.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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);

});
}

Expand Down Expand Up @@ -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) {
Expand All @@ -411,4 +431,4 @@ Posts = ghostBookshelf.Collection.extend({
module.exports = {
Post: Post,
Posts: Posts
};
};
85 changes: 83 additions & 2 deletions core/test/integration/model/model_tags_spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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'}]})
Expand All @@ -221,4 +302,4 @@ describe('Tag Model', function () {

});

});
});

0 comments on commit d79254e

Please sign in to comment.