Skip to content

Update all reports which have updated totals #71683

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
carlosmiceli merged 40 commits into from
Dec 10, 2025
Merged

Update all reports which have updated totals #71683

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
06c656b
Update all reports which have updated totals
ShridharGoel Oct 1, 2025
9fd6a0c
Improve logic to handle Onyx data in a better way + test updates
ShridharGoel Oct 3, 2025
cef9ca7
Lint fixes
ShridharGoel Oct 3, 2025
771e085
Lint fixes
ShridharGoel Oct 3, 2025
52d2442
Pass ONYXKEYS.COLLECTION.NEXT_STEP data via components
ShridharGoel Oct 3, 2025
6941744
Merge
ShridharGoel Oct 6, 2025
05b6f56
Small fixes
ShridharGoel Oct 6, 2025
0eaea12
Merge branch 'main' into nextSteps
ShridharGoel Oct 9, 2025
d3ef6df
Merge main
ShridharGoel Oct 13, 2025
4776735
Merge branch 'nextSteps' of https://github.com/ShridharGoel/Expensify…
ShridharGoel Oct 13, 2025
41ac480
Merge main
ShridharGoel Oct 13, 2025
7365d3d
Update to remove extra condition
ShridharGoel Oct 15, 2025
e921fcf
Merge main
ShridharGoel Oct 28, 2025
c071505
Merge branch 'main' of https://github.com/Expensify/App into nextSteps
ShridharGoel Oct 29, 2025
e331c8f
Improve logic and handle approval scenario
ShridharGoel Oct 29, 2025
3f3990f
Merge main
ShridharGoel Oct 29, 2025
f3656cb
Update tests
ShridharGoel Oct 29, 2025
4e19411
Lint and type fixes
ShridharGoel Oct 29, 2025
a995cd0
Fix
ShridharGoel Oct 29, 2025
1383ec6
Merge branch 'main' into nextSteps
ShridharGoel Nov 4, 2025
ecc0401
Merge main
ShridharGoel Nov 5, 2025
d6966f7
Merge main
ShridharGoel Nov 7, 2025
d3239fc
Update tests
ShridharGoel Nov 7, 2025
469804a
Merge main
ShridharGoel Nov 7, 2025
df6bfb3
Use buildNextStepNew
ShridharGoel Nov 7, 2025
bf8cb1a
Merge branch 'main' of https://github.com/Expensify/App into nextSteps
ShridharGoel Nov 7, 2025
06c841c
Merge main
ShridharGoel Nov 24, 2025
2bb5b67
Update to use cleaner logic and follow backend behaviour
ShridharGoel Nov 24, 2025
83cdbf9
Update to use cleaner logic and follow backend behaviour
ShridharGoel Nov 24, 2025
306b997
Prettier fix
ShridharGoel Nov 24, 2025
467adee
Lint fix
ShridharGoel Nov 24, 2025
2cefc3e
Merge main
ShridharGoel Dec 2, 2025
0732012
Improve logic and add new test
ShridharGoel Dec 4, 2025
43d55a4
Merge branch 'main' into nextSteps
ShridharGoel Dec 4, 2025
97aa338
Lint fixes
ShridharGoel Dec 4, 2025
538bc88
Fix issue
ShridharGoel Dec 5, 2025
3d1afe8
Lint fixes
ShridharGoel Dec 5, 2025
28db791
Fix test
ShridharGoel Dec 5, 2025
2038f2f
Lint fixes
ShridharGoel Dec 5, 2025
6bc42a1
Fix test
ShridharGoel Dec 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
169 changes: 96 additions & 73 deletions src/libs/actions/Transaction.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@
import {getCurrentUserAccountID} from './Report';

const allTransactions: Record<string, Transaction> = {};
Onyx.connect({

Check warning on line 56 in src/libs/actions/Transaction.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.COLLECTION.TRANSACTION,
callback: (transaction, key) => {
if (!key || !transaction) {
Expand All @@ -65,7 +65,7 @@
});

let allTransactionDrafts: OnyxCollection<Transaction> = {};
Onyx.connect({

Check warning on line 68 in src/libs/actions/Transaction.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.COLLECTION.TRANSACTION_DRAFT,
waitForCollectionCallback: true,
callback: (value) => {
Expand All @@ -74,7 +74,7 @@
});

let allReports: OnyxCollection<Report> = {};
Onyx.connect({

Check warning on line 77 in src/libs/actions/Transaction.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.COLLECTION.REPORT,
waitForCollectionCallback: true,
callback: (value) => {
Expand All @@ -86,7 +86,7 @@
});

const allTransactionViolation: OnyxCollection<TransactionViolation[]> = {};
Onyx.connect({

Check warning on line 89 in src/libs/actions/Transaction.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS,
callback: (transactionViolation, key) => {
if (!key || !transactionViolation) {
Expand All @@ -98,7 +98,7 @@
});

let allTransactionViolations: TransactionViolations = [];
Onyx.connect({

Check warning on line 101 in src/libs/actions/Transaction.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS,
callback: (val) => (allTransactionViolations = val ?? []),
});
Expand Down Expand Up @@ -1294,83 +1294,106 @@
}
}

// 9. Update next step for report
// 9. Update next steps for all affected reports
const destinationReportID = reportID === CONST.REPORT.UNREPORTED_REPORT_ID ? (existingSelfDMReportID ?? selfDMReport?.reportID) : reportID;
const destinationReport = destinationReportID
? (allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${destinationReportID}`] ??
(destinationReportID === newReport?.reportID ? newReport : undefined) ??
(destinationReportID === selfDMReport?.reportID ? selfDMReport : undefined))
: undefined;
const destinationTotal = (destinationReportID ? updatedReportTotals[destinationReportID] : undefined) ?? destinationReport?.total ?? newReport?.total;
const nextStepReport = {
...destinationReport,
reportID: destinationReport?.reportID ?? destinationReportID ?? reportID,
total: destinationTotal,
};
Comment on lines -1304 to -1309

@parasharrajat parasharrajat Dec 8, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part was a little different. I do not know the exact reason why we have fallbacks for reportID key and destination total. Is there a reason why you changed this?

To be safe, I would like to keep the logic same for destination report unless we have reason that we don't need to fallbacks.

To be clear.

reportID: affectedReport.reportID ?? affectedReportID,

You are not not falling back to reportID as in destinationReport?.reportID ?? destinationReportID ?? reportID,

destinationTotal = (destinationReportID ? updatedReportTotals[destinationReportID] : undefined) ?? destinationReport?.total ?? newReport?.total;

Here we were falling back to newReport?.total but not in new code.

@parasharrajat parasharrajat Dec 8, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ShridharGoel

@ShridharGoel ShridharGoel Dec 8, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already have the real report in hand before we set next steps, so we don’t need extra fallbacks. Bringing them back wouldn’t help, and could point updates to the wrong report (for example, using the outer reportID instead of
the source report ID). The total fallback also seems unnecessary because when the report is the new one, we already have its total.

@ShridharGoel ShridharGoel Dec 8, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do let me know what you think.

@parasharrajat parasharrajat Dec 8, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, sounds good.

const hasViolations = hasViolationsReportUtils(nextStepReport?.reportID, allTransactionViolation, accountID, email ?? '');

// buildOptimisticNextStep is used in parallel
// eslint-disable-next-line @typescript-eslint/no-deprecated
const optimisticNextStepDeprecated = buildNextStepNew({
report: nextStepReport,
policy,
currentUserAccountIDParam: accountID,
currentUserEmailParam: email,
hasViolations,
isASAPSubmitBetaEnabled,
predictedNextStatus: nextStepReport.statusNum ?? CONST.REPORT.STATUS_NUM.OPEN,
shouldFixViolations,
});
const optimisticNextStep = buildOptimisticNextStep({
report: nextStepReport,
policy,
currentUserAccountIDParam: accountID,
currentUserEmailParam: email,
hasViolations,
isASAPSubmitBetaEnabled,
predictedNextStatus: nextStepReport.statusNum ?? CONST.REPORT.STATUS_NUM.OPEN,
shouldFixViolations,
});
optimisticData.push({
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.NEXT_STEP}${reportID}`,
value: optimisticNextStepDeprecated,
});
optimisticData.push({
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT}${reportID}`,
value: {
nextStep: optimisticNextStep,
pendingFields: {
nextStep: CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE,
const affectedReportIDs = new Set<string>();

for (const reportIDToUpdate of Object.keys(updatedReportTotals)) {
affectedReportIDs.add(reportIDToUpdate);
}

if (destinationReportID) {
affectedReportIDs.add(destinationReportID);
}

for (const affectedReportID of affectedReportIDs) {
const affectedReport =
allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${affectedReportID}`] ??
(affectedReportID === newReport?.reportID ? newReport : undefined) ??
(affectedReportID === selfDMReport?.reportID ? selfDMReport : undefined);

if (!affectedReport) {
return;
}
Comment on lines +1315 to +1317

@parasharrajat parasharrajat Oct 26, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't understand this change. Why are we creating this? What does it give us?

If we do not have the report data, we shouldn't update this report.


const updatedTotal = updatedReportTotals[affectedReportID] ?? affectedReport.total;
const updatedReport = {
...affectedReport,
total: updatedTotal,
reportID: affectedReport.reportID ?? affectedReportID,
};

const predictedNextStatus = updatedReport.statusNum ?? CONST.REPORT.STATUS_NUM.OPEN;

const hasViolations = hasViolationsReportUtils(updatedReport.reportID, allTransactionViolation, accountID, email ?? '');
const isDestinationReport = affectedReportID === destinationReportID;
const shouldFixViolationsForReport = isDestinationReport ? shouldFixViolations : false;
const shouldUseUnreportedNextStepKey = reportID === CONST.REPORT.UNREPORTED_REPORT_ID && isDestinationReport;
const nextStepOnyxReportID = shouldUseUnreportedNextStepKey ? reportID : affectedReportID;

// eslint-disable-next-line @typescript-eslint/no-deprecated
const optimisticNextStepForCollection = buildNextStepNew({
report: updatedReport,
policy,
currentUserAccountIDParam: accountID,
currentUserEmailParam: email,
hasViolations,
isASAPSubmitBetaEnabled,
predictedNextStatus,
shouldFixViolations: shouldFixViolationsForReport,
});
const optimisticNextStepForReport = buildOptimisticNextStep({
report: updatedReport,
policy,
currentUserAccountIDParam: accountID,
currentUserEmailParam: email,
hasViolations,
isASAPSubmitBetaEnabled,
predictedNextStatus,
shouldFixViolations: shouldFixViolationsForReport,
});

optimisticData.push({
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.NEXT_STEP}${nextStepOnyxReportID}`,
value: optimisticNextStepForCollection,
});
optimisticData.push({
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT}${affectedReportID}`,
value: {
nextStep: optimisticNextStepForReport,
pendingFields: {
nextStep: CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE,
},
},
},
});
successData.push({
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT}${reportID}`,
value: {
pendingFields: {
nextStep: null,
});
successData.push({
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT}${affectedReportID}`,
value: {
pendingFields: {
nextStep: null,
},
},
},
});
// @ts-expect-error - will be solved in https://github.com/Expensify/App/issues/73830
failureData.push({
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.NEXT_STEP}${reportID}`,
value: reportNextStep,
});
failureData.push({
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT}${reportID}`,
value: {
nextStep: nextStepReport.nextStep ?? null,
pendingFields: {
nextStep: null,
});
// @ts-expect-error - will be solved in https://github.com/Expensify/App/issues/73830
failureData.push({
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.NEXT_STEP}${nextStepOnyxReportID}`,
value: nextStepOnyxReportID === reportID ? reportNextStep : (affectedReport.nextStep ?? null),
});
failureData.push({
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT}${affectedReportID}`,
value: {
nextStep: affectedReport.nextStep ?? null,
pendingFields: {
nextStep: null,
},
},
},
});
});
}

const parameters: ChangeTransactionsReportParams = {
transactionList: transactionIDs.join(','),
Expand Down
59 changes: 59 additions & 0 deletions tests/unit/TransactionTest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import useOnyx from '@hooks/useOnyx';
import {changeTransactionsReport, saveWaypoint} from '@libs/actions/Transaction';
import DateUtils from '@libs/DateUtils';
import {getAllNonDeletedTransactions} from '@libs/MoneyRequestReportUtils';
import type {buildOptimisticNextStep} from '@libs/NextStepUtils';
import {rand64} from '@libs/NumberUtils';
import {getIOUActionForTransactionID} from '@libs/ReportActionsUtils';
import CONST from '@src/CONST';
Expand Down Expand Up @@ -305,6 +306,64 @@ describe('Transaction', () => {
mockAPIWrite.mockRestore();
});

it('updates the source submitted report next step without reopening when it becomes empty', async () => {
const mockAPIWrite = jest.spyOn(require('@libs/API'), 'write').mockImplementation(() => Promise.resolve());
const buildOptimisticNextStepSpy = jest.spyOn(require('@libs/NextStepUtils'), 'buildOptimisticNextStep');

const transaction = generateTransaction({
reportID: FAKE_OLD_REPORT_ID,
amount: -100,
currency: CONST.CURRENCY.USD,
});
const oldIOUAction: OnyxEntry<ReportAction<typeof CONST.REPORT.ACTIONS.TYPE.IOU>> = {
reportActionID: rand64(),
actionName: CONST.REPORT.ACTIONS.TYPE.IOU,
actorAccountID: CURRENT_USER_ID,
created: DateUtils.getDBTime(),
originalMessage: {
IOUReportID: FAKE_OLD_REPORT_ID,
IOUTransactionID: transaction.transactionID,
amount: transaction.amount,
currency: transaction.currency,
type: CONST.IOU.REPORT_ACTION_TYPE.CREATE,
},
};
const submittedReport: Report = {
...createExpenseReport(6),
reportID: FAKE_OLD_REPORT_ID,
ownerAccountID: CURRENT_USER_ID,
stateNum: CONST.REPORT.STATE_NUM.OPEN,
statusNum: CONST.REPORT.STATUS_NUM.SUBMITTED,
currency: CONST.CURRENCY.USD,
total: -100,
};

await Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`, transaction);
await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${FAKE_OLD_REPORT_ID}`, submittedReport);
await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${FAKE_OLD_REPORT_ID}`, {[oldIOUAction.reportActionID]: oldIOUAction});
const report = await getReportFromUseOnyx(FAKE_NEW_REPORT_ID);

changeTransactionsReport([transaction.transactionID], false, CURRENT_USER_ID, 'test@example.com', report);
await waitForBatchedUpdates();

try {
const buildOptimisticNextStepCalls = buildOptimisticNextStepSpy.mock.calls as Array<[Parameters<typeof buildOptimisticNextStep>[0]]>;
const sourceNextStepCall = buildOptimisticNextStepCalls.find(([params]) => params.report?.reportID === FAKE_OLD_REPORT_ID);

expect(sourceNextStepCall).toBeDefined();
expect(sourceNextStepCall?.[0].predictedNextStatus).toBe(CONST.REPORT.STATUS_NUM.SUBMITTED);

const apiWriteCall = mockAPIWrite.mock.calls.at(0);
const optimisticData = (apiWriteCall?.[2] as {optimisticData?: Array<{key: string}>})?.optimisticData;
const sourceNextStepUpdate = optimisticData?.find((data) => data.key === `${ONYXKEYS.COLLECTION.NEXT_STEP}${FAKE_OLD_REPORT_ID}`);

expect(sourceNextStepUpdate).toBeDefined();
} finally {
buildOptimisticNextStepSpy.mockRestore();
mockAPIWrite.mockRestore();
}
});

it('correctly handles ASAP submit beta enabled when moving transactions', async () => {
const mockAPIWrite = jest.spyOn(require('@libs/API'), 'write').mockImplementation(() => Promise.resolve());

Expand Down
Loading