[$250] Workspace - Single category and tag isn't auto selected when requesting money

[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.63.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:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by:
Slack conversation:

Issue found when executing PR #40217

Action Performed:

1. Log in with a new Gmail account
2. Create a workspace
3. Edit categories and tags so you only have one of each
4. Make tags and categories required
5. Navigate to FAB - Submit expense - Manual
6. Select the workspace chat
7. Tap on the "Show more" button

Expected Result:

Category and tag fields should auto select the single entry

Actual Result:

Single category and tag isn't auto selected when requesting money

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

Bug6454173_1713468126792.OHYC4360.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01342dc85ef896c8e8
• Upwork Job ID: 1782368096132030464
• Last Price Increase: 2024-04-22
• Automatic offers:
• ahmedGaber93 | Reviewer | 0
• dukenv0307 | Contributor | 0
• GandalfGwaihir | Contributor | 0

[melvin-bot]

Triggered auto assignment to @zanyrenney (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[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 #wave-collect - Release 1

[allgandalf]

Proposal

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

Single category and tags aren't auto selected when requesting money.

What is the root cause of that problem?

Here when we have only one tag / one category we have a check which sets the auto selected value, but here before that we have certain conditions as below:
For Category:

App/src/components/MoneyRequestConfirmationList.tsx

Lines 489 to 491 in 8699315

	if (iouCategory || !shouldShowCategories || enabledCategories.length !== 1 || !isCategoryRequired) {
	return;
	}

For Tag:

App/src/components/MoneyRequestConfirmationList.tsx

Lines 501 to 504 in 8699315

	const isTagListRequired = tagList.required === undefined ? false : tagList.required && canUseViolations;
	if (!isTagListRequired || enabledTags.length !== 1 || TransactionUtils.getTag(transaction, index)) {
	return;
	}

When I debugged this I found out that !isCategoryRequired and isTagListRequired are always set to true, regardless of whatever we have set on the BE (In workspace category and tags page, when we set the values as required those values are successfully changed on the BE), so no issue on the BE.

Now coming to why values isCategoryRequired and isTagListRequired are always false regardless of the backend value, that is because we had added an extra check during the feature addition of categories and tags to only set those values to true if the user has beta access by checking the canUseViolations value, we check isCategoryRequired as well as isTagListRequired with canUseViolations and their respective BE values:

App/src/components/MoneyRequestConfirmationList.tsx

Line 296 in 8699315

const isCategoryRequired = canUseViolations && !!policy?.requiresCategory;

App/src/components/MoneyRequestConfirmationList.tsx

Line 501 in 8699315

const isTagListRequired = tagList.required === undefined ? false : tagList.required && canUseViolations;

This causes the values to be always set to false.

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

We should remove the && canUseViolations from both isTagListRequired as well as isCategoryRequired

Result Video

Screen.Recording.2024-04-20.at.5.47.20.AM.mov

Now that I searched more, I think we should get rid of the canUseViolations hook altogether, because this also gives us regression, it doesn't make tags required (Even when we have required value on the BE):

App/src/components/MoneyRequestConfirmationList.tsx

Line 797 in 8699315

const isTagRequired = required === undefined ? false : canUseViolations && required;

So I guess we should remove the use of hook from the page as the categories and tags for collect policy are no longer in beta

[Tony-MK]

Proposal

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

Workspace - Single category and tag isn't auto selected when requesting money

What is the root cause of that problem?

This is a regression caused by #40217 and was suggested in #38744 (comment).

iouCategory and transactionID should be included in the category dependencies required because the iouCategory will change after the first render.

App/src/components/MoneyRequestConfirmationList.tsx

Lines 515 to 523 in 1e510b5

	// Auto select the category if there is only one enabled category and it is required
	useEffect(() => {
	const enabledCategories = Object.values(policyCategories ?? {}).filter((category) => category.enabled);
	if (iouCategory || !shouldShowCategories || enabledCategories.length !== 1 || !isCategoryRequired) {
	return;
	}
	IOU.setMoneyRequestCategory(transactionID, enabledCategories[0].name);
	// eslint-disable-next-line react-hooks/exhaustive-deps
	}, [shouldShowCategories, policyCategories, isCategoryRequired]);

The tag field does not have this issue because the dependencies are set properly.

App/src/components/MoneyRequestConfirmationList.tsx

Lines 537 to 539 in 1e510b5

	IOU.setMoneyRequestTag(transactionID, updatedTagsString);
	}
	}, [policyTagLists, transaction, transactionID, policyTags, canUseViolations]);

This is why I only experienced this with the category field.

I believe the category dependencies should not have the eslint rule line like the tag dependencies`.

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

First, add iouCategory and transactionID as apart dependencies.
Hence, it should look like the code example below.

}, [iouCategory, transactionID, shouldShowCategories, policyCategories, isCategoryRequired]);

Finally, remove the eslint rule line below because there will be no dependency issues.

App/src/components/MoneyRequestConfirmationList.tsx

Line 522 in 1e510b5

// eslint-disable-next-line react-hooks/exhaustive-deps

What alternative solutions did you explore? (Optional)

Since iouCategory is defaulted to an empty string, we can initialize it to the only categories in the MoneyRequestConfirmationListProps if the enabledCategories is equal to one and categories are required.

Note

I noticed you can empty the tag and category for a brief moment before the backend reverts them.

Hence, I recommend we prohibit the ability to change the tag or category in this situation on the edit pages in the PR.

POC.-.Chrome.mov

[Krishna2323]

Proposal Updated

• Added optional to not show category/tag as supplementary when we autoselect.

[Krishna2323]

@Tony-MK, I don't think that it is a regression from my PR, can you pls explain why do you think that it is regression?

[Tony-MK]

Hey @krishna232, the useEffect needs the iouCategory dependency like the useEffect for the tags hook.

[zanyrenney]

adding external label

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01342dc85ef896c8e8

[melvin-bot]

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

[zanyrenney]

@ahmedGaber93 we have a few proposals now, please review them and let me know which we should go forward with. As this is related to release one so if you don't have capacity, please assign yourself and I will assign another C+.

[ahmedGaber93]

@zanyrenney Reviewing today.

[ahmedGaber93]

Thanks for the proposal every one!

@Krishna2323 your proposal will work, but it will auto select the single item even if tags/categories isn't set as a required which not match the expected result.

@GandalfGwaihir your RCA is correct, and your fix should work fine, but that's mean isCategoryRequired and isTagListRequired is under beat canUseViolations, I will ask in slack, and you can dig in the code to find why this is still under beta while we can access categories/tags in IOU request and make them required in workspace settings without this beta.

@Tony-MK your RCA is incorrect, if you have beta canUseViolations it should work fine without your fix.

[Krishna2323]

@ahmedGaber93, I misunderstood the issue here, I think auto-selection works fine when we have single tag/category and it is required. Can you pls tell me what I'm missing?

[ahmedGaber93]

Asking in slack to know why isCategoryRequired and isTagListRequired is under beat canUseViolations, while we can access categories/tags when create IOU request and can make them required in workspace settings categories/tags without this beta.
If this intended, or it is issue ?

https://expensify.slack.com/archives/C02NK2DQWUX/p1714077357630999

[ahmedGaber93]

I misunderstood the issue here, I think auto-selection works fine when we have single tag/category and it is required. Can you pls tell me what I'm missing?

@Krishna2323 if you have beta canUseViolations auto select will work fine, if this intended, this will not been issue.

[Krishna2323]

@ahmedGaber93, thanks, you are correct, I was testing with beta access.

[allgandalf]

and you can dig in the code to find why this is still under beta while we can access categories/tags in IOU request and make them required in workspace settings without this beta.

isCategoryRequired was introduced in 609a1d0 by @dukenv0307 , maybe he can help :)

question for @dukenv0307 :

1. I think category and tags were both in beta at the time you introduced canUseViolations, so was it a temporary fix which was intended to be removed later? Or are we missing something here?

[zanyrenney]

going to manually assign @dukenv0307 to take a look.

[melvin-bot]

📣 @ahmedGaber93 🎉 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]

📣 @dukenv0307 🎉 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 📖

[allgandalf]

@zanyrenney but this is open for external contributors right?

[allgandalf]

Also we have a slack discussion pending on it:
https://expensify.slack.com/archives/C02NK2DQWUX/p1714077357630999

[ahmedGaber93]

I think we can move forward with @GandalfGwaihir's proposal and discuss that with the internal engineer.

Why choosing removing beta canUseViolations check from isCategoryRequired and isTagListRequired?
I think using canUseViolations here is by mistake and not intentionally for these reasons:

1. isCategoryRequired and isTagListRequired are used for UI cases only in autoselect and display label required and in isSupplementary which display it under "show more" section, and not used on validation before sending the API in confirm method, and also not affected on shouldShow condition.

2. isCategoryRequired and isTagListRequired will not affect in the final output, because the user can select category manually instead of autoselect, and can expand "show more" manually instead of isSupplementary, the only effect here is required label will be displayed, and we don't have validation for required tags/categories before sending the API in confirm method, so we may keep canUseViolations in required label only or add validation before send the API and in BE side.

3. we can access categories/tags when create IOU request and can make them required in workspace settings categories/tags without this beta.

🎀 👀 🎀 C+ reviewed

[melvin-bot]

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

[melvin-bot]

📣 @GandalfGwaihir 🎉 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 📖

[zanyrenney]

@GandalfGwaihir please let me know how you get on here!

[melvin-bot]

@cristipaval @ahmedGaber93 @zanyrenney @GandalfGwaihir this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[allgandalf]

Chill Melvin!, PR would be ready in 1 hour :)

[allgandalf]

PR ready for review c.c. @ahmedGaber93 , sorry got under the weather at the time of the above message, feeling better now 😃

[ahmedGaber93]

I am not sure if we should have waited for @cristipaval to accept the proposal before move forward with PR, but as the contributor is assigned to the issue and the PR is already started, I am going to review the PR, and we can move any discussion there.

[allgandalf]

⚠️ Automation Failed here, This was deployed to Production today @zanyrenney

[allgandalf]

I think this is ready for payment @zanyrenney , can you take a look once you find time 😄

Update:

Their GH says they're OoO until June 9, so maybe lets hold this until then, have great vacation @zanyrenney 🏖️