-
Notifications
You must be signed in to change notification settings - Fork 3.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[HOLD for payment 2024-11-13] [$250] Clicking " Please sign in again" does nothing in "Session Expired" page #51546
Comments
This has been labelled "Needs Reproduction". Follow the steps here: https://stackoverflowteams.com/c/expensify/questions/16989 |
Triggered auto assignment to @CortneyOfstad ( |
ProposalPlease re-state the problem that we are trying to solve in this issue.Clicking " Please sign in again" does nothing in "Session Expired" page What is the root cause of that problem?We should use goBack() function instead of navigate(), same like what we do here: App/src/pages/LogInWithShortLivedAuthTokenPage.tsx Lines 29 to 32 in 5103b4b
because it will remove the transition route from the history App/src/pages/ErrorPage/SessionExpiredPage.tsx Lines 35 to 39 in 5103b4b
Because if we use navigate, the transition history still persists which keeps showing the session expired page App/src/libs/Navigation/AppNavigator/PublicScreens.tsx Lines 24 to 33 in 5103b4b
What changes do you think we should make in order to solve the problem?We should use goBack instead of navigate onPress={() => {
if (!NativeModules.HybridAppModule) {
Session.clearSignInData();
Navigation.goBack();
return;
}
NativeModules.HybridAppModule.closeReactNativeApp(true, false);
}} OR onPress={() => {
if (!NativeModules.HybridAppModule) {
Session.clearSignInData();
Navigation.isNavigationReady().then(() => {
Navigation.goBack();
// OPTIONAL: Navigation.navigate(ROUTES.SIGN_IN_MODAL);
});
return;
}
NativeModules.HybridAppModule.closeReactNativeApp(true, false);
}} ResultScreen.Recording.2024-10-27.at.18.29.28.movWhat alternative solutions did you explore? (Optional) |
I am not able to recreate, so going to see if the C+ can recreate based on the SO linked above |
Job added to Upwork: https://www.upwork.com/jobs/~021851043610237148822 |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @aimane-chnaif ( |
@aimane-chnaif — can you attempt to recreate please 🙏 Thank you! |
I can still consistently reproduce this issue
Screen.Recording.2024-10-28.at.20.19.42.mov |
ProposalThe
This ensures that navigation only occurs once the navigation container is fully prepared and ready to handle navigation actions. |
📣 @farazahmad759! 📣
|
Contributor details |
✅ Contributor details stored successfully. Thank you for contributing to Expensify! |
@farazahmad759 please follow proposal template as stated in contributing guideline. |
Triggered auto assignment to @MariaHCD, see https://stackoverflow.com/c/expensify/questions/7972 for more details. |
@NJ-2020's proposal makes sense to me. I see that |
📣 @aimane-chnaif 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app! |
📣 @NJ-2020 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app! Offer link |
To make sure we get redirected to the correct page in case if we have another page before session expired page
Calling goBack() only still works fine and if we pass the route sign in modal as a fallback route to the goBack will push new full stack navigator |
I like the idea of explicitly calling But we can iron this out in the PR. cc: @aimane-chnaif |
PR ready cc: @aimane-chnaif |
Ohh, thank you @aimane-chnaif |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.57-10 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue: If no regressions arise, payment will be issued on 2024-11-13. 🎊 For reference, here are some details about the assignees on this issue:
|
@aimane-chnaif @CortneyOfstad The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button] |
BugZero Checklist:
Bug classificationSource of bug:
Where bug was reported:
Who reported the bug:
Regression Test ProposalPrecondition:N/A Test:
Do we agree 👍 or 👎 |
Thank you @aimane-chnaif! |
Payment Summary@NJ-2020 — paid $250 via Upwork Regression Test |
If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!
Version Number:
Reproducible in staging?: needs reproduction
Reproducible in production?: needs reproduction
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?:
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: @carlosmiceli
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1729880182560339
Action Performed:
Expected Result:
User is taken to sign in screen
Actual Result:
Nothing happens
Workaround:
unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
Add any screenshot/video evidence
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @CortneyOfstadThe text was updated successfully, but these errors were encountered: