🐛 Fixed member filtering for "Unsubscribed from newsletter" filters (#20926)

ref https://linear.app/tryghost/issue/ENG-1466
ref https://linear.app/tryghost/issue/ENG-1484

- Previously, filtering members with multiple "Unsubscribed from
newsletter x" led to no filtering at all, all members were returned
- This was caused by a bug in NQL, that is fixed in version 0.12.5, cf.
[commit](dd18d1d6ca)
- We're also removing the safeguard in the product around bulk deletion
when multiple newsletter filters are in use, as the root problem has
been fixed
This commit is contained in:
Sag 2024-09-16 11:16:49 +02:00 committed by GitHub
parent c29dc48370
commit 430fbdb987
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
13 changed files with 51 additions and 97 deletions

View File

@ -41,7 +41,7 @@
"@tryghost/color-utils": "0.2.2", "@tryghost/color-utils": "0.2.2",
"@tryghost/kg-unsplash-selector": "0.2.4", "@tryghost/kg-unsplash-selector": "0.2.4",
"@tryghost/limit-service": "1.2.14", "@tryghost/limit-service": "1.2.14",
"@tryghost/nql": "0.12.4", "@tryghost/nql": "0.12.5",
"@tryghost/timezone-data": "0.4.3", "@tryghost/timezone-data": "0.4.3",
"react": "18.3.1", "react": "18.3.1",
"react-dom": "18.3.1", "react-dom": "18.3.1",

View File

@ -209,10 +209,8 @@ export default class MembersController extends Controller {
return uniqueColumns.splice(0, 2); // Maximum 2 columns return uniqueColumns.splice(0, 2); // Maximum 2 columns
} }
/* Due to a limitation with NQL when multiple member filters are used in combination, we currently have a safeguard around member bulk deletion. /*
* Member bulk deletion is not permitted when: * Due to a limitation with NQL, member bulk deletion is not permitted if any of the following Stripe subscription filters is used:
* 1) Multiple newsletters exist, and 2 or more newsletter filters are in use
* 2) If any of the following Stripe filters are used, even once:
* - Billing period * - Billing period
* - Stripe subscription status * - Stripe subscription status
* - Paid start date * - Paid start date
@ -220,19 +218,15 @@ export default class MembersController extends Controller {
* - Subscription started on post/page * - Subscription started on post/page
* - Offers * - Offers
* *
* See issue https://linear.app/tryghost/issue/ENG-1484 for more context * For more context, see:
* - https://linear.app/tryghost/issue/ENG-1484
* - https://linear.app/tryghost/issue/ENG-1466
*/ */
get isBulkDeletePermitted() { get isBulkDeletePermitted() {
if (!this.isFiltered) { if (!this.isFiltered) {
return false; return false;
} }
const newsletterFilters = this.filters.filter(f => f.group === 'Newsletters');
if (newsletterFilters && newsletterFilters.length >= 2) {
return false;
}
const stripeFilters = this.filters.filter(f => [ const stripeFilters = this.filters.filter(f => [
'subscriptions.plan_interval', 'subscriptions.plan_interval',
'subscriptions.status', 'subscriptions.status',

View File

@ -53,7 +53,7 @@
"@tryghost/koenig-lexical": "1.3.25", "@tryghost/koenig-lexical": "1.3.25",
"@tryghost/limit-service": "1.2.14", "@tryghost/limit-service": "1.2.14",
"@tryghost/members-csv": "0.0.0", "@tryghost/members-csv": "0.0.0",
"@tryghost/nql": "0.12.4", "@tryghost/nql": "0.12.5",
"@tryghost/nql-lang": "0.6.1", "@tryghost/nql-lang": "0.6.1",
"@tryghost/string": "0.2.12", "@tryghost/string": "0.2.12",
"@tryghost/timezone-data": "0.4.3", "@tryghost/timezone-data": "0.4.3",

View File

@ -143,63 +143,23 @@ describe('Acceptance: Members', function () {
.to.equal('example@domain.com'); .to.equal('example@domain.com');
}); });
/* Due to a limitation with NQL when multiple member filters are used in combination, we currently have a safeguard around member bulk deletion. /*
* Member bulk deletion is not permitted when: * Due to a limitation with NQL, member bulk deletion is not permitted if any of the following Stripe subscription filters is used:
* 1) Multiple newsletters exist, and 2 or more newsletter filters are in use * - Billing period
* 2) If any of the following Stripe filters are used, even once: * - Stripe subscription status
* - Billing period * - Paid start date
* - Stripe subscription status * - Next billing date
* - Paid start date * - Subscription started on post/page
* - Next billing date * - Offers
* - Subscription started on post/page *
* - Offers * For more context, see:
* * - https://linear.app/tryghost/issue/ENG-1484
* See code: ghost/admin/app/controllers/members.js:isBulkDeletePermitted * - https://linear.app/tryghost/issue/ENG-1466
* See issue https://linear.app/tryghost/issue/ENG-1484 for more context *
* * See code: ghost/admin/app/controllers/members.js:isBulkDeletePermitted
* TODO: delete this block of tests once the guardrail has been removed * TODO: delete this block of tests once the guardrail has been removed
*/ */
describe('[Temp] Guardrail against bulk deletion', function () { describe('[Temp] Guardrail against bulk deletion', function () {
it('cannot bulk delete members if more than 1 newsletter filter is used', async function () {
// Create two newsletters and members subscribed to 1 or 2 newsletters
const newsletterOne = this.server.create('newsletter');
const newsletterTwo = this.server.create('newsletter');
this.server.createList('member', 2).forEach(member => member.update({newsletters: [newsletterOne], email_disabled: 0}));
this.server.createList('member', 2).forEach(member => member.update({newsletters: [newsletterOne, newsletterTwo], email_disabled: 0}));
await visit('/members');
expect(findAll('[data-test-member]').length).to.equal(4);
// The delete button should not be visible by default
await click('[data-test-button="members-actions"]');
expect(find('[data-test-button="delete-selected"]')).to.not.exist;
// Apply a first filter
await click('[data-test-button="members-filter-actions"]');
await fillIn('[data-test-members-filter="0"] [data-test-select="members-filter"]', `newsletters.slug:${newsletterOne.slug}`);
await click(`[data-test-button="members-apply-filter"]`);
expect(findAll('[data-test-member]').length).to.equal(4);
expect(currentURL()).to.equal(`/members?filter=(newsletters.slug%3A${newsletterOne.slug}%2Bemail_disabled%3A0)`);
// Bulk deletion is permitted
await click('[data-test-button="members-actions"]');
expect(find('[data-test-button="delete-selected"]')).to.exist;
// Apply a second filter
await click('[data-test-button="members-filter-actions"]');
await click('[data-test-button="add-members-filter"]');
await fillIn('[data-test-members-filter="1"] [data-test-select="members-filter"]', `newsletters.slug:${newsletterTwo.slug}`);
await click(`[data-test-button="members-apply-filter"]`);
expect(findAll('[data-test-member]').length).to.equal(2);
expect(currentURL()).to.equal(`/members?filter=(newsletters.slug%3A${newsletterOne.slug}%2Bemail_disabled%3A0)%2B(newsletters.slug%3A${newsletterTwo.slug}%2Bemail_disabled%3A0)`);
// Bulk deletion is not permitted anymore
await click('[data-test-button="members-actions"]');
expect(find('[data-test-button="delete-selected"]')).to.not.exist;
});
it('can bulk delete members if a non-Stripe subscription filter is in use (member tier, status)', async function () { it('can bulk delete members if a non-Stripe subscription filter is in use (member tier, status)', async function () {
const tier = this.server.create('tier', {id: 'qwerty123456789'}); const tier = this.server.create('tier', {id: 'qwerty123456789'});
this.server.createList('member', 2, {status: 'free'}); this.server.createList('member', 2, {status: 'free'});

View File

@ -23,7 +23,7 @@
"c8": "7.14.0", "c8": "7.14.0",
"mocha": "10.2.0", "mocha": "10.2.0",
"sinon": "15.2.0", "sinon": "15.2.0",
"@tryghost/nql": "0.12.4" "@tryghost/nql": "0.12.5"
}, },
"dependencies": { "dependencies": {
"@tryghost/mongo-utils": "0.6.2", "@tryghost/mongo-utils": "0.6.2",

View File

@ -30,7 +30,7 @@
"@tryghost/errors": "1.3.5", "@tryghost/errors": "1.3.5",
"@tryghost/in-memory-repository": "0.0.0", "@tryghost/in-memory-repository": "0.0.0",
"@tryghost/logging": "2.4.18", "@tryghost/logging": "2.4.18",
"@tryghost/nql": "0.12.4", "@tryghost/nql": "0.12.5",
"@tryghost/nql-filter-expansions": "0.0.0", "@tryghost/nql-filter-expansions": "0.0.0",
"@tryghost/post-events": "0.0.0", "@tryghost/post-events": "0.0.0",
"@tryghost/tpl": "0.1.32", "@tryghost/tpl": "0.1.32",

View File

@ -73,7 +73,7 @@
"@tryghost/api-framework": "0.0.0", "@tryghost/api-framework": "0.0.0",
"@tryghost/api-version-compatibility-service": "0.0.0", "@tryghost/api-version-compatibility-service": "0.0.0",
"@tryghost/audience-feedback": "0.0.0", "@tryghost/audience-feedback": "0.0.0",
"@tryghost/bookshelf-plugins": "0.6.23", "@tryghost/bookshelf-plugins": "0.6.24",
"@tryghost/bootstrap-socket": "0.0.0", "@tryghost/bootstrap-socket": "0.0.0",
"@tryghost/collections": "0.0.0", "@tryghost/collections": "0.0.0",
"@tryghost/color-utils": "0.2.2", "@tryghost/color-utils": "0.2.2",
@ -141,7 +141,7 @@
"@tryghost/mw-version-match": "0.0.0", "@tryghost/mw-version-match": "0.0.0",
"@tryghost/mw-vhost": "0.0.0", "@tryghost/mw-vhost": "0.0.0",
"@tryghost/nodemailer": "0.3.45", "@tryghost/nodemailer": "0.3.45",
"@tryghost/nql": "0.12.4", "@tryghost/nql": "0.12.5",
"@tryghost/oembed-service": "0.0.0", "@tryghost/oembed-service": "0.0.0",
"@tryghost/package-json": "0.0.0", "@tryghost/package-json": "0.0.0",
"@tryghost/post-revisions": "0.0.0", "@tryghost/post-revisions": "0.0.0",
@ -193,7 +193,7 @@
"ghost-storage-base": "1.0.0", "ghost-storage-base": "1.0.0",
"glob": "8.1.0", "glob": "8.1.0",
"got": "11.8.6", "got": "11.8.6",
"gscan": "4.43.3", "gscan": "4.43.4",
"human-number": "2.0.4", "human-number": "2.0.4",
"image-size": "1.1.1", "image-size": "1.1.1",
"intl": "1.2.5", "intl": "1.2.5",

View File

@ -27,7 +27,7 @@
"dependencies": { "dependencies": {
"@tryghost/debug": "0.1.32", "@tryghost/debug": "0.1.32",
"@tryghost/errors": "1.3.5", "@tryghost/errors": "1.3.5",
"@tryghost/nql": "0.12.4", "@tryghost/nql": "0.12.5",
"@tryghost/tpl": "0.1.32", "@tryghost/tpl": "0.1.32",
"lodash": "4.17.21" "lodash": "4.17.21"
} }

View File

@ -25,6 +25,6 @@
"sinon": "15.2.0" "sinon": "15.2.0"
}, },
"dependencies": { "dependencies": {
"@tryghost/nql": "0.12.4" "@tryghost/nql": "0.12.5"
} }
} }

View File

@ -26,7 +26,7 @@
"dependencies": { "dependencies": {
"@tryghost/errors": "1.3.5", "@tryghost/errors": "1.3.5",
"@tryghost/link-redirects": "0.0.0", "@tryghost/link-redirects": "0.0.0",
"@tryghost/nql": "0.12.4", "@tryghost/nql": "0.12.5",
"@tryghost/tpl": "0.1.32", "@tryghost/tpl": "0.1.32",
"bson-objectid": "2.0.4", "bson-objectid": "2.0.4",
"lodash": "4.17.21", "lodash": "4.17.21",

View File

@ -36,7 +36,7 @@
"@tryghost/magic-link": "0.0.0", "@tryghost/magic-link": "0.0.0",
"@tryghost/member-events": "0.0.0", "@tryghost/member-events": "0.0.0",
"@tryghost/members-payments": "0.0.0", "@tryghost/members-payments": "0.0.0",
"@tryghost/nql": "0.12.4", "@tryghost/nql": "0.12.5",
"@tryghost/tpl": "0.1.32", "@tryghost/tpl": "0.1.32",
"@tryghost/validator": "0.2.14", "@tryghost/validator": "0.2.14",
"@types/jsonwebtoken": "9.0.6", "@types/jsonwebtoken": "9.0.6",

View File

@ -24,7 +24,7 @@
}, },
"dependencies": { "dependencies": {
"@tryghost/errors": "1.3.5", "@tryghost/errors": "1.3.5",
"@tryghost/nql": "0.12.4", "@tryghost/nql": "0.12.5",
"@tryghost/post-events": "0.0.0", "@tryghost/post-events": "0.0.0",
"@tryghost/tpl": "0.1.32", "@tryghost/tpl": "0.1.32",
"bson-objectid": "2.0.4" "bson-objectid": "2.0.4"

View File

@ -7456,14 +7456,14 @@
"@tryghost/debug" "^0.1.33" "@tryghost/debug" "^0.1.33"
lodash "^4.17.21" lodash "^4.17.21"
"@tryghost/bookshelf-filter@^0.5.19": "@tryghost/bookshelf-filter@^0.5.20":
version "0.5.19" version "0.5.20"
resolved "https://registry.yarnpkg.com/@tryghost/bookshelf-filter/-/bookshelf-filter-0.5.19.tgz#bc43e7823462762b8599a7ac09e82cc281d36301" resolved "https://registry.yarnpkg.com/@tryghost/bookshelf-filter/-/bookshelf-filter-0.5.20.tgz#e1c7a99314305178488dd8024b1b66c4a492caae"
integrity sha512-q8hF2+Rt35D0v+VIWPqkQlLAPsF5R5R4Mq2q1GhQ+n+OcYerlX7vFR9FKlGfrVeBg9HsLUBTNlZacm407HaLHA== integrity sha512-gPdss759jRIzI+o8U3wPCAL0qBB0DmLlsT8TPicF33kN4d/MSx5ab4bUtKqH/5AaYaSBQvPhZEEv5SZUI3Ztpw==
dependencies: dependencies:
"@tryghost/debug" "^0.1.33" "@tryghost/debug" "^0.1.33"
"@tryghost/errors" "^1.3.6" "@tryghost/errors" "^1.3.6"
"@tryghost/nql" "^0.12.4" "@tryghost/nql" "0.12.5"
"@tryghost/tpl" "^0.1.33" "@tryghost/tpl" "^0.1.33"
"@tryghost/bookshelf-has-posts@^0.1.33": "@tryghost/bookshelf-has-posts@^0.1.33":
@ -7498,15 +7498,15 @@
"@tryghost/tpl" "^0.1.33" "@tryghost/tpl" "^0.1.33"
lodash "^4.17.21" lodash "^4.17.21"
"@tryghost/bookshelf-plugins@0.6.23": "@tryghost/bookshelf-plugins@0.6.24":
version "0.6.23" version "0.6.24"
resolved "https://registry.yarnpkg.com/@tryghost/bookshelf-plugins/-/bookshelf-plugins-0.6.23.tgz#3b17f49d397a618911065593c8aa044881491853" resolved "https://registry.yarnpkg.com/@tryghost/bookshelf-plugins/-/bookshelf-plugins-0.6.24.tgz#f44c783e11da55fab95d8a5064a96ae58360aede"
integrity sha512-Muuk8J9t3Dv630FpT1cqhan6J6GGIXUjZ7r25IcymQPideUbAvZXt0jEXWN7jpB1pq3+MdbRJSOmw+KrVlWbtg== integrity sha512-tX35hQDTR7dXyY2Nd8t88hscVhRTe27okwxvIM9sR+QzJQ6yu7RUhQCqpdlRPtfzMwbF3vRxxaEAAaqsLZ4b6Q==
dependencies: dependencies:
"@tryghost/bookshelf-collision" "^0.1.46" "@tryghost/bookshelf-collision" "^0.1.46"
"@tryghost/bookshelf-custom-query" "^0.1.28" "@tryghost/bookshelf-custom-query" "^0.1.28"
"@tryghost/bookshelf-eager-load" "^0.1.32" "@tryghost/bookshelf-eager-load" "^0.1.32"
"@tryghost/bookshelf-filter" "^0.5.19" "@tryghost/bookshelf-filter" "^0.5.20"
"@tryghost/bookshelf-has-posts" "^0.1.33" "@tryghost/bookshelf-has-posts" "^0.1.33"
"@tryghost/bookshelf-include-count" "^0.3.16" "@tryghost/bookshelf-include-count" "^0.3.16"
"@tryghost/bookshelf-order" "^0.1.28" "@tryghost/bookshelf-order" "^0.1.28"
@ -7907,10 +7907,10 @@
dependencies: dependencies:
date-fns "^2.28.0" date-fns "^2.28.0"
"@tryghost/nql@0.12.4", "@tryghost/nql@^0.12.0", "@tryghost/nql@^0.12.4": "@tryghost/nql@0.12.5", "@tryghost/nql@^0.12.5":
version "0.12.4" version "0.12.5"
resolved "https://registry.yarnpkg.com/@tryghost/nql/-/nql-0.12.4.tgz#ae482a78faa5cdd23da0d440b8248b2c942cfc6c" resolved "https://registry.yarnpkg.com/@tryghost/nql/-/nql-0.12.5.tgz#dc4531e26af06fd40c3b182295a480923cc44591"
integrity sha512-t1qTtFwFETjNe2CEZneLWzfDeLO3eQQMhYtyC/LvanOtpr/dn5gn5HCBCQ/MYi+nLwXeYlUMy7Lcjj1V3ToN5Q== integrity sha512-vc7IBsmYLb7zszLTFxGcDobkUKQ3cBKTEq3P71OGt7ranVI3KD1Du5Pq90eowQflT61DTy5eU7U9mltXOGvEkw==
dependencies: dependencies:
"@tryghost/mongo-knex" "^0.9.1" "@tryghost/mongo-knex" "^0.9.1"
"@tryghost/mongo-utils" "^0.6.2" "@tryghost/mongo-utils" "^0.6.2"
@ -18855,17 +18855,17 @@ growly@^1.3.0:
resolved "https://registry.yarnpkg.com/growly/-/growly-1.3.0.tgz#f10748cbe76af964b7c96c93c6bcc28af120c081" resolved "https://registry.yarnpkg.com/growly/-/growly-1.3.0.tgz#f10748cbe76af964b7c96c93c6bcc28af120c081"
integrity sha512-+xGQY0YyAWCnqy7Cd++hc2JqMYzlm0dG30Jd0beaA64sROr8C4nt8Yc9V5Ro3avlSUDTN0ulqP/VBKi1/lLygw== integrity sha512-+xGQY0YyAWCnqy7Cd++hc2JqMYzlm0dG30Jd0beaA64sROr8C4nt8Yc9V5Ro3avlSUDTN0ulqP/VBKi1/lLygw==
gscan@4.43.3: gscan@4.43.4:
version "4.43.3" version "4.43.4"
resolved "https://registry.yarnpkg.com/gscan/-/gscan-4.43.3.tgz#2d04863569ff3eec72d941eb84afd0cfd94ee56c" resolved "https://registry.yarnpkg.com/gscan/-/gscan-4.43.4.tgz#f5ec47539d4ceeb57b853f4a1e67956214436c15"
integrity sha512-ReFWSD4RCRtfu8zchUXiYrIvXq6oFwCsBRrbMUFLB4QAnHtIwyYAypGXFWrNAhVbs2F1gSZ6a7kyCVF0di/wlA== integrity sha512-/oELLApcxX/C9IeYjljxOOi2jcGVeIVGttDo5ipkQOLcb1xUsUm4TrMLYWpmw//KhZwpwgP//12vRDJHDvR7lA==
dependencies: dependencies:
"@sentry/node" "^7.73.0" "@sentry/node" "^7.73.0"
"@tryghost/config" "^0.2.18" "@tryghost/config" "^0.2.18"
"@tryghost/debug" "^0.1.26" "@tryghost/debug" "^0.1.26"
"@tryghost/errors" "^1.2.26" "@tryghost/errors" "^1.2.26"
"@tryghost/logging" "^2.4.7" "@tryghost/logging" "^2.4.7"
"@tryghost/nql" "^0.12.0" "@tryghost/nql" "^0.12.5"
"@tryghost/pretty-cli" "^1.2.38" "@tryghost/pretty-cli" "^1.2.38"
"@tryghost/server" "^0.1.37" "@tryghost/server" "^0.1.37"
"@tryghost/zip" "^1.1.42" "@tryghost/zip" "^1.1.42"