-
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 2023-07-21] [$1000] Message “Complete task…” should not be displayed when the user is marked as done task not assigned #20542
Comments
Triggered auto assignment to @puneetlath ( |
Bug0 Triage Checklist (Main S/O)
|
ProposalPlease re-state the problem that we are trying to solve in this issue.The task that has no assignee can be marked as done by other user What is the root cause of that problem?There are three places from where a task can be marked as completed
At all these places we are not adding any check if the button can be clicked by particular user or not. What changes do you think we should make in order to solve the problem?We can add the check to confirm if the user has right access to perform certain action. As mentioned in this comment #20542 (comment)
We need to decide on the message that we need to show to user. To handle the
What alternative solutions did you explore? (Optional)None |
Triggered auto assignment to @NicMendonca ( |
This comment was marked as duplicate.
This comment was marked as duplicate.
@NicMendonca re-assigning over to you since i'm going OOO for the next week. Thanks! |
ProposalPlease re-state the problem that we are trying to solve in this issue.The message “completed task” and “Re open task” does not appear in task detail when ticking the checkbox or press “Mark as done”, “Mark as incomplete” on task not assinged What is the root cause of that problem?Now, Only the task creator and assignee can complete and repoen the task. After calling completeTask and reopenTask, reportAction is saved to Onyx with optimisticData, But after API return permission error, failureData has not deleted reportAction saved above. Lines 204 to 208 in 4cad248
Lines 260 to 264 in 4cad248
What changes do you think we should make in order to solve the problem?To solve this problem we will set reportAction = null if the API returns an error.
In reopenTask:
Result: Screen.Recording.2023-06-10.at.05.08.13.movWhat alternative solutions did you explore? (Optional)Disable button, checkbox where these 2 APIs are called by checking if current user is owner or assigned |
ProposalPlease re-state the problem that we are trying to solve in this issue.The message "Complete task..." should not be displayed when the user is marked as done task that the user is not owner or assignee What is the root cause of that problem?When the user completes the task, Line 207 in 035783a
This issue also happens when the user re-opens the task Line 263 in 035783a
What changes do you think we should make in order to solve the problem?When the user completes or re-opens the task, after API return error
What alternative solutions did you explore? (Optional)We can disable checkbox, mask as done button and re-open task button for the user that is not owner or assignee. But I think all users can click on them and we will display the error with the user doesn't have permission is good because that help the users learn more about the usage of task and also understand why they cannot complete or re-open the task. Result for main solutionScreencast.from.12-06-2023.10.09.49.webm |
Job added to Upwork: https://www.upwork.com/jobs/~019928155ae5bbd4b8 |
Current assignee @NicMendonca is eligible for the External assigner, not assigning anyone new. |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @mollfpr ( |
Triggered auto assignment to @alex-mechler ( |
Thanks for the proposals! We will hide the Mark as done button in this issue #19631 (comment) So I'd like a solution for handling the failure action in this issue. @alex-mechler I'm curious if we will have another error from the API other than the permission error on completing task? |
@mollfpr Can you take a look at my proposal #20542 (comment). it handles the failureData for action to display the error. |
@dukenv0307 Is the error from API is only about permission? |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.40-5 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 2023-07-21. 🎊 After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.
As a reminder, here are the bonuses/penalties that should be applied for any External issue:
|
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:
|
No offending PR. This is an improvement on the task feature.
The regression step should be enough.
|
@namhihi237 can you accept the offer please? |
@dukenv0307 you've been paid |
@mollfpr just to confirm you'll be requesting payment via Expensify? |
@NicMendonca Nope, still using same old Upwork. |
I just accepted @NicMendonca |
everyone has been paid ✅ BZ checklist done ✅ Thanks! |
@NicMendonca I think this issue is eligible for a bonus timeline. Because
|
@dukenv0307 I am not reading this as within 3 business days:
|
|
I'm sorry - I am still not reading this as the PR being merged within 3 business days. If you were assigned on the 30th on June and requested internal eng on July 6th, then that is 5 business days |
discussing internally here |
@NicMendonca It seems you miss this point |
@dukenv0307 I am not counting the weekend. I am counting as followed:
^ This is 4 business days. |
My timezone (GMT + 7) I was assigned on July 1 After the weekend, I created the PR on July 3 And I added this comment #20542 (comment) to wait for internal team to add waiting for copy label to assign a member that can help us confirm the translation key. So from 3/6 to 6/6 the PR was held for this and I think it shouldn't be calculated to work day. After the confirmation C+ approve the PR on July 6 Before the CME of this issue back from OOO, we just need to merge main to resolve the conflict. Correct me If I missed something. |
@dukenv0307 you're not missing something, but there is a lot of grey areas here. So to that end, @mollfpr @dukenv0307 speed bonus has been paid via Upwork.
|
If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!
Action Performed:
Expected Result:
The message complete task shouldn’t show
Actual Result::
The message complete task show
Workaround:
Can the user still use Expensify without this being fixed? Have you informed them of the workaround?
Platforms:
Which of our officially supported platforms is this issue occurring on?
Version Number: 1.3.26-1
Reproducible in staging?: y
Reproducible in production?: y
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
Notes/Photos/Videos: Any additional supporting documentation
Screen.Recording.2023-06-08.at.16.20.23.mov
Recording.931.mp4
Expensify/Expensify Issue URL:
Issue reported by: @namhihi237
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1685993057669719
View all open jobs on GitHub
Upwork Automation - Do Not Edit
The text was updated successfully, but these errors were encountered: