[HOLD for payment 2024-04-05] [$500] Task - Anonymous user can delete task in public room

[lanitochka17]

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: 1.4.49-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
Issue reported by: Applause - Internal Team

Action Performed:

1. Log out if logged in
2. Go to a public room with task. (https://staging.new.expensify.com/r/2376199970894587
   )
3. Click on the task
4. Click three-dot menu > Delete task
5. Delete the task

Expected Result:

User will be presented with login modal when attempting to delete task anonymous

Actual Result:

Anonymous user can delete the task. The task is not deleted, but there is no action from preventing user to delete it

Workaround:

Unknown

Platforms:

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

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6407175_1709916697304.20240309_004424.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~019d7666768e954e45
• Upwork Job ID: 1767210887617179648
• Last Price Increase: 2024-03-11

[melvin-bot]

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

[lanitochka17]

@zanyrenney 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

[lanitochka17]

We think that this bug might be related to #vip-vsp
CC @quinthar

[allgandalf]

Proposal

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

Anonymous user can delete task in public room

What is the root cause of that problem?

We call the delete task action whatever be the return value of checkIfActionIsAllowed is:

App/src/pages/home/HeaderView.js

Line 377 in f9ea69b

Session.checkIfActionIsAllowed(Task.deleteTask(props.report));

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

We need to update the call to:

Session.checkIfActionIsAllowed(() => Task.deleteTask(props.report));

Note: We also need to do the same update to task marked as completed, because in anonymous rooms, we see don't see the delete option if the task has been marked completed, so updating both is required

App/src/pages/home/HeaderView.js

Lines 142 to 143 in f9ea69b

	// Task is marked as completed
	if (ReportUtils.isCompletedTaskReport(props.report) && canModifyTask) {

Result

simplescreenrecorder-2024-03-09_02.55.08.mp4

What alternative solutions did you explore? (Optional)

I guess our actual intent should be to not display the delete option for Anonymous Users, this will be in sync with what we do with employee's of collect policy, we do not show them the delete task option with canModifyTask:

App/src/pages/home/HeaderView.js

Lines 152 to 153 in f9ea69b

	if (props.report.stateNum !== CONST.REPORT.STATE_NUM.APPROVED && props.report.statusNum !== CONST.REPORT.STATUS_NUM.CLOSED && canModifyTask) {
	threeDotMenuItems.push({

For this we need to update the canModifyTask:

App/src/pages/home/HeaderView.js

Line 140 in f9ea69b

const canModifyTask = Task.canModifyTask(props.report, props.session.accountID);

Update it to:

 const canModifyTask = Task.canModifyTask(props.report, props.session.accountID) && !Session.isAnonymousUser(); 

Also if the task was market as completed by the admin then when we access deeplink anonymously then we see Mark as incomplete option.

So updating the canModifyTask with anonymous check seems to be a good option as well.

Also can we check if the bounty for this one be increased? Cause now we fix 2 bugs within a single issue ;)

[neonbhai]

Proposal

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

Anonymous user can delete task in public room

What is the root cause of that problem?

We don't check if it is an anonymous user when showing the 3dot menuitem

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

We should add the isAnonymousUser check to canModifyTask() function so it returns false and the task is not added to the menu:

if(Session.isAnonymousUser()) {
  return false;
}

• We can have the check in HeaderView here:
  

  App/src/pages/home/HeaderView.js

  Line 152 in f9ea69b

  if (props.report.stateNum !== CONST.REPORT.STATE_NUM.APPROVED && props.report.statusNum !== CONST.REPORT.STATUS_NUM.CLOSED && canModifyTask) {

  ...  && !Session.isAnonymousUser()

Alternatively

If we want to allow the anonymous user to see the Delete Task menuitem, we will have to:

• pass Task.deleteTask in an anonymous function
• change checkIfActionAllowed to call itself

We do this since checkIfActionIsAllowed always returns a function. We will need to call it:

Session.checkIfActionIsAllowed(() => Task.deleteTask(props.report))();

Result

This works perfectly:

Screen.Recording.2024-03-08.at.11.58.26.PM.mov

If we don't want the self call, we can use onModalHide to setIsDeleteTaskConfirmModalVisible(false);

• Then onConfirm will be simply:

  onConfirm{Session.checkIfActionIsAllowed(() => Task.deleteTask(props.report))}

[allgandalf]

Updated Proposal

No change in proposal at all, added result video only, can check edit history too ;)

[nkdengineer]

Proposal

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

• Task - Anonymous user can delete task in public room

What is the root cause of that problem?

• In here, we allow user to click on "Delete" button without checking if user is anonymous user or not.

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

• Just need to wrap the () => setIsDeleteTaskConfirmModalVisible(true) in here by the Session.checkIfActionIsAllowed as we did with other buttons
• Then we can remove the checkIfActionIsAllowed in here

What alternative solutions did you explore? (Optional)

• NA

Result

Screen.Recording.2024-03-11.at.19.12.04.mov

[zanyrenney]

Adding the external label to get this resolved!

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~019d7666768e954e45

[melvin-bot]

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

[nkdengineer]

Anyone in the design team can help confirm that, when should we display the sign in modal? When user clicks on the "Delete" option as we did with other buttons, or when user clicks on in "Confirm" button?

[allgandalf]

Updated Proposal

Did more research and found out that when the admin marks the task as completed then the delete button is removed and we get a mark as incomplete option instead of delete.

Updated the proposal accordingly to cover this case and have added an alternative approach to avoid repeated code :)

Also can we check if the bounty for this one be increased? Cause now we fix 2 bugs within a single issue ;)

Let me know what you think @zanyrenney @c3024

[c3024]

I think it is logical to do the check before displaying the confirmation modal.

This is similar to the description and assignee fields. When we click on the description we check directly if the user is anonymous and show Sign-in page in the RHP (instead of showing description page and checking if the user is anonymous after clicking Save button on the description page).

So, the proposal here by @nkdengineer looks good to me.

🎀 👀 🎀 C+ Reviewed

[melvin-bot]

Triggered auto assignment to @blimpich, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[allgandalf]

@c3024 @blimpich , can you please check my previously updated proposal once? The delete option vanished when we mark the task as completed :)

[melvin-bot]

❌ There was an error making the offer to @c3024 for the Reviewer role. The BZ member will need to manually hire the contributor.

[zanyrenney]

I also can't repro this anymore, i think we can close this one out as seems like it was solved and we're tackling any remaining issue in the issue linked above!

[nkdengineer]

@zanyrenney I still can reproduce this bug and PR is ready to review because the blocked issue is fixed

[blimpich]

Yeah I can still repro this, re-opening

[blimpich]

Merged in the open PR

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.57-5 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Fix: anonymous user can delete task #38272

If no regressions arise, payment will be issued on 2024-04-05. 🎊

For reference, here are some details about the assignees on this issue:

• @c3024 requires payment (Needs manual offer from BZ)
• @nkdengineer requires payment (Needs manual offer from BZ)

[melvin-bot]

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:

• [@c3024] The PR that introduced the bug has been identified. Link to the PR:
• [@c3024] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@c3024] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@c3024] Determine if we should create a regression test for this bug.
• [@c3024] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@zanyrenney] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[zanyrenney]

@c3024 have invited you to the job.

@nkdengineer what is your upwork profile please?

[c3024]

@zanyrenney thanks. Accepted the invite.

[zanyrenney]

payment summary

paid @c3024 $500 via upwork.

[zanyrenney]

can't find @nkdengineer on upwork.

[melvin-bot]

Payment Summary

Upwork Job

• ROLE: @c3024 paid $(AMOUNT) via Upwork (LINK)
• ROLE: @nkdengineer paid $(AMOUNT) via Upwork (LINK)

BugZero Checklist (@zanyrenney)

• I have verified the correct assignees and roles are listed above and updated the neccesary manual offers
• I have verified that there are no duplicate or incorrect contracts on Upwork for this job (https://www.upwork.com/ab/applicants/1767210887617179648/hired)
• I have paid out the Upwork contracts or cancelled the ones that are incorrect
• I have verified the payment summary above is correct

[zanyrenney]

Not heard back from @nkdengineer still:

[zanyrenney]

@nkdengineer has not accepted the invite still.

[nkdengineer]

@nkdengineer has not accepted the invite still.

@zanyrenney I accepted the invitation, thanks a lot!

[zanyrenney]

payment summary
ROLE: @c3024 paid $500 via Upwork
ROLE: @nkdengineer paid $500 via Upwork