From 6c91a0eb93ba62ae69d3013634930b52934baf8f Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 16 Aug 2026 19:09:38 +0000 Subject: [PATCH] =?UTF-8?q?fix(gantt,map,calendar):=20=E4=B8=89=E4=BB=BD?= =?UTF-8?q?=E7=A7=81=E6=9C=89=20sort=E2=86=92$orderby=20=E5=89=AF=E6=9C=AC?= =?UTF-8?q?=E6=94=B6=E6=95=9B=E5=88=B0=20core=20=E5=85=B1=E4=BA=AB=20sink?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit gantt / map / calendar 各自内联了一份逐字相同的 convertSortToQueryParams。 该副本要求数组项同时带 field 与 order,缺 order 的项被静默丢弃 —— 存视图里 sort: [{ field: 'amount' }] 于是整条排序都到不了 wire。同一份副本对字符串 写法("amount")却已按升序处理,所以这是两种写法之间的不一致。 三处改为从 @object-ui/core 导入共享 sink(objectstack#7137 引入,timeline 与 record:line_items 已在用),私有副本删除 —— 全仓仅剩 core 一处声明。 随迁移落地两处行为变化(均是更贴合已声明契约,不是新增容忍): - 缺 order 的数组项按升序排,不再消失($orderby 自己的成员形状即 { field: string; order?: 'asc' | 'desc' })。 - 无可排序内容时不再下发 {},而是不带 $orderby。 每个消费点补「不带 order 的项不丢」钉子;calendar 原先的 expect($orderby).toBeTruthy() 收紧为逐值断言 —— 空对象同样 truthy,正是旧 副本丢光所有项时的产物。core 里被本 PR 证伪的注释(「三份副本仍在」)一并 更正,sink 逻辑未动。 Fixes #4022 Co-authored-by: Claude --- .../gantt-map-calendar-shared-sort-sink.md | 37 +++++++++++++++ packages/core/src/utils/sort-query.ts | 22 +++++---- .../ObjectCalendar.elementDataSource.test.tsx | 31 ++++++++++++- .../plugin-calendar/src/ObjectCalendar.tsx | 34 +++----------- .../ObjectGantt.elementDataSource.test.tsx | 46 +++++++++++++++++++ packages/plugin-gantt/src/ObjectGantt.tsx | 35 +++----------- .../src/ObjectMap.elementDataSource.test.tsx | 26 +++++++++++ packages/plugin-map/src/ObjectMap.tsx | 29 +----------- 8 files changed, 165 insertions(+), 95 deletions(-) create mode 100644 .changeset/gantt-map-calendar-shared-sort-sink.md diff --git a/.changeset/gantt-map-calendar-shared-sort-sink.md b/.changeset/gantt-map-calendar-shared-sort-sink.md new file mode 100644 index 0000000000..e495ae23aa --- /dev/null +++ b/.changeset/gantt-map-calendar-shared-sort-sink.md @@ -0,0 +1,37 @@ +--- +'@object-ui/plugin-gantt': patch +'@object-ui/plugin-map': patch +'@object-ui/plugin-calendar': patch +--- + +`object-gantt` / `object-map` / `object-calendar` no longer drop a sort entry that omits `order`. + +The three blocks each inlined a byte-identical private copy of the `sort` → +`$orderby` conversion. That copy required BOTH `field` and `order` on an array +entry and silently skipped any entry missing one, so a stored view sorting by +`[{ field: 'amount' }]` reached the wire with no ordering at all — the authored +sort key was lost, not applied. The same copy already treated the STRING +spelling `"amount"` as ascending, so this was an inconsistency between two +spellings of one thing rather than deliberate strictness. + +All three now import the shared `convertSortToQueryParams` sink from +`@object-ui/core` (introduced by objectstack#7137, already used by +`object-timeline` and `record:line_items`), and the private copies are gone — +the sink is the repo's only definition. Two behavior changes come with it, both +of which make the blocks more faithful to what is already declared rather than +more tolerant: + +- An array entry that omits `order` now orders ASCENDING instead of vanishing. + That is what `QueryParams.$orderby`'s own member shape + (`{ field: string; order?: 'asc' | 'desc' }`) says, and what + `@object-ui/data-objectstack`'s `serializeOrderBy` already did with a missing + direction. +- When nothing orderable was authored, the query now carries no `$orderby` at + all instead of an empty object. `{}` is truthy and meant "no ordering" only by + accident of the adapter's serializer. + +Reachability, so the size of this is not overstated: `SortConfig.order` and +`ElementDataSourceSort.order` are REQUIRED in objectui's own types, so a typed +caller could never author the dropped shape. The affected surface is untyped +stored view metadata (`ElementSavedView` is a loose record by design) — which is +exactly where an order-less entry can arrive today. diff --git a/packages/core/src/utils/sort-query.ts b/packages/core/src/utils/sort-query.ts index 53ac1b0ba8..2960bbdfda 100644 --- a/packages/core/src/utils/sort-query.ts +++ b/packages/core/src/utils/sort-query.ts @@ -17,21 +17,23 @@ * accepts four shapes; normalizing here means one shape reaches the wire and a * block does not have to know which spelling its author used. * - * This is the same conversion three sibling blocks already inline privately - * (`ObjectGantt` / `ObjectMap` / `ObjectCalendar`, byte-identical copies). It is - * hoisted here because objectstack#7137 added two more read sites - * (`object-timeline`, `record:line_items`), and a fifth and sixth private copy is - * how the conversions start disagreeing. Collapsing the three existing copies - * onto this one is deliberately NOT part of #7137 — it is tracked as - * objectstack#7148. + * This is now the ONLY definition in the repo. It was hoisted here because + * objectstack#7137 added two more read sites (`object-timeline`, + * `record:line_items`) next to three sibling blocks that each inlined a + * byte-identical private copy (`ObjectGantt` / `ObjectMap` / `ObjectCalendar`), + * and a fifth and sixth copy is how the conversions start disagreeing. Those + * three copies were deliberately left alone by #7137 and collapsed onto this + * function by objectui#4022; every block now imports it from here. * - * Two deliberate differences from those private copies, both of which make this - * function more faithful to the declared contract rather than adding tolerance: + * Two deliberate differences from those retired private copies, both of which + * make this function more faithful to the declared contract rather than adding + * tolerance — and both are BEHAVIOUR CHANGES the migration delivered, not pure + * refactor: * * - **`order` is optional in `SortConfig`**, so an entry that omits it means * ascending (that is what `$orderby`'s own * `Array<{ field: string; order?: 'asc' | 'desc' }>` shape says). The private - * copies require BOTH keys and silently drop such an entry, which loses an + * copies required BOTH keys and silently dropped such an entry, which lost an * authored sort key instead of ordering by it. * - **Nothing usable yields `undefined`, never `{}`.** An empty object is a * truthy value that means "no ordering" only by accident of the adapter's diff --git a/packages/plugin-calendar/src/ObjectCalendar.elementDataSource.test.tsx b/packages/plugin-calendar/src/ObjectCalendar.elementDataSource.test.tsx index a110c2122a..82f6e5a054 100644 --- a/packages/plugin-calendar/src/ObjectCalendar.elementDataSource.test.tsx +++ b/packages/plugin-calendar/src/ObjectCalendar.elementDataSource.test.tsx @@ -33,6 +33,17 @@ const HOT_VIEW = { pagination: { pageSize: 7 }, }; +/** + * A sort entry that omits `order` — reachable only through UNTYPED saved-view + * metadata (`ElementSavedView` is `Record< string, unknown >`; `SortConfig.order` + * is required in objectui's own types). See objectui#4022. + */ +const PARTIAL_SORT_VIEW = { + name: 'partial', + label: 'Partially ordered', + sort: [{ field: 'starts_at' }, { field: 'name', order: 'desc' }], +}; + const CALENDAR = { startDateField: 'starts_at', endDateField: 'ends_at', titleField: 'name' }; function makeAdapter(listViews: Record = { hot: HOT_VIEW }) { @@ -74,7 +85,10 @@ describe('object-calendar — dataSource: { object, view } (objectstack#6953)', const [object, params] = adapter.find.mock.calls[0] as [string, any]; expect(object).toBe('account'); expect(params.$filter).toEqual([['rating', '=', 'hot']]); - expect(params.$orderby).toBeTruthy(); + // Was `toBeTruthy()`, which an EMPTY object also satisfies — the one value the + // old private copy produced when it had dropped every entry. Pinned to the + // actual map so this case can still fail for the right reason (objectui#4022). + expect(params.$orderby).toEqual({ name: 'desc' }); }); it('reports an unresolvable `view` instead of fetching the whole object', async () => { @@ -107,4 +121,19 @@ describe('object-calendar — dataSource: { object, view } (objectstack#6953)', expect(object).toBe('account'); expect(params.$filter).toEqual([['owner', '=', 'me']]); }); + + it('orders by a sort entry that omits `order` instead of dropping it', async () => { + const adapter = makeAdapter({ partial: PARTIAL_SORT_VIEW }); + renderBlock( + { type: 'object-calendar', calendar: CALENDAR, dataSource: { object: 'account', view: 'partial' } }, + adapter, + ); + + await waitFor(() => expect(adapter.find).toHaveBeenCalled()); + const [, params] = adapter.find.mock.calls[0] as [string, any]; + // The private copy required BOTH keys and silently dropped `starts_at`, + // sending `{ name: 'desc' }`. The shared sink reads a missing `order` as + // ascending — what `QueryParams.$orderby`'s member shape declares. + expect(params.$orderby).toEqual({ starts_at: 'asc', name: 'desc' }); + }); }); diff --git a/packages/plugin-calendar/src/ObjectCalendar.tsx b/packages/plugin-calendar/src/ObjectCalendar.tsx index 7425628c4c..a139ff01b3 100644 --- a/packages/plugin-calendar/src/ObjectCalendar.tsx +++ b/packages/plugin-calendar/src/ObjectCalendar.tsx @@ -46,7 +46,12 @@ import { Label, toast, } from '@object-ui/components'; -import { extractRecords, buildExpandFields, getRecordDisplayName } from '@object-ui/core'; +import { + extractRecords, + buildExpandFields, + convertSortToQueryParams, + getRecordDisplayName, +} from '@object-ui/core'; export interface CalendarSchema { type: 'calendar'; @@ -121,33 +126,6 @@ function getDataConfig(schema: ObjectGridSchema | CalendarSchema): ViewData | nu return null; } -/** - * Helper to convert sort config to QueryParams format - */ -function convertSortToQueryParams(sort: string | any[] | undefined): Record | undefined { - if (!sort) return undefined; - - // If it's a string like "name desc" - if (typeof sort === 'string') { - const parts = sort.split(' '); - const field = parts[0]; - const order = (parts[1]?.toLowerCase() === 'desc' ? 'desc' : 'asc') as 'asc' | 'desc'; - return { [field]: order }; - } - - // If it's an array of SortConfig objects - if (Array.isArray(sort)) { - return sort.reduce((acc, item) => { - if (item.field && item.order) { - acc[item.field] = item.order; - } - return acc; - }, {} as Record); - } - - return undefined; -} - /** * Helper to get calendar configuration from schema */ diff --git a/packages/plugin-gantt/src/ObjectGantt.elementDataSource.test.tsx b/packages/plugin-gantt/src/ObjectGantt.elementDataSource.test.tsx index a6cb7bd65d..802aebabba 100644 --- a/packages/plugin-gantt/src/ObjectGantt.elementDataSource.test.tsx +++ b/packages/plugin-gantt/src/ObjectGantt.elementDataSource.test.tsx @@ -49,6 +49,19 @@ const HOT_VIEW = { pagination: { pageSize: 7 }, }; +/** + * A sort entry that omits `order` is only reachable through UNTYPED saved-view + * metadata: `SortConfig.order` / `ElementDataSourceSort.order` are REQUIRED in + * objectui's own types, so a typed caller cannot spell this, while + * `ElementSavedView` is `Record< string, unknown >` by design. That is the + * authoring surface this fixture stands in for (objectui#4022). + */ +const PARTIAL_SORT_VIEW = { + name: 'partial', + label: 'Partially ordered', + sort: [{ field: 'end_date' }, { field: 'name', order: 'desc' }], +}; + const GANTT = { startDateField: 'start_date', endDateField: 'end_date', titleField: 'name' }; function makeAdapter(listViews: Record = { hot: HOT_VIEW }) { @@ -163,4 +176,37 @@ describe('object-gantt — dataSource: { object, view } (objectstack#7121)', () expect(params.$filter).toEqual([['owner', '=', 'me']]); expect(params.$orderby).toEqual({ end_date: 'asc' }); }); + + it('orders by a sort entry that omits `order` instead of dropping it', async () => { + const adapter = makeAdapter({ partial: PARTIAL_SORT_VIEW }); + renderBlock( + { type: 'object-gantt', gantt: GANTT, dataSource: { object: 'task', view: 'partial' } }, + adapter, + ); + + await waitFor(() => expect(adapter.find).toHaveBeenCalled()); + const [, params] = adapter.find.mock.calls[0] as [string, any]; + // The private copy this block used to inline required BOTH keys and silently + // dropped `end_date`, sending `{ name: 'desc' }` — an authored sort key lost + // on the way to the wire. The shared sink reads a missing `order` as + // ascending, which is what `QueryParams.$orderby`'s own member shape + // (`{ field: string; order?: 'asc' | 'desc' }`) declares, and what the string + // spelling (`"end_date"`) already meant in the very same copy. + expect(params.$orderby).toEqual({ end_date: 'asc', name: 'desc' }); + }); + + it('sends NO $orderby when nothing orderable was authored', async () => { + const adapter = makeAdapter(); + renderBlock( + { type: 'object-gantt', objectName: 'task', gantt: GANTT, sort: [] }, + adapter, + ); + + await waitFor(() => expect(adapter.find).toHaveBeenCalled()); + const [, params] = adapter.find.mock.calls[0] as [string, any]; + // The private copy reduced an empty array to `{}` — a TRUTHY value that only + // means "no ordering" by accident of the adapter serializer. `undefined` says + // it outright, so the query simply carries no `$orderby`. + expect(params.$orderby).toBeUndefined(); + }); }); diff --git a/packages/plugin-gantt/src/ObjectGantt.tsx b/packages/plugin-gantt/src/ObjectGantt.tsx index efc9f363fc..80174e851a 100644 --- a/packages/plugin-gantt/src/ObjectGantt.tsx +++ b/packages/plugin-gantt/src/ObjectGantt.tsx @@ -40,7 +40,13 @@ import { AlertDialogTitle, cn, } from '@object-ui/components'; -import { extractRecords, buildExpandFields, getRecordDisplayName, resolveDataSource } from '@object-ui/core'; +import { + extractRecords, + buildExpandFields, + convertSortToQueryParams, + getRecordDisplayName, + resolveDataSource, +} from '@object-ui/core'; import { getSemanticColorName, getSemanticHex, @@ -309,33 +315,6 @@ function getDataConfig(schema: ObjectGridSchema): ViewData | null { return null; } -/** - * Helper to convert sort config to QueryParams format - */ -function convertSortToQueryParams(sort: string | any[] | undefined): Record | undefined { - if (!sort) return undefined; - - // If it's a string like "name desc" - if (typeof sort === 'string') { - const parts = sort.split(' '); - const field = parts[0]; - const order = (parts[1]?.toLowerCase() === 'desc' ? 'desc' : 'asc') as 'asc' | 'desc'; - return { [field]: order }; - } - - // If it's an array of SortConfig objects - if (Array.isArray(sort)) { - return sort.reduce((acc, item) => { - if (item.field && item.order) { - acc[item.field] = item.order; - } - return acc; - }, {} as Record); - } - - return undefined; -} - /** * Pull a human-readable message out of a failed write. ApiDataSource embeds * the raw response body at the end of its Error message diff --git a/packages/plugin-map/src/ObjectMap.elementDataSource.test.tsx b/packages/plugin-map/src/ObjectMap.elementDataSource.test.tsx index 2a3f3d2bee..a079578c6f 100644 --- a/packages/plugin-map/src/ObjectMap.elementDataSource.test.tsx +++ b/packages/plugin-map/src/ObjectMap.elementDataSource.test.tsx @@ -44,6 +44,17 @@ const HOT_VIEW = { pagination: { pageSize: 7 }, }; +/** + * A sort entry that omits `order` — reachable only through UNTYPED saved-view + * metadata (`ElementSavedView` is `Record< string, unknown >`; `SortConfig.order` + * is required in objectui's own types). See objectui#4022. + */ +const PARTIAL_SORT_VIEW = { + name: 'partial', + label: 'Partially ordered', + sort: [{ field: 'rating' }, { field: 'name', order: 'desc' }], +}; + const MAP = { latitudeField: 'lat', longitudeField: 'lng', titleField: 'name' }; function makeAdapter(listViews: Record = { hot: HOT_VIEW }) { @@ -149,4 +160,19 @@ describe('object-map — dataSource: { object, view } (objectstack#7121)', () => expect(params.$filter).toEqual([['owner', '=', 'me']]); expect(params.$orderby).toEqual({ name: 'asc' }); }); + + it('orders by a sort entry that omits `order` instead of dropping it', async () => { + const adapter = makeAdapter({ partial: PARTIAL_SORT_VIEW }); + renderBlock( + { type: 'object-map', map: MAP, dataSource: { object: 'store', view: 'partial' } }, + adapter, + ); + + await waitFor(() => expect(adapter.find).toHaveBeenCalled()); + const [, params] = adapter.find.mock.calls[0] as [string, any]; + // The private copy required BOTH keys and silently dropped `rating`, sending + // `{ name: 'desc' }`. The shared sink reads a missing `order` as ascending — + // what `QueryParams.$orderby`'s member shape declares. + expect(params.$orderby).toEqual({ rating: 'asc', name: 'desc' }); + }); }); diff --git a/packages/plugin-map/src/ObjectMap.tsx b/packages/plugin-map/src/ObjectMap.tsx index d5ae9269d8..613c444fb3 100644 --- a/packages/plugin-map/src/ObjectMap.tsx +++ b/packages/plugin-map/src/ObjectMap.tsx @@ -24,7 +24,7 @@ import React, { useEffect, useState, useMemo, useRef, useCallback } from 'react' import type { ObjectGridSchema, DataSource, ViewData } from '@object-ui/types'; import { useNavigationOverlay } from '@object-ui/react'; import { NavigationOverlay, cn, useIsMobile } from '@object-ui/components'; -import { extractRecords, buildExpandFields } from '@object-ui/core'; +import { extractRecords, buildExpandFields, convertSortToQueryParams } from '@object-ui/core'; import { z } from 'zod'; import MapGL, { NavigationControl, Marker, Popup } from 'react-map-gl/maplibre'; import type { MapRef } from 'react-map-gl/maplibre'; @@ -107,33 +107,6 @@ function getDataConfig(schema: ObjectGridSchema): ViewData | null { return null; } -/** - * Helper to convert sort config to QueryParams format - */ -function convertSortToQueryParams(sort: string | any[] | undefined): Record | undefined { - if (!sort) return undefined; - - // If it's a string like "name desc" - if (typeof sort === 'string') { - const parts = sort.split(' '); - const field = parts[0]; - const order = (parts[1]?.toLowerCase() === 'desc' ? 'desc' : 'asc') as 'asc' | 'desc'; - return { [field]: order }; - } - - // If it's an array of SortConfig objects - if (Array.isArray(sort)) { - return sort.reduce((acc, item) => { - if (item.field && item.order) { - acc[item.field] = item.order; - } - return acc; - }, {} as Record); - } - - return undefined; -} - const isDev = (): boolean => (globalThis as { process?: { env?: Record } }).process?.env ?.NODE_ENV !== 'production';