[Wave 6: Categories] Category/Tag Selector Show up in DM Money Requests

[yuwenmemon]

Action Performed:

1. Create a new account
2. Create a money request from another user
3. On the confirmation step, observe that the category and tag selectors show for the money request:
   https://github.com/Expensify/App/assets/4741899/16a59421-b77a-42da-a35b-0a0e49d25822

Expected Result:

• The Category and Tag selectors should not show for a DM Money Request because that chat report does not belong to a policy (They belong to the policy _FAKE_)

Actual Result:

• The Category/Tag selector is visible for the Money Request on the confirmation page.

Workaround:

• This feature is under a beta so real-life users can't see it yet.

Platforms:

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

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: N/A
Reproducible in staging?: Yes
Reproducible in production?: Yes
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): N/A
Logs: N/A
Notes/Photos/Videos: Any additional supporting documentation
Expensify/Expensify Issue URL: N/A
Issue reported by: @rezkiy37
Slack conversation: https://expensify.slack.com/archives/C02MW39LT9N/p1694793499639819

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~019f9ca0cd0d1a9a22
• Upwork Job ID: 1702759634044727296
• Last Price Increase: 2023-09-15
• Automatic offers:
• situchan | Reviewer | 26699580

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[neonbhai]

Proposal

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

Category/Tag Selector Show up in DM Money Requests

What is the root cause of that problem?

For Tags:

We don't check if the report is a policyExpenseChat when use set value of canUseTags here:

App/src/components/MoneyRequestConfirmationList.js

Line 200 in 191bda1

const canUseTags = Permissions.canUseTags(props.betas);

For Category:

🤔 Cannot see Category on latest main, which checks out, as we check for props.policyCategories here:

App/src/components/MoneyRequestConfirmationList.js

Line 194 in 191bda1

const shouldCategoryBeEditable = !_.isEmpty(props.policyCategories) && Permissions.canUseCategories(props.betas);

Since if chat is not a policy, policy chat will not be populated. So shouldCategoryBeEditable will be false and the Category menuItem will not be rendered:

App/src/components/MoneyRequestConfirmationList.js

Lines 513 to 514 in 191bda1

	{shouldCategoryBeEditable && (
	<MenuItemWithTopDescription

Although I feel the name is misleading, as it feels like shouldCategoryBeEditable being false will disable the component, instead of not showing it altogether. It should be changed to shouldShowCategory

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

We should add

ReportUtils.isPolicyExpenseChat(ReportUtils.getReport(props.reportID));

to canUseTags to check if report is a policy Chat.

And change shouldCategoryBeEditable here to shouldShowCategory to better reflect its purpose in code.

Result

Tags.webm

What alternative solutions did you explore? (Optional)

xx

[rezkiy37]

Hello, I am Mykhailo from Callstack. I would like to work on this issue.

[melvin-bot]

📣 @situchan 🎉 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

[rezkiy37]

Working on the issue.

[rezkiy37]

@yuwenmemon, I am going to fix it in a scope of #24464 (#27459). Because I need a few same new helpers there. Are we okay with it?

[yuwenmemon]

Yep sounds good to me!

[rezkiy37]

The PR - #27459 - has been merged.

[puneetlath]

@situchan I see you assigned to the issue, but it doesn't look like you were involved in the PR review, is that correct?

[situchan]

@situchan I see you assigned to the issue, but it doesn't look like you were involved in the PR review, is that correct?

correct. I wasn't aware that linked PR was in progress.
Seems like #27459 wasn't reviewed by any C+.

@rezkiy37 in the future, please link issues like this so that automation would work correctly

Wrong:

$ https://github.com/Expensify/App/issues/24464, https://github.com/Expensify/App/issues/27556

Correct:

$ https://github.com/Expensify/App/issues/24464
$ https://github.com/Expensify/App/issues/27556

[puneetlath]

Ok, thanks for confirming @situchan. We're good to close this out in that case once the PR is deployed to production.

[rezkiy37]

@situchan I see you assigned to the issue, but it doesn't look like you were involved in the PR review, is that correct?

correct. I wasn't aware that linked PR was in progress. Seems like #27459 wasn't reviewed by any C+.

@rezkiy37 in the future, please link issues like this so that automation would work correctly

Wrong:

$ https://github.com/Expensify/App/issues/24464, https://github.com/Expensify/App/issues/27556

Correct:

$ https://github.com/Expensify/App/issues/24464
$ https://github.com/Expensify/App/issues/27556

Sorry, my bad 😅

[rezkiy37]

The issue (#24464), in which scope I fixed the bug, already closed.

[puneetlath]

@rezkiy37 just to clarify, you're saying this bug has been fixed and we can close this issue, is that correct?

[rezkiy37]

@rezkiy37 just to clarify, you're saying this bug has been fixed and we can close this issue, is that correct?

Yes, it is correct.

[puneetlath]

Great!