-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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-18] [$250] Workspace -App redirects to workspace editor with error when returning from not here page #46646
Comments
Triggered auto assignment to @stephanieelliott ( |
We think this issue might be related to the #collect project. |
This seems like this may be a regression from #46183 -- posted there to have the PR author take a look. |
This issue is present before the PR. |
Thanks @bernhardoj -- we'll carry on and treat this as a standalone issue then. |
Job added to Upwork: https://www.upwork.com/jobs/~015e953d4b48a8b7a8 |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @thesahindia ( |
ProposalPlease re-state the problem that we are trying to solve in this issue.In Step 4, app returns to workspace editor with the error "Invalid Policy ID". What is the root cause of that problem?In App/src/libs/Navigation/AppNavigator/createCustomFullScreenNavigator/CustomFullScreenRouter.tsx Line 16 in 2c60fc9
WORKSPACE.INITIAL page into the route if it does not exist yet and we're accessing a workspace page.
So even when the workspace does not exist/is not accessible, we still do that, so when the user swipe from the not found more feature page, they will see the What changes do you think we should make in order to solve the problem?In App/src/libs/Navigation/AppNavigator/createCustomFullScreenNavigator/CustomFullScreenRouter.tsx Line 16 in 2c60fc9
WORKSPACE.INITIAL page into the route if the workspace is valid and accessible.
The condition for "workspace is valid and accessible" that is checked by So we can add to this App/src/libs/Navigation/AppNavigator/createCustomFullScreenNavigator/CustomFullScreenRouter.tsx Line 17 in 2c60fc9
What alternative solutions did you explore? (Optional)There could be a chance that when deeplinking, the above condition is evaluated before the policy data is loaded. To prevent false positive in that case we can add a check for
isLoadingReportData is false .
Then in
|
Taking over here on request here. https://expensify.slack.com/archives/C02NK2DQWUX/p1723190106955179 @dominictb Can you post a test branch here? |
@shubham1206agra Of course, the test branch is https://github.com/dominictb/epsf-app/tree/fix/46646-test Looking forward to your review 🙏 |
@dominictb Your proposal works, but it looks unstable, as you have mentioned already. Your alternative solution cannot be adopted as we should not use |
@stephanieelliott, @thesahindia Huh... This is 4 days overdue. Who can take care of this? |
📣 @shubham1206agra 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app! Offer link |
Hey @dominictb seems like you were investigating a few things, what's the latest on this one? |
@adamgrzybowski what's your opinion about my proposal? |
I'll provide update today. |
Could you please give the current state of the situation? Thanks! |
I fixed the issue and posted an update in the PR. |
PR is still under review #49226 |
Hey @dominictb last comment on the PR says that you're doing some testing -- can you give an update on where we are with that? |
I already fixed the C+ found issue on large screen but discovered several other issues in deep linking scenario. There's an unsynchronization of data between |
PR is under active review |
Hey @shubham1206agra the PR is blocked on you, can you review? |
friendly bump @shubham1206agra as the PR has been ready for review for a few days already. |
Sorry for the delay |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.59-3 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-18. 🎊 For reference, here are some details about the assignees on this issue:
|
@shubham1206agra / @dominictb @stephanieelliott 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] |
Summarizing payment on this issue:
Upwork job is here: https://www.upwork.com/jobs/~021859354157979620903 |
@stephanieelliott I think you need to pay me, not @lakchote (internal employee) |
Exactly, @stephanieelliott please pay @shubham1206agra as I'm an internal employee |
Oh my gosh @lakchote, I'm sorry -- I know you are an internal employee 🤦♀️. I extended this to the wrong person, @shubham1206agra offer was extended to you https://www.upwork.com/nx/wm/offer/104996565 |
All paid up! |
If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!
Version Number: 9.0.15-4
Reproducible in staging?: Y
Reproducible in production?: Y
Found when executing PR : #46479
Logs: https://stackoverflow.com/c/expensify/questions/4856
Issue reported by: Applause-Internal team
Action Performed:
in any chat.
Expected Result:
In Step 4, app will not return to workspace editor as the workspace link is invalid.
Actual Result:
In Step 4, app returns to workspace editor with the error "Invalid Policy ID".
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
Bug6558699_1722468815726.ScreenRecording_08-01-2024_07-24-08_1.mp4
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @stephanieelliottThe text was updated successfully, but these errors were encountered: