Skip to content

Commit

Permalink
✨ Support filtering based on primary_tag (#9124)
Browse files Browse the repository at this point in the history 
closes #8668, refs #8920

- Updated tests to include internal tags
  - Tests had no example of an internal tag
  - Need this to show that the new filtering works as expected
- primary_tag is a calculated field
- This ensures that we can alias the field to equivalent logic in API filters
- By replacing primary_tag by a lookup based on a tag which has order 0
- bump ghost-gql to 0.0.8

**NOTE:**
Until GQL is refactored, there are limitations on what else can be filtered when using primary_tag in a filter e.g. it wont be possible to do a filter based on primary_tag AND/OR other tag filters.
  • Loading branch information
@ErisDS @kirrg001
ErisDS authored and kirrg001 committed Oct 10, 2017
1 parent 2941932 commit 7999c38
Show file tree
Hide file tree
Showing 7 changed files with 158 additions and 25 deletions.
19 changes: 16 additions & 3 deletions core/server/models/plugins/filter.js
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
var _ = require('lodash'),
var _ = require('lodash'),
errors = require('../../errors'),
gql = require('ghost-gql'),
i18n = require('../../i18n'),
gql = require('ghost-gql'),
i18n = require('../../i18n'),
filter,
filterUtils;

Expand Down Expand Up @@ -83,6 +83,17 @@ filter = function filter(Bookshelf) {
enforcedFilters: function enforcedFilters() {},
defaultFilters: function defaultFilters() {},

preProcessFilters: function preProcessFilters() {
this._filters.statements = gql.json.replaceStatements(this._filters.statements, {prop: /primary_tag/}, function (statement) {
statement.prop = 'tags.slug';
return {group: [
statement,
{prop: 'posts_tags.sort_order', op: '=', value: 0},
{prop: 'tags.visibility', op: '=', value: 'public'}
]};
});
},

/**
* ## Post process Filters
* Post Process filters looking for joins etc
Expand Down Expand Up @@ -160,6 +171,8 @@ filter = function filter(Bookshelf) {
gql.json.printStatements(this._filters.statements);
}

this.preProcessFilters(options);

this.query(function (qb) {
gql.knexify(qb, self._filters);
});
Expand Down
115 changes: 108 additions & 7 deletions core/test/integration/api/advanced_browse_spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -154,11 +154,11 @@ describe('Advanced Browse', function () {
});
});

describe('4. Posts - filter="author:[leslie,pat]+(tag:audio,image:-null)"', function () {
describe('4. Posts - filter="author:[leslie,pat]+(tag:hash-audio,image:-null)"', function () {
// Note that `pat` doesn't exist (it's `pat-smith`)
it('Will fetch posts by the author `leslie` or `pat` which are either have tag `audio` or an image.', function (done) {
it('Will fetch posts by the author `leslie` or `pat` which are either have tag `hash-audio` or an image.', function (done) {
PostAPI.browse({
filter: 'author:[leslie,pat]+(tag:audio,feature_image:-null)',
filter: 'author:[leslie,pat]+(tag:hash-audio,feature_image:-null)',
include: 'author,tags'
}).then(function (result) {
var ids, authors;
Expand All @@ -177,15 +177,15 @@ describe('Advanced Browse', function () {
});
authors.should.matchAny(/leslie|pat/);

// Each post must either be featured or have the tag 'audio'
// Each post must either be featured or have the tag 'hash-audio'
_.each(result.posts, function (post) {
var tags = _.map(post.tags, 'slug');
// This construct ensures we get an assertion or a failure
if (!_.isEmpty(post.feature_image)) {
post.feature_image.should.not.be.empty();
} else {
tags = _.map(post.tags, 'slug');
tags.should.containEql('audio');
tags.should.containEql('hash-audio');
}
});

Expand Down Expand Up @@ -365,6 +365,107 @@ describe('Advanced Browse', function () {
});
});

describe('Primary Tags', function () {
it('Will fetch posts which have a primary tag of photo', function (done) {
PostAPI.browse({
filter: 'primary_tag:photo',
include: 'tags'
}).then(function (result) {
var ids;

// 1. Result should have the correct base structure
should.exist(result);
result.should.have.property('posts');
result.should.have.property('meta');

// 2. The data part of the response should be correct
// We should have 5 matching items
result.posts.should.be.an.Array().with.lengthOf(4);

ids = _.map(result.posts, 'id');
ids.should.eql([
testUtils.filterData.data.posts[10].id,
testUtils.filterData.data.posts[8].id,
testUtils.filterData.data.posts[2].id,
testUtils.filterData.data.posts[1].id
]);

_.each(result.posts, function (post) {
post.page.should.be.false();
post.status.should.eql('published');
});

// 3. The meta object should contain the right details
result.meta.should.have.property('pagination');
result.meta.pagination.should.be.an.Object().with.properties(['page', 'limit', 'pages', 'total', 'next', 'prev']);
result.meta.pagination.page.should.eql(1);
result.meta.pagination.limit.should.eql(15);
result.meta.pagination.pages.should.eql(1);
result.meta.pagination.total.should.eql(4);
should.equal(result.meta.pagination.next, null);
should.equal(result.meta.pagination.prev, null);

done();
}).catch(done);
});

it('Will fetch empty list if no post has matching primary-tag', function (done) {
PostAPI.browse({
filter: 'primary_tag:no-posts',
include: 'tags'
}).then(function (result) {
// 1. Result should have the correct base structure
should.exist(result);
result.should.have.property('posts');
result.should.have.property('meta');

// 2. The data part of the response should be correct
// We should have 5 matching items
result.posts.should.be.an.Array().with.lengthOf(0);

// 3. The meta object should contain the right details
result.meta.should.have.property('pagination');
result.meta.pagination.should.be.an.Object().with.properties(['page', 'limit', 'pages', 'total', 'next', 'prev']);
result.meta.pagination.page.should.eql(1);
result.meta.pagination.limit.should.eql(15);
result.meta.pagination.pages.should.eql(1);
result.meta.pagination.total.should.eql(0);
should.equal(result.meta.pagination.next, null);
should.equal(result.meta.pagination.prev, null);

done();
}).catch(done);
});

it('Will fetch empty list if primary_tag is internal', function (done) {
PostAPI.browse({
filter: 'primary_tag:no-posts',
include: 'tags'
}).then(function (result) {
// 1. Result should have the correct base structure
should.exist(result);
result.should.have.property('posts');
result.should.have.property('meta');

// 2. The data part of the response should be correct
// We should have 5 matching items
result.posts.should.be.an.Array().with.lengthOf(0);

// 3. The meta object should contain the right details
result.meta.should.have.property('pagination');
result.meta.pagination.should.be.an.Object().with.properties(['page', 'limit', 'pages', 'total', 'next', 'prev']);
result.meta.pagination.page.should.eql(1);
result.meta.pagination.limit.should.eql(15);
result.meta.pagination.pages.should.eql(1);
result.meta.pagination.total.should.eql(0);
should.equal(result.meta.pagination.next, null);
should.equal(result.meta.pagination.prev, null);

done();
}).catch(done);
});
});

describe('Count capabilities', function () {
it('can fetch `count.posts` for tags (public data only)', function (done) {
TagAPI.browse({include: 'count.posts'}).then(function (result) {
Expand All @@ -391,7 +492,7 @@ describe('Advanced Browse', function () {
}).count.posts.should.eql(5);

_.find(result.tags, function (tag) {
return tag.name === 'Audio';
return tag.name === '#Audio';
}).count.posts.should.eql(6);

_.find(result.tags, function (tag) {
Expand Down Expand Up @@ -443,7 +544,7 @@ describe('Advanced Browse', function () {
}).count.posts.should.eql(5);

_.find(result.tags, function (tag) {
return tag.name === 'Audio';
return tag.name === '#Audio';
}).count.posts.should.eql(6);

_.find(result.tags, function (tag) {
Expand Down
9 changes: 7 additions & 2 deletions core/test/unit/models_plugins/filter_spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,8 @@ describe('Filter', function () {
fetchSpy = sandbox.stub(ghostBookshelf.Model.prototype, 'fetchAndCombineFilters');
filterGQL.knexify = sandbox.stub();
filterGQL.json = {
printStatements: sandbox.stub()
printStatements: sandbox.stub(),
replaceStatements: sandbox.stub().returnsArg(0)
};

restoreGQL = filter.__set__('gql', filterGQL);
Expand Down Expand Up @@ -226,8 +227,12 @@ describe('Filter', function () {
});
});

describe('Pre Process Filters', function () {
it('should not have tests yet, as this needs to be removed');
});

describe('Post Process Filters', function () {
it('should not have tests yet, as this is about to be removed');
it('should not have tests yet, as this needs to be removed');
});
});

Expand Down
25 changes: 19 additions & 6 deletions core/test/utils/fixtures/data-generator.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,19 @@ DataGenerator.forKnex = (function () {
});
}

function createTag(overrides) {
var newObj = _.cloneDeep(overrides);

return _.defaults(newObj, {
id: ObjectId.generate(),
visibility: 'public',
created_by: DataGenerator.Content.users[0].id,
created_at: new Date(),
updated_by: DataGenerator.Content.users[0].id,
updated_at: new Date()
});
}

function createPost(overrides) {
var newObj = _.cloneDeep(overrides),
mobiledoc = JSON.parse(overrides.mobiledoc || '{}');
Expand Down Expand Up @@ -521,11 +534,11 @@ DataGenerator.forKnex = (function () {
];

tags = [
createBasic(DataGenerator.Content.tags[0]),
createBasic(DataGenerator.Content.tags[1]),
createBasic(DataGenerator.Content.tags[2]),
createBasic(DataGenerator.Content.tags[3]),
createBasic(DataGenerator.Content.tags[4])
createTag(DataGenerator.Content.tags[0]),
createTag(DataGenerator.Content.tags[1]),
createTag(DataGenerator.Content.tags[2]),
createTag(DataGenerator.Content.tags[3]),
createTag(DataGenerator.Content.tags[4])
];

roles = [
Expand Down Expand Up @@ -616,7 +629,7 @@ DataGenerator.forKnex = (function () {
return {
createPost: createPost,
createGenericPost: createGenericPost,
createTag: createBasic,
createTag: createTag,
createUser: createUser,
createClient: createClient,
createGenericUser: createGenericUser,
Expand Down
7 changes: 4 additions & 3 deletions core/test/utils/fixtures/filter-param/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,11 @@ data.tags = [
},
{
id: ObjectId.generate(),
name: 'Audio',
slug: 'audio',
name: '#Audio',
slug: 'hash-audio',
feature_image: 'some/image/path.jpg',
description: 'Audio posts',
visibility: 'internal',
created_by: data.users[0].id
},
{
Expand Down Expand Up @@ -308,7 +309,7 @@ function createUsers(knex, DataGenerator) {

function createTags(knex, DataGenerator) {
data.tags = _.map(data.tags, function (tag) {
return DataGenerator.forKnex.createBasic(tag);
return DataGenerator.forKnex.createTag(tag);
});

// Next, insert it into the database & return the correctly indexed data
Expand Down
2 changes: 1 addition & 1 deletion package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
"express-hbs": "1.0.4",
"extract-zip-fork": "1.5.1",
"fs-extra": "3.0.1",
"ghost-gql": "0.0.6",
"ghost-gql": "0.0.8",
"ghost-ignition": "2.8.14",
"ghost-storage-base": "0.0.1",
"glob": "5.0.15",
Expand Down
6 changes: 3 additions & 3 deletions yarn.lock
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1745,9 +1745,9 @@ getsetdeep@~2.0.0:
dependencies:
typechecker "~2.0.1"

ghost-gql@0.0.6:
version "0.0.6"
resolved "https://registry.yarnpkg.com/ghost-gql/-/ghost-gql-0.0.6.tgz#be811bc95f8f72671009c33100fc85d2d02758ee"
ghost-gql@0.0.8:
version "0.0.8"
resolved "https://registry.yarnpkg.com/ghost-gql/-/ghost-gql-0.0.8.tgz#630410cf1f71ccffbdab3d9d01419981c794b0ce"
dependencies:
lodash "^4.17.4"

Expand Down

0 comments on commit 7999c38

Please sign in to comment.