From 8768859e04357c3bfd0992c30de0e21c2d7d5605 Mon Sep 17 00:00:00 2001 From: gijoe0295 Date: Fri, 20 Sep 2024 09:42:33 +0700 Subject: [PATCH 1/5] fix: deleted workspace with invoices is accessible by url --- src/pages/workspace/WorkspaceInitialPage.tsx | 11 ++++++----- .../workspace/WorkspacePageWithSections.tsx | 19 +++++++++---------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/pages/workspace/WorkspaceInitialPage.tsx b/src/pages/workspace/WorkspaceInitialPage.tsx index fd7a45e31acb..7f51af6192a5 100644 --- a/src/pages/workspace/WorkspaceInitialPage.tsx +++ b/src/pages/workspace/WorkspaceInitialPage.tsx @@ -94,6 +94,7 @@ function WorkspaceInitialPage({policyDraft, policy: policyProp, policyCategories const [isCurrencyModalOpen, setIsCurrencyModalOpen] = useState(false); const hasPolicyCreationError = !!(policy?.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD && !isEmptyObject(policy.errors)); const [connectionSyncProgress] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_CONNECTION_SYNC_PROGRESS}${policy?.id}`); + const [currentUserLogin] = useOnyx(ONYXKEYS.SESSION, {selector: (session) => session?.email}); const hasSyncError = PolicyUtils.hasSyncError(policy, isConnectionInProgress(connectionSyncProgress, policy)); const waitForNavigate = useWaitForNavigation(); const {singleExecution, isExecuting} = useSingleExecution(); @@ -306,11 +307,11 @@ function WorkspaceInitialPage({policyDraft, policy: policyProp, policyCategories const prevProtectedMenuItems = usePrevious(protectedCollectPolicyMenuItems); const enabledItem = protectedCollectPolicyMenuItems.find((curItem) => !prevProtectedMenuItems.some((prevItem) => curItem.routeName === prevItem.routeName)); + const shouldShowPolicy = useMemo(() => PolicyUtils.shouldShowPolicy(policy, isOffline, currentUserLogin), [policy, isOffline, currentUserLogin]); + const prevShouldShowPolicy = usePrevious(shouldShowPolicy); + // We check shouldShowPolicy and prevShouldShowPolicy to prevent the NotFound view from showing right after we delete the workspace // eslint-disable-next-line rulesdir/no-negated-variables - const shouldShowNotFoundPage = - isEmptyObject(policy) || - // We check isPendingDelete for both policy and prevPolicy to prevent the NotFound view from showing right after we delete the workspace - (PolicyUtils.isPendingDeletePolicy(policy) && PolicyUtils.isPendingDeletePolicy(prevPolicy)); + const shouldShowNotFoundPage = isEmptyObject(policy) || (!shouldShowPolicy && !prevShouldShowPolicy); useEffect(() => { if (isEmptyObject(prevPolicy) || PolicyUtils.isPendingDeletePolicy(prevPolicy) || !PolicyUtils.isPendingDeletePolicy(policy)) { @@ -360,7 +361,7 @@ function WorkspaceInitialPage({policyDraft, policy: policyProp, policyCategories onBackButtonPress={Navigation.dismissModal} onLinkPress={Navigation.resetToHome} shouldShow={shouldShowNotFoundPage} - subtitleKey={isEmptyObject(policy) ? undefined : 'workspace.common.notAuthorized'} + subtitleKey={shouldShowPolicy ? 'workspace.common.notAuthorized' : undefined} > fetchData(policyID, shouldSkipVBBACall)}); + const {isOffline} = useNetwork({onReconnect: () => fetchData(policyID, shouldSkipVBBACall)}); + const [currentUserLogin] = useOnyx(ONYXKEYS.SESSION, {selector: (session) => session?.email}); // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing const isLoading = (reimbursementAccount?.isLoading || isPageLoading) ?? true; @@ -148,7 +149,6 @@ function WorkspacePageWithSections({ const {shouldUseNarrowLayout} = useResponsiveLayout(); const firstRender = useRef(showLoadingAsFirstRender); const isFocused = useIsFocused(); - const prevPolicy = usePrevious(policy); useEffect(() => { // Because isLoading is false before merging in Onyx, we need firstRender ref to display loading page as well before isLoading is change to true @@ -161,19 +161,18 @@ function WorkspacePageWithSections({ }, [policyID, shouldSkipVBBACall]), ); + const shouldShowPolicy = useMemo(() => PolicyUtils.shouldShowPolicy(policy, isOffline, currentUserLogin), [policy, isOffline, currentUserLogin]); + const prevShouldShowPolicy = usePrevious(shouldShowPolicy); const shouldShow = useMemo(() => { // If the policy object doesn't exist or contains only error data, we shouldn't display it. if (((isEmptyObject(policy) || (Object.keys(policy).length === 1 && !isEmptyObject(policy.errors))) && isEmptyObject(policyDraft)) || shouldShowNotFoundPage) { return true; } - // We check isPendingDelete for both policy and prevPolicy to prevent the NotFound view from showing right after we delete the workspace - return ( - (!isEmptyObject(policy) && !PolicyUtils.isPolicyAdmin(policy) && !shouldShowNonAdmin) || - (PolicyUtils.isPendingDeletePolicy(policy) && PolicyUtils.isPendingDeletePolicy(prevPolicy)) - ); + // We check shouldShowPolicy and prevShouldShowPolicy to prevent the NotFound view from showing right after we delete the workspace + return (!isEmptyObject(policy) && !PolicyUtils.isPolicyAdmin(policy) && !shouldShowNonAdmin) || (!shouldShowPolicy && !prevShouldShowPolicy); // eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps - }, [policy, shouldShowNonAdmin]); + }, [policy, shouldShowNonAdmin, shouldShowPolicy, prevShouldShowPolicy]); return ( Date: Sat, 28 Sep 2024 01:33:25 +0700 Subject: [PATCH 2/5] use prevPolicy --- src/pages/workspace/WorkspaceInitialPage.tsx | 2 +- src/pages/workspace/WorkspacePageWithSections.tsx | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/pages/workspace/WorkspaceInitialPage.tsx b/src/pages/workspace/WorkspaceInitialPage.tsx index 33330be8d9fb..156282c9f281 100644 --- a/src/pages/workspace/WorkspaceInitialPage.tsx +++ b/src/pages/workspace/WorkspaceInitialPage.tsx @@ -298,7 +298,7 @@ function WorkspaceInitialPage({policyDraft, policy: policyProp, route}: Workspac const enabledItem = protectedCollectPolicyMenuItems.find((curItem) => !prevProtectedMenuItems.some((prevItem) => curItem.routeName === prevItem.routeName)); const shouldShowPolicy = useMemo(() => PolicyUtils.shouldShowPolicy(policy, isOffline, currentUserLogin), [policy, isOffline, currentUserLogin]); - const prevShouldShowPolicy = usePrevious(shouldShowPolicy); + const prevShouldShowPolicy = useMemo(() => PolicyUtils.shouldShowPolicy(prevPolicy, isOffline, currentUserLogin), [prevPolicy, isOffline, currentUserLogin]); // We check shouldShowPolicy and prevShouldShowPolicy to prevent the NotFound view from showing right after we delete the workspace // eslint-disable-next-line rulesdir/no-negated-variables const shouldShowNotFoundPage = isEmptyObject(policy) || (!shouldShowPolicy && !prevShouldShowPolicy); diff --git a/src/pages/workspace/WorkspacePageWithSections.tsx b/src/pages/workspace/WorkspacePageWithSections.tsx index fec440da970a..cf473ebec0ba 100644 --- a/src/pages/workspace/WorkspacePageWithSections.tsx +++ b/src/pages/workspace/WorkspacePageWithSections.tsx @@ -139,6 +139,7 @@ function WorkspacePageWithSections({ const {shouldUseNarrowLayout} = useResponsiveLayout(); const firstRender = useRef(showLoadingAsFirstRender); const isFocused = useIsFocused(); + const prevPolicy = usePrevious(policy); useEffect(() => { // Because isLoading is false before merging in Onyx, we need firstRender ref to display loading page as well before isLoading is change to true @@ -152,7 +153,7 @@ function WorkspacePageWithSections({ ); const shouldShowPolicy = useMemo(() => PolicyUtils.shouldShowPolicy(policy, isOffline, currentUserLogin), [policy, isOffline, currentUserLogin]); - const prevShouldShowPolicy = usePrevious(shouldShowPolicy); + const prevShouldShowPolicy = useMemo(() => PolicyUtils.shouldShowPolicy(prevPolicy, isOffline, currentUserLogin), [prevPolicy, isOffline, currentUserLogin]); const shouldShow = useMemo(() => { // If the policy object doesn't exist or contains only error data, we shouldn't display it. if (((isEmptyObject(policy) || (Object.keys(policy).length === 1 && !isEmptyObject(policy.errors))) && isEmptyObject(policyDraft)) || shouldShowNotFoundPage) { From 093f361d0cee939407b5872c889ddc565ff85881 Mon Sep 17 00:00:00 2001 From: gijoe0295 Date: Sat, 28 Sep 2024 01:33:41 +0700 Subject: [PATCH 3/5] remove redundant changes --- src/pages/workspace/WorkspacePageWithSections.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/src/pages/workspace/WorkspacePageWithSections.tsx b/src/pages/workspace/WorkspacePageWithSections.tsx index cf473ebec0ba..26175c9793d9 100644 --- a/src/pages/workspace/WorkspacePageWithSections.tsx +++ b/src/pages/workspace/WorkspacePageWithSections.tsx @@ -140,7 +140,6 @@ function WorkspacePageWithSections({ const firstRender = useRef(showLoadingAsFirstRender); const isFocused = useIsFocused(); const prevPolicy = usePrevious(policy); - useEffect(() => { // Because isLoading is false before merging in Onyx, we need firstRender ref to display loading page as well before isLoading is change to true firstRender.current = false; From e441bc600890f40f8cf4cf1892616320fbf3fbe6 Mon Sep 17 00:00:00 2001 From: gijoe0295 Date: Sat, 16 Nov 2024 00:03:01 +0700 Subject: [PATCH 4/5] do not reuseConnection in withPolicy to avoid infinite loading --- src/pages/workspace/withPolicy.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pages/workspace/withPolicy.tsx b/src/pages/workspace/withPolicy.tsx index 16f05fa3e324..5060761359f6 100644 --- a/src/pages/workspace/withPolicy.tsx +++ b/src/pages/workspace/withPolicy.tsx @@ -78,7 +78,7 @@ export default function ( function WithPolicy(props: Omit, ref: ForwardedRef) { const policyID = getPolicyIDFromRoute(props.route as PolicyRoute); - const [policy, policyResults] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`); + const [policy, policyResults] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, {reuseConnection: false}); const [policyDraft, policyDraftResults] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_DRAFTS}${policyID}`); const isLoadingPolicy = isLoadingOnyxValue(policyResults, policyDraftResults); From 649137c05a4e56c8c46630e902475bf0a572e525 Mon Sep 17 00:00:00 2001 From: gijoe0295 Date: Tue, 19 Nov 2024 15:30:34 +0700 Subject: [PATCH 5/5] reference issue --- src/pages/workspace/withPolicy.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/src/pages/workspace/withPolicy.tsx b/src/pages/workspace/withPolicy.tsx index 5060761359f6..6f8d51316d94 100644 --- a/src/pages/workspace/withPolicy.tsx +++ b/src/pages/workspace/withPolicy.tsx @@ -78,6 +78,7 @@ export default function ( function WithPolicy(props: Omit, ref: ForwardedRef) { const policyID = getPolicyIDFromRoute(props.route as PolicyRoute); + // Disable reuseConnection to temporarily fix the infinite loading status after Onyx.set(null). Reference: https://github.com/Expensify/App/issues/52640 const [policy, policyResults] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, {reuseConnection: false}); const [policyDraft, policyDraftResults] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_DRAFTS}${policyID}`); const isLoadingPolicy = isLoadingOnyxValue(policyResults, policyDraftResults);