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 d13876af42..2c78512aab 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 @@ -356,6 +356,149 @@ Object { } `; +exports[`Members API Adding newsletters to member with no subscriptions works even with subscribed false 1: [body] 1`] = ` +Object { + "members": Array [ + Object { + "attribution": Object { + "id": null, + "referrer_medium": "Ghost Admin", + "referrer_source": "Created manually", + "referrer_url": null, + "title": null, + "type": "url", + "url": null, + }, + "avatar_image": null, + "comped": false, + "created_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/, + "email": "memberAddNewsletterSubscribed@test.com", + "email_count": 0, + "email_open_rate": null, + "email_opened_count": 0, + "email_suppression": Object { + "info": null, + "suppressed": false, + }, + "geolocation": null, + "id": StringMatching /\\[a-f0-9\\]\\{24\\}/, + "labels": Any, + "last_seen_at": null, + "name": "test newsletter", + "newsletters": Array [], + "note": null, + "status": "free", + "subscribed": false, + "subscriptions": Any, + "tiers": Array [], + "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\\}/, + }, + ], +} +`; + +exports[`Members API Adding newsletters to member with no subscriptions works even with subscribed false 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": "694", + "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\\?:\\\\/\\\\/\\.\\*\\?\\\\/members\\\\/\\[a-f0-9\\]\\{24\\}\\\\//, + "vary": "Accept-Version, Origin, Accept-Encoding", + "x-powered-by": "Express", +} +`; + +exports[`Members API Adding newsletters to member with no subscriptions works even with subscribed false 3: [body] 1`] = ` +Object { + "members": Array [ + Object { + "attribution": Object { + "id": null, + "referrer_medium": "Ghost Admin", + "referrer_source": "Created manually", + "referrer_url": null, + "title": null, + "type": "url", + "url": null, + }, + "avatar_image": null, + "comped": false, + "created_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/, + "email": "memberAddNewsletterSubscribed@test.com", + "email_count": 0, + "email_open_rate": null, + "email_opened_count": 0, + "email_suppression": Object { + "info": null, + "suppressed": false, + }, + "geolocation": null, + "id": StringMatching /\\[a-f0-9\\]\\{24\\}/, + "labels": Any, + "last_seen_at": null, + "name": "test newsletter", + "newsletters": Array [ + Object { + "body_font_category": "serif", + "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": "http://127.0.0.1:2369/content/images/2022/05/test.jpg", + "id": StringMatching /\\[a-f0-9\\]\\{24\\}/, + "name": "Daily newsletter", + "sender_email": "jamie@example.com", + "sender_name": "Jamie", + "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": "daily-newsletter", + "sort_order": 1, + "status": "active", + "subscribe_on_signup": false, + "title_alignment": "center", + "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", + }, + ], + "note": null, + "status": "free", + "subscribed": true, + "subscriptions": Any, + "tiers": Array [], + "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\\}/, + }, + ], +} +`; + +exports[`Members API Adding newsletters to member with no subscriptions works even with subscribed false 4: [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": "1531", + "content-type": "application/json; charset=utf-8", + "content-version": StringMatching /v\\\\d\\+\\\\\\.\\\\d\\+/, + "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/, + "vary": "Accept-Version, Origin, Accept-Encoding", + "x-powered-by": "Express", +} +`; + exports[`Members API Bulk operations Can bulk delete a label from members 1: [body] 1`] = ` Object { "bulk": Object { diff --git a/ghost/core/test/e2e-api/admin/members.test.js b/ghost/core/test/e2e-api/admin/members.test.js index 538c771a13..30480c8ee2 100644 --- a/ghost/core/test/e2e-api/admin/members.test.js +++ b/ghost/core/test/e2e-api/admin/members.test.js @@ -2442,6 +2442,71 @@ describe('Members API', function () { assert.ok(changedMember.newsletters.find(n => n.id === testUtils.DataGenerator.Content.newsletters[1].id), 'The member is still subscribed for the newsletter it subscribed to'); }); + it('Adding newsletters to member with no subscriptions works even with subscribed false', async function () { + // Add member with no subscriptions + const member = { + name: 'test newsletter', + email: 'memberAddNewsletterSubscribed@test.com', + newsletters: [] + }; + + const {body} = await agent + .post(`/members/`) + .body({members: [member]}) + .expectStatus(201) + .matchBodySnapshot({ + members: [{ + id: anyObjectId, + uuid: anyUuid, + created_at: anyISODateTime, + updated_at: anyISODateTime, + subscriptions: anyArray, + labels: anyArray, + newsletters: Array(0).fill(newsletterSnapshot) + }] + }) + .matchHeaderSnapshot({ + 'content-version': anyContentVersion, + etag: anyEtag, + location: anyLocationFor('members') + }); + + const memberId = body.members[0].id; + + const editedMember = { + subscribed: false, + newsletters: [ + { + id: testUtils.DataGenerator.Content.newsletters[0].id + } + ] + }; + + // Edit member + const {body: body2} = await agent + .put(`/members/${memberId}`) + .body({members: [editedMember]}) + .expectStatus(200) + .matchBodySnapshot({ + members: [{ + id: anyObjectId, + uuid: anyUuid, + created_at: anyISODateTime, + updated_at: anyISODateTime, + subscriptions: anyArray, + labels: anyArray, + newsletters: Array(1).fill(newsletterSnapshot) + }] + }) + .matchHeaderSnapshot({ + 'content-version': anyContentVersion, + etag: anyEtag + }); + const changedMember = body2.members[0]; + assert.equal(changedMember.newsletters.length, 1); + assert.ok(changedMember.newsletters.find(n => n.id === testUtils.DataGenerator.Content.newsletters[0].id), 'The member should be subscribed to the newsletter'); + }); + it('Updating member data without newsletters does not change newsletters', async function () { // check that this newsletter is archived, or this test would not make sense const archivedNewsletterId = testUtils.DataGenerator.Content.newsletters[2].id; diff --git a/ghost/members-api/lib/repositories/member.js b/ghost/members-api/lib/repositories/member.js index 2351f2e150..de844f7027 100644 --- a/ghost/members-api/lib/repositories/member.js +++ b/ghost/members-api/lib/repositories/member.js @@ -539,12 +539,14 @@ module.exports = class MemberRepository { if (needsNewsletters) { const existingNewsletters = initialMember.related('newsletters').models; - // This maps the old subscribed property to the new newsletters field - if (memberData.subscribed === false) { - memberData.newsletters = []; - } else if (memberData.subscribed === true && !existingNewsletters.find(n => n.get('status') === 'active')) { - const browseOptions = _.pick(options, 'transacting'); - memberData.newsletters = await this.getSubscribeOnSignupNewsletters(browseOptions); + // This maps the old subscribed property to the new newsletters field and is only used to keep backward compatibility + if (!memberData.newsletters) { + if (memberData.subscribed === false) { + memberData.newsletters = []; + } else if (memberData.subscribed === true && !existingNewsletters.find(n => n.get('status') === 'active')) { + const browseOptions = _.pick(options, 'transacting'); + memberData.newsletters = await this.getSubscribeOnSignupNewsletters(browseOptions); + } } // only ever populated with active newsletters - never archived ones