From f36555bdc007009627ac66dc4b6e99d4705d870e Mon Sep 17 00:00:00 2001 From: Marie <51697796+ijreilly@users.noreply.github.com> Date: Fri, 6 Dec 2024 18:46:06 +0100 Subject: [PATCH] Fix infinite loading on field settings (#8938) We were experiencing infinite loading on field settings pages (creation of new field), due to the fact that the component was being rendered on and on and on. This was due to useGetCurrentUserQuery calls outside of the update function, causing renders in cascade. We also had an issue with the component being unmounted too often. --------- Co-authored-by: Charles Bochet --- .../ObjectMetadataItemsLoadEffect.tsx | 18 ++++++++--- .../hooks/useUpdateOneFieldMetadataItem.ts | 18 +++-------- .../graphql/fragments/userQueryFragment.ts | 3 ++ .../users/graphql/queries/getCurrentUser.ts | 3 ++ .../fragments/workspaceMemberQueryFragment.ts | 2 +- .../data-model/SettingsObjectFieldEdit.tsx | 32 ++----------------- 6 files changed, 28 insertions(+), 48 deletions(-) diff --git a/packages/twenty-front/src/modules/object-metadata/components/ObjectMetadataItemsLoadEffect.tsx b/packages/twenty-front/src/modules/object-metadata/components/ObjectMetadataItemsLoadEffect.tsx index aeb16de86a..4142a52f57 100644 --- a/packages/twenty-front/src/modules/object-metadata/components/ObjectMetadataItemsLoadEffect.tsx +++ b/packages/twenty-front/src/modules/object-metadata/components/ObjectMetadataItemsLoadEffect.tsx @@ -16,10 +16,12 @@ export const ObjectMetadataItemsLoadEffect = () => { const currentWorkspace = useRecoilValue(currentWorkspaceState); const isLoggedIn = useIsLogged(); - const { objectMetadataItems: newObjectMetadataItems } = - useFindManyObjectMetadataItems({ - skip: !isLoggedIn, - }); + const { + objectMetadataItems: newObjectMetadataItems, + loading: isObjectMetadataLoading, + } = useFindManyObjectMetadataItems({ + skip: !isLoggedIn, + }); const updateObjectMetadataItems = useRecoilCallback( ({ set, snapshot }) => @@ -32,6 +34,7 @@ export const ObjectMetadataItemsLoadEffect = () => { : newObjectMetadataItems; if ( + !isObjectMetadataLoading && !isDeeplyEqual( snapshot.getLoadable(objectMetadataItemsState).getValue(), toSetObjectMetadataItems, @@ -40,7 +43,12 @@ export const ObjectMetadataItemsLoadEffect = () => { set(objectMetadataItemsState, toSetObjectMetadataItems); } }, - [currentUser, currentWorkspace?.activationStatus, newObjectMetadataItems], + [ + currentUser, + currentWorkspace?.activationStatus, + isObjectMetadataLoading, + newObjectMetadataItems, + ], ); useEffect(() => { diff --git a/packages/twenty-front/src/modules/object-metadata/hooks/useUpdateOneFieldMetadataItem.ts b/packages/twenty-front/src/modules/object-metadata/hooks/useUpdateOneFieldMetadataItem.ts index 6a137010a3..cc66a69e0a 100644 --- a/packages/twenty-front/src/modules/object-metadata/hooks/useUpdateOneFieldMetadataItem.ts +++ b/packages/twenty-front/src/modules/object-metadata/hooks/useUpdateOneFieldMetadataItem.ts @@ -12,9 +12,8 @@ import { FIND_MANY_OBJECT_METADATA_ITEMS } from '../graphql/queries'; import { currentWorkspaceState } from '@/auth/states/currentWorkspaceState'; import { CoreObjectNameSingular } from '@/object-metadata/types/CoreObjectNameSingular'; import { useFindManyRecordsQuery } from '@/object-record/hooks/useFindManyRecordsQuery'; +import { GET_CURRENT_USER } from '@/users/graphql/queries/getCurrentUser'; import { useSetRecoilState } from 'recoil'; -import { isDefined } from 'twenty-ui'; -import { useGetCurrentUserQuery } from '~/generated/graphql'; import { useApolloMetadataClient } from './useApolloMetadataClient'; export const useUpdateOneFieldMetadataItem = () => { @@ -23,15 +22,7 @@ export const useUpdateOneFieldMetadataItem = () => { const setCurrentWorkspace = useSetRecoilState(currentWorkspaceState); - const { refetch: refetchCurrentUser } = useGetCurrentUserQuery({ - onCompleted: (data) => { - if (isDefined(data?.currentUser?.defaultWorkspace)) { - setCurrentWorkspace(data.currentUser.defaultWorkspace); - } - }, - }); - - const { findManyRecordsQuery } = useFindManyRecordsQuery({ + const { findManyRecordsQuery: findManyViewsQuery } = useFindManyRecordsQuery({ objectNameSingular: CoreObjectNameSingular.View, recordGqlFields: { id: true, @@ -80,10 +71,11 @@ export const useUpdateOneFieldMetadataItem = () => { refetchQueries: [getOperationName(FIND_MANY_OBJECT_METADATA_ITEMS) ?? ''], }); - await refetchCurrentUser(); + const { data } = await apolloClient.query({ query: GET_CURRENT_USER }); + setCurrentWorkspace(data?.currentUser?.defaultWorkspace); await apolloClient.query({ - query: findManyRecordsQuery, + query: findManyViewsQuery, variables: { filter: { objectMetadataId: { diff --git a/packages/twenty-front/src/modules/users/graphql/fragments/userQueryFragment.ts b/packages/twenty-front/src/modules/users/graphql/fragments/userQueryFragment.ts index 1d5e0201db..53dbb892c4 100644 --- a/packages/twenty-front/src/modules/users/graphql/fragments/userQueryFragment.ts +++ b/packages/twenty-front/src/modules/users/graphql/fragments/userQueryFragment.ts @@ -1,3 +1,4 @@ +import { WORKSPACE_MEMBER_QUERY_FRAGMENT } from '@/workspace-member/graphql/fragments/workspaceMemberQueryFragment'; import { gql } from '@apollo/client'; export const USER_QUERY_FRAGMENT = gql` @@ -62,4 +63,6 @@ export const USER_QUERY_FRAGMENT = gql` } userVars } + + ${WORKSPACE_MEMBER_QUERY_FRAGMENT} `; diff --git a/packages/twenty-front/src/modules/users/graphql/queries/getCurrentUser.ts b/packages/twenty-front/src/modules/users/graphql/queries/getCurrentUser.ts index 78d376d6a4..06c4784587 100644 --- a/packages/twenty-front/src/modules/users/graphql/queries/getCurrentUser.ts +++ b/packages/twenty-front/src/modules/users/graphql/queries/getCurrentUser.ts @@ -1,4 +1,5 @@ // This query cannot be put in the graphQL folder because it cannot be generated by the graphQL codegen. +import { USER_QUERY_FRAGMENT } from '@/users/graphql/fragments/userQueryFragment'; import { gql } from '@apollo/client'; export const GET_CURRENT_USER = gql` @@ -7,4 +8,6 @@ export const GET_CURRENT_USER = gql` ...UserQueryFragment } } + + ${USER_QUERY_FRAGMENT} `; diff --git a/packages/twenty-front/src/modules/workspace-member/graphql/fragments/workspaceMemberQueryFragment.ts b/packages/twenty-front/src/modules/workspace-member/graphql/fragments/workspaceMemberQueryFragment.ts index 16aa562ed4..394253eb4c 100644 --- a/packages/twenty-front/src/modules/workspace-member/graphql/fragments/workspaceMemberQueryFragment.ts +++ b/packages/twenty-front/src/modules/workspace-member/graphql/fragments/workspaceMemberQueryFragment.ts @@ -1,6 +1,6 @@ import { gql } from '@apollo/client'; -export const USER_QUERY_FRAGMENT = gql` +export const WORKSPACE_MEMBER_QUERY_FRAGMENT = gql` fragment WorkspaceMemberQueryFragment on WorkspaceMember { id name { diff --git a/packages/twenty-front/src/pages/settings/data-model/SettingsObjectFieldEdit.tsx b/packages/twenty-front/src/pages/settings/data-model/SettingsObjectFieldEdit.tsx index 485022381d..9f7729977e 100644 --- a/packages/twenty-front/src/pages/settings/data-model/SettingsObjectFieldEdit.tsx +++ b/packages/twenty-front/src/pages/settings/data-model/SettingsObjectFieldEdit.tsx @@ -1,8 +1,7 @@ -import { useApolloClient } from '@apollo/client'; import { zodResolver } from '@hookform/resolvers/zod'; import omit from 'lodash.omit'; import pick from 'lodash.pick'; -import { useEffect, useState } from 'react'; +import { useEffect } from 'react'; import { FormProvider, useForm } from 'react-hook-form'; import { useNavigate, useParams } from 'react-router-dom'; import { @@ -21,7 +20,6 @@ import { useUpdateOneFieldMetadataItem } from '@/object-metadata/hooks/useUpdate import { formatFieldMetadataItemInput } from '@/object-metadata/utils/formatFieldMetadataItemInput'; import { getFieldSlug } from '@/object-metadata/utils/getFieldSlug'; import { isLabelIdentifierField } from '@/object-metadata/utils/isLabelIdentifierField'; -import { useFindManyRecordsQuery } from '@/object-record/hooks/useFindManyRecordsQuery'; import { RecordFieldValueSelectorContextProvider } from '@/object-record/record-store/contexts/RecordFieldValueSelectorContext'; import { SaveAndCancelButtons } from '@/settings/components/SaveAndCancelButtons/SaveAndCancelButtons'; import { SettingsPageContainer } from '@/settings/components/SettingsPageContainer'; @@ -49,7 +47,6 @@ type SettingsDataModelFieldEditFormValues = z.infer< export const SettingsObjectFieldEdit = () => { const navigate = useNavigate(); const { enqueueSnackBar } = useSnackBar(); - const [isPersisting, setIsPersisting] = useState(false); const { objectSlug = '', fieldSlug = '' } = useParams(); const { findObjectMetadataItemBySlug } = useFilteredObjectMetadataItems(); @@ -66,20 +63,6 @@ export const SettingsObjectFieldEdit = () => { const getRelationMetadata = useGetRelationMetadata(); const { updateOneFieldMetadataItem } = useUpdateOneFieldMetadataItem(); - const apolloClient = useApolloClient(); - - const { findManyRecordsQuery } = useFindManyRecordsQuery({ - objectNameSingular: objectMetadataItem?.nameSingular || '', - }); - - const refetchRecords = async () => { - if (!objectMetadataItem) return; - await apolloClient.query({ - query: findManyRecordsQuery, - fetchPolicy: 'network-only', - }); - }; - const formConfig = useForm({ mode: 'onTouched', resolver: zodResolver(settingsFieldFormSchema()), @@ -93,11 +76,10 @@ export const SettingsObjectFieldEdit = () => { }); useEffect(() => { - if (isPersisting) return; if (!objectMetadataItem || !fieldMetadataItem) { navigate(AppPath.NotFound); } - }, [navigate, objectMetadataItem, fieldMetadataItem, isPersisting]); + }, [navigate, objectMetadataItem, fieldMetadataItem]); const { isDirty, isValid, isSubmitting } = formConfig.formState; const canSave = isDirty && isValid && !isSubmitting; @@ -128,8 +110,6 @@ export const SettingsObjectFieldEdit = () => { }) ?? {}; if (isDefined(relationFieldMetadataItem)) { - setIsPersisting(true); - await updateOneFieldMetadataItem({ objectMetadataId: objectMetadataItem.id, fieldMetadataIdToUpdate: relationFieldMetadataItem.id, @@ -146,7 +126,7 @@ export const SettingsObjectFieldEdit = () => { Object.keys(otherDirtyFields), ); - setIsPersisting(true); + navigate(`/settings/objects/${objectSlug}`); await updateOneFieldMetadataItem({ objectMetadataId: objectMetadataItem.id, @@ -154,16 +134,10 @@ export const SettingsObjectFieldEdit = () => { updatePayload: formattedInput, }); } - - navigate(`/settings/objects/${objectSlug}`); - - refetchRecords(); } catch (error) { enqueueSnackBar((error as Error).message, { variant: SnackBarVariant.Error, }); - } finally { - setIsPersisting(false); } };