[HOLD 296932][$1000] Task - Mark as Done button is unavailable for the Policy admin user #23407

lanitochka17 · 2023-07-22T19:41:36Z

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

Issue found when executing PR #22851

Action Performed:

Open Expensify as User A.
Go to any room.
Click on 'Assign Task,' fill in the task details, and share it in the room.
Open Expensify as Policy admin user , who is not the assignee, assignor
Try to use Mark as Done button

Expected Result:

Policy admin user should be able to change Task's title, description and mark as done

Actual Result:

The Policy admin user can' use Mark as done button (the button is greyed out) while it's possible to change Task's fields and set Checkbox

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

Version Number: 1.3.44.0

Reproducible in staging?: Yes

Reproducible in production?: Yes

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

Bug6137801_Screen_Recording_2023-07-22_at_18.33.40.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~01efe1be768b7b85bc
Upwork Job ID: 1683545187614375936
Last Price Increase: 2023-07-31

melvin-bot · 2023-07-22T19:41:41Z

Triggered auto assignment to @kevinksullivan (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

melvin-bot · 2023-07-22T19:41:43Z

Bug0 Triage Checklist (Main S/O)

This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
This bug is not a duplicate report (check E/App issues and #expensify-bugs)
- If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
This bug is reproducible using the reproduction steps in the OP. S/O
- If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
- If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
This issue is filled out as thoroughly and clearly as possible
- Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

samh-nl · 2023-07-22T19:55:33Z

Proposal

Please re-state the problem that we are trying to solve in this issue.

The mark as done button is unavailable for the policy admin.

What is the root cause of that problem?

In the TaskHeaderActionButton component, the disabled prop of the Mark as done button does not check the workspace permissions.

App/src/components/TaskHeaderActionButton.js

Line 49 in b7d17e0

    
           isDisabled={ReportUtils.isCanceledTaskReport(props.report) || !Task.isTaskAssigneeOrTaskOwner(props.report, props.session.accountID)}

This problem extends to usages elsewhere of Task.isTaskAssigneeOrTaskOwner, except for TaskView.js where the correct permissions are checked:

App/src/components/ReportActionItem/TaskView.js

Line 54 in cd851d0

    
           const canEdit = PolicyUtils.isPolicyAdmin(policy) || Task.isTaskAssigneeOrTaskOwner(props.report, props.currentUserPersonalDetails.accountID);

What changes do you think we should make in order to solve the problem?

Add a condition to the disabled prop to see if the user is the policy admin (isPolicyAdmin) and enable the Mark as done button if either this is the case or the user is the task owner/assignee (provided the task hasn't been canceled).

For convenience, we could define a new util function that encompasses both checks, which we can apply here and elsewhere.

What alternative solutions did you explore? (Optional)

N/A

KrAbhas · 2023-07-22T20:00:04Z

Proposal

Please re-state the problem that we are trying to solve in this issue.

Mark as Done button is unavailable for the Policy admin user

What is the root cause of that problem?

In TaskHeaderActionButton.js only Task.isTaskAssigneeOrTaskOwner(props.report, props.session.accountID) is being checked for enabling the button.

App/src/components/TaskHeaderActionButton.js

Lines 47 to 50 in b7d17e0

    
           <Button 
        
               success 
        
               isDisabled={ReportUtils.isCanceledTaskReport(props.report) || !Task.isTaskAssigneeOrTaskOwner(props.report, props.session.accountID)} 
        
               medium

What changes do you think we should make in order to solve the problem?

We should add a check for isPolicyAdmin as well for keeping the button enabled. For this we will use policyId and check if the current user isPolicyAdmin from policyUtils.js then do not disable the button.

App/src/libs/PolicyUtils.js

Line 119 in cd851d0

    
           const isPolicyAdmin = (policy) => lodashGet(policy, 'role') === CONST.POLICY.ROLE.ADMIN;

Finally

admin_mark_as_done_enable.mov

What alternative solutions did you explore? (Optional)

NA

BhuvaneshPatil · 2023-07-22T22:08:12Z

Proposal

Please re-state the problem that we are trying to solve in this issue.

Task - Mark as Done button is unavailable for the Policy admin user

What is the root cause of that problem?

We have added the check for disabling the Button in TaskHeaderActionButton

App/src/components/TaskHeaderActionButton.js

Line 49 in cd851d0

    
           isDisabled={ReportUtils.isCanceledTaskReport(props.report) || !Task.isTaskAssigneeOrTaskOwner(props.report, props.session.accountID)}

It restricts the button only enabled to task assignee and task creator, that's why

What changes do you think we should make in order to solve the problem?

We shall modify the check to disable button, we shall add another condition - enable if current user is policy admin or not.
Similar to what we have done here -

App/src/components/ReportActionItem/TaskView.js

Line 54 in cd851d0

    
           const canEdit = PolicyUtils.isPolicyAdmin(policy) || Task.isTaskAssigneeOrTaskOwner(props.report, props.currentUserPersonalDetails.accountID);

the final condition here is -

isDisabled={ReportUtils.isCanceledTaskReport(props.report) || !(PolicyUtils.isPolicyAdmin(policy) || Task.isTaskAssigneeOrTaskOwner(props.report, props.currentUserPersonalDetails.accountID))}

While the above two proposals mention the same root cause, and solution partially. Because there is no method isPolicyAdminUser. We have used above logic (in TaskView) to enable editing(perform actions) the task.

What alternative solutions did you explore? (Optional)

This bug involves solving for Mark As Done button, but if we want to improve at other places, we shall modify the isTaskAssigneeOrTaskOwner function. In modified version we will return the true, if current user is policy admin.
We can rename the function to canCurrentUserEditTask that will return true if user is - creator || assignee || policyAdmin
else false. and use it in place of isTaskAssigneeOrTaskOwner in following components -

TaskHeaderActionButton
HeaderView
TaskView

dukenv0307 · 2023-07-23T07:04:18Z

Proposal

Please re-state the problem that we are trying to solve in this issue.

Task - Mark as Done button is unavailable for the Policy admin user

What is the root cause of that problem?

This is the current condition to disable the Mark as done Button

App/src/components/TaskHeaderActionButton.js

Line 49 in cd851d0

    
           isDisabled={ReportUtils.isCanceledTaskReport(props.report) || !Task.isTaskAssigneeOrTaskOwner(props.report, props.session.accountID)}

See that we don't have logic to check if the current user is the policy admin

Similar bug with same Root cause

Before coming to my solution, I see that we have some similar bugs with the same RCA that we don't check if the current user is policy admin or not

In the above image, the three-dot in the header doesn't appear like this

The root cause is we don't check if the current user is admin or not in here

App/src/pages/home/HeaderView.js

Line 104 in cd851d0

if (ReportUtils.isOpenTaskReport(props.report) && isTaskAssigneeOrTaskOwner) {

App/src/pages/home/HeaderView.js

Line 113 in cd851d0

    
           if (ReportUtils.isCompletedTaskReport(props.report) && isTaskAssigneeOrTaskOwner) {

App/src/pages/home/HeaderView.js

Line 122 in cd851d0

    
           if (props.report.stateNum !== CONST.REPORT.STATE_NUM.SUBMITTED && props.report.statusNum !== CONST.REPORT.STATUS.CLOSED && isTaskAssigneeOrTaskOwner) {

What changes do you think we should make in order to solve the problem?

To fix all cases, we should update the name of isTaskAssigneeOrTaskOwner function to isTaskAssigneeOrTaskOwnerOrPolicyAdmin, and add logic to check if the current user is policy admin

function isTaskAssigneeOrTaskOwnerOrPolicyAdmin(taskReport, sessionAccountID) {
    const policy = ReportUtils.getPolicy(taskReport.policyID);
    return sessionAccountID === getTaskOwnerAccountID(taskReport) || sessionAccountID === getTaskAssigneeAccountID(taskReport) || PolicyUtils.isPolicyAdmin(policy) 
}

Note that: In all places we use isTaskAssigneeOrTaskOwner we also need to check if the current user is policy admin

What alternative solutions did you explore? (Optional)

We add the condition to check if the current user is policy admin to following places:

App/src/pages/home/HeaderView.js

Line 104 in cd851d0

if (ReportUtils.isOpenTaskReport(props.report) && isTaskAssigneeOrTaskOwner) {

App/src/pages/home/HeaderView.js

Line 113 in cd851d0

    
           if (ReportUtils.isCompletedTaskReport(props.report) && isTaskAssigneeOrTaskOwner) {

App/src/pages/home/HeaderView.js

Line 122 in cd851d0

    
           if (props.report.stateNum !== CONST.REPORT.STATE_NUM.SUBMITTED && props.report.statusNum !== CONST.REPORT.STATUS.CLOSED && isTaskAssigneeOrTaskOwner) {

App/src/components/TaskHeaderActionButton.js

Line 49 in cd851d0

    
           isDisabled={ReportUtils.isCanceledTaskReport(props.report) || !Task.isTaskAssigneeOrTaskOwner(props.report, props.session.accountID)}

Result

Screen.Recording.2023-07-23.at.13.59.17.mov

melvin-bot · 2023-07-24T18:30:28Z

Job added to Upwork: https://www.upwork.com/jobs/~01efe1be768b7b85bc

melvin-bot · 2023-07-24T18:30:33Z

Current assignee @kevinksullivan is eligible for the External assigner, not assigning anyone new.

melvin-bot · 2023-07-24T18:30:39Z

Triggered auto assignment to Contributor-plus team member for initial proposal review - @parasharrajat (External)

parasharrajat · 2023-07-24T19:46:11Z

@lanitochka17 Is this is a regression or new request? Is it expected in the App that Admin can change tasks created by others?

Also, was this happening previously before #22851?

BhuvaneshPatil · 2023-07-24T19:49:54Z

@parasharrajat
#21579 (comment)

please have a look at this comment. I have raised a similar issue before. And this seems like a bug because policy admin should be able to edit the task.

melvin-bot · 2023-07-27T20:10:11Z

@kevinksullivan, @parasharrajat Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

parasharrajat · 2023-07-27T20:27:06Z

Tracking the issue above #23407 (comment).

kevinksullivan · 2023-07-28T23:02:51Z

@parasharrajat meaning we should put this on hold for now?

dukenv0307 · 2023-07-29T07:46:30Z

We don't need to put this on hold. This is a bug as mentioned here: #21579 (comment) and we need to fix it

parasharrajat · 2023-07-29T12:57:55Z

Looks like we are making some changes on the backend to change task permissions Expensify/expensify/issues/296932. I don't have access to it so can't tell for sure. IMO, we should be holding this issue on the internal one.

dukenv0307 · 2023-07-29T13:36:28Z

@thienlnam Ping you here because this comment. Could you help to make a decision about what we should do about this issue?

parasharrajat · 2023-07-30T12:07:48Z

Sure.

parasharrajat · 2023-07-30T12:09:27Z

@thienlnam You mentioned solving task permission on the backend #21579 (comment). Can you help us determine if we should be holding this issue or not?

melvin-bot · 2023-07-31T16:00:25Z

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

thienlnam · 2023-07-31T16:38:57Z

Yes let's please hold this issue - I have yet to make the corresponding App changes once I'm done with the back-end changes

melvin-bot · 2023-07-31T16:39:43Z

Current assignee @parasharrajat is eligible for the Internal assigner, not assigning anyone new.

thienlnam · 2023-07-31T16:39:55Z

Will be taken care of internally in -> https://github.com/Expensify/Expensify/issues/296932

kevinksullivan · 2023-08-01T19:22:00Z

In other words we should close @thienlnam ?

thienlnam · 2023-08-01T19:23:31Z

Yeah we can close this, basically a dupe

kevinksullivan · 2023-08-01T19:54:33Z

sounds good

lanitochka17 added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Jul 22, 2023

melvin-bot bot assigned kevinksullivan Jul 22, 2023

mvtglobally mentioned this issue Jul 24, 2023

Deploy Checklist: New Expensify 2023-07-21 #23362

Closed

marcaaron added the External Added to denote the issue can be worked on by a contributor label Jul 24, 2023

melvin-bot bot changed the title ~~Task - Mark as Done button is unavailable for the Policy admin user~~ [$1000] Task - Mark as Done button is unavailable for the Policy admin user Jul 24, 2023

melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Jul 24, 2023

melvin-bot bot assigned parasharrajat Jul 24, 2023

melvin-bot bot added the Overdue label Jul 26, 2023

melvin-bot bot removed the Overdue label Jul 27, 2023

thienlnam changed the title ~~[$1000] Task - Mark as Done button is unavailable for the Policy admin user~~ [HOLD 296932][$1000] Task - Mark as Done button is unavailable for the Policy admin user Jul 31, 2023

thienlnam self-assigned this Jul 31, 2023

thienlnam added Internal Requires API changes or must be handled by Expensify staff and removed External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors labels Jul 31, 2023

kevinksullivan closed this as completed Aug 1, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[HOLD 296932][$1000] Task - Mark as Done button is unavailable for the Policy admin user #23407

[HOLD 296932][$1000] Task - Mark as Done button is unavailable for the Policy admin user #23407

lanitochka17 commented Jul 22, 2023 •

edited by melvin-bot bot

Loading

melvin-bot bot commented Jul 22, 2023

melvin-bot bot commented Jul 22, 2023

samh-nl commented Jul 22, 2023 •

edited

Loading

KrAbhas commented Jul 22, 2023 •

edited

Loading

BhuvaneshPatil commented Jul 22, 2023 •

edited

Loading

dukenv0307 commented Jul 23, 2023

melvin-bot bot commented Jul 24, 2023

melvin-bot bot commented Jul 24, 2023

melvin-bot bot commented Jul 24, 2023

parasharrajat commented Jul 24, 2023 •

edited

Loading

BhuvaneshPatil commented Jul 24, 2023 •

edited

Loading

melvin-bot bot commented Jul 27, 2023

parasharrajat commented Jul 27, 2023

kevinksullivan commented Jul 28, 2023

dukenv0307 commented Jul 29, 2023

parasharrajat commented Jul 29, 2023

dukenv0307 commented Jul 29, 2023

parasharrajat commented Jul 30, 2023

parasharrajat commented Jul 30, 2023

melvin-bot bot commented Jul 31, 2023

thienlnam commented Jul 31, 2023

melvin-bot bot commented Jul 31, 2023

thienlnam commented Jul 31, 2023

kevinksullivan commented Aug 1, 2023

thienlnam commented Aug 1, 2023

kevinksullivan commented Aug 1, 2023

[HOLD 296932][$1000] Task - Mark as Done button is unavailable for the Policy admin user #23407

[HOLD 296932][$1000] Task - Mark as Done button is unavailable for the Policy admin user #23407

Comments

lanitochka17 commented Jul 22, 2023 • edited by melvin-bot bot Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

melvin-bot bot commented Jul 22, 2023

melvin-bot bot commented Jul 22, 2023

Bug0 Triage Checklist (Main S/O)

samh-nl commented Jul 22, 2023 • edited Loading

Proposal

Please re-state the problem that we are trying to solve in this issue.

What is the root cause of that problem?

What changes do you think we should make in order to solve the problem?

What alternative solutions did you explore? (Optional)

KrAbhas commented Jul 22, 2023 • edited Loading

Proposal

Please re-state the problem that we are trying to solve in this issue.

What is the root cause of that problem?

What changes do you think we should make in order to solve the problem?

What alternative solutions did you explore? (Optional)

BhuvaneshPatil commented Jul 22, 2023 • edited Loading

Proposal

Please re-state the problem that we are trying to solve in this issue.

What is the root cause of that problem?

What changes do you think we should make in order to solve the problem?

What alternative solutions did you explore? (Optional)

dukenv0307 commented Jul 23, 2023

Proposal

Please re-state the problem that we are trying to solve in this issue.

What is the root cause of that problem?

Similar bug with same Root cause

What changes do you think we should make in order to solve the problem?

What alternative solutions did you explore? (Optional)

Result

melvin-bot bot commented Jul 24, 2023

melvin-bot bot commented Jul 24, 2023

melvin-bot bot commented Jul 24, 2023

parasharrajat commented Jul 24, 2023 • edited Loading

BhuvaneshPatil commented Jul 24, 2023 • edited Loading

melvin-bot bot commented Jul 27, 2023

parasharrajat commented Jul 27, 2023

kevinksullivan commented Jul 28, 2023

dukenv0307 commented Jul 29, 2023

parasharrajat commented Jul 29, 2023

dukenv0307 commented Jul 29, 2023

parasharrajat commented Jul 30, 2023

parasharrajat commented Jul 30, 2023

melvin-bot bot commented Jul 31, 2023

thienlnam commented Jul 31, 2023

melvin-bot bot commented Jul 31, 2023

thienlnam commented Jul 31, 2023

kevinksullivan commented Aug 1, 2023

thienlnam commented Aug 1, 2023

kevinksullivan commented Aug 1, 2023

lanitochka17 commented Jul 22, 2023 •

edited by melvin-bot bot

Loading

samh-nl commented Jul 22, 2023 •

edited

Loading

KrAbhas commented Jul 22, 2023 •

edited

Loading

BhuvaneshPatil commented Jul 22, 2023 •

edited

Loading

parasharrajat commented Jul 24, 2023 •

edited

Loading

BhuvaneshPatil commented Jul 24, 2023 •

edited

Loading