From 56570a7035d49a071e31476182e9663ab22d49b5 Mon Sep 17 00:00:00 2001 From: truph01 Date: Tue, 10 Dec 2024 15:58:14 +0700 Subject: [PATCH 01/10] fix: Hmm it's not here in RHP is displayed after deleting a workspace --- src/libs/PolicyUtils.ts | 15 +++++++++ src/libs/ReportUtils.ts | 4 +-- tests/unit/PolicyUtilsTest.ts | 63 +++++++++++++++++++++++++++++++++++ 3 files changed, 79 insertions(+), 3 deletions(-) diff --git a/src/libs/PolicyUtils.ts b/src/libs/PolicyUtils.ts index f6b277d69d6b..28027303007f 100644 --- a/src/libs/PolicyUtils.ts +++ b/src/libs/PolicyUtils.ts @@ -62,6 +62,14 @@ Onyx.connect({ callback: (value) => (allPolicies = value), }); +let currentUserEmail: string; +Onyx.connect({ + key: ONYXKEYS.SESSION, + callback: (val) => { + currentUserEmail = val?.email ?? ''; + }, +}); + Onyx.connect({ key: ONYXKEYS.NVP_ACTIVE_POLICY_ID, callback: (value) => (activePolicyId = value), @@ -1131,6 +1139,12 @@ function areAllGroupPoliciesExpenseChatDisabled(policies = allPolicies) { return !groupPolicies.some((policy) => !!policy?.isPolicyExpenseChatEnabled); } +function getFilteredPolicies(policies: OnyxCollection) { + return Object.values(policies ?? {}).filter( + (policy) => policy && policy.type !== CONST.POLICY.TYPE.PERSONAL && policy.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE && !!getPolicyRole(policy, currentUserEmail), + ); +} + export { canEditTaxRate, extractPolicyIDFromPath, @@ -1254,6 +1268,7 @@ export { getActivePolicy, isPolicyAccessible, areAllGroupPoliciesExpenseChatDisabled, + getFilteredPolicies, }; export type {MemberEmailsToAccountIDs}; diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index 9fff3ade23f9..0398ba048d85 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -8212,9 +8212,7 @@ function createDraftTransactionAndNavigateToParticipantSelector(transactionID: s mccGroup, } as Transaction); - const filteredPolicies = Object.values(allPolicies ?? {}).filter( - (policy) => policy && policy.type !== CONST.POLICY.TYPE.PERSONAL && policy.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE, - ); + const filteredPolicies = PolicyUtils.getFilteredPolicies(allPolicies); if (actionName === CONST.IOU.ACTION.CATEGORIZE) { const activePolicy = getPolicy(activePolicyID); diff --git a/tests/unit/PolicyUtilsTest.ts b/tests/unit/PolicyUtilsTest.ts index 71830443063a..d63ea8cdd565 100644 --- a/tests/unit/PolicyUtilsTest.ts +++ b/tests/unit/PolicyUtilsTest.ts @@ -1,5 +1,15 @@ +import Onyx from 'react-native-onyx'; +import type {OnyxCollection} from 'react-native-onyx'; import * as PolicyUtils from '@libs/PolicyUtils'; +import CONST from '@src/CONST'; +import ONYXKEYS from '@src/ONYXKEYS'; +import {Policy} from '@src/types/onyx'; +import createRandomPolicy from '../utils/collections/policies'; +import * as TestHelper from '../utils/TestHelper'; +import waitForBatchedUpdates from '../utils/waitForBatchedUpdates'; +const CARLOS_EMAIL = 'cmartins@expensifail.com'; +const CARLOS_ACCOUNT_ID = 1; function toLocaleDigitMock(dot: string): string { return dot; } @@ -81,4 +91,57 @@ describe('PolicyUtils', () => { }); }); }); + describe('getFilteredPolicies', () => { + beforeAll(() => { + Onyx.init({ + keys: ONYXKEYS, + initialKeyStates: { + // Given mock data for session + [ONYXKEYS.SESSION]: {accountID: CARLOS_ACCOUNT_ID, email: CARLOS_EMAIL}, + }, + }); + }); + + beforeEach(() => { + global.fetch = TestHelper.getGlobalFetchMock(); + return Onyx.clear().then(waitForBatchedUpdates); + }); + it('should return empty array', () => { + // Given mock data for policies + const policies = { + 1: { + ...createRandomPolicy(1, CONST.POLICY.TYPE.CORPORATE), + role: '', + }, + }; + // When calling getFilteredPolicies with policies + const result = PolicyUtils.getFilteredPolicies(policies as OnyxCollection); + // The result should be empty array since the policies contains only one policy which does not have role field. + expect(result.length).toBe(0); + }); + it('should return array contains policy which has id = 1', () => { + // Given mock data for policies + const randomPolicy1 = createRandomPolicy(1, CONST.POLICY.TYPE.CORPORATE); + const policies = { + 1: randomPolicy1, + }; + // When calling getFilteredPolicies with policies + const result = PolicyUtils.getFilteredPolicies(policies as OnyxCollection); + // The result should contains the policy which has id = 1 since it is a valid policy. + expect(result).toContainEqual(randomPolicy1); + }); + it('should return empty array', () => { + // Given mock data for policies + const policies = { + 1: { + ...createRandomPolicy(1, CONST.POLICY.TYPE.CORPORATE), + pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE, + }, + }; + // When calling getFilteredPolicies with policies + const result = PolicyUtils.getFilteredPolicies(policies as OnyxCollection); + // The result should be empty array since the policies contains only one policy which has pendingAction is 'delete' . + expect(result).toEqual([]); + }); + }); }); From a01d0cbea604204d28db04cb2563424119cf55ae Mon Sep 17 00:00:00 2001 From: truph01 Date: Tue, 10 Dec 2024 16:14:40 +0700 Subject: [PATCH 02/10] fix: remove space end line --- tests/unit/PolicyUtilsTest.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/PolicyUtilsTest.ts b/tests/unit/PolicyUtilsTest.ts index d63ea8cdd565..26cda5ac4036 100644 --- a/tests/unit/PolicyUtilsTest.ts +++ b/tests/unit/PolicyUtilsTest.ts @@ -140,7 +140,7 @@ describe('PolicyUtils', () => { }; // When calling getFilteredPolicies with policies const result = PolicyUtils.getFilteredPolicies(policies as OnyxCollection); - // The result should be empty array since the policies contains only one policy which has pendingAction is 'delete' . + // The result should be empty array since the policies contains only one policy which has pendingAction is 'delete'. expect(result).toEqual([]); }); }); From 8d50fecc65f5b195ffbbd98fff836f79dd49497a Mon Sep 17 00:00:00 2001 From: truph01 Date: Tue, 10 Dec 2024 16:18:24 +0700 Subject: [PATCH 03/10] fix: lint --- tests/unit/PolicyUtilsTest.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/unit/PolicyUtilsTest.ts b/tests/unit/PolicyUtilsTest.ts index 26cda5ac4036..8d0945c629bb 100644 --- a/tests/unit/PolicyUtilsTest.ts +++ b/tests/unit/PolicyUtilsTest.ts @@ -3,7 +3,7 @@ import type {OnyxCollection} from 'react-native-onyx'; import * as PolicyUtils from '@libs/PolicyUtils'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; -import {Policy} from '@src/types/onyx'; +import type {Policy} from '@src/types/onyx'; import createRandomPolicy from '../utils/collections/policies'; import * as TestHelper from '../utils/TestHelper'; import waitForBatchedUpdates from '../utils/waitForBatchedUpdates'; @@ -109,6 +109,7 @@ describe('PolicyUtils', () => { it('should return empty array', () => { // Given mock data for policies const policies = { + // eslint-disable-next-line @typescript-eslint/naming-convention 1: { ...createRandomPolicy(1, CONST.POLICY.TYPE.CORPORATE), role: '', @@ -123,6 +124,7 @@ describe('PolicyUtils', () => { // Given mock data for policies const randomPolicy1 = createRandomPolicy(1, CONST.POLICY.TYPE.CORPORATE); const policies = { + // eslint-disable-next-line @typescript-eslint/naming-convention 1: randomPolicy1, }; // When calling getFilteredPolicies with policies @@ -133,6 +135,7 @@ describe('PolicyUtils', () => { it('should return empty array', () => { // Given mock data for policies const policies = { + // eslint-disable-next-line @typescript-eslint/naming-convention 1: { ...createRandomPolicy(1, CONST.POLICY.TYPE.CORPORATE), pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE, From 57bbc2aacb84b79224964b81b2b2583b935642e7 Mon Sep 17 00:00:00 2001 From: truph01 Date: Wed, 11 Dec 2024 18:10:36 +0700 Subject: [PATCH 04/10] fix: reuse getActivePolicies --- src/libs/PolicyUtils.ts | 17 ++++++++--------- src/libs/ReportUtils.ts | 2 +- src/pages/workspace/WorkspaceNewRoomPage.tsx | 3 +-- tests/unit/PolicyUtilsTest.ts | 16 +++++++--------- 4 files changed, 17 insertions(+), 21 deletions(-) diff --git a/src/libs/PolicyUtils.ts b/src/libs/PolicyUtils.ts index 28027303007f..2dc436a4d76c 100644 --- a/src/libs/PolicyUtils.ts +++ b/src/libs/PolicyUtils.ts @@ -79,9 +79,15 @@ Onyx.connect({ * Filter out the active policies, which will exclude policies with pending deletion * These are policies that we can use to create reports with in NewDot. */ -function getActivePolicies(policies: OnyxCollection | null): Policy[] { +function getActivePolicies(policies: OnyxCollection | null, excludePersonalPolicy = false): Policy[] { return Object.values(policies ?? {}).filter( - (policy): policy is Policy => !!policy && policy.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE && !!policy.name && !!policy.id, + (policy): policy is Policy => + !!policy && + policy.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE && + !!policy.name && + !!policy.id && + (!excludePersonalPolicy || policy.type !== CONST.POLICY.TYPE.PERSONAL) && + !!getPolicyRole(policy, currentUserEmail), ); } /** @@ -1139,12 +1145,6 @@ function areAllGroupPoliciesExpenseChatDisabled(policies = allPolicies) { return !groupPolicies.some((policy) => !!policy?.isPolicyExpenseChatEnabled); } -function getFilteredPolicies(policies: OnyxCollection) { - return Object.values(policies ?? {}).filter( - (policy) => policy && policy.type !== CONST.POLICY.TYPE.PERSONAL && policy.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE && !!getPolicyRole(policy, currentUserEmail), - ); -} - export { canEditTaxRate, extractPolicyIDFromPath, @@ -1268,7 +1268,6 @@ export { getActivePolicy, isPolicyAccessible, areAllGroupPoliciesExpenseChatDisabled, - getFilteredPolicies, }; export type {MemberEmailsToAccountIDs}; diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index be97bbb1e5e8..026d55966943 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -8216,7 +8216,7 @@ function createDraftTransactionAndNavigateToParticipantSelector(transactionID: s mccGroup, } as Transaction); - const filteredPolicies = PolicyUtils.getFilteredPolicies(allPolicies); + const filteredPolicies = PolicyUtils.getActivePolicies(allPolicies, true); if (actionName === CONST.IOU.ACTION.CATEGORIZE) { const activePolicy = getPolicy(activePolicyID); diff --git a/src/pages/workspace/WorkspaceNewRoomPage.tsx b/src/pages/workspace/WorkspaceNewRoomPage.tsx index 37686ba1c3a7..a23d35d955a2 100644 --- a/src/pages/workspace/WorkspaceNewRoomPage.tsx +++ b/src/pages/workspace/WorkspaceNewRoomPage.tsx @@ -64,8 +64,7 @@ function WorkspaceNewRoomPage() { const workspaceOptions = useMemo( () => - PolicyUtils.getActivePolicies(policies) - ?.filter((policy) => policy.type !== CONST.POLICY.TYPE.PERSONAL) + PolicyUtils.getActivePolicies(policies, true) .map((policy) => ({ label: policy.name, value: policy.id, diff --git a/tests/unit/PolicyUtilsTest.ts b/tests/unit/PolicyUtilsTest.ts index 8d0945c629bb..927f1171d7c2 100644 --- a/tests/unit/PolicyUtilsTest.ts +++ b/tests/unit/PolicyUtilsTest.ts @@ -91,7 +91,7 @@ describe('PolicyUtils', () => { }); }); }); - describe('getFilteredPolicies', () => { + describe('getActivePolicies', () => { beforeAll(() => { Onyx.init({ keys: ONYXKEYS, @@ -107,7 +107,7 @@ describe('PolicyUtils', () => { return Onyx.clear().then(waitForBatchedUpdates); }); it('should return empty array', () => { - // Given mock data for policies + // Given mock data for policies, contains only one workspace does not has role prop. const policies = { // eslint-disable-next-line @typescript-eslint/naming-convention 1: { @@ -115,25 +115,23 @@ describe('PolicyUtils', () => { role: '', }, }; - // When calling getFilteredPolicies with policies - const result = PolicyUtils.getFilteredPolicies(policies as OnyxCollection); + const result = PolicyUtils.getActivePolicies(policies as OnyxCollection); // The result should be empty array since the policies contains only one policy which does not have role field. expect(result.length).toBe(0); }); it('should return array contains policy which has id = 1', () => { - // Given mock data for policies + // Given mock data for policies, contains only one control workspace const randomPolicy1 = createRandomPolicy(1, CONST.POLICY.TYPE.CORPORATE); const policies = { // eslint-disable-next-line @typescript-eslint/naming-convention 1: randomPolicy1, }; - // When calling getFilteredPolicies with policies - const result = PolicyUtils.getFilteredPolicies(policies as OnyxCollection); + const result = PolicyUtils.getActivePolicies(policies as OnyxCollection); // The result should contains the policy which has id = 1 since it is a valid policy. expect(result).toContainEqual(randomPolicy1); }); it('should return empty array', () => { - // Given mock data for policies + // Given mock data for policies, contains only one control workspace which is pending delete. const policies = { // eslint-disable-next-line @typescript-eslint/naming-convention 1: { @@ -142,7 +140,7 @@ describe('PolicyUtils', () => { }, }; // When calling getFilteredPolicies with policies - const result = PolicyUtils.getFilteredPolicies(policies as OnyxCollection); + const result = PolicyUtils.getActivePolicies(policies as OnyxCollection); // The result should be empty array since the policies contains only one policy which has pendingAction is 'delete'. expect(result).toEqual([]); }); From 9f17db9785029a406661985d97a59aac8ebbd3f3 Mon Sep 17 00:00:00 2001 From: truph01 Date: Wed, 11 Dec 2024 18:27:10 +0700 Subject: [PATCH 05/10] fix: unit test --- tests/unit/PolicyUtilsTest.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/PolicyUtilsTest.ts b/tests/unit/PolicyUtilsTest.ts index 927f1171d7c2..b8c2e562b21e 100644 --- a/tests/unit/PolicyUtilsTest.ts +++ b/tests/unit/PolicyUtilsTest.ts @@ -121,7 +121,7 @@ describe('PolicyUtils', () => { }); it('should return array contains policy which has id = 1', () => { // Given mock data for policies, contains only one control workspace - const randomPolicy1 = createRandomPolicy(1, CONST.POLICY.TYPE.CORPORATE); + const randomPolicy1 = {...createRandomPolicy(1, CONST.POLICY.TYPE.CORPORATE), pendingAction: null}; const policies = { // eslint-disable-next-line @typescript-eslint/naming-convention 1: randomPolicy1, From 0382872a89f4d058bf38c3377ecdb1a51e12cdc8 Mon Sep 17 00:00:00 2001 From: truph01 Date: Wed, 11 Dec 2024 18:42:56 +0700 Subject: [PATCH 06/10] fix: remove comment --- tests/unit/PolicyUtilsTest.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/unit/PolicyUtilsTest.ts b/tests/unit/PolicyUtilsTest.ts index b8c2e562b21e..880e21ea0aca 100644 --- a/tests/unit/PolicyUtilsTest.ts +++ b/tests/unit/PolicyUtilsTest.ts @@ -139,7 +139,6 @@ describe('PolicyUtils', () => { pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE, }, }; - // When calling getFilteredPolicies with policies const result = PolicyUtils.getActivePolicies(policies as OnyxCollection); // The result should be empty array since the policies contains only one policy which has pendingAction is 'delete'. expect(result).toEqual([]); From 1e6ee20d898833b668bb08f84b4958467f49c1df Mon Sep 17 00:00:00 2001 From: truph01 Date: Thu, 12 Dec 2024 03:08:36 +0700 Subject: [PATCH 07/10] fix: update comments --- tests/unit/PolicyUtilsTest.ts | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/tests/unit/PolicyUtilsTest.ts b/tests/unit/PolicyUtilsTest.ts index 880e21ea0aca..e13db0f3a04b 100644 --- a/tests/unit/PolicyUtilsTest.ts +++ b/tests/unit/PolicyUtilsTest.ts @@ -96,7 +96,6 @@ describe('PolicyUtils', () => { Onyx.init({ keys: ONYXKEYS, initialKeyStates: { - // Given mock data for session [ONYXKEYS.SESSION]: {accountID: CARLOS_ACCOUNT_ID, email: CARLOS_EMAIL}, }, }); @@ -107,7 +106,7 @@ describe('PolicyUtils', () => { return Onyx.clear().then(waitForBatchedUpdates); }); it('should return empty array', () => { - // Given mock data for policies, contains only one workspace does not has role prop. + // Given a user with a single archived paid policy. const policies = { // eslint-disable-next-line @typescript-eslint/naming-convention 1: { @@ -115,23 +114,23 @@ describe('PolicyUtils', () => { role: '', }, }; - const result = PolicyUtils.getActivePolicies(policies as OnyxCollection); - // The result should be empty array since the policies contains only one policy which does not have role field. + const result = PolicyUtils.getActivePolicies(policies as OnyxCollection, true); + // The result should be empty array since the policies contains only archived paid policy. expect(result.length).toBe(0); }); it('should return array contains policy which has id = 1', () => { - // Given mock data for policies, contains only one control workspace + // Given a user with only a paid policy. const randomPolicy1 = {...createRandomPolicy(1, CONST.POLICY.TYPE.CORPORATE), pendingAction: null}; const policies = { // eslint-disable-next-line @typescript-eslint/naming-convention 1: randomPolicy1, }; - const result = PolicyUtils.getActivePolicies(policies as OnyxCollection); - // The result should contains the policy which has id = 1 since it is a valid policy. + const result = PolicyUtils.getActivePolicies(policies as OnyxCollection, true); + // The result should contain the mock paid policy. expect(result).toContainEqual(randomPolicy1); }); it('should return empty array', () => { - // Given mock data for policies, contains only one control workspace which is pending delete. + // Given a user with only one control workspace which is pending delete. const policies = { // eslint-disable-next-line @typescript-eslint/naming-convention 1: { @@ -139,8 +138,8 @@ describe('PolicyUtils', () => { pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE, }, }; - const result = PolicyUtils.getActivePolicies(policies as OnyxCollection); - // The result should be empty array since the policies contains only one policy which has pendingAction is 'delete'. + const result = PolicyUtils.getActivePolicies(policies as OnyxCollection, true); + // The result should be empty array since there is only one policy which has pendingAction is 'delete'. expect(result).toEqual([]); }); }); From 2f27b87127add2731072639e790f1d57b75cc3d5 Mon Sep 17 00:00:00 2001 From: truph01 Date: Fri, 13 Dec 2024 16:33:24 +0700 Subject: [PATCH 08/10] Update tests/unit/PolicyUtilsTest.ts Co-authored-by: Joel Davies --- tests/unit/PolicyUtilsTest.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/PolicyUtilsTest.ts b/tests/unit/PolicyUtilsTest.ts index e13db0f3a04b..11497f87f8a8 100644 --- a/tests/unit/PolicyUtilsTest.ts +++ b/tests/unit/PolicyUtilsTest.ts @@ -115,7 +115,7 @@ describe('PolicyUtils', () => { }, }; const result = PolicyUtils.getActivePolicies(policies as OnyxCollection, true); - // The result should be empty array since the policies contains only archived paid policy. + // The result should be an empty array since we have no active policies. expect(result.length).toBe(0); }); it('should return array contains policy which has id = 1', () => { From 814bb3da28a9543b2dc0f5fb2a539660fe9b5e28 Mon Sep 17 00:00:00 2001 From: truph01 Date: Fri, 13 Dec 2024 16:33:40 +0700 Subject: [PATCH 09/10] Update tests/unit/PolicyUtilsTest.ts Co-authored-by: Joel Davies --- tests/unit/PolicyUtilsTest.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/PolicyUtilsTest.ts b/tests/unit/PolicyUtilsTest.ts index 11497f87f8a8..b1560da49aa5 100644 --- a/tests/unit/PolicyUtilsTest.ts +++ b/tests/unit/PolicyUtilsTest.ts @@ -126,7 +126,7 @@ describe('PolicyUtils', () => { 1: randomPolicy1, }; const result = PolicyUtils.getActivePolicies(policies as OnyxCollection, true); - // The result should contain the mock paid policy. + // The result should contain the mock paid policy, since it is our only active paid policy. expect(result).toContainEqual(randomPolicy1); }); it('should return empty array', () => { From 73cfbfa593a521560090a5008fbb91f5202c9e2d Mon Sep 17 00:00:00 2001 From: truph01 Date: Fri, 13 Dec 2024 16:33:59 +0700 Subject: [PATCH 10/10] Update tests/unit/PolicyUtilsTest.ts Co-authored-by: Joel Davies --- tests/unit/PolicyUtilsTest.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/PolicyUtilsTest.ts b/tests/unit/PolicyUtilsTest.ts index b1560da49aa5..488b7f797f0d 100644 --- a/tests/unit/PolicyUtilsTest.ts +++ b/tests/unit/PolicyUtilsTest.ts @@ -139,7 +139,7 @@ describe('PolicyUtils', () => { }, }; const result = PolicyUtils.getActivePolicies(policies as OnyxCollection, true); - // The result should be empty array since there is only one policy which has pendingAction is 'delete'. + // The result should be an empty array since there is only one policy which is pending deletion, so we have no active paid policies. expect(result).toEqual([]); }); });