From fbd8714c768ef4b75a2aca56238ca81af7273171 Mon Sep 17 00:00:00 2001 From: Thomas Trompette Date: Fri, 31 May 2024 14:17:49 +0200 Subject: [PATCH] Make positions possibly negatives (#5690) Closes https://github.com/twentyhq/twenty/issues/5427 --- .../record-board/components/RecordBoard.tsx | 11 ++++---- .../get-dragged-record-position.util.test.ts | 27 +++++++++++++++++++ .../utils/get-dragged-record-position.util.ts | 16 +++++++++++ .../__tests__/record-position.factory.spec.ts | 4 +-- .../factories/record-position.factory.ts | 20 ++++++++++---- .../graphql-types/scalars/position.scalar.ts | 2 +- 6 files changed, 66 insertions(+), 14 deletions(-) create mode 100644 packages/twenty-front/src/modules/object-record/record-board/utils/__tests__/get-dragged-record-position.util.test.ts create mode 100644 packages/twenty-front/src/modules/object-record/record-board/utils/get-dragged-record-position.util.ts 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 d1e011e5904c..23aa8612882b 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 000000000000..1359c2d597be --- /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 000000000000..1a694c4e6ed6 --- /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 e556e1514d34..179637d6aebc 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 1548977144e7..8aaf879a1386 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 a9619d098c39..73622e8a2749 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)) {