diff --git a/core/server/models/base/plugins/generate-slug.js b/core/server/models/base/plugins/generate-slug.js index 311f4e8d1f..4086b091d8 100644 --- a/core/server/models/base/plugins/generate-slug.js +++ b/core/server/models/base/plugins/generate-slug.js @@ -13,7 +13,7 @@ module.exports = function (Bookshelf) { * Create a string to act as the permalink for an object. * @param {Bookshelf['Model']} Model Model type to generate a slug for * @param {String} base The string for which to generate a slug, usually a title or name - * @param {Object} options Options to pass to findOne + * @param {GenerateSlugOptions} [options] Options to pass to findOne * @return {Promise} Resolves to a unique slug string */ generateSlug: function generateSlug(Model, base, options) { @@ -98,6 +98,10 @@ module.exports = function (Bookshelf) { slug = baseName; } + if (options && options.skipDuplicateChecks === true) { + return slug; + } + // Test for duplicate slugs. return checkIfSlugExists(slug); } @@ -107,3 +111,11 @@ module.exports = function (Bookshelf) { /** * @type {import('bookshelf')} Bookshelf */ + +/** + * @typedef {object} GenerateSlugOptions + * @property {string} [status] Used for posts, to also filter by post status when generating a slug + * @property {boolean} [importing] Set to true to don't cut the slug on import + * @property {boolean} [shortSlug] If it's a user, let's try to cut it down (unless this is a human request) + * @property {boolean} [skipDuplicateChecks] Don't append unique identifiers when the slug is not unique (this prevents any database queries) + */ \ No newline at end of file diff --git a/core/server/models/post.js b/core/server/models/post.js index ab6bf15e7e..60b7064a68 100644 --- a/core/server/models/post.js +++ b/core/server/models/post.js @@ -15,6 +15,7 @@ const limitService = require('../services/limits'); const mobiledocLib = require('../lib/mobiledoc'); const relations = require('./relations'); const urlUtils = require('../../shared/url-utils'); +const {Tag} = require('./tag'); const messages = { isAlreadyPublished: 'Your post is already published, please reload your page.', @@ -552,15 +553,24 @@ Post = ghostBookshelf.Model.extend({ tagsToSave = []; // and deduplicate upper/lowercase tags - _.each(this.get('tags'), function each(item) { + loopTags: for (const tag of this.get('tags')) { + if (!tag.id && !tag.tag_id && tag.slug) { + // Clean up the provided slugs before we do any matching with existing tags + tag.slug = await ghostBookshelf.Model.generateSlug( + Tag, + tag.slug, + {skipDuplicateChecks: true} + ); + } + for (i = 0; i < tagsToSave.length; i = i + 1) { - if (tagsToSave[i].name && item.name && tagsToSave[i].name.toLocaleLowerCase() === item.name.toLocaleLowerCase()) { - return; + if (tagsToSave[i].name && tag.name && tagsToSave[i].name.toLocaleLowerCase() === tag.name.toLocaleLowerCase()) { + continue loopTags; } } - tagsToSave.push(item); - }); + tagsToSave.push(tag); + } this.set('tags', tagsToSave); } diff --git a/test/regression/api/admin/posts.test.js b/test/regression/api/admin/posts.test.js index b6fb9a27fa..e86e20e279 100644 --- a/test/regression/api/admin/posts.test.js +++ b/test/regression/api/admin/posts.test.js @@ -396,6 +396,143 @@ describe('Posts API (canary)', function () { res.body.posts[0].tags[1].slug.should.equal('four'); }); }); + + it('can add with tags - slug with spaces', async function () { + const res = await request + .post(localUtils.API.getApiQuery('posts/')) + .set('Origin', config.get('url')) + .send({ + posts: [{ + title: 'Tags test 5', + tags: [{slug: 'five spaces'}] + }] + }) + .expect('Content-Type', /json/) + .expect('Cache-Control', testUtils.cacheRules.private) + .expect(201); + + should.exist(res.body.posts); + should.exist(res.body.posts[0].title); + res.body.posts[0].title.should.equal('Tags test 5'); + res.body.posts[0].tags.length.should.equal(1); + res.body.posts[0].tags[0].slug.should.equal('five-spaces'); + + // Expected behaviour when creating a slug with spaces: + res.body.posts[0].tags[0].name.should.equal('five-spaces'); + + // If we create another post again now that the five-spaces tag exists, + // we need to make sure it matches correctly and doesn't create a new tag again + + const res2 = await request + .post(localUtils.API.getApiQuery('posts/')) + .set('Origin', config.get('url')) + .send({ + posts: [{ + title: 'Tags test 6', + tags: [{slug: 'five spaces'}] + }] + }) + .expect('Content-Type', /json/) + .expect('Cache-Control', testUtils.cacheRules.private) + .expect(201); + + should.exist(res2.body.posts); + should.exist(res2.body.posts[0].title); + res2.body.posts[0].title.should.equal('Tags test 6'); + res2.body.posts[0].tags.length.should.equal(1); + res2.body.posts[0].tags[0].id.should.equal(res.body.posts[0].tags[0].id); + }); + + it('can add with tags - slug with spaces not automated', async function () { + // Make sure that the matching still works when using a different name + // this is important because it invalidates any solution that would just consider a slug as the name + const res = await request + .post(localUtils.API.getApiQuery('posts/')) + .set('Origin', config.get('url')) + .send({ + posts: [{ + title: 'Tags test 7', + tags: [{slug: 'six-spaces', name: 'Not automated name for six spaces'}] + }] + }) + .expect('Content-Type', /json/) + .expect('Cache-Control', testUtils.cacheRules.private) + .expect(201); + + should.exist(res.body.posts); + should.exist(res.body.posts[0].title); + res.body.posts[0].title.should.equal('Tags test 7'); + res.body.posts[0].tags.length.should.equal(1); + res.body.posts[0].tags[0].slug.should.equal('six-spaces'); + res.body.posts[0].tags[0].name.should.equal('Not automated name for six spaces'); + + // If we create another post again now that the five-spaces tag exists, + // we need to make sure it matches correctly and doesn't create a new tag again + + const res2 = await request + .post(localUtils.API.getApiQuery('posts/')) + .set('Origin', config.get('url')) + .send({ + posts: [{ + title: 'Tags test 8', + tags: [{slug: 'six spaces'}] + }] + }) + .expect('Content-Type', /json/) + .expect('Cache-Control', testUtils.cacheRules.private) + .expect(201); + + should.exist(res2.body.posts); + should.exist(res2.body.posts[0].title); + res2.body.posts[0].title.should.equal('Tags test 8'); + res2.body.posts[0].tags.length.should.equal(1); + res2.body.posts[0].tags[0].id.should.equal(res.body.posts[0].tags[0].id); + }); + + it('can add with tags - too long slug', async function () { + const tooLongSlug = 'a'.repeat(190); + + const res = await request + .post(localUtils.API.getApiQuery('posts/')) + .set('Origin', config.get('url')) + .send({ + posts: [{ + title: 'Tags test 9', + tags: [{slug: tooLongSlug}] + }] + }) + .expect('Content-Type', /json/) + .expect('Cache-Control', testUtils.cacheRules.private) + .expect(201); + + should.exist(res.body.posts); + should.exist(res.body.posts[0].title); + res.body.posts[0].title.should.equal('Tags test 9'); + res.body.posts[0].tags.length.should.equal(1); + res.body.posts[0].tags[0].slug.should.equal(tooLongSlug.substring(0, 185)); + + // If we create another post again now that the very long tag exists, + // we need to make sure it matches correctly and doesn't create a new tag again + + const res2 = await request + .post(localUtils.API.getApiQuery('posts/')) + .set('Origin', config.get('url')) + .send({ + posts: [{ + title: 'Tags test 10', + tags: [{slug: tooLongSlug}] + }] + }) + .expect('Content-Type', /json/) + .expect('Cache-Control', testUtils.cacheRules.private) + .expect(201); + + should.exist(res2.body.posts); + should.exist(res2.body.posts[0].title); + res2.body.posts[0].title.should.equal('Tags test 10'); + res2.body.posts[0].tags.length.should.equal(1); + res2.body.posts[0].tags[0].id.should.equal(res.body.posts[0].tags[0].id); + }); }); describe('Edit', function () {