From 6915989ce571a94d76e9e3d5b44fe10098eab493 Mon Sep 17 00:00:00 2001 From: Simon Gardner Date: Wed, 18 Mar 2026 15:58:14 +0000 Subject: [PATCH] [stripe migration] less strict address validation on upsert script (#32207) * less strict address validation * remove redundant name and address coalescing functions * update missing name handling GitOrigin-RevId: 4d15b16c840fa3edc50df6592d33f324dd26722c --- ...te_recurly_customers_to_stripe.helpers.mjs | 87 +--------- ..._customers_to_stripe.helpers.node.test.mjs | 159 +++--------------- .../migrate_recurly_customers_to_stripe.mjs | 13 +- 3 files changed, 36 insertions(+), 223 deletions(-) diff --git a/services/web/scripts/helpers/migrate_recurly_customers_to_stripe.helpers.mjs b/services/web/scripts/helpers/migrate_recurly_customers_to_stripe.helpers.mjs index 6a21bcb6e0..7c77f412f6 100644 --- a/services/web/scripts/helpers/migrate_recurly_customers_to_stripe.helpers.mjs +++ b/services/web/scripts/helpers/migrate_recurly_customers_to_stripe.helpers.mjs @@ -1,13 +1,10 @@ /* eslint-disable @overleaf/require-script-runner */ -import lodash from 'lodash' /* * * This file can be deleted once the Recurly to Stripe migration is complete. */ -const { isEqual } = lodash - export function coalesceOrEqualOrThrow(a, b, fieldName) { const isSetA = !!a const isSetB = !!b @@ -28,44 +25,6 @@ export function normalizeName(firstName, lastName) { return full || null } -/** - * Extract and coalesce customer name from Recurly data. - * - * Atomic behavior: first+last name are taken from the same source. - * - * Coalesce/equality behavior: - * - Prefer billingInfo name when both first+last are present. - * - Fall back to account name otherwise. - * - If both billingInfo and account have a complete (first+last) name and they differ, throw. - * - * @param {object} account - Recurly account object - * @returns {string|null} - */ -export function coalesceOrEqualOrThrowName(account) { - const billingHasFullName = !!( - account.billingInfo?.firstName && account.billingInfo?.lastName - ) - const accountHasFullName = !!(account.firstName && account.lastName) - - const billingName = billingHasFullName - ? normalizeName( - account.billingInfo?.firstName, - account.billingInfo?.lastName - ) - : null - const accountName = accountHasFullName - ? normalizeName(account.firstName, account.lastName) - : null - - if (billingHasFullName && accountHasFullName && billingName !== accountName) { - throw new Error( - `Name differs between billingInfo and account (${billingName} != ${accountName})` - ) - } - - return billingName ?? accountName -} - /** * Extract customer name from billing info only (no fallback to account). * @@ -88,10 +47,8 @@ export function extractNameFromBillingInfo(account) { * @returns {string|null} */ export function extractNameFromAccount(account) { - const accountHasFullName = !!(account.firstName && account.lastName) - return accountHasFullName - ? normalizeName(account.firstName, account.lastName) - : null + // some accounts have only a firstName field populated with the full name, so normalizeName falls back to just firstName if lastName is missing + return normalizeName(account.firstName, account.lastName) } /** @@ -131,54 +88,26 @@ export function normalizeRecurlyAddressToStripe(address) { .toUpperCase() // Only send an address if it has enough data to be plausibly accepted/usable by Stripe. - // eslint-disable-next-line camelcase - if (!line1 || !postal_code || !country) return null + // For now we'll accept just bare minimum of a country code if (!/^[A-Z]{2}$/.test(country)) return null const line2 = (address.street2 || '').trim() const city = (address.city || '').trim() const state = (address.region || '').trim() + // Intentionally include empty-string fields so Stripe clears any existing + // stale values on the customer address when Recurly has blanks. return { line1, - ...(line2 ? { line2 } : {}), - ...(city ? { city } : {}), - ...(state ? { state } : {}), + line2, + city, + state, // eslint-disable-next-line camelcase postal_code, country, } } -/** - * Extract address from Recurly data. - * - * Prefers billingInfo address as this is what the customer entered during checkout. - * Falls back to account address for manually-created accounts or legacy data. - * - * @param {object} account - Recurly account object - * @returns {import('stripe').Stripe.AddressParam|null} - */ -export function coalesceOrEqualOrThrowAddress(account) { - const billingAddress = normalizeRecurlyAddressToStripe( - account.billingInfo?.address - ) - const accountAddress = normalizeRecurlyAddressToStripe(account?.address) - - const isBillingAddressValid = !!billingAddress - const isAccountAddressValid = !!accountAddress - - if (!isBillingAddressValid && !isAccountAddressValid) return null - if (isBillingAddressValid && !isAccountAddressValid) return billingAddress - if (!isBillingAddressValid && isAccountAddressValid) return accountAddress - - if (!isEqual(billingAddress, accountAddress)) { - throw new Error('Billing address and account address differ') - } - - return billingAddress -} - /** * EU member state country codes for VAT purposes. */ diff --git a/services/web/scripts/helpers/migrate_recurly_customers_to_stripe.helpers.node.test.mjs b/services/web/scripts/helpers/migrate_recurly_customers_to_stripe.helpers.node.test.mjs index b35c1f735f..81f7e3d099 100644 --- a/services/web/scripts/helpers/migrate_recurly_customers_to_stripe.helpers.node.test.mjs +++ b/services/web/scripts/helpers/migrate_recurly_customers_to_stripe.helpers.node.test.mjs @@ -12,9 +12,8 @@ import assert from 'node:assert/strict' import { coalesceOrEqualOrThrow, - coalesceOrEqualOrThrowAddress, - coalesceOrEqualOrThrowName, coalesceOrThrowVATNumber, + extractNameFromAccount, getCanadaTaxIdType, getAustraliaTaxIdType, getBrazilTaxIdType, @@ -40,6 +39,24 @@ import { normalisedGBVATNumber, } from './migrate_recurly_customers_to_stripe.helpers.mjs' +test('extractNameFromAccount returns normalized full name when first and last are present', () => { + const account = { + firstName: ' Alice ', + lastName: ' Example ', + } + + assert.equal(extractNameFromAccount(account), 'Alice Example') +}) + +test('extractNameFromAccount falls back to firstName when lastName is missing', () => { + const account = { + firstName: 'Alice Example', + lastName: '', + } + + assert.equal(extractNameFromAccount(account), 'Alice Example') +}) + test('coalesceOrEqualOrThrow returns primary when set', () => { assert.equal(coalesceOrEqualOrThrow('a', undefined, 'field'), 'a') }) @@ -59,144 +76,6 @@ test('coalesceOrEqualOrThrow throws when both are set but differ', () => { ) }) -test('coalesceOrEqualOrThrowAddress returns null when neither is valid', () => { - assert.equal(coalesceOrEqualOrThrowAddress({}), null) - assert.equal( - coalesceOrEqualOrThrowAddress({ - address: { street1: '', postalCode: '', country: '' }, - billingInfo: { address: { street1: '', postalCode: '', country: '' } }, - }), - null - ) - - assert.equal( - coalesceOrEqualOrThrowAddress({ - address: { street1: ' ', postalCode: ' ', country: ' ' }, - }), - null - ) -}) - -test('coalesceOrEqualOrThrowAddress returns account when billing invalid', () => { - const account = { - address: { street1: '1 Road', postalCode: 'ABC', country: 'GB' }, - billingInfo: { - address: { street1: '', postalCode: 'ABC', country: 'GB' }, - }, - } - assert.deepEqual(coalesceOrEqualOrThrowAddress(account), { - line1: '1 Road', - postal_code: 'ABC', - country: 'GB', - }) -}) - -test('coalesceOrEqualOrThrowAddress returns billing when account invalid', () => { - const account = { - address: { street1: '', postalCode: 'ABC', country: 'GB' }, - billingInfo: { - address: { street1: '1 Road', postalCode: 'ABC', country: 'GB' }, - }, - } - assert.deepEqual(coalesceOrEqualOrThrowAddress(account), { - line1: '1 Road', - postal_code: 'ABC', - country: 'GB', - }) -}) - -test('coalesceOrEqualOrThrowAddress returns billing when both valid+equal', () => { - const addr = { street1: '1 Road', postalCode: 'ABC', country: 'GB' } - assert.deepEqual( - coalesceOrEqualOrThrowAddress({ - address: { ...addr }, - billingInfo: { address: addr }, - }), - { line1: '1 Road', postal_code: 'ABC', country: 'GB' } - ) -}) - -test('coalesceOrEqualOrThrowAddress normalizes Recurly-style address fields', () => { - const account = { - billingInfo: { - address: { - street1: 'as', - street2: '', - city: '', - region: '', - postalCode: '12312', - country: 'AI', - }, - }, - } - - assert.deepEqual(coalesceOrEqualOrThrowAddress(account), { - line1: 'as', - postal_code: '12312', - country: 'AI', - }) -}) - -test('coalesceOrEqualOrThrowAddress throws when both valid but differ', () => { - const account = { - address: { street1: '1 Road', postalCode: 'ABC', country: 'GB' }, - billingInfo: { - address: { street1: '2 Road', postalCode: 'ABC', country: 'GB' }, - }, - } - assert.throws( - () => coalesceOrEqualOrThrowAddress(account), - /Billing address and account address differ/ - ) -}) - -test('coalesceOrEqualOrThrowName returns billingInfo name when both sources match', () => { - const account = { - firstName: 'Alice', - lastName: 'Billing', - billingInfo: { firstName: 'Alice', lastName: 'Billing' }, - } - assert.equal(coalesceOrEqualOrThrowName(account), 'Alice Billing') -}) - -test('coalesceOrEqualOrThrowName prefers billingInfo when billingInfo is full but account is not', () => { - const account = { - firstName: 'Alice', - lastName: '', - billingInfo: { firstName: 'Alice', lastName: 'Billing' }, - } - assert.equal(coalesceOrEqualOrThrowName(account), 'Alice Billing') -}) - -test('coalesceOrEqualOrThrowName falls back to account when billingInfo missing last name', () => { - const account = { - firstName: 'Alice', - lastName: 'Account', - billingInfo: { firstName: 'Alice', lastName: '' }, - } - assert.equal(coalesceOrEqualOrThrowName(account), 'Alice Account') -}) - -test('coalesceOrEqualOrThrowName returns null when both sources are empty', () => { - assert.equal(coalesceOrEqualOrThrowName({}), null) - assert.equal( - coalesceOrEqualOrThrowName({ firstName: '', lastName: '' }), - null - ) -}) - -test('coalesceOrEqualOrThrowName throws when both full names are present but differ', () => { - const account = { - firstName: 'Alice', - lastName: 'Account', - billingInfo: { firstName: 'Alice', lastName: 'Billing' }, - } - assert.throws( - () => coalesceOrEqualOrThrowName(account), - /Name differs between billingInfo and account/ - ) -}) - test('coalesceOrThrowVATNumber returns billingInfo VAT when set', () => { const account = { vatNumber: '', diff --git a/services/web/scripts/recurly/migrate_recurly_customers_to_stripe.mjs b/services/web/scripts/recurly/migrate_recurly_customers_to_stripe.mjs index 4ad8d85244..80985ee167 100644 --- a/services/web/scripts/recurly/migrate_recurly_customers_to_stripe.mjs +++ b/services/web/scripts/recurly/migrate_recurly_customers_to_stripe.mjs @@ -92,8 +92,6 @@ import { scriptRunner } from '../lib/ScriptRunner.mjs' import { areStripeAndRecurlyCardDetailsEqual, - coalesceOrEqualOrThrowAddress, - coalesceOrEqualOrThrowName, coalesceOrThrowPaymentMethod, extractNameFromAccount, extractNameFromBillingInfo, @@ -1284,8 +1282,8 @@ async function resolveCustomerIdentity(account, recurlyAccountCode, context) { if (!hasConflict) { // No conflict: use the standard coalesce logic (billing info preferred) - name = coalesceOrEqualOrThrowName(account) - address = coalesceOrEqualOrThrowAddress(account) + name = billingName ?? accountName + address = billingAddress ?? accountAddress companyName = billingCompany ?? accountCompany vatNumber = billingVat ?? accountVat collectionMethod = null @@ -1489,6 +1487,13 @@ async function processCustomer( billingInfoForPaymentMethod, } = await resolveCustomerIdentity(account, recurlyAccountCode, context) + if (name === null && companyName === null) { + // This should not happen since we're handling all the known cases in resolveCustomerIdentity but just in case + throw new Error( + 'Unable to resolve customer name: both billing info and account fields are missing' + ) + } + let taxIdType = null let createdTaxId = null let taxInfoPendingValue = null