Merge pull request #38881 from FitseTLT/fix-unordered-tags-on-violati…

…on-bug
Expensify · Mar 29, 2024 · d6a36d3 · d6a36d3
2 parents ff794e7 + c6343c8
commit d6a36d3
Show file tree

Hide file tree

Showing 5 changed files with 23 additions and 12 deletions.
diff --git a/src/libs/ModifiedExpenseMessage.ts b/src/libs/ModifiedExpenseMessage.ts
@@ -193,9 +193,10 @@ function getForReportAction(reportID: string | undefined, reportAction: OnyxEntr
         const splittedTag = TransactionUtils.getTagArrayFromName(transactionTag);
         const splittedOldTag = TransactionUtils.getTagArrayFromName(oldTransactionTag);
         const localizedTagListName = Localize.translateLocal('common.tag');
+        const sortedTagKeys = PolicyUtils.getSortedTagKeys(policyTags);
 
-        Object.keys(policyTags).forEach((policyTagKey, index) => {
-            const policyTagListName = PolicyUtils.getTagListName(policyTags, index) || localizedTagListName;
+        sortedTagKeys.forEach((policyTagKey, index) => {
+            const policyTagListName = policyTags[policyTagKey].name || localizedTagListName;
 
             const newTag = splittedTag[index] ?? '';
             const oldTag = splittedOldTag[index] ?? '';

diff --git a/src/libs/PolicyUtils.ts b/src/libs/PolicyUtils.ts
@@ -170,6 +170,14 @@ function getIneligibleInvitees(policyMembers: OnyxEntry<PolicyMembers>, personal
     return memberEmailsToExclude;
 }
 
+function getSortedTagKeys(policyTagList: OnyxEntry<PolicyTagList>): Array<keyof PolicyTagList> {
+    if (isEmptyObject(policyTagList)) {
+        return [];
+    }
+
+    return Object.keys(policyTagList).sort((key1, key2) => policyTagList[key1].orderWeight - policyTagList[key2].orderWeight);
+}
+
 /**
  * Gets a tag name of policy tags based on a tag index.
  */
@@ -178,7 +186,7 @@ function getTagListName(policyTagList: OnyxEntry<PolicyTagList>, tagIndex: numbe
         return '';
     }
 
-    const policyTagKeys = Object.keys(policyTagList ?? {});
+    const policyTagKeys = getSortedTagKeys(policyTagList ?? {});
     const policyTagKey = policyTagKeys[tagIndex] ?? '';
 
     return policyTagList?.[policyTagKey]?.name ?? '';
@@ -316,6 +324,7 @@ export {
     getIneligibleInvitees,
     getTagLists,
     getTagListName,
+    getSortedTagKeys,
     canEditTaxRate,
     getTagList,
     getCleanedTagName,

diff --git a/src/libs/Violations/ViolationsUtils.ts b/src/libs/Violations/ViolationsUtils.ts
@@ -2,6 +2,7 @@ import reject from 'lodash/reject';
 import Onyx from 'react-native-onyx';
 import type {OnyxUpdate} from 'react-native-onyx';
 import type {Phrase, PhraseParameters} from '@libs/Localize';
+import {getSortedTagKeys} from '@libs/PolicyUtils';
 import CONST from '@src/CONST';
 import type {TranslationPaths} from '@src/languages/types';
 import ONYXKEYS from '@src/ONYXKEYS';
@@ -55,7 +56,7 @@ function getTagViolationsForMultiLevelTags(
     policyRequiresTags: boolean,
     policyTagList: PolicyTagList,
 ): TransactionViolation[] {
-    const policyTagKeys = Object.keys(policyTagList);
+    const policyTagKeys = getSortedTagKeys(policyTagList);
     const selectedTags = updatedTransaction.tag?.split(CONST.COLON) ?? [];
     let newTransactionViolations = [...transactionViolations];
     newTransactionViolations = newTransactionViolations.filter(

diff --git a/src/libs/actions/Policy.ts b/src/libs/actions/Policy.ts
@@ -2416,7 +2416,7 @@ function buildOptimisticPolicyRecentlyUsedTags(policyID?: string, transactionTag
     }
 
     const policyTags = allPolicyTags?.[`${ONYXKEYS.COLLECTION.POLICY_TAGS}${policyID}`] ?? {};
-    const policyTagKeys = Object.keys(policyTags);
+    const policyTagKeys = PolicyUtils.getSortedTagKeys(policyTags);
     const policyRecentlyUsedTags = allRecentlyUsedTags?.[`${ONYXKEYS.COLLECTION.POLICY_RECENTLY_USED_TAGS}${policyID}`] ?? {};
     const newOptimisticPolicyRecentlyUsedTags: RecentlyUsedTags = {};
 
@@ -2426,7 +2426,7 @@ function buildOptimisticPolicyRecentlyUsedTags(policyID?: string, transactionTag
         }
 
         const tagListKey = policyTagKeys[index];
-        newOptimisticPolicyRecentlyUsedTags[tagListKey] = [...new Set([...tag, ...(policyRecentlyUsedTags[tagListKey] ?? [])])];
+        newOptimisticPolicyRecentlyUsedTags[tagListKey] = [...new Set([tag, ...(policyRecentlyUsedTags[tagListKey] ?? [])])];
     });
 
     return newOptimisticPolicyRecentlyUsedTags;

diff --git a/tests/unit/ViolationUtilsTest.ts b/tests/unit/ViolationUtilsTest.ts
@@ -211,7 +211,7 @@ describe('getViolationsOnyxData', () => {
                         },
                     },
                     required: true,
-                    orderWeight: 1,
+                    orderWeight: 2,
                 },
                 Region: {
                     name: 'Region',
@@ -222,7 +222,7 @@ describe('getViolationsOnyxData', () => {
                         },
                     },
                     required: true,
-                    orderWeight: 2,
+                    orderWeight: 1,
                 },
                 Project: {
                     name: 'Project',
@@ -251,25 +251,25 @@ describe('getViolationsOnyxData', () => {
             expect(result.value).toEqual([someTagLevelsRequiredViolation]);
 
             // Test case where transaction has 1 tag
-            transaction.tag = 'Accounting';
+            transaction.tag = 'Africa';
             someTagLevelsRequiredViolation.data = {errorIndexes: [1, 2]};
             result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policyRequiresTags, policyTags, policyRequiresCategories, policyCategories);
             expect(result.value).toEqual([someTagLevelsRequiredViolation]);
 
             // Test case where transaction has 2 tags
-            transaction.tag = 'Accounting::Project1';
+            transaction.tag = 'Africa::Project1';
             someTagLevelsRequiredViolation.data = {errorIndexes: [1]};
             result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policyRequiresTags, policyTags, policyRequiresCategories, policyCategories);
             expect(result.value).toEqual([someTagLevelsRequiredViolation]);
 
             // Test case where transaction has all tags
-            transaction.tag = 'Accounting:Africa:Project1';
+            transaction.tag = 'Africa:Accounting:Project1';
             result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policyRequiresTags, policyTags, policyRequiresCategories, policyCategories);
             expect(result.value).toEqual([]);
         });
         it('should return tagOutOfPolicy when a tag is not enabled in the policy but is set in the transaction', () => {
             policyTags.Department.tags.Accounting.enabled = false;
-            transaction.tag = 'Accounting:Africa:Project1';
+            transaction.tag = 'Africa:Accounting:Project1';
             const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policyRequiresTags, policyTags, policyRequiresCategories, policyCategories);
             const violation = {...tagOutOfPolicyViolation, data: {tagName: 'Department'}};
             expect(result.value).toEqual([violation]);