Fix optimistic tag and category violations on the Require fields page

Signed-off-by: krishna2323 <belivethatkg@gmail.com>

Author: Krishna2323

src/libs/PolicyUtils.ts

@@ -2012,14 +2012,15 @@ function hasDependentTags(policy: OnyxEntry<Policy>, policyTagList: OnyxEntry<Po
     if (!policy?.hasMultipleTagLists) {
         return false;
     }
-    return Object.values(policyTagList ?? {}).some((tagList) => Object.values(tagList.tags).some((tag) => !!tag.rules?.parentTagsFilter || !!tag.parentTagsFilter));
+    // An empty tag list arrives without the `tags` key, despite the type.
+    return Object.values(policyTagList ?? {}).some((tagList) => Object.values(tagList.tags ?? {}).some((tag) => !!tag.rules?.parentTagsFilter || !!tag.parentTagsFilter));
 }
 
 function hasIndependentTags(policy: OnyxEntry<Policy>, policyTagList: OnyxEntry<PolicyTagLists>) {
     if (!policy?.hasMultipleTagLists || hasDependentTags(policy, policyTagList)) {
         return false;
     }
-    return Object.values(policyTagList ?? {}).some((tagList) => Object.values(tagList.tags).length > 0);
+    return Object.values(policyTagList ?? {}).some((tagList) => Object.values(tagList.tags ?? {}).length > 0);
 }
 
 /**

src/libs/actions/Policy/Category.ts

@@ -1209,7 +1209,8 @@ function setPolicyCategoryGLCode(policyID: string, categoryName: string, glCode:
     API.write(WRITE_COMMANDS.UPDATE_POLICY_CATEGORY_GL_CODE, parameters, onyxData);
 }
 
-function setWorkspaceRequiresCategory(policyData: PolicyData, requiresCategory: boolean) {
+/** Pass shouldRecomputeViolations = false when tag Required changes in the same save: each recompute SETs violations from the pre-save snapshot, so two overwrite each other. */
+function setWorkspaceRequiresCategory(policyData: PolicyData, requiresCategory: boolean, shouldRecomputeViolations = true) {
     const policyID = policyData.policy?.id;
     const policyOptimisticData: Partial<Policy> = {
         requiresCategory,
@@ -1258,7 +1259,9 @@ function setWorkspaceRequiresCategory(policyData: PolicyData, requiresCategory:
         ],
     };
 
-    pushTransactionViolationsOnyxData(onyxData, policyData, policyOptimisticData);
+    if (shouldRecomputeViolations) {
+        pushTransactionViolationsOnyxData(onyxData, policyData, policyOptimisticData);
+    }
 
     const parameters = {
         policyID,

src/libs/actions/Policy/Tag.ts

@@ -1058,10 +1058,12 @@ function renamePolicyTagList(policyID: string, policyTagListName: {oldName: stri
     API.write(WRITE_COMMANDS.RENAME_POLICY_TAG_LIST, parameters, onyxData);
 }
 
-function setPolicyRequiresTag(policyData: PolicyData, requiresTag: boolean) {
+/** extraPolicyUpdate folds a caller's same-save requiresCategory change into this action's single violation recompute. */
+function setPolicyRequiresTag(policyData: PolicyData, requiresTag: boolean, extraPolicyUpdate: Partial<Policy> = {}) {
     const policyID = policyData.policy?.id;
 
     const policyOptimisticData: Partial<Policy> = {
+        ...extraPolicyUpdate,
         requiresTag,
         // A manual toggle is explicit, so any pending switch-level restore intent is no longer needed.
         pendingRequiresTagRestore: null,
@@ -1266,7 +1268,7 @@ function setPolicyTagsRequired(policyData: PolicyData, requiresTag: boolean, tag
  * key, the last request would overwrite the others — e.g. requiring Region while clearing Department would drop the
  * Missing Region violation the first half just added.
  */
-function setPolicyTagLevelsRequired(policyData: PolicyData, requiredByOrderWeight: Record<number, boolean>) {
+function setPolicyTagLevelsRequired(policyData: PolicyData, requiredByOrderWeight: Record<number, boolean>, extraPolicyUpdate: Partial<Policy> = {}) {
     const policyID = policyData.policy?.id;
     if (!policyID || !policyData.tags) {
         return;
@@ -1282,6 +1284,11 @@ function setPolicyTagLevelsRequired(policyData: PolicyData, requiredByOrderWeigh
         combinedTagsUpdate[tagList.name] = {required: requiredByOrderWeight[tagList.orderWeight]};
     }
 
+    // ViolationsUtils gates every tag violation behind policy.requiresTag, so mirror what the backend derives from the
+    // levels instead of waiting for the next policy refresh.
+    const requiresTag = Object.values(policyData.tags).some((tagList) => requiredByOrderWeight[tagList.orderWeight] ?? !!tagList.required);
+    const policyUpdate: Partial<Policy> = {...extraPolicyUpdate, requiresTag};
+
     const buildOnyxData = (tagLists: Array<PolicyTagLists[string]>, isRequired: boolean, shouldRecomputeViolations: boolean) => {
         const optimisticValue: Record<string, Partial<PolicyTagLists[string]>> = {};
         const successValue: Record<string, Partial<PolicyTagLists[string]>> = {};
@@ -1301,14 +1308,21 @@ function setPolicyTagLevelsRequired(policyData: PolicyData, requiredByOrderWeigh
             };
         }
 
-        const onyxData: OnyxData<typeof ONYXKEYS.COLLECTION.POLICY_TAGS> = {
+        const onyxData: OnyxData<typeof ONYXKEYS.COLLECTION.POLICY | typeof ONYXKEYS.COLLECTION.POLICY_TAGS> = {
             optimisticData: [{onyxMethod: Onyx.METHOD.MERGE, key: `${ONYXKEYS.COLLECTION.POLICY_TAGS}${policyID}`, value: optimisticValue}],
             successData: [{onyxMethod: Onyx.METHOD.MERGE, key: `${ONYXKEYS.COLLECTION.POLICY_TAGS}${policyID}`, value: successValue}],
             failureData: [{onyxMethod: Onyx.METHOD.MERGE, key: `${ONYXKEYS.COLLECTION.POLICY_TAGS}${policyID}`, value: failureValue}],
         };
 
+        // Only the request that owns the recompute writes the derived policy flags, so the other can't revert them.
         if (shouldRecomputeViolations) {
-            pushTransactionViolationsOnyxData(onyxData, policyData, {}, {}, combinedTagsUpdate);
+            onyxData.optimisticData?.push({onyxMethod: Onyx.METHOD.MERGE, key: `${ONYXKEYS.COLLECTION.POLICY}${policyID}`, value: policyUpdate});
+            onyxData.failureData?.push({
+                onyxMethod: Onyx.METHOD.MERGE,
+                key: `${ONYXKEYS.COLLECTION.POLICY}${policyID}`,
+                value: {requiresTag: policyData.policy?.requiresTag ?? false},
+            });
+            pushTransactionViolationsOnyxData(onyxData, policyData, policyUpdate, {}, combinedTagsUpdate);
         }
 
         return onyxData;

src/pages/workspace/rules/RulesRequireFieldsPage.tsx

@@ -57,7 +57,7 @@ function RulesRequireFieldsPage({
     const isCategoryFeatureDisabled = !policy?.areCategoriesEnabled;
     const isCategoryToggleDisabled = isCategoryFeatureDisabled || !hasEnabledCategories || isConnectedToAccounting;
 
-    const hasEnabledTags = hasEnabledOptions(Object.values(policyTags ?? {}).flatMap(({tags}) => Object.values(tags)));
+    const hasEnabledTags = hasEnabledOptions(Object.values(policyTags ?? {}).flatMap(({tags}) => Object.values(tags ?? {})));
     const isTagFeatureDisabled = !policy?.areTagsEnabled;
     // A connection owns the tag lists, not whether an expense must carry one, so unlike Categories it doesn't lock this.
     const isTagToggleDisabled = isTagFeatureDisabled || !hasEnabledTags;
@@ -107,16 +107,21 @@ function RulesRequireFieldsPage({
             return;
         }
 
-        if (categoryRequired !== initialCategoryRequired) {
-            setWorkspaceRequiresCategory(policyData, categoryRequired);
+        const hasCategoryChange = categoryRequired !== initialCategoryRequired;
+        const hasTagLevelChanges = hasPerLevelTagRequired && changedTagLevels.length > 0;
+        const hasSingleTagChange = !hasPerLevelTagRequired && tagRequired !== initialTagRequired;
+        const categoryUpdateForTagRecompute = hasCategoryChange ? {requiresCategory: categoryRequired} : {};
+
+        if (hasCategoryChange) {
+            // With a tag change in the same save, the tag action owns the one violation recompute and carries requiresCategory into it.
+            setWorkspaceRequiresCategory(policyData, categoryRequired, !hasTagLevelChanges && !hasSingleTagChange);
         }
 
-        if (hasPerLevelTagRequired) {
-            // All changed levels go in one call so violations are recomputed once from the combined end state. Every changed
-            // level has a pending value by definition, and it can only be the opposite of the saved one.
-            setPolicyTagLevelsRequired(policyData, Object.fromEntries(changedTagLevels.map((tagList) => [tagList.orderWeight, !tagList.required])));
-        } else if (tagRequired !== initialTagRequired) {
-            setPolicyRequiresTag(policyData, tagRequired);
+        if (hasTagLevelChanges) {
+            // One call for every changed level, so violations are recomputed once from the combined end state.
+            setPolicyTagLevelsRequired(policyData, Object.fromEntries(changedTagLevels.map((tagList) => [tagList.orderWeight, !tagList.required])), categoryUpdateForTagRecompute);
+        } else if (hasSingleTagChange) {
+            setPolicyRequiresTag(policyData, tagRequired, categoryUpdateForTagRecompute);
         }
 
         Navigation.setNavigationActionToMicrotaskQueue(Navigation.goBack);