From 4a6ad48a6219dede778e3d61406f3d8e87f716c2 Mon Sep 17 00:00:00 2001 From: Marie <51697796+ijreilly@users.noreply.github.com> Date: Fri, 19 Sep 2025 12:24:09 +0200 Subject: [PATCH] Fix - Record text should always be visible and in first position in views (#14598) Fixes https://github.com/twentyhq/twenty/issues/14442 Issues were 1. Table headers always used label metadata identifier (or record text) as first column, while table body followed viewFields positions 2. Label metadata identifier should always be visible and in first position for the table views to work as expected, while when updating the label metadata identifier for an object, no changes were brought to the viewFields To fix that 1. In the BE: - a new logic is implemented to i) update label identifier's viewFields position and visibility when an object's label identifier is updated to a new field; ii) add validation on the update of a viewField's position and visibility to make sure the label identifier's viewField always has the lowest position + is visible - a command was added to check all existing views and viewfields 3. In the FE: at first I tried to replicate the logic of the headers (based on the label identifier rather than the positions) on the body, but it was too complex and error-prone as in multiple places we are based on the positions. It also feels more right to have only one source of truth which is the viewField position. @lucasbordeau if that does not suit you, we can throw an error if the field with the lowest position is not the label metadata identifier, as you said the table view will be very buggy / wont work if the label identifier is not in the first position. but now it should never be the case thanks to the validation implemented in the BE This should be migrated to viewFieldService V2 when relevant @Weiko @prastoin --- .../components/RecordTableHeader.tsx | 4 +- ...ell.tsx => RecordTableHeaderFirstCell.tsx} | 12 +- .../getVisibleFieldWithLowestPosition.util.ts | 15 + ...ntifier-position-and-visibility.command.ts | 132 +++++++ .../1-6/1-6-upgrade-version-command.module.ts | 9 +- .../services/tests/view-field.service.spec.ts | 224 ++++++++++++ .../view/services/view-field.service.ts | 345 ++++++++++++++---- .../view/services/view.service.ts | 16 + .../field-metadata-related-records.service.ts | 3 +- .../object-metadata.service.ts | 19 + ...object-metadata-related-records.service.ts | 57 ++- .../views/companies-all.view.ts | 3 +- .../constants/DEFAULT_VIEW_FIELD_SIZE.ts | 1 + .../views/custom-all.view.ts | 5 +- 14 files changed, 761 insertions(+), 84 deletions(-) rename packages/twenty-front/src/modules/object-record/record-table/record-table-header/components/{RecordTableHeaderLabelIdentifierCell.tsx => RecordTableHeaderFirstCell.tsx} (90%) create mode 100644 packages/twenty-front/src/modules/object-record/record-table/record-table-header/utils/getVisibleFieldWithLowestPosition.util.ts create mode 100644 packages/twenty-server/src/database/commands/upgrade-version-command/1-6/1-6-fix-label-identifier-position-and-visibility.command.ts create mode 100644 packages/twenty-server/src/engine/workspace-manager/standard-objects-prefill-data/views/constants/DEFAULT_VIEW_FIELD_SIZE.ts diff --git a/packages/twenty-front/src/modules/object-record/record-table/record-table-header/components/RecordTableHeader.tsx b/packages/twenty-front/src/modules/object-record/record-table/record-table-header/components/RecordTableHeader.tsx index 9d3593265e..2dbbeb1b56 100644 --- a/packages/twenty-front/src/modules/object-record/record-table/record-table-header/components/RecordTableHeader.tsx +++ b/packages/twenty-front/src/modules/object-record/record-table/record-table-header/components/RecordTableHeader.tsx @@ -4,8 +4,8 @@ import { RecordTableHeaderAddColumnButton } from '@/object-record/record-table/r import { RecordTableHeaderCell } from '@/object-record/record-table/record-table-header/components/RecordTableHeaderCell'; import { RecordTableHeaderCheckboxColumn } from '@/object-record/record-table/record-table-header/components/RecordTableHeaderCheckboxColumn'; import { RecordTableHeaderDragDropColumn } from '@/object-record/record-table/record-table-header/components/RecordTableHeaderDragDropColumn'; +import { RecordTableHeaderFirstCell } from '@/object-record/record-table/record-table-header/components/RecordTableHeaderFirstCell'; import { RecordTableHeaderFirstScrollableCell } from '@/object-record/record-table/record-table-header/components/RecordTableHeaderFirstScrollableCell'; -import { RecordTableHeaderLabelIdentifierCell } from '@/object-record/record-table/record-table-header/components/RecordTableHeaderLabelIdentifierCell'; import { RecordTableHeaderLastEmptyColumn } from '@/object-record/record-table/record-table-header/components/RecordTableHeaderLastEmptyColumn'; import { useResizeTableHeader } from '@/object-record/record-table/record-table-header/hooks/useResizeTableHeader'; import { filterOutByProperty } from 'twenty-shared/utils'; @@ -29,7 +29,7 @@ export const RecordTableHeader = () => { <> - + {recordFieldsWithoutLabelIdentifierAndFirstOne.map( (recordField, index) => ( diff --git a/packages/twenty-front/src/modules/object-record/record-table/record-table-header/components/RecordTableHeaderLabelIdentifierCell.tsx b/packages/twenty-front/src/modules/object-record/record-table/record-table-header/components/RecordTableHeaderFirstCell.tsx similarity index 90% rename from packages/twenty-front/src/modules/object-record/record-table/record-table-header/components/RecordTableHeaderLabelIdentifierCell.tsx rename to packages/twenty-front/src/modules/object-record/record-table/record-table-header/components/RecordTableHeaderFirstCell.tsx index 57511d7a39..1876559b9b 100644 --- a/packages/twenty-front/src/modules/object-record/record-table/record-table-header/components/RecordTableHeaderLabelIdentifierCell.tsx +++ b/packages/twenty-front/src/modules/object-record/record-table/record-table-header/components/RecordTableHeaderFirstCell.tsx @@ -1,6 +1,5 @@ import styled from '@emotion/styled'; -import { useRecordIndexContextOrThrow } from '@/object-record/record-index/contexts/RecordIndexContext'; import { useRecordTableContextOrThrow } from '@/object-record/record-table/contexts/RecordTableContext'; import { RecordTableColumnHeadWithDropdown } from '@/object-record/record-table/record-table-header/components/RecordTableColumnHeadWithDropdown'; import { RecordTableHeaderResizeHandler } from '@/object-record/record-table/record-table-header/components/RecordTableHeaderResizeHandler'; @@ -9,6 +8,7 @@ import { RecordTableHeaderCellContainer } from '@/object-record/record-table/rec import { hasRecordGroupsComponentSelector } from '@/object-record/record-group/states/selectors/hasRecordGroupsComponentSelector'; import { RecordTableHeaderLabelIdentifierCellPlusButton } from '@/object-record/record-table/record-table-header/components/RecordTableHeaderLabelIdentifierCellPlusButton'; +import { getVisibleFieldWithLowestPosition } from '@/object-record/record-table/record-table-header/utils/getVisibleFieldWithLowestPosition.util'; import { isRecordTableRowActiveComponentFamilyState } from '@/object-record/record-table/states/isRecordTableRowActiveComponentFamilyState'; import { isRecordTableRowFocusActiveComponentState } from '@/object-record/record-table/states/isRecordTableRowFocusActiveComponentState'; import { isRecordTableRowFocusedComponentFamilyState } from '@/object-record/record-table/states/isRecordTableRowFocusedComponentFamilyState'; @@ -19,7 +19,7 @@ import { useRecoilComponentFamilyValue } from '@/ui/utilities/state/component-st import { useRecoilComponentValue } from '@/ui/utilities/state/component-state/hooks/useRecoilComponentValue'; import { cx } from '@linaria/core'; import { useState } from 'react'; -import { findByProperty, isDefined } from 'twenty-shared/utils'; +import { isDefined } from 'twenty-shared/utils'; const StyledColumnHeadContainer = styled.div` cursor: pointer; @@ -30,7 +30,7 @@ const StyledColumnHeadContainer = styled.div` overflow: hidden; `; -export const RecordTableHeaderLabelIdentifierCell = () => { +export const RecordTableHeaderFirstCell = () => { const { objectMetadataItem, visibleRecordFields } = useRecordTableContextOrThrow(); @@ -46,11 +46,7 @@ export const RecordTableHeaderLabelIdentifierCell = () => { 0, ); - const { labelIdentifierFieldMetadataItem } = useRecordIndexContextOrThrow(); - - const recordField = visibleRecordFields.find( - findByProperty('fieldMetadataItemId', labelIdentifierFieldMetadataItem?.id), - ); + const recordField = getVisibleFieldWithLowestPosition(visibleRecordFields); const isScrolledVertically = useRecoilComponentValue( isRecordTableScrolledVerticallyComponentState, diff --git a/packages/twenty-front/src/modules/object-record/record-table/record-table-header/utils/getVisibleFieldWithLowestPosition.util.ts b/packages/twenty-front/src/modules/object-record/record-table/record-table-header/utils/getVisibleFieldWithLowestPosition.util.ts new file mode 100644 index 0000000000..e4999a045e --- /dev/null +++ b/packages/twenty-front/src/modules/object-record/record-table/record-table-header/utils/getVisibleFieldWithLowestPosition.util.ts @@ -0,0 +1,15 @@ +import { type RecordField } from '@/object-record/record-field/types/RecordField'; + +export const getVisibleFieldWithLowestPosition = ( + visibleRecordFields: RecordField[], +) => { + if (visibleRecordFields.length === 0) { + return undefined; + } + return visibleRecordFields.reduce((lowestPositionField, currentField) => { + if (currentField?.position < lowestPositionField?.position) { + return currentField; + } + return lowestPositionField; + }, visibleRecordFields[0]); +}; diff --git a/packages/twenty-server/src/database/commands/upgrade-version-command/1-6/1-6-fix-label-identifier-position-and-visibility.command.ts b/packages/twenty-server/src/database/commands/upgrade-version-command/1-6/1-6-fix-label-identifier-position-and-visibility.command.ts new file mode 100644 index 0000000000..077a43f373 --- /dev/null +++ b/packages/twenty-server/src/database/commands/upgrade-version-command/1-6/1-6-fix-label-identifier-position-and-visibility.command.ts @@ -0,0 +1,132 @@ +import { InjectRepository } from '@nestjs/typeorm'; + +import { Command } from 'nest-commander'; +import { Repository } from 'typeorm'; + +import { + ActiveOrSuspendedWorkspacesMigrationCommandRunner, + type RunOnWorkspaceArgs, +} from 'src/database/commands/command-runners/active-or-suspended-workspaces-migration.command-runner'; +import { ViewFieldEntity } from 'src/engine/core-modules/view/entities/view-field.entity'; +import { ViewEntity } from 'src/engine/core-modules/view/entities/view.entity'; +import { Workspace } from 'src/engine/core-modules/workspace/workspace.entity'; +import { TwentyORMGlobalManager } from 'src/engine/twenty-orm/twenty-orm-global.manager'; + +@Command({ + name: 'upgrade:1-6:fix-label-identifier-position-and-visibility', + description: + 'Fix label identifier position to ensure it has the minimal position in each view', +}) +export class FixLabelIdentifierPositionAndVisibilityCommand extends ActiveOrSuspendedWorkspacesMigrationCommandRunner { + constructor( + @InjectRepository(Workspace) + protected readonly workspaceRepository: Repository, + protected readonly twentyORMGlobalManager: TwentyORMGlobalManager, + @InjectRepository(ViewEntity) + private readonly viewRepository: Repository, + @InjectRepository(ViewFieldEntity) + private readonly viewFieldRepository: Repository, + ) { + super(workspaceRepository, twentyORMGlobalManager); + } + + override async runOnWorkspace({ + workspaceId, + options, + }: RunOnWorkspaceArgs): Promise { + this.logger.log( + `Checking label identifiers position and visibility for workspace ${workspaceId}`, + ); + + const views = await this.viewRepository.find({ + where: { workspaceId }, + relations: { + objectMetadata: true, + viewFields: true, + }, + }); + + for (const view of views) { + const { objectMetadata, viewFields } = view; + + if (!objectMetadata.labelIdentifierFieldMetadataId) { + continue; + } + + const labelIdentifierViewField = viewFields.find( + (viewField: ViewFieldEntity) => + viewField.fieldMetadataId === + objectMetadata.labelIdentifierFieldMetadataId, + ); + + if (!labelIdentifierViewField) { + continue; + } + + // Find minimum position and count fields at that position in single pass + const minPositionData = viewFields.reduce( + (acc, viewField: ViewFieldEntity) => { + if (viewField.position < acc.minPosition) { + return { minPosition: viewField.position, count: 1 }; + } + if (viewField.position === acc.minPosition) { + return { ...acc, count: acc.count + 1 }; + } + + return acc; + }, + { minPosition: Number.MAX_SAFE_INTEGER, count: 0 }, + ); + + const minPosition = minPositionData.minPosition; + const numberOfViewFieldsAtTheMinimalPosition = minPositionData.count; + + const labelIdentifierPositionIsAlreadyTheMinimalPosition = + labelIdentifierViewField.position === minPosition && + numberOfViewFieldsAtTheMinimalPosition === 1; + + const labelIdentifierIsAlreadyVisible = + labelIdentifierViewField.isVisible; + + if ( + labelIdentifierPositionIsAlreadyTheMinimalPosition && + labelIdentifierIsAlreadyVisible + ) { + continue; + } + + if (!labelIdentifierPositionIsAlreadyTheMinimalPosition) { + // Update the label identifier position to be the minimal one + const newPosition = minPosition - 1; + + if (!options.dryRun) { + await this.viewFieldRepository.update( + { id: labelIdentifierViewField.id }, + { position: newPosition }, + ); + + this.logger.log( + `Fixed label identifier position for view ${view.id} in workspace ${workspaceId}: ${labelIdentifierViewField.position} -> ${newPosition}`, + ); + } else { + this.logger.log( + `Would fix label identifier position for view ${view.id} in workspace ${workspaceId}: ${labelIdentifierViewField.position} -> ${newPosition}`, + ); + } + } + + if (!labelIdentifierIsAlreadyVisible) { + if (!options.dryRun) { + await this.viewFieldRepository.update( + { id: labelIdentifierViewField.id }, + { isVisible: true }, + ); + } + + this.logger.log( + `Fixed label identifier visibility for view ${view.id} in workspace ${workspaceId}: ${labelIdentifierViewField.isVisible} -> true`, + ); + } + } + } +} diff --git a/packages/twenty-server/src/database/commands/upgrade-version-command/1-6/1-6-upgrade-version-command.module.ts b/packages/twenty-server/src/database/commands/upgrade-version-command/1-6/1-6-upgrade-version-command.module.ts index 92fb5610a5..a943af9e1e 100644 --- a/packages/twenty-server/src/database/commands/upgrade-version-command/1-6/1-6-upgrade-version-command.module.ts +++ b/packages/twenty-server/src/database/commands/upgrade-version-command/1-6/1-6-upgrade-version-command.module.ts @@ -1,6 +1,9 @@ import { Module } from '@nestjs/common'; import { TypeOrmModule } from '@nestjs/typeorm'; +import { FixLabelIdentifierPositionAndVisibilityCommand } from 'src/database/commands/upgrade-version-command/1-6/1-6-fix-label-identifier-position-and-visibility.command'; +import { ViewFieldEntity } from 'src/engine/core-modules/view/entities/view-field.entity'; +import { ViewEntity } from 'src/engine/core-modules/view/entities/view.entity'; import { Workspace } from 'src/engine/core-modules/workspace/workspace.entity'; import { FieldMetadataEntity } from 'src/engine/metadata-modules/field-metadata/field-metadata.entity'; import { ObjectMetadataEntity } from 'src/engine/metadata-modules/object-metadata/object-metadata.entity'; @@ -14,12 +17,14 @@ import { WorkspaceDataSourceModule } from 'src/engine/workspace-datasource/works Workspace, FieldMetadataEntity, ObjectMetadataEntity, + ViewEntity, + ViewFieldEntity, ]), WorkspaceDataSourceModule, WorkspaceSchemaManagerModule, WorkspaceMetadataVersionModule, ], - providers: [], - exports: [], + providers: [FixLabelIdentifierPositionAndVisibilityCommand], + exports: [FixLabelIdentifierPositionAndVisibilityCommand], }) export class V1_6_UpgradeVersionCommandModule {} diff --git a/packages/twenty-server/src/engine/core-modules/view/services/tests/view-field.service.spec.ts b/packages/twenty-server/src/engine/core-modules/view/services/tests/view-field.service.spec.ts index 7bf501b16b..c3e83958de 100644 --- a/packages/twenty-server/src/engine/core-modules/view/services/tests/view-field.service.spec.ts +++ b/packages/twenty-server/src/engine/core-modules/view/services/tests/view-field.service.spec.ts @@ -1,9 +1,11 @@ import { Test, type TestingModule } from '@nestjs/testing'; import { getRepositoryToken } from '@nestjs/typeorm'; +import { UserInputError } from 'apollo-server-core'; import { type Repository } from 'typeorm'; import { ViewFieldEntity } from 'src/engine/core-modules/view/entities/view-field.entity'; +import { type ViewEntity } from 'src/engine/core-modules/view/entities/view.entity'; import { ViewFieldException, ViewFieldExceptionCode, @@ -12,10 +14,12 @@ import { generateViewFieldUserFriendlyExceptionMessage, } from 'src/engine/core-modules/view/exceptions/view-field.exception'; import { ViewFieldService } from 'src/engine/core-modules/view/services/view-field.service'; +import { ViewService } from 'src/engine/core-modules/view/services/view.service'; describe('ViewFieldService', () => { let viewFieldService: ViewFieldService; let viewFieldRepository: Repository; + let viewService: ViewService; const mockViewField = { id: 'view-field-id', @@ -45,6 +49,12 @@ describe('ViewFieldService', () => { delete: jest.fn(), }, }, + { + provide: ViewService, + useValue: { + findByIdWithRelatedObjectMetadata: jest.fn(), + }, + }, ], }).compile(); @@ -52,6 +62,7 @@ describe('ViewFieldService', () => { viewFieldRepository = module.get>( getRepositoryToken(ViewFieldEntity), ); + viewService = module.get(ViewService); }); it('should be defined', () => { @@ -203,6 +214,54 @@ describe('ViewFieldService', () => { ), ); }); + + it('should throw exception if position is lower than label metadata identifier', async () => { + const labelIdentifierFieldMetadataId = + 'label-identifier-field-matadata-id'; + const labelIdentifierViewFieldId = + 'view-field-for-label-metadata-identifier-id'; + + const labelIdentifierViewField = { + ...mockViewField, + id: labelIdentifierViewFieldId, + fieldMetadataId: labelIdentifierFieldMetadataId, + position: 0, + }; + + const mockView = { + id: 'view-id', + objectMetadata: { + labelIdentifierFieldMetadataId, + }, + viewFields: [ + labelIdentifierViewField, + { ...mockViewField, position: 1 }, + ], + } as ViewEntity; + + jest.spyOn(viewFieldService, 'findById').mockImplementation((id) => { + if (id === mockViewField.id) { + return Promise.resolve(mockViewField); + } + + return Promise.resolve(null); + }); + jest + .spyOn(viewService, 'findByIdWithRelatedObjectMetadata') + .mockResolvedValue(mockView); + + const invalidData = { ...validViewFieldData, position: -1 }; + + await expect(viewFieldService.create(invalidData)).rejects.toThrow( + new UserInputError( + 'Label metadata identifier must keep the minimal position in the view.', + { + userFriendlyMessage: + 'Record text must be in first position of the view.', + }, + ), + ); + }); }); describe('update', () => { @@ -212,10 +271,21 @@ describe('ViewFieldService', () => { const updateData = { position: 1 }; const updatedViewField = { ...mockViewField, ...updateData }; + const mockView = { + id: 'view-id', + objectMetadata: { + labelIdentifierFieldMetadataId: mockViewField.fieldMetadataId, + }, + viewFields: [mockViewField], + } as ViewEntity; + jest.spyOn(viewFieldService, 'findById').mockResolvedValue(mockViewField); jest .spyOn(viewFieldRepository, 'save') .mockResolvedValue(updatedViewField); + jest + .spyOn(viewService, 'findByIdWithRelatedObjectMetadata') + .mockResolvedValue(mockView); const result = await viewFieldService.update(id, workspaceId, updateData); @@ -246,6 +316,160 @@ describe('ViewFieldService', () => { ), ); }); + + it('should throw exception when label metadata identifier is not in first position (label metadata identifier field update case)', async () => { + const workspaceId = 'workspace-id'; + const updateData = { position: 2 }; + const labelIdentifierFieldMetadataId = + 'label-identifier-field-matadata-id'; + const labelIdentifierViewFieldId = + 'view-field-for-label-metadata-identifier-id'; + + const labelIdentifierViewField = { + ...mockViewField, + id: labelIdentifierViewFieldId, + fieldMetadataId: labelIdentifierFieldMetadataId, + position: 0, + }; + + const mockView = { + id: 'view-id', + objectMetadata: { + labelIdentifierFieldMetadataId, + }, + viewFields: [ + labelIdentifierViewField, + { ...mockViewField, position: 1 }, + ], + } as ViewEntity; + + jest.spyOn(viewFieldService, 'findById').mockImplementation((id) => { + if (id === labelIdentifierViewFieldId) { + return Promise.resolve(labelIdentifierViewField); + } + + return Promise.resolve(null); + }); + jest + .spyOn(viewService, 'findByIdWithRelatedObjectMetadata') + .mockResolvedValue(mockView); + + await expect( + viewFieldService.update( + labelIdentifierViewFieldId, + workspaceId, + updateData, + ), + ).rejects.toThrow( + new UserInputError( + 'Label metadata identifier must keep the minimal position in the view.', + { + userFriendlyMessage: + 'Record text must be in first position of the view.', + }, + ), + ); + }); + + it('should throw exception when label metadata identifier is not in first position (regular field update case)', async () => { + const workspaceId = 'workspace-id'; + const updateData = { position: -1 }; + const labelIdentifierFieldMetadataId = + 'label-identifier-field-matadata-id'; + const labelIdentifierViewFieldId = + 'view-field-for-label-metadata-identifier-id'; + + const labelIdentifierViewField = { + ...mockViewField, + id: labelIdentifierViewFieldId, + fieldMetadataId: labelIdentifierFieldMetadataId, + position: 0, + }; + + const mockView = { + id: 'view-id', + objectMetadata: { + labelIdentifierFieldMetadataId, + }, + viewFields: [ + labelIdentifierViewField, + { ...mockViewField, position: 1 }, + ], + } as ViewEntity; + + jest.spyOn(viewFieldService, 'findById').mockImplementation((id) => { + if (id === mockViewField.id) { + return Promise.resolve(mockViewField); + } + + return Promise.resolve(null); + }); + jest + .spyOn(viewService, 'findByIdWithRelatedObjectMetadata') + .mockResolvedValue(mockView); + + await expect( + viewFieldService.update(mockViewField.id, workspaceId, updateData), + ).rejects.toThrow( + new UserInputError( + 'Label metadata identifier must keep the minimal position in the view.', + { + userFriendlyMessage: + 'Record text must be in first position of the view.', + }, + ), + ); + }); + + it('should throw exception when attempting to make label metadata identifier invisible', async () => { + const workspaceId = 'workspace-id'; + const updateData = { isVisible: false }; + const labelIdentifierFieldMetadataId = + 'label-identifier-field-matadata-id'; + const labelIdentifierViewFieldId = + 'view-field-for-label-metadata-identifier-id'; + + const labelIdentifierViewField = { + ...mockViewField, + id: labelIdentifierViewFieldId, + fieldMetadataId: labelIdentifierFieldMetadataId, + position: 0, + }; + + const mockView = { + id: 'view-id', + objectMetadata: { + labelIdentifierFieldMetadataId, + }, + viewFields: [labelIdentifierViewField, mockViewField], + } as ViewEntity; + + jest.spyOn(viewFieldService, 'findById').mockImplementation((id) => { + if (id === labelIdentifierViewFieldId) { + return Promise.resolve(labelIdentifierViewField); + } + + return Promise.resolve(null); + }); + jest + .spyOn( + viewFieldService['viewService'], + 'findByIdWithRelatedObjectMetadata', + ) + .mockResolvedValue(mockView); + + await expect( + viewFieldService.update( + labelIdentifierViewField.id, + workspaceId, + updateData, + ), + ).rejects.toThrow( + new UserInputError('Label metadata identifier must stay visible.', { + userFriendlyMessage: 'Record text must stay visible.', + }), + ); + }); }); describe('delete', () => { diff --git a/packages/twenty-server/src/engine/core-modules/view/services/view-field.service.ts b/packages/twenty-server/src/engine/core-modules/view/services/view-field.service.ts index 39e4077fff..8621938b20 100644 --- a/packages/twenty-server/src/engine/core-modules/view/services/view-field.service.ts +++ b/packages/twenty-server/src/engine/core-modules/view/services/view-field.service.ts @@ -4,6 +4,7 @@ import { InjectRepository } from '@nestjs/typeorm'; import { isDefined } from 'twenty-shared/utils'; import { IsNull, Repository } from 'typeorm'; +import { UserInputError } from 'src/engine/core-modules/graphql/utils/graphql-errors.util'; import { ViewFieldEntity } from 'src/engine/core-modules/view/entities/view-field.entity'; import { ViewFieldException, @@ -12,12 +13,14 @@ import { generateViewFieldExceptionMessage, generateViewFieldUserFriendlyExceptionMessage, } from 'src/engine/core-modules/view/exceptions/view-field.exception'; +import { ViewService } from 'src/engine/core-modules/view/services/view.service'; @Injectable() export class ViewFieldService { constructor( @InjectRepository(ViewFieldEntity) private readonly viewFieldRepository: Repository, + private readonly viewService: ViewService, ) {} async findByWorkspaceId(workspaceId: string): Promise { @@ -65,35 +68,105 @@ export class ViewFieldService { async create( viewFieldData: Partial, ): Promise { - if (!isDefined(viewFieldData.workspaceId)) { - throw new ViewFieldException( - generateViewFieldExceptionMessage( - ViewFieldExceptionMessageKey.WORKSPACE_ID_REQUIRED, - ), - ViewFieldExceptionCode.INVALID_VIEW_FIELD_DATA, - { - userFriendlyMessage: generateViewFieldUserFriendlyExceptionMessage( + if (this.hasRequiredFields(viewFieldData)) { + try { + if (isDefined(viewFieldData.position)) { + const viewFieldDataWithPosition = + viewFieldData as typeof viewFieldData & { position: number }; + + await this.verifyLabelMetadataIdentifierIsInFirstPositionOrThrow( + viewFieldDataWithPosition, + viewFieldData.workspaceId, + ); + } + + await this.verifyLabelMetadataIdentifierIsVisibleOrThrow( + viewFieldData, + viewFieldData.workspaceId, + ); + + const viewFieldDataWithPosition = await this.formatViewFieldData( + viewFieldData, + viewFieldData.workspaceId, + ); + + const viewField = this.viewFieldRepository.create( + viewFieldDataWithPosition, + ); + + const savedViewField = await this.viewFieldRepository.save(viewField); + const createdViewField = await this.findById( + savedViewField.id, + viewFieldData.workspaceId, + ); + + if (!isDefined(createdViewField)) { + throw new ViewFieldException( + generateViewFieldExceptionMessage( + ViewFieldExceptionMessageKey.VIEW_FIELD_NOT_FOUND, + ), + ViewFieldExceptionCode.VIEW_FIELD_NOT_FOUND, + { + userFriendlyMessage: + generateViewFieldUserFriendlyExceptionMessage( + ViewFieldExceptionMessageKey.VIEW_FIELD_NOT_FOUND, + ), + }, + ); + } + + return createdViewField; + } catch (error) { + if ( + error.message.includes( + 'duplicate key value violates unique constraint', + ) + ) { + throw new ViewFieldException( + generateViewFieldExceptionMessage( + ViewFieldExceptionMessageKey.VIEW_FIELD_ALREADY_EXISTS, + ), + ViewFieldExceptionCode.INVALID_VIEW_FIELD_DATA, + { + userFriendlyMessage: + generateViewFieldUserFriendlyExceptionMessage( + ViewFieldExceptionMessageKey.VIEW_FIELD_ALREADY_EXISTS, + ), + }, + ); + } + + throw error; + } + } else { + if (!isDefined(viewFieldData.workspaceId)) { + throw new ViewFieldException( + generateViewFieldExceptionMessage( ViewFieldExceptionMessageKey.WORKSPACE_ID_REQUIRED, ), - }, - ); - } + ViewFieldExceptionCode.INVALID_VIEW_FIELD_DATA, + { + userFriendlyMessage: generateViewFieldUserFriendlyExceptionMessage( + ViewFieldExceptionMessageKey.WORKSPACE_ID_REQUIRED, + ), + }, + ); + } - if (!isDefined(viewFieldData.viewId)) { - throw new ViewFieldException( - generateViewFieldExceptionMessage( - ViewFieldExceptionMessageKey.VIEW_ID_REQUIRED, - ), - ViewFieldExceptionCode.INVALID_VIEW_FIELD_DATA, - { - userFriendlyMessage: generateViewFieldUserFriendlyExceptionMessage( + if (!isDefined(viewFieldData.viewId)) { + throw new ViewFieldException( + generateViewFieldExceptionMessage( ViewFieldExceptionMessageKey.VIEW_ID_REQUIRED, ), - }, - ); - } + ViewFieldExceptionCode.INVALID_VIEW_FIELD_DATA, + { + userFriendlyMessage: generateViewFieldUserFriendlyExceptionMessage( + ViewFieldExceptionMessageKey.VIEW_ID_REQUIRED, + ), + }, + ); + } - if (!isDefined(viewFieldData.fieldMetadataId)) { throw new ViewFieldException( generateViewFieldExceptionMessage( ViewFieldExceptionMessageKey.FIELD_METADATA_ID_REQUIRED, @@ -106,50 +179,6 @@ export class ViewFieldService { }, ); } - - try { - const viewField = this.viewFieldRepository.create(viewFieldData); - - const savedViewField = await this.viewFieldRepository.save(viewField); - const createdViewField = await this.findById( - savedViewField.id, - viewFieldData.workspaceId, - ); - - if (!isDefined(createdViewField)) { - throw new ViewFieldException( - generateViewFieldExceptionMessage( - ViewFieldExceptionMessageKey.VIEW_FIELD_NOT_FOUND, - ), - ViewFieldExceptionCode.VIEW_FIELD_NOT_FOUND, - { - userFriendlyMessage: generateViewFieldUserFriendlyExceptionMessage( - ViewFieldExceptionMessageKey.VIEW_FIELD_NOT_FOUND, - ), - }, - ); - } - - return createdViewField; - } catch (error) { - if ( - error.message.includes('duplicate key value violates unique constraint') - ) { - throw new ViewFieldException( - generateViewFieldExceptionMessage( - ViewFieldExceptionMessageKey.VIEW_FIELD_ALREADY_EXISTS, - ), - ViewFieldExceptionCode.INVALID_VIEW_FIELD_DATA, - { - userFriendlyMessage: generateViewFieldUserFriendlyExceptionMessage( - ViewFieldExceptionMessageKey.VIEW_FIELD_ALREADY_EXISTS, - ), - }, - ); - } - - throw error; - } } async update( @@ -168,6 +197,26 @@ export class ViewFieldService { ViewFieldExceptionCode.VIEW_FIELD_NOT_FOUND, ); } + const viewId = existingViewField.viewId; + + if (this.updatesPosition(updateData)) { + await this.verifyLabelMetadataIdentifierIsInFirstPositionOrThrow( + { + ...updateData, + viewId, + id, + fieldMetadataId: existingViewField.fieldMetadataId, + }, + workspaceId, + ); + } + + if (this.disablesVisibility(updateData)) { + await this.verifyLabelMetadataIdentifierIsVisibleOrThrow( + { ...updateData, viewId, id }, + workspaceId, + ); + } const updatedViewField = await this.viewFieldRepository.save({ id, @@ -212,4 +261,166 @@ export class ViewFieldService { return viewField; } + + private updatesPosition( + data: Partial, + ): data is Partial & { position: number } { + return isDefined(data.position); + } + + private disablesVisibility( + data: Partial, + ): data is Partial & { isVisible: boolean } { + return data.isVisible === false; + } + + private async verifyLabelMetadataIdentifierIsVisibleOrThrow( + newOrUpdatedViewField: Partial & { + viewId: string; + }, + workspaceId: string, + ) { + const view = await this.viewService.findByIdWithRelatedObjectMetadata( + newOrUpdatedViewField.viewId, + workspaceId, + ); + + if (!isDefined(view)) { + throw new Error(`View not found: ${newOrUpdatedViewField.viewId}`); + } + + const labelMetadataIdentifierFieldMetadataId = + view.objectMetadata.labelIdentifierFieldMetadataId; + + const labelMetadataIdentifierViewField = view.viewFields.find( + (viewField) => + viewField.fieldMetadataId === labelMetadataIdentifierFieldMetadataId, + ); + + if ( + !isDefined(labelMetadataIdentifierViewField) || + labelMetadataIdentifierViewField.id !== newOrUpdatedViewField.id + ) { + return; + } + + if (newOrUpdatedViewField.isVisible === false) { + throw new UserInputError('Label metadata identifier must stay visible.', { + userFriendlyMessage: 'Record text must stay visible.', + }); + } + } + + private async verifyLabelMetadataIdentifierIsInFirstPositionOrThrow( + newOrUpdatedViewField: Partial & { viewId: string } & { + position: number; + }, + workspaceId: string, + ) { + const view = await this.viewService.findByIdWithRelatedObjectMetadata( + newOrUpdatedViewField.viewId, + workspaceId, + ); + + if (!isDefined(view)) { + throw new Error(`View not found: ${newOrUpdatedViewField.viewId}`); + } + + const viewFieldsWithoutUpdatedViewField = view.viewFields.filter( + (viewField) => viewField.id !== newOrUpdatedViewField?.id, + ); + + if (viewFieldsWithoutUpdatedViewField.length === 0) { + return; + } + + const labelMetadataIdentifierFieldMetadataId = + view.objectMetadata.labelIdentifierFieldMetadataId; + + if ( + labelMetadataIdentifierFieldMetadataId === + newOrUpdatedViewField.fieldMetadataId + ) { + const minPositionInViewWithoutUpdatedViewField = + viewFieldsWithoutUpdatedViewField.reduce( + (minViewField, viewField) => + viewField.position < minViewField.position + ? viewField + : minViewField, + viewFieldsWithoutUpdatedViewField[0], + ).position; + + if ( + newOrUpdatedViewField.position >= + minPositionInViewWithoutUpdatedViewField + ) { + throw new UserInputError( + 'Label metadata identifier must keep the minimal position in the view.', + { + userFriendlyMessage: + 'Record text must be in first position of the view.', + }, + ); + } + } else { + const labelMetadataIdentifierViewFieldPosition = view.viewFields.find( + (viewField) => + viewField.fieldMetadataId === labelMetadataIdentifierFieldMetadataId, + )?.position; + + if ( + isDefined(labelMetadataIdentifierViewFieldPosition) && + newOrUpdatedViewField.position <= + labelMetadataIdentifierViewFieldPosition + ) { + throw new UserInputError( + 'Label metadata identifier must keep the minimal position in the view.', + { + userFriendlyMessage: + 'Record text must be in first position of the view.', + }, + ); + } + } + } + + private async formatViewFieldData( + viewFieldData: Partial & { viewId: string }, + workspaceId: string, + ): Promise> { + if (!isDefined(viewFieldData.position)) { + const view = await this.viewService.findByIdWithRelatedObjectMetadata( + viewFieldData.viewId, + workspaceId, + ); + + if (!isDefined(view)) { + throw new Error(`View not found: ${viewFieldData.viewId}`); + } + + const highestPositionInView = view.viewFields.reduce( + (maxViewField, viewField) => + viewField.position > maxViewField.position ? viewField : maxViewField, + view.viewFields[0], + ); + + return { ...viewFieldData, position: highestPositionInView.position + 1 }; + } else { + return viewFieldData; + } + } + + private hasRequiredFields( + data: Partial, + ): data is Partial & { + viewId: string; + fieldMetadataId: string; + workspaceId: string; + } { + return ( + isDefined(data.viewId) && + isDefined(data.fieldMetadataId) && + isDefined(data.workspaceId) + ); + } } diff --git a/packages/twenty-server/src/engine/core-modules/view/services/view.service.ts b/packages/twenty-server/src/engine/core-modules/view/services/view.service.ts index 35cb861391..9b83c4ff7b 100644 --- a/packages/twenty-server/src/engine/core-modules/view/services/view.service.ts +++ b/packages/twenty-server/src/engine/core-modules/view/services/view.service.ts @@ -84,6 +84,22 @@ export class ViewService { return view || null; } + async findByIdWithRelatedObjectMetadata( + id: string, + workspaceId: string, + ): Promise { + const view = await this.viewRepository.findOne({ + where: { + id, + workspaceId, + deletedAt: IsNull(), + }, + relations: ['workspace', 'objectMetadata', 'viewFields'], + }); + + return view || null; + } + async create(viewData: Partial): Promise { if (!isDefined(viewData.workspaceId)) { throw new ViewException( diff --git a/packages/twenty-server/src/engine/metadata-modules/field-metadata/services/field-metadata-related-records.service.ts b/packages/twenty-server/src/engine/metadata-modules/field-metadata/services/field-metadata-related-records.service.ts index 7b451f448a..160668f096 100644 --- a/packages/twenty-server/src/engine/metadata-modules/field-metadata/services/field-metadata-related-records.service.ts +++ b/packages/twenty-server/src/engine/metadata-modules/field-metadata/services/field-metadata-related-records.service.ts @@ -22,6 +22,7 @@ import { } from 'src/engine/metadata-modules/field-metadata/field-metadata.exception'; import { isSelectFieldMetadataType } from 'src/engine/metadata-modules/field-metadata/utils/is-select-field-metadata-type.util'; import { type SelectOrMultiSelectFieldMetadataEntity } from 'src/engine/metadata-modules/field-metadata/utils/is-select-or-multi-select-field-metadata.util'; +import { DEFAULT_VIEW_FIELD_SIZE } from 'src/engine/workspace-manager/standard-objects-prefill-data/views/constants/DEFAULT_VIEW_FIELD_SIZE'; type Differences = { created: T[]; @@ -376,7 +377,7 @@ export class FieldMetadataRelatedRecordsService { fieldMetadataId: createdFieldMetadata.id, position: lastPosition + 1, isVisible, - size: 180, + size: DEFAULT_VIEW_FIELD_SIZE, viewId: indexView.id, workspaceId: createdFieldMetadata.workspaceId, }); diff --git a/packages/twenty-server/src/engine/metadata-modules/object-metadata/object-metadata.service.ts b/packages/twenty-server/src/engine/metadata-modules/object-metadata/object-metadata.service.ts index 228e00e44d..fdaa3bfdd9 100644 --- a/packages/twenty-server/src/engine/metadata-modules/object-metadata/object-metadata.service.ts +++ b/packages/twenty-server/src/engine/metadata-modules/object-metadata/object-metadata.service.ts @@ -401,6 +401,7 @@ export class ObjectMetadataService extends TypeOrmQueryService + viewField.fieldMetadataId === + newLabelMetadataIdentifierFieldMetadata.id, + ); + + const currentMinPositionAmongViewFields = this.getViewFieldsMinPosition( + view.viewFields, + ); + + if (!labelMetadataIdentifierViewField) { + await this.viewFieldService.create({ + fieldMetadataId: newLabelMetadataIdentifierFieldMetadata.id, + position: currentMinPositionAmongViewFields - 1, + isVisible: true, + size: DEFAULT_VIEW_FIELD_SIZE, + viewId: view.id, + workspaceId: newLabelMetadataIdentifierFieldMetadata.workspaceId, + }); + continue; + } + + await this.viewFieldService.update( + labelMetadataIdentifierViewField.id, + newLabelMetadataIdentifierFieldMetadata.workspaceId, + { + position: currentMinPositionAmongViewFields - 1, + isVisible: true, + }, + ); + } + } + + private getViewFieldsMinPosition(viewFields: ViewFieldEntity[]): number { + if (viewFields.length === 0) { + return 0; + } + + return viewFields.reduce((min, field) => Math.min(min, field.position), 0); + } + private async createView( objectMetadata: ObjectMetadataEntity, ): Promise { @@ -56,7 +111,7 @@ export class ObjectMetadataRelatedRecordsService { fieldMetadataId: field.id, position: index, isVisible: true, - size: 180, + size: DEFAULT_VIEW_FIELD_SIZE, viewId: viewId, workspaceId: objectMetadata.workspaceId, })); diff --git a/packages/twenty-server/src/engine/workspace-manager/standard-objects-prefill-data/views/companies-all.view.ts b/packages/twenty-server/src/engine/workspace-manager/standard-objects-prefill-data/views/companies-all.view.ts index a852d9c63a..e1882954fb 100644 --- a/packages/twenty-server/src/engine/workspace-manager/standard-objects-prefill-data/views/companies-all.view.ts +++ b/packages/twenty-server/src/engine/workspace-manager/standard-objects-prefill-data/views/companies-all.view.ts @@ -2,6 +2,7 @@ import { msg } from '@lingui/core/macro'; import { AggregateOperations } from 'src/engine/api/graphql/graphql-query-runner/constants/aggregate-operations.constant'; import { type ObjectMetadataEntity } from 'src/engine/metadata-modules/object-metadata/object-metadata.entity'; +import { DEFAULT_VIEW_FIELD_SIZE } from 'src/engine/workspace-manager/standard-objects-prefill-data/views/constants/DEFAULT_VIEW_FIELD_SIZE'; import { BASE_OBJECT_STANDARD_FIELD_IDS, COMPANY_STANDARD_FIELD_IDS, @@ -37,7 +38,7 @@ export const companiesAllView = ( )?.id ?? '', position: 0, isVisible: true, - size: 180, + size: DEFAULT_VIEW_FIELD_SIZE, }, { fieldMetadataId: diff --git a/packages/twenty-server/src/engine/workspace-manager/standard-objects-prefill-data/views/constants/DEFAULT_VIEW_FIELD_SIZE.ts b/packages/twenty-server/src/engine/workspace-manager/standard-objects-prefill-data/views/constants/DEFAULT_VIEW_FIELD_SIZE.ts new file mode 100644 index 0000000000..6be2a05d01 --- /dev/null +++ b/packages/twenty-server/src/engine/workspace-manager/standard-objects-prefill-data/views/constants/DEFAULT_VIEW_FIELD_SIZE.ts @@ -0,0 +1 @@ +export const DEFAULT_VIEW_FIELD_SIZE = 180; diff --git a/packages/twenty-server/src/engine/workspace-manager/standard-objects-prefill-data/views/custom-all.view.ts b/packages/twenty-server/src/engine/workspace-manager/standard-objects-prefill-data/views/custom-all.view.ts index 8d7e98022a..ae8a315d15 100644 --- a/packages/twenty-server/src/engine/workspace-manager/standard-objects-prefill-data/views/custom-all.view.ts +++ b/packages/twenty-server/src/engine/workspace-manager/standard-objects-prefill-data/views/custom-all.view.ts @@ -1,6 +1,7 @@ import { msg } from '@lingui/core/macro'; import { type ObjectMetadataEntity } from 'src/engine/metadata-modules/object-metadata/object-metadata.entity'; +import { DEFAULT_VIEW_FIELD_SIZE } from 'src/engine/workspace-manager/standard-objects-prefill-data/views/constants/DEFAULT_VIEW_FIELD_SIZE'; export const customAllView = ( objectMetadataItem: ObjectMetadataEntity, @@ -39,13 +40,13 @@ export const customAllView = ( ?.id ?? '', position: 0, isVisible: true, - size: 180, + size: DEFAULT_VIEW_FIELD_SIZE, }, ...otherFields.map((field, index) => ({ fieldMetadataId: field.id, position: index + 1, isVisible: true, - size: 180, + size: DEFAULT_VIEW_FIELD_SIZE, })), ], };