From 2ded191f349cc28ec853dd1e8708255ba050b9d0 Mon Sep 17 00:00:00 2001 From: Dariusz Biela Date: Wed, 24 Sep 2025 13:57:15 +0200 Subject: [PATCH 1/2] refactor: isolates clearPolicyTagListErrorField from Onyx.connect data --- src/libs/actions/Policy/Tag.ts | 11 +- .../workspace/tags/WorkspaceViewTagsPage.tsx | 2 +- tests/actions/PolicyTagTest.ts | 123 +++++++++++++++++- 3 files changed, 132 insertions(+), 4 deletions(-) diff --git a/src/libs/actions/Policy/Tag.ts b/src/libs/actions/Policy/Tag.ts index 0c6ffbfaabcc..f08df7d4de12 100644 --- a/src/libs/actions/Policy/Tag.ts +++ b/src/libs/actions/Policy/Tag.ts @@ -479,8 +479,15 @@ function clearPolicyTagErrors(policyID: string, tagName: string, tagListIndex: n }); } -function clearPolicyTagListErrorField(policyID: string, tagListIndex: number, errorField: string) { - const policyTag = PolicyUtils.getTagLists(allPolicyTags?.[`${ONYXKEYS.COLLECTION.POLICY_TAGS}${policyID}`] ?? {})?.at(tagListIndex); +type ClearPolicyTagListErrorFieldProps = { + policyID: string; + tagListIndex: number; + errorField: string; + policyTags: OnyxEntry; +}; + +function clearPolicyTagListErrorField({policyID, tagListIndex, errorField, policyTags}: ClearPolicyTagListErrorFieldProps) { + const policyTag = PolicyUtils.getTagLists(policyTags ?? {})?.at(tagListIndex); if (!policyTag) { return; diff --git a/src/pages/workspace/tags/WorkspaceViewTagsPage.tsx b/src/pages/workspace/tags/WorkspaceViewTagsPage.tsx index 05d7191bdede..566fab2bbeff 100644 --- a/src/pages/workspace/tags/WorkspaceViewTagsPage.tsx +++ b/src/pages/workspace/tags/WorkspaceViewTagsPage.tsx @@ -379,7 +379,7 @@ function WorkspaceViewTagsPage({route}: WorkspaceViewTagsProps) { }} pendingAction={currentPolicyTag.pendingFields?.required} errors={currentPolicyTag?.errorFields?.required ?? undefined} - onCloseError={() => clearPolicyTagListErrorField(policyID, route.params.orderWeight, 'required')} + onCloseError={() => clearPolicyTagListErrorField({policyID, tagListIndex: route.params.orderWeight, errorField: 'required', policyTags})} disabled={!currentPolicyTag?.required && !Object.values(currentPolicyTag?.tags ?? {}).some((tag) => tag.enabled)} showLockIcon={isMakingLastRequiredTagListOptional(policy, policyTags, [currentPolicyTag])} /> diff --git a/tests/actions/PolicyTagTest.ts b/tests/actions/PolicyTagTest.ts index f590f3be27b8..9372d4297c92 100644 --- a/tests/actions/PolicyTagTest.ts +++ b/tests/actions/PolicyTagTest.ts @@ -1,6 +1,8 @@ +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 {createPolicyTag, deletePolicyTags, renamePolicyTag, renamePolicyTagList, setPolicyRequiresTag, setWorkspaceTagEnabled} from '@libs/actions/Policy/Tag'; +import {clearPolicyTagListErrorField, 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'; @@ -743,4 +745,123 @@ describe('actions/Policy', () => { ); }); }); + + describe('ClearPolicyTagListErrorField', () => { + it('should clear specific error field from tag list', async () => { + const fakePolicy = createRandomPolicy(0); + const tagListName = 'Test tag list'; + const fakePolicyTags = createRandomPolicyTags(tagListName, 2); + + fakePolicyTags[tagListName] = { + ...fakePolicyTags[tagListName], + errorFields: { + name: {genericError: 'Name error'}, + required: {genericError: 'Required error'}, + maxTagsSelected: {genericError: 'Max tags error'}, + }, + }; + + await Onyx.set(`${ONYXKEYS.COLLECTION.POLICY_TAGS}${fakePolicy.id}`, fakePolicyTags); + + clearPolicyTagListErrorField({policyID: fakePolicy.id, tagListIndex: 0, errorField: 'required', policyTags: fakePolicyTags}); + await waitForBatchedUpdates(); + + 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].errorFields?.required).toBeUndefined(); + expect(updatedPolicyTags?.[tagListName].errorFields?.name).toEqual({genericError: 'Name error'}); + expect(updatedPolicyTags?.[tagListName].errorFields?.maxTagsSelected).toEqual({genericError: 'Max tags error'}); + }); + + it('should not modify Onyx data when tag list does not exist', async () => { + const fakePolicy = createRandomPolicy(0); + const fakePolicyTags = {}; + + await Onyx.set(`${ONYXKEYS.COLLECTION.POLICY_TAGS}${fakePolicy.id}`, fakePolicyTags); + + expect(() => { + clearPolicyTagListErrorField({policyID: fakePolicy.id, tagListIndex: 0, errorField: 'required', policyTags: fakePolicyTags}); + }).not.toThrow(); + + let updatedPolicyTags: PolicyTagLists | undefined; + await TestHelper.getOnyxData({ + key: `${ONYXKEYS.COLLECTION.POLICY_TAGS}${fakePolicy.id}`, + callback: (val) => (updatedPolicyTags = val), + }); + + expect(updatedPolicyTags).toEqual(fakePolicyTags); + }); + + it('should not modify Onyx data when tag list has no name', async () => { + const fakePolicy = createRandomPolicy(0); + const tagListName = 'Test tag list'; + const fakePolicyTags = createRandomPolicyTags(tagListName, 1); + + fakePolicyTags[tagListName] = { + ...fakePolicyTags[tagListName], + name: '', + errorFields: { + required: {genericError: 'This error should not be cleared'}, + name: {genericError: 'This error should also remain'}, + }, + }; + + await Onyx.set(`${ONYXKEYS.COLLECTION.POLICY_TAGS}${fakePolicy.id}`, fakePolicyTags); + + expect(() => { + clearPolicyTagListErrorField({policyID: fakePolicy.id, tagListIndex: 0, errorField: 'required', policyTags: fakePolicyTags}); + }).not.toThrow(); + + await waitForBatchedUpdates(); + + let updatedPolicyTags: PolicyTagLists | undefined; + await TestHelper.getOnyxData({ + key: `${ONYXKEYS.COLLECTION.POLICY_TAGS}${fakePolicy.id}`, + callback: (val) => (updatedPolicyTags = val), + }); + + expect(updatedPolicyTags?.[tagListName].errorFields?.required).toEqual({genericError: 'This error should not be cleared'}); + expect(updatedPolicyTags?.[tagListName].errorFields?.name).toEqual({genericError: 'This error should also remain'}); + expect(updatedPolicyTags?.[tagListName].name).toBe(''); + }); + + it('should work with data from useOnyx hook', async () => { + const fakePolicy = createRandomPolicy(0); + const tagListName = 'Test tag list'; + const fakePolicyTags = createRandomPolicyTags(tagListName, 1); + + fakePolicyTags[tagListName] = { + ...fakePolicyTags[tagListName], + errorFields: { + required: {genericError: 'Required field error'}, + name: {genericError: 'Name field 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(); + }); + + clearPolicyTagListErrorField({policyID: fakePolicy.id, tagListIndex: 0, errorField: 'name', policyTags: result.current[0]}); + await waitForBatchedUpdates(); + + let updatedPolicyTags: PolicyTagLists | undefined; + await TestHelper.getOnyxData({ + key: `${ONYXKEYS.COLLECTION.POLICY_TAGS}${fakePolicy.id}`, + callback: (val) => (updatedPolicyTags = val), + }); + + expect(updatedPolicyTags?.[tagListName].errorFields?.name).toBeUndefined(); + expect(updatedPolicyTags?.[tagListName].errorFields?.required).toEqual({genericError: 'Required field error'}); + }); + }); }); From ba2f622c011d547bf42b74c84691a9285fa04522 Mon Sep 17 00:00:00 2001 From: Dariusz Biela Date: Mon, 6 Oct 2025 13:44:05 +0200 Subject: [PATCH 2/2] refactor: refactors test comments for ClearPolicyTagListErrorField in PolicyTagTest.ts --- tests/actions/PolicyTagTest.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/actions/PolicyTagTest.ts b/tests/actions/PolicyTagTest.ts index d631992dd686..f8c8778cf4a8 100644 --- a/tests/actions/PolicyTagTest.ts +++ b/tests/actions/PolicyTagTest.ts @@ -958,6 +958,7 @@ describe('actions/Policy', () => { describe('ClearPolicyTagListErrorField', () => { it('should clear specific error field from tag list', async () => { + // Given a policy with a tag list that has multiple error fields const fakePolicy = createRandomPolicy(0); const tagListName = 'Test tag list'; const fakePolicyTags = createRandomPolicyTags(tagListName, 2); @@ -973,6 +974,7 @@ describe('actions/Policy', () => { await Onyx.set(`${ONYXKEYS.COLLECTION.POLICY_TAGS}${fakePolicy.id}`, fakePolicyTags); + // When clearing only the 'required' error field from the tag list clearPolicyTagListErrorField({policyID: fakePolicy.id, tagListIndex: 0, errorField: 'required', policyTags: fakePolicyTags}); await waitForBatchedUpdates(); @@ -982,6 +984,7 @@ describe('actions/Policy', () => { callback: (val) => (updatedPolicyTags = val), }); + // Then only the 'required' error field should be cleared while other error fields remain expect(updatedPolicyTags?.[tagListName]).toBeDefined(); expect(updatedPolicyTags?.[tagListName].errorFields?.required).toBeUndefined(); expect(updatedPolicyTags?.[tagListName].errorFields?.name).toEqual({genericError: 'Name error'}); @@ -989,11 +992,13 @@ describe('actions/Policy', () => { }); it('should not modify Onyx data when tag list does not exist', async () => { + // Given a policy with no tag lists const fakePolicy = createRandomPolicy(0); const fakePolicyTags = {}; await Onyx.set(`${ONYXKEYS.COLLECTION.POLICY_TAGS}${fakePolicy.id}`, fakePolicyTags); + // When attempting to clear an error field from a non-existent tag list expect(() => { clearPolicyTagListErrorField({policyID: fakePolicy.id, tagListIndex: 0, errorField: 'required', policyTags: fakePolicyTags}); }).not.toThrow(); @@ -1004,10 +1009,12 @@ describe('actions/Policy', () => { callback: (val) => (updatedPolicyTags = val), }); + // Then the policy tags should remain unchanged because the tag list does not exist expect(updatedPolicyTags).toEqual(fakePolicyTags); }); it('should not modify Onyx data when tag list has no name', async () => { + // Given a policy with a tag list that has an empty name and error fields const fakePolicy = createRandomPolicy(0); const tagListName = 'Test tag list'; const fakePolicyTags = createRandomPolicyTags(tagListName, 1); @@ -1023,6 +1030,7 @@ describe('actions/Policy', () => { await Onyx.set(`${ONYXKEYS.COLLECTION.POLICY_TAGS}${fakePolicy.id}`, fakePolicyTags); + // When attempting to clear an error field from a tag list with no name expect(() => { clearPolicyTagListErrorField({policyID: fakePolicy.id, tagListIndex: 0, errorField: 'required', policyTags: fakePolicyTags}); }).not.toThrow(); @@ -1035,12 +1043,14 @@ describe('actions/Policy', () => { callback: (val) => (updatedPolicyTags = val), }); + // Then the error fields should remain unchanged because the tag list name is empty expect(updatedPolicyTags?.[tagListName].errorFields?.required).toEqual({genericError: 'This error should not be cleared'}); expect(updatedPolicyTags?.[tagListName].errorFields?.name).toEqual({genericError: 'This error should also remain'}); expect(updatedPolicyTags?.[tagListName].name).toBe(''); }); it('should work with data from useOnyx hook', async () => { + // Given a policy with a tag list that has error fields and is accessed via useOnyx hook const fakePolicy = createRandomPolicy(0); const tagListName = 'Test tag list'; const fakePolicyTags = createRandomPolicyTags(tagListName, 1); @@ -1061,6 +1071,7 @@ describe('actions/Policy', () => { expect(result.current[0]).toBeDefined(); }); + // When clearing the 'name' error field using data from the useOnyx hook clearPolicyTagListErrorField({policyID: fakePolicy.id, tagListIndex: 0, errorField: 'name', policyTags: result.current[0]}); await waitForBatchedUpdates(); @@ -1070,6 +1081,7 @@ describe('actions/Policy', () => { callback: (val) => (updatedPolicyTags = val), }); + // Then only the 'name' error field should be cleared while the 'required' error field remains expect(updatedPolicyTags?.[tagListName].errorFields?.name).toBeUndefined(); expect(updatedPolicyTags?.[tagListName].errorFields?.required).toEqual({genericError: 'Required field error'}); });