Refactor ImportSpreadsheetConfirmModal flow#90620
Conversation
melvin-bot
Bot
requested review from
ZhenjaHorbach and
trjExpensify
and removed request for
a team and
trjExpensify
|
@ZhenjaHorbach 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] |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c72d3360d0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
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.
|
|
@ZhenjaHorbach kind ping here. thanks |
|
@KJ21-ENG can you comment on (and resolve if you can) the review comments you think you've addressed? |
|
@roryabraham Done. I commented on each addressed review thread and resolved the ones covered by a23f967. |
|
@roryabraham @ZhenjaHorbach kind ping here. |
|
@roryabraham @ZhenjaHorbach kind ping here. Thanks |
|
Will recheck today |
|
@ZhenjaHorbach kind ping here. Thanks |
|
@codex |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 62a1efa630
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@ZhenjaHorbach I pushed 43210e8 to address the two latest Codex P1s. The queued imports stay on |
Reviewer Checklist
Screenshots/VideosAndroid: HybridApp2026-05-21.13.25.22.mov2026-05-21.13.26.26.mov2026-05-21.13.26.42.mov2026-05-21.13.27.49.mov2026-05-21.13.28.18.mov2026-05-21.13.28.42.mov2026-05-21.13.29.07.movAndroid: mWeb Chrome2026-05-21.13.13.54.mov2026-05-21.13.16.41.mov2026-05-21.13.17.03.mov2026-05-21.13.18.57.mov2026-05-21.13.19.22.mov2026-05-21.13.19.43.mov2026-05-21.13.20.03.moviOS: HybridApp2026-05-21.13.25.22.mov2026-05-21.13.26.26.mov2026-05-21.13.26.42.mov2026-05-21.13.27.49.mov2026-05-21.13.28.18.mov2026-05-21.13.28.42.mov2026-05-21.13.29.07.moviOS: mWeb Safari2026-05-21.13.13.54.mov2026-05-21.13.16.41.mov2026-05-21.13.17.03.mov2026-05-21.13.18.57.mov2026-05-21.13.19.22.mov2026-05-21.13.19.43.mov2026-05-21.13.20.03.movMacOS: Chrome / Safari2026-05-21.12.22.47.mov2026-05-21.12.32.00.mov2026-05-21.12.47.39.mov2026-05-21.12.52.42.mov2026-05-21.12.54.06.mov2026-05-21.12.54.32.mov2026-05-21.12.55.54.mov |
…sheet-modal-refactor # Conflicts: # src/pages/workspace/members/ImportedMembersConfirmationPage.tsx
|
@KJ21-ENG |
@ZhenjaHorbach Actually, I uploaded them, but due to some reason (may be they were slightly over 10 MB), the upload got rejected and I completely overlooked it the whole time. I thought they had already been uploaded. Now, I am uploading them again. For IOS, I am using the simulator, and I am facing an issue related to CSV upload through the file picker (not code side issue, most probably simulator side). I believe I’ll be able to fix it and prepare the recording by the end of the day. I will ping you once it’s ready. Sorry for the delay. |
Still working on this but not able to go through it, trying to fix it ASAP. |
You can drag and drop files into the iOS emulator |
Actually I have tried all these 😅. And problme is not that i am not abel to get files on simulator problem is somehow i am not able to select those csv files from the filepicker. I have found some info regrading this from the Apple Developer Forum that this is known issue. No worries i will do it on physical device and will try to upload rest of the recordings ASAP. Sorry for the delay. |
|
@ZhenjaHorbach Sorry for the dealy, now i have attached pending recordings of iOS native and iOS mWeb. |
|
🚧 @roryabraham has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
4 participants
Explanation of Change
Refactors the import-spreadsheet confirmation flow so the result modal is shown from the import button flow instead of through the deleted
ImportSpreadsheetConfirmModalnull-rendering effect component.The import actions now return an
ImportFinalModalresult and useAPI.makeRequestWithSideEffectsfor import commands that need the server response before showing the modal. The transient modal fields were removed fromONYXKEYS.IMPORTED_SPREADSHEET, while the durable spreadsheet state remains on Onyx.This updates the current import consumers for categories, members, member role confirmation, tags, multi-level tags, per diem, company cards, and wallet transactions. Company-card imports preserve the existing pending-message copy by returning
pendingMessageKeywith the modal data.Fixed Issues
$ #89302
PROPOSAL: #89302 (comment)
Tests
Manual flow checks to run before final review:
Regression checks already run locally:
git diff --checknpm run lint -- <changed files>npm run react-compiler-compliance-check check-changed -- --remote upstreamnpm test -- --runTestsByPath tests/actions/PolicyCategoryTest.ts tests/actions/PolicyMemberTest.ts tests/actions/PolicyTagTest.ts tests/unit/ImportTransactions.test.ts tests/unit/CompanyCardsRBRTest.ts tests/actions/IOU/PerDiemTest.tsnpm run typecheck-tsgowas also run and currently fails only in unrelated baseline files:src/components/MapView/utils.tsandsrc/libs/migrations/ConvertGpsPointsTo2DArray.ts.Offline tests
Manual offline check to run before final review:
QA Steps
Same as Tests.
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.Screenshots/Videos
Android: Native
1779373969496024_under_10mb.mp4
Android: mWeb Chrome
1779374883027426_under_10mb.mp4
iOS: Native
WhatsApp.Video.2026-05-26.at.00.17.24.1.1.mp4
iOS: mWeb Safari
1779743491287751.mp4
MacOS: Chrome / Safari
1779364881560466_under_10mb.mp4