diff --git a/ghost/admin/app/styles/layouts/members.css b/ghost/admin/app/styles/layouts/members.css index a4739ba101..c51e9a3bd7 100644 --- a/ghost/admin/app/styles/layouts/members.css +++ b/ghost/admin/app/styles/layouts/members.css @@ -2564,9 +2564,10 @@ p.gh-members-import-errordetail:first-of-type { .gh-cp-membertier-attribution .gh-tier-card-header { display: flex; align-items: flex-start; + position: relative; } -.gh-cp-membertier-attribution .gh-tier-card-header:nth-of-type(2) { +.gh-cp-membertier-attribution .gh-tier-card-header:not(:first-child) { margin-top: 16px; border-top: 1px solid var(--whitegrey); padding-top: 16px; @@ -2609,8 +2610,12 @@ p.gh-members-import-errordetail:first-of-type { .gh-cp-membertier-attribution .action-menu { position: absolute; - top: 24px; - right: 24px; + top: 0px; + right: 0px; +} + +.gh-cp-membertier-attribution .gh-tier-card-header:not(:first-child) .action-menu { + top: 16px; } .gh-cp-membertier-attribution span.archived { diff --git a/ghost/core/test/unit/server/services/staff/index.test.js b/ghost/core/test/unit/server/services/staff/index.test.js index 8a2f8f3999..c52ed93168 100644 --- a/ghost/core/test/unit/server/services/staff/index.test.js +++ b/ghost/core/test/unit/server/services/staff/index.test.js @@ -6,7 +6,7 @@ const DomainEvents = require('@tryghost/domain-events'); const {mockManager} = require('../../../../utils/e2e-framework'); const models = require('../../../../../core/server/models'); -const {SubscriptionCreatedEvent, SubscriptionCancelledEvent, MemberCreatedEvent} = require('@tryghost/member-events'); +const {SubscriptionCancelledEvent, MemberCreatedEvent, SubscriptionActivatedEvent} = require('@tryghost/member-events'); describe('Staff Service:', function () { before(function () { @@ -132,7 +132,7 @@ describe('Staff Service:', function () { it('sends email for member source', async function () { await staffService.init(); - DomainEvents.dispatch(SubscriptionCreatedEvent.create({ + DomainEvents.dispatch(SubscriptionActivatedEvent.create({ source: 'member', ...eventData })); @@ -148,7 +148,7 @@ describe('Staff Service:', function () { it('sends email for api source', async function () { await staffService.init(); - DomainEvents.dispatch(SubscriptionCreatedEvent.create({ + DomainEvents.dispatch(SubscriptionActivatedEvent.create({ source: 'api', ...eventData })); @@ -164,7 +164,7 @@ describe('Staff Service:', function () { it('does not send email for importer source', async function () { await staffService.init(); - DomainEvents.dispatch(SubscriptionCreatedEvent.create({ + DomainEvents.dispatch(SubscriptionActivatedEvent.create({ source: 'import', ...eventData })); diff --git a/ghost/member-events/index.js b/ghost/member-events/index.js index 2cdf54bd3b..4f6fdb94ed 100644 --- a/ghost/member-events/index.js +++ b/ghost/member-events/index.js @@ -7,8 +7,9 @@ module.exports = { MemberPaidConverstionEvent: require('./lib/MemberPaidConversionEvent'), MemberPaidCancellationEvent: require('./lib/MemberPaidCancellationEvent'), MemberPageViewEvent: require('./lib/MemberPageViewEvent'), - SubscriptionCreatedEvent: require('./lib/SubscriptionCreatedEvent'), MemberCommentEvent: require('./lib/MemberCommentEvent'), + SubscriptionCreatedEvent: require('./lib/SubscriptionCreatedEvent'), + SubscriptionActivatedEvent: require('./lib/SubscriptionActivatedEvent'), SubscriptionCancelledEvent: require('./lib/SubscriptionCancelledEvent'), MemberLinkClickEvent: require('./lib/MemberLinkClickEvent') }; diff --git a/ghost/member-events/lib/SubscriptionActivatedEvent.js b/ghost/member-events/lib/SubscriptionActivatedEvent.js new file mode 100644 index 0000000000..033a077949 --- /dev/null +++ b/ghost/member-events/lib/SubscriptionActivatedEvent.js @@ -0,0 +1,30 @@ +/** + * Fired when a subscription becomes active. + * + * @typedef {object} SubscriptionActivatedEventData + * @prop {string} source + * @prop {string} memberId + * @prop {string} batchId + * @prop {string} tierId + * @prop {string} subscriptionId + * @prop {string} offerId + */ + +module.exports = class SubscriptionActivatedEvent { + /** + * @param {SubscriptionActivatedEventData} data + * @param {Date} timestamp + */ + constructor(data, timestamp) { + this.data = data; + this.timestamp = timestamp; + } + + /** + * @param {SubscriptionActivatedEventData} data + * @param {Date} [timestamp] + */ + static create(data, timestamp) { + return new SubscriptionActivatedEvent(data, timestamp ?? new Date); + } +}; diff --git a/ghost/member-events/lib/SubscriptionCreatedEvent.js b/ghost/member-events/lib/SubscriptionCreatedEvent.js index e8f303fa51..a31d6207b1 100644 --- a/ghost/member-events/lib/SubscriptionCreatedEvent.js +++ b/ghost/member-events/lib/SubscriptionCreatedEvent.js @@ -1,4 +1,6 @@ /** + * Fired when a subscription is created. This can also happen when inactive subscriptions are created (incomplete, canceled...). + * * @typedef {object} SubscriptionCreatedEventData * @prop {string} source * @prop {string} memberId diff --git a/ghost/members-api/lib/repositories/member.js b/ghost/members-api/lib/repositories/member.js index 8156f6a61b..d036b1a861 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, SubscriptionCancelledEvent} = require('@tryghost/member-events'); +const {SubscriptionActivatedEvent, MemberCreatedEvent, SubscriptionCreatedEvent, MemberSubscribeEvent, SubscriptionCancelledEvent} = require('@tryghost/member-events'); const ObjectId = require('bson-objectid').default; const {NotFoundError} = require('@tryghost/errors'); const validator = require('@tryghost/validator'); @@ -970,6 +970,25 @@ module.exports = class MemberRepository { offer_id: offerId }; + const getStatus = (modelToCheck) => { + const status = modelToCheck.get('status'); + const canceled = modelToCheck.get('cancel_at_period_end'); + + if (status === 'canceled') { + return 'expired'; + } + + if (canceled) { + return 'canceled'; + } + + if (this.isActiveSubscriptionStatus(status)) { + return 'active'; + } + + return 'inactive'; + }; + let eventData = {}; if (model) { // CASE: Offer is already mapped against sub, don't overwrite it with NULL @@ -986,25 +1005,6 @@ module.exports = class MemberRepository { const originalMrrDelta = model.get('mrr'); const updatedMrrDelta = updated.get('mrr'); - const getStatus = (modelToCheck) => { - const status = modelToCheck.get('status'); - const canceled = modelToCheck.get('cancel_at_period_end'); - - if (status === 'canceled') { - return 'expired'; - } - - if (canceled) { - return 'canceled'; - } - - if (this.isActiveSubscriptionStatus(status)) { - return 'active'; - } - - return 'inactive'; - }; - const getEventName = (originalStatus, updatedStatus) => { if (originalStatus === updatedStatus) { return 'updated'; @@ -1031,6 +1031,24 @@ module.exports = class MemberRepository { currency: subscriptionPriceData.currency, mrr_delta: mrrDelta }, options); + + // Did we activate this subscription? + // This happens when an incomplete subscription is completed + // This always happens during the 3D secure flow, so it is important to catch + if (originalStatus !== 'active' && updatedStatus === 'active') { + const context = options?.context || {}; + const source = this._resolveContextSource(context); + + const event = SubscriptionActivatedEvent.create({ + source, + tierId: ghostProduct?.get('id'), + memberId: member.id, + subscriptionId: updated.get('id'), + offerId: offerId, + batchId: options.batch_id + }); + this.dispatchEvent(event, options); + } } } else { eventData.created_at = new Date(subscription.start_date * 1000); @@ -1060,6 +1078,18 @@ module.exports = class MemberRepository { batchId: options.batch_id }); this.dispatchEvent(event, options); + + if (getStatus(subscriptionModel) === 'active') { + const activatedEvent = SubscriptionActivatedEvent.create({ + source, + tierId: ghostProduct?.get('id'), + memberId: member.id, + subscriptionId: subscriptionModel.get('id'), + offerId: offerId, + batchId: options.batch_id + }); + this.dispatchEvent(activatedEvent, options); + } } let memberProducts = (await member.related('products').fetch(options)).toJSON(); diff --git a/ghost/members-api/lib/services/member-bread.js b/ghost/members-api/lib/services/member-bread.js index 1cc2a452e0..d5c6efb2b9 100644 --- a/ghost/members-api/lib/services/member-bread.js +++ b/ghost/members-api/lib/services/member-bread.js @@ -64,9 +64,12 @@ module.exports = class MemberBREADService { } const subscriptionProducts = (member.subscriptions || []) - .filter(sub => sub.status !== 'canceled') + .filter(sub => this.memberRepository.isActiveSubscriptionStatus(sub.status)) .map(sub => sub.price.product.product_id); + // Remove incomplete subscriptions from the API + member.subscriptions = member.subscriptions.filter(sub => sub.status !== 'incomplete' && sub.status !== 'incomplete_expired'); + for (const product of member.products) { if (!subscriptionProducts.includes(product.id)) { const productAddEvent = member.productEvents.find(event => event.product_id === product.id); diff --git a/ghost/staff-service/lib/staff-service.js b/ghost/staff-service/lib/staff-service.js index 0b6fe42b72..3bba8f5e84 100644 --- a/ghost/staff-service/lib/staff-service.js +++ b/ghost/staff-service/lib/staff-service.js @@ -1,4 +1,4 @@ -const {MemberCreatedEvent, SubscriptionCancelledEvent, SubscriptionCreatedEvent} = require('@tryghost/member-events'); +const {MemberCreatedEvent, SubscriptionCancelledEvent, SubscriptionActivatedEvent} = require('@tryghost/member-events'); const {MentionCreatedEvent} = require('@tryghost/webmentions'); // @NOTE: 'StaffService' is a vague name that does not describe what it's actually doing. @@ -93,7 +93,7 @@ class StaffService { if (type === MemberCreatedEvent && member.status === 'free') { await this.emails.notifyFreeMemberSignup(member); - } else if (type === SubscriptionCreatedEvent) { + } else if (type === SubscriptionActivatedEvent) { await this.emails.notifyPaidSubscriptionStarted({ member, offer, @@ -116,16 +116,16 @@ class StaffService { try { await this.handleEvent(MemberCreatedEvent, event); } catch (e) { - this.logging.error(`Failed to notify free member signup - ${event?.data?.memberId}`); + this.logging.error(e, `Failed to notify free member signup - ${event?.data?.memberId}`); } }); // Trigger email on paid subscription start - this.DomainEvents.subscribe(SubscriptionCreatedEvent, async (event) => { + this.DomainEvents.subscribe(SubscriptionActivatedEvent, async (event) => { try { - await this.handleEvent(SubscriptionCreatedEvent, event); + await this.handleEvent(SubscriptionActivatedEvent, event); } catch (e) { - this.logging.error(`Failed to notify paid member subscription start - ${event?.data?.memberId}`); + this.logging.error(e, `Failed to notify paid member subscription start - ${event?.data?.memberId}`); } }); @@ -134,7 +134,7 @@ class StaffService { try { await this.handleEvent(SubscriptionCancelledEvent, event); } catch (e) { - this.logging.error(`Failed to notify paid member subscription cancel - ${event?.data?.memberId}`); + this.logging.error(e, `Failed to notify paid member subscription cancel - ${event?.data?.memberId}`); } }); @@ -143,7 +143,7 @@ class StaffService { try { await this.handleEvent(MentionCreatedEvent, event); } catch (e) { - this.logging.error(`Failed to notify webmention`); + this.logging.error(e, `Failed to notify webmention`); } }); } diff --git a/ghost/staff-service/test/staff-service.test.js b/ghost/staff-service/test/staff-service.test.js index f2ab18a81a..c3756c0c10 100644 --- a/ghost/staff-service/test/staff-service.test.js +++ b/ghost/staff-service/test/staff-service.test.js @@ -1,7 +1,7 @@ // Switch these lines once there are useful utils // const testUtils = require('./utils'); const sinon = require('sinon'); -const {MemberCreatedEvent, SubscriptionCancelledEvent, SubscriptionCreatedEvent} = require('@tryghost/member-events'); +const {MemberCreatedEvent, SubscriptionCancelledEvent, SubscriptionActivatedEvent} = require('@tryghost/member-events'); const {MentionCreatedEvent} = require('@tryghost/webmentions'); require('./utils'); @@ -183,7 +183,7 @@ describe('StaffService', function () { it('subscribes to events', async function () { service.subscribeEvents(); subscribeStub.callCount.should.eql(4); - subscribeStub.calledWith(SubscriptionCreatedEvent).should.be.true(); + subscribeStub.calledWith(SubscriptionActivatedEvent).should.be.true(); subscribeStub.calledWith(SubscriptionCancelledEvent).should.be.true(); subscribeStub.calledWith(MemberCreatedEvent).should.be.true(); subscribeStub.calledWith(MentionCreatedEvent).should.be.true(); @@ -287,7 +287,7 @@ describe('StaffService', function () { }); it('handles paid member created event', async function () { - await service.handleEvent(SubscriptionCreatedEvent, { + await service.handleEvent(SubscriptionActivatedEvent, { data: { source: 'member', memberId: 'member-1', @@ -316,7 +316,7 @@ describe('StaffService', function () { sinon.match({subject: '⚠️ Cancellation: Jamie'}) ).should.be.true(); }); - + it('handles new mention notification', async function () { await service.handleEvent(MentionCreatedEvent, { data: {