From 60b38aabd354490771a6c0d97df2ee74e4925c52 Mon Sep 17 00:00:00 2001 From: Daniel Frett Date: Fri, 18 Sep 2026 16:22:55 -0600 Subject: [PATCH 1/3] [MPDX-10044] Stop flagging pre-history-window months as failed transfers SAA only returns about the last year of transfer history, but the missed-month scan in filteredTransfers started at recurringStart. A recurring transfer that started more than a year ago had no rows for its older months, so every one of them was shown as a failed transfer even when it ran fine. - Have TransfersPage compute the history window start (start of the month one year ago, matching the SAA/mpdx_api default), send it to the query as transactedAtStart, and pass it to filteredTransfers - Begin the missed-month scan at the first monthly occurrence on or after that window start, stepping in whole months from recurringStart so the failed dates keep the schedule's day of the month - Flag rows whose recurring transfer started before the window as historyTruncated, and note in the failed-transfer modal for those rows only that just the last year of history is shown Co-Authored-By: Claude Fable 5.1 --- public/locales/en/translation.json | 1 + .../FailedTransferModal.test.tsx | 25 +- .../FailedTransferModal.tsx | 5 + .../Helper/filterTransfers.test.ts | 218 +++++++++++++++--- .../Helper/filterTransfers.ts | 31 ++- .../Table/TransfersTable.test.tsx | 13 +- .../Table/TransfersTable.tsx | 1 + .../TransfersPage/TransfersPage.test.tsx | 71 +++++- .../TransfersPage/TransfersPage.tsx | 21 +- .../HrTools/SavingsFundTransfer/mockData.ts | 2 + 10 files changed, 337 insertions(+), 51 deletions(-) diff --git a/public/locales/en/translation.json b/public/locales/en/translation.json index 3d26661204..90925f858b 100644 --- a/public/locales/en/translation.json +++ b/public/locales/en/translation.json @@ -2170,6 +2170,7 @@ "Only delete if you know that this user will not be returning to any other missional organization that uses {{appName}}. You may need to confirm this with them.": "Only delete if you know that this user will not be returning to any other missional organization that uses {{appName}}. You may need to confirm this with them.", "Only import contacts from certain groups": "Only import contacts from certain groups", "Only include medical expenses that are not reimbursable through your staff account.": "Only include medical expenses that are not reimbursable through your staff account.", + "Only the last year of transfer history is shown.": "Only the last year of transfer history is shown.", "Only the portion not reimbursed as ministry expense.": "Only the portion not reimbursed as ministry expense.", "Open hours per week calculator": "Open hours per week calculator", "Opening Balance": "Opening Balance", diff --git a/src/components/HrTools/SavingsFundTransfer/FailedTransferModal/FailedTransferModal.test.tsx b/src/components/HrTools/SavingsFundTransfer/FailedTransferModal/FailedTransferModal.test.tsx index 9a09c9f623..0030cf9507 100644 --- a/src/components/HrTools/SavingsFundTransfer/FailedTransferModal/FailedTransferModal.test.tsx +++ b/src/components/HrTools/SavingsFundTransfer/FailedTransferModal/FailedTransferModal.test.tsx @@ -110,19 +110,20 @@ const mockTransfer: Transfers = { missingMonths: [DateTime.fromISO('2023-08-15')], }; -const TestComponent: React.FC = () => { +const TestComponent: React.FC<{ transfer?: Transfers }> = ({ + transfer = mockTransfer, +}) => { return ( - + ); }; +const historyNote = 'Only the last year of transfer history is shown.'; + describe('FailedTransferModal', () => { it('renders the modal', () => { const { getByText, getAllByRole, getByRole } = render(); @@ -137,6 +138,20 @@ describe('FailedTransferModal', () => { expect(button[1]).toBeInTheDocument(); }); + it('explains that only the last year of history is shown when the transfer started before the window', () => { + const { getByText } = render( + , + ); + + expect(getByText(historyNote)).toBeInTheDocument(); + }); + + it('omits the history note when the whole transfer fits inside the window', () => { + const { queryByText } = render(); + + expect(queryByText(historyNote)).not.toBeInTheDocument(); + }); + it('renders the correct number of transfer rows', () => { const { getByRole } = render(); diff --git a/src/components/HrTools/SavingsFundTransfer/FailedTransferModal/FailedTransferModal.tsx b/src/components/HrTools/SavingsFundTransfer/FailedTransferModal/FailedTransferModal.tsx index daa8742377..3a9b12cd61 100644 --- a/src/components/HrTools/SavingsFundTransfer/FailedTransferModal/FailedTransferModal.tsx +++ b/src/components/HrTools/SavingsFundTransfer/FailedTransferModal/FailedTransferModal.tsx @@ -123,6 +123,11 @@ export const FailedTransferModal: React.FC = ({ + {transfer.historyTruncated && ( + + {t('Only the last year of transfer history is shown.')} + + )} For more information about failed transfers, email{' '} diff --git a/src/components/HrTools/SavingsFundTransfer/Helper/filterTransfers.test.ts b/src/components/HrTools/SavingsFundTransfer/Helper/filterTransfers.test.ts index 2a8771424c..5ab2792873 100644 --- a/src/components/HrTools/SavingsFundTransfer/Helper/filterTransfers.test.ts +++ b/src/components/HrTools/SavingsFundTransfer/Helper/filterTransfers.test.ts @@ -2,6 +2,9 @@ import { DateTime, Settings } from 'luxon'; import { Transactions } from 'src/components/HrTools/SavingsFundTransfer/mockData'; import { filteredTransfers } from './filterTransfers'; +// Earlier than every fixture below, so the window never clips a scan unless a test wants it to. +const historyStart = DateTime.fromISO('2023-01-01'); + const mockTransactions: Transactions[] = [ { transaction: { @@ -152,20 +155,68 @@ const mockTransactions: Transactions[] = [ }, ]; +// One positive $20 transaction per date, all tied to the same recurring transfer. +const makeRecurringTransactions = ({ + id, + recurringStart, + recurringEnd = null, + active = true, + dates, +}: { + id: string; + recurringStart: string; + recurringEnd?: string | null; + active?: boolean; + dates: string[]; +}): Transactions[] => { + const recurringTransfer = { + id, + amount: 20, + recurringStart: DateTime.fromISO(recurringStart), + recurringEnd: recurringEnd ? DateTime.fromISO(recurringEnd) : null, + active, + }; + return dates.map((date, index) => ({ + transaction: { + id: `${id}-${index}`, + amount: 20, + description: null, + transactedAt: DateTime.fromISO(date), + }, + subCategory: { + id: '1', + name: 'deposit', + }, + transfer: { + sourceFundTypeName: 'Primary', + destinationFundTypeName: 'Savings', + }, + recurringTransfer, + baseAmount: 20, + failedCount: 0, + })); +}; + describe('useFilteredTransfers', () => { beforeEach(() => { Settings.now = () => Date.parse('2024-01-15'); }); it('should return the correct number of transfers', () => { - const { filtered, upcoming } = filteredTransfers(mockTransactions); + const { filtered, upcoming } = filteredTransfers( + mockTransactions, + historyStart, + ); expect(filtered).toHaveLength(2); // upcoming holds the future-dated recurring transfer and the scheduled transfer expect(upcoming).toHaveLength(2); }); it('should route a pending scheduled transfer to upcoming without summarizing it', () => { - const { filtered, upcoming } = filteredTransfers(mockTransactions); + const { filtered, upcoming } = filteredTransfers( + mockTransactions, + historyStart, + ); const scheduled = upcoming.filter((tx) => tx.scheduledTransfer); expect(scheduled).toHaveLength(1); @@ -175,7 +226,7 @@ describe('useFilteredTransfers', () => { }); it('should correctly add amounts for recurring transfers', () => { - const { filtered } = filteredTransfers(mockTransactions); + const { filtered } = filteredTransfers(mockTransactions, historyStart); const recurringTransfer = filtered.find( (tx) => tx.recurringTransfer?.id === '1', ); @@ -183,7 +234,7 @@ describe('useFilteredTransfers', () => { }); it('should include one-time transfers', () => { - const { filtered } = filteredTransfers(mockTransactions); + const { filtered } = filteredTransfers(mockTransactions, historyStart); const oneTimeTransfer = filtered.find( (tx) => tx.recurringTransfer === null, ); @@ -192,7 +243,7 @@ describe('useFilteredTransfers', () => { }); it('should exclude transfers with zero or negative amounts', () => { - const { filtered } = filteredTransfers(mockTransactions); + const { filtered } = filteredTransfers(mockTransactions, historyStart); const negativeAmountTransfer = filtered.find( (tx) => tx.transaction!.amount < 0, ); @@ -200,7 +251,7 @@ describe('useFilteredTransfers', () => { }); it('should correctly calculate failedCount for recurring transfers', () => { - const { filtered } = filteredTransfers(mockTransactions); + const { filtered } = filteredTransfers(mockTransactions, historyStart); const recurringTransfer = filtered.find( (tx) => tx.recurringTransfer?.id === '1', ); @@ -208,7 +259,7 @@ describe('useFilteredTransfers', () => { }); it('should find missing months for recurring transfers', () => { - const { filtered } = filteredTransfers(mockTransactions); + const { filtered } = filteredTransfers(mockTransactions, historyStart); const recurringTransfer = filtered.find( (tx) => tx.recurringTransfer?.id === '1', ); @@ -218,39 +269,21 @@ describe('useFilteredTransfers', () => { }); describe('stopped recurring transfers', () => { - const makeStoppedTransactions = ( - recurringEnd: DateTime | null, - ): Transactions[] => { - const recurringTransfer = { + // Started in September, ran September and November, then stopped. + const makeStoppedTransactions = (recurringEnd: string | null) => + makeRecurringTransactions({ id: '3', - amount: 20, - recurringStart: DateTime.fromISO('2023-09-15'), + recurringStart: '2023-09-15', recurringEnd, active: false, - }; - return ['2023-09-15', '2023-11-15'].map((date, index) => ({ - transaction: { - id: `stopped-${index}`, - amount: 20, - description: null, - transactedAt: DateTime.fromISO(date), - }, - subCategory: { - id: '1', - name: 'deposit', - }, - transfer: { - sourceFundTypeName: 'Primary', - destinationFundTypeName: 'Savings', - }, - recurringTransfer, - baseAmount: 20, - failedCount: 0, - })); - }; + dates: ['2023-09-15', '2023-11-15'], + }); it('should not count months after the last transaction as missing when stopped with no end date', () => { - const { filtered } = filteredTransfers(makeStoppedTransactions(null)); + const { filtered } = filteredTransfers( + makeStoppedTransactions(null), + historyStart, + ); const stoppedTransfer = filtered.find( (tx) => tx.recurringTransfer?.id === '3', ); @@ -262,7 +295,8 @@ describe('useFilteredTransfers', () => { it('should not count months after the last transaction as missing when stopped before a future end date', () => { const { filtered } = filteredTransfers( - makeStoppedTransactions(DateTime.fromISO('2024-06-15')), + makeStoppedTransactions('2024-06-15'), + historyStart, ); const stoppedTransfer = filtered.find( (tx) => tx.recurringTransfer?.id === '3', @@ -275,7 +309,8 @@ describe('useFilteredTransfers', () => { it('should scan through the end date for inactive transfers that ended naturally', () => { const { filtered } = filteredTransfers( - makeStoppedTransactions(DateTime.fromISO('2023-12-15')), + makeStoppedTransactions('2023-12-15'), + historyStart, ); const endedTransfer = filtered.find( (tx) => tx.recurringTransfer?.id === '3', @@ -288,7 +323,8 @@ describe('useFilteredTransfers', () => { it('should scan through the end date when a stopped transfer ends today', () => { const { filtered } = filteredTransfers( - makeStoppedTransactions(DateTime.fromISO('2024-01-15')), + makeStoppedTransactions('2024-01-15'), + historyStart, ); const endedTransfer = filtered.find( (tx) => tx.recurringTransfer?.id === '3', @@ -299,4 +335,110 @@ describe('useFilteredTransfers', () => { expect(endedTransfer?.failedCount).toBe(3); }); }); + + describe('history window', () => { + // The page only requests about the last year of transactions, so a long-running recurring + // transfer has no rows before the window even when every month ran successfully. + const makeLongRunningTransactions = ( + dates: string[], + recurringStart = '2022-06-15', + ) => makeRecurringTransactions({ id: '4', recurringStart, dates }); + + it('should not count months before the history window as missing, and flag the history as truncated', () => { + const { filtered } = filteredTransfers( + makeLongRunningTransactions([ + '2023-01-15', + '2023-02-15', + '2023-03-15', + '2023-04-15', + '2023-05-15', + '2023-06-15', + '2023-07-15', + '2023-08-15', + '2023-09-15', + '2023-10-15', + '2023-11-15', + '2023-12-15', + '2024-01-15', + ]), + historyStart, + ); + const longRunning = filtered.find( + (tx) => tx.recurringTransfer?.id === '4', + ); + expect(longRunning?.missingMonths).toEqual([]); + expect(longRunning?.failedCount).toBe(0); + expect(longRunning?.historyTruncated).toBe(true); + }); + + it('should still flag a missed month inside the history window', () => { + const { filtered } = filteredTransfers( + makeLongRunningTransactions([ + '2023-01-15', + '2023-02-15', + '2023-03-15', + '2023-04-15', + '2023-05-15', + '2023-06-15', + '2023-07-15', + '2023-08-15', + '2023-09-15', + '2023-10-15', + '2023-12-15', + '2024-01-15', + ]), + historyStart, + ); + const longRunning = filtered.find( + (tx) => tx.recurringTransfer?.id === '4', + ); + expect( + longRunning?.missingMonths?.map((month) => month.toISODate()), + ).toEqual(['2023-11-15']); + expect(longRunning?.failedCount).toBe(1); + }); + + it('should keep the recurring day of month when the first scanned month is missing', () => { + const { filtered } = filteredTransfers( + makeLongRunningTransactions( + [ + '2023-02-20', + '2023-03-20', + '2023-04-20', + '2023-05-20', + '2023-06-20', + '2023-07-20', + '2023-08-20', + '2023-09-20', + '2023-10-20', + '2023-11-20', + '2023-12-20', + '2024-01-20', + ], + '2022-06-20', + ), + historyStart, + ); + const longRunning = filtered.find( + (tx) => tx.recurringTransfer?.id === '4', + ); + expect( + longRunning?.missingMonths?.map((month) => month.toISODate()), + ).toEqual(['2023-01-20']); + }); + + it('should scan from recurringStart, and not flag the history as truncated, when it is inside the window', () => { + const { filtered } = filteredTransfers( + makeLongRunningTransactions(['2023-10-15', '2024-01-15'], '2023-09-15'), + historyStart, + ); + const longRunning = filtered.find( + (tx) => tx.recurringTransfer?.id === '4', + ); + expect( + longRunning?.missingMonths?.map((month) => month.toISODate()), + ).toEqual(['2023-09-15', '2023-11-15', '2023-12-15']); + expect(longRunning?.historyTruncated).toBe(false); + }); + }); }); diff --git a/src/components/HrTools/SavingsFundTransfer/Helper/filterTransfers.ts b/src/components/HrTools/SavingsFundTransfer/Helper/filterTransfers.ts index 266e6a23be..d4ad820011 100644 --- a/src/components/HrTools/SavingsFundTransfer/Helper/filterTransfers.ts +++ b/src/components/HrTools/SavingsFundTransfer/Helper/filterTransfers.ts @@ -13,7 +13,13 @@ interface Summary { // Transfer history contains multiple transactions for recurring transfers. // This hook summarizes those recurring transfers into a single transaction with the total amount. // It also identifies any missed transfers and includes them as separate transactions with a failed status. -export function filteredTransfers(transfers: Transactions[]) { +// +// historyStart is the start of the window the page requested the transfers for. Months before it +// have no rows even when they ran, so the missed-month scan must not begin before it (MPDX-10044). +export function filteredTransfers( + transfers: Transactions[], + historyStart: DateTime, +) { const filtered: Transactions[] = []; const upcoming: Transactions[] = []; const summary = new Map(); @@ -57,11 +63,13 @@ export function filteredTransfers(transfers: Transactions[]) { } } + const currentDate = DateTime.local().startOf('day'); + const windowStart = historyStart.startOf('day'); + for (const [, item] of summary) { const { index, seenMonths, transactions } = item; const transferRow = filtered[index]; - const currentDate = DateTime.local().startOf('day'); const recurring = transferRow.recurringTransfer; const start = recurring?.recurringStart.startOf('day'); const recurringEnd = recurring?.recurringEnd?.startOf('day') ?? null; @@ -84,8 +92,9 @@ export function filteredTransfers(transfers: Transactions[]) { } transferRow.missingMonths = []; + transferRow.historyTruncated = start < windowStart; - let current = start; + let current = firstOccurrenceOnOrAfter(start, windowStart); while (current <= end) { const key = `${current.year}-${current.month}`; if (!seenMonths.has(key)) { @@ -100,3 +109,19 @@ export function filteredTransfers(transfers: Transactions[]) { return { filtered, upcoming }; } + +// The first monthly occurrence of a schedule starting at `start` that falls on or after `floor`. +// Steps in whole months from `start` (rather than snapping to `floor`) so the result keeps the +// schedule's day of the month, which is what the failed-transfer modal displays. +function firstOccurrenceOnOrAfter(start: DateTime, floor: DateTime): DateTime { + if (start >= floor) { + return start; + } + + const wholeMonths = Math.floor(floor.diff(start, 'months').months); + let occurrence = start.plus({ months: wholeMonths }); + if (occurrence < floor) { + occurrence = start.plus({ months: wholeMonths + 1 }); + } + return occurrence; +} diff --git a/src/components/HrTools/SavingsFundTransfer/Table/TransfersTable.test.tsx b/src/components/HrTools/SavingsFundTransfer/Table/TransfersTable.test.tsx index 51653cab52..cdd84cf404 100644 --- a/src/components/HrTools/SavingsFundTransfer/Table/TransfersTable.test.tsx +++ b/src/components/HrTools/SavingsFundTransfer/Table/TransfersTable.test.tsx @@ -15,7 +15,7 @@ import { Transfers, mockData, } from '../mockData'; -import { TransfersTable } from './TransfersTable'; +import { CreateTransferRows, TransfersTable } from './TransfersTable'; const mutationSpy = jest.fn(); const handleOpenMock = jest.fn(); @@ -371,4 +371,15 @@ describe('TransferHistoryTable', () => { expect(nextPaymentCell(container)).toBe(''); }); }); + + describe('CreateTransferRows', () => { + it('carries the truncated-history flag through to the row the failed modal reads', () => { + const row = CreateTransferRows({ + ...mockHistory[0], + historyTruncated: true, + }); + + expect(row.historyTruncated).toBe(true); + }); + }); }); diff --git a/src/components/HrTools/SavingsFundTransfer/Table/TransfersTable.tsx b/src/components/HrTools/SavingsFundTransfer/Table/TransfersTable.tsx index 566552e91f..1924c4ddfa 100644 --- a/src/components/HrTools/SavingsFundTransfer/Table/TransfersTable.tsx +++ b/src/components/HrTools/SavingsFundTransfer/Table/TransfersTable.tsx @@ -52,6 +52,7 @@ export const CreateTransferRows = (history: Transfers): Transfers => ({ baseAmount: history.baseAmount, summarizedTransfers: history.summarizedTransfers ?? null, missingMonths: history.missingMonths ?? null, + historyTruncated: history.historyTruncated ?? false, }); const createToolbar = (history: Transfers[], type: TableTypeEnum) => { diff --git a/src/components/HrTools/SavingsFundTransfer/TransfersPage/TransfersPage.test.tsx b/src/components/HrTools/SavingsFundTransfer/TransfersPage/TransfersPage.test.tsx index df394ba4ce..fc13db27ee 100644 --- a/src/components/HrTools/SavingsFundTransfer/TransfersPage/TransfersPage.test.tsx +++ b/src/components/HrTools/SavingsFundTransfer/TransfersPage/TransfersPage.test.tsx @@ -210,9 +210,11 @@ jest.mock('notistack', () => ({ const Components = ({ title = 'Staff Savings Fund Transfers', usStaffGroup = UsStaffGroupEnum.SeniorStaff, + transfers, }: { title?: string; usStaffGroup?: UsStaffGroupEnum; + transfers?: ReportsSavingsFundTransferQuery['reportsSavingsFundTransfer']; }) => ( @@ -224,7 +226,15 @@ const Components = ({ FundBalances: FundBalancesQuery; GetUser: GetUserQuery; }> - mocks={{ ...mock, GetUser: { user: { usStaffGroup } } }} + mocks={{ + ...mock, + ...(transfers && { + ReportsSavingsFundTransfer: { + reportsSavingsFundTransfer: transfers, + }, + }), + GetUser: { user: { usStaffGroup } }, + }} onCall={mutationSpy} > @@ -305,6 +315,65 @@ describe('TransfersPage', () => { expect(within(tables[0]).getAllByRole('columnheader')).toHaveLength(9); }); + it('requests transfer history from the start of the month one year ago', async () => { + render(); + + // SAA would default to this same window; sending it explicitly lets the page + // know where the history begins so it does not flag earlier months as failed. + await waitFor(() => + expect(mutationSpy).toHaveGraphqlOperation('ReportsSavingsFundTransfer', { + transactedAtStart: '2023-01-01', + }), + ); + }); + + it('only lists failed months inside the history window for a long-running recurring transfer', async () => { + // Started in 2022, so everything before the 2023-01-01 window start has no rows. + const recurringTransfer = { + id: '4', + amount: 50, + recurringStart: '2022-06-15', + recurringEnd: null, + active: true, + }; + const longRunning = ['2023-11-15', '2024-01-15'].map((date, index) => ({ + transaction: { + id: `long-${index}`, + amount: 50, + description: null, + transactedAt: `${date}T00:00:00+00:00`, + }, + subCategory: { id: '1', name: 'deposit' }, + transfer: { + sourceFundTypeName: 'Primary', + destinationFundTypeName: 'Savings', + }, + recurringTransfer, + scheduledTransfer: null, + })); + const { findByTitle, findByRole } = render( + , + ); + + userEvent.click( + await findByTitle('Failed Transfers', {}, { timeout: 10000 }), + ); + + const dialog = await findByRole('dialog'); + expect( + within(dialog).getByText( + 'Only the last year of transfer history is shown.', + ), + ).toBeInTheDocument(); + + const dates = within(dialog) + .getAllByRole('row') + .slice(1) + .map((row) => row.querySelectorAll('td')[2]?.textContent); + expect(dates[0]).toBe('Jan 15, 2023'); + expect(dates.some((date) => date?.endsWith('2022'))).toBe(false); + }); + it.each([ UsStaffGroupEnum.SeniorInternationalStaff, UsStaffGroupEnum.NewInternationalStaff, diff --git a/src/components/HrTools/SavingsFundTransfer/TransfersPage/TransfersPage.tsx b/src/components/HrTools/SavingsFundTransfer/TransfersPage/TransfersPage.tsx index b28449e3e2..914e911f0b 100644 --- a/src/components/HrTools/SavingsFundTransfer/TransfersPage/TransfersPage.tsx +++ b/src/components/HrTools/SavingsFundTransfer/TransfersPage/TransfersPage.tsx @@ -105,8 +105,21 @@ export const TransfersPage: React.FC = ({ title }) => { const { data: staffAccountData, error: staffAccountError } = useStaffAccountQuery(); + // The page shows about a year of history. SAA would default to this same window if none were + // sent, but sending it explicitly lets the missed-month scan know where the fetched history + // begins, so a recurring transfer that started earlier is not shown as failed for months that + // have no rows (MPDX-10044). + const historyStart = useMemo( + () => DateTime.local().minus({ years: 1 }).startOf('month'), + [], + ); + const { data: reportData, loading: reportLoading } = - useReportsSavingsFundTransferQuery(); + useReportsSavingsFundTransferQuery({ + variables: { + transactedAtStart: historyStart.toISODate(), + }, + }); const { data: fundsData, error: fundsError } = useFundBalancesQuery({ variables: { fundTypes, @@ -166,14 +179,15 @@ export const TransfersPage: React.FC = ({ title }) => { failedCount: 0, summarizedTransfers: null, missingMonths: null, + historyTruncated: false, }; }), [reportData], ); const { filtered, upcoming } = useMemo( - () => filteredTransfers(transactions), - [transactions], + () => filteredTransfers(transactions, historyStart), + [transactions, historyStart], ); const transferHistory: Transfers[] = filtered.map((tx) => { @@ -202,6 +216,7 @@ export const TransfersPage: React.FC = ({ title }) => { failedCount: tx.failedCount, summarizedTransfers: tx.summarizedTransfers, missingMonths: tx.missingMonths, + historyTruncated: tx.historyTruncated, }; }); diff --git a/src/components/HrTools/SavingsFundTransfer/mockData.ts b/src/components/HrTools/SavingsFundTransfer/mockData.ts index 8b780e4cbd..bd37ca4f66 100644 --- a/src/components/HrTools/SavingsFundTransfer/mockData.ts +++ b/src/components/HrTools/SavingsFundTransfer/mockData.ts @@ -86,6 +86,7 @@ export interface Transactions { failedCount?: number; summarizedTransfers?: Map | null; missingMonths?: DateTime[] | null; + historyTruncated?: boolean; } export interface Transfers { @@ -106,6 +107,7 @@ export interface Transfers { failedCount?: number; summarizedTransfers?: Map | null; missingMonths?: DateTime[] | null; + historyTruncated?: boolean; } export const incomingTransfers = [ From 52ab2ff73ed91500a78303f84f1e5000a05afc7a Mon Sep 17 00:00:00 2001 From: Daniel Frett Date: Mon, 21 Sep 2026 10:59:55 -0600 Subject: [PATCH 2/3] Keep end-of-month recurring transfers on their day when listing missed months The missed-month scan advanced with plus({ months: 1 }) from the previous occurrence. Luxon clamps Jan 31 + 1 month to Feb 28, and stepping on from that clamped date put every later occurrence on the 28th, so the failed-transfer modal showed "Mar 28" for a schedule that runs on the 31st. The failed count was unaffected because the year-month keys stayed unique. Add whole months to the schedule start each iteration instead, the same way getNextPaymentDate already does, so each occurrence keeps the schedule's day of the month. Co-Authored-By: Claude Fable 5.1 --- .../Helper/filterTransfers.test.ts | 65 +++++++++++++++++++ .../Helper/filterTransfers.ts | 28 ++++---- 2 files changed, 80 insertions(+), 13 deletions(-) diff --git a/src/components/HrTools/SavingsFundTransfer/Helper/filterTransfers.test.ts b/src/components/HrTools/SavingsFundTransfer/Helper/filterTransfers.test.ts index 5ab2792873..17b53d2fe1 100644 --- a/src/components/HrTools/SavingsFundTransfer/Helper/filterTransfers.test.ts +++ b/src/components/HrTools/SavingsFundTransfer/Helper/filterTransfers.test.ts @@ -336,6 +336,43 @@ describe('useFilteredTransfers', () => { }); }); + describe('invalid dates', () => { + // Every comparison against an invalid DateTime is false, so the scan's `current > end` + // exit would never fire; the row has to be skipped before the scan starts. + it('should report no missed months when recurringStart cannot be parsed', () => { + const { filtered } = filteredTransfers( + makeRecurringTransactions({ + id: '5', + recurringStart: '2023-13-01', + dates: ['2023-10-15', '2023-12-15'], + }), + historyStart, + ); + const invalidStart = filtered.find( + (tx) => tx.recurringTransfer?.id === '5', + ); + expect(invalidStart?.missingMonths ?? []).toHaveLength(0); + expect(invalidStart?.failedCount).toBe(0); + }); + + it('should report no missed months when a stopped transfer has an unparseable transaction date', () => { + const { filtered } = filteredTransfers( + makeRecurringTransactions({ + id: '6', + recurringStart: '2023-09-15', + active: false, + dates: ['2023-09-15', 'not-a-date'], + }), + historyStart, + ); + const invalidEnd = filtered.find( + (tx) => tx.recurringTransfer?.id === '6', + ); + expect(invalidEnd?.missingMonths ?? []).toHaveLength(0); + expect(invalidEnd?.failedCount).toBe(0); + }); + }); + describe('history window', () => { // The page only requests about the last year of transactions, so a long-running recurring // transfer has no rows before the window even when every month ran successfully. @@ -427,6 +464,34 @@ describe('useFilteredTransfers', () => { ).toEqual(['2023-01-20']); }); + it('should keep an end-of-month schedule on the last day of each month', () => { + const { filtered } = filteredTransfers( + makeLongRunningTransactions( + [ + '2023-01-31', + '2023-02-28', + '2023-04-30', + '2023-05-31', + '2023-06-30', + '2023-07-31', + '2023-08-31', + '2023-09-30', + '2023-10-31', + '2023-11-30', + '2023-12-31', + ], + '2022-01-31', + ), + historyStart, + ); + const longRunning = filtered.find( + (tx) => tx.recurringTransfer?.id === '4', + ); + expect( + longRunning?.missingMonths?.map((month) => month.toISODate()), + ).toEqual(['2023-03-31']); + }); + it('should scan from recurringStart, and not flag the history as truncated, when it is inside the window', () => { const { filtered } = filteredTransfers( makeLongRunningTransactions(['2023-10-15', '2024-01-15'], '2023-09-15'), diff --git a/src/components/HrTools/SavingsFundTransfer/Helper/filterTransfers.ts b/src/components/HrTools/SavingsFundTransfer/Helper/filterTransfers.ts index d4ad820011..949d536c51 100644 --- a/src/components/HrTools/SavingsFundTransfer/Helper/filterTransfers.ts +++ b/src/components/HrTools/SavingsFundTransfer/Helper/filterTransfers.ts @@ -87,20 +87,24 @@ export function filteredTransfers( end = DateTime.min(end, lastTransactedAt); } - if (!start) { + // An invalid DateTime compares false against everything, which would keep the scan + // below from ever failing its `current <= end` check, so bail out before it starts. + if (!start?.isValid || !end.isValid || !windowStart.isValid) { continue; } transferRow.missingMonths = []; transferRow.historyTruncated = start < windowStart; - let current = firstOccurrenceOnOrAfter(start, windowStart); + // Add whole months to the start so an end-of-month day does not drift once clamped. + let months = monthsUntilOnOrAfter(start, windowStart); + let current = start.plus({ months }); while (current <= end) { const key = `${current.year}-${current.month}`; if (!seenMonths.has(key)) { transferRow.missingMonths.push(current); } - current = current.plus({ months: 1 }); + current = start.plus({ months: ++months }); } transferRow.failedCount = transferRow.missingMonths.length; @@ -110,18 +114,16 @@ export function filteredTransfers( return { filtered, upcoming }; } -// The first monthly occurrence of a schedule starting at `start` that falls on or after `floor`. -// Steps in whole months from `start` (rather than snapping to `floor`) so the result keeps the -// schedule's day of the month, which is what the failed-transfer modal displays. -function firstOccurrenceOnOrAfter(start: DateTime, floor: DateTime): DateTime { +// Number of whole months to add to `start` so the monthly occurrence lands on or after `floor`. +// Counting months (rather than snapping to `floor`) keeps the schedule's day of the month, +// which is what the failed-transfer modal displays. +function monthsUntilOnOrAfter(start: DateTime, floor: DateTime): number { if (start >= floor) { - return start; + return 0; } const wholeMonths = Math.floor(floor.diff(start, 'months').months); - let occurrence = start.plus({ months: wholeMonths }); - if (occurrence < floor) { - occurrence = start.plus({ months: wholeMonths + 1 }); - } - return occurrence; + return start.plus({ months: wholeMonths }) < floor + ? wholeMonths + 1 + : wholeMonths; } From 773f01d630b969e25e4b63e9a8a38745e855de4a Mon Sep 17 00:00:00 2001 From: Daniel Frett Date: Mon, 21 Sep 2026 14:15:06 -0600 Subject: [PATCH 3/3] Share the monthly occurrence math between the missed-month scan and next payment date filterTransfers and getNextPaymentDate each counted whole months from a schedule's start to reach a floor date, guarding against Luxon's end-of-month clamp in their own way. Extract one helper, monthsUntilOccurrenceOnOrAfter, and call it from both. The helper takes the whole months from Luxon's diff and adds one if that occurrence still falls short of the floor. Luxon's diff backtracks when adding the counted months overshoots the later date, so the whole part is always the last occurrence on or before the floor, including when an end-of-month start clamps in a shorter month. Its tests pin the end-of-month, leap-day, exact landing, and normalization cases, asserting both the count and the date it lands on. Co-Authored-By: Claude Fable 5.1 --- .../Helper/filterTransfers.ts | 17 +-- .../Helper/getNextPaymentDate.ts | 11 +- .../monthsUntilOccurrenceOnOrAfter.test.ts | 129 ++++++++++++++++++ .../Helper/monthsUntilOccurrenceOnOrAfter.ts | 20 +++ 4 files changed, 155 insertions(+), 22 deletions(-) create mode 100644 src/components/HrTools/SavingsFundTransfer/Helper/monthsUntilOccurrenceOnOrAfter.test.ts create mode 100644 src/components/HrTools/SavingsFundTransfer/Helper/monthsUntilOccurrenceOnOrAfter.ts diff --git a/src/components/HrTools/SavingsFundTransfer/Helper/filterTransfers.ts b/src/components/HrTools/SavingsFundTransfer/Helper/filterTransfers.ts index 949d536c51..8e31e6994c 100644 --- a/src/components/HrTools/SavingsFundTransfer/Helper/filterTransfers.ts +++ b/src/components/HrTools/SavingsFundTransfer/Helper/filterTransfers.ts @@ -1,5 +1,6 @@ import { DateTime } from 'luxon'; import { Transactions } from 'src/components/HrTools/SavingsFundTransfer/mockData'; +import { monthsUntilOccurrenceOnOrAfter } from './monthsUntilOccurrenceOnOrAfter'; // index is the position of the summarized transfer in the filtered array // seenMonths is a set of months that have been seen for the recurring transfer @@ -97,7 +98,7 @@ export function filteredTransfers( transferRow.historyTruncated = start < windowStart; // Add whole months to the start so an end-of-month day does not drift once clamped. - let months = monthsUntilOnOrAfter(start, windowStart); + let months = monthsUntilOccurrenceOnOrAfter(start, windowStart); let current = start.plus({ months }); while (current <= end) { const key = `${current.year}-${current.month}`; @@ -113,17 +114,3 @@ export function filteredTransfers( return { filtered, upcoming }; } - -// Number of whole months to add to `start` so the monthly occurrence lands on or after `floor`. -// Counting months (rather than snapping to `floor`) keeps the schedule's day of the month, -// which is what the failed-transfer modal displays. -function monthsUntilOnOrAfter(start: DateTime, floor: DateTime): number { - if (start >= floor) { - return 0; - } - - const wholeMonths = Math.floor(floor.diff(start, 'months').months); - return start.plus({ months: wholeMonths }) < floor - ? wholeMonths + 1 - : wholeMonths; -} diff --git a/src/components/HrTools/SavingsFundTransfer/Helper/getNextPaymentDate.ts b/src/components/HrTools/SavingsFundTransfer/Helper/getNextPaymentDate.ts index 70a325c250..dfa7201528 100644 --- a/src/components/HrTools/SavingsFundTransfer/Helper/getNextPaymentDate.ts +++ b/src/components/HrTools/SavingsFundTransfer/Helper/getNextPaymentDate.ts @@ -1,5 +1,6 @@ import { DateTime } from 'luxon'; import { ScheduleEnum, StatusEnum, Transfers } from '../mockData'; +import { monthsUntilOccurrenceOnOrAfter } from './monthsUntilOccurrenceOnOrAfter'; // SAA does not report the next run, so derive it: monthly on the start day. export function getNextPaymentDate(transfer: Transfers): DateTime | null { @@ -30,13 +31,9 @@ export function getNextPaymentDate(transfer: Transfers): DateTime | null { return null; } - // Add whole months to the start so an end-of-month day does not drift once clamped. - let months = Math.floor(today.diff(start, 'months').months); - let next = start.plus({ months }); - while (next < today) { - months += 1; - next = start.plus({ months }); - } + const next = start.plus({ + months: monthsUntilOccurrenceOnOrAfter(start, today), + }); if (endDate?.isValid && next > toViewerDate(endDate)) { return null; diff --git a/src/components/HrTools/SavingsFundTransfer/Helper/monthsUntilOccurrenceOnOrAfter.test.ts b/src/components/HrTools/SavingsFundTransfer/Helper/monthsUntilOccurrenceOnOrAfter.test.ts new file mode 100644 index 0000000000..dd6c1097e3 --- /dev/null +++ b/src/components/HrTools/SavingsFundTransfer/Helper/monthsUntilOccurrenceOnOrAfter.test.ts @@ -0,0 +1,129 @@ +import { DateTime } from 'luxon'; +import { monthsUntilOccurrenceOnOrAfter } from './monthsUntilOccurrenceOnOrAfter'; + +const months = (start: string, floor: string) => + monthsUntilOccurrenceOnOrAfter( + DateTime.fromISO(start), + DateTime.fromISO(floor), + ); + +const landing = (start: string, floor: string) => + DateTime.fromISO(start) + .plus({ months: months(start, floor) }) + .toISODate(); + +describe('monthsUntilOccurrenceOnOrAfter', () => { + it('returns 0 when the start equals the floor', () => { + expect(months('2023-09-15', '2023-09-15')).toBe(0); + expect(landing('2023-09-15', '2023-09-15')).toBe('2023-09-15'); + }); + + it('returns 0 when the start is after the floor', () => { + expect(months('2023-09-15', '2023-01-01')).toBe(0); + expect(landing('2023-09-15', '2023-01-01')).toBe('2023-09-15'); + }); + + it('returns 1 when the floor is one day after the start', () => { + expect(months('2023-09-15', '2023-09-16')).toBe(1); + expect(landing('2023-09-15', '2023-09-16')).toBe('2023-10-15'); + }); + + it('returns 1 when the floor is exactly one month after the start', () => { + expect(months('2023-09-15', '2023-10-15')).toBe(1); + expect(landing('2023-09-15', '2023-10-15')).toBe('2023-10-15'); + }); + + it('counts up to the first occurrence on or after a floor between occurrences', () => { + expect(months('2022-06-15', '2023-01-01')).toBe(7); + expect(landing('2022-06-15', '2023-01-01')).toBe('2023-01-15'); + }); + + it('counts an occurrence that lands exactly on the floor', () => { + expect(months('2022-06-15', '2023-01-15')).toBe(7); + expect(landing('2022-06-15', '2023-01-15')).toBe('2023-01-15'); + }); + + it('needs one more month when the floor is a day past an occurrence', () => { + expect(months('2022-06-15', '2023-01-16')).toBe(8); + expect(landing('2022-06-15', '2023-01-16')).toBe('2023-02-15'); + }); + + it('lands on the clamped day when an end-of-month start hits a short month exactly', () => { + expect(months('2024-01-31', '2024-02-29')).toBe(1); + expect(landing('2024-01-31', '2024-02-29')).toBe('2024-02-29'); + }); + + it('does not skip a month when the floor is before the clamped day', () => { + expect(months('2024-01-31', '2024-02-15')).toBe(1); + expect(landing('2024-01-31', '2024-02-15')).toBe('2024-02-29'); + }); + + it('adds a month when the clamped occurrence falls short of the floor', () => { + // Jan 31, 2024 + 13 months clamps to Feb 28, 2025, which is before Mar 1. + expect(months('2024-01-31', '2025-03-01')).toBe(14); + expect(landing('2024-01-31', '2025-03-01')).toBe('2025-03-31'); + }); + + it('lands a leap-day start on its clamped anniversary', () => { + expect(months('2024-02-29', '2025-02-28')).toBe(12); + expect(landing('2024-02-29', '2025-02-28')).toBe('2025-02-28'); + }); + + it('adds a month when a leap-day anniversary falls short of the floor', () => { + expect(months('2024-02-29', '2025-03-01')).toBe(13); + expect(landing('2024-02-29', '2025-03-01')).toBe('2025-03-29'); + }); + + it('lands exactly on the floor across a ten-year span', () => { + expect(months('2015-06-15', '2025-06-15')).toBe(120); + expect(landing('2015-06-15', '2025-06-15')).toBe('2025-06-15'); + }); + + it('treats a later time on the same day as after the start, so callers should normalize first', () => { + const start = DateTime.fromISO('2023-09-15T00:00:00'); + const laterThatDay = DateTime.fromISO('2023-09-15T10:00:00'); + + const unnormalized = monthsUntilOccurrenceOnOrAfter(start, laterThatDay); + expect(unnormalized).toBe(1); + expect(start.plus({ months: unnormalized }).toISODate()).toBe('2023-10-15'); + + const normalized = monthsUntilOccurrenceOnOrAfter( + start, + laterThatDay.startOf('day'), + ); + expect(normalized).toBe(0); + expect(start.plus({ months: normalized }).toISODate()).toBe('2023-09-15'); + }); + + it('expects callers to re-anchor a start from another zone before comparing', () => { + // A date parsed with setZone keeps the API's offset, so its year and month fields can + // disagree with a local floor near a month boundary. Re-anchoring keeps the wall-clock + // date and puts both operands in one zone. + const start = DateTime.fromISO('2023-09-15T00:00:00+04:00', { + setZone: true, + }); + const floor = DateTime.fromISO('2023-09-15T00:00:00Z', { setZone: true }); + + const reanchored = start.setZone(floor.zone, { keepLocalTime: true }); + const months = monthsUntilOccurrenceOnOrAfter(reanchored, floor); + expect(months).toBe(0); + expect(reanchored.plus({ months }).toISODate()).toBe('2023-09-15'); + }); + + it('is unaffected by a daylight saving change between the start and the floor', () => { + const zone = 'America/New_York'; + const start = DateTime.fromISO('2024-01-15', { zone }); + const floor = DateTime.fromISO('2024-04-15', { zone }); + + const onFloor = monthsUntilOccurrenceOnOrAfter(start, floor); + expect(onFloor).toBe(3); + expect(start.plus({ months: onFloor }).toISODate()).toBe('2024-04-15'); + + const pastFloor = monthsUntilOccurrenceOnOrAfter( + start, + floor.plus({ days: 1 }), + ); + expect(pastFloor).toBe(4); + expect(start.plus({ months: pastFloor }).toISODate()).toBe('2024-05-15'); + }); +}); diff --git a/src/components/HrTools/SavingsFundTransfer/Helper/monthsUntilOccurrenceOnOrAfter.ts b/src/components/HrTools/SavingsFundTransfer/Helper/monthsUntilOccurrenceOnOrAfter.ts new file mode 100644 index 0000000000..c7acac450c --- /dev/null +++ b/src/components/HrTools/SavingsFundTransfer/Helper/monthsUntilOccurrenceOnOrAfter.ts @@ -0,0 +1,20 @@ +import { DateTime } from 'luxon'; + +// Number of months to add to `start` so the monthly occurrence lands on or after `floor`. +// +// Add the result to `start` in one step rather than walking month by month: Luxon clamps an +// end-of-month day in shorter months (Jan 31 + 1 month = Feb 28), and stepping on from the +// clamped date would put every later occurrence on the 28th. +export function monthsUntilOccurrenceOnOrAfter( + start: DateTime, + floor: DateTime, +): number { + if (start >= floor) { + return 0; + } + + const wholeMonths = Math.floor(floor.diff(start, 'months').months); + return start.plus({ months: wholeMonths }) >= floor + ? wholeMonths + : wholeMonths + 1; +}