Skip to content

Commit

Permalink
🐛 Fixed Tips & Donations checkout error for sites with long titles
Browse files Browse the repository at this point in the history
ref https://linear.app/tryghost/issue/ONC-296

Our `stripe_prices.nickname` field had a length of 50 chars which meant we could error out trying to save a donation Stripe price with a generated product nickname containing a long site title.

- updated db schema and added a migration to change column length to 255
- added truncation to nickname generation to enforce a limit of 250 chars to match Stripe's limit
  • Loading branch information
kevinansfield committed Sep 3, 2024
1 parent b2d7922 commit 0130413
Show file tree
Hide file tree
Showing 6 changed files with 85 additions and 3 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
const logging = require('@tryghost/logging');
const {createNonTransactionalMigration} = require('../../utils');
const DatabaseInfo = require('@tryghost/database-info');

module.exports = createNonTransactionalMigration(
async function up(knex) {
if (DatabaseInfo.isSQLite(knex)) {
logging.warn('Skipping migration for SQLite3');
return;
}
logging.info('Changing stripe_prices.nickname column from VARCHAR(50) to VARCHAR(255)');
await knex.schema.alterTable('stripe_prices', function (table) {
table.string('nickname', 255).alter();
});
},
async function down() {
logging.warn('Not changing stripe_prices.nickname column');
}
);
2 changes: 1 addition & 1 deletion ghost/core/core/server/data/schema/schema.js
Original file line number Diff line number Diff line change
Expand Up @@ -781,7 +781,7 @@ module.exports = {
stripe_price_id: {type: 'string', maxlength: 255, nullable: false, unique: true},
stripe_product_id: {type: 'string', maxlength: 255, nullable: false, unique: false, references: 'stripe_products.stripe_product_id'},
active: {type: 'boolean', nullable: false},
nickname: {type: 'string', maxlength: 50, nullable: true},
nickname: {type: 'string', maxlength: 255, nullable: true},
// @note: this is longer than originally intended due to a bug - https://github.com/TryGhost/Ghost/pull/15606
// so we should decide whether we should reduce it down in the future
currency: {type: 'string', maxlength: 191, nullable: false},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,12 @@ Object {
}
`;

exports[`Create Stripe Checkout Session for Donations can create a checkout session for a site with a long title 1: [body] 1`] = `
Object {
"url": "https://checkout.stripe.com/c/pay/fake-data",
}
`;

exports[`Create Stripe Checkout Session for Donations check if donation message is in email 1: [body] 1`] = `
Object {
"url": "https://checkout.stripe.com/c/pay/fake-data",
Expand Down
48 changes: 48 additions & 0 deletions ghost/core/test/e2e-api/members/donation-checkout-session.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ describe('Create Stripe Checkout Session for Donations', function () {
assert.equal(lastDonation.get('attribution_type'), 'post');
assert.equal(lastDonation.get('attribution_url'), url);
});

it('check if donation message is in email', async function () {
const post = await getPost(fixtureManager.get('posts', 0).id);
const url = urlService.getUrlByResourceId(post.id, {absolute: false});
Expand Down Expand Up @@ -281,4 +282,51 @@ describe('Create Stripe Checkout Session for Donations', function () {
text: /You are the best! Have a lovely day!/
});
});

// We had a bug where the stripe_prices.nickname column was too short for the site title
// Stripe is also limited to 250 chars so we need to truncate the nickname
it('can create a checkout session for a site with a long title', async function () {
// Ensure site title is longer than 250 characters
mockManager.mockSetting('title', 'a'.repeat(251));

// clear out existing prices to guarantee we're creating a new one
await models.StripePrice.where('type', 'donation').destroy().catch((e) => {
if (e.message !== 'No Rows Deleted') {
throw e;
}
});

// Fake a visit to a post
const post = await getPost(fixtureManager.get('posts', 0).id);
const url = urlService.getUrlByResourceId(post.id, {absolute: false});

await membersAgent.post('/api/create-stripe-checkout-session/')
.body({
customerEmail: '[email protected]',
type: 'donation',
successUrl: 'https://example.com/?type=success',
cancelUrl: 'https://example.com/?type=cancel',
metadata: {
test: 'hello',
urlHistory: [
{
path: url,
time: Date.now(),
referrerMedium: null,
referrerSource: 'ghost-explore',
referrerUrl: 'https://example.com/blog/'
}
]
}
})
.expectStatus(200)
.matchBodySnapshot();

const latestStripePrice = await models.StripePrice
.where('type', 'donation')
.orderBy('created_at', 'DESC')
.fetch({require: true});

latestStripePrice.get('nickname').should.have.length(250);
});
});
2 changes: 1 addition & 1 deletion ghost/core/test/unit/server/data/schema/integrity.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ const validateRouteSettings = require('../../../../../core/server/services/route
*/
describe('DB version integrity', function () {
// Only these variables should need updating
const currentSchemaHash = 'b59d502d0e7965a837bb1dfb5c583562';
const currentSchemaHash = 'a4f016480ff73c6f52ee4c86482b45a7';
const currentFixturesHash = 'a489d615989eab1023d4b8af0ecee7fd';
const currentSettingsHash = '051ef2a50e2edb8723e89461448313cb';
const currentRoutesHash = '3d180d52c663d173a6be791ef411ed01';
Expand Down
11 changes: 10 additions & 1 deletion ghost/payments/lib/PaymentsService.js
Original file line number Diff line number Diff line change
Expand Up @@ -276,11 +276,20 @@ class PaymentsService {
};
}

/**
* Stripe's nickname field is limited to 250 characters
* @returns {string}
*/
getDonationPriceNickname() {
const nickname = 'Support ' + this.settingsCache.get('title');
return nickname.substring(0, 250);
}

/**
* @returns {Promise<{id: string}>}
*/
async getPriceForDonations() {
const nickname = 'Support ' + this.settingsCache.get('title');
const nickname = this.getDonationPriceNickname();
const currency = this.settingsCache.get('donations_currency');
const suggestedAmount = this.settingsCache.get('donations_suggested_amount');

Expand Down

0 comments on commit 0130413

Please sign in to comment.