From 7f5acd2bf12339b0bcb7f4f607b383e386238372 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Ska=C5=82ka?= Date: Mon, 13 Oct 2025 11:04:52 +0200 Subject: [PATCH 1/5] refactor: isolate enablePolicyTags from Onyx.connect data --- src/libs/actions/Policy/Tag.ts | 13 +- .../workspace/WorkspaceMoreFeaturesPage.tsx | 2 +- tests/actions/PolicyTagTest.ts | 143 ++++++++++++++++++ 3 files changed, 153 insertions(+), 5 deletions(-) diff --git a/src/libs/actions/Policy/Tag.ts b/src/libs/actions/Policy/Tag.ts index 69171310f946..ca2f531b32a6 100644 --- a/src/libs/actions/Policy/Tag.ts +++ b/src/libs/actions/Policy/Tag.ts @@ -653,7 +653,13 @@ function renamePolicyTag(policyID: string, policyTag: {oldName: string; newName: API.write(WRITE_COMMANDS.RENAME_POLICY_TAG, parameters, onyxData); } -function enablePolicyTags(policyID: string, enabled: boolean) { +type EnablePolicyTagsProps = { + policyID: string; + enabled: boolean; + policyTags?: PolicyTagLists; +}; + +function enablePolicyTags({policyID, enabled, policyTags}: EnablePolicyTagsProps) { const onyxData: OnyxData = { optimisticData: [ { @@ -692,8 +698,7 @@ function enablePolicyTags(policyID: string, enabled: boolean) { ], }; - const policyTagList = allPolicyTags?.[`${ONYXKEYS.COLLECTION.POLICY_TAGS}${policyID}`]; - if (!policyTagList) { + if (!policyTags) { const defaultTagList: PolicyTagLists = { Tag: { name: 'Tag', @@ -713,7 +718,7 @@ function enablePolicyTags(policyID: string, enabled: boolean) { value: null, }); } else if (!enabled) { - const policyTag = PolicyUtils.getTagLists(policyTagList).at(0); + const policyTag = PolicyUtils.getTagLists(policyTags).at(0); if (!policyTag) { return; diff --git a/src/pages/workspace/WorkspaceMoreFeaturesPage.tsx b/src/pages/workspace/WorkspaceMoreFeaturesPage.tsx index b19ab1838fae..9dc3ad6d5774 100644 --- a/src/pages/workspace/WorkspaceMoreFeaturesPage.tsx +++ b/src/pages/workspace/WorkspaceMoreFeaturesPage.tsx @@ -281,7 +281,7 @@ function WorkspaceMoreFeaturesPage({policy, route}: WorkspaceMoreFeaturesPagePro if (!policyID) { return; } - enablePolicyTags(policyID, isEnabled); + enablePolicyTags({policyID, enabled: isEnabled, policyTags: policyTagLists}); }, }, { diff --git a/tests/actions/PolicyTagTest.ts b/tests/actions/PolicyTagTest.ts index d1aac4939607..9b155a1fa693 100644 --- a/tests/actions/PolicyTagTest.ts +++ b/tests/actions/PolicyTagTest.ts @@ -9,6 +9,7 @@ import { clearPolicyTagListErrorField, createPolicyTag, deletePolicyTags, + enablePolicyTags, renamePolicyTag, renamePolicyTagList, setPolicyRequiresTag, @@ -1382,4 +1383,146 @@ describe('actions/Policy', () => { }); }); }); + + describe('EnablePolicyTags', () => { + it('should enable tags and create default tag list if none exists', async () => { + // Given a policy without tags + const fakePolicy = createRandomPolicy(0); + fakePolicy.areTagsEnabled = false; + + mockFetch?.pause?.(); + + await Onyx.set(`${ONYXKEYS.COLLECTION.POLICY}${fakePolicy.id}`, fakePolicy); + + // When enabling tags + enablePolicyTags({policyID: fakePolicy.id, enabled: true}); + await waitForBatchedUpdates(); + + // Then the policy should be updated optimistically + const optimisticPolicy = await OnyxUtils.get(`${ONYXKEYS.COLLECTION.POLICY}${fakePolicy.id}`); + expect(optimisticPolicy?.areTagsEnabled).toBe(true); + expect(optimisticPolicy?.pendingFields?.areTagsEnabled).toBe(CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE); + + // And a default tag list should be created + const optimisticPolicyTags = await OnyxUtils.get(`${ONYXKEYS.COLLECTION.POLICY_TAGS}${fakePolicy.id}`); + expect(optimisticPolicyTags?.Tag?.name).toBe('Tag'); + expect(optimisticPolicyTags?.Tag?.orderWeight).toBe(0); + expect(optimisticPolicyTags?.Tag?.required).toBe(false); + expect(optimisticPolicyTags?.Tag?.tags).toEqual({}); + + mockFetch?.resume?.(); + await waitForBatchedUpdates(); + + // And after API success, pending fields should be cleared + const successPolicy = await OnyxUtils.get(`${ONYXKEYS.COLLECTION.POLICY}${fakePolicy.id}`); + expect(successPolicy?.pendingFields?.areTagsEnabled).toBeFalsy(); + }); + + it('should disable tags and update existing tag list', async () => { + // Given a policy with enabled tags and existing tag list + const fakePolicy = createRandomPolicy(0); + fakePolicy.areTagsEnabled = true; + + const tagListName = 'Tag'; + const fakePolicyTags = createRandomPolicyTags(tagListName, 2); + const existingTags = fakePolicyTags[tagListName]?.tags ?? {}; + + mockFetch?.pause?.(); + + await Onyx.set(`${ONYXKEYS.COLLECTION.POLICY}${fakePolicy.id}`, fakePolicy); + await Onyx.set(`${ONYXKEYS.COLLECTION.POLICY_TAGS}${fakePolicy.id}`, fakePolicyTags); + + // When disabling tags + enablePolicyTags({policyID: fakePolicy.id, enabled: false, policyTags: fakePolicyTags}); + await waitForBatchedUpdates(); + + // Then the policy should be updated optimistically + const optimisticPolicy = await OnyxUtils.get(`${ONYXKEYS.COLLECTION.POLICY}${fakePolicy.id}`); + expect(optimisticPolicy?.areTagsEnabled).toBe(false); + expect(optimisticPolicy?.requiresTag).toBe(false); + expect(optimisticPolicy?.pendingFields?.areTagsEnabled).toBe(CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE); + + // And all tags should be disabled + const optimisticPolicyTags = await OnyxUtils.get(`${ONYXKEYS.COLLECTION.POLICY_TAGS}${fakePolicy.id}`); + Object.keys(existingTags).forEach((tagName) => { + expect(optimisticPolicyTags?.[tagListName]?.tags[tagName]?.enabled).toBe(false); + }); + + mockFetch?.resume?.(); + await waitForBatchedUpdates(); + + // And after API success, pending fields should be cleared + const successPolicy = await OnyxUtils.get(`${ONYXKEYS.COLLECTION.POLICY}${fakePolicy.id}`); + expect(successPolicy?.pendingFields?.areTagsEnabled).toBeFalsy(); + }); + + it('should reset changes when API returns error', async () => { + // Given a policy with disabled tags + const fakePolicy = createRandomPolicy(0); + fakePolicy.areTagsEnabled = false; + + mockFetch?.pause?.(); + + await Onyx.set(`${ONYXKEYS.COLLECTION.POLICY}${fakePolicy.id}`, fakePolicy); + + mockFetch?.fail?.(); + + // When enabling tags fails + enablePolicyTags({policyID: fakePolicy.id, enabled: true}); + await waitForBatchedUpdates(); + + mockFetch?.resume?.(); + await waitForBatchedUpdates(); + + // Then the policy should be reset to original state + const failurePolicy = await OnyxUtils.get(`${ONYXKEYS.COLLECTION.POLICY}${fakePolicy.id}`); + expect(failurePolicy?.areTagsEnabled).toBe(false); + expect(failurePolicy?.pendingFields?.areTagsEnabled).toBeFalsy(); + + // And no tag list should be created + const failurePolicyTags = await OnyxUtils.get(`${ONYXKEYS.COLLECTION.POLICY_TAGS}${fakePolicy.id}`); + expect(failurePolicyTags).toBeFalsy(); + }); + + it('should work with data from useOnyx hook', async () => { + // Given a policy data loaded via useOnyx + const fakePolicy = createRandomPolicy(0); + fakePolicy.areTagsEnabled = false; + + mockFetch?.pause?.(); + + await Onyx.set(`${ONYXKEYS.COLLECTION.POLICY}${fakePolicy.id}`, fakePolicy); + + const {result} = renderHook(() => { + const [policy] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY}${fakePolicy.id}`); + const [policyTags] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_TAGS}${fakePolicy.id}`); + return {policy, policyTags}; + }); + + await waitFor(() => { + expect(result.current.policy).toBeDefined(); + }); + + // When enabling tags with data from useOnyx + enablePolicyTags({policyID: fakePolicy.id, enabled: true, policyTags: result.current.policyTags}); + await waitForBatchedUpdates(); + + // Then the policy should be updated optimistically + const optimisticPolicy = await OnyxUtils.get(`${ONYXKEYS.COLLECTION.POLICY}${fakePolicy.id}`); + expect(optimisticPolicy?.areTagsEnabled).toBe(true); + + mockFetch?.resume?.(); + await waitForBatchedUpdates(); + + // And after API success, policy should be enabled + await waitFor(() => { + expect(result.current.policy?.areTagsEnabled).toBe(true); + }); + + // And default tag list should be created + await waitFor(() => { + expect(result.current.policyTags?.Tag).toBeDefined(); + }); + }); + }); }); From bc4deb540ad49ecefad4436614a71c72c84e9fa8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Ska=C5=82ka?= Date: Mon, 13 Oct 2025 11:38:58 +0200 Subject: [PATCH 2/5] fix: add missing canBeMissing param to useOnyx --- tests/actions/PolicyTagTest.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/actions/PolicyTagTest.ts b/tests/actions/PolicyTagTest.ts index 9b155a1fa693..75678d6f683f 100644 --- a/tests/actions/PolicyTagTest.ts +++ b/tests/actions/PolicyTagTest.ts @@ -1494,8 +1494,8 @@ describe('actions/Policy', () => { await Onyx.set(`${ONYXKEYS.COLLECTION.POLICY}${fakePolicy.id}`, fakePolicy); const {result} = renderHook(() => { - const [policy] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY}${fakePolicy.id}`); - const [policyTags] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_TAGS}${fakePolicy.id}`); + const [policy] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY}${fakePolicy.id}`, {canBeMissing: true}); + const [policyTags] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_TAGS}${fakePolicy.id}`, {canBeMissing: true}); return {policy, policyTags}; }); From d9d830d90128a1ea68f6f3b3e4ab050018b8e4ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Ska=C5=82ka?= Date: Tue, 14 Oct 2025 11:54:17 +0200 Subject: [PATCH 3/5] refactor: add review cahnges --- tests/actions/PolicyTagTest.ts | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/tests/actions/PolicyTagTest.ts b/tests/actions/PolicyTagTest.ts index 75678d6f683f..3c174cc34104 100644 --- a/tests/actions/PolicyTagTest.ts +++ b/tests/actions/PolicyTagTest.ts @@ -1,4 +1,4 @@ -import {renderHook, waitFor} from '@testing-library/react-native'; +import {act, renderHook, waitFor} from '@testing-library/react-native'; import Onyx from 'react-native-onyx'; import OnyxUtils from 'react-native-onyx/dist/OnyxUtils'; import useOnyx from '@hooks/useOnyx'; @@ -1390,7 +1390,7 @@ describe('actions/Policy', () => { const fakePolicy = createRandomPolicy(0); fakePolicy.areTagsEnabled = false; - mockFetch?.pause?.(); + mockFetch.pause(); await Onyx.set(`${ONYXKEYS.COLLECTION.POLICY}${fakePolicy.id}`, fakePolicy); @@ -1410,7 +1410,7 @@ describe('actions/Policy', () => { expect(optimisticPolicyTags?.Tag?.required).toBe(false); expect(optimisticPolicyTags?.Tag?.tags).toEqual({}); - mockFetch?.resume?.(); + mockFetch.resume(); await waitForBatchedUpdates(); // And after API success, pending fields should be cleared @@ -1427,7 +1427,7 @@ describe('actions/Policy', () => { const fakePolicyTags = createRandomPolicyTags(tagListName, 2); const existingTags = fakePolicyTags[tagListName]?.tags ?? {}; - mockFetch?.pause?.(); + mockFetch.pause(); await Onyx.set(`${ONYXKEYS.COLLECTION.POLICY}${fakePolicy.id}`, fakePolicy); await Onyx.set(`${ONYXKEYS.COLLECTION.POLICY_TAGS}${fakePolicy.id}`, fakePolicyTags); @@ -1448,7 +1448,7 @@ describe('actions/Policy', () => { expect(optimisticPolicyTags?.[tagListName]?.tags[tagName]?.enabled).toBe(false); }); - mockFetch?.resume?.(); + mockFetch.resume(); await waitForBatchedUpdates(); // And after API success, pending fields should be cleared @@ -1461,11 +1461,11 @@ describe('actions/Policy', () => { const fakePolicy = createRandomPolicy(0); fakePolicy.areTagsEnabled = false; - mockFetch?.pause?.(); + mockFetch.pause(); await Onyx.set(`${ONYXKEYS.COLLECTION.POLICY}${fakePolicy.id}`, fakePolicy); - mockFetch?.fail?.(); + mockFetch.fail(); // When enabling tags fails enablePolicyTags({policyID: fakePolicy.id, enabled: true}); @@ -1489,7 +1489,7 @@ describe('actions/Policy', () => { const fakePolicy = createRandomPolicy(0); fakePolicy.areTagsEnabled = false; - mockFetch?.pause?.(); + mockFetch.pause(); await Onyx.set(`${ONYXKEYS.COLLECTION.POLICY}${fakePolicy.id}`, fakePolicy); @@ -1504,14 +1504,19 @@ describe('actions/Policy', () => { }); // When enabling tags with data from useOnyx - enablePolicyTags({policyID: fakePolicy.id, enabled: true, policyTags: result.current.policyTags}); + await act(async () => { + enablePolicyTags({policyID: fakePolicy.id, enabled: true, policyTags: result.current.policyTags}); + await waitForBatchedUpdates(); + }); await waitForBatchedUpdates(); // Then the policy should be updated optimistically const optimisticPolicy = await OnyxUtils.get(`${ONYXKEYS.COLLECTION.POLICY}${fakePolicy.id}`); expect(optimisticPolicy?.areTagsEnabled).toBe(true); - mockFetch?.resume?.(); + await act(async () => { + await mockFetch.resume(); + }); await waitForBatchedUpdates(); // And after API success, policy should be enabled From 82534992edbfc0e80c83d5b7df8c8ccb220df033 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Ska=C5=82ka?= Date: Tue, 14 Oct 2025 16:18:18 +0200 Subject: [PATCH 4/5] refactor: put waitForBatchedUpdates into act --- tests/actions/PolicyTagTest.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/actions/PolicyTagTest.ts b/tests/actions/PolicyTagTest.ts index 3c174cc34104..f8ea19da4f4e 100644 --- a/tests/actions/PolicyTagTest.ts +++ b/tests/actions/PolicyTagTest.ts @@ -1508,7 +1508,6 @@ describe('actions/Policy', () => { enablePolicyTags({policyID: fakePolicy.id, enabled: true, policyTags: result.current.policyTags}); await waitForBatchedUpdates(); }); - await waitForBatchedUpdates(); // Then the policy should be updated optimistically const optimisticPolicy = await OnyxUtils.get(`${ONYXKEYS.COLLECTION.POLICY}${fakePolicy.id}`); @@ -1516,8 +1515,8 @@ describe('actions/Policy', () => { await act(async () => { await mockFetch.resume(); + await waitForBatchedUpdates(); }); - await waitForBatchedUpdates(); // And after API success, policy should be enabled await waitFor(() => { From 8c887130af9aab9ba44c68e0d3d1921debdb5480 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Ska=C5=82ka?= Date: Wed, 15 Oct 2025 14:12:00 +0200 Subject: [PATCH 5/5] refactor: add review suggestion --- tests/actions/PolicyTagTest.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/actions/PolicyTagTest.ts b/tests/actions/PolicyTagTest.ts index f8ea19da4f4e..c96778c6d2ed 100644 --- a/tests/actions/PolicyTagTest.ts +++ b/tests/actions/PolicyTagTest.ts @@ -1471,7 +1471,7 @@ describe('actions/Policy', () => { enablePolicyTags({policyID: fakePolicy.id, enabled: true}); await waitForBatchedUpdates(); - mockFetch?.resume?.(); + mockFetch.resume(); await waitForBatchedUpdates(); // Then the policy should be reset to original state