fix: ensure interface-provided 'orderBy' parameter overrides view's d…

…efault sort
teableio · Mar 18, 2024 · ed5837a · ed5837a
1 parent aa7b944
commit ed5837a
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 4 deletions.
diff --git a/apps/nestjs-backend/test/sort.e2e-spec.ts b/apps/nestjs-backend/test/sort.e2e-spec.ts
@@ -2,7 +2,7 @@
 /* eslint-disable @typescript-eslint/no-explicit-any */
 import type { INestApplication } from '@nestjs/common';
 import type { IFieldRo, IGetRecordsRo, ISortItem, ITableFullVo } from '@teable/core';
-import { FieldType, CellValueType, SortFunc } from '@teable/core';
+import { CellValueType, SortFunc, FieldType } from '@teable/core';
 import { updateViewSort as apiSetViewSort } from '@teable/openapi';
 import { isEmpty, orderBy } from 'lodash';
 import type { SingleSelectOptionsDto } from '../src/features/field/model/field-dto/single-select-field.dto';
@@ -202,6 +202,33 @@ describe('OpenAPI Sort (e2e) Base CellValueType', () => {
     expect(ascOriginRecords).toEqual(ascManualSortRecords);
     expect(descOriginRecords).toEqual(descManualSortRecords);
   });
+
+  test('view sort property should be merged after by interface parameter orderBy', async () => {
+    const { id: subTableId, fields: fields2, defaultViewId } = table;
+    const field = fields2.find(
+      (field) => field.type === FieldType.Number
+    ) as ITableFullVo['fields'][number];
+    const { id: fieldId } = field;
+
+    const booleanField = fields2.find((field) => field.type === FieldType.Checkbox);
+    const { id: booleanFieldId } = booleanField!;
+
+    const ascOrders: IGetRecordsRo['orderBy'] = [{ fieldId, order: SortFunc.Asc }];
+    const descOrders: IGetRecordsRo['orderBy'] = [
+      { fieldId: booleanFieldId, order: SortFunc.Desc },
+    ];
+    await setRecordsOrder(subTableId, defaultViewId!, ascOrders);
+    const originRecords = await getSortRecords(subTableId, {
+      viewId: defaultViewId,
+      orderBy: descOrders,
+    });
+    const manualSortRecords = getRecordsByOrder(
+      originRecords,
+      [...descOrders, ...ascOrders],
+      fields2
+    );
+    expect(originRecords).toEqual(manualSortRecords);
+  });
 });
 
 describe('OpenAPI Sort (e2e) Multiple CellValueType', () => {

diff --git a/packages/core/src/models/view/sort/sort.ts b/packages/core/src/models/view/sort/sort.ts
@@ -59,12 +59,13 @@ export function mergeWithDefaultSort(
     return [];
   }
 
-  let mergeSort = viewSort?.sortObjs || [];
+  let mergeSort = querySort || [];
+  const sortObjs = viewSort?.sortObjs || [];
 
-  if (querySort?.length) {
+  if (sortObjs?.length) {
     // merge the same fieldId item, query first
     const map = new Map(mergeSort.map((sortItem) => [sortItem.fieldId, sortItem]));
-    querySort.forEach((sortItem) => map.set(sortItem.fieldId, sortItem));
+    sortObjs.forEach((sortItem) => map.set(sortItem.fieldId, sortItem));
     mergeSort = Array.from(map.values());
   }