-
Notifications
You must be signed in to change notification settings - Fork 3k
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-09-12] [$250] Workspace - Workspace profile page becomes grayed out when go offline #47260
Comments
Triggered auto assignment to @puneetlath ( |
@puneetlath FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors |
We think that this bug might be related to #vip-vsb |
ProposalPlease re-state the problem that we are trying to solve in this issue.Workspace - Workspace profile page becomes grayed out when go offline What is the root cause of that problem?Pending fields are not cleared in success data. App/src/libs/actions/Policy/Policy.ts Lines 1596 to 1602 in 5d57ae8
App/src/libs/actions/Policy/Policy.ts Lines 1673 to 1677 in 5d57ae8
What changes do you think we should make in order to solve the problem?Clear pending fields in success data for What alternative solutions did you explore? (Optional) |
ProposalPlease re-state the problem that we are trying to solve in this issue.Workspace profile page becomes grayed out when go offline What is the root cause of that problem?We add these pendingFields in optimistic data App/src/libs/actions/Policy/Policy.ts Lines 1600 to 1601 in 659011e
But we do not remove them in Success data App/src/libs/actions/Policy/Policy.ts Lines 1600 to 1601 in 659011e
What changes do you think we should make in order to solve the problem?Actually, there's no need to include these lines because we're not waiting for a backend response for them. The backend response here doesn’t return a description or general settings, so we can safely remove these two lines. App/src/libs/actions/Policy/Policy.ts Lines 1600 to 1601 in 659011e
What alternative solutions did you explore? (Optional) |
Job added to Upwork: https://www.upwork.com/jobs/~0182138e49511ea4ac |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @shubham1206agra ( |
Edited by proposal-police: This proposal was edited at 2024-08-14 11:16:16 UTC. ProposalPlease re-state the problem that we are trying to solve in this issue.Workspace profile page becomes grayed out when user goes offline What is the root cause of that problem?In this PR, we plan to just add And these pendingFields are not cleared in App/src/libs/actions/Policy/Policy.ts Lines 1673 to 1677 in 5d57ae8
What changes do you think we should make in order to solve the problem?
in: App/src/libs/actions/Policy/Policy.ts Line 2254 in 5d57ae8
and
in: App/src/libs/actions/Policy/Policy.ts Line 2321 in 5d57ae8
and then also clear these in What alternative solutions did you explore? (Optional)
then update:
Similar logic should applied to:
|
@shubham1206agra, just to note, the changes in this PR should be handled there only, or we can hold off until that PR is merged. It has nothing to do with this issue. The general solution will be the same, to add missing pending fields. The pending fields were intentionally added in this PR, so I think we shouldn't remove them. |
@puneetlath, @shubham1206agra Uh oh! This issue is overdue by 2 days. Don't forget to update your issues! |
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸 |
@puneetlath, @shubham1206agra 6 days overdue. This is scarier than being forced to listen to Vogon poetry! |
@shubham1206agra thoughts on the proposals? |
@puneetlath, @shubham1206agra Now this issue is 8 days overdue. Are you sure this should be a Daily? Feel free to change it! |
@Krishna2323's proposal looks good. 🎀👀🎀 C+ reviewed |
Current assignee @puneetlath is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new. |
@shubham1206agra Can you check my proposal? It will work in the latest code changes in main branch. |
I think @Krishna2323 proposal and #47260 (comment) is fine with me. |
With the changes in the selected proposal, the policy name is not grey out when creating policy in offline. Because now, we use
|
Btw, as mentioned in @Krishna2323 's comment:
So I think he can handle the details when creating PR. So that solution looks good. |
Waiting on assignment from @puneetlath |
If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results. If a regression has occurred and you are the assigned CM follow the instructions here. If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future. |
@shubham1206agra just a quick update: we no longer use App/src/libs/actions/Policy/Policy.ts Line 1632 in 9ddca5c
In the backend response, we don’t check for cc:@puneetlath |
@Nodebrute Thank you for the input. But your reasoning is wrong. We are using pattern B here. We should keep the description greyed out while creating the workspace. So, we will proceed with @Krishna2323's proposal as usual. And I will ask the deploy blocker to be marked as closed. |
@shubham1206agra Thanks for clearing that up 🙂 |
@puneetlath I think you wrote in the wrong issue |
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸 |
📣 @shubham1206agra 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app! |
📣 @Krishna2323 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app! Offer link |
@trjExpensify Can you add label for payment on 12th September? |
@puneetlath has actioned on that, I've added the |
I've paid @Krishna2323. @shubham1206agra can you please complete the checklist? |
BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:
|
Ok great. All paid. Thanks everyone! |
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.19-0
Reproducible in staging?: Y
Reproducible in production?: Y
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): [email protected]
Issue reported by: Applause - Internal Team
Action Performed:
Expected Result:
Workspace profile page should not be grayed out since the workspace was created in online mode
Actual Result:
Workspace profile page becomes grayed out when user goes offline
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
Add any screenshot/video evidence
Bug6570133_1723480443136.Recording__3695.mp4
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @shubham1206agraThe text was updated successfully, but these errors were encountered: