From e0b8cf022c46c42f54fa5cb519022217403db6a8 Mon Sep 17 00:00:00 2001 From: Justin Gasper Date: Thu, 2 Jul 2026 11:37:15 +1000 Subject: [PATCH] PM-5245: Prefer finance engagement splits What was broken The billing account details modal could still show stale or markup-derived engagement member payment and challenge fee values when billing-account rows already carried split-looking fields. Root cause The follow-up fix only hydrated finance payments for engagement rows missing paymentAmount or challengeFee, and the modal preferred billing-account split fields before any hydrated finance match. Payment normalization also preferred a generic amount field before explicit paymentAmount when both were present. What was changed Consumed engagement rows now always hydrate matching finance payment splits in the modal. Matched finance paymentAmount and challengeFee values take precedence before billing-account aliases, and explicit paymentAmount now wins over generic amount in payment helpers. Any added/updated tests Updated BillingAccountLineItemsModal coverage for finance splits overriding stale billing-account split fields. Added payment utility coverage for payloads containing both amount and paymentAmount. --- .../BillingAccountLineItemsModal.spec.tsx | 48 ++++++++++++++++++- .../BillingAccountLineItemsModal.tsx | 21 ++++---- .../work/src/lib/utils/payment.utils.spec.ts | 10 ++++ src/apps/work/src/lib/utils/payment.utils.ts | 8 ++-- 4 files changed, 70 insertions(+), 17 deletions(-) diff --git a/src/apps/work/src/lib/components/BillingAccountLineItemsModal/BillingAccountLineItemsModal.spec.tsx b/src/apps/work/src/lib/components/BillingAccountLineItemsModal/BillingAccountLineItemsModal.spec.tsx index 67c5d603a..1d13988d2 100644 --- a/src/apps/work/src/lib/components/BillingAccountLineItemsModal/BillingAccountLineItemsModal.spec.tsx +++ b/src/apps/work/src/lib/components/BillingAccountLineItemsModal/BillingAccountLineItemsModal.spec.tsx @@ -432,10 +432,54 @@ describe('BillingAccountLineItemsModal', () => { { amount: '762.66', date: '2026-06-02T13:10:48.235Z', - externalId: 'assignment-5245', + externalId: 'assignment-5245-finance', + externalName: 'Eng BA', + externalType: 'ENGAGEMENT', + memberPaymentAmount: '753.42', + }, + ], + consumedBudget: 762.66, + markup: 0.01226408, + totalBudgetRemaining: 237.34, + }) + + await waitFor(() => { + expect(screen.getByText('$342.00')) + .toBeTruthy() + expect(screen.getByText('$420.66')) + .toBeTruthy() + }) + expect(screen.queryByText('$753.42')) + .toBeNull() + expect(screen.queryByText('$9.24')) + .toBeNull() + expect(mockedFetchAssignmentPaymentSplits) + .toHaveBeenCalledWith('assignment-5245-finance') + }) + + it('uses matching finance engagement payment splits before stale billing-account split fields', async () => { + mockedFetchAssignmentPaymentSplits.mockResolvedValue([ + { + amount: 762.66, + billingAccountId: '80001063', + challengeFee: '420.66', + paymentAmount: '342.00', + paymentId: 'd2223b35-10fc-410e-b3f5-6d6ac482caef', + }, + ]) + + renderModal({ + ...baseBillingAccountDetails, + consumedAmounts: [ + { + amount: '762.66', + challengeFee: '9.24', + date: '2026-06-02T13:10:48.235Z', + externalId: 'assignment-5245-stale', externalName: 'Eng BA', externalType: 'ENGAGEMENT', memberPaymentAmount: '753.42', + paymentAmount: '753.42', }, ], consumedBudget: 762.66, @@ -454,7 +498,7 @@ describe('BillingAccountLineItemsModal', () => { expect(screen.queryByText('$9.24')) .toBeNull() expect(mockedFetchAssignmentPaymentSplits) - .toHaveBeenCalledWith('assignment-5245') + .toHaveBeenCalledWith('assignment-5245-stale') }) it('builds engagement links from assignment-backed billing rows for copilot views', () => { diff --git a/src/apps/work/src/lib/components/BillingAccountLineItemsModal/BillingAccountLineItemsModal.tsx b/src/apps/work/src/lib/components/BillingAccountLineItemsModal/BillingAccountLineItemsModal.tsx index df2a01675..331a13cd3 100644 --- a/src/apps/work/src/lib/components/BillingAccountLineItemsModal/BillingAccountLineItemsModal.tsx +++ b/src/apps/work/src/lib/components/BillingAccountLineItemsModal/BillingAccountLineItemsModal.tsx @@ -379,8 +379,8 @@ function getChallengeLineItemIds(items: BillingAccountLineItem[]): string[] { * @param items Normalized billing-account line items. * @returns Unique assignment ids from engagement consumed rows. * @remarks Locked engagement rows do not correspond to completed finance - * payments, and rows that already carry the persisted split do not need - * additional finance lookups. + * payments. Consumed rows always hydrate from finance so exact payment splits + * can override stale or markup-derived billing-account aliases. */ function getEngagementPaymentAssignmentIds(items: BillingAccountLineItem[]): string[] { return Array.from(new Set( @@ -388,7 +388,6 @@ function getEngagementPaymentAssignmentIds(items: BillingAccountLineItem[]): str .filter(item => ( item.externalType === 'ENGAGEMENT' && item.status === 'consumed' - && (item.paymentAmount === undefined || item.challengeFee === undefined) )) .map(item => normalizeRouteId(item.externalId)) .filter((id): id is string => !!id), @@ -643,14 +642,14 @@ function getLineItemChallengeFeeAmount( billingAccountDetails: BillingAccountDetails, challengeDetailsById: ChallengeDetailsById | undefined, ): number | undefined { - if (item.externalType === 'ENGAGEMENT' && item.challengeFee !== undefined) { - return Number(item.challengeFee.toFixed(2)) - } - if (engagementPaymentSplit?.challengeFee !== undefined) { return engagementPaymentSplit.challengeFee } + if (item.externalType === 'ENGAGEMENT' && item.challengeFee !== undefined) { + return Number(item.challengeFee.toFixed(2)) + } + const consumedChallengeFeeAmount = getConsumedChallengeFeeAmount(item) if (consumedChallengeFeeAmount !== undefined) { @@ -680,14 +679,14 @@ function getEngagementMemberPaymentAmount( billingAccountDetails: BillingAccountDetails, engagementPaymentSplit: EngagementPaymentSplit | undefined, ): number | undefined { - if (item.paymentAmount !== undefined) { - return item.paymentAmount - } - if (engagementPaymentSplit?.paymentAmount !== undefined) { return engagementPaymentSplit.paymentAmount } + if (item.paymentAmount !== undefined) { + return item.paymentAmount + } + if (item.memberPaymentAmount !== undefined) { return item.memberPaymentAmount } diff --git a/src/apps/work/src/lib/utils/payment.utils.spec.ts b/src/apps/work/src/lib/utils/payment.utils.spec.ts index d89b1bd00..4c9a32326 100644 --- a/src/apps/work/src/lib/utils/payment.utils.spec.ts +++ b/src/apps/work/src/lib/utils/payment.utils.spec.ts @@ -48,6 +48,16 @@ describe('payment.utils', () => { .toBe('80004466') }) + it('prefers explicit paymentAmount over generic amount when both are present', () => { + const payment: AssignmentPayment = { + amount: 762.66, + paymentAmount: '342.00', + } + + expect(getPaymentAmount(payment)) + .toBe(342) + }) + it('falls back to the total-versus-gross delta for older payment payloads', () => { const payment: AssignmentPayment = { details: [ diff --git a/src/apps/work/src/lib/utils/payment.utils.ts b/src/apps/work/src/lib/utils/payment.utils.ts index 4ecb093ea..3cb12f822 100644 --- a/src/apps/work/src/lib/utils/payment.utils.ts +++ b/src/apps/work/src/lib/utils/payment.utils.ts @@ -85,14 +85,14 @@ export function formatCurrency(value: unknown): string { } export function getPaymentAmount(payment: AssignmentPayment): number | undefined { - if (payment.amount !== undefined) { - return toNumber(payment.amount) - } - if (payment.paymentAmount !== undefined) { return toNumber(payment.paymentAmount) } + if (payment.amount !== undefined) { + return toNumber(payment.amount) + } + const firstDetail = getFirstPaymentDetail(payment) if (firstDetail) {