Repository navigation
[Payment due @mananjadhav] Read attachment migration flag with Onyx.connectWithoutView - #100531
Conversation
|
@mananjadhav 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] |
|
This PR adds a new |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
tgolen
left a comment
There was a problem hiding this comment.
Thanks for refactoring this!
|
@mananjadhav let's test this, upload the checklist and then we can merge, please 🙏 |
|
Yes. Getting to checklist in next 30 mins. |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppandroid-read-attach-migration.movAndroid: mWeb ChromeNAiOS: HybridAppios-read-attach-migration.moviOS: mWeb SafariNAMacOS: Chrome / SafariNA |
|
🎯 @mananjadhav, thanks for reviewing and testing this PR! 🎉 A payment issue will be created for your review once this PR is deployed to production. If payment is not needed (e.g., regression PR review fix etc), react with 👎 to this comment to prevent the payment issue from being created. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚧 NicolasBonet 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! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/NicolasBonet in version: 9.4.75-0 🚀
|
|
🤖 No help site changes are required for this PR. I did not create a docs PR. Why: the diff touches one file, How I checked the help siteI searched
The help site describes product behavior, not client-side storage internals. This change has no user-facing surface to document. @NicolasBonet, please review the linked help site PR and confirm it reflects the current behavior. Then mark the linked help site PR (Note: there is no linked help site PR — I found no docs change to make. If you believe a specific article should still be updated, reply with |
|
Hi @NicolasBonet. How can we execute Step 2 if we’re using the latest build? Should we download the lowest build first, complete the required steps, and then install the latest build to continue?
cc @tgolen @flaviadefaria @Valforte al |
Continue with steps 2 and so on from the QA steps. |
|
🚀 Deployed to production by https://github.com/mountiny in version: 9.4.75-3 🚀
Bundle Size Analysis (Sentry): |
|
🤖 Payment issue created: #100894 |
|
🚀 Deployed to production by https://github.com/mountiny in version: 9.4.75-3 🚀
Bundle Size Analysis (Sentry): |

Explanation of Change
Replaces the unsupported
OnyxUtils.get()call withOnyx.connectWithoutView()when checking whether attachment paths have already been migrated. Follows the same Promise pattern as #99004 and #98714.Fixed Issues
$ https://github.com/Expensify/Expensify/issues/643611
PROPOSAL: #96531 (comment)
Tests
Passed locally using the existing App dependency installation:
npm test -- --runInBand --watchman=false --runTestsByPath tests/unit/MoveFilesOutOfDocumentsTest.ts— all 11 tests passed, including first-run migration, record preservation, completed-migration skipping, and cleanup failure handling.npm run typecheck— all five projects passed.npm run lint-changed.npm run spell-changed.oxfmt --check src/libs/migrations/MoveFilesOutOfDocuments/index.native.ts.git diff --check.Native device testing has not been performed. Jest reported duplicate manual-mock warnings in unrelated modules; the suite passed.
Offline tests
Pending native verification:
QA Steps
Pending on iOS and Android:
PR Author Checklist
Onyx.connectWithoutView()call is needed.Screenshots/Videos
No UI changes. iOS native QA evidence below; Android still pending.
iOS QA test report (iPhone 17 simulator, HybridApp dev build)
All QA steps passed. The upgrade was emulated by seeding a fresh install with pre-#96531 files before first launch, which exercises the same code paths as a real upgrade.
2024.pdf): still visible in the Files app after relaunch