From 9064914829c598a6b01b803f975f088b82ff1ca0 Mon Sep 17 00:00:00 2001
From: William Dibbern <gotdibbs@outlook.com>
Date: Thu, 5 Sep 2013 21:09:47 -0500
Subject: [PATCH] Added redirect to get rid of /page/1/

Fixes #592

- Added *permanent* redirect to ensure `/page/1/` isn't used and that
`/` is used instead.
- Added pageUrl helper (and unit tests) to generate client side url
fragment for blog pages conforming to the above standard.
- Updated pagination helper to use new `pageUrl` theme helper.
- Added functional tests for redirects and added scaffolding for
functional frontend tests in general.
---
 Gruntfile.js                                   |  2 +-
 core/server/controllers/frontend.js            |  9 +++++++--
 core/server/helpers/index.js                   | 12 ++++++++++++
 core/server/helpers/tpl/pagination.hbs         |  4 ++--
 core/test/functional/frontend/01_route_test.js | 16 ++++++++++++++++
 core/test/unit/server_helpers_index_spec.js    | 12 ++++++++++++
 6 files changed, 50 insertions(+), 5 deletions(-)
 create mode 100644 core/test/functional/frontend/01_route_test.js

diff --git a/Gruntfile.js b/Gruntfile.js
index 435e72550f..15a370449e 100644
--- a/Gruntfile.js
+++ b/Gruntfile.js
@@ -361,7 +361,7 @@ var path = require('path'),
         grunt.registerTask('spawn-casperjs', function () {
             var done = this.async(),
                 options = ['host', 'noPort', 'port', 'email', 'password'],
-                args = ['test', 'admin/', '--includes=base.js', '--direct', '--log-level=debug', '--port=2369'];
+                args = ['test', 'admin/', 'frontend/', '--includes=base.js', '--direct', '--log-level=debug', '--port=2369'];
 
             // Forward parameters from grunt to casperjs
             _.each(options, function processOption(option) {
diff --git a/core/server/controllers/frontend.js b/core/server/controllers/frontend.js
index 461dc8699d..8f465b595e 100644
--- a/core/server/controllers/frontend.js
+++ b/core/server/controllers/frontend.js
@@ -21,10 +21,15 @@ frontendControllers = {
         // No negative pages
         if (isNaN(pageParam) || pageParam < 1) {
             //redirect to 404 page?
-            return res.redirect("/page/1/");
+            return res.redirect("/");
         }
         options.page = pageParam;
 
+        // Redirect '/page/1/' to '/' for all teh good SEO
+        if (pageParam === 1 && req.route.path === '/page/:page/') {
+            return res.redirect('/');
+        }
+
         // No negative posts per page, must be number
         if (!isNaN(postsPerPage) && postsPerPage > 0) {
             options.limit = postsPerPage;
@@ -42,7 +47,7 @@ frontendControllers = {
 
             // If page is greater than number of pages we have, redirect to last page
             if (pageParam > maxPage) {
-                return res.redirect("/page/" + maxPage + "/");
+                return res.redirect(maxPage === 1 ? '/' : ('/page/' + maxPage + '/'));
             }
 
             // Render the page of posts
diff --git a/core/server/helpers/index.js b/core/server/helpers/index.js
index 656b5de49e..a129ae75cd 100644
--- a/core/server/helpers/index.js
+++ b/core/server/helpers/index.js
@@ -43,6 +43,18 @@ coreHelpers = function (ghost) {
         return date;
     });
 
+    // ### Page URL Helper
+    // 
+    // *Usage example:*
+    // `{{pageUrl 2}}`
+    // 
+    // Returns the URL for the page specified in the current object
+    // context.
+    //
+    ghost.registerThemeHelper('pageUrl', function (context, block) {
+        return context === 1 ? '/' : ('/page/' + context + '/');
+    });
+
     // ### URL helper
     //
     // *Usage example:*
diff --git a/core/server/helpers/tpl/pagination.hbs b/core/server/helpers/tpl/pagination.hbs
index 0aaa57c7ec..e2e48826e3 100644
--- a/core/server/helpers/tpl/pagination.hbs
+++ b/core/server/helpers/tpl/pagination.hbs
@@ -1,9 +1,9 @@
 <nav class="pagination" role="pagination">
     {{#if prev}}
-        <a class="newer-posts" href="/page/{{prev}}/">&larr; Newer Posts</a>
+        <a class="newer-posts" href="{{pageUrl prev}}">&larr; Newer Posts</a>
     {{/if}}
     <span class="page-number">Page {{page}} of {{pages}}</span>
     {{#if next}}
-        <a class="older-posts" href="/page/{{next}}/">Older Posts &rarr;</a>
+        <a class="older-posts" href="{{pageUrl next}}">Older Posts &rarr;</a>
     {{/if}}
 </nav>
\ No newline at end of file
diff --git a/core/test/functional/frontend/01_route_test.js b/core/test/functional/frontend/01_route_test.js
new file mode 100644
index 0000000000..a10c4a23e6
--- /dev/null
+++ b/core/test/functional/frontend/01_route_test.js
@@ -0,0 +1,16 @@
+/**
+ * Tests logging out and attempting to sign up
+ */
+
+/*globals casper, __utils__, url, testPost, falseUser, email */
+casper.test.begin('Redirects page 1 request', 1, function suite(test) {
+    test.filename = '01_route_test_redirects_page_1.png';
+
+    casper.start(url + 'page/1/', function then(response) {
+        test.assertEqual(casper.getCurrentUrl().indexOf('page/'), -1, 'Should be redirected to "/".');
+    }).viewport(1280, 1024);
+
+    casper.run(function () {
+        test.done();
+    });
+});
\ No newline at end of file
diff --git a/core/test/unit/server_helpers_index_spec.js b/core/test/unit/server_helpers_index_spec.js
index 1c021bfd5e..001230b84f 100644
--- a/core/test/unit/server_helpers_index_spec.js
+++ b/core/test/unit/server_helpers_index_spec.js
@@ -245,6 +245,18 @@ describe('Core Helpers', function () {
         });
     });
 
+    describe('Page Url Helper', function () {
+        it('has loaded pageUrl helper', function () {
+            should.exist(handlebars.helpers.pageUrl);
+        });
+
+        it('can return a valid url', function () {
+            handlebars.helpers.pageUrl(1).should.equal('/');
+            handlebars.helpers.pageUrl(2).should.equal('/page/2/');
+            handlebars.helpers.pageUrl(50).should.equal('/page/50/');
+        });
+    });
+
     describe("Pagination helper", function () {
         var paginationRegex = /class="pagination"/,
             newerRegex = /class="newer-posts"/,