Infrastructure/ghost
0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2025-03-25 02:31:59 -05:00
Code Issues Activity

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

Browse source 
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 https://github.com/TryGhost/Ghost/pull/10159!
This commit is contained in:
Katharina Irrgang 2018-11-15 15:53:24 +01:00 • committed by GitHub
parent 2e81852b22
commit e89a27f3ab
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 92 additions and 124 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

12
core/server/models/base/index.js
View file

@ -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);

4
core/server/models/invite.js
View file

@ -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 || {};

4
core/server/models/plugins/filter.js
View file

@ -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;

85
core/server/models/post.js
View file

@ -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.

6
core/server/models/subscriber.js
View file

@ -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),

7
core/server/models/tag.js
View file

@ -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),

62
core/server/models/user.js
View file

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

24
core/test/unit/models/plugins/filter_spec.js
View file

@ -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']);

12
core/test/unit/models/post_spec.js
View file

@ -272,7 +272,7 @@ 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]',
@ -280,14 +280,8 @@ describe('Unit: models/post', function () {
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');
});
});