Просмотр исходного кода

[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
Simon Gardner 4 месяцев назад
Родитель
Сommit
6915989ce5

+ 8 - 79
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.
  */

+ 18 - 139
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,160 +39,40 @@ import {
   normalisedGBVATNumber,
 } from './migrate_recurly_customers_to_stripe.helpers.mjs'
 
-test('coalesceOrEqualOrThrow returns primary when set', () => {
-  assert.equal(coalesceOrEqualOrThrow('a', undefined, 'field'), 'a')
-})
-
-test('coalesceOrEqualOrThrow returns fallback when primary is unset', () => {
-  assert.equal(coalesceOrEqualOrThrow(undefined, 'b', 'field'), 'b')
-})
-
-test('coalesceOrEqualOrThrow returns value when both are set and equal', () => {
-  assert.equal(coalesceOrEqualOrThrow('same', 'same', 'field'), 'same')
-})
-
-test('coalesceOrEqualOrThrow throws when both are set but differ', () => {
-  assert.throws(
-    () => coalesceOrEqualOrThrow('a', 'b', 'field'),
-    /Primary and fallback values are both 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', () => {
+test('extractNameFromAccount returns normalized full name when first and last are present', () => {
   const account = {
-    address: { street1: '', postalCode: 'ABC', country: 'GB' },
-    billingInfo: {
-      address: { street1: '1 Road', postalCode: 'ABC', country: 'GB' },
-    },
+    firstName: ' Alice ',
+    lastName: ' Example ',
   }
-  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',
-  })
+  assert.equal(extractNameFromAccount(account), 'Alice Example')
 })
 
-test('coalesceOrEqualOrThrowAddress throws when both valid but differ', () => {
+test('extractNameFromAccount falls back to firstName when lastName is missing', () => {
   const account = {
-    address: { street1: '1 Road', postalCode: 'ABC', country: 'GB' },
-    billingInfo: {
-      address: { street1: '2 Road', postalCode: 'ABC', country: 'GB' },
-    },
+    firstName: 'Alice Example',
+    lastName: '',
   }
-  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')
+  assert.equal(extractNameFromAccount(account), 'Alice Example')
 })
 
-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('coalesceOrEqualOrThrow returns primary when set', () => {
+  assert.equal(coalesceOrEqualOrThrow('a', undefined, 'field'), 'a')
 })
 
-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('coalesceOrEqualOrThrow returns fallback when primary is unset', () => {
+  assert.equal(coalesceOrEqualOrThrow(undefined, 'b', 'field'), 'b')
 })
 
-test('coalesceOrEqualOrThrowName returns null when both sources are empty', () => {
-  assert.equal(coalesceOrEqualOrThrowName({}), null)
-  assert.equal(
-    coalesceOrEqualOrThrowName({ firstName: '', lastName: '' }),
-    null
-  )
+test('coalesceOrEqualOrThrow returns value when both are set and equal', () => {
+  assert.equal(coalesceOrEqualOrThrow('same', 'same', 'field'), 'same')
 })
 
-test('coalesceOrEqualOrThrowName throws when both full names are present but differ', () => {
-  const account = {
-    firstName: 'Alice',
-    lastName: 'Account',
-    billingInfo: { firstName: 'Alice', lastName: 'Billing' },
-  }
+test('coalesceOrEqualOrThrow throws when both are set but differ', () => {
   assert.throws(
-    () => coalesceOrEqualOrThrowName(account),
-    /Name differs between billingInfo and account/
+    () => coalesceOrEqualOrThrow('a', 'b', 'field'),
+    /Primary and fallback values are both set but differ/
   )
 })
 

+ 9 - 4
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