OpenSourceArk/Ghost
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

🐛 Fixed subscriptions visible as "Active" within Ghost Admin (#16255)

Browse Source 
fixes https://github.com/TryGhost/Team/issues/2542 
fixes https://github.com/TryGhost/Team/issues/2543 
fixes https://github.com/TryGhost/Team/issues/2544

- Hides incomplete subscriptions
- Shows Past Due subscriptions
- Fixed UI issues with 3+ subscriptions
- Fixed missing complimentary subscription when one subscription was
incomplete/inactive
- Fixed sending a paid subscription started email for incomplete
subscriptions. This change also required us to actually send the email
when the incomplete subscription eventually becomes active. So the
introduction of a new `SubscriptionActivatedEvent` made sense/was
required (because sending a SubscriptionCreatedEvent again would cause
other issues).
This commit is contained in:
Simon Backx 2023-02-13 13:07:53 +01:00 committed by GitHub
parent 87f207a3ea
commit 77032262c4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 112 additions and 41 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
ghost/admin/app/styles/layouts/members.css
View File

@ -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 {

8
ghost/core/test/unit/server/services/staff/index.test.js
View File

@ -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
}));

3
ghost/member-events/index.js
View File

@ -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')
};

30
ghost/member-events/lib/SubscriptionActivatedEvent.js Normal file
View File

@ -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);
}
};

2
ghost/member-events/lib/SubscriptionCreatedEvent.js
View File

@ -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

70
ghost/members-api/lib/repositories/member.js
View File

@ -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();

5
ghost/members-api/lib/services/member-bread.js
View File

@ -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);

16
ghost/staff-service/lib/staff-service.js
View File

@ -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`);
}
});
}

8
ghost/staff-service/test/staff-service.test.js
View File

@ -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: {