Persist reimbursable selection on optimistic CSV import card - #98486
Persist reimbursable selection on optimistic CSV import card#98486MelvinBot wants to merge 1 commit into
Conversation
Co-authored-by: Bruno Rocha <brunovjk@users.noreply.github.com>
|
👋
Every actual code/build check passes — |
|
Every code/build check passes ( |
|
Great work @MelvinBot, please fill out the author checklist, check off all of the ones you can please. Also mark this PR ready for review. |
Reviewer Checklist
Screenshots/VideosAndroid: HybridApp98486_android_native.movAndroid: mWeb Chrome98486_android_web.moviOS: HybridApp98486_ios_native.moviOS: mWeb Safari98486_ios_native.movMacOS: Chrome / Safari98486_web_chrome.mov |
|
Done — I filled out the PR Author Checklist and marked this PR ready for review. Checked off the items I could verify by code inspection and the added test:
Left unchecked because they're human/multi-platform items I can't honestly claim on your behalf:
Two things still need a human before merge:
Web verification notesI drove the flow on web (Chrome): sign-in, Settings > Wallet > Import transactions, set the card display name, and toggled Transactions are reimbursable OFF — all worked. I couldn't complete the CSV upload → RHP toggle check end-to-end because the web automation harness has no file-upload verb, so the runtime "no console errors" / RHP-toggle items are left unchecked. The fix itself is present on the branch — Screenshots from the web run: Import transactions from file RHP, initial stateCard display name saved"Transactions are reimbursable" toggled OFF |
|
@MelvinBot I tested it on all platforms and everything works fine; you can fill in the missing checkboxes. Thanks. |
|
@MelvinBot I do not believe offline testing is possible, as CSV imports can only be performed while online. The QA steps are the same as the tests. Update the PR Author Checklist. |
|
Thanks for testing across platforms! I've checked off the boxes your testing covers:
I left a few unchecked because they assert PR content that isn't there yet or steps you didn't mention — so only you can accurately complete them:
Once the screenshots and QA steps are in, the checklist will be complete. |
|
Why it's failing: this PR has no reviews yet ( The Check summary
Next step: assign/ping a reviewer to approve the PR — that clears the only remaining red check. |
|
Updated the PR Author Checklist:
Still left unchecked (genuine cross-platform / human-run items): all-platform screenshots + tests, the High-Traffic-account run, failure-scenario steps, and the runtime "no console errors" checks. |
|
No commit is needed from me here. |
|
Both go green after you finish the checklist/QA section and a reviewer approves — no push from me will resolve either. |
|
Ran the PR's test steps on Android (standalone NewDot). All steps passed — the fix works: the "Mark transactions as reimbursable" toggle in the card details RHP shows OFF immediately after importing a non-reimbursable CSV (no ON-then-flip), and stays OFF after closing and reopening the RHP.
Screenshots (Android): 1. Import settings — "Transactions are reimbursable" ON (default)2. Toggled OFF before import3. Card RHP immediately after import — toggle OFF (the fix)4. RHP reopened — toggle still OFFNotes on evidence & console
|







Explanation of Change
When a user imports transactions from a CSV and toggles Transactions are reimbursable OFF, that selection was applied to the optimistic transactions, the saved column layout, and the API request — but not to the optimistic card written to
CARD_LIST.buildOptimisticCard()never receivedisReimbursable, so the card'sreimbursablefield wasundefined. The card details page readscard?.reimbursable ?? true, so the toggle rendered enabled right after import, and only self-corrected once the card was re-fetched from the server.This change passes
isReimbursableintobuildOptimisticCard()and setsreimbursableon the optimistic card, so the local state matches the user's selection immediately with no reliance on a server round-trip.Fixed Issues
$ #98243
PROPOSAL: #98243 (comment)
Tests
Offline tests
N/A — CSV imports can only be performed while online, so there is no offline behavior to test for this flow.
QA Steps
Same as tests.
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