-
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-11] [$250] mWeb/Chrome - Workspace Settings-Back arrow leads to the forward page instead of the back page #51107
Comments
Triggered auto assignment to @dylanexpensify ( |
Triggered auto assignment to @jliexpensify ( |
Hey @jliexpensify! I'm heading out on parental leave so reassigning this! TY! 🙇♂️ |
Edited by proposal-police: This proposal was edited at 2024-10-18 23:33:52 UTC. ProposalPlease re-state the problem that we are trying to solve in this issue.The back arrow leads to the forward page instead of the back page What is the root cause of that problem?
we call the goBack function with App/src/libs/Navigation/Navigation.ts Lines 234 to 237 in f7d08e0
What changes do you think we should make in order to solve the problem?
What alternative solutions did you explore? (Optional) |
Edited by proposal-police: This proposal was edited at 2024-10-20 05:49:59 UTC. ProposalPlease re-state the problem that we are trying to solve in this issue.The back arrow leads to the forward page instead of the back page What is the root cause of that problem?We pass fallback route to policy accounting page when when back button is pressed
Since we pass the What changes do you think we should make in order to solve the problem?Instead of redirecting to policy account page we can pass the backTo params since we also have this page for RHP Navigation.goBack(route.params?.backTo);
We can do the same approach in other pages like QBO, NetSuite, etc What alternative solutions did you explore? (Optional) |
Job added to Upwork: https://www.upwork.com/jobs/~021847870580076584100 |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @ikevin127 ( |
I'm on Android 51-1 (Pixel 7) and can inconsistently reproduce this. I'll set it as External in case @ikevin127 can consistently reproduce. |
@ikevin127 Can you reproduce the bug? |
Will attempt to reproduce and if I can will proceed with reviewing proposals, otherwise let you know. |
I've tried to reproduce the issue again with the latest main, and it's still reproducible Screen.Recording.2024-10-22.at.21.11.52.mov |
I was able to reproduce consistently even though not being able to connect Quickbooks Desktop, but the issue is present in Quickbooks Online connection as well: android-mweb-repro.webmGiven this, I Identified the following components where the same navigation go back with fallback logic is being used and by looking at previous PRs (like this one) I couldn't find a specific reason as to why the fallback was added. These are the components where the same logic is used:
The only ones which lead to our issue from what I tested are the ✅ ones and I'm not saying we should remove the fallback from all of them as that would be out of scope for our issue and also we would risk regressions. |
Triggered auto assignment to @mountiny, see https://stackoverflow.com/c/expensify/questions/7972 for more details. |
@NJ-2020 Thanks for your proposal. Unfortunately it came in second, the only difference being the replacement of the fallback route with |
📣 @ikevin127 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app! |
📣 @truph01 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app! Offer link |
@ikevin127 the purpose of FYI we have an issue with the same RCA as this one and proposing a general fix that will prevent similar issues as this 👍 |
@FitseTLT Unless you tested the selected solution in this specific issue's case and have found that it breaks something - I think we're good to go here. I looked over at your proposal's RCA and solution from the other issue and the solution is suggesting calling If CME (@mountiny) considers that we should hold for the other issue / proposed solution, I won't have anything against that - as this is not about who get's to push their solution first 👍 |
@ikevin127 I saw in this docs, connecting QBD requires Windows device but I don't have. Can you connect it? |
@truph01 Wasn't able to connect QBD for the same reason but we have the same issue on Quickbooks Online integration which you should be able to connect - you can show that in your videos for the PR Checklist and make sure to apply the fix for the Quickbooks Desktop integration. |
@jliexpensify, @mountiny, @ikevin127, @truph01 Whoops! This issue is 2 days overdue. Let's get this updated quick! |
Looks like PR was not opened yet. @truph01 Any updates ? |
I am working on the PR |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.56-9 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-11. 🎊 For reference, here are some details about the assignees on this issue:
|
@ikevin127 @jliexpensify 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:
Test 1Prerequisite: User has connected to QBD.
Test 2Prerequisite: User has connected to QBO.
Do we agree 👍 or 👎. |
Payment Summary
|
Paid and job closed! |
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: 9.0.50
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: N/A
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by:
Slack conversation:
Issue found when executing PR #50307
Action Performed:
Prerequisite: The user is logged in to the app
Expected Result:
The back arrow leads to the back page
Actual Result:
The back arrow leads to the forward page instead of the back page
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
Add any screenshot/video evidence
Bug6638967_1729272438158.50307.mp4
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @jliexpensifyThe text was updated successfully, but these errors were encountered: