From 2fbaa7b9bcaa24e021ee9bcd5d400ce4f308a622 Mon Sep 17 00:00:00 2001 From: Rishabh Date: Fri, 9 Sep 2022 19:59:06 +0530 Subject: [PATCH] Moved member email alert trigger to member creation closes https://github.com/TryGhost/Team/issues/1864 refs https://github.com/TryGhost/Team/issues/1881 - triggers free member email alert via event dispatch from member create method - passes subscription/stripe data to member creation for paid members so free member alert can be ignored for them - moves subscription created event being called from webhook controller to `linkSubscription`, allows creating subscription events for all new subscriptions instead of ones just via webhooks --- .../e2e-api/members/send-magic-link.test.js | 8 ++ ghost/members-api/lib/MembersAPI.js | 5 -- ghost/members-api/lib/repositories/member.js | 34 +++++++- ghost/stripe/lib/WebhookController.js | 80 +++++++++---------- 4 files changed, 78 insertions(+), 49 deletions(-) diff --git a/ghost/core/test/e2e-api/members/send-magic-link.test.js b/ghost/core/test/e2e-api/members/send-magic-link.test.js index bfc285d409..37527bbe79 100644 --- a/ghost/core/test/e2e-api/members/send-magic-link.test.js +++ b/ghost/core/test/e2e-api/members/send-magic-link.test.js @@ -3,6 +3,12 @@ const should = require('should'); let membersAgent, membersService; +async function sleep(ms) { + return new Promise((resolve) => { + setTimeout(resolve, ms); + }); +} + describe('sendMagicLink', function () { before(async function () { const agents = await agentProvider.getAgentsForMembers(); @@ -127,6 +133,8 @@ describe('sendMagicLink', function () { // Get member data from token const data = await membersService.api.getMemberDataFromMagicLinkToken(token); + // Wait for the dispatched events (because this happens async) + await sleep(250); // Check member alert is sent to site owners mockManager.assert.sentEmail({ to: 'jbloggs@example.com', diff --git a/ghost/members-api/lib/MembersAPI.js b/ghost/members-api/lib/MembersAPI.js index f84335dce8..901425c725 100644 --- a/ghost/members-api/lib/MembersAPI.js +++ b/ghost/members-api/lib/MembersAPI.js @@ -232,11 +232,6 @@ module.exports = function MembersAPI({ const newMember = await users.create({name, email, labels, newsletters, attribution, geolocation}); - // Notify staff users of new free member signup - if (labsService.isSet('emailAlerts')) { - await staffService.notifyFreeMemberSignup(newMember.toJSON()); - } - await MemberLoginEvent.add({member_id: newMember.id}); return getMemberIdentityData(email); } diff --git a/ghost/members-api/lib/repositories/member.js b/ghost/members-api/lib/repositories/member.js index a311737a7c..7642c513fc 100644 --- a/ghost/members-api/lib/repositories/member.js +++ b/ghost/members-api/lib/repositories/member.js @@ -3,7 +3,7 @@ const errors = require('@tryghost/errors'); const logging = require('@tryghost/logging'); const tpl = require('@tryghost/tpl'); const DomainEvents = require('@tryghost/domain-events'); -const {MemberCreatedEvent, SubscriptionCreatedEvent, MemberSubscribeEvent} = require('@tryghost/member-events'); +const {MemberCreatedEvent, SubscriptionCreatedEvent, MemberSubscribeEvent, SubscriptionCancelledEvent} = require('@tryghost/member-events'); const ObjectId = require('bson-objectid'); const {NotFoundError} = require('@tryghost/errors'); @@ -203,6 +203,8 @@ module.exports = class MemberRepository { * @param {Date} [data.created_at] * @param {Object[]} [data.products] * @param {Object[]} [data.newsletters] + * @param {Object} [data.stripeCustomer] + * @param {string} [data.offerId] * @param {import('@tryghost/member-attribution/lib/history').Attribution} [data.attribution] * @param {*} options * @returns @@ -212,7 +214,7 @@ module.exports = class MemberRepository { options = {}; } - const {labels} = data; + const {labels, stripeCustomer, offerId, attribution} = data; if (labels) { labels.forEach((label, index) => { @@ -308,6 +310,34 @@ module.exports = class MemberRepository { }, eventData.created_at)); } + // For paid members created via stripe checkout webhook event, link subscription + if (stripeCustomer) { + await this.upsertCustomer({ + member_id: member.id, + customer_id: stripeCustomer.id, + name: stripeCustomer.name, + email: stripeCustomer.email + }); + + for (const subscription of stripeCustomer.subscriptions.data) { + try { + await this.linkSubscription({ + id: member.id, + subscription, + offerId, + attribution + }); + } catch (err) { + if (err.code !== 'ER_DUP_ENTRY' && err.code !== 'SQLITE_CONSTRAINT') { + throw err; + } + throw new errors.ConflictError({ + err + }); + } + } + } + DomainEvents.dispatch(MemberCreatedEvent.create({ memberId: member.id, attribution: data.attribution, diff --git a/ghost/stripe/lib/WebhookController.js b/ghost/stripe/lib/WebhookController.js index bb9cd3ac7f..db0760a04a 100644 --- a/ghost/stripe/lib/WebhookController.js +++ b/ghost/stripe/lib/WebhookController.js @@ -1,8 +1,6 @@ const _ = require('lodash'); const logging = require('@tryghost/logging'); const errors = require('@tryghost/errors'); -const DomainEvents = require('@tryghost/domain-events'); -const {SubscriptionCreatedEvent} = require('@tryghost/member-events'); module.exports = class WebhookController { /** @@ -229,11 +227,11 @@ module.exports = class WebhookController { id: session.metadata.attribution_id ?? null, url: session.metadata.attribution_url ?? null, type: session.metadata.attribution_type ?? null - }; + }; const payerName = _.get(customer, 'subscriptions.data[0].default_payment_method.billing_details.name'); const name = metadataName || payerName || null; - + const memberData = {email: customer.email, name, attribution}; if (metadataNewsletters) { try { @@ -242,57 +240,55 @@ module.exports = class WebhookController { logging.error(`Ignoring invalid newsletters data - ${metadataNewsletters}.`); } } - member = await this.deps.memberRepository.create(memberData); + + const offerId = session.metadata?.offer; + + const memberDataWithStripeCustomer = { + ...memberData, + stripeCustomer: customer, + offerId + }; + member = await this.deps.memberRepository.create(memberDataWithStripeCustomer); } else { const payerName = _.get(customer, 'subscriptions.data[0].default_payment_method.billing_details.name'); + const attribution = { + id: session.metadata?.attribution_id ?? null, + url: session.metadata?.attribution_url ?? null, + type: session.metadata?.attribution_type ?? null + }; if (payerName && !member.get('name')) { await this.deps.memberRepository.update({name: payerName}, {id: member.get('id')}); } - } - await this.deps.memberRepository.upsertCustomer({ - customer_id: customer.id, - member_id: member.id, - name: customer.name, - email: customer.email - }); + await this.deps.memberRepository.upsertCustomer({ + customer_id: customer.id, + member_id: member.id, + name: customer.name, + email: customer.email + }); - for (const subscription of customer.subscriptions.data) { - try { - const offerId = session.metadata?.offer; + for (const subscription of customer.subscriptions.data) { + try { + const offerId = session.metadata?.offer; - await this.deps.memberRepository.linkSubscription({ - id: member.id, - subscription, - offerId - }); - } catch (err) { - if (err.code !== 'ER_DUP_ENTRY' && err.code !== 'SQLITE_CONSTRAINT') { - throw err; + await this.deps.memberRepository.linkSubscription({ + id: member.id, + subscription, + offerId, + attribution + }); + } catch (err) { + if (err.code !== 'ER_DUP_ENTRY' && err.code !== 'SQLITE_CONSTRAINT') { + throw err; + } + throw new errors.ConflictError({ + err + }); } - throw new errors.ConflictError({ - err - }); } } - const subscription = await this.deps.memberRepository.getSubscriptionByStripeID(session.subscription); - - // TODO: should we check if we don't send this event multiple times if Stripe calls the webhook multiple times? - const event = SubscriptionCreatedEvent.create({ - memberId: member.id, - subscriptionId: subscription.id, - offerId: session.metadata.offer || null, - attribution: { - id: session.metadata.attribution_id ?? null, - url: session.metadata.attribution_url ?? null, - type: session.metadata.attribution_type ?? null - } - }); - - DomainEvents.dispatch(event); - if (checkoutType !== 'upgrade') { this.sendSignupEmail(customer.email); }