[Internal QA] Fix: Part 1: New Expensify Supportal access issues#72170
Conversation
|
@eh2077 please ignore this ping, this one would be internalQA |
|
Spellcheck failure seems unrelated to the changes in our PR Error: Unknown word (Incase)
src/components/PopoverProvider/index.tsx:46:16 Unknown word (Incase)
Files checked: 6249, Issues found: 1 in 1 files.
Error: 1 spelling issue found in 1 of the 6249 files checked.
Done. |
|
CI is green now 🥳 |
MariaHCD
left a comment
There was a problem hiding this comment.
The RCA and the code makes sense to me but I'm still seeing some buggy behavior with Supportal
- When Supportaling to ND, the ND login screen flashes briefly
- In ND, sometimes the page results in infinite loading (maybe because there is no reportID in the url?)
Restore stashed loginresulted in the workspaces of both the agent and the user being displayed
Screen.Recording.2025-10-09.at.2.02.47.PM.mov
|
Making adhoc as we can also test on it if you really fast with the url |
|
🚧 @mountiny 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, Desktop, and Web. Happy testing! 🧪🧪
|
This issue is part of bug 2, which i posted here, once the expected behaviour is learned, i can raise a PR to fix this
Maybe all these issues were because you were on |
|
Strange - I tested with Screen.Recording.2025-10-09.at.5.57.53.PM.mov |
|
Hmm very strange, I can run the whole flow pretty neatly: Screen.Recording.2025-10-10.at.11.43.34.AM.movIs there any specific setup you have which i am missing? Also just a wild guess, can you perform the flow without opening the console ? There are reactors on the FE which respond to changes of screen size, not 100% sure but just something that came to my mind |
|
Tried without opening up the console but still got some buggy behavior... Screen.Recording.2025-10-10.at.2.31.54.PM.movThe account I'm supportal-ing into has quite a bit of data (reports, workspaces, etc.) so maybe that is a factor here? I can test with another smaller account to test that theory. |
|
Hmm, no, seems like it doesn't really improve even if I supportal into a new account with little data: Screen.Recording.2025-10-10.at.2.38.53.PM.mov |
|
Tried testing with the adhoc builds but the transition doesn't work at all 🤔 Screen.Recording.2025-10-10.at.2.50.23.PM.mov |
|
On the adhoc you need to be really fast so the token is not used |
|
I have managed to do it on the adhoc only once, but its really tricky for the steps @allgandalf needs here so yeah definitely better to test locally this one. Screen.Recording.2025-10-10.at.15.42.57.mp4 |
|
Alright so I tested locally and seems to work well (I added throttling to 3G so its a bit easier to open the networks tab in time etc) Seems like the But from what I could see it worked fine for me 🙌 @allgandalf Screen.Recording.2025-10-10.at.16.38.35.mp4 |
mountiny
left a comment
There was a problem hiding this comment.
Works fine for me from my testing
| const splitNavigatorScreenOptions = useSplitNavigatorScreenOptions(); | ||
|
|
||
| // Determine if the current URL indicates a transition. | ||
| const isTransitioning = useMemo(() => { |
There was a problem hiding this comment.
I think this memo would be useless when we turn on the React Compiler so I wonder if we need it here. Probably good for now ✅
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppNA Android: mWeb ChromeNA iOS: HybridAppNA iOS: mWeb SafariNA MacOS: Chrome / SafariScreen.Recording.2025-10-10.at.16.38.35.mp4MacOS: DesktopNA |
MariaHCD
left a comment
There was a problem hiding this comment.
It might be that my local tests are something to do with my own setup considering that both you guys tested this successfully. The code looks good! Going to merge. Then we can test once this is on staging.
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/MariaHCD in version: 9.2.30-0 🚀
|
|
Tested on staging and it's working pretty well! 🎊 I do notice the login screen sometimes flashes when Support logging into the user and when restoring a stashed login but we can look into that as a follow up improvement: Screen.Recording.2025-10-14.at.9.34.22.AM.mov |
|
Nice job @allgandalf ! |
|
🚀 Deployed to production by https://github.com/mountiny in version: 9.2.30-6 🚀
|
Explanation of Change
Problem:
During an account transition (e.g., support login via
/transitionURL), anOpenReportAPI call for areportIDfrom the previous user's session was prematurely initiated. This occurred before the new user'sSignInWithSupportAuthTokenAPI request had completed, leading to display of stale data.Note that this used to only happen if the supportal agent had a workspace chat open before supportal into customer account
Root Cause:
The
ReportsSplitNavigatorcomponent, responsible for determining the initial report to display, would callReportUtils.findLastAccessedReport()during itsuseStateinitialization forinitialReportID. At this early stage of a/transition(before new user session data fully loads and overwrites stale Onyx data),findLastAccessedReport()would still retrieve a "last accessed report" from the previous session's cached Onyx data. This oldreportIDwas then passed toReportScreenviainitialParams, which in turn triggered anopenReportAPI call for this irrelevant report.Solution:
The fix prevents
ReportsSplitNavigatorfrom attempting to resolve aninitialReportIDfrom cached data when a transition is in progress and no explicit report ID is provided in the URL.ReportsSplitNavigator.tsx: AnisTransitioningcheck should be added. If the current URL is a/transitionand noreportIDis present in the URL path,ReportsSplitNavigatornow explicitly sets thereportIDpassed toReportScreento an empty string ('').Fixed Issues
$ https://github.com/Expensify/Expensify/issues/545504
PROPOSAL:
Tests
Offline tests
QA Steps
https://www.staging. expensify.com/_support/with the same supportal agent accountVerify that there is no
openReportcall beforeSignInWithSupportAuthTokenapi callVerify that you do not see any stale data of workspaces of the supportal agent who was previously logged in
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand 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.Screen.Recording.2025-10-09.at.12.41.58.PM.mov