Stash session and credentials during copilot access#52980
Conversation
|
Okay, this looks like it works properly with https://github.com/Expensify/Mobile-Expensify/pull/13289 and the web-e / backend changes! |
mollfpr
left a comment
There was a problem hiding this comment.
The changes seem straightforward, but I can't test them since it need internal access. So I'm gonna approve it to pull someone internal.
|
@dangrous Sure! I'll let you know when I'm finish the test with current BE. |
|
How are we looking on this one @mollfpr ? Thanks! |
|
I'm going to test it now! Sorry for the delay 🙏 |
|
@dangrous Looks good! Although, I got logout when switch back to copilot account but I only experience it once. Screen.Recording.2024-12-05.at.23.18.41.mp4Do you want to proceed with test the BE changes? |
|
Hm, weird - I wonder if that's just something with the site issues we're having right now. I guess we can move forward, and if you notice that come up again let me know! I'll get my backend prepped with the necessary changes and ping you in Slack with an |
|
@dangrous I got an Onyx error after signing in. The |
|
I'm able to modify the onyx so the error won't be raised. Continue to testing! |
Reviewer Checklist
Screenshots/VideosAndroid: Native52980.Android2.mp4Android: mWeb Chrome52980.mWeb-Chrome2.mp4iOS: Native52980.iOS2.mp4iOS: mWeb Safari52980.mWeb-Safari2.mp4MacOS: Chrome / Safari52980.Web2.mp4MacOS: Desktop52980.Desktop2.mp4 |
|
@Beamanator 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] |
|
Thanks @mollfpr! To confirm about the set/merge thing - is that something I need to look into changing in my back end PR? Or is it resolved? |
|
Cool, I've merged main in both front and back end. Do you think we need to retest, or are we good for final approval from @blimpich (or @Beamanator who seems to have been auto-added unnecessarily)? I misread your initial message too, the error is on sign in not related code. |
|
@dangrous I think it's good for final approval since it's not related to the issue changes. |
|
ah okay perfect - @blimpich can you give it the final run down? Thanks! |
|
✋ 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/blimpich in version: 9.0.74-0 🚀
|
|
🚀 Deployed to production by https://github.com/luacmartins in version: 9.0.74-8 🚀
|


Explanation of Change
This will allow us to remove the code removed in https://github.com/Expensify/Web-Expensify/pull/44547, but should not yet require that change. However, please test with and without that backend change locally (I did too but just to be safe)
Fixed Issues
$ #51042
PROPOSAL:
Tests
Internally - please do the following
Update your authtokens to expire after a short period (e.g. 20 seconds) here - I don't remember which one of these is the one that matters so you can update both to be safe
As noted above please double check this works with and without https://github.com/Expensify/Web-Expensify/pull/44547.
This can also be tested on Hybrid App with https://github.com/Expensify/Mobile-Expensify/pull/13289 - you don't need to do that here I don't think, but calling out that I did do that :)
Create an account, verify it (log out and back in again) and add a copilot
Log in as that copilot
Copilot into the original account
Verify that everything seems to work fine (requests go through, etc.)
Switch back to the copilot's original account
Verify that everything seems to work fine (requests go through, etc.)
Wait 20 seconds (or however long you set your authtokens to expire)
Do some more actions, and check for the Authenticate call to go through successfully.
Verify that everything seems to work fine
Repeat steps 3-9 (just to make sure that nothing goes weird with the copilot tokens).
Offline tests
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))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
2024-11-25_14-56-20.mp4
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop