Fix slug multiple hyphens and short words

Should close TryGhost#228 and TryGhost#238
dnlek · Jul 8, 2013 · d8d88f4 · d8d88f4
1 parent 49e4e77
commit d8d88f4
Show file tree

Hide file tree

Showing 3 changed files with 54 additions and 9 deletions.
diff --git a/core/shared/models/post.js b/core/shared/models/post.js
@@ -67,6 +67,7 @@ Post = GhostBookshelf.Model.extend({
     // Create a string act as the permalink for a post.
     generateSlug: function (title) {
         var slug,
+            shortSlug,
             slugTryCount = 1,
             // Look for a post with a matching slug, append an incrementing number if so
             checkIfSlugExists = function (slugToFind) {
@@ -85,14 +86,31 @@ Post = GhostBookshelf.Model.extend({
 
         // Remove reserved chars: `:/?#[]@!$&'()*+,;=` as well as `\` and convert spaces to hyphens
         slug = title.replace(/[:\/\?#\[\]@!$&'()*+,;=\\]/g, '').replace(/\s/g, '-').toLowerCase();
-        // Remove trailing hypen
+        // Remove trailing hyphen
         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;
 
+        // Replace multiple hyphens and anything with less than two letters with one hyphen
+        shortSlug = _.filter(slug.split('-'), function (part) {
+            return part.length > 2;
+        }).join('-');
+
+        // If we've gotten rid of everything, come back through with lower threshold
+        if (shortSlug.length < 1) {
+            shortSlug = _.filter(slug.split('-'), function (part) {
+                return part.length > 1;
+            }).join('-');
+
+            // If we've still got no slug, default to just 'post'
+            if (shortSlug.length < 1) {
+                shortSlug = "post";
+            }
+        }
+
         // Test for duplicate slugs.
-        return checkIfSlugExists(slug);
+        return checkIfSlugExists(shortSlug);
     },
 
     user: function () {

diff --git a/core/test/ghost/api_posts_spec.js b/core/test/ghost/api_posts_spec.js
@@ -74,12 +74,12 @@ describe('Post Model', function () {
             createdPost.get('status').should.equal('draft');
             createdPost.get('title').should.equal(newPost.title, "title is correct");
             createdPost.get('content').should.equal(newPost.content, "content is correct");
-            createdPost.get('slug').should.equal(newPost.title.toLowerCase().replace(/ /g, '-'), 'slug is correct');
-            //createdPost.get('created_at').should.be.instanceOf(Date); - why is this not true?
+            createdPost.get('slug').should.equal('test-title');
+            createdPost.get('created_at').should.be.below(new Date().getTime()).and.be.above(new Date(0).getTime());
             createdPost.get('created_by').should.equal(1);
             createdPost.get('author_id').should.equal(1);
             createdPost.get('created_by').should.equal(createdPost.get('author_id'));
-            //createdPost.get('updated_at').should.be.instanceOf(Date); - why is this not true?
+            createdPost.get('updated_at').should.be.below(new Date().getTime()).and.be.above(new Date(0).getTime());
             createdPost.get('updated_by').should.equal(1);
             should.equal(createdPost.get('published_at'), null);
             should.equal(createdPost.get('published_by'), null);
@@ -121,6 +121,34 @@ describe('Post Model', function () {
         }).otherwise(done);
     });
 
+    it('can generate slugs without 2 letter words', function (done) {
+        var newPost = {
+            title: 'I am an idiot',
+            content: 'Test Content 1'
+        };
+
+        PostModel.add(newPost).then(function (createdPost) {
+
+            createdPost.get('slug').should.equal('idiot');
+
+            done();
+        });
+    });
+
+    it('can generate slugs without duplicate hyphens', function (done) {
+        var newPost = {
+            title: 'apprehensive  titles  have  too  many  spaces  ',
+            content: 'Test Content 1'
+        };
+
+        PostModel.add(newPost).then(function (createdPost) {
+
+            createdPost.get('slug').should.equal('apprehensive-titles-have-too-many-spaces');
+
+            done();
+        });
+    });
+
     it('can delete', function (done) {
         var firstPostId;
         PostModel.browse().then(function (results) {
@@ -136,7 +164,6 @@ describe('Post Model', function () {
 
             ids = _.pluck(newResults.models, "id");
             hasDeletedId = _.any(ids, function (id) {
-
                 return id === firstPostId;
             });
             hasDeletedId.should.equal(false);

diff --git a/core/test/ghost/import_spec.js b/core/test/ghost/import_spec.js
@@ -17,7 +17,7 @@ describe("Import", function () {
     beforeEach(function (done) {
         helpers.resetData().then(function () {
             done();
-        }, done);
+        }, errors.logAndThrowError);
     });
 
     it("resolves 001", function (done) {
@@ -32,7 +32,7 @@ describe("Import", function () {
             importStub.restore();
 
             done();
-        }, errors.throwError);
+        }, errors.logAndThrowError);
     });
 
     describe("001", function () {
@@ -72,7 +72,7 @@ describe("Import", function () {
                 importedData[2].length.should.equal(exportData.data.settings.length);
 
                 done();
-            });
+            }).otherwise(errors.logAndThrowError);
         });
     });
 });