API refactor / cleanup

closes #1303

- removed where and orderBy from being passed from the API through to bookshelf, and ultimately knex
- ordering is now consistent across both front and backend, which fixes #1303
- validated / cleaned up all the API parameters
- added API tests for the status and staticPages parameters

core/client/router.js

@@ -36,7 +36,7 @@
         blog: function () {
             var posts = new Ghost.Collections.Posts();
             NProgress.start();
-            posts.fetch({ data: { status: 'all', orderBy: ['updated_at', 'DESC'], where: { page: 'all' } } }).then(function () {
+            posts.fetch({ data: { status: 'all', staticPages: 'all'} }).then(function () {
                 Ghost.currentView = new Ghost.Views.Blog({ el: '#main', collection: posts });
                 NProgress.done();
             });

core/client/views/blog.js

@@ -94,8 +94,7 @@
                 data: {
                     status: 'all',
                     page: (self.collection.currentPage + 1),
-                    where: { page: 'all' },
+                    staticPages: 'all'
-                    orderBy: ['updated_at', 'DESC']
                 }
             }).then(function onSuccess(response) {
                 /*jslint unparam:true*/

core/server/models/post.js

@@ -224,30 +224,44 @@ Post = ghostBookshelf.Model.extend({
      * @params opts
      */
     findPage: function (opts) {
-        var postCollection;
+        var postCollection,
+            permittedOptions = ['page', 'limit', 'status', 'staticPages'];
+
+        // sanitize opts
+        opts = _.pick(opts, permittedOptions);
 
         // Allow findPage(n)
         if (_.isString(opts) || _.isNumber(opts)) {
             opts = {page: opts};
         }
 
+        // Without this we are automatically passing through any and all query strings
+        // to Bookshelf / Knex. Although the API requires auth, we should prevent this
+        // until such time as we can design the API properly and safely.
+        opts.where = {};
+
         opts = _.extend({
-            page: 1,
+            page: 1, // pagination page
             limit: 15,
-            where: { page: false },
+            staticPages: false, // include static pages
-            status: 'published',
+            status: 'published'
-            orderBy: ['published_at', 'DESC']
         }, opts);
 
         postCollection = Posts.forge();
 
-        if (opts.where && opts.where.page === 'all') {
+        if (opts.staticPages !== 'all') {
-            delete opts.where.page;
+            // convert string true/false to boolean
+            if (!_.isBoolean(opts.staticPages)) {
+                opts.staticPages = opts.staticPages === 'true' || opts.staticPages === '1' ? true : false;
+            }
+            opts.where.page = opts.staticPages;
         }
 
         // Unless `all` is passed as an option, filter on
         // the status provided.
         if (opts.status !== 'all') {
+            // make sure that status is valid
+            opts.status = _.indexOf(['published', 'draft'], opts.status) !== -1 ? opts.status : 'published';
             opts.where.status = opts.status;
         }
 
@@ -266,8 +280,10 @@ Post = ghostBookshelf.Model.extend({
         return postCollection
             .query('limit', opts.limit)
             .query('offset', opts.limit * (opts.page - 1))
-            .query('orderBy', opts.orderBy[0], opts.orderBy[1])
+            .query('orderBy', 'status', 'ASC')
-            .fetch(_.omit(opts, 'page', 'limit', 'where', 'status', 'orderBy'))
+            .query('orderBy', 'updated_at', 'DESC')
+            .query('orderBy', 'published_at', 'DESC')
+            .fetch(_.omit(opts, 'page', 'limit'))
             .then(function (collection) {
                 var qb;
 

core/test/functional/api/posts_test.js

@@ -40,7 +40,9 @@ describe('Post API', function () {
             }, done);
     });
 
-    it('can retrieve all posts', function (done) {
+    // ## Browse
+
+    it('retrieves all published posts only by default', function (done) {
         request.get(testUtils.API.getApiURL('posts/'), function (error, response, body) {
             response.should.have.status(200);
             should.not.exist(response.headers['x-cache-invalidate']);
@@ -54,8 +56,68 @@ describe('Post API', function () {
         });
     });
 
+    it('can retrieve all published posts and pages', function (done) {
+        request.get(testUtils.API.getApiURL('posts/?staticPages=all'), function (error, response, body) {
+            response.should.have.status(200);
+            should.not.exist(response.headers['x-cache-invalidate']);
+            response.should.be.json;
+            var jsonResponse = JSON.parse(body);
+            jsonResponse.posts.should.exist;
+            testUtils.API.checkResponse(jsonResponse, 'posts');
+            jsonResponse.posts.should.have.length(6);
+            testUtils.API.checkResponse(jsonResponse.posts[0], 'post');
+            done();
+        });
+    });
+
+    // Test bits of the API we don't use in the app yet to ensure the API behaves properly
+
+    it('can retrieve all status posts and pages', function (done) {
+        request.get(testUtils.API.getApiURL('posts/?staticPages=all&status=all'), function (error, response, body) {
+            response.should.have.status(200);
+            should.not.exist(response.headers['x-cache-invalidate']);
+            response.should.be.json;
+            var jsonResponse = JSON.parse(body);
+            jsonResponse.posts.should.exist;
+            testUtils.API.checkResponse(jsonResponse, 'posts');
+            jsonResponse.posts.should.have.length(8);
+            testUtils.API.checkResponse(jsonResponse.posts[0], 'post');
+            done();
+        });
+    });
+
+    it('can retrieve just published pages', function (done) {
+        request.get(testUtils.API.getApiURL('posts/?staticPages=true'), function (error, response, body) {
+            response.should.have.status(200);
+            should.not.exist(response.headers['x-cache-invalidate']);
+            response.should.be.json;
+            var jsonResponse = JSON.parse(body);
+            jsonResponse.posts.should.exist;
+            testUtils.API.checkResponse(jsonResponse, 'posts');
+            jsonResponse.posts.should.have.length(1);
+            testUtils.API.checkResponse(jsonResponse.posts[0], 'post');
+            done();
+        });
+    });
+
+    it('can retrieve just draft posts', function (done) {
+        request.get(testUtils.API.getApiURL('posts/?status=draft'), function (error, response, body) {
+            response.should.have.status(200);
+            should.not.exist(response.headers['x-cache-invalidate']);
+            response.should.be.json;
+            var jsonResponse = JSON.parse(body);
+            jsonResponse.posts.should.exist;
+            testUtils.API.checkResponse(jsonResponse, 'posts');
+            jsonResponse.posts.should.have.length(1);
+            testUtils.API.checkResponse(jsonResponse.posts[0], 'post');
+            done();
+        });
+    });
+
+    // ## Read
+
     it('can retrieve a post', function (done) {
-        request.get(testUtils.API.getApiURL('posts/5/'), function (error, response, body) {
+        request.get(testUtils.API.getApiURL('posts/1/'), function (error, response, body) {
             response.should.have.status(200);
             should.not.exist(response.headers['x-cache-invalidate']);
             response.should.be.json;
@@ -68,7 +130,7 @@ describe('Post API', function () {
     });
 
     it('can retrieve a static page', function (done) {
-        request.get(testUtils.API.getApiURL('posts/6/'), function (error, response, body) {
+        request.get(testUtils.API.getApiURL('posts/7/'), function (error, response, body) {
             response.should.have.status(200);
             should.not.exist(response.headers['x-cache-invalidate']);
             response.should.be.json;
@@ -80,6 +142,44 @@ describe('Post API', function () {
         });
     });
 
+    it('can\'t retrieve non existent post', function (done) {
+        request.get(testUtils.API.getApiURL('posts/99/'), function (error, response, body) {
+            response.should.have.status(404);
+            should.not.exist(response.headers['x-cache-invalidate']);
+            response.should.be.json;
+            var jsonResponse = JSON.parse(body);
+            jsonResponse.should.exist;
+            testUtils.API.checkResponseValue(jsonResponse, ['error']);
+            done();
+        });
+    });
+
+    it('can\'t retrieve a draft post', function (done) {
+        request.get(testUtils.API.getApiURL('posts/5/'), function (error, response, body) {
+            response.should.have.status(404);
+            should.not.exist(response.headers['x-cache-invalidate']);
+            response.should.be.json;
+            var jsonResponse = JSON.parse(body);
+            jsonResponse.should.exist;
+            testUtils.API.checkResponseValue(jsonResponse, ['error']);
+            done();
+        });
+    });
+
+    it('can\'t retrieve a draft page', function (done) {
+        request.get(testUtils.API.getApiURL('posts/8/'), function (error, response, body) {
+            response.should.have.status(404);
+            should.not.exist(response.headers['x-cache-invalidate']);
+            response.should.be.json;
+            var jsonResponse = JSON.parse(body);
+            jsonResponse.should.exist;
+            testUtils.API.checkResponseValue(jsonResponse, ['error']);
+            done();
+        });
+    });
+
+    // ## Add
+
     it('can create a new draft, publish post, update post', function (done) {
         var newTitle = 'My Post',
             changedTitle = 'My Post changed',
@@ -121,18 +221,7 @@ describe('Post API', function () {
         });
     });
 
-
+    // ## edit
-    it('can\'t retrieve non existent post', function (done) {
-        request.get(testUtils.API.getApiURL('posts/99/'), function (error, response, body) {
-            response.should.have.status(404);
-            should.not.exist(response.headers['x-cache-invalidate']);
-            response.should.be.json;
-            var jsonResponse = JSON.parse(body);
-            jsonResponse.should.exist;
-            testUtils.API.checkResponseValue(jsonResponse, ['error']);
-            done();
-        });
-    });
 
     it('can edit a post', function (done) {
         request.get(testUtils.API.getApiURL('posts/1/'), function (error, response, body) {
@@ -156,6 +245,8 @@ describe('Post API', function () {
         });
     });
 
+    // ## delete
+
     it('can\'t edit non existent post', function (done) {
         request.get(testUtils.API.getApiURL('posts/1/'), function (error, response, body) {
             var jsonResponse = JSON.parse(body),
@@ -230,6 +321,4 @@ describe('Post API', function () {
             });
         });
     });
-
-
 });

core/test/integration/model/model_posts_spec.js

@@ -116,7 +116,7 @@ describe('Post Model', function () {
         }).then(null, done);
     });
 
-    it('can add, defaulting as a draft', function (done) {
+    it('can add, defaults are all correct', function (done) {
         var createdPostUpdatedDate,
             newPost = testUtils.DataGenerator.forModel.posts[2],
             newPostDB = testUtils.DataGenerator.Content.posts[2];
@@ -132,6 +132,14 @@ describe('Post Model', function () {
             createdPost.has('html').should.equal(true);
             createdPost.get('html').should.equal(newPostDB.html);
             createdPost.get('slug').should.equal(newPostDB.slug + '-2');
+            (!!createdPost.get('featured')).should.equal(false);
+            (!!createdPost.get('page')).should.equal(false);
+            createdPost.get('language').should.equal('en_US');
+            // testing for nulls
+            (createdPost.get('image') === null).should.equal(true);
+            (createdPost.get('meta_title') === null).should.equal(true);
+            (createdPost.get('meta_description') === null).should.equal(true);
+
             createdPost.get('created_at').should.be.above(new Date(0).getTime());
             createdPost.get('created_by').should.equal(1);
             createdPost.get('author_id').should.equal(1);
@@ -157,6 +165,24 @@ describe('Post Model', function () {
 
     });
 
+    it('can add, with previous published_at date', function (done) {
+        var previousPublishedAtDate = new Date(2013, 8, 21, 12);
+
+        PostModel.add({
+            status: 'published',
+            published_at: previousPublishedAtDate,
+            title: 'published_at test',
+            markdown: 'This is some content'
+        }).then(function (newPost) {
+
+            should.exist(newPost);
+            new Date(newPost.get('published_at')).getTime().should.equal(previousPublishedAtDate.getTime());
+
+            done();
+
+        }).otherwise(done);
+    });
+
     it('can trim title', function (done) {
         var untrimmedCreateTitle = '  test trimmed create title  ',
             untrimmedUpdateTitle = '  test trimmed update title  ',
@@ -238,7 +264,7 @@ describe('Post Model', function () {
         PostModel.add(newPost).then(function (createdPost) {
             createdPost.get('slug').should.equal('bhute-dhddkii-bhrvnnaaraa-aahet');
             done();
-        })
+        });
     });
 
     it('detects duplicate slugs before saving', function (done) {
@@ -312,24 +338,6 @@ describe('Post Model', function () {
         }).then(null, done);
     });
 
-    it('can create a new Post with a previous published_at date', function (done) {
-        var previousPublishedAtDate = new Date(2013, 8, 21, 12);
-
-        PostModel.add({
-            status: 'published',
-            published_at: previousPublishedAtDate,
-            title: 'published_at test',
-            markdown: 'This is some content'
-        }).then(function (newPost) {
-
-            should.exist(newPost);
-            //newPost.get('published_at').should.equal(previousPublishedAtDate.getTime());
-
-            done();
-
-        }).otherwise(done);
-    });
-
     it('can fetch a paginated set, with various options', function (done) {
         testUtils.insertMorePosts().then(function () {
 
@@ -354,12 +362,12 @@ describe('Post Model', function () {
             paginationResult.posts.length.should.equal(30);
             paginationResult.pages.should.equal(2);
 
-            return PostModel.findPage({limit: 10, page: 2, where: {language: 'fr'}});
+            return PostModel.findPage({limit: 10, staticPages: true});
         }).then(function (paginationResult) {
-            paginationResult.page.should.equal(2);
+            paginationResult.page.should.equal(1);
             paginationResult.limit.should.equal(10);
-            paginationResult.posts.length.should.equal(10);
+            paginationResult.posts.length.should.equal(1);
-            paginationResult.pages.should.equal(3);
+            paginationResult.pages.should.equal(1);
 
             return PostModel.findPage({limit: 10, page: 2, status: 'all'});
         }).then(function (paginationResult) {

core/test/utils/fixtures/data-generator.js

@@ -12,7 +12,7 @@ DataGenerator.Content = {
         {
             title: "Kitchen Sink",
             slug: "kitchen-sink",
-            markdown: "<h1>HTML Ipsum Presents</h1><p><strong>Pellentesque habitant morbi tristique</strong> senectus et netus et malesuada fames ac turpis egestas. Vestibulum tortor quam, feugiat vitae, ultricies eget, tempor sit amet, ante. Donec eu libero sit amet quam egestas semper. <em>Aenean ultricies mi vitae est.</em> Mauris placerat eleifend leo. Quisque sit amet est et sapien ullamcorper pharetra. Vestibulum erat wisi, condimentum sed, <code>commodo vitae</code>, ornare sit amet, wisi. Aenean fermentum, elit eget tincidunt condimentum, eros ipsum rutrum orci, sagittis tempus lacus enim ac dui. <a href=\"#\">Donec non enim</a> in turpis pulvinar facilisis. Ut felis.</p><h2>Header Level 2</h2><ol><li>Lorem ipsum dolor sit amet, consectetuer adipiscing elit.</li><li>Aliquam tincidunt mauris eu risus.</li></ol><blockquote><p>Lorem ipsum dolor sit amet, consectetur adipiscing elit. Vivamus magna. Cras in mi at felis aliquet congue. Ut a est eget ligula molestie gravida. Curabitur massa. Donec eleifend, libero at sagittis mollis, tellus est malesuada tellus, at luctus turpis elit sit amet quam. Vivamus pretium ornare est.</p></blockquote><h3>Header Level 3</h3><ul><li>Lorem ipsum dolor sit amet, consectetuer adipiscing elit.</li><li>Aliquam tincidunt mauris eu risus.</li></ul><pre><code>#header h1 a{display: block;width: 300px;height: 80px;}</code></pre>",
+            markdown: "<h1>HTML Ipsum Presents</h1><p><strong>Pellentesque habitant morbi tristique</strong> senectus et netus et malesuada fames ac turpis egestas. Vestibulum tortor quam, feugiat vitae, ultricies eget, tempor sit amet, ante. Donec eu libero sit amet quam egestas semper. <em>Aenean ultricies mi vitae est.</em> Mauris placerat eleifend leo. Quisque sit amet est et sapien ullamcorper pharetra. Vestibulum erat wisi, condimentum sed, <code>commodo vitae</code>, ornare sit amet, wisi. Aenean fermentum, elit eget tincidunt condimentum, eros ipsum rutrum orci, sagittis tempus lacus enim ac dui. <a href=\"#\">Donec non enim</a> in turpis pulvinar facilisis. Ut felis.</p><h2>Header Level 2</h2><ol><li>Lorem ipsum dolor sit amet, consectetuer adipiscing elit.</li><li>Aliquam tincidunt mauris eu risus.</li></ol><blockquote><p>Lorem ipsum dolor sit amet, consectetur adipiscing elit. Vivamus magna. Cras in mi at felis aliquet congue. Ut a est eget ligula molestie gravida. Curabitur massa. Donec eleifend, libero at sagittis mollis, tellus est malesuada tellus, at luctus turpis elit sit amet quam. Vivamus pretium ornare est.</p></blockquote><h3>Header Level 3</h3><ul><li>Lorem ipsum dolor sit amet, consectetuer adipiscing elit.</li><li>Aliquam tincidunt mauris eu risus.</li></ul><pre><code>#header h1 a{display: block;width: 300px;height: 80px;}</code></pre>"
         },
         {
             title: "Short and Sweet",
@@ -20,6 +20,12 @@ DataGenerator.Content = {
             markdown: "## testing\n\nmctesters\n\n- test\n- line\n- items",
             html: "<h2 id=\"testing\">testing</h2>\n\n<p>mctesters</p>\n\n<ul>\n<li>test</li>\n<li>line</li>\n<li>items</li>\n</ul>"
         },
+        {
+            title: "Not finished yet",
+            slug: "unfinished",
+            markdown: "<h1>HTML Ipsum Presents</h1><p><strong>Pellentesque habitant morbi tristique</strong> senectus et netus et malesuada fames ac turpis egestas. Vestibulum tortor quam, feugiat vitae, ultricies eget, tempor sit amet, ante. Donec eu libero sit amet quam egestas semper. <em>Aenean ultricies mi vitae est.</em> Mauris placerat eleifend leo. Quisque sit amet est et sapien ullamcorper pharetra. Vestibulum erat wisi, condimentum sed, <code>commodo vitae</code>, ornare sit amet, wisi. Aenean fermentum, elit eget tincidunt condimentum, eros ipsum rutrum orci, sagittis tempus lacus enim ac dui. <a href=\"#\">Donec non enim</a> in turpis pulvinar facilisis. Ut felis.</p><h2>Header Level 2</h2><ol><li>Lorem ipsum dolor sit amet, consectetuer adipiscing elit.</li><li>Aliquam tincidunt mauris eu risus.</li></ol><blockquote><p>Lorem ipsum dolor sit amet, consectetur adipiscing elit. Vivamus magna. Cras in mi at felis aliquet congue. Ut a est eget ligula molestie gravida. Curabitur massa. Donec eleifend, libero at sagittis mollis, tellus est malesuada tellus, at luctus turpis elit sit amet quam. Vivamus pretium ornare est.</p></blockquote><h3>Header Level 3</h3><ul><li>Lorem ipsum dolor sit amet, consectetuer adipiscing elit.</li><li>Aliquam tincidunt mauris eu risus.</li></ul><pre><code>#header h1 a{display: block;width: 300px;height: 80px;}</code></pre>",
+            status: "draft"
+        },
         {
             title: "Not so short, bit complex",
             slug: "not-so-short-bit-complex",
@@ -30,6 +36,13 @@ DataGenerator.Content = {
             slug: "static-page-test",
             markdown: "<h1>Static page test is what this is for.</h1><p>Hopefully you don't find it a bore.</p>",
             page: 1
+        },
+        {
+            title: "This is a draft static page",
+            slug: "static-page-draft",
+            markdown: "<h1>Static page test is what this is for.</h1><p>Hopefully you don't find it a bore.</p>",
+            page: 1,
+            status: "draft"
         }
     ],
 
@@ -166,7 +179,9 @@ DataGenerator.forKnex = (function () {
         createPost(DataGenerator.Content.posts[1]),
         createPost(DataGenerator.Content.posts[2]),
         createPost(DataGenerator.Content.posts[3]),
-        createPost(DataGenerator.Content.posts[4])
+        createPost(DataGenerator.Content.posts[4]),
+        createPost(DataGenerator.Content.posts[5]),
+        createPost(DataGenerator.Content.posts[6])
     ];
 
     tags = [