From 5a5ddcb6098e1a08d58a4363bf4a31a77566268e Mon Sep 17 00:00:00 2001 From: Fabien 'egg' O'Carroll Date: Wed, 13 Mar 2024 00:25:42 +0700 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Fixed=20Tiers=20API=20erroring?= =?UTF-8?q?=20when=20invalid=20filter=20passed=20(#19845)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit closes ENG-730 closes https://linear.app/tryghost/issue/ENG-730/ We've updated the input serializer to parse the filter, and responded with an error if it cannot be parsed correctly. Now that it's parsed, we can pass a mongo query object through the stack, which will lend itself to better typing for this code, which is a direction we want to go in anyway. We've had to update all the internal usages of the `browse` method to use mongo query objects. --- .../utils/serializers/input/tiers.js | 32 +++++++++++++++++-- .../core/server/services/members/service.js | 4 ++- .../server/services/tiers/TierRepository.js | 3 +- ghost/tiers/lib/InMemoryTierRepository.js | 3 +- ghost/tiers/lib/TiersAPI.js | 16 ++++++++-- 5 files changed, 50 insertions(+), 8 deletions(-) diff --git a/ghost/core/core/server/api/endpoints/utils/serializers/input/tiers.js b/ghost/core/core/server/api/endpoints/utils/serializers/input/tiers.js index 578c3242e0..8442be7895 100644 --- a/ghost/core/core/server/api/endpoints/utils/serializers/input/tiers.js +++ b/ghost/core/core/server/api/endpoints/utils/serializers/input/tiers.js @@ -1,10 +1,26 @@ +const {BadRequestError} = require('@tryghost/errors'); const localUtils = require('../../index'); +const nql = require('@tryghost/nql-lang'); +const tpl = require('@tryghost/tpl'); + +const messages = { + invalidNQLFilter: 'The NQL filter you passed was invalid.' +}; const forceActiveFilter = (frame) => { if (frame.options.filter) { - frame.options.filter = `(${frame.options.filter})+active:true`; + frame.options.filter = { + $and: [ + { + active: true + }, + frame.options.filter + ] + }; } else { - frame.options.filter = 'active:true'; + frame.options.filter = { + active: true + }; } }; @@ -41,6 +57,18 @@ function convertTierInput(input) { module.exports = { all(_apiConfig, frame) { + if (frame.options.filter) { + try { + frame.options.filter = nql.parse(frame.options.filter); + } catch (err) { + throw new BadRequestError({ + message: tpl(messages.invalidNQLFilter) + }); + } + } else { + frame.options.filter = null; + } + if (localUtils.isContentAPI(frame)) { // CASE: content api can only have active tiers forceActiveFilter(frame); diff --git a/ghost/core/core/server/services/members/service.js b/ghost/core/core/server/services/members/service.js index e3a1508b67..d6a400d1ca 100644 --- a/ghost/core/core/server/services/members/service.js +++ b/ghost/core/core/server/services/members/service.js @@ -55,7 +55,9 @@ const initMembersCSVImporter = ({stripeAPIService}) => { }, getTierByName: async (name) => { const tiers = await tiersService.api.browse({ - filter: `name:'${name}'` + filter: { + name + } }); if (tiers.data.length > 0) { diff --git a/ghost/core/core/server/services/tiers/TierRepository.js b/ghost/core/core/server/services/tiers/TierRepository.js index aa874b60e8..12012a854b 100644 --- a/ghost/core/core/server/services/tiers/TierRepository.js +++ b/ghost/core/core/server/services/tiers/TierRepository.js @@ -86,7 +86,8 @@ module.exports = class TierRepository { * @returns {Promise} */ async getAll(options = {}) { - const filter = nql(options.filter, {}); + const filter = nql(); + filter.filter = options.filter || {}; return Promise.all(this.#store.slice().filter((item) => { return filter.queryJSON(this.toPrimitive(item)); }).map((tier) => { diff --git a/ghost/tiers/lib/InMemoryTierRepository.js b/ghost/tiers/lib/InMemoryTierRepository.js index 682efcaacf..fbff8a9fc0 100644 --- a/ghost/tiers/lib/InMemoryTierRepository.js +++ b/ghost/tiers/lib/InMemoryTierRepository.js @@ -55,7 +55,8 @@ class InMemoryTierRepository { * @returns {Promise} */ async getAll(options = {}) { - const filter = nql(options.filter, {}); + const filter = nql(); + filter.filter = options.filter || {}; return this.#store.slice().filter((item) => { return filter.queryJSON(this.toPrimitive(item)); }); diff --git a/ghost/tiers/lib/TiersAPI.js b/ghost/tiers/lib/TiersAPI.js index 62e29ef0a9..48daaca51f 100644 --- a/ghost/tiers/lib/TiersAPI.js +++ b/ghost/tiers/lib/TiersAPI.js @@ -1,5 +1,5 @@ const ObjectID = require('bson-objectid').default; -const {BadRequestError} = require('@tryghost/errors'); +const {BadRequestError, IncorrectUsageError} = require('@tryghost/errors'); const Tier = require('./Tier'); /** @@ -42,11 +42,16 @@ module.exports = class TiersAPI { /** * @param {object} [options] - * @param {string} [options.filter] - An NQL filter string + * @param {any} [options.filter] - A mongo query object * * @returns {Promise>} */ async browse(options = {}) { + if (typeof options.filter === 'string') { + throw new IncorrectUsageError({ + message: 'filter must be a mongo query object' + }); + } const tiers = await this.#repository.getAll(options); return { @@ -83,7 +88,12 @@ module.exports = class TiersAPI { */ async readDefaultTier(options = {}) { const [defaultTier] = await this.#repository.getAll({ - filter: 'type:paid+active:true', + filter: { + $and: [ + {type: 'paid'}, + {active: true} + ] + }, limit: 1, ...options });