From b81ffcc77c09ed5f7e7433e05cb775a5374059bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Bosi?= <71827178+bosiraphael@users.noreply.github.com> Date: Thu, 16 Jan 2025 18:41:19 +0100 Subject: [PATCH] Add viewId to recordIndexId (#9647) Before the `recordIndexId` was the name plural. This caused problems because the component states were the same for every view of an object. When we switched from one view to another, some states weren't reset. This PR fixes this by: - Creating an effect at the same level of page change effect to set the `currentViewId` inside the object `contextStore` - Adding the `currentViewId` to the `recordIndexId` Follow ups: - We need to get rid of `packages/twenty-front/src/modules/views/states/currentViewIdComponentState.ts` and use the context store instead --- packages/twenty-front/nyc.config.cjs | 8 +- .../src/__stories__/AppRouter.stories.tsx | 83 -------------- .../app/components/AppRouterProviders.tsx | 10 +- .../components/ContextStoreViewIdEffect.tsx | 104 ++++++++++++++++++ .../pages/object-record/RecordIndexPage.tsx | 14 ++- 5 files changed, 130 insertions(+), 89 deletions(-) delete mode 100644 packages/twenty-front/src/__stories__/AppRouter.stories.tsx create mode 100644 packages/twenty-front/src/modules/context-store/components/ContextStoreViewIdEffect.tsx diff --git a/packages/twenty-front/nyc.config.cjs b/packages/twenty-front/nyc.config.cjs index 48d309ced..94a92b282 100644 --- a/packages/twenty-front/nyc.config.cjs +++ b/packages/twenty-front/nyc.config.cjs @@ -1,8 +1,8 @@ const globalCoverage = { - branches: 24, - statements: 40, - lines: 40, - functions: 30, + branches: 23, + statements: 39, + lines: 39, + functions: 28, exclude: ['src/generated/**/*'], }; diff --git a/packages/twenty-front/src/__stories__/AppRouter.stories.tsx b/packages/twenty-front/src/__stories__/AppRouter.stories.tsx deleted file mode 100644 index f322919ba..000000000 --- a/packages/twenty-front/src/__stories__/AppRouter.stories.tsx +++ /dev/null @@ -1,83 +0,0 @@ -import { getOperationName } from '@apollo/client/utilities'; -import { jest } from '@storybook/jest'; -import { Meta, StoryObj } from '@storybook/react'; -import { HttpResponse, graphql } from 'msw'; -import { HelmetProvider } from 'react-helmet-async'; -import { RecoilRoot } from 'recoil'; - -import { AppErrorBoundary } from '@/error-handler/components/AppErrorBoundary'; -import indexAppPath from '@/navigation/utils/indexAppPath'; -import { SnackBarProviderScope } from '@/ui/feedback/snack-bar-manager/scopes/SnackBarProviderScope'; -import { GET_CURRENT_USER } from '@/users/graphql/queries/getCurrentUser'; - -import { AppRouter } from '@/app/components/AppRouter'; -import { AppPath } from '@/types/AppPath'; -import { IconsProvider } from 'twenty-ui'; -import { graphqlMocks } from '~/testing/graphqlMocks'; -import { mockedUserData } from '~/testing/mock-data/users'; - -const meta: Meta = { - title: 'App/AppRouter', - component: AppRouter, - decorators: [ - (Story) => { - return ( - - - - - - - - - - - - ); - }, - ], - parameters: { - msw: graphqlMocks, - }, -}; - -export default meta; -export type Story = StoryObj; - -export const Default: Story = { - play: async () => { - jest - .spyOn(indexAppPath, 'getIndexAppPath') - .mockReturnValue('iframe.html' as AppPath); - }, -}; - -export const DarkMode: Story = { - play: async () => { - jest - .spyOn(indexAppPath, 'getIndexAppPath') - .mockReturnValue('iframe.html' as AppPath); - }, - parameters: { - msw: { - handlers: [ - ...graphqlMocks.handlers.filter((handler) => { - return (handler.info as any).operationName !== 'GetCurrentUser'; - }), - graphql.query(getOperationName(GET_CURRENT_USER) ?? '', () => { - return HttpResponse.json({ - data: { - currentUser: { - ...mockedUserData, - workspaceMember: { - ...mockedUserData.workspaceMember, - colorScheme: 'Dark', - }, - }, - }, - }); - }), - ], - }, - }, -}; diff --git a/packages/twenty-front/src/modules/app/components/AppRouterProviders.tsx b/packages/twenty-front/src/modules/app/components/AppRouterProviders.tsx index 43ce095f4..2b2cc8f16 100644 --- a/packages/twenty-front/src/modules/app/components/AppRouterProviders.tsx +++ b/packages/twenty-front/src/modules/app/components/AppRouterProviders.tsx @@ -6,6 +6,7 @@ import { ChromeExtensionSidecarEffect } from '@/chrome-extension-sidecar/compone import { ChromeExtensionSidecarProvider } from '@/chrome-extension-sidecar/components/ChromeExtensionSidecarProvider'; import { ClientConfigProvider } from '@/client-config/components/ClientConfigProvider'; import { ClientConfigProviderEffect } from '@/client-config/components/ClientConfigProviderEffect'; +import { ContextStoreViewIdEffect } from '@/context-store/components/ContextStoreViewIdEffect'; import { PromiseRejectionEffect } from '@/error-handler/components/PromiseRejectionEffect'; import { ApolloMetadataClientProvider } from '@/object-metadata/components/ApolloMetadataClientProvider'; import { ObjectMetadataItemsGater } from '@/object-metadata/components/ObjectMetadataItemsGater'; @@ -21,13 +22,15 @@ import { PageTitle } from '@/ui/utilities/page-title/components/PageTitle'; import { UserProvider } from '@/users/components/UserProvider'; import { UserProviderEffect } from '@/users/components/UserProviderEffect'; import { WorkspaceProviderEffect } from '@/workspace/components/WorkspaceProviderEffect'; +import { isNonEmptyString } from '@sniptt/guards'; import { StrictMode } from 'react'; -import { Outlet, useLocation } from 'react-router-dom'; +import { Outlet, useLocation, useParams } from 'react-router-dom'; import { getPageTitleFromPath } from '~/utils/title-utils'; export const AppRouterProviders = () => { const { pathname } = useLocation(); const pageTitle = getPageTitleFromPath(pathname); + const objectNamePlural = useParams().objectNamePlural; return ( @@ -61,6 +64,11 @@ export const AppRouterProviders = () => { + {isNonEmptyString(objectNamePlural) && ( + + )} diff --git a/packages/twenty-front/src/modules/context-store/components/ContextStoreViewIdEffect.tsx b/packages/twenty-front/src/modules/context-store/components/ContextStoreViewIdEffect.tsx new file mode 100644 index 000000000..d4f7efc66 --- /dev/null +++ b/packages/twenty-front/src/modules/context-store/components/ContextStoreViewIdEffect.tsx @@ -0,0 +1,104 @@ +import { contextStoreCurrentViewIdComponentState } from '@/context-store/states/contextStoreCurrentViewIdComponentState'; +import { useLastVisitedObjectMetadataItem } from '@/navigation/hooks/useLastVisitedObjectMetadataItem'; +import { useLastVisitedView } from '@/navigation/hooks/useLastVisitedView'; +import { useFilteredObjectMetadataItems } from '@/object-metadata/hooks/useFilteredObjectMetadataItems'; +import { usePrefetchedData } from '@/prefetch/hooks/usePrefetchedData'; +import { PrefetchKey } from '@/prefetch/types/PrefetchKey'; +import { useSetRecoilComponentStateV2 } from '@/ui/utilities/state/component-state/hooks/useSetRecoilComponentStateV2'; +import { useViewFromQueryParams } from '@/views/hooks/internal/useViewFromQueryParams'; +import { View } from '@/views/types/View'; +import { isUndefined } from '@sniptt/guards'; +import { useEffect } from 'react'; +import { isDeeplyEqual } from '~/utils/isDeeplyEqual'; +import { isDefined } from '~/utils/isDefined'; + +export const ContextStoreViewIdEffect = ({ + objectNamePlural, +}: { + objectNamePlural: string; +}) => { + const { viewIdQueryParam } = useViewFromQueryParams(); + + const { records: viewsOnCurrentObject } = usePrefetchedData( + PrefetchKey.AllViews, + ); + + const { findObjectMetadataItemByNamePlural } = + useFilteredObjectMetadataItems(); + const objectMetadataItemId = + findObjectMetadataItemByNamePlural(objectNamePlural); + const { getLastVisitedViewIdFromObjectNamePlural, setLastVisitedView } = + useLastVisitedView(); + const { lastVisitedObjectMetadataItemId, setLastVisitedObjectMetadataItem } = + useLastVisitedObjectMetadataItem(); + + const lastVisitedViewId = + getLastVisitedViewIdFromObjectNamePlural(objectNamePlural); + const isLastVisitedObjectMetadataItemDifferent = !isDeeplyEqual( + objectMetadataItemId?.id, + lastVisitedObjectMetadataItemId, + ); + const setContextStoreCurrentViewId = useSetRecoilComponentStateV2( + contextStoreCurrentViewIdComponentState, + objectNamePlural, + ); + + useEffect(() => { + const indexView = viewsOnCurrentObject.find((view) => view.key === 'INDEX'); + + if (isUndefined(viewIdQueryParam) && isDefined(lastVisitedViewId)) { + if (isLastVisitedObjectMetadataItemDifferent) { + setLastVisitedObjectMetadataItem(objectNamePlural); + setLastVisitedView({ + objectNamePlural, + viewId: lastVisitedViewId, + }); + } + setContextStoreCurrentViewId(lastVisitedViewId); + return; + } + + if (isDefined(viewIdQueryParam)) { + if (isLastVisitedObjectMetadataItemDifferent) { + setLastVisitedObjectMetadataItem(objectNamePlural); + } + if (!isDeeplyEqual(viewIdQueryParam, lastVisitedViewId)) { + setLastVisitedView({ + objectNamePlural, + viewId: viewIdQueryParam, + }); + } + setContextStoreCurrentViewId(viewIdQueryParam); + return; + } + + if (isDefined(indexView)) { + if (isLastVisitedObjectMetadataItemDifferent) { + setLastVisitedObjectMetadataItem(objectNamePlural); + } + if (!isDeeplyEqual(indexView.id, lastVisitedViewId)) { + setLastVisitedView({ + objectNamePlural, + viewId: indexView.id, + }); + } + setContextStoreCurrentViewId(indexView.id); + return; + } + + return () => { + setContextStoreCurrentViewId(null); + }; + }, [ + isLastVisitedObjectMetadataItemDifferent, + lastVisitedViewId, + objectNamePlural, + setContextStoreCurrentViewId, + setLastVisitedObjectMetadataItem, + setLastVisitedView, + viewIdQueryParam, + viewsOnCurrentObject, + ]); + + return <>; +}; diff --git a/packages/twenty-front/src/pages/object-record/RecordIndexPage.tsx b/packages/twenty-front/src/pages/object-record/RecordIndexPage.tsx index 269e515f1..67e818ec4 100644 --- a/packages/twenty-front/src/pages/object-record/RecordIndexPage.tsx +++ b/packages/twenty-front/src/pages/object-record/RecordIndexPage.tsx @@ -4,6 +4,7 @@ import { useParams } from 'react-router-dom'; import { ActionMenuComponentInstanceContext } from '@/action-menu/states/contexts/ActionMenuComponentInstanceContext'; import { getActionMenuIdFromRecordIndexId } from '@/action-menu/utils/getActionMenuIdFromRecordIndexId'; import { MainContextStoreComponentInstanceIdSetterEffect } from '@/context-store/components/MainContextStoreComponentInstanceIdSetterEffect'; +import { contextStoreCurrentViewIdComponentState } from '@/context-store/states/contextStoreCurrentViewIdComponentState'; import { ContextStoreComponentInstanceContext } from '@/context-store/states/contexts/ContextStoreComponentInstanceContext'; import { useObjectMetadataItem } from '@/object-metadata/hooks/useObjectMetadataItem'; import { useObjectNameSingularFromPlural } from '@/object-metadata/hooks/useObjectNameSingularFromPlural'; @@ -17,9 +18,11 @@ import { useHandleIndexIdentifierClick } from '@/object-record/record-index/hook import { PageBody } from '@/ui/layout/page/components/PageBody'; import { PageContainer } from '@/ui/layout/page/components/PageContainer'; import { PageTitle } from '@/ui/utilities/page-title/components/PageTitle'; +import { useRecoilComponentValueV2 } from '@/ui/utilities/state/component-state/hooks/useRecoilComponentValueV2'; import { ViewComponentInstanceContext } from '@/views/states/contexts/ViewComponentInstanceContext'; import { useRecoilCallback } from 'recoil'; import { capitalize } from 'twenty-shared'; +import { isDefined } from 'twenty-ui'; const StyledIndexContainer = styled.div` display: flex; @@ -30,7 +33,12 @@ const StyledIndexContainer = styled.div` export const RecordIndexPage = () => { const objectNamePlural = useParams().objectNamePlural ?? ''; - const recordIndexId = objectNamePlural ?? ''; + const contextStoreCurrentViewId = useRecoilComponentValueV2( + contextStoreCurrentViewIdComponentState, + objectNamePlural, + ); + + const recordIndexId = `${objectNamePlural}-${contextStoreCurrentViewId}`; const { objectNameSingular } = useObjectNameSingularFromPlural({ objectNamePlural, @@ -54,6 +62,10 @@ export const RecordIndexPage = () => { [], ); + if (!isDefined(contextStoreCurrentViewId)) { + return; + } + return (