Workspace - Category and Tag "Required" switch is enabled and locked evenif all tags are disable

[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: 9.0.87-0
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: N/A
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): [email protected]
Issue reported by: Applause - Internal Team
Component: Workspace Settings

Action Performed:

1. Navigate to https://staging.new.expensify.com/
2. Create WS and enable Tag
3. Add some tags and enable "Members must tag all expenses" toggle
4. Navigate to Category and enable "Members must categorize all expenses" toggle
5. Select all Category and disable all category.
6. Navigate to setting and note that the toggle is is disabled and locked
7. Go back and navigate again and note that the toggle is enabled and locked
8. Navigate to Tag and Select all tags.
9. Disable all tags and navigate to setting and note that the toggle is enabled and locked

Expected Result:

The toggle is disabled and locked when all tags and categories are disabled

Actual Result:

The toggle is enabled and locked when all tags and categories are disabled

Workaround:

Unknown

Platforms:

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

• Android: Standalone
• Android: HybridApp
• Android: mWeb Chrome
• iOS: Standalone
• iOS: HybridApp
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6717031_1737140837036.bandicam_2025-01-17_21-46-17-653.mp4

View all open jobs on GitHub

[melvin-bot]

Triggered auto assignment to @alexpensify (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.

[Themoonalsofall]

Template proposal

Proposal

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

Workspace - Category and Tag "Required" switch is enabled and locked evenif all tags are disable

What is the root cause of that problem?

we define the toggle logic active

App/src/pages/workspace/categories/WorkspaceCategoriesSettingsPage.tsx

Line 108 in 72723ab

isActive={policy?.requiresCategory ?? false}

in case we have turned it on before, the toggle will still be on and locked even though all categories are disabled

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

We should turn off the active state of that toggle by update logic to

         isActive={(policy?.requiresCategory && hasEnabledOptions) ?? false}

and do the same with the Tag page

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

NA

What alternative solutions did you explore? (Optional)

Or we can add new useEffect to set policy.requiresCategory to false when all categories are disabled

    useEffect(()=>(
        if (!hasEnabledOptions) {
            updateWorkspaceRequiresCategory(false);
        }
    ),[hasEnabledOptions]);

Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.

[gijoe0295]

The second OpenPolicyCategoriesPage call when we dismiss the RHP returns policy's requiresCategory as true which is not correct, while the first same call returns false. This is BE related.

[Shahidullah-Muffakir]

The second OpenPolicyCategoriesPage call when we dismiss the RHP returns policy's requiresCategory as true which is not correct, while the first same call returns false. This is BE related.

But if the user enables some categories again, the toggle should turn on. If the backend sets requiresCategory to false after all categories are disabled, then after enabling some categories, the toggle will stay off.

[allgandalf]

@alexpensify this is a BE issue, please use Internal label

[melvin-bot]

@alexpensify Whoops! This issue is 2 days overdue. Let's get this updated quick!

[alexpensify]

I'm adding Hot Pick, but I'm still reviewing which wave to associate with this one to get more attention.

[alexpensify]

Update: Hot Pick

[melvin-bot]

@alexpensify this issue was created 2 weeks ago. Are we close to a solution? Let's make sure we're treating this as a top priority. Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[alexpensify]

Hot Pick!

[alexpensify]

Weird, I thought this one was associated with a Project but updated it for more visibility.

[alexpensify]

Weekly Update: Hot Pick

[alexpensify]

Weekly Update: Hot Pick!

[tgolen]

I will start looking into this. I'm going to start by just looking into categories as I feel that whatever I find there will also apply to tags, but best to look at the separately to start with.

@luacmartins You might know about this more than others.

Investigation

• E/App assumes that if all categories are being disabled, then requiresCategory should be set to false (here)
• Web-E calls mergeCategories() here which doesn't have any such logic, and then PolicyStore::save() is called with the changed categories
• Auth also doesn't appear to have any such logic either (I can't find anywhere in Auth that directly changes requiresCategory)
• Since requiresCategory was never changed in the database, the command OpenPolicyWorkflowsPage continues to return requiresCategory: true when the frontend assumes it should be false

Conclusion

While the front end added this logic, there doesn't appear to be any matching backend logic, so the requiresCategory is never being set to false as the front end assumes.

Solution

We have a few options:

1. Remove the frontend logic and stop making the assumption that requiresCategory is supposed to change. I don't think this is the best solution because it's very apparent that the frontend was specifically built with this logic for a reason.
2. Add logic to PHP that looks to see if all categories have been disabled and then calls $policy->setRequiresCategory(false);
3. Instead of adding that logic to PHP, add it to Auth in the SavePolicy command

Since PolicyAPI::mergeCategories() hasn't been fully migrated to Auth yet, and it's still 1:1:1, then I kinda think option 2 is the best solution here.

[luacmartins]

It seems like we already had this issue before (or a similar one) and we landed on this. It seems like we need some backend logic to enforce this.

[tgolen]

OK, thanks. That validates the BE logic needing to be added. I'll go ahead and write up a PR for it. I think I might scope it just to NewDot commands though.

Both OldDot and NewDot share the same command logic, and I'm not in the mood to change how OldDot works for this one.

[tgolen]

I'm running into a problem with this... and even the frontend functionality is flawed I think. Let me illustrate the problem:

1. Workspace is set to require categories
2. All categories are disabled
3. Workspace is now set to not require categories
4. One category is enabled
5. Workspace is now set to require categories

That works totally fine. Now, here is the part that doesn't work (the only step that changes is step 1)

1. Workspace does NOT require categories
2. All categories are disabled
3. Workspace is now set to not require categories
4. One category is enabled
5. Workspace is now set to require categories

Step 5 is wrong in this case because the workspace owner never wanted to require tags in the first place. Unfortunately, there is no way to prevent this from happening and the current logic cannot be fixed because there is no way for Step 5 to know what the workspace owner wanted in Step 1.

I believe the true solution for this is that requiresCategory and requiresTag should never be changed based on categories/tags being enabled or disabled. It should ONLY be changed when the user toggles the actual "Requires Tag/Category" setting.

Any other desired functionality based on all the tags/categories being disabled or not should strictly relate to the tags/categories being enabled/disabled.

tagging @trjExpensify on this one to see if you have any suggestions.

[trjExpensify]

Initially, in the update how we display / report expenses doc, the logic was introduced to force requireCategories on the workspace when connected to an accounting solution (both NewDot and OldDot). Reason being, a corresponding account is the bare minimum required to export to accounting - and so with the immediacy of instant submit as a consideration, that would be insta-failure on creation unless flagged as a violation to hold back the auto-export.

So in that case, you shouldn't be able to disable requireCategories on the workspace. Tags are totally optional coding, so there shouldn't be any logic that sets those as required when connected to an accounting solution.

I believe the true solution for this is that requiresCategory and requiresTag should never be changed based on categories/tags being enabled or disabled.

Yeah, I also think this is a fine approach to clean this up, because there's nothing preventing a user from submitting an expense with a requiredCategory/tag coding field -- it'll get flagged as a violation (preventing the export attempt), and if the categories list is empty, they can ask their admin why they disabled all of the values. 😅

It should ONLY be changed when the user toggles the actual "Requires Tag/Category" setting.

Agree for requiredTag. For requiredCategories though, it should be enabled when connecting to an accounting solution. It can remain enabled when accounting is disconnected, but just noting the desire to have requireCategories enabled when connected to an accounting solution.

[tgolen]

Thanks, Tom. I don't see any problem with having requireCategories be set to true when connected to an accounting solution. I think that's OK. My point is more just related to when a person manually goes in and enables/disables specific tags/categories.

Solution

It sounds like the best path forward for this right now is:

• [FE] Remove shouldDisableRequiresCategory variable here and the resulting optimistic data (it's only set when all categories have been disabled)
• [FE] Remove shouldDisableRequiredTag variable here and the resulting optimistic data (it's only set when all tags have been disabled)

There is a possible complimentary solution:

• [FE] If Tags/Categories are required, but all tags/categories have been disabled, display a clear warning at the top of the Tags/Categories page that lets the workspace admin know that's not an ideal situation.

No backend changes would be required for any of these solutions and none of this would changes what happens to requiresTag and requiresCategory when an accounting package is connected.

@trjExpensify Would you be OK with all three of these items?

[trjExpensify]

Sounds great to me!

[twilight2294]

Proposal

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

Category and Tag "Required" switch is enabled and locked evenif all tags are disable

What is the root cause of that problem?

The RCA is that the FE and BE logic doesn't match, the details are pointed out by @tgolen here.

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

According to the expected solution from here:

• Remove the variable shouldDisableRequiresCategory here:
  

  App/src/libs/actions/Policy/Category.ts

  Line 230 in fe3d37c

  const shouldDisableRequiresCategory = !hasEnabledOptions({...policyCategories, ...optimisticPolicyCategoriesData});

App/src/libs/actions/Policy/Category.ts

Line 882 in fe3d37c

const shouldDisableRequiresCategory = !hasEnabledOptions(

And also remove the optimistic and success / failure data here:

App/src/libs/actions/Policy/Category.ts

Lines 918 to 920 in fe3d37c

	if (shouldDisableRequiresCategory) {
	onyxData.optimisticData?.push({
	onyxMethod: Onyx.METHOD.MERGE,

App/src/libs/actions/Policy/Category.ts

Lines 279 to 280 in fe3d37c

	if (shouldDisableRequiresCategory) {
	onyxData.optimisticData?.push({

• Remove the variable shouldDisableRequiredTag here:
  

  App/src/libs/actions/Policy/Tag.ts

  Line 263 in fe3d37c

  const shouldDisableRequiredTag = !OptionsListUtils.hasEnabledOptions({...policyTag.tags, ...optimisticPolicyTagsData});

And also remove the optimistic and success / failure data here:

App/src/libs/actions/Policy/Tag.ts

Line 272 in fe3d37c

...(shouldDisableRequiredTag ? {required: false, pendingFields: {required: CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE}} : {}),

App/src/libs/actions/Policy/Tag.ts

Line 284 in fe3d37c

...(shouldDisableRequiredTag ? {pendingFields: {required: null}} : {}),

App/src/libs/actions/Policy/Tag.ts

Line 311 in fe3d37c

...(shouldDisableRequiredTag ? {pendingFields: {required: null}, required: policyTag.required} : {}),

• If Tags/Categories are required, but all tags/categories have been disabled, display a clear warning at the top of the Tags/Categories page that lets the workspace admin know that's not an ideal situation.

For this, we would require inputs from the design team for the exact mock, but we need to update the WorkspaceCategoriesPage page and WorkspaceTagsPage:

App/src/pages/workspace/categories/WorkspaceCategoriesPage.tsx

Line 61 in fe3d37c

function WorkspaceCategoriesPage({route}: WorkspaceCategoriesPageProps) {

App/src/pages/workspace/tags/WorkspaceTagsPage.tsx

Line 63 in fe3d37c

function WorkspaceTagsPage({route}: WorkspaceTagsPageProps) {

Here For example in Categories Page, we will check: if requiresCategory is true and then check if all the categories in POLICY_CATEGORIES onyx key for that particular policy are disabled the we will display a clear warning at the top of the Tags/Categories page that lets the workspace admin know that's not an ideal situation.

Same would be done for tags.

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

We probably need to update some existing unit tests and also add new ones to check the new state of onyx data, we would check the state where we disable all category /tags and check on the UI if the warning modal is present

What alternative solutions did you explore? (Optional)

N/A

[tgolen]

@Expensify/design Could you help us come up with a mockup for this page? I would like it to display a warning at the top of it for when all categories are disabled. The message would say something like:

Heads up! You have required members to categorize all expenses but all of the categories are disabled.

We would also do the same for the tags page.

[dannymcclain]

@Expensify/design is this an illegal use of Jon's brand new banner component??

Do we need a Lil Banner™ or something?

Just tossing some ideas out there. Please don't put me in jail!

[shawnborton]

@tgolen is it possible to prevent the user from deselecting all of the categories if categorization is required? I'm just trying to think what the UI looks like to the user who is creating an expense in this case... would they be able to go edit the Category push row but then just see a blank list? I feel like we should try to prevent that?