[Awaiting Payment] [$250] Multi tags - Main tag shows "Required" status when all the subtags are disabled

[izarutskaya]

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.30-4
Reproducible in staging?: Y
Reproducible in production?: Y
Found when validation PR: #48381
Email or phone of affected tester (no customers): [email protected]
Logs: https://stackoverflow.com/c/expensify/questions/4856
Issue reported by: Applause-Internal team

Action Performed:

Precondition:

• Workspace is connected to QBO.

1. Go to staging.new.expensify.com
2. Go to workspace settings > Tags.
3. Click on any main tag.
4. Enable "Required" switch.
5. Select all the subtags > Disable.
6. Disable all the subtags.
7. Return to main tag page.

Expected Result:

The main tag will not show "Required" status when all the subtags are disabled.

Actual Result:

The main tag shows "Required" status when all the subtags are disabled.
The "Required" status is gone after reopening the subtag list.

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

Bug6594733_1725578358405.1725577741534_Screen_Recording_20240906_070850_New_Expensify.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021833824644157847399
• Upwork Job ID: 1833824644157847399
• Last Price Increase: 2024-09-11
• Automatic offers:
• ikevin127 | Reviewer | 103912305

Issue Owner

Current Issue Owner: @

Issue Owner

Current Issue Owner: @ikevin127

[melvin-bot]

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

[FitseTLT]

Edited by proposal-police: This proposal was edited at 2024-09-06 10:27:23 UTC.

Proposal

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

Multi tags - Main tag shows "Required" status when all the subtags are disabled

What is the root cause of that problem?

We are not checking if it has one enabled tag when displaying required tag here

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

Line 100 in 2ee0bfc

required: policyTagList.required,

It is gone after opening the tag because we set required to false here

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

Lines 235 to 236 in 2ee0bfc

	if (!!currentPolicyTag?.required && !Object.values(currentPolicyTag?.tags ?? {}).some((tag) => tag.enabled)) {
	Tag.setPolicyTagsRequired(policyID, false, route.params.orderWeight);

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

We should add && OptionsListUtils.hasEnabledOptions(tags) to check if it has an enabled option

                required: policyTagList.required&&OptionsListUtils.hasEnabledOptions(policyTagList.tags),

In WorkspaceViewTagsPage we can apply the same condition to switch of the required toggle or we can also disable the toggle when there are no enabled tags.

What alternative solutions did you explore? (Optional)

We can also setPolicyTagsRequired to false in WorkspaceTagsPage as we did here.
for isMultiLevelTags case we can iterate on policyTagsLists array and setPolicyTagsRequired to false for every muiti-level tag where required is true and all sub tags are disabled.

[garrettmknight]

Checking for a regression on the linked PR

[Shahidullah-Muffakir]

@garrettmknight

To hide the "Required" label when all tags are disabled, we should implement a similar check to what we applied for the Required toggle. The current check only verifies if the policyTagList is required, but it doesn't account for the scenario where all subtags are disabled

in this :

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

Line 103 in 7dc2515

labelText={policyTagList.required ? translate('common.required') : undefined}

We can update the logic as follows:
labelText={policyTagList.required && !!Object.values(policyTagList?.tags ?? {}).some((tag) => tag.enabled) ? translate('common.required') : undefined}
This way, the label will only appear if the main tag is required and at least one subtag is enabled.

Additionally, I'm not sure if this issue is a regression—it seems more like a separate bug that was overlooked in the previous implementation.

[Shahidullah-Muffakir]

@garrettmknight I’ve opened a PR #48738 to address this issue. It’s now ready for review. Thank you.

[ikevin127]

Just 🟢 Approved the PR for this 🚀

FYI we determined that the other issue didn't cause this as in regression but moreso revealed some incomplete logic, therefore this should not be treated as a regression coming from the other issue's PR - more context in #47818 (comment) (see comment above for proof).

Given the context above, I guess payment should be treated separately for this issue as I noticed automation already created payment issue #48766 once I approved the PR.

cc @garrettmknight

[sonialiap]

@garrettmknight I think melvin got too excited and created a payment issue, maybe because this hadn't been moved to "external" before the PRs were approved? I'm going to close #48766 in favor of processing payment here, on the main issue

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

📣 @Shahidullah-Muffakir You have been assigned to this job!
Please apply to the Upwork job and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Once you apply to this job, your Upwork ID will be stored and you will be automatically hired for future jobs!
Keep in mind: Code of Conduct | Contributing 📖

[garrettmknight]

@FitseTLT appreciate that you put forward a proposal for this, I'll pay you half if your proposal matches what was implemented.

[garrettmknight]

@ikevin127 @Shahidullah-Muffakir I'll pay you both out for this issue since it's a diff one than the other we highlighted.

[garrettmknight]

@ikevin127 can your review this proposal and let me know if it was significantly similar to the PR that solves this bug?

[FitseTLT]

@FitseTLT appreciate that you put forward a proposal for this, I'll pay you half if your proposal matches what was implemented.

No it doesn't match @garrettmknight

[Shahidullah-Muffakir]

📣 @Shahidullah-Muffakir You have been assigned to this job! Please apply to the Upwork job and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻 Once you apply to this job, your Upwork ID will be stored and you will be automatically hired for future jobs! Keep in mind: Code of Conduct | Contributing 📖

@garrettmknight,
I see I've been assigned to this task, but since the PR is already merged, can you explain the procedure? Do I still need to apply directly on Upwork? Thank you!

[ikevin127]

@ikevin127 can your review #48683 (comment) and let me know if it was significantly similar to the #48738 that solves this bug?

I confirm that the proposed solution and the one applied in the PR completely different.

[garrettmknight]

@Shahidullah-Muffakir just sent you an offer.

[Shahidullah-Muffakir]

@Shahidullah-Muffakir just sent you an offer.

@garrettmknight Thank you, accepted.

[ikevin127]

⚠️ Automation failed here -> this should have been paid out on 2024-09-17 according to production deploy checklist Deploy Checklist: New Expensify 2024-09-09 which was shipped to production on 2024-09-10.

I did not receive the payment yet, here's the contract: https://www.upwork.com/nx/wm/workroom/38345594. In case I'm mistaking and I did receive the payment already - please dismiss 👍

cc @garrettmknight

[garrettmknight]

All paid out, sorry for the wait! @ikevin127 can you complete the checklist when you get a chance?

[ikevin127]

• [@ikevin127] The PR that introduced the bug has been identified. Link to the PR: Disable Required Toggle When No Subtags Are Enabled #48381
• [@ikevin127] 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: Disable Required Toggle When No Subtags Are Enabled #48381 (comment)
• [@ikevin127] 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: N/A.
• [@ikevin127] Determine if we should create a regression test for this bug.
• [@ikevin127] 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

1. Go to staging.new.expensify.com .
2. Go to workspace settings > Tags.
3. Click on any main tag.
4. Enable "Required" switch.
5. Select all the subtags > Disable all subtags.
6. Return to main tag page.
7. Verify that the main tag will not show "Required" label.

Do we agree 👍 or 👎.