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

🐛 Handled deleted Stripe objects in the Stripe Checkout flow

Browse Source 
closes https://github.com/TryGhost/Team/issues/2222

Whilst we were checking for Stripe objects being active, we were not
checking for them existing in Stripe. This adds handling to all read
request to Stripe in the payment link flow, so that we can gracefully
handle deleted objects.

We've also included an automated test which fails without this fix.

We've also improved the query to find Stripe Prices which will result
in less request to the Stripe API to check if it is valid.
This commit is contained in:
Fabien "egg" O'Carroll 2022-11-08 12:37:13 +07:00 committed by Fabien 'egg' O'Carroll
parent 5f9e354cae
commit 69aa52bd8e
7 changed files with 287 additions and 61 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

100
ghost/core/test/e2e-api/members/create-stripe-checkout-session.test.js
View File

@ -95,7 +95,10 @@ describe('Create Stripe Checkout Session', function () {
id: id,
active: true,
currency: 'usd',
unit_amount: 500
unit_amount: 500,
recurring: {
interval: 'month'
}
}];
}
}
@ -112,7 +115,19 @@ describe('Create Stripe Checkout Session', function () {
}
if (uri === '/v1/coupons') {
return [200, {id: 'coupon_123', url: 'https://site.com'}];
return [200, {id: 'coupon_123'}];
}
if (uri === '/v1/prices') {
return [200, {
id: 'price_1',
active: true,
currency: 'usd',
unit_amount: 500,
recurring: {
interval: 'month'
}
}];
}
return [500];
@ -150,7 +165,10 @@ describe('Create Stripe Checkout Session', function () {
id: id,
active: true,
currency: 'usd',
unit_amount: 500
unit_amount: 500,
recurring: {
interval: 'month'
}
}];
}
}
@ -171,6 +189,18 @@ describe('Create Stripe Checkout Session', function () {
return [200, {id: 'cs_123', url: 'https://site.com'}];
}
if (uri === '/v1/prices') {
return [200, {
id: 'price_2',
active: true,
currency: 'usd',
unit_amount: 500,
recurring: {
interval: 'month'
}
}];
}
return [500];
});
@ -205,7 +235,10 @@ describe('Create Stripe Checkout Session', function () {
id: id,
active: true,
currency: 'usd',
unit_amount: 500
unit_amount: 500,
recurring: {
interval: 'month'
}
}];
}
}
@ -220,6 +253,17 @@ describe('Create Stripe Checkout Session', function () {
if (uri === '/v1/checkout/sessions') {
return [200, {id: 'cs_123', url: 'https://site.com'}];
}
if (uri === '/v1/prices') {
return [200, {
id: 'price_3',
active: true,
currency: 'usd',
unit_amount: 500,
recurring: {
interval: 'month'
}
}];
}
return [500];
});
@ -262,7 +306,10 @@ describe('Create Stripe Checkout Session', function () {
id: id,
active: true,
currency: 'usd',
unit_amount: 500
unit_amount: 500,
recurring: {
interval: 'month'
}
}];
}
}
@ -282,6 +329,17 @@ describe('Create Stripe Checkout Session', function () {
return [200, {id: 'cs_123', url: 'https://site.com'}];
}
if (uri === '/v1/prices') {
return [200, {
id: 'price_4',
active: true,
currency: 'usd',
unit_amount: 500,
recurring: {
interval: 'month'
}
}];
}
return [500];
});
@ -332,7 +390,10 @@ describe('Create Stripe Checkout Session', function () {
id: id,
active: true,
currency: 'usd',
unit_amount: 500
unit_amount: 50,
recurring: {
interval: 'month'
}
}];
}
}
@ -352,6 +413,17 @@ describe('Create Stripe Checkout Session', function () {
return [200, {id: 'cs_123', url: 'https://site.com'}];
}
if (uri === '/v1/prices') {
return [200, {
id: 'price_5',
active: true,
currency: 'usd',
unit_amount: 500,
recurring: {
interval: 'month'
}
}];
}
return [500];
});
@ -399,7 +471,10 @@ describe('Create Stripe Checkout Session', function () {
id: id,
active: true,
currency: 'usd',
unit_amount: 500
unit_amount: 500,
recurring: {
interval: 'month'
}
}];
}
}
@ -419,6 +494,17 @@ describe('Create Stripe Checkout Session', function () {
return [200, {id: 'cs_123', url: 'https://site.com'}];
}
if (uri === '/v1/prices') {
return [200, {
id: 'price_6',
active: true,
currency: 'usd',
unit_amount: 500,
recurring: {
interval: 'month'
}
}];
}
return [500];
});

38
ghost/payments/lib/payments.js
View File

@ -1,3 +1,4 @@
const logging = require('@tryghost/logging');
const DomainEvents = require('@tryghost/domain-events');
const {TierCreatedEvent, TierPriceChangeEvent, TierNameChangeEvent} = require('@tryghost/tiers');
const OfferCreatedEvent = require('@tryghost/members-offers').events.OfferCreatedEvent;
@ -111,9 +112,13 @@ class PaymentsService {
}).query().select('customer_id');
for (const row of rows) {
const customer = await this.stripeAPIService.getCustomer(row.customer_id);
if (!customer.deleted) {
return customer;
try {
const customer = await this.stripeAPIService.getCustomer(row.customer_id);
if (!customer.deleted) {
return customer;
}
} catch (err) {
logging.warn(err);
}
}
@ -149,9 +154,13 @@ class PaymentsService {
.select('stripe_product_id');
for (const row of rows) {
const product = await this.stripeAPIService.getProduct(row.stripe_product_id);
if (product.active) {
return {id: product.id};
try {
const product = await this.stripeAPIService.getProduct(row.stripe_product_id);
if (product.active) {
return {id: product.id};
}
} catch (err) {
logging.warn(err);
}
}
@ -199,20 +208,25 @@ class PaymentsService {
*/
async getPriceForTierCadence(tier, cadence) {
const product = await this.getProductForTier(tier);
const currency = tier.currency;
const currency = tier.currency.toLowerCase();
const amount = tier.getPrice(cadence);
const rows = await this.StripePriceModel.where({
stripe_product_id: product.id,
currency,
interval: cadence,
amount
}).query().select('stripe_price_id');
for (const row of rows) {
const price = await this.stripeAPIService.getPrice(row.stripe_price_id);
if (price.active && price.currency.toUpperCase() === currency && price.unit_amount === amount) {
return {
id: price.id
};
try {
const price = await this.stripeAPIService.getPrice(row.stripe_price_id);
if (price.active && price.currency.toUpperCase() === currency && price.unit_amount === amount && price.recurring?.interval === cadence) {
return {
id: price.id
};
}
} catch (err) {
logging.warn(err);
}
}

10
ghost/payments/test/hello.test.js
View File

@ -1,10 +0,0 @@
// Switch these lines once there are useful utils
// const testUtils = require('./utils');
require('./utils');
describe('Hello world', function () {
it('Runs a test', function () {
// TODO: Write me!
'hello'.should.eql('hello');
});
});

168
ghost/payments/test/lib/payments.test.js Normal file
View File

@ -0,0 +1,168 @@
const assert = require('assert');
const sinon = require('sinon');
const knex = require('knex');
const {Tier} = require('@tryghost/tiers');
const PaymentsService = require('../../lib/payments');
describe('PaymentsService', function () {
let Bookshelf;
let db;
before(async function () {
db = knex({
client: 'sqlite3',
useNullAsDefault: true,
connection: {
filename: ':memory:'
}
});
await db.schema.createTable('offers', function (table) {
table.string('id', 24);
table.string('stripe_coupon_id', 255);
table.string('discount_type', 191);
});
await db.schema.createTable('stripe_products', function (table) {
table.string('id', 24);
table.string('product_id', 24);
table.string('stripe_product_id', 255);
});
await db.schema.createTable('stripe_prices', function (table) {
table.string('id', 24);
table.string('stripe_price_id', 255);
table.string('stripe_product_id', 255);
table.boolean('active');
table.string('nickname', 191);
table.string('currency', 50);
table.integer('amount');
table.string('type', 50);
table.string('interval', 50);
});
await db.schema.createTable('stripe_customers', function (table) {
table.string('id', 24);
table.string('member_id', 24);
table.string('stripe_customer_id', 255);
table.string('name', 191);
table.string('email', 191);
});
Bookshelf = require('bookshelf')(db);
});
beforeEach(async function () {
await db('offers').truncate();
await db('stripe_products').truncate();
await db('stripe_prices').truncate();
await db('stripe_customers').truncate();
});
after(async function () {
await db.destroy();
});
describe('getPaymentLink', function () {
it('Can handle 404 from Stripe', async function () {
const BaseModel = Bookshelf.Model.extend({}, {
async add() {},
async edit() {}
});
const Offer = BaseModel.extend({
tableName: 'offers'
});
const StripeProduct = BaseModel.extend({
tableName: 'stripe_products'
});
const StripePrice = BaseModel.extend({
tableName: 'stripe_prices'
});
const StripeCustomer = BaseModel.extend({
tableName: 'stripe_customers'
});
const offersAPI = {};
const stripeAPIService = {
createCheckoutSession: sinon.fake.resolves({
url: 'https://checkout.session'
}),
getCustomer: sinon.fake(),
createCustomer: sinon.fake(),
getProduct: sinon.fake.resolves({
id: 'prod_1',
active: true
}),
editProduct: sinon.fake(),
createProduct: sinon.fake.resolves({
id: 'prod_1',
active: true
}),
getPrice: sinon.fake.rejects(new Error('Price does not exist')),
createPrice: sinon.fake(function (data) {
return Promise.resolve({
id: 'price_1',
active: data.active,
unit_amount: data.amount,
currency: data.currency,
nickname: data.nickname,
recurring: {
interval: data.interval
}
});
}),
createCoupon: sinon.fake()
};
const service = new PaymentsService({
Offer,
StripeProduct,
StripePrice,
StripeCustomer,
offersAPI,
stripeAPIService
});
const tier = await Tier.create({
name: 'Test tier',
slug: 'test-tier',
currency: 'usd',
monthlyPrice: 1000,
yearlyPrice: 10000
});
const price = StripePrice.forge({
id: 'id_1',
stripe_price_id: 'price_1',
stripe_product_id: 'prod_1',
active: true,
interval: 'month',
nickname: 'Monthly',
currency: 'usd',
amount: 1000,
type: 'recurring'
});
const product = StripeProduct.forge({
id: 'id_1',
stripe_product_id: 'prod_1',
product_id: tier.id.toHexString()
});
await price.save(null, {method: 'insert'});
await product.save(null, {method: 'insert'});
const cadence = 'month';
const offer = null;
const member = null;
const metadata = {};
const options = {};
const url = await service.getPaymentLink({
tier,
cadence,
offer,
member,
metadata,
options
});
assert(url);
});
});
});

11
ghost/payments/test/utils/assertions.js
View File

@ -1,11 +0,0 @@
/**
* Custom Should Assertions
*
* Add any custom assertions to this file.
*/
// Example Assertion
// should.Assertion.add('ExampleAssertion', function () {
// this.params = {operator: 'to be a valid Example Assertion'};
// this.obj.should.be.an.Object;
// });

11
ghost/payments/test/utils/index.js
View File

@ -1,11 +0,0 @@
/**
* Test Utilities
*
* Shared utils for writing tests
*/
// Require overrides - these add globals for tests
require('./overrides');
// Require assertions - adds custom should assertions
require('./assertions');

10
ghost/payments/test/utils/overrides.js
View File

@ -1,10 +0,0 @@
// This file is required before any test is run
// Taken from the should wiki, this is how to make should global
// Should is a global in our eslint test config
global.should = require('should').noConflict();
should.extend();
// Sinon is a simple case
// Sinon is a global in our eslint test config
global.sinon = require('sinon');