Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions .changeset/gantt-map-calendar-shared-sort-sink.md
Original file line number Diff line number Diff line change
@@ -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.
22 changes: 12 additions & 10 deletions packages/core/src/utils/sort-query.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, unknown> = { hot: HOT_VIEW }) {
Expand Down Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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' });
});
});
34 changes: 6 additions & 28 deletions packages/plugin-calendar/src/ObjectCalendar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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<string, 'asc' | 'desc'> | 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<string, 'asc' | 'desc'>);
}

return undefined;
}

/**
* Helper to get calendar configuration from schema
*/
Expand Down
46 changes: 46 additions & 0 deletions packages/plugin-gantt/src/ObjectGantt.elementDataSource.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, unknown> = { hot: HOT_VIEW }) {
Expand Down Expand Up @@ -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();
});
});
35 changes: 7 additions & 28 deletions packages/plugin-gantt/src/ObjectGantt.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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<string, 'asc' | 'desc'> | 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<string, 'asc' | 'desc'>);
}

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
Expand Down
26 changes: 26 additions & 0 deletions packages/plugin-map/src/ObjectMap.elementDataSource.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, unknown> = { hot: HOT_VIEW }) {
Expand Down Expand Up @@ -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' });
});
});
29 changes: 1 addition & 28 deletions packages/plugin-map/src/ObjectMap.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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<string, 'asc' | 'desc'> | 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<string, 'asc' | 'desc'>);
}

return undefined;
}

const isDev = (): boolean =>
(globalThis as { process?: { env?: Record<string, string | undefined> } }).process?.env
?.NODE_ENV !== 'production';
Expand Down
Loading