Keep the selected distance rate on offline-created distance expenses - #100623
Keep the selected distance rate on offline-created distance expenses#100623MelvinBot wants to merge 1 commit into
Conversation
createDistanceRequest destructured customUnitRateID but only forwarded it to the API params, so the optimistic transaction was built with customUnitRateID undefined. buildOptimisticTransaction aliased the draft's customUnit object and then wrote that undefined onto it, wiping the draft's rate too, so the following fastMerge could not restore it. Offline there is no server response to correct the transaction, so the Rate page highlighted nothing. Co-authored-by: abdulrahuman5196 <abdulrahuman5196@users.noreply.github.com>
|
🤖 Web test steps verification results: ✅ pass Created a workspace and opened its expense chat, forced the app offline, created a manual distance expense with typed start/stop waypoints while offline, and confirmed the Rate selection page shows the expense's rate (Default Rate) as selected with a checkmark — verifying the offline distance-rate-selection fix. The PR's Two caveats worth noting: the test workspace had only one distance rate, so this confirms the rate stays selected but does not distinguish between multiple rates; and VERIFY 6 could not be checked against a real console dump. view run · no recording available |
|
@MelvinBot publish the PR |
|
@abdulrahuman5196 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] |
|
🤖 Published — this PR is out of draft and ready for review. PullerBear picked it up and assigned All checks pass except Two things still need a human:
For reference, I did verify the fix on web ahead of this (earlier comment): created a workspace expense chat, went offline, created a manual distance expense, and confirmed the Rate page shows the expense's rate selected. |









Explanation of Change
createDistanceRequestdestructuredcustomUnitRateIDbut only forwarded it to the API params, never to the params used to build the optimistic transaction.buildOptimisticTransactiontherefore ran withcustomUnitRateID: undefined— and because it aliased the draft transaction's owncustomUnitobject instead of copying it, writing thatundefinedalso wiped the rate ID off the draft. The followingfastMerge(existingTransaction, optimisticTransaction)had nothing left to restore from, because both sides pointed at the same cleared object.Online this is invisible: the server response corrects the transaction within about a second. Offline there is no response, so the transaction keeps
comment.customUnit.customUnitRateID === undefined, matches no policy rate on the Rate page, and no row is highlighted.Changes:
createDistanceRequestnow passes the already-destructuredcustomUnitRateIDinto thetransactionParamsit hands togetMoneyRequestInformation.createSplitsAndOnyxDataaccepts and forwardscustomUnitRateID(its params type no longerOmits it).buildOptimisticTransaction: copy the existingcustomUnitinstead of aliasing it, and only overwritecustomUnitRateIDwhen a caller actually passed one. That way no future caller that omits the param can silently wipe the draft's rate.Added unit tests for both creation paths and for the
buildOptimisticTransactionguard. All four fail onmainand pass with this change.Fixed Issues
$ #98194
PROPOSAL: #98194 (comment)
Tests
// TODO: The human co-author must fill out the tests they ran before marking this PR as "ready for review".
Offline tests
// TODO: The human co-author must fill out the offline tests they ran before marking this PR as "ready for review".
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, 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.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