From ac7d7401350063abd02df240e635b3d442208a80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9my=20M?= Date: Wed, 6 Nov 2024 11:41:44 +0100 Subject: [PATCH] fix: when field metadata SELECT type is edited update view groups (#8344) Co-authored-by: Charles Bochet --- .../hooks/__mocks__/useFieldMetadataItem.ts | 32 ++++ .../__tests__/useFieldMetadataItem.test.tsx | 51 ++++-- .../hooks/useFieldMetadataItem.ts | 16 +- .../hooks/useUpdateOneFieldMetadataItem.ts | 37 ++++- .../internal/useSetRecordBoardColumns.ts | 8 +- .../SettingsObjectFieldItemTableRow.tsx | 9 +- .../data-model/SettingsObjectFieldEdit.tsx | 6 +- .../field-metadata/field-metadata.module.ts | 2 + .../field-metadata/field-metadata.service.ts | 13 ++ .../field-metadata-related-records.service.ts | 151 ++++++++++++++++++ .../is-select-field-metadata-type.util.ts | 7 + 11 files changed, 304 insertions(+), 28 deletions(-) create mode 100644 packages/twenty-server/src/engine/metadata-modules/field-metadata/services/field-metadata-related-records.service.ts create mode 100644 packages/twenty-server/src/engine/metadata-modules/field-metadata/utils/is-select-field-metadata-type.util.ts diff --git a/packages/twenty-front/src/modules/object-metadata/hooks/__mocks__/useFieldMetadataItem.ts b/packages/twenty-front/src/modules/object-metadata/hooks/__mocks__/useFieldMetadataItem.ts index b33c4dc1d..b69870f22 100644 --- a/packages/twenty-front/src/modules/object-metadata/hooks/__mocks__/useFieldMetadataItem.ts +++ b/packages/twenty-front/src/modules/object-metadata/hooks/__mocks__/useFieldMetadataItem.ts @@ -21,6 +21,7 @@ const baseFields = ` settings `; + export const queries = { deleteMetadataField: gql` mutation DeleteOneFieldMetadataItem($idToDelete: UUID!) { @@ -29,6 +30,37 @@ export const queries = { } } `, + findManyViewsQuery: gql` + query FindManyViews($filter: ViewFilterInput, $orderBy: [ViewOrderByInput], $lastCursor: String, $limit: Int) { + views(filter: $filter, orderBy: $orderBy, first: $limit, after: $lastCursor) { + edges { + node { + __typename + id + viewGroups { + edges { + node { + __typename + fieldMetadataId + fieldValue + id + isVisible + position + } + } + } + } + cursor + } + pageInfo { + hasNextPage + hasPreviousPage + startCursor + endCursor + } + totalCount + } + }`, deleteMetadataFieldRelation: gql` mutation DeleteOneRelationMetadataItem($idToDelete: UUID!) { deleteOneRelation(input: { id: $idToDelete }) { diff --git a/packages/twenty-front/src/modules/object-metadata/hooks/__tests__/useFieldMetadataItem.test.tsx b/packages/twenty-front/src/modules/object-metadata/hooks/__tests__/useFieldMetadataItem.test.tsx index 6795897e3..a864e3060 100644 --- a/packages/twenty-front/src/modules/object-metadata/hooks/__tests__/useFieldMetadataItem.test.tsx +++ b/packages/twenty-front/src/modules/object-metadata/hooks/__tests__/useFieldMetadataItem.test.tsx @@ -1,12 +1,11 @@ -import { MockedProvider } from '@apollo/client/testing'; import { renderHook } from '@testing-library/react'; -import { act, ReactNode } from 'react'; -import { RecoilRoot } from 'recoil'; +import { act } from 'react'; import { useFieldMetadataItem } from '@/object-metadata/hooks/useFieldMetadataItem'; import { FieldMetadataItem } from '@/object-metadata/types/FieldMetadataItem'; import { FieldMetadataType, RelationDefinitionType } from '~/generated/graphql'; +import { getJestMetadataAndApolloMocksWrapper } from '~/testing/jest/getJestMetadataAndApolloMocksWrapper'; import { FIELD_METADATA_ID, FIELD_RELATION_METADATA_ID, @@ -58,6 +57,31 @@ const fieldRelationMetadataItem: FieldMetadataItem = { }; const mocks = [ + { + request: { + query: queries.findManyViewsQuery, + variables: { + filter: { + objectMetadataId: { eq: '25611fce-6637-4089-b0ca-91afeec95784' }, + }, + }, + }, + result: jest.fn(() => ({ + data: { + views: { + __typename: 'ViewConnection', + totalCount: 0, + pageInfo: { + __typename: 'PageInfo', + hasNextPage: false, + startCursor: '', + endCursor: '', + }, + edges: [], + }, + }, + })), + }, { request: { query: queries.deleteMetadataField, @@ -115,13 +139,9 @@ const mocks = [ }, ]; -const Wrapper = ({ children }: { children: ReactNode }) => ( - - - {children} - - -); +const Wrapper = getJestMetadataAndApolloMocksWrapper({ + apolloMocks: mocks, +}); describe('useFieldMetadataItem', () => { it('should activateMetadataField', async () => { @@ -130,7 +150,10 @@ describe('useFieldMetadataItem', () => { }); await act(async () => { - const res = await result.current.activateMetadataField(fieldMetadataItem); + const res = await result.current.activateMetadataField( + fieldMetadataItem.id, + objectMetadataId, + ); expect(res.data).toEqual({ updateOneField: responseData.default, @@ -162,8 +185,10 @@ describe('useFieldMetadataItem', () => { }); await act(async () => { - const res = - await result.current.deactivateMetadataField(fieldMetadataItem); + const res = await result.current.deactivateMetadataField( + fieldMetadataItem.id, + objectMetadataId, + ); expect(res.data).toEqual({ updateOneField: responseData.default, diff --git a/packages/twenty-front/src/modules/object-metadata/hooks/useFieldMetadataItem.ts b/packages/twenty-front/src/modules/object-metadata/hooks/useFieldMetadataItem.ts index 3f39f8927..81e66472a 100644 --- a/packages/twenty-front/src/modules/object-metadata/hooks/useFieldMetadataItem.ts +++ b/packages/twenty-front/src/modules/object-metadata/hooks/useFieldMetadataItem.ts @@ -40,15 +40,23 @@ export const useFieldMetadataItem = () => { }); }; - const activateMetadataField = (metadataField: FieldMetadataItem) => + const activateMetadataField = ( + fieldMetadataId: string, + objectMetadataId: string, + ) => updateOneFieldMetadataItem({ - fieldMetadataIdToUpdate: metadataField.id, + objectMetadataId: objectMetadataId, + fieldMetadataIdToUpdate: fieldMetadataId, updatePayload: { isActive: true }, }); - const deactivateMetadataField = (metadataField: FieldMetadataItem) => + const deactivateMetadataField = ( + fieldMetadataId: string, + objectMetadataId: string, + ) => updateOneFieldMetadataItem({ - fieldMetadataIdToUpdate: metadataField.id, + objectMetadataId: objectMetadataId, + fieldMetadataIdToUpdate: fieldMetadataId, updatePayload: { isActive: false }, }); 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 af7da6979..55048b3e3 100644 --- a/packages/twenty-front/src/modules/object-metadata/hooks/useUpdateOneFieldMetadataItem.ts +++ b/packages/twenty-front/src/modules/object-metadata/hooks/useUpdateOneFieldMetadataItem.ts @@ -1,4 +1,4 @@ -import { useMutation } from '@apollo/client'; +import { useApolloClient, useMutation } from '@apollo/client'; import { getOperationName } from '@apollo/client/utilities'; import { @@ -9,10 +9,27 @@ import { import { UPDATE_ONE_FIELD_METADATA_ITEM } from '../graphql/mutations'; import { FIND_MANY_OBJECT_METADATA_ITEMS } from '../graphql/queries'; +import { CoreObjectNameSingular } from '@/object-metadata/types/CoreObjectNameSingular'; +import { useFindManyRecordsQuery } from '@/object-record/hooks/useFindManyRecordsQuery'; import { useApolloMetadataClient } from './useApolloMetadataClient'; export const useUpdateOneFieldMetadataItem = () => { const apolloMetadataClient = useApolloMetadataClient(); + const apolloClient = useApolloClient(); + + const { findManyRecordsQuery } = useFindManyRecordsQuery({ + objectNameSingular: CoreObjectNameSingular.View, + recordGqlFields: { + id: true, + viewGroups: { + id: true, + fieldMetadataId: true, + isVisible: true, + fieldValue: true, + position: true, + }, + }, + }); const [mutate] = useMutation< UpdateOneFieldMetadataItemMutation, @@ -22,9 +39,11 @@ export const useUpdateOneFieldMetadataItem = () => { }); const updateOneFieldMetadataItem = async ({ + objectMetadataId, fieldMetadataIdToUpdate, updatePayload, }: { + objectMetadataId: string; fieldMetadataIdToUpdate: UpdateOneFieldMetadataItemMutationVariables['idToUpdate']; updatePayload: Pick< UpdateOneFieldMetadataItemMutationVariables['updatePayload'], @@ -37,7 +56,7 @@ export const useUpdateOneFieldMetadataItem = () => { | 'options' >; }) => { - return await mutate({ + const result = await mutate({ variables: { idToUpdate: fieldMetadataIdToUpdate, updatePayload: { @@ -48,6 +67,20 @@ export const useUpdateOneFieldMetadataItem = () => { awaitRefetchQueries: true, refetchQueries: [getOperationName(FIND_MANY_OBJECT_METADATA_ITEMS) ?? ''], }); + + await apolloClient.query({ + query: findManyRecordsQuery, + variables: { + filter: { + objectMetadataId: { + eq: objectMetadataId, + }, + }, + }, + fetchPolicy: 'network-only', + }); + + return result; }; return { diff --git a/packages/twenty-front/src/modules/object-record/record-board/hooks/internal/useSetRecordBoardColumns.ts b/packages/twenty-front/src/modules/object-record/record-board/hooks/internal/useSetRecordBoardColumns.ts index 712fc2571..ec2dffc65 100644 --- a/packages/twenty-front/src/modules/object-record/record-board/hooks/internal/useSetRecordBoardColumns.ts +++ b/packages/twenty-front/src/modules/object-record/record-board/hooks/internal/useSetRecordBoardColumns.ts @@ -1,8 +1,8 @@ import { useRecoilCallback } from 'recoil'; import { useRecordBoardStates } from '@/object-record/record-board/hooks/internal/useRecordBoardStates'; -import { isDeeplyEqual } from '~/utils/isDeeplyEqual'; import { RecordGroupDefinition } from '@/object-record/record-group/types/RecordGroupDefinition'; +import { isDeeplyEqual } from '~/utils/isDeeplyEqual'; export const useSetRecordBoardColumns = (recordBoardId?: string) => { const { scopeId, columnIdsState, columnsFamilySelector } = @@ -19,12 +19,10 @@ export const useSetRecordBoardColumns = (recordBoardId?: string) => { .filter(({ isVisible }) => isVisible) .map(({ id }) => id); - if (isDeeplyEqual(currentColumnsIds, columnIds)) { - return; + if (!isDeeplyEqual(currentColumnsIds, columnIds)) { + set(columnIdsState, columnIds); } - set(columnIdsState, columnIds); - columns.forEach((column) => { const currentColumn = snapshot .getLoadable(columnsFamilySelector(column.id)) diff --git a/packages/twenty-front/src/modules/settings/data-model/object-details/components/SettingsObjectFieldItemTableRow.tsx b/packages/twenty-front/src/modules/settings/data-model/object-details/components/SettingsObjectFieldItemTableRow.tsx index 046064310..b1d2341d2 100644 --- a/packages/twenty-front/src/modules/settings/data-model/object-details/components/SettingsObjectFieldItemTableRow.tsx +++ b/packages/twenty-front/src/modules/settings/data-model/object-details/components/SettingsObjectFieldItemTableRow.tsx @@ -127,7 +127,10 @@ export const SettingsObjectFieldItemTableRow = ({ const handleDisableField = async ( activeFieldMetadatItem: FieldMetadataItem, ) => { - await deactivateMetadataField(activeFieldMetadatItem); + await deactivateMetadataField( + activeFieldMetadatItem.id, + objectMetadataItem.id, + ); const deletedViewIds = allViews .map((view) => { @@ -272,7 +275,9 @@ export const SettingsObjectFieldItemTableRow = ({ isCustomField={fieldMetadataItem.isCustom === true} scopeKey={fieldMetadataItem.id} onEdit={() => navigate(linkToNavigate)} - onActivate={() => activateMetadataField(fieldMetadataItem)} + onActivate={() => + activateMetadataField(fieldMetadataItem.id, objectMetadataItem.id) + } onDelete={() => deleteMetadataField(fieldMetadataItem)} /> ) : ( 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 2c28d7950..6d6779145 100644 --- a/packages/twenty-front/src/pages/settings/data-model/SettingsObjectFieldEdit.tsx +++ b/packages/twenty-front/src/pages/settings/data-model/SettingsObjectFieldEdit.tsx @@ -137,6 +137,7 @@ export const SettingsObjectFieldEdit = () => { if (isDefined(relationFieldMetadataItem)) { await updateOneFieldMetadataItem({ + objectMetadataId: objectMetadataItem.id, fieldMetadataIdToUpdate: relationFieldMetadataItem.id, updatePayload: formValues.relation.field, }); @@ -152,6 +153,7 @@ export const SettingsObjectFieldEdit = () => { ); await updateOneFieldMetadataItem({ + objectMetadataId: objectMetadataItem.id, fieldMetadataIdToUpdate: fieldMetadataItem.id, updatePayload: formattedInput, }); @@ -168,12 +170,12 @@ export const SettingsObjectFieldEdit = () => { }; const handleDeactivate = async () => { - await deactivateMetadataField(fieldMetadataItem); + await deactivateMetadataField(fieldMetadataItem.id, objectMetadataItem.id); navigate(`/settings/objects/${objectSlug}`); }; const handleActivate = async () => { - await activateMetadataField(fieldMetadataItem); + await activateMetadataField(fieldMetadataItem.id, objectMetadataItem.id); navigate(`/settings/objects/${objectSlug}`); }; diff --git a/packages/twenty-server/src/engine/metadata-modules/field-metadata/field-metadata.module.ts b/packages/twenty-server/src/engine/metadata-modules/field-metadata/field-metadata.module.ts index b6cb8e8ed..bf4c1978b 100644 --- a/packages/twenty-server/src/engine/metadata-modules/field-metadata/field-metadata.module.ts +++ b/packages/twenty-server/src/engine/metadata-modules/field-metadata/field-metadata.module.ts @@ -15,6 +15,7 @@ import { FieldMetadataDTO } from 'src/engine/metadata-modules/field-metadata/dto import { FieldMetadataValidationService } from 'src/engine/metadata-modules/field-metadata/field-metadata-validation.service'; import { FieldMetadataResolver } from 'src/engine/metadata-modules/field-metadata/field-metadata.resolver'; import { FieldMetadataGraphqlApiExceptionInterceptor } from 'src/engine/metadata-modules/field-metadata/interceptors/field-metadata-graphql-api-exception.interceptor'; +import { FieldMetadataRelatedRecordsService } from 'src/engine/metadata-modules/field-metadata/services/field-metadata-related-records.service'; import { IsFieldMetadataDefaultValue } from 'src/engine/metadata-modules/field-metadata/validators/is-field-metadata-default-value.validator'; import { IsFieldMetadataOptions } from 'src/engine/metadata-modules/field-metadata/validators/is-field-metadata-options.validator'; import { ObjectMetadataEntity } from 'src/engine/metadata-modules/object-metadata/object-metadata.entity'; @@ -48,6 +49,7 @@ import { UpdateFieldInput } from './dtos/update-field.input'; services: [ IsFieldMetadataDefaultValue, FieldMetadataService, + FieldMetadataRelatedRecordsService, FieldMetadataValidationService, ], resolvers: [ diff --git a/packages/twenty-server/src/engine/metadata-modules/field-metadata/field-metadata.service.ts b/packages/twenty-server/src/engine/metadata-modules/field-metadata/field-metadata.service.ts index 4291fe44a..86c273026 100644 --- a/packages/twenty-server/src/engine/metadata-modules/field-metadata/field-metadata.service.ts +++ b/packages/twenty-server/src/engine/metadata-modules/field-metadata/field-metadata.service.ts @@ -21,6 +21,7 @@ import { FieldMetadataException, FieldMetadataExceptionCode, } from 'src/engine/metadata-modules/field-metadata/field-metadata.exception'; +import { FieldMetadataRelatedRecordsService } from 'src/engine/metadata-modules/field-metadata/services/field-metadata-related-records.service'; import { assertDoesNotNullifyDefaultValueForNonNullableField } from 'src/engine/metadata-modules/field-metadata/utils/assert-does-not-nullify-default-value-for-non-nullable-field.util'; import { computeColumnName, @@ -28,6 +29,7 @@ import { } from 'src/engine/metadata-modules/field-metadata/utils/compute-column-name.util'; import { generateNullable } from 'src/engine/metadata-modules/field-metadata/utils/generate-nullable'; import { isCompositeFieldMetadataType } from 'src/engine/metadata-modules/field-metadata/utils/is-composite-field-metadata-type.util'; +import { isSelectFieldMetadataType } from 'src/engine/metadata-modules/field-metadata/utils/is-select-field-metadata-type.util'; import { ObjectMetadataEntity } from 'src/engine/metadata-modules/object-metadata/object-metadata.entity'; import { assertMutationNotOnRemoteObject } from 'src/engine/metadata-modules/object-metadata/utils/assert-mutation-not-on-remote-object.util'; import { @@ -83,6 +85,7 @@ export class FieldMetadataService extends TypeOrmQueryService = { + created: T[]; + updated: { old: T; new: T }[]; + deleted: T[]; +}; + +@Injectable() +export class FieldMetadataRelatedRecordsService { + constructor( + private readonly twentyORMGlobalManager: TwentyORMGlobalManager, + ) {} + + public async updateRelatedViewGroups( + oldFieldMetadata: FieldMetadataEntity, + newFieldMetadata: FieldMetadataEntity, + ) { + if ( + !isSelectFieldMetadataType(newFieldMetadata.type) || + !isSelectFieldMetadataType(oldFieldMetadata.type) + ) { + return; + } + + const views = await this.getFieldMetadataViews(newFieldMetadata); + + const { created, updated, deleted } = this.getOptionsDifferences( + oldFieldMetadata.options, + newFieldMetadata.options, + ); + + const viewGroupRepository = + await this.twentyORMGlobalManager.getRepositoryForWorkspace( + newFieldMetadata.workspaceId, + 'viewGroup', + ); + + for (const view of views) { + const maxPosition = view.viewGroups.reduce( + (max, viewGroup) => Math.max(max, viewGroup.position), + 0, + ); + + const viewGroupsToCreate = created.map((option, index) => + viewGroupRepository.create({ + fieldMetadataId: newFieldMetadata.id, + fieldValue: option.value, + position: maxPosition + index, + isVisible: true, + viewId: view.id, + }), + ); + + await viewGroupRepository.insert(viewGroupsToCreate); + + for (const { old: oldOption, new: newOption } of updated) { + const viewGroup = view.viewGroups.find( + (viewGroup) => viewGroup.fieldValue === oldOption.value, + ); + + if (!viewGroup) { + throw new Error(`View group not found for option ${oldOption.value}`); + } + + await viewGroupRepository.update( + { + id: viewGroup.id, + }, + { + fieldValue: newOption.value, + }, + ); + } + + const valuesToDelete = deleted.map((option) => option.value); + + await viewGroupRepository.delete({ + fieldMetadataId: newFieldMetadata.id, + fieldValue: In(valuesToDelete), + }); + } + } + + private getOptionsDifferences( + oldOptions: (FieldMetadataDefaultOption | FieldMetadataComplexOption)[], + newOptions: (FieldMetadataDefaultOption | FieldMetadataComplexOption)[], + ) { + const differences: Differences< + FieldMetadataDefaultOption | FieldMetadataComplexOption + > = { + created: [], + updated: [], + deleted: [], + }; + + const oldOptionsMap = new Map( + oldOptions.map((option) => [option.id, option]), + ); + const newOptionsMap = new Map( + newOptions.map((option) => [option.id, option]), + ); + + for (const newOption of newOptions) { + const oldOption = oldOptionsMap.get(newOption.id); + + if (!oldOption) { + differences.created.push(newOption); + } else if (oldOption.value !== newOption.value) { + differences.updated.push({ old: oldOption, new: newOption }); + } + } + + for (const oldOption of oldOptions) { + if (!newOptionsMap.has(oldOption.id)) { + differences.deleted.push(oldOption); + } + } + + return differences; + } + + private async getFieldMetadataViews( + fieldMetadata: FieldMetadataEntity, + ): Promise { + const viewRepository = + await this.twentyORMGlobalManager.getRepositoryForWorkspace( + fieldMetadata.workspaceId, + 'view', + ); + + return await viewRepository.find({ + where: { + kanbanFieldMetadataId: fieldMetadata.id, + }, + relations: ['viewGroups'], + }); + } +} diff --git a/packages/twenty-server/src/engine/metadata-modules/field-metadata/utils/is-select-field-metadata-type.util.ts b/packages/twenty-server/src/engine/metadata-modules/field-metadata/utils/is-select-field-metadata-type.util.ts new file mode 100644 index 000000000..56a918911 --- /dev/null +++ b/packages/twenty-server/src/engine/metadata-modules/field-metadata/utils/is-select-field-metadata-type.util.ts @@ -0,0 +1,7 @@ +import { FieldMetadataType } from 'src/engine/metadata-modules/field-metadata/field-metadata.entity'; + +export const isSelectFieldMetadataType = ( + type: FieldMetadataType, +): type is FieldMetadataType.SELECT => { + return type === FieldMetadataType.SELECT; +};