Skip to content

fix(plugin-grid): order a grouped grid's groups by the level's orderBy - #25977

Merged
objectstack-fleet[bot] merged 5 commits into
mainfrom
claude/project-thread-6wnpcu
Oct 26, 2026
Merged

objectstack-fleet[bot] merged 5 commits into
mainfrom
claude/project-thread-6wnpcu

Conversation

@objectstack-fleet

@objectstack-fleet objectstack-fleet Bot commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Requested by Steve · project thread

Fixes #25858

Clause-②: no
Metadata-protocol: none

What changes

A grouped grid now orders a level's groups by the level's grouping.fields[].orderBy (the spec key landed in #25828), while the group label still comes from field. A list grouped by period_label with orderBy: 'period_start', order: 'desc' renders one header per label in descending period_start order, the undated group last.

Measured on origin/main before the change, the key was dropped in three places, not one:

  1. ObjectGrid.tsx (serverGroupingKey): the server grouping entries were rebuilt as field, order and collapsed only, so orderBy never left the grid.
  2. useServerGrouping.ts (groupingOf): the header query was compiled from field names alone, so compileListViewGroupQuery never added the min_FIELD / max_FIELD node and the header rows never carried an order value.
  3. useGroupedData.ts (compareGroupEntries): no orderBy handling on either sort path.

The card named (3) and allowed (1) and (2) "if the header column is not forwarded"; it was not, so all three are fixed here.

  • Server path. ObjectGrid forwards orderBy; useServerGroupHeaders compiles each level with its orderBy and order (a level ordered by its own key still carries the name alone, so flipping its order re-asks nothing; a changed orderBy re-asks, since it changes the answer's columns). useGroupedData reads each level's order value off the column listViewGroupOrderColumn(grouping, depth) names, never a re-derived alias.
  • Client path (rows handed in whole). Each group's order value is the min (asc) or max (desc) of orderBy over its rows, folded once before the sort.
  • One comparator for both. compareGroupEntries orders by the order value first, in order; a null value sorts last in both directions; equal values fall back to the existing key comparison (declared option order, then label) in the same direction. A level without orderBy, or with orderBy naming the grouping field, is unchanged. Values are reduced with @object-ui/core's getSortValue / compareSortValues; a numeric string (a driver's decimal) is read as the number it spells, so 3.10 does not sort after 3.9.
  • Read gate. An orderBy the principal may not read (groupingKeyReadable, the gate the grouping key already passes) is not forwarded, and that level orders by its own key: the header row would otherwise carry the hidden field's min/max and the group order would disclose it.
  • Type mirror (card item 4). None to change: UsableGroupingField derives from @objectstack/spec/ui's GroupingConfig, and the @object-ui/types zod faces take GroupingConfigSchema by reference, so orderBy was already on both.

scripts/sdui-manifest.record.json is re-recorded because check-sdui-manifest hashes the ui/packages registry sources; sdui.manifest.json itself is byte-identical (106 components, sha256 1f6e1ccb925a…).

Tests

New ui/packages/plugin-grid/src/__tests__/groupOrderBy-25858.test.tsx, 18 tests:

  • the card's acceptance set (Q4 2025, Sep 2026, Oct 2026, Q4 2026, Nov 2026, Q1 2027, plus an undated period) on header rows produced by answering compileListViewGroupQuery's own output, both directions, the undated group last in both; an outer level of a two-level grouping; counts unchanged;
  • the client path on the same rows, plus min-for-asc / max-for-desc over a group whose rows span values, and numeric-string values;
  • precedence over a declared option order, with ties falling back to it in the same direction;
  • through ObjectGrid: the header query carries max_period_start and the painted headers read in descending start; rows handed in order the same way;
  • controls: no orderBy (label order, option order, and a header query whose only aggregation is count), and orderBy naming the grouping field itself.

Runs (code tree b3b58b69; the final head 19f3f573 adds only the sdui record file):

  • pnpm exec vitest run packages/plugin-grid/src/__tests__/groupOrderBy-25858.test.tsx (from ui/): Test Files 1 passed, Tests 18 passed.
  • pnpm exec vitest run packages/plugin-grid/ (from ui/): Test Files 248 passed (248), Tests 2222 passed (2222).
  • pnpm --filter @object-ui/plugin-grid run type-check (tsc --noEmit && tsc -p tsconfig.test.json): exit 0.

Ablations, each through scripts/ablation-replace.mjs (anchor hit 1, restored blob equal to HEAD, git diff HEAD empty), none left in the tree:

  • the order-value arm of compareGroupEntries disabled: 10 failed / 8 passed (every orderBy assertion red, every control green);
  • the numeric-string reading disabled: 1 failed (the numeric-string test);
  • headerLevelOf forwarding no orderBy: 1 failed (the ObjectGrid server-path test);
  • ObjectGrid forwarding no orderBy: 1 failed (the same test).

Gates

node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack at 19f3f573 derives 47 families; all 47 were run and exited 0, and --ran with per-command exit codes answers "47 run, 0 NOT-MEASURED (a DERIVED zero)". check-sdui-manifest was red before the record was regenerated (node scripts/gen-sdui-manifest-node.mjs) and is green after. The type-check lanes and the CI-owned whole-tree families are left to this PR's checks.

Acceptance notes

  • The client path is live only for rows handed in whole (inline rows or a value provider): a grid or ListView that fetches its own rows over a source with no header query refuses grouping (objectui#10881), so no projection change is needed for orderBy there; such hosts' rows already carry every field.
  • ⛔ Not this PR: the HotCRM forecast.view.ts edit. Its follow-up is to drop the outer period_start level once this lands.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Baqv5mDLFH63hvh9G4YGn5


Generated by Claude Code

A level naming grouping.fields[].orderBy now orders its groups by that
field: the server path forwards orderBy into the header query and reads
the min/max column listViewGroupOrderColumn names; the client path folds
min (asc) / max (desc) from each group's rows. Nulls sort last in both
directions; ties fall back to the key comparison in the same direction.

Claude-Session: https://claude.ai/code/session_01Baqv5mDLFH63hvh9G4YGn5
Co-authored-by: Claude <noreply@anthropic.com>
… edit

sdui.manifest.json is byte-identical; only the recorded source hash moves.

Claude-Session: https://claude.ai/code/session_01Baqv5mDLFH63hvh9G4YGn5
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added size/l documentation Improvements or additions to documentation tooling labels Oct 26, 2026
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

席位核对 · ACCEPT · seat domain:ui#1 · 2026-10-26T08:03Z

Head 19f3f573d2036c5c9c0353317a46095a1f5ab25f (read from the PR object, not the report).

  • Shape. Draft on main; Fixes #25858 on the first body line after the attribution block; Clause-②: no, Metadata-protocol: none; changeset .changeset/25858-grid-group-order-by.md in the root .changeset/ (patch, @object-ui/plugin-grid).
  • Checks pinned to the head. 61 named check runs, latest per name: 52 success, 9 skipped, 0 failing, 0 pending. Lint & Repo Gates success, TypeScript Type Check success, Test UI 16/16, Type Check UI, Build & E2E UI all success.
  • Closing-Target Claim Guard. The first run failed on the seat's claim wording, not on this PR: the claim's Branch: line carried an issue-shaped placeholder that CLAIM_BRANCH_SHAPE parsed as a second branch. The seat reworded the claim (comment 6409882318) and re-ran the guard through a PR-body edit; the newer run is success.
  • Governed surface. check-governed-merges.mjs --pr 25977: 0 of 6 paths governed, +520/-22, ordinary queue landing.
  • Out-of-surface file. scripts/sdui-manifest.record.json moves registrySourcesSha256 only. Re-ran check-sdui-manifest.mjs on the head: 922 sources, sha256 7734fe6bc376…, matching the record. It is the derived hash the gate requires when ui registry sources change, so it is a bounded in-place change.
  • Changeset against the diff. Server path forwards a readable orderBy (with order) into the header query and reads the order value under listViewGroupOrderColumn; client path folds min/max of orderBy over a group's rows; null order values sort last both ways; ties fall back to the existing key comparison; orderBy absent or equal to field is unchanged. Each sentence matches the code.
  • Spot checks. An unreadable orderBy is dropped from the header query key, and the client then reads undefined → null for every group and falls back to key order (no disclosure, no throw). listViewGroupOrderColumn is only called over a non-empty answered header set, and a whitespace orderBy is refused by the view schema before it gets here.

Landing: ready + auto-merge through the relay.

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 26, 2026 08:05
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge October 26, 2026 08:05
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 26, 2026
Merged via the queue into main with commit 62ba5ef Oct 26, 2026
68 of 69 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/project-thread-6wnpcu branch October 26, 2026 08:35
os-steve pushed a commit that referenced this pull request Oct 26, 2026
Resolve the scripts/sdui-manifest.record.json conflict with #25977 by
regenerating the record with node scripts/gen-sdui-manifest-node.mjs.

Claude-Session: https://claude.ai/code/session_01C6T942afhqAP5w9XBmfTDC
Co-authored-by: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/l tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

plugin-grid: order a grouped grid's groups by the level's orderBy (spec grouping.fields[].orderBy), the label still from field

2 participants