From f8c84ea0f9e48e403bc602ee43dcf698283170e0 Mon Sep 17 00:00:00 2001 From: Dariusz Biela Date: Wed, 24 Sep 2025 15:17:16 +0200 Subject: [PATCH 1/2] refactor: isolates clearPolicyTagListErrors from Onyx.connect data --- src/libs/actions/Policy/Tag.ts | 10 +- .../tags/WorkspaceTagsSettingsPage.tsx | 2 +- .../workspace/tags/WorkspaceViewTagsPage.tsx | 2 +- tests/actions/PolicyTagTest.ts | 199 +++++++++++++++++- 4 files changed, 208 insertions(+), 5 deletions(-) diff --git a/src/libs/actions/Policy/Tag.ts b/src/libs/actions/Policy/Tag.ts index e16f03f7e3c1..8691f07d603a 100644 --- a/src/libs/actions/Policy/Tag.ts +++ b/src/libs/actions/Policy/Tag.ts @@ -506,8 +506,14 @@ function clearPolicyTagListErrorField(policyID: string, tagListIndex: number, er }); } -function clearPolicyTagListErrors(policyID: string, tagListIndex: number) { - const policyTag = PolicyUtils.getTagLists(allPolicyTags?.[`${ONYXKEYS.COLLECTION.POLICY_TAGS}${policyID}`] ?? {})?.at(tagListIndex); +type ClearPolicyTagListErrorsProps = { + policyID: string; + tagListIndex: number; + policyTags: OnyxEntry; +}; + +function clearPolicyTagListErrors({policyID, tagListIndex, policyTags}: ClearPolicyTagListErrorsProps) { + const policyTag = PolicyUtils.getTagLists(policyTags ?? {})?.at(tagListIndex); if (!policyTag) { return; diff --git a/src/pages/workspace/tags/WorkspaceTagsSettingsPage.tsx b/src/pages/workspace/tags/WorkspaceTagsSettingsPage.tsx index 5e0d1fe9c7ae..2e8125481120 100644 --- a/src/pages/workspace/tags/WorkspaceTagsSettingsPage.tsx +++ b/src/pages/workspace/tags/WorkspaceTagsSettingsPage.tsx @@ -73,7 +73,7 @@ function WorkspaceTagsSettingsPage({route}: WorkspaceTagsSettingsPageProps) { {!isMultiLevelTags && ( clearPolicyTagListErrors(policyID, policyTagLists.at(0)?.orderWeight ?? 0)} + onClose={() => clearPolicyTagListErrors({policyID, tagListIndex: policyTagLists.at(0)?.orderWeight ?? 0, policyTags})} pendingAction={policyTags?.[policyTagLists.at(0)?.name ?? '']?.pendingAction} errorRowStyles={styles.mh5} > diff --git a/src/pages/workspace/tags/WorkspaceViewTagsPage.tsx b/src/pages/workspace/tags/WorkspaceViewTagsPage.tsx index 37bc66e96b14..2517ed1e52bd 100644 --- a/src/pages/workspace/tags/WorkspaceViewTagsPage.tsx +++ b/src/pages/workspace/tags/WorkspaceViewTagsPage.tsx @@ -386,7 +386,7 @@ function WorkspaceViewTagsPage({route}: WorkspaceViewTagsProps) { )} clearPolicyTagListErrors(policyID, currentPolicyTag.orderWeight)} + onClose={() => clearPolicyTagListErrors({policyID, tagListIndex: currentPolicyTag.orderWeight, policyTags})} pendingAction={currentPolicyTag.pendingAction} errorRowStyles={styles.mh5} > diff --git a/tests/actions/PolicyTagTest.ts b/tests/actions/PolicyTagTest.ts index 0093cda73023..14ed8a6f5e27 100644 --- a/tests/actions/PolicyTagTest.ts +++ b/tests/actions/PolicyTagTest.ts @@ -2,7 +2,16 @@ import {renderHook, waitFor} from '@testing-library/react-native'; import Onyx from 'react-native-onyx'; import useOnyx from '@hooks/useOnyx'; import OnyxUpdateManager from '@libs/actions/OnyxUpdateManager'; -import {clearPolicyTagErrors, createPolicyTag, deletePolicyTags, renamePolicyTag, renamePolicyTagList, setPolicyRequiresTag, setWorkspaceTagEnabled} from '@libs/actions/Policy/Tag'; +import { + clearPolicyTagErrors, + clearPolicyTagListErrors, + createPolicyTag, + deletePolicyTags, + renamePolicyTag, + renamePolicyTagList, + setPolicyRequiresTag, + setWorkspaceTagEnabled, +} from '@libs/actions/Policy/Tag'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; import type {PolicyTagLists, PolicyTags} from '@src/types/onyx'; @@ -823,6 +832,194 @@ describe('actions/Policy', () => { }); }); + describe('ClearPolicyTagListErrors', () => { + it('should clear errors for a tag list', async () => { + const fakePolicy = createRandomPolicy(0); + const tagListName = 'Test tag list'; + const fakePolicyTags = createRandomPolicyTags(tagListName, 2); + + // Add errors to the tag list + fakePolicyTags[tagListName] = { + ...fakePolicyTags[tagListName], + errors: {field1: 'Error on tag list'}, + }; + + await Onyx.set(`${ONYXKEYS.COLLECTION.POLICY_TAGS}${fakePolicy.id}`, fakePolicyTags); + + clearPolicyTagListErrors({policyID: fakePolicy.id, tagListIndex: 0, policyTags: fakePolicyTags}); + await waitForBatchedUpdates(); + + let updatedPolicyTags: PolicyTagLists | undefined; + await TestHelper.getOnyxData({ + key: `${ONYXKEYS.COLLECTION.POLICY_TAGS}${fakePolicy.id}`, + callback: (val) => (updatedPolicyTags = val), + }); + + // Verify that errors are cleared from the tag list + expect(updatedPolicyTags?.[tagListName]).toBeDefined(); + expect(updatedPolicyTags?.[tagListName]?.errors).toBeUndefined(); + // Other properties should remain unchanged + expect(updatedPolicyTags?.[tagListName]?.name).toBe(tagListName); + expect(updatedPolicyTags?.[tagListName]?.orderWeight).toBe(0); + expect(Object.keys(updatedPolicyTags?.[tagListName]?.tags ?? {}).length).toBe(2); + }); + + it('should not modify Onyx data when tag list does not exist at given index', async () => { + const fakePolicy = createRandomPolicy(0); + const tagListName = 'Test tag list'; + const fakePolicyTags = createRandomPolicyTags(tagListName, 2); + + await Onyx.set(`${ONYXKEYS.COLLECTION.POLICY_TAGS}${fakePolicy.id}`, fakePolicyTags); + + // Try to clear errors for a non-existent tag list (invalid index) + clearPolicyTagListErrors({policyID: fakePolicy.id, tagListIndex: 99, policyTags: fakePolicyTags}); + await waitForBatchedUpdates(); + + let updatedPolicyTags: PolicyTagLists | undefined; + await TestHelper.getOnyxData({ + key: `${ONYXKEYS.COLLECTION.POLICY_TAGS}${fakePolicy.id}`, + callback: (val) => (updatedPolicyTags = val), + }); + + // The policy tags should remain unchanged + expect(updatedPolicyTags).toEqual(fakePolicyTags); + }); + + it('should not modify Onyx data when tag list name is empty', async () => { + const fakePolicy = createRandomPolicy(0); + const tagListName = 'Test tag list'; + const fakePolicyTags = createRandomPolicyTags(tagListName, 2); + + // Remove the name property from the tag list + fakePolicyTags[tagListName] = { + ...fakePolicyTags[tagListName], + name: '', + }; + + await Onyx.set(`${ONYXKEYS.COLLECTION.POLICY_TAGS}${fakePolicy.id}`, fakePolicyTags); + + clearPolicyTagListErrors({policyID: fakePolicy.id, tagListIndex: 0, policyTags: fakePolicyTags}); + await waitForBatchedUpdates(); + + let updatedPolicyTags: PolicyTagLists | undefined; + await TestHelper.getOnyxData({ + key: `${ONYXKEYS.COLLECTION.POLICY_TAGS}${fakePolicy.id}`, + callback: (val) => (updatedPolicyTags = val), + }); + + // The policy tags should remain unchanged + expect(updatedPolicyTags).toEqual(fakePolicyTags); + }); + + it('should clear multiple errors from a tag list', async () => { + const fakePolicy = createRandomPolicy(0); + const tagListName = 'Test tag list'; + const fakePolicyTags = createRandomPolicyTags(tagListName, 3); + + // Add multiple errors to the tag list + fakePolicyTags[tagListName] = { + ...fakePolicyTags[tagListName], + errors: { + field1: 'Error 1', + field2: 'Error 2', + field3: 'Error 3', + }, + }; + + await Onyx.set(`${ONYXKEYS.COLLECTION.POLICY_TAGS}${fakePolicy.id}`, fakePolicyTags); + + clearPolicyTagListErrors({policyID: fakePolicy.id, tagListIndex: 0, policyTags: fakePolicyTags}); + await waitForBatchedUpdates(); + + let updatedPolicyTags: PolicyTagLists | undefined; + await TestHelper.getOnyxData({ + key: `${ONYXKEYS.COLLECTION.POLICY_TAGS}${fakePolicy.id}`, + callback: (val) => (updatedPolicyTags = val), + }); + + // Verify that all errors are cleared + expect(updatedPolicyTags?.[tagListName]?.errors).toBeUndefined(); + }); + + it('should handle multiple tag lists correctly', async () => { + const fakePolicy = createRandomPolicy(0); + const tagListName1 = 'Tag list 1'; + const tagListName2 = 'Tag list 2'; + + const fakePolicyTags: PolicyTagLists = { + [tagListName1]: { + name: tagListName1, + orderWeight: 0, + required: false, + tags: { + tag1: {name: 'tag1', enabled: true}, + }, + errors: {field: 'Error on list 1'}, + }, + [tagListName2]: { + name: tagListName2, + orderWeight: 1, + required: false, + tags: { + tag2: {name: 'tag2', enabled: true}, + }, + errors: {field: 'Error on list 2'}, + }, + }; + + await Onyx.set(`${ONYXKEYS.COLLECTION.POLICY_TAGS}${fakePolicy.id}`, fakePolicyTags); + + // Clear errors only for the second tag list + clearPolicyTagListErrors({policyID: fakePolicy.id, tagListIndex: 1, policyTags: fakePolicyTags}); + await waitForBatchedUpdates(); + + let updatedPolicyTags: PolicyTagLists | undefined; + await TestHelper.getOnyxData({ + key: `${ONYXKEYS.COLLECTION.POLICY_TAGS}${fakePolicy.id}`, + callback: (val) => (updatedPolicyTags = val), + }); + + // Verify that only the second list has errors cleared + expect(updatedPolicyTags?.[tagListName1]?.errors).toEqual({field: 'Error on list 1'}); + expect(updatedPolicyTags?.[tagListName2]?.errors).toBeUndefined(); + }); + + it('should work with data from useOnyx hook', async () => { + const fakePolicy = createRandomPolicy(0); + const tagListName = 'Test tag list'; + const fakePolicyTags = createRandomPolicyTags(tagListName, 2); + + // Add errors to the tag list + fakePolicyTags[tagListName] = { + ...fakePolicyTags[tagListName], + errors: {field: 'Test error'}, + }; + + await Onyx.set(`${ONYXKEYS.COLLECTION.POLICY_TAGS}${fakePolicy.id}`, fakePolicyTags); + + const {result} = renderHook(() => useOnyx(`${ONYXKEYS.COLLECTION.POLICY_TAGS}${fakePolicy.id}`)); + + await waitFor(() => { + expect(result.current[0]).toBeDefined(); + }); + + clearPolicyTagListErrors({policyID: fakePolicy.id, tagListIndex: 0, policyTags: result.current[0]}); + await waitForBatchedUpdates(); + + // Verify errors are cleared + let updatedPolicyTags: PolicyTagLists | undefined; + await TestHelper.getOnyxData({ + key: `${ONYXKEYS.COLLECTION.POLICY_TAGS}${fakePolicy.id}`, + callback: (val) => (updatedPolicyTags = val), + }); + + expect(updatedPolicyTags?.[tagListName]).toBeDefined(); + expect(updatedPolicyTags?.[tagListName]?.errors).toBeUndefined(); + expect(updatedPolicyTags?.[tagListName]?.name).toBe(tagListName); + expect(updatedPolicyTags?.[tagListName]?.orderWeight).toBe(0); + }); + }); + describe('ClearPolicyTagErrors', () => { it('should clear errors for a tag', async () => { const fakePolicy = createRandomPolicy(0); From d4405acb66c02a12f134b772226bd95c0b7410ed Mon Sep 17 00:00:00 2001 From: Dariusz Biela Date: Mon, 6 Oct 2025 13:38:56 +0200 Subject: [PATCH 2/2] refactor: refactors test comments for ClearPolicyTagListErrors in PolicyTagTest.ts --- tests/actions/PolicyTagTest.ts | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/tests/actions/PolicyTagTest.ts b/tests/actions/PolicyTagTest.ts index 14ed8a6f5e27..8e7043ce2e94 100644 --- a/tests/actions/PolicyTagTest.ts +++ b/tests/actions/PolicyTagTest.ts @@ -834,11 +834,11 @@ describe('actions/Policy', () => { describe('ClearPolicyTagListErrors', () => { it('should clear errors for a tag list', async () => { + // Given a policy with a tag list that has errors const fakePolicy = createRandomPolicy(0); const tagListName = 'Test tag list'; const fakePolicyTags = createRandomPolicyTags(tagListName, 2); - // Add errors to the tag list fakePolicyTags[tagListName] = { ...fakePolicyTags[tagListName], errors: {field1: 'Error on tag list'}, @@ -846,6 +846,7 @@ describe('actions/Policy', () => { await Onyx.set(`${ONYXKEYS.COLLECTION.POLICY_TAGS}${fakePolicy.id}`, fakePolicyTags); + // When clearing the errors from the tag list clearPolicyTagListErrors({policyID: fakePolicy.id, tagListIndex: 0, policyTags: fakePolicyTags}); await waitForBatchedUpdates(); @@ -855,23 +856,23 @@ describe('actions/Policy', () => { callback: (val) => (updatedPolicyTags = val), }); - // Verify that errors are cleared from the tag list + // Then the errors should be cleared while other properties remain unchanged expect(updatedPolicyTags?.[tagListName]).toBeDefined(); expect(updatedPolicyTags?.[tagListName]?.errors).toBeUndefined(); - // Other properties should remain unchanged expect(updatedPolicyTags?.[tagListName]?.name).toBe(tagListName); expect(updatedPolicyTags?.[tagListName]?.orderWeight).toBe(0); expect(Object.keys(updatedPolicyTags?.[tagListName]?.tags ?? {}).length).toBe(2); }); it('should not modify Onyx data when tag list does not exist at given index', async () => { + // Given a policy with a tag list const fakePolicy = createRandomPolicy(0); const tagListName = 'Test tag list'; const fakePolicyTags = createRandomPolicyTags(tagListName, 2); await Onyx.set(`${ONYXKEYS.COLLECTION.POLICY_TAGS}${fakePolicy.id}`, fakePolicyTags); - // Try to clear errors for a non-existent tag list (invalid index) + // When attempting to clear errors for a non-existent tag list using an invalid index clearPolicyTagListErrors({policyID: fakePolicy.id, tagListIndex: 99, policyTags: fakePolicyTags}); await waitForBatchedUpdates(); @@ -881,16 +882,16 @@ describe('actions/Policy', () => { callback: (val) => (updatedPolicyTags = val), }); - // The policy tags should remain unchanged + // Then the policy tags should remain unchanged because the index is invalid expect(updatedPolicyTags).toEqual(fakePolicyTags); }); it('should not modify Onyx data when tag list name is empty', async () => { + // Given a policy with a tag list that has an empty name const fakePolicy = createRandomPolicy(0); const tagListName = 'Test tag list'; const fakePolicyTags = createRandomPolicyTags(tagListName, 2); - // Remove the name property from the tag list fakePolicyTags[tagListName] = { ...fakePolicyTags[tagListName], name: '', @@ -898,6 +899,7 @@ describe('actions/Policy', () => { await Onyx.set(`${ONYXKEYS.COLLECTION.POLICY_TAGS}${fakePolicy.id}`, fakePolicyTags); + // When attempting to clear errors for the tag list with empty name clearPolicyTagListErrors({policyID: fakePolicy.id, tagListIndex: 0, policyTags: fakePolicyTags}); await waitForBatchedUpdates(); @@ -907,16 +909,16 @@ describe('actions/Policy', () => { callback: (val) => (updatedPolicyTags = val), }); - // The policy tags should remain unchanged + // Then the policy tags should remain unchanged because the tag list name is empty expect(updatedPolicyTags).toEqual(fakePolicyTags); }); it('should clear multiple errors from a tag list', async () => { + // Given a policy with a tag list that has multiple errors const fakePolicy = createRandomPolicy(0); const tagListName = 'Test tag list'; const fakePolicyTags = createRandomPolicyTags(tagListName, 3); - // Add multiple errors to the tag list fakePolicyTags[tagListName] = { ...fakePolicyTags[tagListName], errors: { @@ -928,6 +930,7 @@ describe('actions/Policy', () => { await Onyx.set(`${ONYXKEYS.COLLECTION.POLICY_TAGS}${fakePolicy.id}`, fakePolicyTags); + // When clearing errors from the tag list clearPolicyTagListErrors({policyID: fakePolicy.id, tagListIndex: 0, policyTags: fakePolicyTags}); await waitForBatchedUpdates(); @@ -937,11 +940,12 @@ describe('actions/Policy', () => { callback: (val) => (updatedPolicyTags = val), }); - // Verify that all errors are cleared + // Then all errors should be cleared from the tag list expect(updatedPolicyTags?.[tagListName]?.errors).toBeUndefined(); }); it('should handle multiple tag lists correctly', async () => { + // Given a policy with multiple tag lists that both have errors const fakePolicy = createRandomPolicy(0); const tagListName1 = 'Tag list 1'; const tagListName2 = 'Tag list 2'; @@ -969,7 +973,7 @@ describe('actions/Policy', () => { await Onyx.set(`${ONYXKEYS.COLLECTION.POLICY_TAGS}${fakePolicy.id}`, fakePolicyTags); - // Clear errors only for the second tag list + // When clearing errors only for the second tag list clearPolicyTagListErrors({policyID: fakePolicy.id, tagListIndex: 1, policyTags: fakePolicyTags}); await waitForBatchedUpdates(); @@ -979,17 +983,17 @@ describe('actions/Policy', () => { callback: (val) => (updatedPolicyTags = val), }); - // Verify that only the second list has errors cleared + // Then only the second list should have errors cleared while the first list keeps its errors expect(updatedPolicyTags?.[tagListName1]?.errors).toEqual({field: 'Error on list 1'}); expect(updatedPolicyTags?.[tagListName2]?.errors).toBeUndefined(); }); it('should work with data from useOnyx hook', async () => { + // Given a policy with a tag list that has errors and is accessed via useOnyx hook const fakePolicy = createRandomPolicy(0); const tagListName = 'Test tag list'; const fakePolicyTags = createRandomPolicyTags(tagListName, 2); - // Add errors to the tag list fakePolicyTags[tagListName] = { ...fakePolicyTags[tagListName], errors: {field: 'Test error'}, @@ -1003,16 +1007,17 @@ describe('actions/Policy', () => { expect(result.current[0]).toBeDefined(); }); + // When clearing errors using data from the useOnyx hook clearPolicyTagListErrors({policyID: fakePolicy.id, tagListIndex: 0, policyTags: result.current[0]}); await waitForBatchedUpdates(); - // Verify errors are cleared let updatedPolicyTags: PolicyTagLists | undefined; await TestHelper.getOnyxData({ key: `${ONYXKEYS.COLLECTION.POLICY_TAGS}${fakePolicy.id}`, callback: (val) => (updatedPolicyTags = val), }); + // Then the errors should be cleared and other properties should remain unchanged expect(updatedPolicyTags?.[tagListName]).toBeDefined(); expect(updatedPolicyTags?.[tagListName]?.errors).toBeUndefined(); expect(updatedPolicyTags?.[tagListName]?.name).toBe(tagListName);