From 0cbd1e8ec501fe7ba29f5678fec00e83b9d907b8 Mon Sep 17 00:00:00 2001 From: Ranjeet Baraik <84462743+anadi45@users.noreply.github.com> Date: Thu, 30 Oct 2025 22:01:26 +0530 Subject: [PATCH] fix: login redirection to active objects (#15366) Fixes - https://github.com/twentyhq/twenty/issues/15364 - Removed `activeNonSystemObjectMetadataItems` from the hook as it was not necessary. Hook use `alphaSortedActiveNonSystemObjectMetadataItems` now. - Correctly check for read permissions. --------- Co-authored-by: Charles Bochet --- .../effect-components/PageChangeEffect.tsx | 17 ++++++++- .../states/isAppEffectRedirectEnabledState.ts | 5 +++ .../src/modules/auth/hooks/useAuth.ts | 13 ++++++- .../hooks/useDefaultHomePagePath.ts | 38 ++++++------------- .../ObjectMetadataItemsProvider.tsx | 8 ++-- .../hooks/useLoadMockedObjectMetadataItems.ts | 14 ++++--- .../hooks/useRefreshObjectMetadataItems.ts | 15 +++++++- ...isAppWaitingForFreshObjectMetadataState.ts | 5 --- .../states/shouldAppBeLoadingState.ts | 5 +++ .../__stories__/WidgetPlaceholder.stories.tsx | 4 +- .../__stories__/WidgetRenderer.stories.tsx | 4 +- .../admin-panel/hooks/useImpersonationAuth.ts | 14 ++++--- 12 files changed, 87 insertions(+), 55 deletions(-) create mode 100644 packages/twenty-front/src/modules/app/states/isAppEffectRedirectEnabledState.ts delete mode 100644 packages/twenty-front/src/modules/object-metadata/states/isAppWaitingForFreshObjectMetadataState.ts create mode 100644 packages/twenty-front/src/modules/object-metadata/states/shouldAppBeLoadingState.ts diff --git a/packages/twenty-front/src/modules/app/effect-components/PageChangeEffect.tsx b/packages/twenty-front/src/modules/app/effect-components/PageChangeEffect.tsx index 79164665a7..3bcfe8c126 100644 --- a/packages/twenty-front/src/modules/app/effect-components/PageChangeEffect.tsx +++ b/packages/twenty-front/src/modules/app/effect-components/PageChangeEffect.tsx @@ -12,6 +12,7 @@ import { useEventTracker, } from '@/analytics/hooks/useEventTracker'; import { useExecuteTasksOnAnyLocationChange } from '@/app/hooks/useExecuteTasksOnAnyLocationChange'; +import { isAppEffectRedirectEnabledState } from '@/app/states/isAppEffectRedirectEnabledState'; import { useRequestFreshCaptchaToken } from '@/captcha/hooks/useRequestFreshCaptchaToken'; import { isCaptchaScriptLoadedState } from '@/captcha/states/isCaptchaScriptLoadedState'; import { isCaptchaRequiredForPath } from '@/captcha/utils/isCaptchaRequiredForPath'; @@ -88,6 +89,10 @@ export const PageChangeEffect = () => { const { executeTasksOnAnyLocationChange } = useExecuteTasksOnAnyLocationChange(); + const isAppEffectRedirectEnabled = useRecoilValue( + isAppEffectRedirectEnabledState, + ); + const { closeCommandMenu } = useCommandMenu(); const { resetFocusStackToFocusItem } = useResetFocusStackToFocusItem(); @@ -110,10 +115,18 @@ export const PageChangeEffect = () => { useEffect(() => { initializeQueryParamState(); - if (isDefined(pageChangeEffectNavigateLocation)) { + if ( + isDefined(pageChangeEffectNavigateLocation) && + isAppEffectRedirectEnabled + ) { navigate(pageChangeEffectNavigateLocation); } - }, [navigate, pageChangeEffectNavigateLocation, initializeQueryParamState]); + }, [ + navigate, + pageChangeEffectNavigateLocation, + initializeQueryParamState, + isAppEffectRedirectEnabled, + ]); useEffect(() => { const isLeavingRecordIndexPage = !!matchPath( diff --git a/packages/twenty-front/src/modules/app/states/isAppEffectRedirectEnabledState.ts b/packages/twenty-front/src/modules/app/states/isAppEffectRedirectEnabledState.ts new file mode 100644 index 0000000000..b931e89aa8 --- /dev/null +++ b/packages/twenty-front/src/modules/app/states/isAppEffectRedirectEnabledState.ts @@ -0,0 +1,5 @@ +import { createState } from 'twenty-ui/utilities'; +export const isAppEffectRedirectEnabledState = createState({ + key: 'isAppEffectRedirectEnabledState', + defaultValue: true, +}); diff --git a/packages/twenty-front/src/modules/auth/hooks/useAuth.ts b/packages/twenty-front/src/modules/auth/hooks/useAuth.ts index a601019458..290820d376 100644 --- a/packages/twenty-front/src/modules/auth/hooks/useAuth.ts +++ b/packages/twenty-front/src/modules/auth/hooks/useAuth.ts @@ -66,10 +66,14 @@ import { type AuthToken } from '~/generated/graphql'; import { cookieStorage } from '~/utils/cookie-storage'; import { getWorkspaceUrl } from '~/utils/getWorkspaceUrl'; import { loginTokenState } from '../states/loginTokenState'; +import { isAppEffectRedirectEnabledState } from '@/app/states/isAppEffectRedirectEnabledState'; export const useAuth = () => { const setTokenPair = useSetRecoilState(tokenPairState); const setLoginToken = useSetRecoilState(loginTokenState); + const setIsAppEffectRedirectEnabled = useSetRecoilState( + isAppEffectRedirectEnabledState, + ); const { origin } = useOrigin(); const { requestFreshCaptchaToken } = useRequestFreshCaptchaToken(); @@ -317,13 +321,20 @@ export const useAuth = () => { async (authTokens: AuthTokenPair) => { handleSetAuthTokens(authTokens); + setIsAppEffectRedirectEnabled(false); + // TODO: We can't parallelize this yet because when loadCurrentUSer is loaded // then UserProvider updates its children and PrefetchDataProvider is then triggered // which requires the correct metadata to be loaded (not the mocks) await loadCurrentUser(); await refreshObjectMetadataItems(); }, - [loadCurrentUser, handleSetAuthTokens, refreshObjectMetadataItems], + [ + loadCurrentUser, + handleSetAuthTokens, + refreshObjectMetadataItems, + setIsAppEffectRedirectEnabled, + ], ); const handleGetAuthTokensFromLoginToken = useCallback( diff --git a/packages/twenty-front/src/modules/navigation/hooks/useDefaultHomePagePath.ts b/packages/twenty-front/src/modules/navigation/hooks/useDefaultHomePagePath.ts index 8ca54e82f7..c4ed6db14e 100644 --- a/packages/twenty-front/src/modules/navigation/hooks/useDefaultHomePagePath.ts +++ b/packages/twenty-front/src/modules/navigation/hooks/useDefaultHomePagePath.ts @@ -16,10 +16,8 @@ export const useDefaultHomePagePath = () => { const currentUser = useRecoilValue(currentUserState); const { objectPermissionsByObjectMetadataId } = useObjectPermissions(); - const { - activeNonSystemObjectMetadataItems, - alphaSortedActiveNonSystemObjectMetadataItems, - } = useFilteredObjectMetadataItems(); + const { alphaSortedActiveNonSystemObjectMetadataItems } = + useFilteredObjectMetadataItems(); const readableAlphaSortedActiveNonSystemObjectMetadataItems = useMemo(() => { return alphaSortedActiveNonSystemObjectMetadataItems.filter((item) => { @@ -36,11 +34,11 @@ export const useDefaultHomePagePath = () => { const getActiveObjectMetadataItemMatchingId = useCallback( (objectMetadataId: string) => { - return activeNonSystemObjectMetadataItems.find( + return readableAlphaSortedActiveNonSystemObjectMetadataItems.find( (item) => item.id === objectMetadataId, ); }, - [activeNonSystemObjectMetadataItems], + [readableAlphaSortedActiveNonSystemObjectMetadataItems], ); const getFirstView = useRecoilCallback(({ snapshot }) => { @@ -76,20 +74,13 @@ export const useDefaultHomePagePath = () => { .getLoadable(lastVisitedObjectMetadataItemIdState) .getValue(); - if ( - !isDefined(lastVisitedObjectMetadataItemId) || - !getObjectPermissionsFromMapByObjectMetadataId({ - objectPermissionsByObjectMetadataId, - objectMetadataId: lastVisitedObjectMetadataItemId, - }).canReadObjectRecords - ) { - return firstObjectPathInfo; - } - - const lastVisitedObjectMetadataItem = - getActiveObjectMetadataItemMatchingId( - lastVisitedObjectMetadataItemId, - ); + const lastVisitedObjectMetadataItem = isDefined( + lastVisitedObjectMetadataItemId, + ) + ? getActiveObjectMetadataItemMatchingId( + lastVisitedObjectMetadataItemId, + ) + : undefined; if (isDefined(lastVisitedObjectMetadataItem)) { return { @@ -101,12 +92,7 @@ export const useDefaultHomePagePath = () => { return firstObjectPathInfo; }; }, - [ - firstObjectPathInfo, - getActiveObjectMetadataItemMatchingId, - getFirstView, - objectPermissionsByObjectMetadataId, - ], + [firstObjectPathInfo, getActiveObjectMetadataItemMatchingId, getFirstView], ); const defaultHomePagePath = useMemo(() => { diff --git a/packages/twenty-front/src/modules/object-metadata/components/ObjectMetadataItemsProvider.tsx b/packages/twenty-front/src/modules/object-metadata/components/ObjectMetadataItemsProvider.tsx index 0065020c7c..fde9228076 100644 --- a/packages/twenty-front/src/modules/object-metadata/components/ObjectMetadataItemsProvider.tsx +++ b/packages/twenty-front/src/modules/object-metadata/components/ObjectMetadataItemsProvider.tsx @@ -2,8 +2,8 @@ import React from 'react'; import { useRecoilValue } from 'recoil'; import { PreComputedChipGeneratorsProvider } from '@/object-metadata/components/PreComputedChipGeneratorsProvider'; -import { isAppWaitingForFreshObjectMetadataState } from '@/object-metadata/states/isAppWaitingForFreshObjectMetadataState'; import { objectMetadataItemsState } from '@/object-metadata/states/objectMetadataItemsState'; +import { shouldAppBeLoadingState } from '@/object-metadata/states/shouldAppBeLoadingState'; import { UserOrMetadataLoader } from '~/loading/components/UserOrMetadataLoader'; export const ObjectMetadataItemsProvider = ({ @@ -11,12 +11,10 @@ export const ObjectMetadataItemsProvider = ({ }: React.PropsWithChildren) => { const objectMetadataItems = useRecoilValue(objectMetadataItemsState); - const isAppWaitingForFreshObjectMetadata = useRecoilValue( - isAppWaitingForFreshObjectMetadataState, - ); + const shouldAppBeLoading = useRecoilValue(shouldAppBeLoadingState); const shouldDisplayChildren = - !isAppWaitingForFreshObjectMetadata && objectMetadataItems.length > 0; + !shouldAppBeLoading && objectMetadataItems.length > 0; return ( <> diff --git a/packages/twenty-front/src/modules/object-metadata/hooks/useLoadMockedObjectMetadataItems.ts b/packages/twenty-front/src/modules/object-metadata/hooks/useLoadMockedObjectMetadataItems.ts index 3bced0ba53..9c6d879d59 100644 --- a/packages/twenty-front/src/modules/object-metadata/hooks/useLoadMockedObjectMetadataItems.ts +++ b/packages/twenty-front/src/modules/object-metadata/hooks/useLoadMockedObjectMetadataItems.ts @@ -1,5 +1,6 @@ -import { isAppWaitingForFreshObjectMetadataState } from '@/object-metadata/states/isAppWaitingForFreshObjectMetadataState'; +import { isAppEffectRedirectEnabledState } from '@/app/states/isAppEffectRedirectEnabledState'; import { objectMetadataItemsState } from '@/object-metadata/states/objectMetadataItemsState'; +import { shouldAppBeLoadingState } from '@/object-metadata/states/shouldAppBeLoadingState'; import { useRecoilCallback } from 'recoil'; import { generatedMockObjectMetadataItems } from '~/testing/utils/generatedMockObjectMetadataItems'; import { isDeeplyEqual } from '~/utils/isDeeplyEqual'; @@ -17,12 +18,15 @@ export const useLoadMockedObjectMetadataItems = () => { set(objectMetadataItemsState, generatedMockObjectMetadataItems); } + if (snapshot.getLoadable(shouldAppBeLoadingState).getValue() === true) { + set(shouldAppBeLoadingState, false); + } + if ( - snapshot - .getLoadable(isAppWaitingForFreshObjectMetadataState) - .getValue() === true + snapshot.getLoadable(isAppEffectRedirectEnabledState).getValue() === + false ) { - set(isAppWaitingForFreshObjectMetadataState, false); + set(isAppEffectRedirectEnabledState, true); } }, [], diff --git a/packages/twenty-front/src/modules/object-metadata/hooks/useRefreshObjectMetadataItems.ts b/packages/twenty-front/src/modules/object-metadata/hooks/useRefreshObjectMetadataItems.ts index 0301a311c2..ca0883b3ed 100644 --- a/packages/twenty-front/src/modules/object-metadata/hooks/useRefreshObjectMetadataItems.ts +++ b/packages/twenty-front/src/modules/object-metadata/hooks/useRefreshObjectMetadataItems.ts @@ -1,7 +1,8 @@ +import { isAppEffectRedirectEnabledState } from '@/app/states/isAppEffectRedirectEnabledState'; import { currentUserWorkspaceState } from '@/auth/states/currentUserWorkspaceState'; import { FIND_MANY_OBJECT_METADATA_ITEMS } from '@/object-metadata/graphql/queries'; -import { isAppWaitingForFreshObjectMetadataState } from '@/object-metadata/states/isAppWaitingForFreshObjectMetadataState'; import { objectMetadataItemsState } from '@/object-metadata/states/objectMetadataItemsState'; +import { shouldAppBeLoadingState } from '@/object-metadata/states/shouldAppBeLoadingState'; import { type ObjectMetadataItem } from '@/object-metadata/types/ObjectMetadataItem'; import { enrichObjectMetadataItemsWithPermissions } from '@/object-metadata/utils/enrichObjectMetadataItemsWithPermissions'; import { mapPaginatedObjectMetadataItemsToObjectMetadataItems } from '@/object-metadata/utils/mapPaginatedObjectMetadataItemsToObjectMetadataItems'; @@ -75,7 +76,17 @@ export const useRefreshObjectMetadataItems = ( newObjectMetadataItems.length > 0 ) { set(objectMetadataItemsState, newObjectMetadataItems); - set(isAppWaitingForFreshObjectMetadataState, false); + } + + if (snapshot.getLoadable(shouldAppBeLoadingState).getValue() === true) { + set(shouldAppBeLoadingState, false); + } + + if ( + snapshot.getLoadable(isAppEffectRedirectEnabledState).getValue() === + false + ) { + set(isAppEffectRedirectEnabledState, true); } return newObjectMetadataItems; diff --git a/packages/twenty-front/src/modules/object-metadata/states/isAppWaitingForFreshObjectMetadataState.ts b/packages/twenty-front/src/modules/object-metadata/states/isAppWaitingForFreshObjectMetadataState.ts deleted file mode 100644 index b9ba7290cb..0000000000 --- a/packages/twenty-front/src/modules/object-metadata/states/isAppWaitingForFreshObjectMetadataState.ts +++ /dev/null @@ -1,5 +0,0 @@ -import { createState } from 'twenty-ui/utilities'; -export const isAppWaitingForFreshObjectMetadataState = createState({ - key: 'isAppWaitingForFreshObjectMetadataState', - defaultValue: false, -}); diff --git a/packages/twenty-front/src/modules/object-metadata/states/shouldAppBeLoadingState.ts b/packages/twenty-front/src/modules/object-metadata/states/shouldAppBeLoadingState.ts new file mode 100644 index 0000000000..d17904fda3 --- /dev/null +++ b/packages/twenty-front/src/modules/object-metadata/states/shouldAppBeLoadingState.ts @@ -0,0 +1,5 @@ +import { createState } from 'twenty-ui/utilities'; +export const shouldAppBeLoadingState = createState({ + key: 'shouldAppBeLoadingState', + defaultValue: false, +}); diff --git a/packages/twenty-front/src/modules/page-layout/widgets/components/__stories__/WidgetPlaceholder.stories.tsx b/packages/twenty-front/src/modules/page-layout/widgets/components/__stories__/WidgetPlaceholder.stories.tsx index 9474cf7224..e5b0aaf665 100644 --- a/packages/twenty-front/src/modules/page-layout/widgets/components/__stories__/WidgetPlaceholder.stories.tsx +++ b/packages/twenty-front/src/modules/page-layout/widgets/components/__stories__/WidgetPlaceholder.stories.tsx @@ -1,5 +1,5 @@ -import { isAppWaitingForFreshObjectMetadataState } from '@/object-metadata/states/isAppWaitingForFreshObjectMetadataState'; import { objectMetadataItemsState } from '@/object-metadata/states/objectMetadataItemsState'; +import { shouldAppBeLoadingState } from '@/object-metadata/states/shouldAppBeLoadingState'; import { PageLayoutTestWrapper } from '@/page-layout/hooks/__tests__/PageLayoutTestWrapper'; import { WidgetPlaceholder } from '@/page-layout/widgets/components/WidgetPlaceholder'; import { type Meta, type StoryObj } from '@storybook/react'; @@ -18,7 +18,7 @@ const meta: Meta = { objectMetadataItemsState, generatedMockObjectMetadataItems, ); - snapshot.set(isAppWaitingForFreshObjectMetadataState, false); + snapshot.set(shouldAppBeLoadingState, false); }; return ( diff --git a/packages/twenty-front/src/modules/page-layout/widgets/components/__stories__/WidgetRenderer.stories.tsx b/packages/twenty-front/src/modules/page-layout/widgets/components/__stories__/WidgetRenderer.stories.tsx index fa3647f54b..3edd838d22 100644 --- a/packages/twenty-front/src/modules/page-layout/widgets/components/__stories__/WidgetRenderer.stories.tsx +++ b/packages/twenty-front/src/modules/page-layout/widgets/components/__stories__/WidgetRenderer.stories.tsx @@ -9,8 +9,8 @@ import { MemoryRouter } from 'react-router-dom'; import { type MutableSnapshot } from 'recoil'; import { ApolloCoreClientContext } from '@/object-metadata/contexts/ApolloCoreClientContext'; -import { isAppWaitingForFreshObjectMetadataState } from '@/object-metadata/states/isAppWaitingForFreshObjectMetadataState'; import { objectMetadataItemsState } from '@/object-metadata/states/objectMetadataItemsState'; +import { shouldAppBeLoadingState } from '@/object-metadata/states/shouldAppBeLoadingState'; import { CoreObjectNameSingular } from '@/object-metadata/types/CoreObjectNameSingular'; import { PageLayoutTestWrapper } from '@/page-layout/hooks/__tests__/PageLayoutTestWrapper'; import { WidgetRenderer } from '@/page-layout/widgets/components/WidgetRenderer'; @@ -122,7 +122,7 @@ const meta: Meta = { objectMetadataItemsState, generatedMockObjectMetadataItems, ); - snapshot.set(isAppWaitingForFreshObjectMetadataState, false); + snapshot.set(shouldAppBeLoadingState, false); }; return ( diff --git a/packages/twenty-front/src/modules/settings/admin-panel/hooks/useImpersonationAuth.ts b/packages/twenty-front/src/modules/settings/admin-panel/hooks/useImpersonationAuth.ts index 228004b44a..157e6600e8 100644 --- a/packages/twenty-front/src/modules/settings/admin-panel/hooks/useImpersonationAuth.ts +++ b/packages/twenty-front/src/modules/settings/admin-panel/hooks/useImpersonationAuth.ts @@ -1,17 +1,21 @@ +import { isAppEffectRedirectEnabledState } from '@/app/states/isAppEffectRedirectEnabledState'; import { useAuth } from '@/auth/hooks/useAuth'; -import { isAppWaitingForFreshObjectMetadataState } from '@/object-metadata/states/isAppWaitingForFreshObjectMetadataState'; +import { shouldAppBeLoadingState } from '@/object-metadata/states/shouldAppBeLoadingState'; import { useSetRecoilState } from 'recoil'; export const useImpersonationAuth = () => { const { getAuthTokensFromLoginToken } = useAuth(); - const setIsAppWaitingForFreshObjectMetadata = useSetRecoilState( - isAppWaitingForFreshObjectMetadataState, + const setShouldAppBeLoading = useSetRecoilState(shouldAppBeLoadingState); + const setIsAppEffectRedirectEnabled = useSetRecoilState( + isAppEffectRedirectEnabledState, ); const executeImpersonationAuth = async (loginToken: string) => { - setIsAppWaitingForFreshObjectMetadata(true); + setShouldAppBeLoading(true); + setIsAppEffectRedirectEnabled(false); await getAuthTokensFromLoginToken(loginToken); - setIsAppWaitingForFreshObjectMetadata(false); + setShouldAppBeLoading(false); + setIsAppEffectRedirectEnabled(true); }; return { executeImpersonationAuth };