From 7498b520e4e01eeef40c8443cf49cf2be02eef21 Mon Sep 17 00:00:00 2001 From: Jacob Gable Date: Tue, 25 Jun 2013 20:42:51 -0500 Subject: [PATCH] Increment slug if duplicate Refactored the generateSlug method to return a promise and check for existing posts with matching slugs. Should close #221 --- core/shared/errorHandling.js | 2 +- core/shared/models/post.js | 39 +++++++++++++++++++++++------ core/test/ghost/api_posts_spec.js | 21 ++++++++++++++++ core/test/ghost/permissions_spec.js | 2 +- 4 files changed, 54 insertions(+), 10 deletions(-) diff --git a/core/shared/errorHandling.js b/core/shared/errorHandling.js index 26377236bb..f54d5ad4de 100644 --- a/core/shared/errorHandling.js +++ b/core/shared/errorHandling.js @@ -20,7 +20,7 @@ errors = { logError: function (err) { err = err || "Unknown"; // TODO: Logging framework hookup - console.log("Error occurred: ", err.message || err); + console.log("Error occurred: ", err.message || err, err.stack || ""); }, logAndThrowError: function (err) { diff --git a/core/shared/models/post.js b/core/shared/models/post.js index 98836a9ba6..50725ccf90 100644 --- a/core/shared/models/post.js +++ b/core/shared/models/post.js @@ -45,10 +45,7 @@ Post = GhostBookshelf.Model.extend({ }, creating: function () { - if (!this.get('slug')) { - this.generateSlug(); - } - + var self = this; if (!this.get('created_by')) { this.set('created_by', 1); } @@ -56,20 +53,46 @@ Post = GhostBookshelf.Model.extend({ if (!this.get('author_id')) { this.set('author_id', 1); } + + if (!this.get('slug')) { + // Generating a slug requires a db call to look for conflicting slugs + return this.generateSlug(this.get('title')) + .then(function (slug) { + self.set({slug: slug}); + }); + } }, // #### generateSlug // Create a string act as the permalink for a post. - generateSlug: function () { + generateSlug: function (title) { + var slug, + slugTryCount = 1, + // Look for a post with a matching slug, append an incrementing number if so + checkIfSlugExists = function (slugToFind) { + return Post.read({slug: slugToFind}).then(function (found) { + if (!found) { + return when.resolve(slugToFind); + } + + slugTryCount += 1; + + // TODO: Bug out (when.reject) if over 10 tries or something? + + return checkIfSlugExists(slugToFind + slugTryCount); + }); + }; + // Remove reserved chars: `:/?#[]@!$&'()*+,;=` as well as `\` and convert spaces to hyphens - var slug = this.get('title').replace(/[:\/\?#\[\]@!$&'()*+,;=\\]/g, '').replace(/\s/g, '-').toLowerCase(); + slug = title.replace(/[:\/\?#\[\]@!$&'()*+,;=\\]/g, '').replace(/\s/g, '-').toLowerCase(); // Remove trailing hypen slug = slug.charAt(slug.length - 1) === '-' ? slug.substr(0, slug.length - 2) : slug; // Check the filtered slug doesn't match any of the reserved keywords slug = /^(ghost|ghost\-admin|admin|wp\-admin|dashboard|login|archive|archives|category|categories|tag|tags|page|pages|post|posts)$/g .test(slug) ? slug + '-post' : slug; - // TODO: test for duplicate slugs and increment - return this.set('slug', slug); + + // Test for duplicate slugs. + return checkIfSlugExists(slug); }, user: function () { diff --git a/core/test/ghost/api_posts_spec.js b/core/test/ghost/api_posts_spec.js index 39f6984982..9cb8e04c7d 100644 --- a/core/test/ghost/api_posts_spec.js +++ b/core/test/ghost/api_posts_spec.js @@ -100,6 +100,27 @@ describe('Post Model', function () { }); + it('can generate a non conflicting slug', function (done) { + var newPost = { + title: 'Test Title', + content: 'Test Content 1' + }; + + PostModel.add(newPost).then(function (createdPost) { + + createdPost.get('slug').should.equal('test-title'); + + newPost.content = 'Test Content 2'; + return PostModel.add(newPost); + }).then(function (secondPost) { + + secondPost.get('slug').should.equal('test-title2'); + secondPost.get('content').should.equal("Test Content 2"); + + done(); + }).otherwise(done); + }); + it('can delete', function (done) { var firstPostId; PostModel.browse().then(function (results) { diff --git a/core/test/ghost/permissions_spec.js b/core/test/ghost/permissions_spec.js index 5c061e5fa2..b330dc8cd9 100644 --- a/core/test/ghost/permissions_spec.js +++ b/core/test/ghost/permissions_spec.js @@ -20,7 +20,7 @@ describe('permissions', function () { return when(helpers.insertDefaultUser()); }).then(function (results) { done(); - }); + }).otherwise(errors.logAndThrowError); }); // beforeEach(function (done) {