Skip to content

[Onyx Audit] Migrate keys to RAM-only part 3/3#86660

Open
JKobrynski wants to merge 19 commits intoExpensify:mainfrom
callstack-internal:JKobrynski/feat/80091-migrate-keys-to-ram-only-part-3
Open

[Onyx Audit] Migrate keys to RAM-only part 3/3#86660
JKobrynski wants to merge 19 commits intoExpensify:mainfrom
callstack-internal:JKobrynski/feat/80091-migrate-keys-to-ram-only-part-3

Conversation

@JKobrynski
Copy link
Copy Markdown
Contributor

@JKobrynski JKobrynski commented Mar 30, 2026

Explanation of Change

Coming from this comment. The main goal of this PR is to remove initWithStoredValues from keys that turned out to be more problematic than anticipated during migration part 1. At the moment, it's about migrating the following keys:

  • IS_CHECKING_PUBLIC_ROOM
  • IS_LOADING_APP
  • IS_LOADING_REPORT_DATA
  • PLAID_LINK_TOKEN

Disclaimer:

"Migration" doesn't mean that keys need to be migrated to RAM-only, it means we should remove initWithStoredValues param from their calls and consider whether or not they should be migrated or not (and apply adjustments if necessary to maintain functionality).

Fixed Issues

$ #80091
PROPOSAL: N/A

Tests

IS_CHECKING_PUBLIC_ROOM

Deep link sign-in (Android/iOS)

  1. Sign in as User A on the mobile app
  2. Kill the app completely
  3. Open a transition deep link (e.g. from web-to-app flow signed in as User B)
  4. Verify you are signed in as User B viewing the target report
  5. Sign out
  6. Kill the app
  7. Reopen the app by tapping the app icon (NOT via a deep link)
  8. Expected: You should see the sign-in page, NOT be automatically re-authenticated as User B

Public room deep link (web)

  1. Sign out of the app
  2. Open a deep link to a public room (e.g. https://dev.new.expensify.com/r/<publicRoomID>)
  3. Expected: The public room should load correctly for anonymous users

Normal app load

  1. Open the app normally (no deep link)
  2. Expected: App loads to the inbox without getting stuck on the splash screen

HybridApp

  1. Open the app via the HybridApp (OldDot -> NewDot transition)
  2. Expected: App loads normally, no splash screen freeze

IS_LOADING_APP

Normal app load

  1. Run the web app (npm run web)
  2. Sign in and verify the inbox loads with reports visible
  3. Expected: No stuck loading skeletons, no infinite spinner

IS_LOADING_REPORT_DATA

Normal app load

  1. Sign in and verify the inbox loads with reports visible
  2. Expected: No stuck loading skeletons in the sidebar or report list

Concierge chat

  1. Navigate to the Concierge chat
  2. Expected: Chat opens correctly without errors or delays

Onboarding (fresh account)

  1. If possible, sign in with a fresh test account
  2. Expected: Onboarding flow proceeds normally, no skipped steps

PLAID_LINK_TOKEN

Workspace bank account connection

  1. Sign in and go to a workspace
  2. Navigate to Settings > Bank account
  3. Start the bank account connection flow, choose "Connect via Plaid"
  4. Expected: A loading spinner appears briefly, then the Plaid modal opens
  5. Exit the Plaid modal without completing
  6. Expected: No stuck spinner after exiting

Re-entering the Plaid flow

  1. After exiting the Plaid modal (from the previous test), navigate away from the bank account page
  2. Come back and start the Plaid flow again
  3. Expected: Flow works the same as the first time, no stale token issues
  • Verify that no errors appear in the JS console

Offline tests

IS_LOADING_APP

  1. Let the app fully load (inbox visible)
  2. Enable Offline mode
  3. Refresh the page
  4. Expected: The app should render the inbox with cached data, NOT show an infinite loading spinner
  5. Disable Offline mode
  6. Expected: The app reconnects and refreshes data normally

IS_LOADING_REPORT_DATA

  1. Let the app fully load

  2. Go offline and refresh

  3. Expected: App renders with cached data, no infinite loading state

  4. Enable airplane mode / disconnect from network

  5. Open the app (cold start)

  6. Confirm the splash screen disappears and the app loads (offline mode)

  7. If a deep link was used, confirm the sign-in page is shown (not a forever-loading splash)

  8. Disable airplane mode and confirm the app reconnects normally

PLAID_LINK_TOKEN

  1. Open DevTools > Network tab
  2. Start the Plaid flow
  3. Before the Plaid modal appears, enable Offline mode
  4. Expected: The spinner should eventually stop (not spin forever)
  5. Go back online
  6. Expected: You can retry the flow successfully

QA Steps

Same as Tests section above

  • Verify that no errors appear in the JS console

PR Author Checklist

  • I linked the correct issue in the ### Fixed Issues section above
  • I wrote clear testing steps that cover the changes made in this PR
    • I added steps for local testing in the Tests section
    • I added steps for the expected offline behavior in the Offline steps section
    • I added steps for Staging and/or Production testing in the QA steps section
    • I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
    • I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
    • I tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).
  • I included screenshots or videos for tests on all platforms
  • I ran the tests on all platforms & verified they passed on:
    • Android: Native
    • Android: mWeb Chrome
    • iOS: Native
    • iOS: mWeb Safari
    • MacOS: Chrome / Safari
  • I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
  • I followed proper code patterns (see Reviewing the code)
    • I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick)
    • I verified that comments were added to code that is not self explanatory
    • I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
    • I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
      • If any non-english text was added/modified, I used JaimeGPT to get English > Spanish translation. I then posted it in #expensify-open-source and it was approved by an internal Expensify engineer. Link to Slack message:
    • I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
    • I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
    • I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
    • I verified the JSDocs style guidelines (in STYLE.md) were followed
  • If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
  • I followed the guidelines as stated in the Review Guidelines
  • I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
  • I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
  • I verified any variables that can be defined as constants (ie. in CONST.ts or at the top of the file that uses the constant) are defined as such
  • I verified that if a function's arguments changed that all usages have also been updated correctly
  • If any new file was added I verified that:
    • The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
  • If a new CSS style is added I verified that:
    • A similar style doesn't already exist
    • The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))
  • If new assets were added or existing ones were modified, I verified that:
    • The assets are optimized and compressed (for SVG files, run npm run compress-svg)
    • The assets load correctly across all supported platforms.
  • If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
  • If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
  • If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
  • If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
  • If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:
    • I verified that all the inputs inside a form are aligned with each other.
    • I added Design label and/or tagged @Expensify/design so the design team can review the changes.
  • If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
  • I added unit tests for any new feature or bug fix in this PR to help automatically prevent regressions in this user flow.
  • If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.

Screenshots/Videos

Android: Native
android-compressed.webm
Android: mWeb Chrome
iOS: Native
ios-compressed.mp4
iOS: mWeb Safari
MacOS: Chrome / Safari
web-compressed.mov

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 10, 2026

Codecov Report

✅ Changes either increased or maintained existing code coverage, great job!

Files with missing lines Coverage Δ
src/CONST/index.ts 92.30% <ø> (ø)
src/Expensify.tsx 87.50% <100.00%> (+0.11%) ⬆️
src/ONYXKEYS.ts 100.00% <ø> (ø)
src/libs/ExportOnyxState/common.ts 79.87% <ø> (ø)
src/libs/actions/App.ts 46.76% <ø> (ø)
src/libs/actions/QueuedOnyxUpdates.ts 100.00% <ø> (ø)
...ctions/ReimbursementAccount/resetUSDBankAccount.ts 61.53% <ø> (ø)
src/libs/actions/Report/index.ts 69.24% <100.00%> (+0.11%) ⬆️
src/libs/actions/Welcome/index.ts 48.43% <100.00%> (+4.36%) ⬆️
src/pages/EnablePayments/EnablePayments.tsx 0.00% <ø> (ø)
... and 22 more
... and 8 files with indirect coverage changes

@JKobrynski JKobrynski marked this pull request as ready for review April 10, 2026 15:05
@JKobrynski JKobrynski requested review from a team as code owners April 10, 2026 15:05
@melvin-bot melvin-bot Bot requested a review from Krishna2323 April 10, 2026 15:05
@melvin-bot
Copy link
Copy Markdown

melvin-bot Bot commented Apr 10, 2026

@Krishna2323 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]

@melvin-bot melvin-bot Bot requested review from flaviadefaria and removed request for a team April 10, 2026 15:05
@mountiny
Copy link
Copy Markdown
Contributor

@Krishna2323 can you review please?

@Krishna2323
Copy link
Copy Markdown
Contributor

Reviewing...

@Krishna2323
Copy link
Copy Markdown
Contributor

Couldn’t finish testing yesterday—will take another pass shortly. This needs thorough testing since it changes keys that previously caused regressions.

@flaviadefaria
Copy link
Copy Markdown
Contributor

No new product considerations - removing my assignment and unsubscribing.

@flaviadefaria flaviadefaria removed their request for review April 13, 2026 08:08
@Krishna2323
Copy link
Copy Markdown
Contributor

Reviewing...

@Krishna2323
Copy link
Copy Markdown
Contributor

@fabioh8010 I tested all scenarios on iOS/Android (native + mWeb), and on desktop web this was the final one to verify—turns out it’s still not fixed 😭🤣

Monosnap.screencast.2026-04-14.01-16-53.mp4

@mountiny
Copy link
Copy Markdown
Contributor

How is this looking @fabioh8010

@JKobrynski
Copy link
Copy Markdown
Contributor Author

@mountiny @Krishna2323 I'm back so I will be helping Fabio with this or taking over from here! I'm on it

@JKobrynski
Copy link
Copy Markdown
Contributor Author

@Krishna2323 ok so when I initially checked this it was working for me, but then I realised the branch was updated while I was OOO. I pulled the latest changes and the flow is indeed failing. It looks like some of the latest changes must've caused this. I'm searching through them to find the cause.

@Krishna2323
Copy link
Copy Markdown
Contributor

@JKobrynski please also check: #88328 (comment)

@fabioh8010
Copy link
Copy Markdown
Contributor

@Krishna2323 Julian left the project, I will handle this PR from now on

@fabioh8010
Copy link
Copy Markdown
Contributor

fabioh8010 commented Apr 21, 2026

@Krishna2323

Addressed #88328 (comment) in 33d6902

Julian showed me that we were still missing migration of ISSUE_NEW_EXPENSIFY_CARD that was reverted here due to this issue #88106.

I transferred the changes to this PR in d998f91 and didn't find any issues.


  1. Go to the Expensify Card page
  2. Issue a virtual Expensify Card for yourself
  3. Click the Issue card button again to issue another card
  4. Issue card button should be responsive and allow issuing a second card
Screen.Recording.2026-04-21.at.15.16.11.mov

  1. Go to the Expensify Card page
  2. Start the Issue card flow and advance some steps
  3. Kill the app (or refresh browser) and open it again
  4. Assert user is back to first step
Screen.Recording.2026-04-21.at.15.35.42.mov

@fabioh8010
Copy link
Copy Markdown
Contributor

@Krishna2323 could you review again? Thanks!

@Krishna2323
Copy link
Copy Markdown
Contributor

thanks for the updates. Will review today.

@mountiny
Copy link
Copy Markdown
Contributor

@Krishna2323 how is it looking?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants