Skip to content

Commit

Permalink
Refactor old processOptions/where to use GQL JSON
Browse files Browse the repository at this point in the history
refs TryGhost#5943

- no longer assume the options in processOptions are set
- set where to a new GQL JSON-like statement object
- rather than setting options, add statements which can be understood by knexify
- pass the statements through knexify to build the query
  • Loading branch information
ErisDS committed Nov 12, 2015
1 parent 088dd24 commit 4dac01c
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 36 deletions.
23 changes: 12 additions & 11 deletions core/server/models/base/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -274,20 +274,16 @@ ghostBookshelf.Model = ghostBookshelf.Model.extend({
// Filter options so that only permitted ones remain
options = this.filterOptions(options, 'findPage');

// Extend the model defaults
options = _.defaults(options, this.findPageDefaultOptions());

// Run specific conversion of model query options to where options
options = this.processOptions(itemCollection, options);

// Ensure only valid fields/columns are added to query
if (options.columns) {
options.columns = _.intersection(options.columns, this.prototype.permittedAttributes());
}
// 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.
this.processOptions(_.defaults(options, this.findPageDefaultOptions()));

// If there are `where` conditionals specified, add those to the query.
if (options.where) {
itemCollection.query('where', options.where);
itemCollection.query(function (qb) {
gql.knexify(qb, options.where);
});
}

// Apply FILTER
Expand All @@ -304,6 +300,11 @@ ghostBookshelf.Model = ghostBookshelf.Model.extend({
// TODO: this should just be done for all methods @ the API level
options.withRelated = _.union(options.withRelated, options.include);

// Ensure only valid fields/columns are added to query
if (options.columns) {
options.columns = _.intersection(options.columns, this.prototype.permittedAttributes());
}

if (options.order) {
options.order = self.parseOrderOption(options.order);
} else {
Expand Down
27 changes: 22 additions & 5 deletions core/server/models/post.js
Original file line number Diff line number Diff line change
Expand Up @@ -333,8 +333,7 @@ Post = ghostBookshelf.Model.extend({
findPageDefaultOptions: function findPageDefaultOptions() {
return {
staticPages: false, // include static pages
status: 'published',
where: {}
status: 'published'
};
},

Expand All @@ -347,22 +346,40 @@ Post = ghostBookshelf.Model.extend({
};
},

processOptions: function processOptions(itemCollection, options) {
/**
* @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 !== 'all') {
// convert string true/false to boolean
if (!_.isBoolean(options.staticPages)) {
options.staticPages = _.contains(['true', '1'], options.staticPages);
}
options.where['posts.page'] = 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 !== 'all') {
// make sure that status is valid
options.status = _.contains(['published', 'draft'], options.status) ? options.status : 'published';
options.where['posts.status'] = options.status;
options.where.statements.push({prop: 'status', op: '=', value: options.status});
delete options.status;
} else {
options.where.statements.push({prop: 'status', op: 'IN', value: ['published', 'draft']});
delete options.status;
}

return options;
Expand Down
9 changes: 5 additions & 4 deletions core/server/models/tag.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,16 +59,17 @@ Tag = ghostBookshelf.Model.extend({
}
}, {
findPageDefaultOptions: function findPageDefaultOptions() {
return {
where: {}
};
return {};
},

orderDefaultOptions: function orderDefaultOptions() {
return {};
},

processOptions: function processOptions(itemCollection, options) {
/**
* @deprecated in favour of filter
*/
processOptions: function processOptions(options) {
return options;
},

Expand Down
41 changes: 25 additions & 16 deletions core/server/models/user.js
Original file line number Diff line number Diff line change
Expand Up @@ -166,9 +166,7 @@ User = ghostBookshelf.Model.extend({
}, {
findPageDefaultOptions: function findPageDefaultOptions() {
return {
status: 'active',
where: {},
whereIn: {}
status: 'active'
};
},

Expand All @@ -180,28 +178,39 @@ User = ghostBookshelf.Model.extend({
};
},

processOptions: function processOptions(itemCollection, options) {
// TODO: there are multiple statuses that make a user "active" or "invited" - we a way to translate/map them:
// TODO (cont'd from above): * valid "active" statuses: active, warn-1, warn-2, warn-3, warn-4, locked
// TODO (cont'd from above): * valid "invited" statuses" invited, invited-pending
/**
* @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 allStates = activeStates.concat(invitedStates),
value;

// Filter on the status. A status of 'all' translates to no filter since we want all statuses
if (options.status && options.status !== 'all') {
if (options.status !== 'all') {
// make sure that status is valid
// TODO: need a better way of getting a list of statuses other than hard-coding them...
options.status = _.indexOf(
['active', 'warn-1', 'warn-2', 'warn-3', 'warn-4', 'locked', 'invited', 'inactive'],
options.status) !== -1 ? options.status : 'active';
options.status = allStates.indexOf(options.status) > -1 ? options.status : 'active';
}

if (options.status === 'active') {
itemCollection.query().whereIn('status', activeStates);
value = activeStates;
} else if (options.status === 'invited') {
itemCollection.query().whereIn('status', invitedStates);
} else if (options.status !== 'all') {
options.where.status = options.status;
value = invitedStates;
} 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;
},

Expand Down

0 comments on commit 4dac01c

Please sign in to comment.