diff --git a/ghost/admin/app/components/editor/publish-options/email-recipients.hbs b/ghost/admin/app/components/editor/publish-options/email-recipients.hbs index 887420fc85..d2f77fcd39 100644 --- a/ghost/admin/app/components/editor/publish-options/email-recipients.hbs +++ b/ghost/admin/app/components/editor/publish-options/email-recipients.hbs @@ -22,7 +22,7 @@ {{newsletter.name}} {{!-- TODO: remove conditional when author/editor can fetch member counts --}} {{#if @publishOptions.user.isAdmin}} - {{format-number newsletter.count.members}} + {{format-number newsletter.count.active_members}} {{/if}} diff --git a/ghost/admin/app/components/members/filters/subscribed.js b/ghost/admin/app/components/members/filters/subscribed.js index 2455d82cf6..95b554da18 100644 --- a/ghost/admin/app/components/members/filters/subscribed.js +++ b/ghost/admin/app/components/members/filters/subscribed.js @@ -2,7 +2,7 @@ import {MATCH_RELATION_OPTIONS} from './relation-options'; export const SUBSCRIBED_FILTER = { label: 'Newsletter subscription', - name: 'subscribed', + name: 'subscribed', columnLabel: 'Subscribed', relationOptions: MATCH_RELATION_OPTIONS, valueType: 'options', @@ -31,8 +31,8 @@ export const NEWSLETTERS_FILTER = (newsletterList) => { const value = flt.value; return (relation === 'is' && value === 'true') || (relation === 'is-not' && value === 'false') - ? `newsletters.slug:${newsletter.slug}` - : `newsletters.slug:-${newsletter.slug}`; + ? `newsletters.slug:${newsletter.slug}+email_disabled:0` + : `newsletters.slug:-${newsletter.slug},email_disabled:1`; }, parseNqlFilter: (flt) => { if (!flt['newsletters.slug']) { @@ -62,7 +62,7 @@ export const NEWSLETTERS_FILTER = (newsletterList) => { {label: 'Unsubscribed', name: 'false'} ] }; - newsletters.push(filter); + newsletters.push(filter); }); return newsletters; }; diff --git a/ghost/admin/app/components/modals/newsletters/edit.js b/ghost/admin/app/components/modals/newsletters/edit.js index bbab6b6dee..287f3ef4d3 100644 --- a/ghost/admin/app/components/modals/newsletters/edit.js +++ b/ghost/admin/app/components/modals/newsletters/edit.js @@ -52,7 +52,7 @@ export default class EditNewsletterModal extends Component { const result = yield newsletter.save({ adapterOptions: { - include: 'count.members,count.posts' + include: 'count.active_members,count.posts' } }); diff --git a/ghost/admin/app/components/modals/newsletters/new.hbs b/ghost/admin/app/components/modals/newsletters/new.hbs index ac30f0f37b..5645bc3270 100644 --- a/ghost/admin/app/components/modals/newsletters/new.hbs +++ b/ghost/admin/app/components/modals/newsletters/new.hbs @@ -35,7 +35,7 @@

{{#if this.optInExisting}} - {{#let (members-count-fetcher query=(hash filter="newsletters.status:active")) as |countFetcher|}} + {{#let (members-count-fetcher query=(hash filter="newsletters.status:active+email_disabled:0")) as |countFetcher|}} This newsletter will be available to all members. Your {{#if countFetcher.count}}{{countFetcher.count}}{{/if}} existing subscriber{{#if (gt countFetcher.count 1)}}s{{/if}} will also be opted-in to receive it. {{/let}} {{else}} diff --git a/ghost/admin/app/components/modals/newsletters/new.js b/ghost/admin/app/components/modals/newsletters/new.js index 34d2b69b3a..0661e7189d 100644 --- a/ghost/admin/app/components/modals/newsletters/new.js +++ b/ghost/admin/app/components/modals/newsletters/new.js @@ -49,7 +49,7 @@ export default class NewNewsletterModal extends Component { }); // Re-fetch newsletter data to refresh counts - yield this.store.query('newsletter', {include: 'count.members,count.posts', limit: 'all'}); + yield this.store.query('newsletter', {include: 'count.active_members,count.posts', limit: 'all'}); this.args.data.afterSave?.(result); return result; diff --git a/ghost/admin/app/components/settings/newsletters/newsletter-management.hbs b/ghost/admin/app/components/settings/newsletters/newsletter-management.hbs index af4b96592a..66d7af7d9c 100644 --- a/ghost/admin/app/components/settings/newsletters/newsletter-management.hbs +++ b/ghost/admin/app/components/settings/newsletters/newsletter-management.hbs @@ -71,7 +71,7 @@ {{/if}}

-

{{format-number newsletter.count.members}}

+

{{format-number newsletter.count.active_members}}

Subscribers

diff --git a/ghost/admin/app/components/settings/newsletters/newsletter-management.js b/ghost/admin/app/components/settings/newsletters/newsletter-management.js index 32d8f306de..2396af3878 100644 --- a/ghost/admin/app/components/settings/newsletters/newsletter-management.js +++ b/ghost/admin/app/components/settings/newsletters/newsletter-management.js @@ -132,7 +132,7 @@ export default class NewsletterManagementComponent extends Component { @task *loadNewslettersTask() { - const newsletters = yield this.store.query('newsletter', {include: 'count.members,count.posts', limit: 'all'}); + const newsletters = yield this.store.query('newsletter', {include: 'count.active_members,count.posts', limit: 'all'}); this.updateFilteredNewsletters(); diff --git a/ghost/admin/app/models/newsletter.js b/ghost/admin/app/models/newsletter.js index 45d567ecdb..4c725c0399 100644 --- a/ghost/admin/app/models/newsletter.js +++ b/ghost/admin/app/models/newsletter.js @@ -43,13 +43,13 @@ export default class Newsletter extends Model.extend(ValidationEngine) { @attr _meta; /** - * The filter that we should use to filter out members that are subscribed to this newsletter + * The filter that we should use to filter out members that are actively subscribed to this newsletter */ get recipientFilter() { - const idFilter = 'newsletters.slug:' + this.slug; + const filter = [`newsletters.slug:${this.slug}`, 'email_disabled:0']; if (this.visibility === 'paid') { - return idFilter + '+status:-free'; + filter.push('status:-free'); } - return idFilter; + return filter.join('+'); } } diff --git a/ghost/admin/app/utils/publish-options.js b/ghost/admin/app/utils/publish-options.js index cd309039fa..438758c156 100644 --- a/ghost/admin/app/utils/publish-options.js +++ b/ghost/admin/app/utils/publish-options.js @@ -298,7 +298,7 @@ export default class PublishOptions { // newsletters if (!this.user.isContributor) { - promises.push(this.store.query('newsletter', {status: 'active', limit: 'all', include: 'count.members'})); + promises.push(this.store.query('newsletter', {status: 'active', limit: 'all', include: 'count.active_members'})); } yield Promise.all(promises); diff --git a/ghost/admin/tests/acceptance/editor/publish-flow-test.js b/ghost/admin/tests/acceptance/editor/publish-flow-test.js index 7945d56c31..c84a6f77c5 100644 --- a/ghost/admin/tests/acceptance/editor/publish-flow-test.js +++ b/ghost/admin/tests/acceptance/editor/publish-flow-test.js @@ -156,8 +156,8 @@ describe('Acceptance: Publish flow', function () { // at least one member is required for publish+send to be available const label = this.server.create('label'); - this.server.createList('member', 3, {status: 'free', labels: [label]}); - this.server.createList('member', 4, {status: 'paid'}); + this.server.createList('member', 3, {status: 'free', email_disabled: 0, labels: [label]}); + this.server.createList('member', 4, {status: 'paid', email_disabled: 0}); }); it('can publish+send with single newsletter', async function () { @@ -270,7 +270,7 @@ describe('Acceptance: Publish flow', function () { subscribeOnSignup: true }); - this.server.create('member', {newsletters: [newsletter], status: 'free'}); + this.server.create('member', {newsletters: [newsletter], status: 'free', email_disabled: 0}); await loginAsRole('Administrator', this.server); const post = this.server.create('post', {status: 'draft'}); diff --git a/ghost/core/core/server/api/endpoints/newsletters.js b/ghost/core/core/server/api/endpoints/newsletters.js index 93ef1a7b05..8fa840e1da 100644 --- a/ghost/core/core/server/api/endpoints/newsletters.js +++ b/ghost/core/core/server/api/endpoints/newsletters.js @@ -1,5 +1,5 @@ const models = require('../../models'); -const allowedIncludes = ['count.posts', 'count.members']; +const allowedIncludes = ['count.posts', 'count.members', 'count.active_members']; const newslettersService = require('../../services/newsletters'); diff --git a/ghost/core/core/server/data/migrations/versions/5.56/2023-07-14-10-11-12-add-email-disabled-field-to-members.js b/ghost/core/core/server/data/migrations/versions/5.56/2023-07-14-10-11-12-add-email-disabled-field-to-members.js new file mode 100644 index 0000000000..507600c258 --- /dev/null +++ b/ghost/core/core/server/data/migrations/versions/5.56/2023-07-14-10-11-12-add-email-disabled-field-to-members.js @@ -0,0 +1,7 @@ +const {createAddColumnMigration} = require('../../utils'); + +module.exports = createAddColumnMigration('members', 'email_disabled', { + type: 'boolean', + nullable: false, + defaultTo: false +}); diff --git a/ghost/core/core/server/data/migrations/versions/5.56/2023-07-15-10-11-12-update-members-email-disabled-field.js b/ghost/core/core/server/data/migrations/versions/5.56/2023-07-15-10-11-12-update-members-email-disabled-field.js new file mode 100644 index 0000000000..98c4394bcc --- /dev/null +++ b/ghost/core/core/server/data/migrations/versions/5.56/2023-07-15-10-11-12-update-members-email-disabled-field.js @@ -0,0 +1,22 @@ +const logging = require('@tryghost/logging'); +const {createTransactionalMigration} = require('../../utils'); + +module.exports = createTransactionalMigration( + async function up(knex) { + logging.info('Setting email_disabled to true for all members that have their email on the suppression list'); + + await knex('members') + .join('suppressions', 'members.email', 'suppressions.email') + .update({ + email_disabled: true + }); + }, + async function down(knex) { + logging.info('Setting email_disabled to false for all members'); + + await knex('members') + .update({ + email_disabled: false + }); + } +); diff --git a/ghost/core/core/server/data/schema/schema.js b/ghost/core/core/server/data/schema/schema.js index 24c2558045..e256bd30c5 100644 --- a/ghost/core/core/server/data/schema/schema.js +++ b/ghost/core/core/server/data/schema/schema.js @@ -429,6 +429,7 @@ module.exports = { email_count: {type: 'integer', unsigned: true, nullable: false, defaultTo: 0}, email_opened_count: {type: 'integer', unsigned: true, nullable: false, defaultTo: 0}, email_open_rate: {type: 'integer', unsigned: true, nullable: true, index: true}, + email_disabled: {type: 'boolean', nullable: false, defaultTo: false}, last_seen_at: {type: 'dateTime', nullable: true}, last_commented_at: {type: 'dateTime', nullable: true}, created_at: {type: 'dateTime', nullable: false}, diff --git a/ghost/core/core/server/models/member.js b/ghost/core/core/server/models/member.js index e870aa4b03..903c77fdff 100644 --- a/ghost/core/core/server/models/member.js +++ b/ghost/core/core/server/models/member.js @@ -471,7 +471,11 @@ const Member = ghostBookshelf.Model.extend({ // we use raw queries instead of model relationships because model hydration is expensive const query = ghostBookshelf.knex('members_newsletters') .join('newsletters', 'members_newsletters.newsletter_id', '=', 'newsletters.id') - .where('newsletters.status', 'active') + .join('members', 'members_newsletters.member_id', '=', 'members.id') + .where({ + 'newsletters.status': 'active', + 'members.email_disabled': false + }) .distinct('member_id as id'); if (unfilteredOptions.transacting) { diff --git a/ghost/core/core/server/models/newsletter.js b/ghost/core/core/server/models/newsletter.js index e9783cd3e6..ed6f1ddd80 100644 --- a/ghost/core/core/server/models/newsletter.js +++ b/ghost/core/core/server/models/newsletter.js @@ -151,6 +151,16 @@ const Newsletter = ghostBookshelf.Model.extend({ .whereRaw('members_newsletters.newsletter_id = newsletters.id') .as('count__members'); }); + }, + active_members(modelOrCollection) { + modelOrCollection.query('columns', 'newsletters.*', (qb) => { + qb.count('members_newsletters.id') + .from('members_newsletters') + .join('members', 'members.id', 'members_newsletters.member_id') + .whereRaw('members_newsletters.newsletter_id = newsletters.id') + .andWhere('members.email_disabled', false) + .as('count__active_members'); + }); } }; }, diff --git a/ghost/core/core/server/services/members/middleware.js b/ghost/core/core/server/services/members/middleware.js index 42a58c51a4..05e726e13a 100644 --- a/ghost/core/core/server/services/members/middleware.js +++ b/ghost/core/core/server/services/members/middleware.js @@ -105,11 +105,10 @@ const deleteSuppression = async function (req, res) { try { const member = await membersService.ssr.getMemberDataFromSession(req, res); const options = { - id: member.id, - withRelated: ['newsletters'] + id: member.id }; await emailSuppressionList.removeEmail(member.email); - await membersService.api.members.update({subscribed: true}, options); + await membersService.api.members.update({email_disabled: false}, options); res.writeHead(204); res.end(); diff --git a/ghost/core/test/e2e-api/admin/__snapshots__/activity-feed.test.js.snap b/ghost/core/test/e2e-api/admin/__snapshots__/activity-feed.test.js.snap index 28eb8be520..fb66b96e57 100644 --- a/ghost/core/test/e2e-api/admin/__snapshots__/activity-feed.test.js.snap +++ b/ghost/core/test/e2e-api/admin/__snapshots__/activity-feed.test.js.snap @@ -22699,7 +22699,7 @@ exports[`Activity Feed API Can filter events by post id 2: [headers] 1`] = ` Object { "access-control-allow-origin": "http://127.0.0.1:2369", "cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0", - "content-length": "20437", + "content-length": "20667", "content-type": "application/json; charset=utf-8", "content-version": StringMatching /v\\\\d\\+\\\\\\.\\\\d\\+/, "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/, @@ -23852,7 +23852,7 @@ exports[`Activity Feed API Returns email delivered events in activity feed 2: [h Object { "access-control-allow-origin": "http://127.0.0.1:2369", "cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0", - "content-length": "1338", + "content-length": "1361", "content-type": "application/json; charset=utf-8", "content-version": StringMatching /v\\\\d\\+\\\\\\.\\\\d\\+/, "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/, @@ -23886,7 +23886,7 @@ exports[`Activity Feed API Returns email opened events in activity feed 2: [head Object { "access-control-allow-origin": "http://127.0.0.1:2369", "cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0", - "content-length": "1335", + "content-length": "1358", "content-type": "application/json; charset=utf-8", "content-version": StringMatching /v\\\\d\\+\\\\\\.\\\\d\\+/, "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/, @@ -23944,7 +23944,7 @@ exports[`Activity Feed API Returns email sent events in activity feed 2: [header Object { "access-control-allow-origin": "http://127.0.0.1:2369", "cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0", - "content-length": "5004", + "content-length": "5096", "content-type": "application/json; charset=utf-8", "content-version": StringMatching /v\\\\d\\+\\\\\\.\\\\d\\+/, "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/, @@ -24204,7 +24204,7 @@ exports[`Activity Feed API Returns signup events in activity feed 2: [headers] 1 Object { "access-control-allow-origin": "http://127.0.0.1:2369", "cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0", - "content-length": "23443", + "content-length": "23627", "content-type": "application/json; charset=utf-8", "content-version": StringMatching /v\\\\d\\+\\\\\\.\\\\d\\+/, "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/, diff --git a/ghost/core/test/e2e-api/admin/__snapshots__/members.test.js.snap b/ghost/core/test/e2e-api/admin/__snapshots__/members.test.js.snap index a043c1f8f1..95d72030ca 100644 --- a/ghost/core/test/e2e-api/admin/__snapshots__/members.test.js.snap +++ b/ghost/core/test/e2e-api/admin/__snapshots__/members.test.js.snap @@ -316,7 +316,7 @@ exports[`Members API - member attribution Returns sign up attributions of all ty Object { "access-control-allow-origin": "http://127.0.0.1:2369", "cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0", - "content-length": "9427", + "content-length": "9542", "content-type": "application/json; charset=utf-8", "content-version": StringMatching /v\\\\d\\+\\\\\\.\\\\d\\+/, "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/, @@ -5174,7 +5174,7 @@ exports[`Members API Can subscribe to a newsletter 5: [headers] 1`] = ` Object { "access-control-allow-origin": "http://127.0.0.1:2369", "cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0", - "content-length": "5686", + "content-length": "5778", "content-type": "application/json; charset=utf-8", "content-version": StringMatching /v\\\\d\\+\\\\\\.\\\\d\\+/, "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/, diff --git a/ghost/core/test/e2e-api/admin/__snapshots__/newsletters.test.js.snap b/ghost/core/test/e2e-api/admin/__snapshots__/newsletters.test.js.snap index 84c947ee14..c4fc78b0c0 100644 --- a/ghost/core/test/e2e-api/admin/__snapshots__/newsletters.test.js.snap +++ b/ghost/core/test/e2e-api/admin/__snapshots__/newsletters.test.js.snap @@ -1462,7 +1462,68 @@ Object { } `; -exports[`Newsletters API Can include members & posts counts when browsing newsletters 1: [body] 1`] = ` +exports[`Newsletters API Can include members, active members & posts counts when adding a newsletter 1: [body] 1`] = ` +Object { + "newsletters": Array [ + Object { + "background_color": "light", + "body_font_category": "serif", + "border_color": null, + "count": Object { + "active_members": 0, + "members": 0, + "posts": 0, + }, + "created_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/, + "description": null, + "feedback_enabled": false, + "footer_content": null, + "header_image": null, + "id": StringMatching /\\[a-f0-9\\]\\{24\\}/, + "name": "My test newsletter 2", + "sender_email": null, + "sender_name": "Test", + "sender_reply_to": "newsletter", + "show_badge": true, + "show_comment_cta": true, + "show_feature_image": true, + "show_header_icon": true, + "show_header_name": true, + "show_header_title": true, + "show_latest_posts": false, + "show_post_title_section": true, + "show_subscription_details": false, + "slug": "my-test-newsletter-2", + "sort_order": 4, + "status": "active", + "subscribe_on_signup": true, + "title_alignment": "center", + "title_color": null, + "title_font_category": "serif", + "updated_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/, + "uuid": StringMatching /\\[a-f0-9\\]\\{8\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{12\\}/, + "visibility": "members", + }, + ], +} +`; + +exports[`Newsletters API Can include members, active members & posts counts when adding a newsletter 2: [headers] 1`] = ` +Object { + "access-control-allow-origin": "http://127.0.0.1:2369", + "cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0", + "content-length": "913", + "content-type": "application/json; charset=utf-8", + "content-version": StringMatching /v\\\\d\\+\\\\\\.\\\\d\\+/, + "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/, + "location": StringMatching /https\\?:\\\\/\\\\/\\.\\*\\?\\\\/newsletters\\\\/\\[a-f0-9\\]\\{24\\}\\\\//, + "vary": "Accept-Version, Origin, Accept-Encoding", + "x-cache-invalidate": "/*", + "x-powered-by": "Express", +} +`; + +exports[`Newsletters API Can include members, active members & posts counts when browsing newsletters 1: [body] 1`] = ` Object { "meta": Object { "pagination": Object { @@ -1480,6 +1541,7 @@ Object { "body_font_category": "sans_serif", "border_color": null, "count": Object { + "active_members": 0, "members": 0, "posts": 0, }, @@ -1518,6 +1580,7 @@ Object { "body_font_category": "serif", "border_color": null, "count": Object { + "active_members": 4, "members": 4, "posts": 0, }, @@ -1556,6 +1619,7 @@ Object { "body_font_category": "serif", "border_color": null, "count": Object { + "active_members": 3, "members": 3, "posts": 0, }, @@ -1594,6 +1658,7 @@ Object { "body_font_category": "serif", "border_color": null, "count": Object { + "active_members": 2, "members": 2, "posts": 0, }, @@ -1631,11 +1696,11 @@ Object { } `; -exports[`Newsletters API Can include members & posts counts when browsing newsletters 2: [headers] 1`] = ` +exports[`Newsletters API Can include members, active members & posts counts when browsing newsletters 2: [headers] 1`] = ` Object { "access-control-allow-origin": "http://127.0.0.1:2369", "cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0", - "content-length": "3795", + "content-length": "3871", "content-type": "application/json; charset=utf-8", "content-version": StringMatching /v\\\\d\\+\\\\\\.\\\\d\\+/, "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/, @@ -1644,7 +1709,7 @@ Object { } `; -exports[`Newsletters API Can include members & posts counts when editing newsletters 1: [body] 1`] = ` +exports[`Newsletters API Can include members, active members & posts counts when editing newsletters 1: [body] 1`] = ` Object { "newsletters": Array [ Object { @@ -1652,6 +1717,7 @@ Object { "body_font_category": "serif", "border_color": null, "count": Object { + "active_members": 4, "members": 4, "posts": 0, }, @@ -1689,11 +1755,11 @@ Object { } `; -exports[`Newsletters API Can include members & posts counts when editing newsletters 2: [headers] 1`] = ` +exports[`Newsletters API Can include members, active members & posts counts when editing newsletters 2: [headers] 1`] = ` Object { "access-control-allow-origin": "http://127.0.0.1:2369", "cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0", - "content-length": "963", + "content-length": "982", "content-type": "application/json; charset=utf-8", "content-version": StringMatching /v\\\\d\\+\\\\\\.\\\\d\\+/, "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/, @@ -1703,7 +1769,7 @@ Object { } `; -exports[`Newsletters API Can include members & posts counts when reading a newsletter 1: [body] 1`] = ` +exports[`Newsletters API Can include members, active members & posts counts when reading a newsletter 1: [body] 1`] = ` Object { "newsletters": Array [ Object { @@ -1711,6 +1777,7 @@ Object { "body_font_category": "serif", "border_color": null, "count": Object { + "active_members": 4, "members": 4, "posts": 0, }, @@ -1748,11 +1815,11 @@ Object { } `; -exports[`Newsletters API Can include members & posts counts when reading a newsletter 2: [headers] 1`] = ` +exports[`Newsletters API Can include members, active members & posts counts when reading a newsletter 2: [headers] 1`] = ` Object { "access-control-allow-origin": "http://127.0.0.1:2369", "cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0", - "content-length": "954", + "content-length": "973", "content-type": "application/json; charset=utf-8", "content-version": StringMatching /v\\\\d\\+\\\\\\.\\\\d\\+/, "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/, diff --git a/ghost/core/test/e2e-api/admin/members-exporter.test.js b/ghost/core/test/e2e-api/admin/members-exporter.test.js index 9e5c27a91e..97b9f76aac 100644 --- a/ghost/core/test/e2e-api/admin/members-exporter.test.js +++ b/ghost/core/test/e2e-api/admin/members-exporter.test.js @@ -11,6 +11,7 @@ async function createMember(data) { const member = await models.Member.add({ email: uuid.v4() + '@example.com', name: '', + email_disabled: false, ...data }); diff --git a/ghost/core/test/e2e-api/admin/newsletters.test.js b/ghost/core/test/e2e-api/admin/newsletters.test.js index 264a3d3f1e..eca76b7808 100644 --- a/ghost/core/test/e2e-api/admin/newsletters.test.js +++ b/ghost/core/test/e2e-api/admin/newsletters.test.js @@ -72,9 +72,9 @@ describe('Newsletters API', function () { }); }); - it('Can include members & posts counts when browsing newsletters', async function () { + it('Can include members, active members & posts counts when browsing newsletters', async function () { await agent - .get(`newsletters/?include=count.members,count.posts`) + .get(`newsletters/?include=count.members,count.active_members,count.posts`) .expectStatus(200) .matchBodySnapshot({ newsletters: new Array(4).fill(newsletterSnapshot) @@ -85,9 +85,9 @@ describe('Newsletters API', function () { }); }); - it('Can include members & posts counts when reading a newsletter', async function () { + it('Can include members, active members & posts counts when reading a newsletter', async function () { await agent - .get(`newsletters/${fixtureManager.get('newsletters', 0).id}/?include=count.members,count.posts`) + .get(`newsletters/${fixtureManager.get('newsletters', 0).id}/?include=count.members,count.active_members,count.posts`) .expectStatus(200) .matchBodySnapshot({ newsletters: new Array(1).fill(newsletterSnapshot) @@ -143,7 +143,7 @@ describe('Newsletters API', function () { assert.equal(header_image, transformReadyPath); }); - it('Can include members & posts counts when adding a newsletter', async function () { + it('Can include members, active members & posts counts when adding a newsletter', async function () { const newsletter = { name: 'My test newsletter 2', sender_name: 'Test', @@ -160,7 +160,7 @@ describe('Newsletters API', function () { }; await agent - .post(`newsletters/?include=count.members,count.posts`) + .post(`newsletters/?include=count.members,count.active_members,count.posts`) .body({newsletters: [newsletter]}) .expectStatus(201) .matchBodySnapshot({ @@ -305,9 +305,9 @@ describe('Newsletters API', function () { }); }); - it('Can include members & posts counts when editing newsletters', async function () { + it('Can include members, active members & posts counts when editing newsletters', async function () { const id = fixtureManager.get('newsletters', 0).id; - await agent.put(`newsletters/${id}/?include=count.members,count.posts`) + await agent.put(`newsletters/${id}/?include=count.members,count.active_members,count.posts`) .body({ newsletters: [{ name: 'Updated newsletter name 2' diff --git a/ghost/core/test/e2e-api/members/__snapshots__/webhooks.test.js.snap b/ghost/core/test/e2e-api/members/__snapshots__/webhooks.test.js.snap index 41c30d859f..47222a9c7a 100644 --- a/ghost/core/test/e2e-api/members/__snapshots__/webhooks.test.js.snap +++ b/ghost/core/test/e2e-api/members/__snapshots__/webhooks.test.js.snap @@ -429,7 +429,7 @@ exports[`Members API Member attribution Returns subscription created attribution Object { "access-control-allow-origin": "http://127.0.0.1:2369", "cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0", - "content-length": "7960", + "content-length": "8144", "content-type": "application/json; charset=utf-8", "content-version": StringMatching /v\\\\d\\+\\\\\\.\\\\d\\+/, "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/, diff --git a/ghost/core/test/e2e-api/members/middleware.test.js b/ghost/core/test/e2e-api/members/middleware.test.js index 5823f96285..21624349c3 100644 --- a/ghost/core/test/e2e-api/members/middleware.test.js +++ b/ghost/core/test/e2e-api/members/middleware.test.js @@ -1,7 +1,7 @@ const {agentProvider, mockManager, fixtureManager, matchers} = require('../../utils/e2e-framework'); const {anyEtag, anyObjectId, anyUuid, anyISODateTime} = matchers; const models = require('../../../core/server/models'); -require('should'); +const should = require('should'); let membersAgent; @@ -32,10 +32,6 @@ const memberMatcherUnserialised = (newslettersCount) => { }; }; -async function getDefaultNewsletters() { - return (await models.Newsletter.findAll({filter: 'status:active+subscribe_on_signup:true+visibility:members'})).models; -} - describe('Comments API', function () { before(async function () { membersAgent = await agentProvider.getMembersAPIAgent(); @@ -202,43 +198,31 @@ describe('Comments API', function () { member.get('enable_comment_notifications').should.eql(true); }); - it('can remove member from suppression list and resubscribe to default newsletters', async function () { - const newsletters = await getDefaultNewsletters(); + it('can remove a member\'s email from the suppression list', async function () { + // add member's email to the suppression list + await models.Suppression.add({ + email: member.get('email'), + reason: 'bounce' + }); - // unsubscribe member from all newsletters - await membersAgent - .put(`/api/member/`) - .body({ - newsletters: [] - }) - .expectStatus(200) - .matchHeaderSnapshot({ - etag: anyEtag - }) - .matchBodySnapshot(memberMatcher(0)) - .expect(({body}) => { - body.newsletters.should.eql([]); - }); + // disable member's email + await member.save({email_disabled: true}); - // remove email from suppression list + // remove suppression await membersAgent .delete(`/api/member/suppression`) .expectStatus(204) .expectEmptyBody(); - // check that member re-subscribed to default newsletters after removing from suppression list - await membersAgent - .get(`/api/member/`) - .expectStatus(200) - .matchHeaderSnapshot({ - etag: anyEtag - }) - .matchBodySnapshot(memberMatcher(2)) - .expect(({body}) => { - // body should contain default newsletters - body.newsletters[0].id.should.eql(newsletters[0].get('id')); - body.newsletters[1].id.should.eql(newsletters[1].get('id')); - }); + // check that member is removed from suppression list + const suppression = await models.Suppression.findOne({email: member.get('email')}); + + should(suppression).be.null(); + + // check that member's email is enabled + await member.refresh(); + + should(member.get('email_disabled')).be.false(); }); }); }); diff --git a/ghost/core/test/integration/services/email-service/batch-sending.test.js b/ghost/core/test/integration/services/email-service/batch-sending.test.js index 6f69db08d2..8e4a582207 100644 --- a/ghost/core/test/integration/services/email-service/batch-sending.test.js +++ b/ghost/core/test/integration/services/email-service/batch-sending.test.js @@ -207,7 +207,8 @@ describe('Batch sending tests', function () { status: 'free', newsletters: [{ id: fixtureManager.get('newsletters', 0).id - }] + }], + email_disabled: false }); return r; @@ -652,7 +653,8 @@ describe('Batch sending tests', function () { labels: [{name: 'replacements-tests'}], newsletters: [{ id: fixtureManager.get('newsletters', 0).id - }] + }], + email_disabled: false }); const {html, plaintext} = await sendEmail(agent, { @@ -685,7 +687,8 @@ describe('Batch sending tests', function () { labels: [{name: 'replacements-tests-2'}], newsletters: [{ id: fixtureManager.get('newsletters', 0).id - }] + }], + email_disabled: false }); const {html, plaintext} = await sendEmail(agent, { @@ -859,7 +862,8 @@ describe('Batch sending tests', function () { labels: [{name: 'subscription-box-tests'}], newsletters: [{ id: fixtureManager.get('newsletters', 0).id - }] + }], + email_disabled: false }); mockSetting('email_track_clicks', false); // Disable link replacement for this test @@ -892,7 +896,8 @@ describe('Batch sending tests', function () { newsletters: [{ id: fixtureManager.get('newsletters', 0).id }], - status: 'comped' + status: 'comped', + email_disabled: false }); mockSetting('email_track_clicks', false); // Disable link replacement for this test diff --git a/ghost/core/test/regression/models/model_member_stripe_customer.test.js b/ghost/core/test/regression/models/model_member_stripe_customer.test.js index 1a0b22b96b..b05783e800 100644 --- a/ghost/core/test/regression/models/model_member_stripe_customer.test.js +++ b/ghost/core/test/regression/models/model_member_stripe_customer.test.js @@ -18,7 +18,8 @@ describe('MemberStripeCustomer Model', function run() { const context = testUtils.context.admin; const member = await Member.add({ - email: 'test@test.test' + email: 'test@test.test', + email_disabled: false }); const product = await Product.add({ @@ -83,7 +84,8 @@ describe('MemberStripeCustomer Model', function run() { it('Is correctly mapped to the member', async function () { const context = testUtils.context.admin; const member = await Member.add({ - email: 'test@test.member' + email: 'test@test.member', + email_disabled: false }, context); await MemberStripeCustomer.add({ @@ -110,7 +112,8 @@ describe('MemberStripeCustomer Model', function run() { it('Cascades to members_stripe_customers_subscriptions', async function () { const context = testUtils.context.admin; const member = await Member.add({ - email: 'test@test.member' + email: 'test@test.member', + email_disabled: false }, context); await MemberStripeCustomer.add({ diff --git a/ghost/core/test/regression/models/model_members.test.js b/ghost/core/test/regression/models/model_members.test.js index 9f39f400dc..15a6c38e3a 100644 --- a/ghost/core/test/regression/models/model_members.test.js +++ b/ghost/core/test/regression/models/model_members.test.js @@ -22,7 +22,8 @@ describe('Member Model', function run() { const context = testUtils.context.admin; await Member.add({ email: 'test@test.member', - labels: [] + labels: [], + email_disabled: false }, context); const member = await Member.findOne({ email: 'test@test.member' @@ -115,7 +116,8 @@ describe('Member Model', function run() { it('Is correctly mapped to the stripe customers', async function () { const context = testUtils.context.admin; const testMember = await Member.add({ - email: 'test@test.member' + email: 'test@test.member', + email_disabled: false }, context); await MemberStripeCustomer.add({ @@ -159,7 +161,8 @@ describe('Member Model', function run() { labels: [{ name: 'A label', slug: 'a-unique-slug-for-testing-members-model' - }] + }], + email_disabled: false }, context); const member = await Member.findOne({ email: 'test@test.member' @@ -284,7 +287,8 @@ describe('Member Model', function run() { email: 'testing-products@test.member', products: [{ id: product.id - }] + }], + email_disabled: false }, { ...context, withRelated: ['products'] @@ -317,7 +321,8 @@ describe('Member Model', function run() { email: 'filter-test@test.member', products: [{ id: vipProduct.id - }] + }], + email_disabled: false }, context); const member = await Member.findOne({ @@ -362,7 +367,8 @@ describe('Member Model', function run() { name: 'VIP', slug: 'vip', type: 'paid' - }] + }], + email_disabled: false }, context); const member = await Member.findOne({ @@ -386,7 +392,8 @@ describe('Member Model', function run() { name: 'VIP', slug: 'vip', type: 'paid' - }] + }], + email_disabled: false }, context); const member = await Member.findOne({ @@ -406,7 +413,8 @@ describe('Member Model', function run() { const member = await Member.add({ email: 'test@test.member', - labels: [] + labels: [], + email_disabled: false }, context); const product = await Product.add({ @@ -511,7 +519,8 @@ describe('Member Model', function run() { let email = 'test@offers.com'; const member = await Member.add({ email: email, - labels: [] + labels: [], + email_disabled: false }, context); const product = await Product.add({ diff --git a/ghost/core/test/regression/models/model_stripe_customer_subscription.test.js b/ghost/core/test/regression/models/model_stripe_customer_subscription.test.js index 9e72adafcf..63a15f47f5 100644 --- a/ghost/core/test/regression/models/model_stripe_customer_subscription.test.js +++ b/ghost/core/test/regression/models/model_stripe_customer_subscription.test.js @@ -17,7 +17,8 @@ describe('StripeCustomerSubscription Model', function run() { it('Is correctly mapped to the stripe customer', async function () { const context = testUtils.context.admin; const member = await Member.add({ - email: 'test@test.member' + email: 'test@test.member', + email_disabled: false }, context); await MemberStripeCustomer.add({ member_id: member.get('id'), diff --git a/ghost/core/test/unit/server/data/schema/integrity.test.js b/ghost/core/test/unit/server/data/schema/integrity.test.js index add03ba571..f560f569ff 100644 --- a/ghost/core/test/unit/server/data/schema/integrity.test.js +++ b/ghost/core/test/unit/server/data/schema/integrity.test.js @@ -35,7 +35,7 @@ const validateRouteSettings = require('../../../../../core/server/services/route */ describe('DB version integrity', function () { // Only these variables should need updating - const currentSchemaHash = '8e299caf33efbcf8a12c9cefaf8f6500'; + const currentSchemaHash = '66b77c27bab9cb43e1076fbb6c6d1233'; const currentFixturesHash = 'af43eef1ac4f14fc1bc0ea351300420f'; const currentSettingsHash = '4f23a583335dcb4cb3fae553122ea200'; const currentRoutesHash = '3d180d52c663d173a6be791ef411ed01'; diff --git a/ghost/core/test/utils/fixtures/data-generator.js b/ghost/core/test/utils/fixtures/data-generator.js index 55f46e18f4..f0b2647722 100644 --- a/ghost/core/test/utils/fixtures/data-generator.js +++ b/ghost/core/test/utils/fixtures/data-generator.js @@ -302,14 +302,16 @@ DataGenerator.Content = { email: 'member1@test.com', name: 'Mr Egg', uuid: 'f6f91461-d7d8-4a3f-aa5d-8e582c40b340', - status: 'free' + status: 'free', + email_disabled: false }, { id: ObjectId().toHexString(), email: 'member2@test.com', email_open_rate: 50, uuid: 'f6f91461-d7d8-4a3f-aa5d-8e582c40b341', - status: 'free' + status: 'free', + email_disabled: false }, { id: ObjectId().toHexString(), @@ -317,28 +319,32 @@ DataGenerator.Content = { name: 'Egon Spengler', email_open_rate: 80, uuid: 'f6f91461-d7d8-4a3f-aa5d-8e582c40b342', - status: 'paid' + status: 'paid', + email_disabled: false }, { id: ObjectId().toHexString(), email: 'trialing@test.com', name: 'Ray Stantz', uuid: 'f6f91461-d7d8-4a3f-aa5d-8e582c40b343', - status: 'paid' + status: 'paid', + email_disabled: false }, { id: ObjectId().toHexString(), email: 'comped@test.com', name: 'Vinz Clortho', uuid: 'f6f91461-d7d8-4a3f-aa5d-8e582c40b344', - status: 'paid' + status: 'paid', + email_disabled: false }, { id: ObjectId().toHexString(), email: 'vip@test.com', name: 'Winston Zeddemore', uuid: 'f6f91461-d7d8-4a3f-aa5d-8e582c40b345', - status: 'free' + status: 'free', + email_disabled: false }, { id: ObjectId().toHexString(), @@ -346,7 +352,8 @@ DataGenerator.Content = { name: 'Peter Venkman', uuid: 'f6f91461-d7d8-4a3f-aa5d-8e582c40b346', status: 'paid', - subscribed: false + subscribed: false, + email_disabled: false }, { id: ObjectId().toHexString(), @@ -354,7 +361,8 @@ DataGenerator.Content = { name: 'Dana Barrett', uuid: 'f6f91461-d7d8-4a3f-aa5d-8e582c40b347', status: 'paid', - subscribed: false + subscribed: false, + email_disabled: false } ], diff --git a/ghost/email-service/lib/EmailSegmenter.js b/ghost/email-service/lib/EmailSegmenter.js index c24c01582b..ccdf61f26a 100644 --- a/ghost/email-service/lib/EmailSegmenter.js +++ b/ghost/email-service/lib/EmailSegmenter.js @@ -26,7 +26,7 @@ class EmailSegmenter { } getMemberFilterForSegment(newsletter, emailRecipientFilter, segment) { - const filter = [`newsletters.id:${newsletter.id}`]; + const filter = [`newsletters.id:${newsletter.id}`, 'email_disabled:0']; switch (emailRecipientFilter) { case 'all': diff --git a/ghost/email-service/test/email-segmenter.test.js b/ghost/email-service/test/email-segmenter.test.js index 9c69efd80e..49606ad7aa 100644 --- a/ghost/email-service/test/email-segmenter.test.js +++ b/ghost/email-service/test/email-segmenter.test.js @@ -37,7 +37,7 @@ describe('Email segmenter', function () { ); listStub.calledOnce.should.be.true(); listStub.calledWith({ - filter: 'newsletters.id:newsletter-123' + filter: 'newsletters.id:newsletter-123+email_disabled:0' }).should.be.true(); response.should.eql(12); }); @@ -94,7 +94,7 @@ describe('Email segmenter', function () { listStub.calledOnce.should.be.true(); listStub.calledWith({ - filter: 'newsletters.id:newsletter-123+(labels:test)+status:-free' + filter: 'newsletters.id:newsletter-123+email_disabled:0+(labels:test)+status:-free' }).should.be.true(); response.should.eql(12); }); @@ -118,7 +118,7 @@ describe('Email segmenter', function () { listStub.calledOnce.should.be.true(); listStub.calledWith({ - filter: 'newsletters.id:newsletter-123+(labels:test)+(status:free)' + filter: 'newsletters.id:newsletter-123+email_disabled:0+(labels:test)+(status:free)' }).should.be.true(); response.should.eql(12); }); diff --git a/ghost/members-api/lib/members-api.js b/ghost/members-api/lib/members-api.js index ded91ecde2..0774341cc4 100644 --- a/ghost/members-api/lib/members-api.js +++ b/ghost/members-api/lib/members-api.js @@ -355,7 +355,7 @@ module.exports = function MembersAPI({ if (!member) { return; } - await memberRepository.update({newsletters: []}, {id: member.id}); + await memberRepository.update({email_disabled: true}, {id: member.id}); }); return { diff --git a/ghost/members-api/lib/repositories/MemberRepository.js b/ghost/members-api/lib/repositories/MemberRepository.js index 9be5df4fef..2cd5555825 100644 --- a/ghost/members-api/lib/repositories/MemberRepository.js +++ b/ghost/members-api/lib/repositories/MemberRepository.js @@ -224,6 +224,7 @@ module.exports = class MemberRepository { * @param {Object} [data.stripeCustomer] * @param {string} [data.offerId] * @param {import('@tryghost/member-attribution/lib/Attribution').AttributionResource} [data.attribution] + * @param {boolean} [data.email_disabled] * @param {*} options * @returns */ @@ -247,7 +248,7 @@ module.exports = class MemberRepository { }); } - const memberData = _.pick(data, ['email', 'name', 'note', 'subscribed', 'geolocation', 'created_at', 'products', 'newsletters']); + const memberData = _.pick(data, ['email', 'name', 'note', 'subscribed', 'geolocation', 'created_at', 'products', 'newsletters', 'email_disabled']); // Throw error if email is invalid using latest validator if (!validator.isEmail(memberData.email, {legacy: false})) { @@ -257,6 +258,8 @@ module.exports = class MemberRepository { }); } + memberData.email_disabled = !!memberData.email_disabled; + if (memberData.products && memberData.products.length > 1) { throw new errors.BadRequestError({message: tpl(messages.moreThanOneProduct)}); } @@ -415,7 +418,8 @@ module.exports = class MemberRepository { 'enable_comment_notifications', 'last_seen_at', 'last_commented_at', - 'expertise' + 'expertise', + 'email_disabled' ]); // Trim whitespaces from expertise