fix: sort SELECT fields by option position instead of alphabetical order

https://sonarly.com/issue/35648?type=bug SELECT/MULTI_SELECT fields are sorted alphabetically (using LOWER() + ::text cast) instead of by their defined option position, causing incorrect sort order when filters are applied. Fix: ## Summary Changed SELECT and MULTI_SELECT field sorting from alphabetical order to option position order. ## Root Cause Commit a8331dc43e introduced case-insensitive sorting using `LOWER(column::text)` for TEXT, SELECT, and MULTI_SELECT fields. While appropriate for TEXT fields, this caused SELECT/MULTI_SELECT fields to sort alphabetically by option value instead of by their defined position property. ## Fix 1. **`build-order-by-column-expression.util.ts`**: - Modified `shouldUseCaseInsensitiveOrder()` to return `false` for SELECT fields (TEXT only now) - Added `isSelectFieldType()` helper to identify SELECT/MULTI_SELECT fields - Added `getSelectOptionValuesSortedByPosition()` to extract option values sorted by position - Changed `shouldCastToText()` to return `false` (no longer needed) 2. **`order-by-condition.type.ts`**: - Added `selectOptionValues?: string[]` property to store option values sorted by position 3. **`graphql-query-order.parser.ts`**: - Updated scalar field parsing to extract and pass `selectOptionValues` for SELECT fields - Updated relation field parsing similarly for nested SELECT fields 4. **`graphql-query.parser.ts`**: - Updated `getOrderByRawSQL()` to generate `ARRAY_POSITION(ARRAY['opt1', 'opt2', ...], column)` SQL when `selectOptionValues` is present, instead of `LOWER(column::text)` 5. **`build-order-by-key.spec.ts`**: - Updated existing tests to reflect new behavior - Added tests for `isSelectFieldType()` and `getSelectOptionValuesSortedByPosition()` ## Example SQL Change Before: `ORDER BY LOWER("task"."priority"::text) ASC NULLS FIRST` After: `ORDER BY ARRAY_POSITION(ARRAY['URGENT', 'HIGH', 'MEDIUM', 'LOW'], "task"."priority") ASC NULLS FIRST` This ensures Priority field sorts by defined option position (Urgent → High → Medium → Low) rather than alphabetically (High → Low → Medium → Urgent).
2026-05-07 17:58:27 +00:00
parent 86eab9ac24
commit a2bff662a5
5 changed files with 123 additions and 19 deletions
@@ -2,6 +2,8 @@ import { FieldMetadataType } from 'twenty-shared/types';

 import {
  buildOrderByColumnExpression,
+  getSelectOptionValuesSortedByPosition,
+  isSelectFieldType,
  shouldCastToText,
  shouldUseCaseInsensitiveOrder,
 } from 'src/engine/api/graphql/graphql-query-runner/graphql-query-parsers/graphql-query-order/utils/build-order-by-column-expression.util';
@@ -33,13 +35,13 @@ describe('shouldUseCaseInsensitiveOrder', () => {
    expect(shouldUseCaseInsensitiveOrder(FieldMetadataType.TEXT)).toBe(true);
  });

-  it('should return true for SELECT fields', () => {
-    expect(shouldUseCaseInsensitiveOrder(FieldMetadataType.SELECT)).toBe(true);
+  it('should return false for SELECT fields (uses ARRAY_POSITION instead)', () => {
+    expect(shouldUseCaseInsensitiveOrder(FieldMetadataType.SELECT)).toBe(false);
  });

-  it('should return true for MULTI_SELECT fields', () => {
+  it('should return false for MULTI_SELECT fields (uses ARRAY_POSITION instead)', () => {
    expect(shouldUseCaseInsensitiveOrder(FieldMetadataType.MULTI_SELECT)).toBe(
-      true,
+      false,
    );
  });

@@ -64,13 +66,31 @@ describe('shouldUseCaseInsensitiveOrder', () => {
  });
 });

-describe('shouldCastToText', () => {
+describe('isSelectFieldType', () => {
  it('should return true for SELECT fields', () => {
-    expect(shouldCastToText(FieldMetadataType.SELECT)).toBe(true);
+    expect(isSelectFieldType(FieldMetadataType.SELECT)).toBe(true);
  });

  it('should return true for MULTI_SELECT fields', () => {
-    expect(shouldCastToText(FieldMetadataType.MULTI_SELECT)).toBe(true);
+    expect(isSelectFieldType(FieldMetadataType.MULTI_SELECT)).toBe(true);
+  });
+
+  it('should return false for TEXT fields', () => {
+    expect(isSelectFieldType(FieldMetadataType.TEXT)).toBe(false);
+  });
+
+  it('should return false for NUMBER fields', () => {
+    expect(isSelectFieldType(FieldMetadataType.NUMBER)).toBe(false);
+  });
+});
+
+describe('shouldCastToText', () => {
+  it('should return false for SELECT fields (uses ARRAY_POSITION instead)', () => {
+    expect(shouldCastToText(FieldMetadataType.SELECT)).toBe(false);
+  });
+
+  it('should return false for MULTI_SELECT fields (uses ARRAY_POSITION instead)', () => {
+    expect(shouldCastToText(FieldMetadataType.MULTI_SELECT)).toBe(false);
  });

  it('should return false for TEXT fields', () => {
@@ -85,3 +105,43 @@ describe('shouldCastToText', () => {
    expect(shouldCastToText(FieldMetadataType.DATE_TIME)).toBe(false);
  });
 });
+
+describe('getSelectOptionValuesSortedByPosition', () => {
+  it('should return undefined for null options', () => {
+    expect(getSelectOptionValuesSortedByPosition(null)).toBeUndefined();
+  });
+
+  it('should return undefined for undefined options', () => {
+    expect(getSelectOptionValuesSortedByPosition(undefined)).toBeUndefined();
+  });
+
+  it('should return undefined for empty options array', () => {
+    expect(getSelectOptionValuesSortedByPosition([])).toBeUndefined();
+  });
+
+  it('should return option values sorted by position', () => {
+    const options = [
+      { value: 'MEDIUM', label: 'Medium', position: 2, color: 'yellow' as const },
+      { value: 'URGENT', label: 'Urgent', position: 0, color: 'red' as const },
+      { value: 'HIGH', label: 'High', position: 1, color: 'orange' as const },
+      { value: 'LOW', label: 'Low', position: 3, color: 'green' as const },
+    ];
+
+    const result = getSelectOptionValuesSortedByPosition(options);
+
+    expect(result).toEqual(['URGENT', 'HIGH', 'MEDIUM', 'LOW']);
+  });
+
+  it('should handle options with same position', () => {
+    const options = [
+      { value: 'B', label: 'B', position: 1, color: 'blue' as const },
+      { value: 'A', label: 'A', position: 1, color: 'green' as const },
+    ];
+
+    const result = getSelectOptionValuesSortedByPosition(options);
+
+    expect(result).toHaveLength(2);
+    expect(result).toContain('A');
+    expect(result).toContain('B');
+  });
+});
@@ -1,4 +1,5 @@
 import { isObject } from 'class-validator';
+import { type FieldMetadataComplexOption } from 'twenty-shared/types';
 import { isDefined } from 'twenty-shared/utils';

 import { type ObjectRecordOrderBy } from 'src/engine/api/graphql/workspace-query-builder/interfaces/object-record.interface';
@@ -10,6 +11,8 @@ import {
 } from 'src/engine/api/graphql/graphql-query-runner/errors/graphql-query-runner.exception';
 import {
  buildOrderByColumnExpression,
+  getSelectOptionValuesSortedByPosition,
+  isSelectFieldType,
  shouldCastToText,
  shouldUseCaseInsensitiveOrder,
 } from 'src/engine/api/graphql/graphql-query-runner/graphql-query-parsers/graphql-query-order/utils/build-order-by-column-expression.util';
@@ -143,6 +146,12 @@ export class GraphqlQueryOrderFieldParser {
            fieldName,
          );

+          const selectOptionValues = isSelectFieldType(fieldMetadata.type)
+            ? getSelectOptionValuesSortedByPosition(
+                fieldMetadata.options as FieldMetadataComplexOption[],
+              )
+            : undefined;
+
          orderByConditions[columnExpression] = {
            ...convertOrderByToFindOptionsOrder(
              orderByDirection,
@@ -150,6 +159,7 @@ export class GraphqlQueryOrderFieldParser {
            ),
            useLower: shouldUseCaseInsensitiveOrder(fieldMetadata.type),
            castToText: shouldCastToText(fieldMetadata.type),
+            selectOptionValues,
          };
        }
      }
@@ -256,6 +266,12 @@ export class GraphqlQueryOrderFieldParser {
        nestedFieldMetadata.name,
      );

+      const selectOptionValues = isSelectFieldType(nestedFieldMetadata.type)
+        ? getSelectOptionValuesSortedByPosition(
+            nestedFieldMetadata.options as FieldMetadataComplexOption[],
+          )
+        : undefined;
+
      return {
        orderBy: {
          [columnExpression]: {
@@ -265,6 +281,7 @@ export class GraphqlQueryOrderFieldParser {
            ),
            useLower: shouldUseCaseInsensitiveOrder(nestedFieldMetadata.type),
            castToText: shouldCastToText(nestedFieldMetadata.type),
+            selectOptionValues,
          },
        },
        joinInfo,
@@ -3,4 +3,5 @@ export type OrderByClause = {
  nulls?: 'NULLS FIRST' | 'NULLS LAST';
  useLower?: boolean;
  castToText?: boolean; // For SELECT/MULTI_SELECT fields that need ::text before LOWER
+  selectOptionValues?: string[]; // Option values sorted by position for SELECT/MULTI_SELECT fields
 };
@@ -1,20 +1,35 @@
-import { FieldMetadataType } from 'twenty-shared/types';
+import {
+  type FieldMetadataComplexOption,
+  FieldMetadataType,
+} from 'twenty-shared/types';

 export const shouldUseCaseInsensitiveOrder = (
  fieldType: FieldMetadataType,
 ): boolean => {
+  return fieldType === FieldMetadataType.TEXT;
+};
+
+export const isSelectFieldType = (fieldType: FieldMetadataType): boolean => {
  return (
-    fieldType === FieldMetadataType.TEXT ||
    fieldType === FieldMetadataType.SELECT ||
    fieldType === FieldMetadataType.MULTI_SELECT
  );
 };

 export const shouldCastToText = (fieldType: FieldMetadataType): boolean => {
-  return (
-    fieldType === FieldMetadataType.SELECT ||
-    fieldType === FieldMetadataType.MULTI_SELECT
-  );
+  return false;
+};
+
+export const getSelectOptionValuesSortedByPosition = (
+  options: FieldMetadataComplexOption[] | null | undefined,
+): string[] | undefined => {
+  if (!options || options.length === 0) {
+    return undefined;
+  }
+
+  return [...options]
+    .sort((a, b) => (a.position ?? 0) - (b.position ?? 0))
+    .map((option) => option.value);
 };

 // Returns unquoted column expression for TypeORM's orderBy (e.g., "company.name")
@@ -193,14 +193,25 @@ export class GraphqlQueryParser {
            ? `"${parts[0]}"."${parts[1]}"`
            : `"${orderByField}"`;

-        // Build column expression with optional ::text cast and LOWER()
+        // Build column expression with ARRAY_POSITION for SELECT fields, or optional ::text cast and LOWER()
        let columnExpr = quotedColumn;

-        if (orderByCondition.castToText) {
-          columnExpr = `${columnExpr}::text`;
-        }
-        if (orderByCondition.useLower) {
-          columnExpr = `LOWER(${columnExpr})`;
+        if (
+          orderByCondition.selectOptionValues &&
+          orderByCondition.selectOptionValues.length > 0
+        ) {
+          // Use ARRAY_POSITION for SELECT/MULTI_SELECT fields to sort by option position
+          const optionsArray = orderByCondition.selectOptionValues
+            .map((value) => `'${value.replace(/'/g, "''")}'`)
+            .join(', ');
+          columnExpr = `ARRAY_POSITION(ARRAY[${optionsArray}], ${quotedColumn})`;
+        } else {
+          if (orderByCondition.castToText) {
+            columnExpr = `${columnExpr}::text`;
+          }
+          if (orderByCondition.useLower) {
+            columnExpr = `LOWER(${columnExpr})`;
+          }
        }

        return `${columnExpr} ${orderByCondition.order}${nullsCondition}`;