From 5e32cb215e9174e63e9e50c41b6fcdaa26bfc4b1 Mon Sep 17 00:00:00 2001 From: Lucas Bordeau Date: Tue, 4 Jun 2024 15:10:56 +0200 Subject: [PATCH] Fix 5598 - View field creation (#5732) - Fix duplicate view field creation - Fix redirect to proper settings data model page - Refetch view fields after field creation (temporary solution) Fixes https://github.com/twentyhq/twenty/issues/5598 --- .../RecordTableHeaderPlusButtonContent.tsx | 8 +- .../record-table/hooks/useTableColumns.ts | 23 ++-- .../internal/usePersistViewFieldRecords.ts | 1 + .../views/hooks/useSaveCurrentViewFields.ts | 35 +++-- .../SettingsObjectNewFieldStep2.tsx | 128 +++--------------- 5 files changed, 63 insertions(+), 132 deletions(-) diff --git a/packages/twenty-front/src/modules/object-record/record-table/components/RecordTableHeaderPlusButtonContent.tsx b/packages/twenty-front/src/modules/object-record/record-table/components/RecordTableHeaderPlusButtonContent.tsx index 749861c13..2a19bc15f 100644 --- a/packages/twenty-front/src/modules/object-record/record-table/components/RecordTableHeaderPlusButtonContent.tsx +++ b/packages/twenty-front/src/modules/object-record/record-table/components/RecordTableHeaderPlusButtonContent.tsx @@ -1,10 +1,11 @@ -import { useCallback } from 'react'; +import { useCallback, useContext } from 'react'; import { Link } from 'react-router-dom'; import styled from '@emotion/styled'; import { useRecoilValue } from 'recoil'; import { IconSettings, useIcons } from 'twenty-ui'; import { FieldMetadata } from '@/object-record/record-field/types/FieldMetadata'; +import { RecordTableContext } from '@/object-record/record-table/contexts/RecordTableContext'; import { useRecordTableStates } from '@/object-record/record-table/hooks/internal/useRecordTableStates'; import { useTableColumns } from '@/object-record/record-table/hooks/useTableColumns'; import { ColumnDefinition } from '@/object-record/record-table/types/ColumnDefinition'; @@ -14,6 +15,7 @@ import { useDropdown } from '@/ui/layout/dropdown/hooks/useDropdown'; import { MenuItem } from '@/ui/navigation/menu-item/components/MenuItem'; export const RecordTableHeaderPlusButtonContent = () => { + const { objectMetadataItem } = useContext(RecordTableContext); const { closeDropdown } = useDropdown(); const { hiddenTableColumnsSelector } = useRecordTableStates(); @@ -54,7 +56,9 @@ export const RecordTableHeaderPlusButtonContent = () => { )} - + diff --git a/packages/twenty-front/src/modules/object-record/record-table/hooks/useTableColumns.ts b/packages/twenty-front/src/modules/object-record/record-table/hooks/useTableColumns.ts index 534db9462..894eca219 100644 --- a/packages/twenty-front/src/modules/object-record/record-table/hooks/useTableColumns.ts +++ b/packages/twenty-front/src/modules/object-record/record-table/hooks/useTableColumns.ts @@ -43,21 +43,23 @@ export const useTableColumns = (props?: useRecordTableProps) => { async ( viewField: Omit, 'size' | 'position'>, ) => { - const isNewColumn = !tableColumns.some( - (tableColumns) => - tableColumns.fieldMetadataId === viewField.fieldMetadataId, + const shouldShowColumn = !visibleTableColumns.some( + (visibleColumn) => + visibleColumn.fieldMetadataId === viewField.fieldMetadataId, ); - const lastTableColumnPosition = [...tableColumns] + + const tableColumnPositions = [...tableColumns] .sort((a, b) => b.position - a.position) .map((column) => column.position); - const lastPosition = lastTableColumnPosition[0] ?? 0; + const lastPosition = tableColumnPositions[0] ?? 0; - if (isNewColumn) { + if (shouldShowColumn) { const newColumn = availableTableColumns.find( (availableTableColumn) => availableTableColumn.fieldMetadataId === viewField.fieldMetadataId, ); + if (!newColumn) return; const nextColumns = [ @@ -67,7 +69,7 @@ export const useTableColumns = (props?: useRecordTableProps) => { await handleColumnsChange(nextColumns); } else { - const nextColumns = tableColumns.map((previousColumn) => + const nextColumns = visibleTableColumns.map((previousColumn) => previousColumn.fieldMetadataId === viewField.fieldMetadataId ? { ...previousColumn, isVisible: !viewField.isVisible } : previousColumn, @@ -76,7 +78,12 @@ export const useTableColumns = (props?: useRecordTableProps) => { await handleColumnsChange(nextColumns); } }, - [tableColumns, availableTableColumns, handleColumnsChange], + [ + tableColumns, + availableTableColumns, + handleColumnsChange, + visibleTableColumns, + ], ); const handleMoveTableColumn = useCallback( diff --git a/packages/twenty-front/src/modules/views/hooks/internal/usePersistViewFieldRecords.ts b/packages/twenty-front/src/modules/views/hooks/internal/usePersistViewFieldRecords.ts index c341b0a06..15327a303 100644 --- a/packages/twenty-front/src/modules/views/hooks/internal/usePersistViewFieldRecords.ts +++ b/packages/twenty-front/src/modules/views/hooks/internal/usePersistViewFieldRecords.ts @@ -78,6 +78,7 @@ export const usePersistViewFieldRecords = () => { const updateViewFieldRecords = useCallback( (viewFieldsToUpdate: ViewField[]) => { if (!viewFieldsToUpdate.length) return; + return Promise.all( viewFieldsToUpdate.map((viewField) => apolloClient.mutate({ diff --git a/packages/twenty-front/src/modules/views/hooks/useSaveCurrentViewFields.ts b/packages/twenty-front/src/modules/views/hooks/useSaveCurrentViewFields.ts index fed1518f1..65386abac 100644 --- a/packages/twenty-front/src/modules/views/hooks/useSaveCurrentViewFields.ts +++ b/packages/twenty-front/src/modules/views/hooks/useSaveCurrentViewFields.ts @@ -19,7 +19,7 @@ export const useSaveCurrentViewFields = (viewBarComponentId?: string) => { const saveViewFields = useRecoilCallback( ({ set, snapshot }) => - async (fields: ViewField[]) => { + async (viewFieldsToSave: ViewField[]) => { const currentViewId = snapshot .getLoadable(currentViewIdState) .getValue(); @@ -29,21 +29,27 @@ export const useSaveCurrentViewFields = (viewBarComponentId?: string) => { } set(isPersistingViewFieldsState, true); + const view = await getViewFromCache(currentViewId); if (isUndefinedOrNull(view)) { return; } - const viewFieldsToUpdate = fields - .map((field) => { - const existingField = view.viewFields.find( - (viewField) => viewField.id === field.id, + const currentViewFields = view.viewFields; + + const viewFieldsToUpdate = viewFieldsToSave + .map((viewFieldToSave) => { + const existingField = currentViewFields.find( + (currentViewField) => + currentViewField.fieldMetadataId === + viewFieldToSave.fieldMetadataId, ); if (isUndefinedOrNull(existingField)) { return undefined; } + if ( isDeeplyEqual( { @@ -52,24 +58,33 @@ export const useSaveCurrentViewFields = (viewBarComponentId?: string) => { isVisible: existingField.isVisible, }, { - position: field.position, - size: field.size, - isVisible: field.isVisible, + position: viewFieldToSave.position, + size: viewFieldToSave.size, + isVisible: viewFieldToSave.isVisible, }, ) ) { return undefined; } - return field; + + return { ...viewFieldToSave, id: existingField.id }; }) .filter(isDefined); - const viewFieldsToCreate = fields.filter((field) => !field.id); + const viewFieldsToCreate = viewFieldsToSave.filter( + (viewFieldToSave) => + !currentViewFields.some( + (currentViewField) => + currentViewField.fieldMetadataId === + viewFieldToSave.fieldMetadataId, + ), + ); await Promise.all([ createViewFieldRecords(viewFieldsToCreate, view), updateViewFieldRecords(viewFieldsToUpdate), ]); + set(isPersistingViewFieldsState, false); }, [ diff --git a/packages/twenty-front/src/pages/settings/data-model/SettingsObjectNewField/SettingsObjectNewFieldStep2.tsx b/packages/twenty-front/src/pages/settings/data-model/SettingsObjectNewField/SettingsObjectNewFieldStep2.tsx index 37cd74c65..307ccfbbe 100644 --- a/packages/twenty-front/src/pages/settings/data-model/SettingsObjectNewField/SettingsObjectNewFieldStep2.tsx +++ b/packages/twenty-front/src/pages/settings/data-model/SettingsObjectNewField/SettingsObjectNewFieldStep2.tsx @@ -1,7 +1,7 @@ import { useEffect, useState } from 'react'; import { FormProvider, useForm } from 'react-hook-form'; import { useNavigate, useParams } from 'react-router-dom'; -import { Reference, useApolloClient } from '@apollo/client'; +import { useApolloClient } from '@apollo/client'; import styled from '@emotion/styled'; import { zodResolver } from '@hookform/resolvers/zod'; import pick from 'lodash.pick'; @@ -11,10 +11,7 @@ import { z } from 'zod'; import { useCreateOneRelationMetadataItem } from '@/object-metadata/hooks/useCreateOneRelationMetadataItem'; import { useFieldMetadataItem } from '@/object-metadata/hooks/useFieldMetadataItem'; import { useFilteredObjectMetadataItems } from '@/object-metadata/hooks/useFilteredObjectMetadataItems'; -import { useObjectMetadataItem } from '@/object-metadata/hooks/useObjectMetadataItem'; import { CoreObjectNameSingular } from '@/object-metadata/types/CoreObjectNameSingular'; -import { RecordGqlRefEdge } from '@/object-record/cache/types/RecordGqlRefEdge'; -import { modifyRecordFromCache } from '@/object-record/cache/utils/modifyRecordFromCache'; import { useFindManyRecords } from '@/object-record/hooks/useFindManyRecords'; import { SaveAndCancelButtons } from '@/settings/components/SaveAndCancelButtons/SaveAndCancelButtons'; import { SettingsHeaderContainer } from '@/settings/components/SettingsHeaderContainer'; @@ -51,13 +48,12 @@ export const SettingsObjectNewFieldStep2 = () => { const { objectSlug = '' } = useParams(); const { enqueueSnackBar } = useSnackBar(); - const { findActiveObjectMetadataItemBySlug, findObjectMetadataItemById } = + const { findActiveObjectMetadataItemBySlug } = useFilteredObjectMetadataItems(); const activeObjectMetadataItem = findActiveObjectMetadataItemBySlug(objectSlug); const { createMetadataField } = useFieldMetadataItem(); - const cache = useApolloClient().cache; const formConfig = useForm({ mode: 'onTouched', @@ -70,12 +66,8 @@ export const SettingsObjectNewFieldStep2 = () => { } }, [activeObjectMetadataItem, navigate]); - const [objectViews, setObjectViews] = useState([]); - const [relationObjectViews, setRelationObjectViews] = useState([]); - - const { objectMetadataItem: viewObjectMetadataItem } = useObjectMetadataItem({ - objectNameSingular: CoreObjectNameSingular.View, - }); + const [, setObjectViews] = useState([]); + const [, setRelationObjectViews] = useState([]); useFindManyRecords({ objectNameSingular: CoreObjectNameSingular.View, @@ -111,6 +103,8 @@ export const SettingsObjectNewFieldStep2 = () => { const { createOneRelationMetadataItem: createOneRelationMetadata } = useCreateOneRelationMetadataItem(); + const apolloClient = useApolloClient(); + if (!activeObjectMetadataItem) return null; const canSave = @@ -126,7 +120,7 @@ export const SettingsObjectNewFieldStep2 = () => { ) { const { relation: relationFormValues, ...fieldFormValues } = formValues; - const createdRelation = await createOneRelationMetadata({ + await createOneRelationMetadata({ relationType: relationFormValues.type, field: pick(fieldFormValues, ['icon', 'label', 'description']), objectMetadataId: activeObjectMetadataItem.id, @@ -139,111 +133,21 @@ export const SettingsObjectNewFieldStep2 = () => { }, }); - const relationObjectMetadataItem = findObjectMetadataItemById( - relationFormValues.objectMetadataId, - ); - - objectViews.map(async (view) => { - const viewFieldToCreate = { - viewId: view.id, - fieldMetadataId: - relationFormValues.type === 'MANY_TO_ONE' - ? createdRelation.data?.createOneRelation.toFieldMetadataId - : createdRelation.data?.createOneRelation.fromFieldMetadataId, - position: activeObjectMetadataItem.fields.length, - isVisible: true, - size: 100, - }; - - modifyRecordFromCache({ - objectMetadataItem: viewObjectMetadataItem, - cache: cache, - fieldModifiers: { - viewFields: (viewFieldsRef, { readField }) => { - const edges = readField<{ node: Reference }[]>( - 'edges', - viewFieldsRef, - ); - - if (!edges) return viewFieldsRef; - - return { - ...viewFieldsRef, - edges: [...edges, { node: viewFieldToCreate }], - }; - }, - }, - recordId: view.id, - }); - - relationObjectViews.map(async (view) => { - const viewFieldToCreate = { - viewId: view.id, - fieldMetadataId: - relationFormValues.type === 'MANY_TO_ONE' - ? createdRelation.data?.createOneRelation.fromFieldMetadataId - : createdRelation.data?.createOneRelation.toFieldMetadataId, - position: relationObjectMetadataItem?.fields.length, - isVisible: true, - size: 100, - }; - modifyRecordFromCache({ - objectMetadataItem: viewObjectMetadataItem, - cache: cache, - fieldModifiers: { - viewFields: (viewFieldsRef, { readField }) => { - const edges = readField<{ node: Reference }[]>( - 'edges', - viewFieldsRef, - ); - - if (!edges) return viewFieldsRef; - - return { - ...viewFieldsRef, - edges: [...edges, { node: viewFieldToCreate }], - }; - }, - }, - recordId: view.id, - }); - }); + // TODO: fix optimistic update logic + // Forcing a refetch for now but it's not ideal + await apolloClient.refetchQueries({ + include: ['FindManyViews', 'CombinedFindManyRecords'], }); } else { - const createdMetadataField = await createMetadataField({ + await createMetadataField({ ...formValues, objectMetadataId: activeObjectMetadataItem.id, }); - objectViews.map(async (view) => { - const viewFieldToCreate = { - viewId: view.id, - fieldMetadataId: createdMetadataField.data?.createOneField.id, - position: activeObjectMetadataItem.fields.length, - isVisible: true, - size: 100, - }; - - modifyRecordFromCache({ - objectMetadataItem: viewObjectMetadataItem, - cache: cache, - fieldModifiers: { - viewFields: (cachedViewFieldsConnection, { readField }) => { - const edges = readField( - 'edges', - cachedViewFieldsConnection, - ); - - if (!edges) return cachedViewFieldsConnection; - - return { - ...cachedViewFieldsConnection, - edges: [...edges, { node: viewFieldToCreate }], - }; - }, - }, - recordId: view.id, - }); + // TODO: fix optimistic update logic + // Forcing a refetch for now but it's not ideal + await apolloClient.refetchQueries({ + include: ['FindManyViews', 'CombinedFindManyRecords'], }); }