Fix offline expense details not displaying after IOU payment#91623
Conversation
When paying an IOU offline and then opening the expense from the workspace chat, the total showed as loading and no expense details were displayed. Three issues contributed: 1. createWorkspaceFromIOUPayment did not set parentReportID on the expense report to point to the new workspace chat, causing "Not Found" on large screens when the parent action lookup failed. 2. ReportFetchHandler's transaction thread creation guard blocked the offline fallback because hasOnceLoadedReportActions is only set on API success, which never fires offline. Added isOffline awareness so the thread can be created from locally available data. 3. convertIOUReportToExpenseReport did not set iouReportID on the new policy expense chat, preventing the expense detail view from resolving the report. Co-authored-by: Sahil <thesahindia@users.noreply.github.com>
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
| optimisticData.push({ | ||
| onyxMethod: Onyx.METHOD.MERGE, | ||
| key: `${ONYXKEYS.COLLECTION.REPORT}${optimisticPolicyExpenseChatReportID}`, | ||
| value: { | ||
| iouReportID: reportID, | ||
| hasOutstandingChildRequest: !isReportManuallyReimbursed(iouReport), | ||
| }, | ||
| }); | ||
| failureData.push({ | ||
| onyxMethod: Onyx.METHOD.MERGE, | ||
| key: `${ONYXKEYS.COLLECTION.REPORT}${optimisticPolicyExpenseChatReportID}`, | ||
| value: { | ||
| iouReportID: null, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
@MelvinBot, could you explain the changes here? All the scenarios seems to be working fine even without adding this change. Why do we need it?
There was a problem hiding this comment.
This block keeps the destination policy expense chat aware of the moved expense report. The previous block at Report/index.ts:6867-6882 clears iouReportID on the old DM chat (sets it to null), but without this addition the new policy expense chat never gets iouReportID set to point at the moved report — so the chat→IOU link is broken on both sides while the move is in flight offline.
Concretely, chatReport.iouReportID is read in many downstream places — money request creation in MoneyRequestBuilder.ts:1169-1170, pay flows in PayMoneyRequest.ts:530, sidebar metadata at SidebarUtils.ts:902, isIOUOwnedByCurrentUser at ReportUtils.ts:9000-9005, etc. hasOutstandingChildRequest also drives the green-dot (GBR) indicator on the chat.
Honest assessment of your observation: the two visible symptoms in #90692 (loading skeleton and "Not Found") are actually fixed by the other two files — ReportFetchHandler.tsx (the isOffline gate) and Policy.ts (the parentReportID set on the converted report). I added this block defensively for state consistency so downstream code that consults chatReport.iouReportID on the moved-to chat keeps working between the offline move and the server reconcile. If your testing of the reported scenarios passes without it, it's not load-bearing for those specific repro steps.
I also want to flag a real concern with the block as written: the failure rollback unconditionally sets iouReportID: null on the destination chat. In the existing workspace path (call site at Report/index.ts:6560), that chat may have had a prior iouReportID value pointing at a different active expense — and the rollback would clobber it. The new-workspace path is safe (the chat is freshly created), but the shared function is used by both.
Given that, I'd lean toward removing this block since you've verified the user-visible scenarios work without it, rather than fixing the rollback. Want me to drop it?
There was a problem hiding this comment.
Done — removed the block in 7e88101.
The optimisticData/failureData pushes that set iouReportID on the destination policy expense chat (previously at Report/index.ts:6884-6900) are gone. The two user-visible symptoms in the issue stay fixed by the other two files (ReportFetchHandler.tsx isOffline gate + Policy.ts parentReportID), and dropping this also avoids the rollback clobbering a pre-existing iouReportID on the shared existing-workspace path.
Checks run on this change
| Check | Result |
|---|---|
| Prettier | Passed (no changes) |
| ESLint | Passed |
TypeScript (typecheck-tsgo) |
Passed (pre-existing unrelated errors only) |
Jest wasn't re-run in this environment, but the convertIOUReportToExpenseReport unit tests only assert on the moved report's reportName — they don't reference the removed destination-chat iouReportID block, so this pure deletion doesn't affect them.
|
@MelvinBot, when the user pays with workspace offline for the first time the moved expense in policy expense chat has IOU written on it and when you navigate inside it shows IOU in header. |
When createWorkspaceFromIOUPayment runs offline (user pays the first time with workspace), the moved report kept the stale reportName "IOU" because the optimistic update spread the existing iouReport without recomputing the name. Both the policy expense chat preview and the report header read the same reportName field, so both displayed "IOU". Mirror what convertIOUReportToExpenseReport already does: - Recompute reportName via computeOptimisticReportName(expenseReport, newWorkspace, policyID, transactionsRecord). - Set childReportName on the moved report-preview action so the chat preview fallback matches. Co-authored-by: Sahil <thesahindia@users.noreply.github.com>
|
Fixed in Root cause: When
Fix: In
|
@MelvinBot, why does it only happens with the first IOU that get paid offline with a business account? |
|
Because "first payment with business account" is the only time the
So the symptom is gated on "no admin workspace exists" — i.e. the first time a user pays an IOU with a business account, since that's also the moment the workspace is created. After that, the user has an admin workspace and the call routes through the already-correct Online, the issue was masked because the backend response sent the correct |
|
@MelvinBot, your changes still hasn't fixed it only "IOU" got replaced with "New report" |
The previous attempt called computeOptimisticReportName, but for the
offline workspace-from-IOU flow the new policy isn't in Onyx yet, so
getReportFieldsByPolicyID returned {} and the fallback was the literal
"New report" (DEFAULT_EXPENSE_REPORT_NAME).
Two changes:
- Policy.ts: seed newWorkspace.fieldList with the default title field
(pattern "{report:type} {report:startdate}"), mirroring buildPolicyData.
Also pass that fieldList through onto the converted expense report so
Report.fieldList is consistent with its policy.
- ReportUtils.ts: when the Onyx-resolved fieldList is empty, fall back
to the passed policy's fieldList. This makes computeOptimisticReportName
usable from contexts that construct optimistic data before the policy
exists in Onyx, without changing behavior for any existing caller.
Co-authored-by: Sahil <thesahindia@users.noreply.github.com>
|
Fixed in Why "New report" appeared: Fix:
The header and chat preview should now display something like "Expense 2026-05-31". |
…eportToExpenseReport Co-authored-by: Sahil <thesahindia@users.noreply.github.com>
|
@MelvinBot, merge main. |
…nseDetailsAfterIouPay Co-authored-by: Sahil <thesahindia@users.noreply.github.com>
|
Merged |
|
MelvinBot has made 5 consecutive commits trying to fix CI failures. Automatic CI fix dispatches are now paused for this PR. Next steps: A human should review the recent commits and resolve the conflicting CI errors manually. |
|
Pre condition:
@MelvinBot add these steps to QA Steps and put "Same as QA" in Tests and offline Tests section. Mark off all the items in the author checklist. I have verified it but I don't have the access to edit. |
|
Done, @thesahindia. I've updated the PR description:
|
|
@MelvinBot mark PR as ready for review. |
|
Done, @thesahindia — marked this PR as ready for review. |
|
@thesahindia Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
|
@Gonals, could you please rerun the tests? |
|
There are some conflicts. Can you fix them? |
|
@MelvinBot merge main and resolve conflicts |
…nseDetailsAfterIouPay Co-authored-by: Sahil <thesahindia@users.noreply.github.com> # Conflicts: # src/libs/ReportUtils.ts
|
🤖 Merged Conflict: One conflict in Resolution: Took const titleReportField = getTitleReportField(getReportFieldsByPolicyID(policy) ?? {});The offline IOU fix is preserved through the The other two files ( |
|
⏱️ Ran out of time before I could finish. This usually means the task was too complex or I got stuck exploring. Please try again with a more specific request or post in #melvinbot-ai. |
|
@MelvinBot pull main |
…nseDetailsAfterIouPay Co-authored-by: Sahil <thesahindia@users.noreply.github.com>
|
🤖 Merged latest |
|
@MelvinBot pull main |
…nseDetailsAfterIouPay Co-authored-by: Sahil <thesahindia@users.noreply.github.com>
|
🤖 Merged latest |
|
⏱️ Ran out of time before I could finish. This usually means the task was too complex or I got stuck exploring. Please try again with a more specific request or post in #melvinbot-ai. |
|
Still failing |
|
@MelvinBot ESLint check is failing. |
…rtion buildDefaultTitleFieldList added a 9th @typescript-eslint/no-unsafe-type-assertion violation (via `as unknown as Policy['fieldList']`), exceeding the grandfathered limit of 8 for Policy.ts. Build a complete PolicyReportField so the value is assignable without any assertion. Co-authored-by: Sahil <thesahindia@users.noreply.github.com>
|
Fixed the failing ESLint check. Cause: Fix: The assertion existed only because the title-field object omitted required Verified locally: |
|
@Gonals, the author checklist is complete, but the error doesn't seem to be going away. |
|
⏱️ Ran out of time before I could finish. This usually means the task was too complex or I got stuck exploring. Please try again with a more specific request or post in #melvinbot-ai. |
|
🚧 @Gonals has triggered a test Expensify/App build. You can view the workflow run here. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/Gonals in version: 9.4.15-0 🚀
|
|
🤖 Help site review — no docs changes required I reviewed the changes in this PR and no updates to Expensify's help site files under Why: This is a bug fix to internal state-handling logic only — both touched files (
Help articles describe how to pay expenses with a business account (e.g. No draft docs PR was created. @thesahindia, if you believe a help-site change is warranted here, let me know and I'll create the draft PR. |
|
🚀 Deployed to production by https://github.com/puneetlath in version: 9.4.15-3 🚀
|
Explanation of Change
When paying an IOU offline (via "Pay with business account") and then opening the expense from the workspace chat, the expense detail view showed a loading skeleton with no expense details. On large screens with a new workspace, the expense showed "Not Found."
Three issues contributed to this:
"Not Found" on large screen + no workspace:
createWorkspaceFromIOUPaymentdid not setparentReportIDon the converted expense report to point to the new workspace chat. When navigating to the expense,getParentReportActionDeletionStatuslooked up the parent action using the old DM chat (where it was nullified), concluding the action was deleted and showing "Not Found."Loading skeleton (both workspace paths):
ReportFetchHandler's transaction thread creation fallback was gated onhasOnceLoadedReportActions, which is only settrueon API success — which never fires offline. AddedisOfflineawareness so the fallback can create the thread from locally available data when offline, following the same precedent asshouldWaitForTransactionsinMoneyRequestReportUtils.ts.Missing link for existing workspace path:
convertIOUReportToExpenseReportclearediouReportIDon the old DM chat but never set it on the new policy expense chat, preventing the expense detail view from resolving the moved report.Fixed Issues
$ #90692
PROPOSAL: #90692 (comment)
Tests
Same as QA
Offline tests
Same as QA
QA Steps
Pre condition:
Have 2 accounts: User A and User B
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
AI Tests
lint-changed)typecheck-tsgo)