improves unit testing for pagination and fixes pagination partial template (removed extra </nav>)

removes console logs for error handling in testing and travis environments

core/server/errorHandling.js

@@ -20,7 +20,11 @@ errors = {
     logError: function (err) {
         err = err || "Unknown";
         // TODO: Logging framework hookup
-        console.log("Error occurred: ", err.message || err, err.stack || "");
+//        Eventually we'll have better logging which will know about envs
+        if (process.env.NODE_ENV === 'development' || process.env.NODE_ENV === 'staging'
+                || process.env.NODE_ENV === 'production') {
+            console.log("Error occurred: ", err.message || err, err.stack || "");
+        }
     },
 
     logAndThrowError: function (err) {

core/server/helpers/index.js

@@ -154,6 +154,22 @@ coreHelpers = function (ghost) {
                 errors.logAndThrowError('pagination data is not an object or is a function');
                 return;
             }
+            if (_.isUndefined(this.pagination.page) || _.isUndefined(this.pagination.pages)
+                    || _.isUndefined(this.pagination.total) || _.isUndefined(this.pagination.limit)
+                    || _.isUndefined(this.pagination.prev) || _.isUndefined(this.pagination.prev)) {
+                errors.logAndThrowError('All values must be defined for page, pages, limit, total, prev and next');
+                return;
+            }
+            if ((!_.isNumber(this.pagination.next) && !_.isNull(this.pagination.next))
+                    || (!_.isNumber(this.pagination.prev) && !_.isNull(this.pagination.prev))) {
+                errors.logAndThrowError('Invalid value, Next/Prev must be a number or null');
+                return;
+            }
+            if (!_.isNumber(this.pagination.page) || !_.isNumber(this.pagination.pages)
+                    || !_.isNumber(this.pagination.total) || !_.isNumber(this.pagination.limit)) {
+                errors.logAndThrowError('Invalid value, check page, pages, limit and total are numbers');
+                return;
+            }
             return new hbs.handlebars.SafeString(templateFn(this.pagination));
         });
     });

core/server/helpers/tpl/pagination.hbs

@@ -5,6 +5,6 @@
     {{/if}}
     <div class="page-number">Page {{page}}<span class="extended"> of {{pages}}</span></div>
     {{#if prev}}
-        <div class="next-page"><a href="/page/{{prev}}/">← Newer Posts</a></div></nav>
+        <div class="next-page"><a href="/page/{{prev}}/">← Newer Posts</a></div>
     {{/if}}
 </nav>

core/test/ghost/errorHandling_spec.js

@@ -2,7 +2,11 @@
 var should = require('should'),
     when = require('when'),
     sinon = require('sinon'),
-    errors = require('../../server/errorHandling');
+    errors = require('../../server/errorHandling'),
+    // storing current environment
+    currentEnv = process.env.NODE_ENV;
+
+
 
 describe("Error handling", function () {
 
@@ -39,6 +43,8 @@ describe("Error handling", function () {
         var err = new Error("test1"),
             logStub = sinon.stub(console, "log");
 
+        // give environment a value that will console log
+        process.env.NODE_ENV = "development";
         errors.logError(err);
 
         // Calls log with message on Error objects
@@ -54,6 +60,8 @@ describe("Error handling", function () {
         logStub.calledWith("Error occurred: ", err).should.equal(true);
 
         logStub.restore();
+        process.env.NODE_ENV = currentEnv;
+
     });
 
     it("logs promise errors with custom messages", function (done) {
@@ -61,6 +69,8 @@ describe("Error handling", function () {
             prom = def.promise,
             logStub = sinon.stub(console, "log");
 
+        // give environment a value that will console log
+        process.env.NODE_ENV = "development";
         prom.then(function () {
             throw new Error("Ran success handler");
         }, errors.logErrorWithMessage("test1"));
@@ -71,7 +81,10 @@ describe("Error handling", function () {
 
             done();
         });
-
+        prom.ensure(function () {
+            // gives the environment the correct value back
+            process.env.NODE_ENV = currentEnv;
+        });
         def.reject();
     });
 
@@ -87,6 +100,8 @@ describe("Error handling", function () {
             logStub = sinon.stub(console, "log"),
             redirectStub = sinon.stub(res, "redirect");
 
+        // give environment a value that will console log
+        process.env.NODE_ENV = "development";
         prom.then(function () {
             throw new Error("Ran success handler");
         }, errors.logErrorWithRedirect("test1", "/testurl", req, res));
@@ -100,7 +115,10 @@ describe("Error handling", function () {
 
             done();
         });
-
+        prom.ensure(function () {
+            // gives the environment the correct value back
+            process.env.NODE_ENV = currentEnv;
+        });
         def.reject();
     });
 });

core/test/ghost/frontend_helpers_index_spec.js

@@ -69,4 +69,87 @@ describe('Core Helpers', function () {
             }).then(null, done);
         });
     });
+
+    describe("Pagination helper", function () {
+        it('has loaded paginate helper', function () {
+            should.exist(handlebars.helpers.paginate);
+        });
+
+        it('can render single page with no pagination necessary', function (done) {
+            var rendered;
+            helpers.loadCoreHelpers(ghost).then(function () {
+                rendered = handlebars.helpers.paginate.call({pagination: {page: 1, prev: null, next: null, limit: 15, total: 8, pages: 1}});
+                should.exist(rendered);
+                rendered.string.should.equal('\n<nav id="pagination" role="pagination">\n    \n    <div class="page-number">Page 1<span class="extended"> of 1</span></div>\n    \n</nav>');
+                done();
+            }).then(null, done);
+        });
+
+        it('can render first page of many with older posts link', function (done) {
+            var rendered;
+            helpers.loadCoreHelpers(ghost).then(function () {
+                rendered = handlebars.helpers.paginate.call({pagination: {page: 1, prev: null, next: 2, limit: 15, total: 8, pages: 3}});
+                should.exist(rendered);
+                rendered.string.should.equal('\n<nav id="pagination" role="pagination">\n    \n        <div class="previous-page"><a href="/page/2/">Older Posts →</a></div>\n    \n    <div class="page-number">Page 1<span class="extended"> of 3</span></div>\n    \n</nav>');
+                done();
+            }).then(null, done);
+        });
+
+        it('can render middle pages of many with older and newer posts link', function (done) {
+            var rendered;
+            helpers.loadCoreHelpers(ghost).then(function () {
+                rendered = handlebars.helpers.paginate.call({pagination: {page: 2, prev: 1, next: 3, limit: 15, total: 8, pages: 3}});
+                should.exist(rendered);
+                rendered.string.should.equal('\n<nav id="pagination" role="pagination">\n    \n        <div class="previous-page"><a href="/page/3/">Older Posts →</a></div>\n    \n    <div class="page-number">Page 2<span class="extended"> of 3</span></div>\n    \n        <div class="next-page"><a href="/page/1/">← Newer Posts</a></div>\n    \n</nav>');
+                done();
+            }).then(null, done);
+        });
+
+        it('can render last page of many with newer posts link', function (done) {
+            var rendered;
+            helpers.loadCoreHelpers(ghost).then(function () {
+                rendered = handlebars.helpers.paginate.call({pagination: {page: 3, prev: 2, next: null, limit: 15, total: 8, pages: 3}});
+                should.exist(rendered);
+                rendered.string.should.equal('\n<nav id="pagination" role="pagination">\n    \n    <div class="page-number">Page 3<span class="extended"> of 3</span></div>\n    \n        <div class="next-page"><a href="/page/2/">← Newer Posts</a></div>\n    \n</nav>');
+                done();
+            }).then(null, done);
+        });
+
+        it('validates values', function (done) {
+            helpers.loadCoreHelpers(ghost).then(function () {
+                var runErrorTest = function (data) {
+                    return function () {
+                        handlebars.helpers.paginate.call(data);
+                    };
+                };
+
+                runErrorTest({pagination: {page: 3, prev: true, next: null, limit: 15, total: 8, pages: 3}})
+                    .should.throwError('Invalid value, Next/Prev must be a number or null');
+                runErrorTest({pagination: {page: 3, prev: 2, next: true, limit: 15, total: 8, pages: 3}})
+                    .should.throwError('Invalid value, Next/Prev must be a number or null');
+
+                runErrorTest({pagination: {prev: 2, next: null, limit: 15, total: 8, pages: 3}})
+                    .should.throwError('All values must be defined for page, pages, limit, total, prev and next');
+                runErrorTest({pagination: {page: 3, next: null, limit: 15, total: 8, pages: 3}})
+                    .should.throwError('All values must be defined for page, pages, limit, total, prev and next');
+                runErrorTest({pagination: {page: 3, prev: 2, next: null, total: 8, pages: 3}})
+                    .should.throwError('All values must be defined for page, pages, limit, total, prev and next');
+                runErrorTest({pagination: {page: 3, prev: 2, next: null, limit: 15,pages: 3}})
+                    .should.throwError('All values must be defined for page, pages, limit, total, prev and next');
+                runErrorTest({pagination: {page: 3, prev: 2, next: null, limit: 15, total: 8}})
+                    .should.throwError('All values must be defined for page, pages, limit, total, prev and next');
+
+                runErrorTest({pagination: {page: null, prev: 2, next: null, limit: 15, total: 8, pages: 3}})
+                    .should.throwError('Invalid value, check page, pages, limit and total are numbers');
+                runErrorTest({pagination: {page: 1, prev: 2, next: null, limit: null, total: 8, pages: 3}})
+                    .should.throwError('Invalid value, check page, pages, limit and total are numbers');
+                runErrorTest({pagination: {page: 1, prev: 2, next: null, limit: 15, total: null, pages: 3}})
+                    .should.throwError('Invalid value, check page, pages, limit and total are numbers');
+                runErrorTest({pagination: {page: 1, prev: 2, next: null, limit: 15, total: 8, pages: null}})
+                    .should.throwError('Invalid value, check page, pages, limit and total are numbers');
+
+                done();
+            }).then(null, done);
+        });
+    });
 });