From 4442c57223cb1b1cbbbb6c5cdb5794436ea179e3 Mon Sep 17 00:00:00 2001 From: Jennie Robinson Faber Date: Mon, 27 Apr 2026 11:54:54 +0100 Subject: [PATCH] feat(payments): persist helcimCustomerCode + skip getOrCreateCustomer on card-on-file --- app/composables/useMemberPayment.js | 44 +++++- app/pages/member/payment-setup.vue | 47 ++++-- server/api/auth/member.get.js | 1 + server/api/helcim/customer.post.js | 2 + .../api/helcim/get-or-create-customer.post.js | 19 ++- .../api/internal/reconcile-payments.post.js | 23 ++- server/api/invite/accept.post.js | 1 + server/models/member.js | 1 + .../composables/useMemberPayment.test.js | 145 ++++++++++++++++++ .../api/reconcile-payments-route.test.js | 80 ++++++++-- 10 files changed, 330 insertions(+), 33 deletions(-) create mode 100644 tests/client/composables/useMemberPayment.test.js diff --git a/app/composables/useMemberPayment.js b/app/composables/useMemberPayment.js index 23255a7..fcab6fe 100644 --- a/app/composables/useMemberPayment.js +++ b/app/composables/useMemberPayment.js @@ -25,17 +25,45 @@ export const useMemberPayment = () => { paymentSuccess.value = false try { - // Skip HelcimPay verify if a card's already on file — Helcim refuses - // to re-save it, breaking retries after a partial-failed signup. - const [, existing] = await Promise.all([ - getOrCreateCustomer(), - $fetch('/api/helcim/existing-card').catch((err) => { + // Fast-path: when both Helcim ids are already cached on the member doc + // AND a card's on file, we can skip the paid getOrCreateCustomer round + // trip entirely and go straight to subscription creation. + const hasCachedHelcimIds = Boolean( + memberData.value?.helcimCustomerId && memberData.value?.helcimCustomerCode + ) + + let existing = null + let probedExistingCard = false + let cardToken = null + + if (hasCachedHelcimIds) { + existing = await $fetch('/api/helcim/existing-card').catch((err) => { console.warn('[payment] existing-card lookup failed, falling back to verify flow:', err) return null - }), - ]) + }) + probedExistingCard = true + if (existing?.cardToken) { + customerId.value = memberData.value.helcimCustomerId + customerCode.value = memberData.value.helcimCustomerCode + cardToken = existing.cardToken + } + } - let cardToken = existing?.cardToken || null + if (!cardToken) { + // Skip HelcimPay verify if a card's already on file — Helcim refuses + // to re-save it, breaking retries after a partial-failed signup. + const [, existingFromFull] = await Promise.all([ + getOrCreateCustomer(), + probedExistingCard + ? Promise.resolve(existing) + : $fetch('/api/helcim/existing-card').catch((err) => { + console.warn('[payment] existing-card lookup failed, falling back to verify flow:', err) + return null + }), + ]) + + cardToken = existingFromFull?.cardToken || null + } if (!cardToken) { await initializeHelcimPay( diff --git a/app/pages/member/payment-setup.vue b/app/pages/member/payment-setup.vue index 5efb6f6..2b0efdf 100644 --- a/app/pages/member/payment-setup.vue +++ b/app/pages/member/payment-setup.vue @@ -85,21 +85,46 @@ const initialize = async () => { } try { - // Skip HelcimPay verify if a card's already on file — Helcim refuses - // to re-save it, breaking retries after a partial-failed signup. - const [customer, existing] = await Promise.all([ - $fetch('/api/helcim/get-or-create-customer', { method: 'POST' }), - $fetch('/api/helcim/existing-card').catch((err) => { + // Fast-path: when both Helcim ids are already cached on the member doc + // AND a card's on file, skip the paid get-or-create-customer round trip. + const hasCachedHelcimIds = Boolean( + memberData.value?.helcimCustomerId && memberData.value?.helcimCustomerCode + ); + + let existing = null; + let probedExistingCard = false; + if (hasCachedHelcimIds) { + existing = await $fetch('/api/helcim/existing-card').catch((err) => { console.warn('[payment-setup] existing-card lookup failed, falling back to verify flow:', err); return null; - }), - ]); - customerId.value = customer.customerId; - customerCode.value = customer.customerCode; - hasExistingCard.value = Boolean(existing?.cardToken); + }); + probedExistingCard = true; + if (existing?.cardToken) { + customerId.value = memberData.value.helcimCustomerId; + customerCode.value = memberData.value.helcimCustomerCode; + hasExistingCard.value = true; + } + } if (!hasExistingCard.value) { - await initializeHelcimPay(customerId.value, customerCode.value, 0); + // Skip HelcimPay verify if a card's already on file — Helcim refuses + // to re-save it, breaking retries after a partial-failed signup. + const [customer, existingFromFull] = await Promise.all([ + $fetch('/api/helcim/get-or-create-customer', { method: 'POST' }), + probedExistingCard + ? Promise.resolve(existing) + : $fetch('/api/helcim/existing-card').catch((err) => { + console.warn('[payment-setup] existing-card lookup failed, falling back to verify flow:', err); + return null; + }), + ]); + customerId.value = customer.customerId; + customerCode.value = customer.customerCode; + hasExistingCard.value = Boolean(existingFromFull?.cardToken); + + if (!hasExistingCard.value) { + await initializeHelcimPay(customerId.value, customerCode.value, 0); + } } step.value = 'ready'; } catch (err) { diff --git a/server/api/auth/member.get.js b/server/api/auth/member.get.js index 02316c0..7f0b808 100644 --- a/server/api/auth/member.get.js +++ b/server/api/auth/member.get.js @@ -14,6 +14,7 @@ export default defineEventHandler(async (event) => { contributionAmount: member.contributionAmount, billingCadence: member.billingCadence, helcimCustomerId: member.helcimCustomerId, + helcimCustomerCode: member.helcimCustomerCode, nextBillingDate: member.nextBillingDate, membershipLevel: `${member.circle}-${member.contributionAmount}`, // Profile fields diff --git a/server/api/helcim/customer.post.js b/server/api/helcim/customer.post.js index b267ebd..3382b7f 100644 --- a/server/api/helcim/customer.post.js +++ b/server/api/helcim/customer.post.js @@ -62,6 +62,7 @@ export default defineEventHandler(async (event) => { circle: body.circle, contributionAmount: body.contributionAmount, helcimCustomerId: customerData.id, + helcimCustomerCode: customerData.customerCode, status: 'pending_payment', 'agreement.acceptedAt': new Date() } @@ -75,6 +76,7 @@ export default defineEventHandler(async (event) => { circle: body.circle, contributionAmount: body.contributionAmount, helcimCustomerId: customerData.id, + helcimCustomerCode: customerData.customerCode, status: 'pending_payment', agreement: { acceptedAt: new Date() } }) diff --git a/server/api/helcim/get-or-create-customer.post.js b/server/api/helcim/get-or-create-customer.post.js index a23ac35..5a60897 100644 --- a/server/api/helcim/get-or-create-customer.post.js +++ b/server/api/helcim/get-or-create-customer.post.js @@ -18,6 +18,13 @@ export default defineEventHandler(async (event) => { try { const customer = await getHelcimCustomer(member.helcimCustomerId) if (customer?.id) { + if (!member.helcimCustomerCode && customer.customerCode) { + await Member.findByIdAndUpdate( + member._id, + { $set: { helcimCustomerCode: customer.customerCode } }, + { runValidators: false } + ) + } return { success: true, customerId: customer.id, @@ -49,10 +56,13 @@ export default defineEventHandler(async (event) => { } if (existingCustomer) { - if (!member.helcimCustomerId) { + if (!member.helcimCustomerId || !member.helcimCustomerCode) { await Member.findByIdAndUpdate( member._id, - { $set: { helcimCustomerId: existingCustomer.id } }, + { $set: { + helcimCustomerId: existingCustomer.id, + helcimCustomerCode: existingCustomer.customerCode + } }, { runValidators: false } ) } @@ -73,7 +83,10 @@ export default defineEventHandler(async (event) => { await Member.findByIdAndUpdate( member._id, - { $set: { helcimCustomerId: customerData.id } }, + { $set: { + helcimCustomerId: customerData.id, + helcimCustomerCode: customerData.customerCode + } }, { runValidators: false } ) diff --git a/server/api/internal/reconcile-payments.post.js b/server/api/internal/reconcile-payments.post.js index 0fe90ae..f59e838 100644 --- a/server/api/internal/reconcile-payments.post.js +++ b/server/api/internal/reconcile-payments.post.js @@ -8,7 +8,7 @@ */ import Member from '../../models/member.js' import Payment from '../../models/payment.js' -import { listHelcimCustomerTransactions } from '../../utils/helcim.js' +import { getHelcimCustomer, listHelcimCustomerTransactions } from '../../utils/helcim.js' import { connectDB } from '../../utils/mongoose.js' import { upsertPaymentFromHelcim } from '../../utils/payments.js' @@ -56,7 +56,7 @@ export default defineEventHandler(async (event) => { const members = await Member.find( { helcimCustomerId: { $exists: true, $ne: null } }, - { _id: 1, email: 1, name: 1, helcimCustomerId: 1, helcimSubscriptionId: 1, billingCadence: 1 } + { _id: 1, email: 1, name: 1, helcimCustomerId: 1, helcimCustomerCode: 1, helcimSubscriptionId: 1, billingCadence: 1 } ).lean() let txExamined = 0 @@ -66,6 +66,25 @@ export default defineEventHandler(async (event) => { let memberErrors = 0 async function processMember(member) { + // Opportunistic backfill: members predating the helcimCustomerCode field + // get it filled in here so the daily cron acts as the migration. Only on + // the missing path — no overwrite, no extra API call once populated. + if (!member.helcimCustomerCode) { + try { + const customer = await getHelcimCustomer(member.helcimCustomerId) + if (customer?.customerCode) { + await Member.findByIdAndUpdate( + member._id, + { $set: { helcimCustomerCode: customer.customerCode } }, + { runValidators: false } + ) + } + } catch (err) { + // Backfill is best-effort — never fail the reconcile run on it. + console.warn(`[reconcile] customerCode backfill failed for member=${member._id}: ${err?.message || err}`) + } + } + let txs try { txs = await listTransactionsWithRetry(member.helcimCustomerId) diff --git a/server/api/invite/accept.post.js b/server/api/invite/accept.post.js index 93b46e6..384ae8f 100644 --- a/server/api/invite/accept.post.js +++ b/server/api/invite/accept.post.js @@ -61,6 +61,7 @@ export default defineEventHandler(async (event) => { bio: body.motivation || undefined, status: body.contributionAmount === 0 ? 'active' : 'pending_payment', helcimCustomerId: helcimCustomer?.id, + helcimCustomerCode: helcimCustomer?.customerCode, agreement: { acceptedAt: new Date() }, }) diff --git a/server/models/member.js b/server/models/member.js index 61dd354..3034eb0 100644 --- a/server/models/member.js +++ b/server/models/member.js @@ -42,6 +42,7 @@ const memberSchema = new mongoose.Schema({ default: "pending_payment", }, helcimCustomerId: String, + helcimCustomerCode: String, helcimSubscriptionId: String, billingCadence: { type: String, diff --git a/tests/client/composables/useMemberPayment.test.js b/tests/client/composables/useMemberPayment.test.js new file mode 100644 index 0000000..2a5e1dd --- /dev/null +++ b/tests/client/composables/useMemberPayment.test.js @@ -0,0 +1,145 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest' +import { ref } from 'vue' +import { useMemberPayment } from '../../../app/composables/useMemberPayment.js' + +// Stub Vue's ref/readonly as Nuxt auto-imports +vi.stubGlobal('ref', ref) +vi.stubGlobal('readonly', (v) => v) + +const memberData = ref(null) +const checkMemberStatus = vi.fn() +vi.stubGlobal('useAuth', () => ({ memberData, checkMemberStatus })) + +const initializeHelcimPay = vi.fn() +const verifyPayment = vi.fn() +const cleanupHelcimPay = vi.fn() +vi.stubGlobal('useHelcimPay', () => ({ + initializeHelcimPay, + verifyPayment, + cleanup: cleanupHelcimPay, +})) + +const $fetch = vi.fn() +vi.stubGlobal('$fetch', $fetch) + +describe('useMemberPayment.initiatePaymentSetup — shortcut path', () => { + beforeEach(() => { + vi.clearAllMocks() + memberData.value = null + $fetch.mockReset() + }) + + it('skips getOrCreateCustomer when both helcim ids are cached AND a card is on file', async () => { + memberData.value = { + helcimCustomerId: 'cust-123', + helcimCustomerCode: 'CST-ABC', + contributionAmount: 15, + } + + $fetch.mockImplementation((path) => { + if (path === '/api/helcim/existing-card') { + return Promise.resolve({ cardToken: 'tok-xyz' }) + } + if (path === '/api/helcim/subscription') { + return Promise.resolve({ success: true }) + } + return Promise.reject(new Error(`Unexpected $fetch call: ${path}`)) + }) + + const { initiatePaymentSetup } = useMemberPayment() + const result = await initiatePaymentSetup() + + expect(result.success).toBe(true) + + // The whole point: get-or-create-customer was NOT called. + const calledPaths = $fetch.mock.calls.map((c) => c[0]) + expect(calledPaths).not.toContain('/api/helcim/get-or-create-customer') + expect(calledPaths).not.toContain('/api/helcim/customer-code') + + // We did call existing-card (once) and subscription (once). + expect(calledPaths.filter((p) => p === '/api/helcim/existing-card')).toHaveLength(1) + expect(calledPaths.filter((p) => p === '/api/helcim/subscription')).toHaveLength(1) + + // HelcimPay modal not opened — card was already on file. + expect(initializeHelcimPay).not.toHaveBeenCalled() + expect(verifyPayment).not.toHaveBeenCalled() + + // Subscription called with the cached id/code from memberData. + const subscriptionCall = $fetch.mock.calls.find((c) => c[0] === '/api/helcim/subscription') + expect(subscriptionCall[1].body).toMatchObject({ + customerId: 'cust-123', + customerCode: 'CST-ABC', + cardToken: 'tok-xyz', + contributionAmount: 15, + }) + }) + + it('falls through to get-or-create-customer when helcimCustomerCode is missing', async () => { + memberData.value = { + helcimCustomerId: 'cust-123', + // helcimCustomerCode missing — must NOT take shortcut + contributionAmount: 15, + } + + $fetch.mockImplementation((path) => { + if (path === '/api/helcim/customer-code') { + return Promise.resolve({ customerId: 'cust-123', customerCode: 'CST-FRESH' }) + } + if (path === '/api/helcim/existing-card') { + return Promise.resolve({ cardToken: 'tok-xyz' }) + } + if (path === '/api/helcim/subscription') { + return Promise.resolve({ success: true }) + } + return Promise.reject(new Error(`Unexpected $fetch call: ${path}`)) + }) + + const { initiatePaymentSetup } = useMemberPayment() + await initiatePaymentSetup() + + const calledPaths = $fetch.mock.calls.map((c) => c[0]) + // Existing helcimCustomerId path -> /api/helcim/customer-code is called. + expect(calledPaths).toContain('/api/helcim/customer-code') + }) + + it('falls through to get-or-create-customer when no card is on file', async () => { + memberData.value = { + helcimCustomerId: 'cust-123', + helcimCustomerCode: 'CST-ABC', + contributionAmount: 15, + } + + initializeHelcimPay.mockResolvedValue(undefined) + verifyPayment.mockResolvedValue({ success: true, cardToken: 'tok-new' }) + + let existingCardCalls = 0 + $fetch.mockImplementation((path) => { + if (path === '/api/helcim/existing-card') { + existingCardCalls++ + return Promise.resolve(null) // no card on file + } + if (path === '/api/helcim/customer-code') { + return Promise.resolve({ customerId: 'cust-123', customerCode: 'CST-ABC' }) + } + if (path === '/api/helcim/verify-payment') { + return Promise.resolve({ success: true }) + } + if (path === '/api/helcim/subscription') { + return Promise.resolve({ success: true }) + } + return Promise.reject(new Error(`Unexpected $fetch call: ${path}`)) + }) + + const { initiatePaymentSetup } = useMemberPayment() + await initiatePaymentSetup() + + // Falls into the full flow — modal opens, verify runs. + expect(initializeHelcimPay).toHaveBeenCalled() + expect(verifyPayment).toHaveBeenCalled() + + const calledPaths = $fetch.mock.calls.map((c) => c[0]) + expect(calledPaths).toContain('/api/helcim/customer-code') + // existing-card was reused from the shortcut probe — should not refetch. + expect(existingCardCalls).toBe(1) + }) +}) diff --git a/tests/server/api/reconcile-payments-route.test.js b/tests/server/api/reconcile-payments-route.test.js index d8eb78d..6b7c4c1 100644 --- a/tests/server/api/reconcile-payments-route.test.js +++ b/tests/server/api/reconcile-payments-route.test.js @@ -2,19 +2,20 @@ import { describe, it, expect, vi, beforeEach } from 'vitest' import Member from '../../../server/models/member.js' import Payment from '../../../server/models/payment.js' -import { listHelcimCustomerTransactions } from '../../../server/utils/helcim.js' +import { getHelcimCustomer, listHelcimCustomerTransactions } from '../../../server/utils/helcim.js' import { upsertPaymentFromHelcim } from '../../../server/utils/payments.js' import reconcileHandler from '../../../server/api/internal/reconcile-payments.post.js' import { createMockEvent } from '../helpers/createMockEvent.js' vi.mock('../../../server/models/member.js', () => ({ - default: { find: vi.fn() } + default: { find: vi.fn(), findByIdAndUpdate: vi.fn() } })) vi.mock('../../../server/models/payment.js', () => ({ default: { findOne: vi.fn() } })) vi.mock('../../../server/utils/mongoose.js', () => ({ connectDB: vi.fn() })) vi.mock('../../../server/utils/helcim.js', () => ({ + getHelcimCustomer: vi.fn(), listHelcimCustomerTransactions: vi.fn() })) vi.mock('../../../server/utils/payments.js', () => ({ @@ -88,6 +89,7 @@ describe('POST /api/internal/reconcile-payments', () => { _id: 'm1', email: 'a@example.com', helcimCustomerId: 'cust-1', + helcimCustomerCode: 'CST-1', helcimSubscriptionId: 'sub-1', billingCadence: 'monthly' } @@ -113,6 +115,7 @@ describe('POST /api/internal/reconcile-payments', () => { { helcimCustomerId: { $exists: true, $ne: null } }, expect.objectContaining({ helcimCustomerId: 1, + helcimCustomerCode: 1, helcimSubscriptionId: 1, billingCadence: 1 }) @@ -134,7 +137,7 @@ describe('POST /api/internal/reconcile-payments', () => { it('does NOT pass sendConfirmation: true (no duplicate confirmation emails)', async () => { Member.find.mockReturnValue(leanResolver([ - { _id: 'm1', helcimCustomerId: 'cust-1' } + { _id: 'm1', helcimCustomerId: 'cust-1', helcimCustomerCode: 'CST-1' } ])) listHelcimCustomerTransactions.mockResolvedValue([ { id: 'tx-paid', status: 'paid', amount: 10, currency: 'CAD' } @@ -157,9 +160,9 @@ describe('POST /api/internal/reconcile-payments', () => { it('continues iterating when listHelcimCustomerTransactions throws for one member', async () => { Member.find.mockReturnValue(leanResolver([ - { _id: 'm1', helcimCustomerId: 'cust-1' }, - { _id: 'm2', helcimCustomerId: 'cust-2' }, - { _id: 'm3', helcimCustomerId: 'cust-3' } + { _id: 'm1', helcimCustomerId: 'cust-1', helcimCustomerCode: 'CST-1' }, + { _id: 'm2', helcimCustomerId: 'cust-2', helcimCustomerCode: 'CST-2' }, + { _id: 'm3', helcimCustomerId: 'cust-3', helcimCustomerCode: 'CST-3' } ])) // m1 succeeds first try, m2 fails all 3 retries, m3 succeeds first try. // Keyed by customerCode so it works regardless of call order (chunked Promise.all). @@ -191,7 +194,7 @@ describe('POST /api/internal/reconcile-payments', () => { it('retries transient Helcim errors with exponential backoff (3 attempts)', async () => { vi.useFakeTimers() Member.find.mockReturnValue(leanResolver([ - { _id: 'm1', helcimCustomerId: 'cust-1' } + { _id: 'm1', helcimCustomerId: 'cust-1', helcimCustomerCode: 'CST-1' } ])) listHelcimCustomerTransactions .mockRejectedValueOnce(new Error('boom 1')) @@ -219,7 +222,7 @@ describe('POST /api/internal/reconcile-payments', () => { it('counts memberErrors when all 3 retry attempts fail', async () => { vi.useFakeTimers() Member.find.mockReturnValue(leanResolver([ - { _id: 'm1', helcimCustomerId: 'cust-1' } + { _id: 'm1', helcimCustomerId: 'cust-1', helcimCustomerCode: 'CST-1' } ])) listHelcimCustomerTransactions.mockRejectedValue(new Error('persistent 503')) @@ -241,7 +244,7 @@ describe('POST /api/internal/reconcile-payments', () => { it('honors ?apply=false dry-run mode (Payment.findOne, no upsert)', async () => { Member.find.mockReturnValue(leanResolver([ - { _id: 'm1', helcimCustomerId: 'cust-1' } + { _id: 'm1', helcimCustomerId: 'cust-1', helcimCustomerCode: 'CST-1' } ])) listHelcimCustomerTransactions.mockResolvedValue([ { id: 'tx-existing', status: 'paid', amount: 10 }, @@ -266,4 +269,63 @@ describe('POST /api/internal/reconcile-payments', () => { existed: 1 }) }) + + describe('helcimCustomerCode backfill', () => { + it('writes helcimCustomerCode when missing on the member doc', async () => { + Member.find.mockReturnValue(leanResolver([ + { _id: 'm1', helcimCustomerId: 'cust-1' } // no helcimCustomerCode + ])) + getHelcimCustomer.mockResolvedValue({ id: 'cust-1', customerCode: 'CST-NEW' }) + listHelcimCustomerTransactions.mockResolvedValue([]) + + const event = createMockEvent({ + method: 'POST', + path: '/api/internal/reconcile-payments', + headers: { 'x-reconcile-token': RECONCILE_TOKEN } + }) + await reconcileHandler(event) + + expect(getHelcimCustomer).toHaveBeenCalledWith('cust-1') + expect(Member.findByIdAndUpdate).toHaveBeenCalledWith( + 'm1', + { $set: { helcimCustomerCode: 'CST-NEW' } }, + { runValidators: false } + ) + }) + + it('skips backfill when helcimCustomerCode is already present', async () => { + Member.find.mockReturnValue(leanResolver([ + { _id: 'm1', helcimCustomerId: 'cust-1', helcimCustomerCode: 'CST-EXISTING' } + ])) + listHelcimCustomerTransactions.mockResolvedValue([]) + + const event = createMockEvent({ + method: 'POST', + path: '/api/internal/reconcile-payments', + headers: { 'x-reconcile-token': RECONCILE_TOKEN } + }) + await reconcileHandler(event) + + expect(getHelcimCustomer).not.toHaveBeenCalled() + expect(Member.findByIdAndUpdate).not.toHaveBeenCalled() + }) + + it('does not fail the run when getHelcimCustomer throws during backfill', async () => { + Member.find.mockReturnValue(leanResolver([ + { _id: 'm1', helcimCustomerId: 'cust-1' } + ])) + getHelcimCustomer.mockRejectedValue(new Error('helcim 503')) + listHelcimCustomerTransactions.mockResolvedValue([]) + + const event = createMockEvent({ + method: 'POST', + path: '/api/internal/reconcile-payments', + headers: { 'x-reconcile-token': RECONCILE_TOKEN } + }) + const result = await reconcileHandler(event) + + expect(Member.findByIdAndUpdate).not.toHaveBeenCalled() + expect(result.memberErrors).toBe(0) // backfill failure is best-effort, not fatal + }) + }) })