From 8d0eed2dcd9ba7038afc1bfdee7f2cf957a3b2e4 Mon Sep 17 00:00:00 2001 From: Michael Barrett Date: Thu, 10 Aug 2023 12:59:14 +0100 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Fixed=20member=20filter=20by=20s?= =?UTF-8?q?ingle=20newsletter=20subscription=20(#17668)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fixes https://github.com/TryGhost/Product/issues/3708 When filtering members by the `Newsletter subscription` parameter, If the site has a single newsletter, members with disabled emails were being erroneously returned. This fixes that 👍 --- .../components/members/filters/subscribed.js | 35 +++++++++++++++++-- .../tests/acceptance/members/filter-test.js | 23 ++++++------ 2 files changed, 45 insertions(+), 13 deletions(-) diff --git a/ghost/admin/app/components/members/filters/subscribed.js b/ghost/admin/app/components/members/filters/subscribed.js index 977759b994..883434624f 100644 --- a/ghost/admin/app/components/members/filters/subscribed.js +++ b/ghost/admin/app/components/members/filters/subscribed.js @@ -6,13 +6,44 @@ export const SUBSCRIBED_FILTER = { columnLabel: 'Subscribed', relationOptions: MATCH_RELATION_OPTIONS, valueType: 'options', + buildNqlFilter: (flt) => { + const relation = flt.relation; + const value = flt.value; + + return (relation === 'is' && value === 'true') || (relation === 'is-not' && value === 'false') + ? '(subscribed:true+email_disabled:0)' + : '(subscribed:false,email_disabled:1)'; + }, + parseNqlFilter: (flt) => { + const comparator = flt.$and || flt.$or; + + if (!comparator || comparator.length !== 2) { + return; + } + + if (comparator[0].subscribed === undefined || comparator[1].email_disabled === undefined) { + return; + } + + const subscribed = comparator[0].subscribed; + + return { + value: subscribed ? 'true' : 'false', + relation: 'is' + }; + }, options: [ {label: 'Subscribed', name: 'true'}, {label: 'Unsubscribed', name: 'false'} ], - getColumnValue: (member) => { + getColumnValue: (member, flt) => { + const relation = flt.relation; + const value = flt.value; + return { - text: member.subscribed ? 'Subscribed' : 'Unsubscribed' + text: (relation === 'is' && value === 'true') || (relation === 'is-not' && value === 'false') + ? 'Subscribed' + : 'Unsubscribed' }; } }; diff --git a/ghost/admin/tests/acceptance/members/filter-test.js b/ghost/admin/tests/acceptance/members/filter-test.js index 5882b8b388..5bf2585b7e 100644 --- a/ghost/admin/tests/acceptance/members/filter-test.js +++ b/ghost/admin/tests/acceptance/members/filter-test.js @@ -160,11 +160,11 @@ describe('Acceptance: Members filtering', function () { expect(findAll('[data-test-list="members-list-item"]').length, '# of filtered member rows after delete') .to.equal(7); }); - + it('can filter by offer redeemed', async function () { // add some offers to test the selection dropdown const tier = this.server.create('tier'); - + // create 3 offers const offer = this.server.create('offer', {tier: {id: tier.id}, createdAt: moment.utc().subtract(1, 'day').valueOf()}); this.server.create('offer', {tier: {id: tier.id}, createdAt: moment.utc().subtract(2, 'day').valueOf()}); @@ -196,7 +196,7 @@ describe('Acceptance: Members filtering', function () { // only one redeemed offer so only 1 member should be shown expect(findAll('[data-test-list="members-list-item"]').length, '# of filtered member rows').to.equal(1); }); - + it('can filter by specific newsletter subscription', async function () { // add some members to filters const newsletter = this.server.create('newsletter', {status: 'active', slug: 'test-newsletter'}); @@ -232,13 +232,14 @@ describe('Acceptance: Members filtering', function () { it('can filter by newsletter subscription', async function () { // add some members to filter - this.server.createList('member', 3, {subscribed: true}); - this.server.createList('member', 4, {subscribed: false}); + this.server.createList('member', 3, {subscribed: true, email_disabled: 0}); + this.server.createList('member', 4, {subscribed: false, email_disabled: 0}); + this.server.createList('member', 1, {subscribed: true, email_disabled: 1}); await visit('/members'); expect(findAll('[data-test-list="members-list-item"]').length, '# of initial member rows') - .to.equal(7); + .to.equal(8); await click('[data-test-button="members-filter-actions"]'); @@ -265,18 +266,18 @@ describe('Acceptance: Members filtering', function () { // can change filter await fillIn(`${filterSelector} [data-test-select="members-filter-value"]`, 'false'); expect(findAll('[data-test-list="members-list-item"]').length, '# of filtered member rows - false') - .to.equal(4); + .to.equal(5); expect(find('[data-test-table-column="subscribed"]')).to.exist; // can delete filter await click('[data-test-delete-members-filter="0"]'); expect(findAll('[data-test-list="members-list-item"]').length, '# of filtered member rows after delete') - .to.equal(7); + .to.equal(8); // Can set filter by path await visit('/'); - await visit('/members?filter=' + encodeURIComponent('subscribed:true')); + await visit('/members?filter=' + encodeURIComponent('(subscribed:true+email_disabled:0)')); expect(findAll('[data-test-list="members-list-item"]').length, '# of filtered member rows - true - from URL') .to.equal(3); await click('[data-test-button="members-filter-actions"]'); @@ -284,9 +285,9 @@ describe('Acceptance: Members filtering', function () { // Can set filter by path await visit('/'); - await visit('/members?filter=' + encodeURIComponent('subscribed:false')); + await visit('/members?filter=' + encodeURIComponent('(subscribed:false,email_disabled:1)')); expect(findAll('[data-test-list="members-list-item"]').length, '# of filtered member rows - false - from URL') - .to.equal(4); + .to.equal(5); await click('[data-test-button="members-filter-actions"]'); expect(find(`${filterSelector} [data-test-select="members-filter-value"]`)).to.have.value('false'); });