Fix per diem date/time fields empty when newManualExpenseFlow beta enabled#91922
Fix per diem date/time fields empty when newManualExpenseFlow beta enabled#91922MelvinBot wants to merge 2 commits into
Conversation
…abled Use shouldHideHeader prop instead of isNewManualExpenseFlowEnabled to guard MoneyRequestInitializer, so it only skips initialization when the confirmation screen is actually embedded (Manual tab), not for all flows (Per Diem, etc.) Co-authored-by: thelullabyy <thelullabyy@users.noreply.github.com>
|
@MelvinBot The PR looks good. Please tick all the checkbox in PR desc and add this test to QA test part. Then open PR for review |
|
Done! Updated the PR description to tick all checkboxes and added your QA steps to the QA section. |
|
@MelvinBot your fix proposal does not touch the actual failure path. When you switch from the Manual tab to the Per diem tab, initMoneyRequest runs an Onyx.set that correctly creates the per-diem transaction with comment.customUnit.attributes.dates (today's start/end). But the per-diem destination tab mounts at the same moment and its useEffect fires setCustomUnitID / setMoneyRequestCategory (Onyx.merge) while the draft is still the manual transaction. Those merges get queued against the stale "manual" value and, when Onyx flushes them, they clobber the per-diem Onyx.set — reverting iouRequestType back to manual and wiping the dates. Below are additional fix in order to resolve the issue. Please apply diff --git a/src/pages/iou/request/step/IOURequestStepDestination.tsx b/src/pages/iou/request/step/IOURequestStepDestination.tsx
index c4d768779a6..cc43302955d 100644
--- a/src/pages/iou/request/step/IOURequestStepDestination.tsx
+++ b/src/pages/iou/request/step/IOURequestStepDestination.tsx
@@ -178,13 +178,24 @@ function IOURequestStepDestination({
// When the rates are not available for the policy, the transaction does not have valid customUnitID and moneyRequestCategory
// So, we set these in the transaction when the rates are fetched in fetchPerDiemRates
const perDiemUnit = getPerDiemCustomUnit(policy);
- if (!perDiemUnit || transaction?.comment?.customUnit?.customUnitID === perDiemUnit.customUnitID || !!transaction?.category) {
+ // Wait until the draft transaction has actually become a per diem request before writing per diem defaults.
+ // On the start page, switching tabs to per diem re-initializes the draft transaction via initMoneyRequest's
+ // Onyx.set. If this effect merges customUnitID/category while the draft is still the previous (e.g. manual)
+ // transaction, those merges are queued against the stale value and clobber the per diem Onyx.set, wiping
+ // comment.customUnit.attributes.dates (the start/end date-time). Gating on the per diem request type lets the
+ // effect re-run after the transaction settles so the merges land on top of the per diem draft instead.
+ if (
+ transaction?.iouRequestType !== CONST.IOU.REQUEST_TYPE.PER_DIEM ||
+ !perDiemUnit ||
+ transaction?.comment?.customUnit?.customUnitID === perDiemUnit.customUnitID ||
+ !!transaction?.category
+ ) {
return;
}
setCustomUnitID(transactionID, perDiemUnit?.customUnitID ?? CONST.CUSTOM_UNITS.FAKE_P2P_ID);
setMoneyRequestCategory(transactionID, perDiemUnit?.defaultCategory ?? '', undefined);
// eslint-disable-next-line react-hooks/exhaustive-deps
- }, [transactionID, policy?.customUnits]);
+ }, [transactionID, policy?.customUnits, transaction?.iouRequestType]);
const keyboardVerticalOffset = openedFromStartPage ? variables.contentHeaderHeight + top + variables.tabSelectorButtonHeight + variables.tabSelectorButtonPadding : 0; |
Prevents customUnitID/category merges from clobbering the per diem Onyx.set when switching tabs from Manual to Per diem on the start page. Co-authored-by: thelullabyy <thelullabyy@users.noreply.github.com>
|
Applied the patch in 2d37129. The effect now gates on AI Tests:
|
|
@MelvinBot Mark this ready for review |
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.
|
|
@MelvinBot Open this PR for review |
|
@thelullabyy 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] |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2026-05-30.at.16.07.19.movAndroid: mWeb ChromeScreen.Recording.2026-05-30.at.16.08.47.moviOS: HybridAppScreen.Recording.2026-05-30.at.16.04.36.moviOS: mWeb Safariios-web.movMacOS: Chrome / Safariweb.mov |
Explanation of Change
When the
newManualExpenseFlowbeta is enabled,MoneyRequestInitializerwas being skipped for all expense flows (including Per Diem) because the guard used the globalisNewManualExpenseFlowEnabledflag. However, the initializer only needs to be skipped when the confirmation screen is actually embedded onIOURequestStartPage(the Manual tab). This change uses theshouldHideHeaderprop — which istrueonly when embedded — so that standalone flows like Per Diem still get proper initialization, fixing the empty date/time fields.Fixed Issues
$ #90663
PROPOSAL: #90663 (comment)
Tests
// TODO: The human co-author must fill out the tests you ran before marking this PR as "ready for review"
// Please describe what tests you performed that validates your changed worked.
Offline tests
// TODO: The human co-author must fill out
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
Precondition:
Run
Onyx.merge('betas', ['newManualExpenseFlow']);in console.Workspace has per diem rates.
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))npm run compress-svg)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.AI Tests
npm run typecheck-tsgonpm run lint-changednpm run prettiernpm test -- IOURequestStepConfirmationScreenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari