Replaced all usage of member models with members-api (#12117)

no-issue

* Added stripeSubscriptions relation to member model

This allows us to fetch the subscriptions for a member via standard
model usage, e.g. `withRelated: ['stripeSubscriptions']` rather than
offloading to loops and `decorateWithSubscriptions` functions, this is
more performant and less non-standard than the existing method.

* Updated serialize methods to match existing format

The current usage of `decorateWithSubscriptions` and the usage of
members throughout the codebase has a subscriptions array on a stripe
object on the member, this ensures that when we serialize members to
JSON that we are using the same format.

There is definitely room to change this in future, but this is an
attempt to create as few breaking changes as possible.

* Installed @tryghost/members-api@0.26.0

This includes the required API changes so that everywhere can use
members-api directly rather than models and/or helper methods
This commit is contained in:
Fabien 'egg' O'Carroll 2020-08-12 14:17:44 +01:00 committed by GitHub
parent c696d715c1
commit 1294e3f92c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 184 additions and 92 deletions

View File

@ -1,4 +1,3 @@
const models = require('../../models');
const {i18n} = require('../../lib/common');
const errors = require('@tryghost/errors');
const membersService = require('../../services/members');
@ -12,7 +11,7 @@ module.exports = {
],
permissions: true,
async query(frame) {
let model = await models.Member.findOne(frame.data, frame.options);
let model = await membersService.api.members.get(frame.data, frame.options);
if (!model) {
throw new errors.NotFoundError({

View File

@ -13,18 +13,6 @@ const logging = require('../../../shared/logging');
const db = require('../../data/db');
const _ = require('lodash');
const decorateWithSubscriptions = async function (member) {
// NOTE: this logic is here until relations between Members/MemberStripeCustomer/StripeCustomerSubscription
// are in place
const subscriptions = await membersService.api.members.getStripeSubscriptions(member);
return Object.assign(member, {
stripe: {
subscriptions
}
});
};
/** NOTE: this method should not exist at all and needs to be cleaned up
it was created due to a bug in how CSV is currently created for exports
Export bug was fixed in 3.6 but method exists to handle older csv exports with undefined
@ -90,20 +78,6 @@ function serializeMemberLabels(labels) {
return [];
}
const listMembers = async function (options) {
const res = (await models.Member.findPage(options));
const memberModels = res.data.map(model => model.toJSON(options));
const members = await Promise.all(memberModels.map(async function (member) {
return decorateWithSubscriptions(member);
}));
return {
members: members,
meta: res.meta
};
};
const findOrCreateLabels = async (labels, options) => {
const api = require('./index');
@ -145,7 +119,7 @@ const getUniqueMemberLabels = (members) => {
return _.uniq(allLabels);
};
const members = {
module.exports = {
docName: 'members',
hasActiveStripeSubscriptions: {
@ -174,7 +148,14 @@ const members = {
permissions: true,
validation: {},
async query(frame) {
return listMembers(frame.options);
frame.options.withRelated = ['labels', 'stripeSubscriptions', 'stripeSubscriptions.customer'];
const page = await membersService.api.members.list(frame.options);
const members = page.data.map(model => model.toJSON(frame.options));
return {
members: members,
meta: page.meta
};
}
},
@ -187,7 +168,8 @@ const members = {
validation: {},
permissions: true,
async query(frame) {
let model = await models.Member.findOne(frame.data, frame.options);
frame.options.withRelated = ['labels', 'stripeSubscriptions', 'stripeSubscriptions.customer'];
let model = await membersService.api.members.get(frame.data, frame.options);
if (!model) {
throw new errors.NotFoundError({
@ -195,9 +177,7 @@ const members = {
});
}
const member = model.toJSON(frame.options);
return decorateWithSubscriptions(member);
return model.toJSON(frame.options);
}
},
@ -220,12 +200,10 @@ const members = {
},
permissions: true,
async query(frame) {
let model;
let member;
frame.options.withRelated = ['stripeSubscriptions', 'stripeSubscriptions.customer'];
try {
model = await models.Member.add(frame.data.members[0], frame.options);
const member = model.toJSON(frame.options);
member = await membersService.api.members.create(frame.data.members[0], frame.options);
if (frame.data.members[0].stripe_customer_id) {
if (!membersService.config.isStripeConnected()) {
@ -244,10 +222,10 @@ const members = {
}
if (frame.options.send_email) {
await membersService.api.sendEmailWithMagicLink({email: model.get('email'), requestedType: frame.options.email_type});
await membersService.api.sendEmailWithMagicLink({email: member.get('email'), requestedType: frame.options.email_type});
}
return decorateWithSubscriptions(member);
return member.toJSON(frame.options);
} catch (error) {
if (error.code && error.message.toLowerCase().indexOf('unique') !== -1) {
throw new errors.ValidationError({
@ -260,21 +238,16 @@ const members = {
// It's a bit ugly doing regex matching to detect errors, but it's the easiest way that works without
// introducing additional logic/data format into current error handling
const isStripeLinkingError = error.message && (error.message.match(/customer|plan|subscription/g) || error.context === i18n.t('errors.api.members.stripeNotConnected.context'));
if (model && isStripeLinkingError) {
if (member && isStripeLinkingError) {
if (error.message.indexOf('customer') && error.code === 'resource_missing') {
error.message = `Member not imported. ${error.message}`;
error.context = i18n.t('errors.api.members.stripeCustomerNotFound.context');
error.help = i18n.t('errors.api.members.stripeCustomerNotFound.help');
}
const api = require('./index');
await api.members.destroy.query({
options: {
context: frame.options.context,
id: model.id
}
});
await membersService.api.members.destroy({
id: member.get('id')
}, frame.options);
}
throw error;
@ -297,24 +270,24 @@ const members = {
},
permissions: true,
async query(frame) {
const model = await models.Member.edit(frame.data.members[0], frame.options);
frame.options.withRelated = ['stripeSubscriptions'];
const member = await membersService.api.members.update(frame.data.members[0], frame.options);
const member = model.toJSON(frame.options);
const subscriptions = await membersService.api.members.getStripeSubscriptions(member);
const compedSubscriptions = subscriptions.filter(sub => (sub.plan.nickname === 'Complimentary'));
if (frame.data.members[0].comped !== undefined && (frame.data.members[0].comped !== compedSubscriptions)) {
const hasCompedSubscription = !!(compedSubscriptions.length);
const hasCompedSubscription = !!member.related('stripeSubscriptions').find(subscription => subscription.get('plan_nickname') === 'Complimentary');
if (typeof frame.data.members[0].comped === 'boolean') {
if (frame.data.members[0].comped && !hasCompedSubscription) {
await membersService.api.members.setComplimentarySubscription(member);
} else if (!(frame.data.members[0].comped) && hasCompedSubscription) {
await membersService.api.members.cancelComplimentarySubscription(member);
}
await member.load(['stripeSubscriptions']);
}
return decorateWithSubscriptions(member);
await member.load(['stripeSubscriptions.customer']);
return member.toJSON(frame.options);
}
},
@ -335,23 +308,9 @@ const members = {
permissions: true,
async query(frame) {
frame.options.require = true;
frame.options.cancelStripeSubscriptions = frame.options.cancel;
let member = await models.Member.findOne(frame.options);
if (!member) {
throw new errors.NotFoundError({
message: i18n.t('errors.api.resource.resourceNotFound', {
resource: 'Member'
})
});
}
if (frame.options.cancel === true) {
await membersService.api.members.cancelStripeSubscriptions(member);
}
// Wrapped in bluebird promise to allow "filtered catch"
await Promise.resolve(models.Member.destroy(frame.options))
await Promise.resolve(membersService.api.members.destroy(frame.options))
.catch(models.Member.NotFoundError, () => {
throw new errors.NotFoundError({
message: i18n.t('errors.api.resource.resourceNotFound', {
@ -385,8 +344,14 @@ const members = {
},
validation: {},
async query(frame) {
frame.options.withRelated = ['labels'];
return listMembers(frame.options);
frame.options.withRelated = ['labels', 'stripeSubscriptions', 'stripeSubscriptions.customer'];
const page = await membersService.api.members.list(frame.options);
const members = page.data.map(model => model.toJSON(frame.options));
return {
members: members,
meta: page.meta
};
}
},
@ -801,12 +766,9 @@ const members = {
}
}
};
// NOTE: remove below condition once batched import is production ready,
// remember to swap out current importCSV method when doing so
if (config.get('enableDeveloperExperiments')) {
members.importCSV = members.importCSVBatched;
delete members.importCSVBatched;
module.exports.importCSV = module.exports.importCSVBatched;
delete module.exports.importCSVBatched;
}
module.exports = members;

View File

@ -37,6 +37,30 @@ const Member = ghostBookshelf.Model.extend({
return this.hasMany('MemberStripeCustomer', 'member_id', 'id');
},
stripeSubscriptions() {
return this.belongsToMany(
'StripeCustomerSubscription',
'members_stripe_customers',
'member_id',
'customer_id',
'id',
'customer_id'
).query('whereIn', 'status', ['active', 'trialing']);
},
serialize(options) {
const defaultSerializedObject = ghostBookshelf.Model.prototype.serialize.call(this, options);
if (defaultSerializedObject.stripeSubscriptions) {
defaultSerializedObject.stripe = {
subscriptions: defaultSerializedObject.stripeSubscriptions
};
delete defaultSerializedObject.stripeSubscriptions;
}
return defaultSerializedObject;
},
emitChange: function emitChange(event, options) {
const eventToTrigger = 'member' + '.' + event;
ghostBookshelf.Model.prototype.emitChange.bind(this)(this, eventToTrigger, options);

View File

@ -1,11 +1,48 @@
const ghostBookshelf = require('./base');
const CURRENCY_SYMBOLS = {
usd: '$',
aud: '$',
cad: '$',
gbp: '£',
eur: '€',
inr: '₹'
};
const StripeCustomerSubscription = ghostBookshelf.Model.extend({
tableName: 'members_stripe_customers_subscriptions',
customer() {
return this.belongsTo('MemberStripeCustomer', 'customer_id', 'customer_id');
},
serialize(options) {
const defaultSerializedObject = ghostBookshelf.Model.prototype.serialize.call(this, options);
return {
id: defaultSerializedObject.subscription_id,
customer: {
id: defaultSerializedObject.customer_id,
// TODO? The customer is not fetched by default so these sometimes won't exist
name: defaultSerializedObject.customer ? defaultSerializedObject.customer.name : null,
email: defaultSerializedObject.customer ? defaultSerializedObject.customer.email : null
},
plan: {
id: defaultSerializedObject.plan_id,
nickname: defaultSerializedObject.plan_nickname,
amount: defaultSerializedObject.plan_amount,
interval: defaultSerializedObject.plan_interval,
currency: String.prototype.toUpperCase.call(defaultSerializedObject.plan_currency),
currency_symbol: CURRENCY_SYMBOLS[String.prototype.toLowerCase.call(defaultSerializedObject.plan_currency)]
},
status: defaultSerializedObject.status,
start_date: defaultSerializedObject.start_date,
default_payment_card_last4: defaultSerializedObject.default_payment_card_last4,
cancel_at_period_end: defaultSerializedObject.cancel_at_period_end,
current_period_end: defaultSerializedObject.current_period_end
};
}
}, {
async upsert(data, unfilteredOptions) {
const subscriptionId = unfilteredOptions.subscription_id;

View File

@ -60,7 +60,7 @@ const sendEmail = async (postModel, memberModels) => {
const sendTestEmail = async (postModel, toEmails) => {
const recipients = await Promise.all(toEmails.map(async (email) => {
const member = await models.Member.findOne({email});
const member = await membersService.api.members.get({email});
return member || new models.Member({email});
}));
const {emailTmpl, emails, emailData} = await getEmailData(postModel, recipients);
@ -87,7 +87,7 @@ const addEmail = async (postModel, options) => {
const startRetrieve = Date.now();
debug('addEmail: retrieving members count');
const {meta: {pagination: {total: membersCount}}} = await models.Member.findPage(Object.assign({}, knexOptions, filterOptions));
const {meta: {pagination: {total: membersCount}}} = await membersService.api.members.list(Object.assign({}, knexOptions, filterOptions));
debug(`addEmail: retrieved members count - ${membersCount} members (${Date.now() - startRetrieve}ms)`);
// NOTE: don't create email object when there's nobody to send the email to
@ -214,7 +214,7 @@ async function pendingEmailHandler(emailModel, options) {
const startRetrieve = Date.now();
debug('pendingEmailHandler: retrieving members list');
const {data: members} = await models.Member.findPage(Object.assign({}, knexOptions, filterOptions));
const {data: members} = await membersService.api.members.list(Object.assign({}, knexOptions, filterOptions));
debug(`pendingEmailHandler: retrieved members list - ${members.length} members (${Date.now() - startRetrieve}ms)`);
if (!members.length) {

View File

@ -184,7 +184,7 @@ const doImport = async ({membersBatch: members, allLabelModels, importSetLabels,
if (membersWithStripeCustomers.length) {
await Promise.map(membersWithStripeCustomers, async (stripeMember) => {
try {
await membersService.api.members.linkStripeCustomer(stripeMember.stripe_customer_id, stripeMember);
await membersService.api.members.linkStripeCustomerById(stripeMember.stripe_customer_id, stripeMember.id);
} catch (error) {
if (error.message.indexOf('customer') && error.code === 'resource_missing') {
error.message = `Member not imported. ${error.message}`;
@ -204,7 +204,7 @@ const doImport = async ({membersBatch: members, allLabelModels, importSetLabels,
if (membersWithComplimentaryPlans.length) {
await Promise.map(membersWithComplimentaryPlans, async (complimentaryMember) => {
try {
await membersService.api.members.setComplimentarySubscription(complimentaryMember);
await membersService.api.members.setComplimentarySubscriptionById(complimentaryMember.id);
} catch (error) {
logging.error(error);
invalid.errors.push(error);

View File

@ -55,7 +55,7 @@
"@tryghost/kg-markdown-html-renderer": "2.0.1",
"@tryghost/kg-mobiledoc-html-renderer": "3.0.1",
"@tryghost/magic-link": "0.4.13",
"@tryghost/members-api": "0.25.2",
"@tryghost/members-api": "0.26.0",
"@tryghost/members-csv": "0.2.1",
"@tryghost/members-ssr": "0.8.5",
"@tryghost/mw-session-from-token": "0.1.7",

View File

@ -12,6 +12,76 @@ describe('Member Model', function run() {
beforeEach(testUtils.setup('roles'));
afterEach(testUtils.teardownDb);
describe('stripeSubscriptions', function () {
it('It is correctly mapped to all a members subscriptions, regardless of customer', async function () {
const context = testUtils.context.admin;
await Member.add({
email: 'test@test.member',
labels: []
}, context);
const member = await Member.findOne({
email: 'test@test.member'
}, context);
should.exist(member, 'Member should have been created');
await MemberStripeCustomer.add({
member_id: member.get('id'),
customer_id: 'fake_customer_id1'
}, context);
await MemberStripeCustomer.add({
member_id: member.get('id'),
customer_id: 'fake_customer_id2'
}, context);
await StripeCustomerSubscription.add({
customer_id: 'fake_customer_id1',
subscription_id: 'fake_subscription_id1',
plan_id: 'fake_plan_id',
plan_amount: 1337,
plan_nickname: 'e-LEET',
plan_interval: 'year',
plan_currency: 'btc',
status: 'active',
start_date: new Date(),
current_period_end: new Date(),
cancel_at_period_end: false
}, context);
await StripeCustomerSubscription.add({
customer_id: 'fake_customer_id2',
subscription_id: 'fake_subscription_id2',
plan_id: 'fake_plan_id',
plan_amount: 1337,
plan_nickname: 'e-LEET',
plan_interval: 'year',
plan_currency: 'btc',
status: 'active',
start_date: new Date(),
current_period_end: new Date(),
cancel_at_period_end: false
}, context);
const memberWithRelations = await Member.findOne({
email: 'test@test.member'
}, Object.assign({
withRelated: [
'stripeSubscriptions',
'stripeSubscriptions.customer'
]
}, context));
const subscriptions = memberWithRelations.related('stripeSubscriptions').toJSON();
const subscription1 = subscriptions.find(({id}) => id === 'fake_subscription_id1');
const subscription2 = subscriptions.find(({id}) => id === 'fake_subscription_id2');
should.exist(subscription1);
should.exist(subscription2);
});
});
describe('stripeCustomers', function () {
it('Is correctly mapped to the stripe customers', async function () {
const context = testUtils.context.admin;

View File

@ -408,10 +408,10 @@
jsonwebtoken "^8.5.1"
lodash "^4.17.15"
"@tryghost/members-api@0.25.2":
version "0.25.2"
resolved "https://registry.yarnpkg.com/@tryghost/members-api/-/members-api-0.25.2.tgz#64fd5270f44620761e9f01d4f96925a0bdceb09d"
integrity sha512-oNfFZTSO2TA0BEuEk0mbGSZmSZCUq2wvdD63z2AGup+hiDTQHOVlXJGVwKPRqDP5+DJW2wIISh8UxMriXn+8Ow==
"@tryghost/members-api@0.26.0":
version "0.26.0"
resolved "https://registry.yarnpkg.com/@tryghost/members-api/-/members-api-0.26.0.tgz#6a6fbb58a485da04217c9df0d6cf335003b3188f"
integrity sha512-jZ5Rxt3m7sMonIVbvxJnpElTTq+QPmBGS2bsOFp3cOVjIazD31fCKTWBaV3waGYQ9W5Lx4CrIJ0A4taJKKbKbA==
dependencies:
"@tryghost/magic-link" "^0.4.13"
bluebird "^3.5.4"