From f423d018afe0720262e33b9b26739f37a42f305d Mon Sep 17 00:00:00 2001 From: Ronald Langeveld Date: Thu, 1 Feb 2024 13:56:02 +0200 Subject: [PATCH 1/4] =?UTF-8?q?=F0=9F=90=9B=20Fixed=20members=20events=20f?= =?UTF-8?q?or=20archived=20newsletters?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fixes https://linear.app/tryghost/issue/ENG-604/🐛-members-events-show-member-subscribed-to-archived-newsletter - Members Subscribe events are being created from Archived newsletters that the Member may have subscribed to in the past prior to the newsletter being archived. - This fixes a bug where it doesn't take that into account and would create an Event for subscribing back to those newsletters even though its not the case, which causes some confusion for publishers in Member Events. --- ghost/members-api/lib/repositories/MemberRepository.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/ghost/members-api/lib/repositories/MemberRepository.js b/ghost/members-api/lib/repositories/MemberRepository.js index 4f03192d212..33c3839c53b 100644 --- a/ghost/members-api/lib/repositories/MemberRepository.js +++ b/ghost/members-api/lib/repositories/MemberRepository.js @@ -556,10 +556,10 @@ module.exports = class MemberRepository { // Keep track of the newsletters that were added and removed of a member so we can generate the corresponding events let newslettersToAdd = []; let newslettersToRemove = []; + let newslettersToIgnore = []; // This is used to keep track of archived newsletters where members are still subscribed to if (needsNewsletters) { const existingNewsletters = initialMember.related('newsletters').models; - // 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) { @@ -572,12 +572,15 @@ module.exports = class MemberRepository { // only ever populated with active newsletters - never archived ones if (memberData.newsletters) { + const archivedNewsletters = existingNewsletters.filter(n => n.get('status') === 'archived'); const existingNewsletterIds = existingNewsletters .filter(newsletter => newsletter.attributes.status !== 'archived') .map(newsletter => newsletter.id); const incomingNewsletterIds = memberData.newsletters.map(newsletter => newsletter.id); - + newslettersToIgnore = archivedNewsletters.map(n => n.id); newslettersToAdd = _.differenceWith(incomingNewsletterIds, existingNewsletterIds); + // make sure newslettersToAdd does not contain newslettersToIgnore (archived newsletters since that creates false events) + newslettersToAdd = _.differenceWith(newslettersToAdd, newslettersToIgnore); newslettersToRemove = _.differenceWith(existingNewsletterIds, incomingNewsletterIds); } From 2cd6698f97eecfdd7829c8cfa9712f9ff443f75b Mon Sep 17 00:00:00 2001 From: Ronald Langeveld Date: Thu, 1 Feb 2024 14:23:52 +0200 Subject: [PATCH 2/4] Cleaned up comments --- ghost/members-api/lib/repositories/MemberRepository.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ghost/members-api/lib/repositories/MemberRepository.js b/ghost/members-api/lib/repositories/MemberRepository.js index 33c3839c53b..05fdef67687 100644 --- a/ghost/members-api/lib/repositories/MemberRepository.js +++ b/ghost/members-api/lib/repositories/MemberRepository.js @@ -578,9 +578,9 @@ module.exports = class MemberRepository { .map(newsletter => newsletter.id); const incomingNewsletterIds = memberData.newsletters.map(newsletter => newsletter.id); newslettersToIgnore = archivedNewsletters.map(n => n.id); - newslettersToAdd = _.differenceWith(incomingNewsletterIds, existingNewsletterIds); + // make sure newslettersToAdd does not contain newslettersToIgnore (archived newsletters since that creates false events) - newslettersToAdd = _.differenceWith(newslettersToAdd, newslettersToIgnore); + newslettersToAdd = _.differenceWith(_.differenceWith(incomingNewsletterIds, existingNewsletterIds), newslettersToIgnore); newslettersToRemove = _.differenceWith(existingNewsletterIds, incomingNewsletterIds); } From 88594cdef7c450a64f79a35369644b23d7d148d5 Mon Sep 17 00:00:00 2001 From: Ronald Langeveld Date: Thu, 1 Feb 2024 14:47:05 +0200 Subject: [PATCH 3/4] Simplified code --- ghost/members-api/lib/repositories/MemberRepository.js | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/ghost/members-api/lib/repositories/MemberRepository.js b/ghost/members-api/lib/repositories/MemberRepository.js index 05fdef67687..3aa52efcec0 100644 --- a/ghost/members-api/lib/repositories/MemberRepository.js +++ b/ghost/members-api/lib/repositories/MemberRepository.js @@ -556,7 +556,7 @@ module.exports = class MemberRepository { // Keep track of the newsletters that were added and removed of a member so we can generate the corresponding events let newslettersToAdd = []; let newslettersToRemove = []; - let newslettersToIgnore = []; // This is used to keep track of archived newsletters where members are still subscribed to + // let newslettersToIgnore = []; // This is used to keep track of archived newsletters where members are still subscribed to if (needsNewsletters) { const existingNewsletters = initialMember.related('newsletters').models; @@ -572,15 +572,13 @@ module.exports = class MemberRepository { // only ever populated with active newsletters - never archived ones if (memberData.newsletters) { - const archivedNewsletters = existingNewsletters.filter(n => n.get('status') === 'archived'); + const archivedNewsletters = existingNewsletters.filter(n => n.get('status') === 'archived').map(n => n.id); const existingNewsletterIds = existingNewsletters .filter(newsletter => newsletter.attributes.status !== 'archived') .map(newsletter => newsletter.id); const incomingNewsletterIds = memberData.newsletters.map(newsletter => newsletter.id); - newslettersToIgnore = archivedNewsletters.map(n => n.id); - // make sure newslettersToAdd does not contain newslettersToIgnore (archived newsletters since that creates false events) - newslettersToAdd = _.differenceWith(_.differenceWith(incomingNewsletterIds, existingNewsletterIds), newslettersToIgnore); + newslettersToAdd = _.differenceWith(_.differenceWith(incomingNewsletterIds, existingNewsletterIds), archivedNewsletters); newslettersToRemove = _.differenceWith(existingNewsletterIds, incomingNewsletterIds); } From a47c12698cf3d3e05f2a7454f96943f1d56d5e13 Mon Sep 17 00:00:00 2001 From: Ronald Langeveld Date: Thu, 1 Feb 2024 14:48:21 +0200 Subject: [PATCH 4/4] Removed old comments --- ghost/members-api/lib/repositories/MemberRepository.js | 1 - 1 file changed, 1 deletion(-) diff --git a/ghost/members-api/lib/repositories/MemberRepository.js b/ghost/members-api/lib/repositories/MemberRepository.js index 3aa52efcec0..8b81ba29cdc 100644 --- a/ghost/members-api/lib/repositories/MemberRepository.js +++ b/ghost/members-api/lib/repositories/MemberRepository.js @@ -556,7 +556,6 @@ module.exports = class MemberRepository { // Keep track of the newsletters that were added and removed of a member so we can generate the corresponding events let newslettersToAdd = []; let newslettersToRemove = []; - // let newslettersToIgnore = []; // This is used to keep track of archived newsletters where members are still subscribed to if (needsNewsletters) { const existingNewsletters = initialMember.related('newsletters').models;