Make positions possibly negatives (#5690)

Closes #5427

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,

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);
+  });
+});

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;
+  }
+};

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';

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,17 +34,25 @@ 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,
       [],
       workspaceId,
       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;
   }
 }

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)) {