From 1f5f9645e6f89b5e898753773b510a2b84d5816a Mon Sep 17 00:00:00 2001 From: Simon Backx Date: Thu, 5 May 2022 14:33:26 +0200 Subject: [PATCH] Updated members count to use selected newsletter when publishing refs https://github.com/TryGhost/Team/issues/1576 - Passes the selected newsletter to the recipient selector - Added a `recipientFilter` getter to the newsletter model to make changes easier in the future - Updates the `fullRecipientFilter` in the new publishing flow to use the newsletter's recipientFilter - Already takes future paid only newsletters into account - The counts in the status message after publishing is not updated yet (requires https://github.com/TryGhost/Team/issues/1569) - The counts in the scheduled notification is not updated yet (requires https://github.com/TryGhost/Team/issues/1569) --- .../editor-labs/publish-management.js | 2 +- .../publish-options/email-recipients.hbs | 3 ++- .../components/gh-members-recipient-select.hbs | 2 +- .../components/gh-members-recipient-select.js | 11 ++++++++--- .../app/components/gh-publishmenu-draft.hbs | 3 ++- .../components/gh-publishmenu-scheduled.hbs | 3 ++- ghost/admin/app/components/gh-publishmenu.js | 18 ++++++++---------- .../modals/editor/confirm-publish.js | 4 ++-- ghost/admin/app/controllers/editor.js | 1 + ghost/admin/app/models/newsletter.js | 12 +++++++++++- ghost/admin/tests/acceptance/editor-test.js | 9 ++++++--- 11 files changed, 44 insertions(+), 24 deletions(-) diff --git a/ghost/admin/app/components/editor-labs/publish-management.js b/ghost/admin/app/components/editor-labs/publish-management.js index 95936a0f6d..e2655e5de7 100644 --- a/ghost/admin/app/components/editor-labs/publish-management.js +++ b/ghost/admin/app/components/editor-labs/publish-management.js @@ -162,7 +162,7 @@ export class PublishOptions { } get fullRecipientFilter() { - let filter = `newsletters:${this.newsletter.slug}`; + let filter = this.newsletter.recipientFilter; if (this.recipientFilter) { filter += `+(${this.recipientFilter})`; diff --git a/ghost/admin/app/components/editor-labs/publish-options/email-recipients.hbs b/ghost/admin/app/components/editor-labs/publish-options/email-recipients.hbs index 47c3e43571..1bd3900258 100644 --- a/ghost/admin/app/components/editor-labs/publish-options/email-recipients.hbs +++ b/ghost/admin/app/components/editor-labs/publish-options/email-recipients.hbs @@ -24,9 +24,10 @@ -{{/if}} \ No newline at end of file +{{/if}} diff --git a/ghost/admin/app/components/gh-members-recipient-select.hbs b/ghost/admin/app/components/gh-members-recipient-select.hbs index 9afffafbcc..1581e01f2a 100644 --- a/ghost/admin/app/components/gh-members-recipient-select.hbs +++ b/ghost/admin/app/components/gh-members-recipient-select.hbs @@ -1,4 +1,4 @@ -
+

Free members {{this.freeMemberCountLabel}}

@@ -90,4 +91,4 @@
{{/if}} -
\ No newline at end of file + diff --git a/ghost/admin/app/components/gh-publishmenu-scheduled.hbs b/ghost/admin/app/components/gh-publishmenu-scheduled.hbs index 5b769859a9..7f07dd74ae 100644 --- a/ghost/admin/app/components/gh-publishmenu-scheduled.hbs +++ b/ghost/admin/app/components/gh-publishmenu-scheduled.hbs @@ -59,6 +59,7 @@
@@ -68,4 +69,4 @@ {{/if}} - \ No newline at end of file + diff --git a/ghost/admin/app/components/gh-publishmenu.js b/ghost/admin/app/components/gh-publishmenu.js index f65255bbc7..7ca4cfa818 100644 --- a/ghost/admin/app/components/gh-publishmenu.js +++ b/ghost/admin/app/components/gh-publishmenu.js @@ -401,7 +401,7 @@ export default Component.extend({ post: this.post, emailOnly: this.emailOnly, sendEmailWhenPublished: this.sendEmailWhenPublished, - newsletterId: this.newsletterId, + newsletter: this.selectedNewsletter, isScheduled: saveType === 'schedule', confirm: this.saveWithConfirmedPublish.perform, retryEmailSend: this.retryEmailSendTask.perform @@ -449,17 +449,15 @@ export default Component.extend({ }), fetchNewslettersTask: task(function* () { - if (this.feature.multipleNewsletters) { - const newsletters = yield this.store.query('newsletter', { - filter: 'status:active', - order: 'sort_order ASC' - }); + const newsletters = yield this.store.query('newsletter', { + filter: 'status:active', + order: 'sort_order ASC' + }); - const defaultNewsletter = newsletters.toArray()[0]; + const defaultNewsletter = newsletters.toArray()[0]; - this.defaultNewsletter = defaultNewsletter; - this.set('selectedNewsletter', defaultNewsletter); - } + this.defaultNewsletter = defaultNewsletter; + this.set('selectedNewsletter', defaultNewsletter); }), _saveTask: task(function* () { diff --git a/ghost/admin/app/components/modals/editor/confirm-publish.js b/ghost/admin/app/components/modals/editor/confirm-publish.js index 854b45260b..97c5fec9b7 100644 --- a/ghost/admin/app/components/modals/editor/confirm-publish.js +++ b/ghost/admin/app/components/modals/editor/confirm-publish.js @@ -59,8 +59,8 @@ export default class ConfirmPublishModal extends Component { @task *countRecipientsTask() { - const {sendEmailWhenPublished} = this.args.data; - const filter = `newsletters.status:active+(${sendEmailWhenPublished})`; + const {sendEmailWhenPublished,newsletter} = this.args.data; + const filter = `${newsletter.recipientFilter}+(${sendEmailWhenPublished})`; this.memberCount = sendEmailWhenPublished ? (yield this.membersCountCache.count(filter)) : 0; this.memberCountString = sendEmailWhenPublished ? (yield this.membersCountCache.countString(filter)) : '0 members'; diff --git a/ghost/admin/app/controllers/editor.js b/ghost/admin/app/controllers/editor.js index e67e74d4ab..18265f4bc6 100644 --- a/ghost/admin/app/controllers/editor.js +++ b/ghost/admin/app/controllers/editor.js @@ -1079,6 +1079,7 @@ export default class EditorController extends Controller { let description = emailOnly ? ['Will be sent'] : ['Will be published']; if (emailRecipientFilter && emailRecipientFilter !== 'none') { + // TODO: replace active filter with newsletter.recipientFilter (requires https://github.com/TryGhost/Team/issues/1569) const recipientCount = await this.membersCountCache.countString(`newsletters.status:active+(${emailRecipientFilter})`); description.push(`${!emailOnly ? 'and delivered ' : ''}to ${recipientCount}`); } diff --git a/ghost/admin/app/models/newsletter.js b/ghost/admin/app/models/newsletter.js index 0efe3c9e6d..fedb47ef6c 100644 --- a/ghost/admin/app/models/newsletter.js +++ b/ghost/admin/app/models/newsletter.js @@ -13,7 +13,6 @@ export default class Newsletter extends Model.extend(ValidationEngine) { @attr({defaultValue: 'newsletter'}) senderReplyTo; @attr({defaultValue: 'active'}) status; - @attr({defaultValue: ''}) recipientFilter; @attr({defaultValue: true}) subscribeOnSignup; @attr({defaultValue: 'members'}) visibility; @attr({defaultValue: 0}) sortOrder; @@ -34,4 +33,15 @@ export default class Newsletter extends Model.extend(ValidationEngine) { // HACK - not a real model attribute but a workaround for Ember Data not // exposing meta from save responses @attr _meta; + + /** + * The filter that we should use to filter out members that are subscribed to this newsletter + */ + get recipientFilter() { + const idFilter = 'newsletters.id:' + this.id; + if (this.visibility === 'paid') { + return idFilter + '+status:-free'; + } + return idFilter; + } } diff --git a/ghost/admin/tests/acceptance/editor-test.js b/ghost/admin/tests/acceptance/editor-test.js index 9fe21959a2..394db334a5 100644 --- a/ghost/admin/tests/acceptance/editor-test.js +++ b/ghost/admin/tests/acceptance/editor-test.js @@ -6,6 +6,7 @@ import {authenticateSession, invalidateSession} from 'ember-simple-auth/test-sup import {beforeEach, describe, it} from 'mocha'; import {blur, click, currentRouteName, currentURL, fillIn, find, findAll, triggerEvent} from '@ember/test-helpers'; import {datepickerSelect} from 'ember-power-datepicker/test-support'; +import {enableLabsFlag} from '../helpers/labs-flag'; import {enableMailgun} from '../helpers/mailgun'; import {enableNewsletters} from '../helpers/newsletters'; import {enableStripe} from '../helpers/stripe'; @@ -845,6 +846,7 @@ describe('Acceptance: Editor', function () { const role = this.server.create('role', {name: 'Administrator'}); user = this.server.create('user', {roles: [role]}); this.server.loadFixtures('settings'); + enableLabsFlag(this.server, 'multipleNewsletters'); return await authenticateSession(); }); @@ -876,7 +878,8 @@ describe('Acceptance: Editor', function () { // Enable stripe to also show paid members breakdown enableStripe(this.server); - const newsletter = this.server.create('newsletter', {status: 'active'}); + // Note: we need to set the ID of a newsletter to some string value because of how NQL filters work. + const newsletter = this.server.create('newsletter', {status: 'active', name: 'test newsletter', id: 'test-newsletter'}); this.server.createList('member', 4, {status: 'free', newsletters: [newsletter]}); this.server.createList('member', 2, {status: 'paid', newsletters: [newsletter]}); @@ -889,8 +892,8 @@ describe('Acceptance: Editor', function () { await selectChoose('[data-test-distribution-action-select]', 'send'); await click('[data-test-publishmenu-scheduled-option]'); await datepickerSelect('[data-test-publishmenu-draft] [data-test-date-time-picker-datepicker]', new Date(scheduledTime.format().replace(/\+.*$/, ''))); - - // Expect 4 free and 2 paid recipients here + + // Expect 4 free and 2 paid recipients here expect(find('[data-test-email-count="free-members"]')).to.contain.text('4'); expect(find('[data-test-email-count="paid-members"]')).to.contain.text('2');