Replaced options.where GQL statements with filter notation (#10160)

refs #10105

- `options.where` is an older deprecated logic
- before the filter language was invented, Ghost generates statements for knex
- if we want to replace GQL with NQL, we can't generate these statements
- they are not understood from NQL, because NQL uses mongo JSON
- go through usages and rewrite the statements
- invent `extraFilters` for now
- we need to keep the support for `status` or `staticPages` for now (API requirement)
- IMO both shortcuts in the extra filters should be removed in the future

This commit is required for #10159!

core/server/models/base/index.js

@@ -646,11 +646,6 @@ ghostBookshelf.Model = ghostBookshelf.Model.extend({
         var options = this.filterOptions(unfilteredOptions, 'findAll'),
             itemCollection = this.forge();
 
-        // transforms fictive keywords like 'all' (status:all) into correct allowed values
-        if (this.processOptions) {
-            this.processOptions(options);
-        }
-
         // @TODO: we can't use order raw when running migrations (see https://github.com/tgriesser/knex/issues/2763)
         if (this.orderDefaultRaw && !options.migrating) {
             itemCollection.query((qb) => {
@@ -702,13 +697,6 @@ ghostBookshelf.Model = ghostBookshelf.Model.extend({
         // Set this to true or pass ?debug=true as an API option to get output
         itemCollection.debug = options.debug && config.get('env') !== 'production';
 
-        // This applies default properties like 'staticPages' and 'status'
-        // And then converts them to 'where' options... this behaviour is effectively deprecated in favour
-        // of using filter - it's only be being kept here so that we can transition cleanly.
-        if (this.processOptions) {
-            this.processOptions(options);
-        }
-
         // Add Filter behaviour
         itemCollection.applyDefaultAndCustomFilters(options);
 

core/server/models/invite.js

@@ -24,10 +24,6 @@ Invite = ghostBookshelf.Model.extend({
         return {};
     },
 
-    processOptions: function processOptions(options) {
-        return options;
-    },
-
     add: function add(data, unfilteredOptions) {
         const options = Invite.filterOptions(unfilteredOptions, 'add');
         data = data || {};

core/server/models/plugins/filter.js

@@ -83,6 +83,8 @@ filter = function filter(Bookshelf) {
         },
         defaultFilters: function defaultFilters() {
         },
+        extraFilters: function extraFilters() {
+        },
 
         preProcessFilters: function preProcessFilters() {
             this._filters.statements = gql.json.replaceStatements(this._filters.statements, {prop: /primary_tag/}, function (statement) {
@@ -175,7 +177,7 @@ filter = function filter(Bookshelf) {
                 this.enforcedFilters(options),
                 this.defaultFilters(options),
                 options.filter,
-                options.where
+                this.extraFilters(options)
             );
 
             return this;

core/server/models/post.js

@@ -11,6 +11,7 @@ const config = require('../config');
 const converters = require('../lib/mobiledoc/converters');
 const relations = require('./relations');
 const MOBILEDOC_REVISIONS_COUNT = 10;
+const ALL_STATUSES = ['published', 'draft', 'scheduled'];
 
 let Post;
 let Posts;
@@ -517,6 +518,51 @@ Post = ghostBookshelf.Model.extend({
         }
 
         return options.context && options.context.public ? 'page:false' : 'page:false+status:published';
+    },
+
+    /**
+     * You can pass an extra `status=VALUES` or "staticPages" field.
+     * Long-Term: We should deprecate these short cuts and force users to use the filter param.
+     */
+    extraFilters: function extraFilters(options) {
+        if (!options.staticPages && !options.status) {
+            return null;
+        }
+
+        let filter = null;
+
+        // CASE: "staticPages" is passed
+        if (options.staticPages && options.staticPages !== 'all') {
+            // CASE: convert string true/false to boolean
+            if (!_.isBoolean(options.staticPages)) {
+                options.staticPages = _.includes(['true', '1'], options.staticPages);
+            }
+
+            filter = `page:${options.staticPages}`;
+        } else if (options.staticPages === 'all') {
+            filter = 'page:[true, false]';
+        }
+
+        // CASE: "status" is passed, combine filters
+        if (options.status && options.status !== 'all') {
+            options.status = _.includes(ALL_STATUSES, options.status) ? options.status : 'published';
+
+            if (!filter) {
+                filter = `status:${options.status}`;
+            } else {
+                filter = `${filter}+status:${options.status}`;
+            }
+        } else if (options.status === 'all') {
+            if (!filter) {
+                filter = `status:[${ALL_STATUSES}]`;
+            } else {
+                filter = `${filter}+status:[${ALL_STATUSES}]`;
+            }
+        }
+
+        delete options.status;
+        delete options.staticPages;
+        return filter;
     }
 }, {
     allowedFormats: ['mobiledoc', 'html', 'plaintext'],
@@ -547,45 +593,6 @@ Post = ghostBookshelf.Model.extend({
         return order;
     },
 
-    /**
-     * @deprecated in favour of filter
-     */
-    processOptions: function processOptions(options) {
-        if (!options.staticPages && !options.status) {
-            return options;
-        }
-
-        // This is the only place that 'options.where' is set now
-        options.where = {statements: []};
-
-        // Step 4: Setup filters (where clauses)
-        if (options.staticPages && options.staticPages !== 'all') {
-            // convert string true/false to boolean
-            if (!_.isBoolean(options.staticPages)) {
-                options.staticPages = _.includes(['true', '1'], options.staticPages);
-            }
-            options.where.statements.push({prop: 'page', op: '=', value: options.staticPages});
-            delete options.staticPages;
-        } else if (options.staticPages === 'all') {
-            options.where.statements.push({prop: 'page', op: 'IN', value: [true, false]});
-            delete options.staticPages;
-        }
-
-        // Unless `all` is passed as an option, filter on
-        // the status provided.
-        if (options.status && options.status !== 'all') {
-            // make sure that status is valid
-            options.status = _.includes(['published', 'draft', 'scheduled'], options.status) ? options.status : 'published';
-            options.where.statements.push({prop: 'status', op: '=', value: options.status});
-            delete options.status;
-        } else if (options.status === 'all') {
-            options.where.statements.push({prop: 'status', op: 'IN', value: ['published', 'draft', 'scheduled']});
-            delete options.status;
-        }
-
-        return options;
-    },
-
     /**
      * Returns an array of keys permitted in a method's `options` hash, depending on the current method.
      * @param {String} methodName The name of the method to check valid options for.

core/server/models/subscriber.js

@@ -35,12 +35,6 @@ Subscriber = ghostBookshelf.Model.extend({
     orderDefaultOptions: function orderDefaultOptions() {
         return {};
     },
-    /**
-     * @deprecated in favour of filter
-     */
-    processOptions: function processOptions(options) {
-        return options;
-    },
 
     permittedOptions: function permittedOptions(methodName) {
         var options = ghostBookshelf.Model.permittedOptions.call(this, methodName),

core/server/models/tag.js

@@ -75,13 +75,6 @@ Tag = ghostBookshelf.Model.extend({
         return {};
     },
 
-    /**
-     * @deprecated in favour of filter
-     */
-    processOptions: function processOptions(options) {
-        return options;
-    },
-
     permittedOptions: function permittedOptions(methodName) {
         var options = ghostBookshelf.Model.permittedOptions.call(this, methodName),
 

core/server/models/user.js

@@ -278,47 +278,43 @@ User = ghostBookshelf.Model.extend({
         }
 
         return options.context && options.context.public ? null : 'status:[' + allStates.join(',') + ']';
-    }
-}, {
-    orderDefaultOptions: function orderDefaultOptions() {
-        return {
-            last_seen: 'DESC',
-            name: 'ASC',
-            created_at: 'DESC'
-        };
     },
 
     /**
-     * @deprecated in favour of filter
+     * You can pass an extra `status=VALUES` field.
+     * Long-Term: We should deprecate these short cuts and force users to use the filter param.
      */
-    processOptions: function processOptions(options) {
+    extraFilters: function extraFilters(options) {
         if (!options.status) {
-            return options;
+            return null;
         }
 
-        // This is the only place that 'options.where' is set now
-        options.where = {statements: []};
+        let filter = null;
 
-        var value;
-
-        // Filter on the status.  A status of 'all' translates to no filter since we want all statuses
+        // CASE: Check if the incoming status value is valid, otherwise fallback to "active"
         if (options.status !== 'all') {
-            // make sure that status is valid
             options.status = allStates.indexOf(options.status) > -1 ? options.status : 'active';
         }
 
         if (options.status === 'active') {
-            value = activeStates;
+            filter = `status:[${activeStates}]`;
         } else if (options.status === 'all') {
-            value = allStates;
+            filter = `status:[${allStates}]`;
         } else {
-            value = options.status;
+            filter = `status:${options.status}`;
         }
 
-        options.where.statements.push({prop: 'status', op: 'IN', value: value});
         delete options.status;
 
-        return options;
+        return filter;
+    }
+}, {
+    orderDefaultOptions: function orderDefaultOptions() {
+        return {
+            last_seen: 'DESC',
+            name: 'ASC',
+            created_at: 'DESC'
+        };
     },
 
     /**

core/test/unit/models/plugins/filter_spec.js

@@ -109,9 +109,9 @@ describe('Filter', function () {
             });
 
             it('should call combineFilters with old-style custom filters if set', function () {
-                var result = ghostBookshelf.Model.prototype.fetchAndCombineFilters({
-                    where: 'author:cameron'
-                });
+                sandbox.stub(ghostBookshelf.Model.prototype, 'extraFilters').returns('author:cameron');
+
+                const result = ghostBookshelf.Model.prototype.fetchAndCombineFilters({});
 
                 filterUtils.combineFilters.calledOnce.should.be.true();
                 filterUtils.combineFilters.firstCall.args.should.eql([undefined, undefined, undefined, 'author:cameron']);
@@ -135,19 +135,17 @@ describe('Filter', function () {
             });
 
             it('should call combineFilters with all values if set', function () {
-                var filterSpy = sandbox.stub(ghostBookshelf.Model.prototype, 'enforcedFilters')
-                        .returns('status:published'),
-                    filterSpy2 = sandbox.stub(ghostBookshelf.Model.prototype, 'defaultFilters')
-                        .returns('page:false'),
-                    result;
+                sandbox.stub(ghostBookshelf.Model.prototype, 'defaultFilters').returns('page:false');
+                sandbox.stub(ghostBookshelf.Model.prototype, 'enforcedFilters').returns('status:published');
+                sandbox.stub(ghostBookshelf.Model.prototype, 'extraFilters').returns('author:cameron');
 
-                result = ghostBookshelf.Model.prototype.fetchAndCombineFilters({
-                    filter: 'tag:photo',
-                    where: 'author:cameron'
+                const result = ghostBookshelf.Model.prototype.fetchAndCombineFilters({
+                    filter: 'tag:photo'
                 });
 
-                filterSpy.calledOnce.should.be.true();
-                filterSpy2.calledOnce.should.be.true();
+                ghostBookshelf.Model.prototype.enforcedFilters.calledOnce.should.be.true();
+                ghostBookshelf.Model.prototype.defaultFilters.calledOnce.should.be.true();
+
                 filterUtils.combineFilters.calledOnce.should.be.true();
                 filterUtils.combineFilters.firstCall.args
                     .should.eql(['status:published', 'page:false', 'tag:photo', 'author:cameron']);

core/test/unit/models/post_spec.js

@@ -272,22 +272,16 @@ describe('Unit: models/post', function () {
         });
     });
 
-    describe('processOptions', function () {
+    describe('extraFilters', function () {
         it('generates correct where statement when filter contains unpermitted values', function () {
             const options = {
                 filter: 'status:[published,draft]',
                 limit: 'all',
                 status: 'published'
             };
 
-            models.Post.processOptions(options);
-
-            options.where.statements.should.be.an.Array().with.lengthOf(1);
-            options.where.statements[0].should.deepEqual({
-                prop: 'status',
-                op: '=',
-                value: 'published'
-            });
+            const filter = new models.Post().extraFilters(options);
+            filter.should.eql('status:published');
         });
     });