diff --git a/packages/twenty-front/src/modules/object-record/record-board/components/RecordBoard.tsx b/packages/twenty-front/src/modules/object-record/record-board/components/RecordBoard.tsx index d1e011e590..23aa861288 100644 --- a/packages/twenty-front/src/modules/object-record/record-board/components/RecordBoard.tsx +++ b/packages/twenty-front/src/modules/object-record/record-board/components/RecordBoard.tsx @@ -9,6 +9,7 @@ import { useRecordBoardStates } from '@/object-record/record-board/hooks/interna import { useRecordBoardSelection } from '@/object-record/record-board/hooks/useRecordBoardSelection'; import { RecordBoardColumn } from '@/object-record/record-board/record-board-column/components/RecordBoardColumn'; import { RecordBoardScope } from '@/object-record/record-board/scopes/RecordBoardScope'; +import { getDraggedRecordPosition } from '@/object-record/record-board/utils/get-dragged-record-position.util'; import { recordStoreFamilyState } from '@/object-record/record-store/states/recordStoreFamilyState'; import { TableHotkeyScope } from '@/object-record/record-table/types/TableHotkeyScope'; import { DragSelect } from '@/ui/utilities/drag-select/components/DragSelect'; @@ -107,7 +108,6 @@ export const RecordBoard = ({ recordBoardId }: RecordBoardProps) => { .getLoadable(recordStoreFamilyState(recordBeforeId)) .getValue() : null; - const recordBeforePosition: number | undefined = recordBefore?.position; const recordAfterId = otherRecordsInDestinationColumn[destinationIndexInColumn]; @@ -116,12 +116,11 @@ export const RecordBoard = ({ recordBoardId }: RecordBoardProps) => { .getLoadable(recordStoreFamilyState(recordAfterId)) .getValue() : null; - const recordAfterPosition: number | undefined = recordAfter?.position; - const beforeBoundary = recordBeforePosition ?? 0; - const afterBoundary = recordAfterPosition ?? beforeBoundary + 1; - - const draggedRecordPosition = (beforeBoundary + afterBoundary) / 2; + const draggedRecordPosition = getDraggedRecordPosition( + recordBefore?.position, + recordAfter?.position, + ); updateOneRecord({ idToUpdate: draggedRecordId, diff --git a/packages/twenty-front/src/modules/object-record/record-board/utils/__tests__/get-dragged-record-position.util.test.ts b/packages/twenty-front/src/modules/object-record/record-board/utils/__tests__/get-dragged-record-position.util.test.ts new file mode 100644 index 0000000000..1359c2d597 --- /dev/null +++ b/packages/twenty-front/src/modules/object-record/record-board/utils/__tests__/get-dragged-record-position.util.test.ts @@ -0,0 +1,27 @@ +import { getDraggedRecordPosition } from '../get-dragged-record-position.util'; + +describe('getDraggedRecordPosition', () => { + it('when both records defined and positive, should return the average of the two positions', () => { + expect(getDraggedRecordPosition(1, 3)).toBe(2); + }); + + it('when both records defined and negative, should return the average of the two positions', () => { + expect(getDraggedRecordPosition(-3, -1)).toBe(-2); + }); + + it('when both records defined and one negative, should return the average of the two positions', () => { + expect(getDraggedRecordPosition(-1, 3)).toBe(1); + }); + + it('when only record after, should return the position - 1', () => { + expect(getDraggedRecordPosition(undefined, 3)).toBe(2); + }); + + it('when only record before, should return the position + 1', () => { + expect(getDraggedRecordPosition(1, undefined)).toBe(2); + }); + + it('when both records undefined, should return 1', () => { + expect(getDraggedRecordPosition(undefined, undefined)).toBe(1); + }); +}); diff --git a/packages/twenty-front/src/modules/object-record/record-board/utils/get-dragged-record-position.util.ts b/packages/twenty-front/src/modules/object-record/record-board/utils/get-dragged-record-position.util.ts new file mode 100644 index 0000000000..1a694c4e6e --- /dev/null +++ b/packages/twenty-front/src/modules/object-record/record-board/utils/get-dragged-record-position.util.ts @@ -0,0 +1,16 @@ +import { isDefined } from '~/utils/isDefined'; + +export const getDraggedRecordPosition = ( + recordBeforePosition?: number, + recordAfterPosition?: number, +): number => { + if (isDefined(recordAfterPosition) && isDefined(recordBeforePosition)) { + return (recordBeforePosition + recordAfterPosition) / 2; + } else if (isDefined(recordAfterPosition)) { + return recordAfterPosition - 1; + } else if (isDefined(recordBeforePosition)) { + return recordBeforePosition + 1; + } else { + return 1; + } +}; diff --git a/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/factories/__tests__/record-position.factory.spec.ts b/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/factories/__tests__/record-position.factory.spec.ts index e556e1514d..179637d6ae 100644 --- a/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/factories/__tests__/record-position.factory.spec.ts +++ b/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/factories/__tests__/record-position.factory.spec.ts @@ -48,11 +48,11 @@ describe('RecordPositionFactory', () => { expect(result).toEqual(value); }); - it('should return the existing position / 2 when value is first', async () => { + it('should return the existing position -1 when value is first', async () => { const value = 'first'; const result = await factory.create(value, objectMetadata, workspaceId); - expect(result).toEqual(0.5); + expect(result).toEqual(0); }); it('should return the existing position + 1 when value is last', async () => { const value = 'last'; diff --git a/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/factories/record-position.factory.ts b/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/factories/record-position.factory.ts index 1548977144..8aaf879a13 100644 --- a/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/factories/record-position.factory.ts +++ b/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/factories/record-position.factory.ts @@ -1,5 +1,7 @@ import { Injectable } from '@nestjs/common'; +import { isDefined } from 'class-validator'; + import { WorkspaceDataSourceService } from 'src/engine/workspace-datasource/workspace-datasource.service'; import { RecordPositionQueryFactory, @@ -32,6 +34,8 @@ export class RecordPositionFactory { dataSourceSchema, ); + // If the value was 'first', the first record will be the one with the lowest position + // If the value was 'last', the first record will be the one with the highest position const records = await this.workspaceDataSourceService.executeRawQuery( query, [], @@ -39,10 +43,16 @@ export class RecordPositionFactory { undefined, ); - return ( - (value === 'first' - ? records[0]?.position / 2 - : records[0]?.position + 1) || 1 - ); + if ( + !isDefined(records) || + records.length === 0 || + !isDefined(records[0]?.position) + ) { + return 1; + } + + return value === 'first' + ? records[0].position - 1 + : records[0].position + 1; } } diff --git a/packages/twenty-server/src/engine/api/graphql/workspace-schema-builder/graphql-types/scalars/position.scalar.ts b/packages/twenty-server/src/engine/api/graphql/workspace-schema-builder/graphql-types/scalars/position.scalar.ts index a9619d098c..73622e8a27 100644 --- a/packages/twenty-server/src/engine/api/graphql/workspace-schema-builder/graphql-types/scalars/position.scalar.ts +++ b/packages/twenty-server/src/engine/api/graphql/workspace-schema-builder/graphql-types/scalars/position.scalar.ts @@ -6,7 +6,7 @@ const isValidStringPosition = (value: string): boolean => typeof value === 'string' && (value === 'first' || value === 'last'); const isValidNumberPosition = (value: number): boolean => - typeof value === 'number' && value >= 0; + typeof value === 'number'; const checkPosition = (value: any): PositionType => { if (isValidNumberPosition(value) || isValidStringPosition(value)) {