From f64820b1be41dcceb144f1d996f30c24c4b98f5e Mon Sep 17 00:00:00 2001 From: Sag Date: Mon, 29 Jul 2024 17:33:23 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Disabled=20bulk=20deletion=20whe?= =?UTF-8?q?n=20multiple=20member=20filters=20are=20applied=20(#20681)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fixes https://linear.app/tryghost/issue/ONC-206 ref https://app.incident.io/ghost/incidents/90 - when multiple member filters are used in combination, NQL sometimes hit a limitation that results in the wrong members being returned - while we work on the NQL limitation, we are temporarily disabling bulk member deletion when more than one member filter has been applied --- ghost/admin/app/controllers/members.js | 4 ++ ghost/admin/app/templates/members.hbs | 14 +++--- ghost/admin/tests/acceptance/members-test.js | 49 ++++++++++++++++++- .../tests/acceptance/members/filter-test.js | 7 ++- 4 files changed, 66 insertions(+), 8 deletions(-) diff --git a/ghost/admin/app/controllers/members.js b/ghost/admin/app/controllers/members.js index 016987fda2..1270b5a06a 100644 --- a/ghost/admin/app/controllers/members.js +++ b/ghost/admin/app/controllers/members.js @@ -209,6 +209,10 @@ export default class MembersController extends Controller { return uniqueColumns.splice(0, 2); // Maximum 2 columns } + get isMultiFiltered() { + return this.isFiltered && this.filters.length >= 2; + } + includeTierQuery() { const availableFilters = this.filters.length ? this.filters : this.softFilters; return availableFilters.some((f) => { diff --git a/ghost/admin/app/templates/members.hbs b/ghost/admin/app/templates/members.hbs index c2671358d6..d72c91a904 100644 --- a/ghost/admin/app/templates/members.hbs +++ b/ghost/admin/app/templates/members.hbs @@ -104,12 +104,14 @@ {{/if}} -
  • -
  • - -
    • + {{#unless this.isMultiFiltered}} +
  • +
  • + +
    • + {{/unless}} {{/if}} diff --git a/ghost/admin/tests/acceptance/members-test.js b/ghost/admin/tests/acceptance/members-test.js index 778d29a33b..b59f9514ca 100644 --- a/ghost/admin/tests/acceptance/members-test.js +++ b/ghost/admin/tests/acceptance/members-test.js @@ -143,6 +143,53 @@ describe('Acceptance: Members', function () { .to.equal('example@domain.com'); }); + /* NOTE: Bulk deletion is disabled temporarily when multiple filters are applied, due to a NQL limitation. + * Delete this test once we have fixed the root NQL limitation. + * See https://linear.app/tryghost/issue/ONC-203 + */ + it('cannot bulk delete members if more than 1 filter is selected', async function () { + // Members with label + const labelOne = this.server.create('label'); + const labelTwo = this.server.create('label'); + this.server.createList('member', 2, {labels: [labelOne]}); + this.server.createList('member', 2, {labels: [labelOne, labelTwo]}); + + 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 single filter + await click('[data-test-button="members-filter-actions"]'); + await fillIn('[data-test-members-filter="0"] [data-test-select="members-filter"]', 'label'); + await click('.gh-member-label-input input'); + await click(`[data-test-label-filter="${labelOne.name}"]`); + await click(`[data-test-button="members-apply-filter"]`); + + expect(findAll('[data-test-member]').length).to.equal(4); + expect(currentURL()).to.equal(`/members?filter=label%3A%5B${labelOne.slug}%5D`); + + 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"]', 'label'); + await click('[data-test-members-filter="1"] .gh-member-label-input input'); + await click(`[data-test-members-filter="1"] [data-test-label-filter="${labelTwo.name}"]`); + await click(`[data-test-button="members-apply-filter"]`); + + expect(findAll('[data-test-member]').length).to.equal(2); + expect(currentURL()).to.equal(`/members?filter=label%3A%5B${labelOne.slug}%5D%2Blabel%3A%5B${labelTwo.slug}%5D`); + + await click('[data-test-button="members-actions"]'); + expect(find('[data-test-button="delete-selected"]')).to.not.exist; + }); + it('can bulk delete members', async function () { // members to be kept this.server.createList('member', 6); @@ -167,7 +214,7 @@ describe('Acceptance: Members', function () { await click(`[data-test-button="members-apply-filter"]`); expect(findAll('[data-test-member]').length).to.equal(5); - expect(currentURL()).to.equal('/members?filter=label%3A%5Blabel-0%5D'); + expect(currentURL()).to.equal(`/members?filter=label%3A%5B${label.slug}%5D`); await click('[data-test-button="members-actions"]'); diff --git a/ghost/admin/tests/acceptance/members/filter-test.js b/ghost/admin/tests/acceptance/members/filter-test.js index cf9f3a1e49..3068b9a60e 100644 --- a/ghost/admin/tests/acceptance/members/filter-test.js +++ b/ghost/admin/tests/acceptance/members/filter-test.js @@ -1328,7 +1328,12 @@ describe('Acceptance: Members filtering', function () { expect(find('[data-test-button="add-label-selected"]'), 'add label to selected button').to.exist; expect(find('[data-test-button="remove-label-selected"]'), 'remove label from selected button').to.exist; expect(find('[data-test-button="unsubscribe-selected"]'), 'unsubscribe selected button').to.exist; - expect(find('[data-test-button="delete-selected"]'), 'delete selected button').to.exist; + + /* NOTE: Bulk deletion is disabled temporarily when multiple filters are applied, due to a NQL limitation. + * Re-enable following line once we have fixed the root NQL limitation. + * See https://linear.app/tryghost/issue/ONC-203 + */ + // expect(find('[data-test-button="delete-selected"]'), 'delete selected button').to.exist; // filter is active and has # of filters expect(find('[data-test-button="members-filter-actions"] span'), 'filter button').to.have.class('gh-btn-label-green');