From a2bfef53dec811da3a648b7d6d271ec2617926f5 Mon Sep 17 00:00:00 2001 From: naz Date: Mon, 2 Nov 2020 12:53:57 +1300 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Fixed=20post=20resource=20filter?= =?UTF-8?q?ing=20by=20posts=5Fmeta=20table=20fields=20(#12307)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit refs #11572 - Filtering by fields coming from posts_meta table did not work for post resources. This was due to lack of support for these types of operations on NQL layer. The approach taken here is using same way filtering was done for many:many relations and generates a `WHERE IN` filtering clause. In the future we could look into adding preloading of 1:1 relations which should allow getting rid of `WHERE IN` in favor of `JOIN` and filtering directly by field names. - Changed structure of `EXPANSIONS` filter configuration. Current approach was based on "bag of all the things". Such structure will become problematic as more fields are added. For example, adding all the fields from 1:1 relation posts:posts_meta might collide with any other relations that would have similar naming like meta_description from tags table (if it were was added). - Bumped nql version to 0.5.0. This adds filtering support to 1:1 relations - Added filter expansions which can be unique per model Previous approach with single global expansions lookup wasn't working in case different models would need to declare expansion for same field names. Having a `filterExpansion` method per model works in a similar convention other filter related model methods do (e.g. enforcedFilters, defaultFilters) --- core/server/models/plugins/filter.js | 76 ++++++++++++------- core/server/models/post.js | 11 +++ package.json | 2 +- .../regression/api/canary/admin/posts_spec.js | 63 ++++++++++++++- test/utils/fixtures/data-generator.js | 2 +- yarn.lock | 18 ++--- 6 files changed, 131 insertions(+), 41 deletions(-) diff --git a/core/server/models/plugins/filter.js b/core/server/models/plugins/filter.js index 45a3099c68..6e34b1294c 100644 --- a/core/server/models/plugins/filter.js +++ b/core/server/models/plugins/filter.js @@ -24,36 +24,44 @@ const RELATIONS = { joinTable: 'members_labels', joinFrom: 'member_id', joinTo: 'label_id' + }, + posts_meta: { + tableName: 'posts_meta', + type: 'oneToOne', + joinFrom: 'post_id' } }; -const EXPANSIONS = [{ - key: 'primary_tag', - replacement: 'tags.slug', - expansion: 'posts_tags.sort_order:0+tags.visibility:public' -}, { - key: 'primary_author', - replacement: 'authors.slug', - expansion: 'posts_authors.sort_order:0+authors.visibility:public' -}, { - key: 'authors', - replacement: 'authors.slug' -}, { - key: 'author', - replacement: 'authors.slug' -}, { - key: 'tag', - replacement: 'tags.slug' -}, { - key: 'tags', - replacement: 'tags.slug' -}, { - key: 'label', - replacement: 'labels.slug' -}, { - key: 'labels', - replacement: 'labels.slug' -}]; +const EXPANSIONS = { + posts: [{ + key: 'primary_tag', + replacement: 'tags.slug', + expansion: 'posts_tags.sort_order:0+tags.visibility:public' + }, { + key: 'primary_author', + replacement: 'authors.slug', + expansion: 'posts_authors.sort_order:0+authors.visibility:public' + }, { + key: 'authors', + replacement: 'authors.slug' + }, { + key: 'author', + replacement: 'authors.slug' + }, { + key: 'tag', + replacement: 'tags.slug' + }, { + key: 'tags', + replacement: 'tags.slug' + }], + members: [{ + key: 'label', + replacement: 'labels.slug' + }, { + key: 'labels', + replacement: 'labels.slug' + }] +}; const filter = function filter(Bookshelf) { const Model = Bookshelf.Model.extend({ @@ -63,7 +71,7 @@ const filter = function filter(Bookshelf) { enforcedFilters() {}, defaultFilters() {}, extraFilters() {}, - + filterExpansions() {}, /** * Method which makes the necessary query builder calls (through knex) for the filters set on this model * instance. @@ -71,6 +79,16 @@ const filter = function filter(Bookshelf) { applyDefaultAndCustomFilters: function applyDefaultAndCustomFilters(options) { const nql = require('@nexes/nql'); + const expansions = []; + + if (EXPANSIONS[this.tableName]) { + expansions.push(...EXPANSIONS[this.tableName]); + } + + if (this.filterExpansions()) { + expansions.push(...this.filterExpansions()); + } + let custom = options.filter; let extra = this.extraFilters(options); let overrides = this.enforcedFilters(options); @@ -94,7 +112,7 @@ const filter = function filter(Bookshelf) { this.query((qb) => { nql(custom, { relations: RELATIONS, - expansions: EXPANSIONS, + expansions: expansions, overrides: overrides, defaults: defaults, transformer: transformer diff --git a/core/server/models/post.js b/core/server/models/post.js index fd3504df6b..e47bc07fe2 100644 --- a/core/server/models/post.js +++ b/core/server/models/post.js @@ -99,6 +99,17 @@ Post = ghostBookshelf.Model.extend({ return [...keys, ...postsMetaKeys]; }, + filterExpansions: function filterExpansions() { + const postsMetaKeys = _.without(ghostBookshelf.model('PostsMeta').prototype.orderAttributes(), 'posts_meta.id', 'posts_meta.post_id'); + + return postsMetaKeys.map((pmk) => { + return { + key: pmk.split('.')[1], + replacement: pmk + }; + }); + }, + emitChange: function emitChange(event, options = {}) { let eventToTrigger; let resourceType = this.get('type'); diff --git a/package.json b/package.json index 0574220daa..243346ceca 100644 --- a/package.json +++ b/package.json @@ -40,7 +40,7 @@ "cli": "^1.12.0" }, "dependencies": { - "@nexes/nql": "0.4.0", + "@nexes/nql": "0.5.0", "@sentry/node": "5.27.2", "@tryghost/adapter-manager": "0.1.11", "@tryghost/admin-api-schema": "1.2.0", diff --git a/test/regression/api/canary/admin/posts_spec.js b/test/regression/api/canary/admin/posts_spec.js index 6c7f8b47d5..6f646b377f 100644 --- a/test/regression/api/canary/admin/posts_spec.js +++ b/test/regression/api/canary/admin/posts_spec.js @@ -90,6 +90,67 @@ describe('Posts API', function () { }); }); + it('can filter by fields coming from posts_meta table non null meta_description', function (done) { + request.get(localUtils.API.getApiQuery(`posts/?filter=meta_description:-null`)) + .set('Origin', config.get('url')) + .expect('Content-Type', /json/) + .expect('Cache-Control', testUtils.cacheRules.private) + .expect(200) + .end(function (err, res) { + if (err) { + return done(err); + } + + should.not.exist(res.headers['x-cache-invalidate']); + const jsonResponse = res.body; + should.exist(jsonResponse.posts); + localUtils.API.checkResponse(jsonResponse, 'posts'); + jsonResponse.posts.should.have.length(2); + jsonResponse.posts.forEach((post) => { + should.notEqual(post.meta_description, null); + }); + + localUtils.API.checkResponse( + jsonResponse.posts[0], + 'post' + ); + + localUtils.API.checkResponse(jsonResponse.meta.pagination, 'pagination'); + + done(); + }); + }); + + it('can filter by fields coming from posts_meta table by value', function (done) { + request.get(localUtils.API.getApiQuery(`posts/?filter=meta_description:'meta description for short and sweet'`)) + .set('Origin', config.get('url')) + .expect('Content-Type', /json/) + .expect('Cache-Control', testUtils.cacheRules.private) + .expect(200) + .end(function (err, res) { + if (err) { + return done(err); + } + + should.not.exist(res.headers['x-cache-invalidate']); + const jsonResponse = res.body; + should.exist(jsonResponse.posts); + localUtils.API.checkResponse(jsonResponse, 'posts'); + jsonResponse.posts.should.have.length(1); + jsonResponse.posts[0].id.should.equal(testUtils.DataGenerator.Content.posts[2].id); + jsonResponse.posts[0].meta_description.should.equal('meta description for short and sweet'); + + localUtils.API.checkResponse( + jsonResponse.posts[0], + 'post' + ); + + localUtils.API.checkResponse(jsonResponse.meta.pagination, 'pagination'); + + done(); + }); + }); + it('can order by fields coming from posts_meta table', function (done) { request.get(localUtils.API.getApiQuery('posts/?order=meta_description%20ASC')) .set('Origin', config.get('url')) @@ -109,7 +170,7 @@ describe('Posts API', function () { should.equal(jsonResponse.posts[0].meta_description, null); jsonResponse.posts[12].slug.should.equal('short-and-sweet'); - jsonResponse.posts[12].meta_description.should.equal('test stuff'); + jsonResponse.posts[12].meta_description.should.equal('meta description for short and sweet'); localUtils.API.checkResponse( jsonResponse.posts[0], diff --git a/test/utils/fixtures/data-generator.js b/test/utils/fixtures/data-generator.js index 9be43dc938..ddec87fe40 100644 --- a/test/utils/fixtures/data-generator.js +++ b/test/utils/fixtures/data-generator.js @@ -816,7 +816,7 @@ DataGenerator.forKnex = (function () { { id: ObjectId.generate(), post_id: DataGenerator.Content.posts[2].id, - meta_description: 'test stuff' + meta_description: 'meta description for short and sweet' }, { id: ObjectId.generate(), diff --git a/yarn.lock b/yarn.lock index db69f6e198..ca06523051 100644 --- a/yarn.lock +++ b/yarn.lock @@ -94,10 +94,10 @@ url-regex "~5.0.0" video-extensions "~1.1.0" -"@nexes/mongo-knex@0.3.0": - version "0.3.0" - resolved "https://registry.yarnpkg.com/@nexes/mongo-knex/-/mongo-knex-0.3.0.tgz#e0078f8ddf7fcd4d907fe5cf2390644218753cf0" - integrity sha512-LgPWH4mR+IWYh1/Y/ob22kVMYVBY2k/evE/QdXYifQcEOd2G0NRggRB7SQX8kLQ024FcjtIMfRXC3UjzFamUcw== +"@nexes/mongo-knex@0.4.0": + version "0.4.0" + resolved "https://registry.yarnpkg.com/@nexes/mongo-knex/-/mongo-knex-0.4.0.tgz#bdaef8d8cefeb9935ab7e3a1f5d0bbcfb764722e" + integrity sha512-RyfcK1+WJQ0SM7r0D5fMhKwjggxyrDb8hMaQ1vqfcMe/SE2J/qoSra9+fAdwN8zYRdz6Lovd6fqCaRQ5zFv5ZA== dependencies: debug "^4.1.0" lodash "^4.17.10" @@ -116,12 +116,12 @@ resolved "https://registry.yarnpkg.com/@nexes/nql-lang/-/nql-lang-0.0.1.tgz#a13c023873f9bc11b9e4e284449c6cfbeccc8011" integrity sha1-oTwCOHP5vBG55OKERJxs++zMgBE= -"@nexes/nql@0.4.0": - version "0.4.0" - resolved "https://registry.yarnpkg.com/@nexes/nql/-/nql-0.4.0.tgz#5ae28f8d339d56812eb8452ead9a5e5996087efd" - integrity sha512-fEsV2aMiPpqBneZosamFLypLAQAp8M5SA5MgeeJKMpHF0dQiHn9ifD556RgaSSY5RgO0gbRGoKypgy0YmyE1KQ== +"@nexes/nql@0.5.0": + version "0.5.0" + resolved "https://registry.yarnpkg.com/@nexes/nql/-/nql-0.5.0.tgz#bff22dbd4e2aa697d5c33031675cb100b013a0a4" + integrity sha512-skn8cfr0OJFu6HdcLRgBL19bJ2S7pATrgR/RnZR3v4T4iHfAkZ8zzLDMGib+721eFaAURnpGfPittO+RyPAhiQ== dependencies: - "@nexes/mongo-knex" "0.3.0" + "@nexes/mongo-knex" "0.4.0" "@nexes/mongo-utils" "0.3.0" "@nexes/nql-lang" "0.0.1" mingo "2.2.2"