From bd6d4c6ea977e48fe92fe22f51dd91d6d2b5eb1c Mon Sep 17 00:00:00 2001 From: Wojciech Boman Date: Wed, 10 Apr 2024 17:06:49 +0200 Subject: [PATCH 1/9] Add useActiveWorkspaceFromNavigationState --- .../useActiveWorkspaceFromNavigationState.ts | 13 ++++ src/libs/Navigation/types.ts | 4 +- src/libs/PolicyUtils.ts | 68 ++++++++----------- src/pages/home/sidebar/SidebarLinksData.tsx | 4 +- .../SidebarScreen/BaseSidebarScreen.tsx | 4 +- 5 files changed, 48 insertions(+), 45 deletions(-) create mode 100644 src/hooks/useActiveWorkspaceFromNavigationState.ts diff --git a/src/hooks/useActiveWorkspaceFromNavigationState.ts b/src/hooks/useActiveWorkspaceFromNavigationState.ts new file mode 100644 index 000000000000..8f9da3d9df37 --- /dev/null +++ b/src/hooks/useActiveWorkspaceFromNavigationState.ts @@ -0,0 +1,13 @@ +import {useNavigationState} from '@react-navigation/native'; +import type {BottomTabNavigatorParamList} from '@libs/Navigation/types'; + +/** + * Get the currently selected policy ID stored in the navigation state. This hook should only be called only from screens in BottomTab. + */ +function useActiveWorkspaceFromNavigationState() { + const activeWorkpsaceID = useNavigationState((state) => state.routes.at(-1)?.params?.policyID); + + return activeWorkpsaceID; +} + +export default useActiveWorkspaceFromNavigationState; diff --git a/src/libs/Navigation/types.ts b/src/libs/Navigation/types.ts index 1f1e7ec9a459..04bb7797804e 100644 --- a/src/libs/Navigation/types.ts +++ b/src/libs/Navigation/types.ts @@ -662,8 +662,8 @@ type WelcomeVideoModalNavigatorParamList = { }; type BottomTabNavigatorParamList = { - [SCREENS.HOME]: undefined; - [SCREENS.SETTINGS.ROOT]: undefined; + [SCREENS.HOME]: {policyID?: string}; + [SCREENS.SETTINGS.ROOT]: {policyID?: string}; }; type SharedScreensParamList = { diff --git a/src/libs/PolicyUtils.ts b/src/libs/PolicyUtils.ts index 665830ca7167..6c5e4ea695c2 100644 --- a/src/libs/PolicyUtils.ts +++ b/src/libs/PolicyUtils.ts @@ -8,9 +8,7 @@ import type {PersonalDetailsList, Policy, PolicyCategories, PolicyMembers, Polic import type {PolicyFeatureName, Rate} from '@src/types/onyx/Policy'; import type {EmptyObject} from '@src/types/utils/EmptyObject'; import {isEmptyObject} from '@src/types/utils/EmptyObject'; -import getPolicyIDFromState from './Navigation/getPolicyIDFromState'; -import Navigation, {navigationRef} from './Navigation/Navigation'; -import type {RootStackParamList, State} from './Navigation/types'; +import Navigation from './Navigation/Navigation'; type MemberEmailsToAccountIDs = Record; @@ -310,51 +308,43 @@ function isPolicyFeatureEnabled(policy: OnyxEntry | EmptyObject, feature return Boolean(policy?.[featureName]); } -/** - * Get the currently selected policy ID stored in the navigation state. - */ -function getPolicyIDFromNavigationState() { - return getPolicyIDFromState(navigationRef.getRootState() as State); -} - export { + canEditTaxRate, + extractPolicyIDFromPath, getActivePolicies, + getCleanedTagName, + getCountOfEnabledTagsOfList, + getIneligibleInvitees, + getMemberAccountIDsForWorkspace, + getNumericValue, + getPathWithoutPolicyID, + getPolicyBrickRoadIndicatorStatus, + getPolicyMembersByIdWithoutCurrentUser, + getSortedTagKeys, + getTagList, + getTagListName, + getTagLists, + getTaxByID, + getUnitRateValue, + goBackFromInvalidPolicy, hasAccountingConnections, - hasPolicyMemberError, + hasCustomUnitsError, + hasPolicyCategoriesError, hasPolicyError, hasPolicyErrorFields, - hasCustomUnitsError, - getNumericValue, - getUnitRateValue, - getPolicyBrickRoadIndicatorStatus, - shouldShowPolicy, + hasPolicyMemberError, + hasTaxRateError, isExpensifyTeam, - isInstantSubmitEnabled, isFreeGroupPolicy, - isPolicyAdmin, - isTaxTrackingEnabled, - isSubmitAndClose, - getMemberAccountIDsForWorkspace, - getIneligibleInvitees, - getTagLists, - getTagListName, - getSortedTagKeys, - canEditTaxRate, - getTagList, - getCleanedTagName, - getCountOfEnabledTagsOfList, - isPendingDeletePolicy, - isPolicyMember, + isInstantSubmitEnabled, isPaidGroupPolicy, - extractPolicyIDFromPath, - getPathWithoutPolicyID, - getPolicyMembersByIdWithoutCurrentUser, - goBackFromInvalidPolicy, + isPendingDeletePolicy, + isPolicyAdmin, isPolicyFeatureEnabled, - hasTaxRateError, - getTaxByID, - hasPolicyCategoriesError, - getPolicyIDFromNavigationState, + isPolicyMember, + isSubmitAndClose, + isTaxTrackingEnabled, + shouldShowPolicy, }; export type {MemberEmailsToAccountIDs}; diff --git a/src/pages/home/sidebar/SidebarLinksData.tsx b/src/pages/home/sidebar/SidebarLinksData.tsx index e56962a331a2..4a90d6e96239 100644 --- a/src/pages/home/sidebar/SidebarLinksData.tsx +++ b/src/pages/home/sidebar/SidebarLinksData.tsx @@ -9,7 +9,7 @@ import type {EdgeInsets} from 'react-native-safe-area-context'; import type {ValueOf} from 'type-fest'; import type {CurrentReportIDContextValue} from '@components/withCurrentReportID'; import withCurrentReportID from '@components/withCurrentReportID'; -import useActiveWorkspace from '@hooks/useActiveWorkspace'; +import useActiveWorkspaceFromNavigationState from '@hooks/useActiveWorkspaceFromNavigationState'; import useCurrentUserPersonalDetails from '@hooks/useCurrentUserPersonalDetails'; import useLocalize from '@hooks/useLocalize'; import useNetwork from '@hooks/useNetwork'; @@ -85,7 +85,7 @@ function SidebarLinksData({ const network = useNetwork(); const isFocused = useIsFocused(); const styles = useThemeStyles(); - const {activeWorkspaceID} = useActiveWorkspace(); + const activeWorkspaceID = useActiveWorkspaceFromNavigationState(); const {translate} = useLocalize(); const prevPriorityMode = usePrevious(priorityMode); const policyMemberAccountIDs = getPolicyMembersByIdWithoutCurrentUser(policyMembers, activeWorkspaceID, accountID); diff --git a/src/pages/home/sidebar/SidebarScreen/BaseSidebarScreen.tsx b/src/pages/home/sidebar/SidebarScreen/BaseSidebarScreen.tsx index fe61af021d7f..a8df74cfc4e9 100644 --- a/src/pages/home/sidebar/SidebarScreen/BaseSidebarScreen.tsx +++ b/src/pages/home/sidebar/SidebarScreen/BaseSidebarScreen.tsx @@ -1,11 +1,11 @@ import React, {useEffect} from 'react'; import {View} from 'react-native'; import ScreenWrapper from '@components/ScreenWrapper'; +import useActiveWorkspaceFromNavigationState from '@hooks/useActiveWorkspaceFromNavigationState'; import useThemeStyles from '@hooks/useThemeStyles'; import * as Browser from '@libs/Browser'; import TopBar from '@libs/Navigation/AppNavigator/createCustomBottomTabNavigator/TopBar'; import Performance from '@libs/Performance'; -import {getPolicyIDFromNavigationState} from '@libs/PolicyUtils'; import SidebarLinksData from '@pages/home/sidebar/SidebarLinksData'; import Timing from '@userActions/Timing'; import CONST from '@src/CONST'; @@ -20,7 +20,7 @@ const startTimer = () => { function BaseSidebarScreen() { const styles = useThemeStyles(); - const activeWorkspaceID = getPolicyIDFromNavigationState(); + const activeWorkspaceID = useActiveWorkspaceFromNavigationState(); useEffect(() => { Performance.markStart(CONST.TIMING.SIDEBAR_LOADED); Timing.start(CONST.TIMING.SIDEBAR_LOADED, true); From c3cd931cfaccaece3414103983483b7683c8f04f Mon Sep 17 00:00:00 2001 From: Wojciech Boman Date: Wed, 10 Apr 2024 17:39:17 +0200 Subject: [PATCH 2/9] Remove redundant SCREENS.SETTINGS.ROOT route --- src/libs/Navigation/types.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libs/Navigation/types.ts b/src/libs/Navigation/types.ts index 04bb7797804e..bffe333900e5 100644 --- a/src/libs/Navigation/types.ts +++ b/src/libs/Navigation/types.ts @@ -71,7 +71,6 @@ type BackToParams = { }; type SettingsNavigatorParamList = { - [SCREENS.SETTINGS.ROOT]: undefined; [SCREENS.SETTINGS.SHARE_CODE]: undefined; [SCREENS.SETTINGS.PROFILE.ROOT]: undefined; [SCREENS.SETTINGS.PROFILE.PRONOUNS]: undefined; From 945872ab306ed01b352caf10a3a3aea01d6cc087 Mon Sep 17 00:00:00 2001 From: Wojciech Boman Date: Wed, 10 Apr 2024 17:47:50 +0200 Subject: [PATCH 3/9] Handle mocking useActiveWorkspaceFromNavigationState in tests --- tests/unit/SidebarOrderTest.ts | 1 + tests/unit/SidebarTest.ts | 1 + 2 files changed, 2 insertions(+) diff --git a/tests/unit/SidebarOrderTest.ts b/tests/unit/SidebarOrderTest.ts index 2758d43fb81e..ac74fb50245b 100644 --- a/tests/unit/SidebarOrderTest.ts +++ b/tests/unit/SidebarOrderTest.ts @@ -14,6 +14,7 @@ import wrapOnyxWithWaitForBatchedUpdates from '../utils/wrapOnyxWithWaitForBatch jest.mock('@libs/Permissions'); jest.mock('@hooks/usePermissions.ts'); jest.mock('@components/Icon/Expensicons'); +jest.mock('@src/hooks/useActiveWorkspaceFromNavigationState') const ONYXKEYS = { PERSONAL_DETAILS_LIST: 'personalDetailsList', diff --git a/tests/unit/SidebarTest.ts b/tests/unit/SidebarTest.ts index 23ea0d377634..ba2d950232b5 100644 --- a/tests/unit/SidebarTest.ts +++ b/tests/unit/SidebarTest.ts @@ -12,6 +12,7 @@ import wrapOnyxWithWaitForBatchedUpdates from '../utils/wrapOnyxWithWaitForBatch // Be sure to include the mocked Permissions and Expensicons libraries as well as the usePermissions hook or else the beta tests won't work jest.mock('@src/libs/Permissions'); jest.mock('@src/hooks/usePermissions.ts'); +jest.mock('@src/hooks/useActiveWorkspaceFromNavigationState') jest.mock('@src/components/Icon/Expensicons'); describe('Sidebar', () => { From 3be474286b2de7510841a1e2d80baf45f203ba4f Mon Sep 17 00:00:00 2001 From: Wojciech Boman Date: Wed, 10 Apr 2024 17:55:50 +0200 Subject: [PATCH 4/9] Run prettier --- tests/unit/SidebarOrderTest.ts | 2 +- tests/unit/SidebarTest.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/SidebarOrderTest.ts b/tests/unit/SidebarOrderTest.ts index ac74fb50245b..e0858c4e78b9 100644 --- a/tests/unit/SidebarOrderTest.ts +++ b/tests/unit/SidebarOrderTest.ts @@ -14,7 +14,7 @@ import wrapOnyxWithWaitForBatchedUpdates from '../utils/wrapOnyxWithWaitForBatch jest.mock('@libs/Permissions'); jest.mock('@hooks/usePermissions.ts'); jest.mock('@components/Icon/Expensicons'); -jest.mock('@src/hooks/useActiveWorkspaceFromNavigationState') +jest.mock('@src/hooks/useActiveWorkspaceFromNavigationState'); const ONYXKEYS = { PERSONAL_DETAILS_LIST: 'personalDetailsList', diff --git a/tests/unit/SidebarTest.ts b/tests/unit/SidebarTest.ts index ba2d950232b5..e35a479c1add 100644 --- a/tests/unit/SidebarTest.ts +++ b/tests/unit/SidebarTest.ts @@ -12,7 +12,7 @@ import wrapOnyxWithWaitForBatchedUpdates from '../utils/wrapOnyxWithWaitForBatch // Be sure to include the mocked Permissions and Expensicons libraries as well as the usePermissions hook or else the beta tests won't work jest.mock('@src/libs/Permissions'); jest.mock('@src/hooks/usePermissions.ts'); -jest.mock('@src/hooks/useActiveWorkspaceFromNavigationState') +jest.mock('@src/hooks/useActiveWorkspaceFromNavigationState'); jest.mock('@src/components/Icon/Expensicons'); describe('Sidebar', () => { From a3c565ae84371cb37342c62946603356a138551c Mon Sep 17 00:00:00 2001 From: Wojciech Boman Date: Thu, 11 Apr 2024 14:03:48 +0200 Subject: [PATCH 5/9] Fix sidebar links perf tests --- tests/perf-test/SidebarLinks.perf-test.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/perf-test/SidebarLinks.perf-test.tsx b/tests/perf-test/SidebarLinks.perf-test.tsx index 2848015d5c63..036d4ea84ff9 100644 --- a/tests/perf-test/SidebarLinks.perf-test.tsx +++ b/tests/perf-test/SidebarLinks.perf-test.tsx @@ -10,6 +10,7 @@ import wrapOnyxWithWaitForBatchedUpdates from '../utils/wrapOnyxWithWaitForBatch jest.mock('@libs/Permissions'); jest.mock('@hooks/usePermissions.ts'); +jest.mock('@src/hooks/useActiveWorkspaceFromNavigationState'); jest.mock('@libs/Navigation/Navigation'); jest.mock('@components/Icon/Expensicons'); From 4b1681c76db57613511ac7bbe4be900bbe7954fd Mon Sep 17 00:00:00 2001 From: Wojciech Boman Date: Fri, 12 Apr 2024 08:44:20 +0200 Subject: [PATCH 6/9] Add a comment to useActiveWorkspaceFromNavigationState --- src/hooks/useActiveWorkspaceFromNavigationState.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/hooks/useActiveWorkspaceFromNavigationState.ts b/src/hooks/useActiveWorkspaceFromNavigationState.ts index 8f9da3d9df37..63a6865a516b 100644 --- a/src/hooks/useActiveWorkspaceFromNavigationState.ts +++ b/src/hooks/useActiveWorkspaceFromNavigationState.ts @@ -5,6 +5,7 @@ import type {BottomTabNavigatorParamList} from '@libs/Navigation/types'; * Get the currently selected policy ID stored in the navigation state. This hook should only be called only from screens in BottomTab. */ function useActiveWorkspaceFromNavigationState() { + // The last policyID value is always stored in the last route in BottomTabNavigator. const activeWorkpsaceID = useNavigationState((state) => state.routes.at(-1)?.params?.policyID); return activeWorkpsaceID; From ae46fdf8eb05d8ed6084cba1c66b521379b275d0 Mon Sep 17 00:00:00 2001 From: Wojciech Boman Date: Mon, 15 Apr 2024 11:23:57 +0200 Subject: [PATCH 7/9] Add warning to useActiveWorkspaceFromNavigationState --- src/hooks/useActiveWorkspaceFromNavigationState.ts | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/hooks/useActiveWorkspaceFromNavigationState.ts b/src/hooks/useActiveWorkspaceFromNavigationState.ts index 63a6865a516b..5b117816d40f 100644 --- a/src/hooks/useActiveWorkspaceFromNavigationState.ts +++ b/src/hooks/useActiveWorkspaceFromNavigationState.ts @@ -1,12 +1,21 @@ import {useNavigationState} from '@react-navigation/native'; +import Log from '@libs/Log'; import type {BottomTabNavigatorParamList} from '@libs/Navigation/types'; +import SCREENS from '@src/SCREENS'; /** * Get the currently selected policy ID stored in the navigation state. This hook should only be called only from screens in BottomTab. */ function useActiveWorkspaceFromNavigationState() { // The last policyID value is always stored in the last route in BottomTabNavigator. - const activeWorkpsaceID = useNavigationState((state) => state.routes.at(-1)?.params?.policyID); + const activeWorkpsaceID = useNavigationState((state) => { + // SCREENS.HOME is a screen located in the BottomTabNavigator, if it's not in state.routeNames it means that this hook was called from a screen in another navigator. + if (!state.routeNames.includes(SCREENS.HOME)) { + Log.warn('useActiveWorkspaceFromNavigationState should be called only from BottomTab screens'); + } + + return state.routes.at(-1)?.params?.policyID; + }); return activeWorkpsaceID; } From 63cf4906db781d64cc35a74eb65281c0aacd0a2b Mon Sep 17 00:00:00 2001 From: Wojciech Boman Date: Thu, 25 Apr 2024 11:12:06 +0200 Subject: [PATCH 8/9] Add docs to useActiveWorkspaceFromNavigationState --- src/hooks/useActiveWorkspaceFromNavigationState.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/hooks/useActiveWorkspaceFromNavigationState.ts b/src/hooks/useActiveWorkspaceFromNavigationState.ts index 5b117816d40f..9b8db5dfcab4 100644 --- a/src/hooks/useActiveWorkspaceFromNavigationState.ts +++ b/src/hooks/useActiveWorkspaceFromNavigationState.ts @@ -5,6 +5,10 @@ import SCREENS from '@src/SCREENS'; /** * Get the currently selected policy ID stored in the navigation state. This hook should only be called only from screens in BottomTab. + * Differences between this hook and useActiveWorkspace: + * - useActiveWorkspaceFromNavigationState reads the active workspace id directly from the navigation state, it's a bit slower than useActiveWorkspace and it can be called only from BottomTabScreens. + * It allows to read a value of policyID immediately after the update. + * - useActiveWorkspace allows to read the current policyID anywhere, it's faster because it doesn't require searching in the navigation state. */ function useActiveWorkspaceFromNavigationState() { // The last policyID value is always stored in the last route in BottomTabNavigator. From 04f34ff58c0deaf177bf9eccd8e1ade1d9027fc6 Mon Sep 17 00:00:00 2001 From: Wojciech Boman Date: Tue, 30 Apr 2024 21:28:05 +0200 Subject: [PATCH 9/9] Fix imports in PolicyUtils --- src/libs/PolicyUtils.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libs/PolicyUtils.ts b/src/libs/PolicyUtils.ts index c38918cce00c..5cce7eb6aa5b 100644 --- a/src/libs/PolicyUtils.ts +++ b/src/libs/PolicyUtils.ts @@ -423,6 +423,7 @@ export { isSubmitAndClose, isTaxTrackingEnabled, shouldShowPolicy, + getActiveAdminWorkspaces, canSendInvoice, };