[CP Staging] Fix Onfido crash from stale cached wallet data on EnablePaymentsPage#88328
[CP Staging] Fix Onfido crash from stale cached wallet data on EnablePaymentsPage#88328
Conversation
When EnablePaymentsPage mounts after removing initWithStoredValues: false (PR #87254), useOnyx immediately returns cached wallet data with a stale currentStep (e.g. ONFIDO). This renders OnfidoStep before fresh API data arrives, causing a crash on iOS when the Onfido SDK initializes with an expired token. Add a hasFreshData gate so step content only renders after the openEnablePaymentsPage() loading cycle completes (isLoading: true → false). When offline, fall back to cached data since no API call will happen. Co-authored-by: Vit Horacek <mountiny@users.noreply.github.com>
|
🚧 @mountiny has triggered a test Expensify/App build. You can view the workflow run here. |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
This comment has been minimized.
This comment has been minimized.
|
@MelvinBot I think we need to apply the same |
|
👋 Good catch, Specifically, The same pattern (adding
|
|
@MelvinBot implement the fix in EnablePayments.tsx |
Apply the same stale-data protection to EnablePayments.tsx that was already added to EnablePaymentsPage.tsx — prevents rendering step content (including VerifyIdentity/Onfido) before fresh API data arrives. Co-authored-by: Krishna <Krishna2323@users.noreply.github.com>
|
Applied the same Note: I omitted the |
|
@MelvinBot fill the checklist, add tests and mark this ready for review |
|
👋 Checklist & Tests: The PR Author Checklist and Tests/QA sections are intended to be filled out by the human co-author ( Ready for review: Only the PR assignee (
|
melvin-bot
Bot
requested review from
heyjennahay
and removed request for
a team
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| openEnablePaymentsPage(); | ||
| }, [isOffline]); | ||
|
|
||
| // Only render step content after the fresh data loading cycle (isLoading: true → false) completes, |
There was a problem hiding this comment.
❌ CONSISTENCY-3 (docs)
The wasLoadingRef + hasFreshData + useEffect loading-cycle gate pattern is duplicated verbatim between EnablePayments.tsx and EnablePaymentsPage.tsx. Both files implement the same state machine (track isLoading going from true to false, then set hasFreshData), and the same gating condition (!hasFreshData && !isOffline). This increases maintenance risk -- a bug fix in one file could easily be missed in the other.
Extract a shared custom hook, e.g.:
function useHasFreshWalletData(isOffline: boolean, isLoading: boolean | undefined): boolean {
const wasLoadingRef = useRef(false);
const [hasFreshData, setHasFreshData] = useState(false);
useEffect(() => {
if (isOffline) return;
if (isLoading) {
wasLoadingRef.current = true;
return;
}
if (!wasLoadingRef.current) return;
setHasFreshData(true);
}, [isOffline, isLoading]);
return hasFreshData;
}Then use const hasFreshData = useHasFreshWalletData(isOffline, userWallet?.isLoading); in both components.
Reviewed at: 3598f39 | Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
There was a problem hiding this comment.
Lets leave this for now. We can handle this in Part 3. #86660
cc: @JKobrynski
There was a problem hiding this comment.
I will handle in part 3 👍🏻
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / SafariMonosnap.screencast.2026-04-20.19-20-47.mp4 |
inimaga
changed the title
…lletData Fix Onfido crash from stale cached wallet data on EnablePaymentsPage (cherry picked from commit 0f44d00) (cherry-picked to staging by AndrewGable)
|
🚧 @inimaga has triggered a test Expensify/App build. You can view the workflow run here. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Cherry-picked to staging by https://github.com/AndrewGable in version: 9.3.60-19 🚀
Bundle Size Analysis (Sentry): |
|
No help site changes are required for this PR. This is a purely technical bug fix that adds a loading gate to prevent the Onfido SDK from initializing with stale cached wallet data. There are no changes to user-facing workflows, UI labels, steps, or features. The relevant help site article (Set up your wallet) accurately describes the current wallet setup flow, and nothing in this PR alters that flow from the user's perspective. |
|
🚀 Deployed to production by https://github.com/mountiny in version: 9.3.60-22 🚀
|
|
🚀 Cherry-picked to staging by https://github.com/AndrewGable in version: 9.3.61-0 🚀
Bundle Size Analysis (Sentry): |
|
I reviewed the changes in this PR against the help site articles under No help site changes are required. This PR is a purely internal bug fix — it adds a loading state gate (
The existing help article at |
6 participants
Explanation of Change
PR #87254 removed
initWithStoredValues: falsefrom theuseOnyx(ONYXKEYS.USER_WALLET)call inEnablePaymentsPage, which meansuseOnyxnow immediately returns cached/persisted wallet data on mount — including a stalecurrentStep: ONFIDOfrom a previous session. This causesOnfidoStepto render before fresh API data arrives, initializing the Onfido SDK with an expired token and crashing iOS.This PR adds a
hasFreshDatastate gate to the loading condition so that step content only renders after theopenEnablePaymentsPage()loading cycle (isLoading: true → false) completes. When offline, we fall back to cached data since no API call will happen.Fixed Issues
$ #88173
PROPOSAL: #88173 (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 the offline tests 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 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
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari