From 492c26c6ac6a9275153eed46d2a0d6165a99b685 Mon Sep 17 00:00:00 2001 From: Michael Barrett Date: Tue, 29 Aug 2023 14:20:55 +0100 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Fixed=20member=20newsletter=20fi?= =?UTF-8?q?lter=20when=20multiple=20filters=20applied=20(#17857)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit closes https://github.com/TryGhost/Product/issues/3735 The member newsletter filter was not working correctly when multiple filters were applied due to the regex incorrectly extracting the contents of a grouped filter. This commit splits the regex into two to make it easier to reason about and fixes the underlying issue --- ghost/admin/app/controllers/members.js | 31 +++++++------------------- 1 file changed, 8 insertions(+), 23 deletions(-) diff --git a/ghost/admin/app/controllers/members.js b/ghost/admin/app/controllers/members.js index 0e68214c7f..0149df9f62 100644 --- a/ghost/admin/app/controllers/members.js +++ b/ghost/admin/app/controllers/members.js @@ -216,33 +216,18 @@ export default class MembersController extends Controller { }); } - refineFilterParam(filterParam) { - // We have some crazy regex below, here's a breakdown of what it does: - // \\( - Matches an opening parenthesis "(" - // [^)]* - Matches zero or more characters that are NOT a closing parenthesis ")" - // [,+\\-]? - Matches an optional comma, plus, or minus sign - // email_disabled - Matches the exact string "email_disabled" - // [^)]* - Matches zero or more characters that are NOT a closing parenthesis ")" - // \\) - Matches a closing parenthesis ")" - // (?! - // [+\\-] - Negative lookahead: Asserts what directly follows is neither a plus "+" nor a minus "-" - // | - OR - // :\\'[^']*\\' - Negative lookahead: Asserts what follows is not a colon ":" followed by a string in single quotes - // | - OR - // :\\"[^"]*\\" - Negative lookahead: Asserts what follows is not a colon ":" followed by a string in double quotes - // ) - const regex = new RegExp(`\\(([^)]*[,+\\-]?email_disabled[^)]*)\\)(?![+\\-]|:\\'[^']*\\'|:\\"[^"]*\\")`, 'g'); - return filterParam.replace(regex, '$1'); - } - getApiQueryObject({params, extraFilters = []} = {}) { let {label, paidParam, searchParam, filterParam} = params ? params : this; - // NOTE: this is a temporary fix to help where the API/NQL isn't handling the parentheses correctly - // It's potentially a deeper issue with NQL. This should be removed once the API is fixed. - // This could be related https://github.com/TryGhost/NQL/issues/16 if (filterParam) { - filterParam = this.refineFilterParam(filterParam); + // If the provided filter param is a single filter related to newsletter subscription status + // remove the surrounding brackets to prevent https://github.com/TryGhost/NQL/issues/16 + const NEWSLETTER_SUBSCRIPTION_STATUS_RE = /^\(subscribed:(?:true|false)[+,]email_disabled:[01]\)$/; + const SPECIFIC_NEWSLETTER_SUBSCRIPTION_STATUS_RE = /^\(newsletters\.slug:[^()]+[+,]email_disabled:[01]\)$/; + + if (NEWSLETTER_SUBSCRIPTION_STATUS_RE.test(filterParam) || SPECIFIC_NEWSLETTER_SUBSCRIPTION_STATUS_RE.test(filterParam)) { + filterParam = filterParam.slice(1, -1); + } } let filters = [];