[HOLD for payment 2024-03-19] [HOLD for payment 2024-03-14] [$500] IOU - An employee can access in a paid IOU the tag selection menu via a URL request

[lanitochka17]

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: 1.4.39-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: https://expensify.testrail.io/index.php?/tests/view/4292715
Email or phone of affected tester (no customers): [email protected]
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:

Action Performed:

Preconditions:
Set up an OldDot admin account, enable and add multiple tags, invite an employee to the policy https://sites.google.com/applausemail.com/applause-expensifyproject/wiki-guides/newdot-categories?authuser=0

1. Open https://staging.new.expensify.com/
2. Log in with the account of the employee added to the policy
3. Navigate to the group policy chat room
4. Create a manual request and send it to the WS room
5. Open https://staging.new.expensify.com/
   in incognito mode
6. Log in to the policy administrator account
7. Pay the money request from the employee
8. Go to the details of the paid IOU
9. Click on the Tag field to display a list of available tags
10. Copy the URL address
11. Paste and send the copied URL in the IOU details
12. On behalf of the employee, go to the details of the paid IOU
13. Click on the link sent by the administrator

Expected Result:

An employee should not access in a paid IOU the tag selection menu via a URL request

Actual Result:

An employee can access in a paid IOU the tag selection menu via a URL request

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

Bug6373372_1707478590720.Recording__1318.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01b368611226d91a30
• Upwork Job ID: 1755992751668711424
• Last Price Increase: 2024-02-09
• Automatic offers:
• ishpaul777 | Reviewer | 0
• FitseTLT | Contributor | 0

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[lanitochka17]

We think that this bug might be related to #wave6
CC @greg-schroeder

[FitseTLT]

Proposal

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

An employee can access in a paid IOU the tag selection menu via a URL request

What is the root cause of that problem?

We are displaying the tag picker in IOURequestStepTag whether the user canEdit or not. We haven't used the same logic where we control the interactiveness in moneyrequest view

App/src/components/ReportActionItem/MoneyRequestView.tsx

Lines 119 to 120 in b9aada8

	const canEdit = ReportUtils.canEditMoneyRequest(parentReportAction);
	const canEditAmount = ReportUtils.canEditFieldOfMoneyRequest(parentReportAction, CONST.EDIT_REQUEST_FIELD.AMOUNT);

App/src/components/ReportActionItem/MoneyRequestView.tsx

Lines 346 to 347 in b9aada8

	interactive={canEdit}
	shouldShowRightIcon={canEdit}

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

In IOURequestStepTag when user cannot edit request we should return Not found page here

App/src/pages/iou/request/step/IOURequestStepTag.js

Lines 99 to 104 in 94660d4

	<StepScreenWrapper
	headerTitle={policyTagListName}
	onBackButtonPress={navigateBack}
	shouldShowWrapper
	testID={IOURequestStepTag.displayName}
	>

    const canEdit = canEditMoneyRequest(getReportAction(report.parentReportID, report.parentReportActionID));

App/src/components/ReportActionItem/MoneyRequestView.tsx

Lines 119 to 120 in b9aada8

	const canEdit = ReportUtils.canEditMoneyRequest(parentReportAction);
	const canEditAmount = ReportUtils.canEditFieldOfMoneyRequest(parentReportAction, CONST.EDIT_REQUEST_FIELD.AMOUNT);

As we are controlling the interactiveness of the tag menu item in moneyrequest view

App/src/components/ReportActionItem/MoneyRequestView.tsx

Lines 346 to 347 in b9aada8

	interactive={canEdit}
	shouldShowRightIcon={canEdit}

We can optionally add isEditing condition too

App/src/pages/iou/request/step/IOURequestStepTag.js

Lines 58 to 59 in b9aada8

	const isEditing = action === CONST.IOU.ACTION.EDIT;
	const isSplitBill = iouType === CONST.IOU.TYPE.SPLIT;

What alternative solutions did you explore? (Optional)

Alternatively we can display a message In IOURequestStepTag instead of not found page
And ofcourse we can apply similarly for category and other pages

[jeremy-croff]

Is there any info on how to get access to https://sites.google.com/applausemail.com/applause-expensifyproject/wiki-guides/newdot-categories?authuser=0?

At the current depth I can only observe from without reproducing, It seems we want to protect the endpoint that is returning the tags initially, and have the UI handle the authorization at that level.

[ishpaul777]

reviewing this one today

[ishpaul777]

@jeremy-croff you can follow QA steps in #34127 to create collect policy and enable WS Room chat, then you can invite employee going to that collect workspace, enable tags and create tags, also make sure to change currency in olddot to USD

@FitseTLT your proposal looks good on a high level, but using it in IOURequestStepTag looks like this

Please update your proposal to include where you plan to put the FullPageNotFoundView

[FitseTLT]

@ishpaul777 Here is how it looks when you make FullPageNotFoundView the top-most parent component in IOURequestStepTag

[ishpaul777]

when you make FullPageNotFoundView the top-most parent component in IOURequestStepTag

Please update this within your proposal 👍

[FitseTLT]

Updated

[ishpaul777]

@FitseTLT Proposal looks good to me 🚀

🎀 👀 🎀 C+ reviewed

[melvin-bot]

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

[neil-marcellini]

@FitseTLT Proposal looks good to me 🚀

🎀 👀 🎀 C+ reviewed

I agree, the proposal to show the not found page if the user can't edit is the right approach.

[melvin-bot]

📣 @ishpaul777 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[melvin-bot]

📣 @FitseTLT 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[ishpaul777]

gentle bump @FitseTLT incase you didn't notice the assignment to this issue

[melvin-bot]

⚠️ Looks like this issue was linked to a Deploy Blocker here

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.

[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.48-0 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Fix - An employee can access in a paid IOU the tag selection menu via a URL request #37176

If no regressions arise, payment will be issued on 2024-03-14. 🎊

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

• @FitseTLT requires payment automatic offer (Contributor)
• @ishpaul777 requires payment automatic offer (Reviewer)

[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:

• [@ishpaul777] The PR that introduced the bug has been identified. Link to the PR:
• [@ishpaul777] 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:
• [@ishpaul777] 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:
• [@ishpaul777] Determine if we should create a regression test for this bug.
• [@ishpaul777] 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.
• [@JmillsExpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[melvin-bot]

⚠️ Looks like this issue was linked to a Deploy Blocker here

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.

[melvin-bot]

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

• [CP staging] Fix - allow receipt split bill tag editing #37879

If no regressions arise, payment will be issued on 2024-03-19. 🎊

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

• @FitseTLT requires payment automatic offer (Contributor)
• @ishpaul777 requires payment automatic offer (Reviewer)

[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:

• [@ishpaul777] The PR that introduced the bug has been identified. Link to the PR:
• [@ishpaul777] 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:
• [@ishpaul777] 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:
• [@ishpaul777] Determine if we should create a regression test for this bug.
• [@ishpaul777] 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.
• [@JmillsExpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[ishpaul777]

should be ready for payment tommorrow! will fill out checklist soon

[JmillsExpensify]

Payment summary while we wait on the BZ checklist:

• Contributor: @FitseTLT $250
• Contributor+: @ishpaul777 $250

[ishpaul777]

There was a regression payment should be halved..

[JmillsExpensify]

Thank you! Mind filling out the BZ checklist when you get a chance? Then we can issue payments. 🙌🏼

[ishpaul777]

[@ishpaul777] The PR that introduced the bug has been identified. Link to the PR: #28618
[@ishpaul777] 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: #28618 (comment)
[@ishpaul777] 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: not requires
[@ishpaul777] Determine if we should create a regression test for this bug. - yes
[@ishpaul777] 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.

Regression test proposal:

Preconditions:
Set up an OldDot admin account, enable and add multiple tags, invite an employee to the policy https://sites.google.com/applausemail.com/applause-expensifyproject/wiki-guides/newdot-categories?authuser=0

1. Open Newdot
2. Log in with the account of the employee added to the policy
3. Navigate to the group policy chat room
4. Create a manual request and send it to the WS room
5. open chrome incognito mode/any other browser
6. Log in to the policy administrator account
7. Pay the money request from the employee
8. Go to the details of the paid IOU
9. Click on the Tag field to display a list of available tags
10. Copy the URL address
11. Paste and send the copied URL in the IOU details
12. On behalf of the employee, go to the details of the paid IOU
13. Click on the link sent by the administrator
14. Verify An employee should not access in a paid IOU the tag selection menu via a deeplink and not-found page is displayed

do we agree 👍 or 👎

[JmillsExpensify]

All contributors paid via Upwork and regression test created, so I'm closing the issue.