fix(plugin-grid): select-all-matching replays the host's real query — or abstains — instead of fanning out unfiltered (#4501) - #4510
Merged
Conversation
… or abstains — instead of fanning out unfiltered (#4501) `resolveBulkRows` re-issues the view's query in 500-record pages so a bulk action receives the whole match set rather than the visible window. The query it replayed came from `lastFindParamsRef`, whose only writer is ObjectGrid's own data loader. Under a host that fetches the rows itself — ListView passing `data` + `manualPagination` + `rowCount`, i.e. the console — that loader never runs, so the ref was not the query behind the rows on screen: absent, or stale from an earlier own-fetch. Either way the `?? {}` default let the fan-out ask the server for the WHOLE OBJECT and hand up to 5000 unmatched records to a destructive executor while the bar read "All N matching records are selected". Two halves, per the #4501 ruling: 1. Host params prop. ListView hoists the params object out of its `find` call, records it past the stale-request guard, and hands it down as `findParams` in the same block as `rowCount` / `page` / `onPageChange`. ObjectGrid reads whichever side owns the fetch — `hasInlineData` is the loader's own guard, so it is the precise test for "this grid did not issue the query behind these rows". No grid-side `?? {}` fallback: that is what produced the bug. 2. Abstain floor. With no query for the current data path the escalation is NOT OFFERED — one `canOfferSelectAllMatching` consumed by both `BulkActionBar` sites, so the offer and the thing it promises cannot disagree, and a host that forgets `findParams` loses the affordance instead of silently collecting the whole object. Clause 2 rides along: a changed `findParams` resets the escalation, mirroring the `setSelectAllMatching(false)` the internal loader runs next to its own params write. Compared by content, so a host re-render that rebuilds an equal object does not drop the user's escalation. Red-first, measured by reverting both sources to main with the tests in place: 7 of 11 red, the pre-fix fan-out issuing `{$select:['id','name'],$skip:0,$top:500}` — `$filter`, `$orderby` and `$search` all absent — and the affordance offering "Select all 40 matching" over a 26-record host window. The internal-loader path is unchanged and stays green either side. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
yinlianghui
marked this pull request as ready for review
August 13, 2026 02:04
Collaborator
Author
|
ACCEPT — step-7 复核 by PM session
Flipping ready + arming auto-merge. Sequencing continues as ruled: PR #4503 flips + arms only after THIS lands in Generated by Claude Code Generated by Claude Code |
yinlianghui
pushed a commit
that referenced
this pull request
Aug 13, 2026
…l compose PR #4510 (#4501's abstain floor) landed on the same two `BulkActionBar` prop sites this branch rewrites, so both hunks conflicted. Composed rather than picked: the offer requires #4510's FLOOR (`canOfferSelectAllMatching` — no escalation without a query to replay) and carries #4503's RESOLVED total (`resolvedTotalMatching` — the host's `rowCount` on the external path), i.e. totalMatching={canOfferSelectAllMatching ? resolvedTotalMatching : undefined} at both sites. The floor subsumes the `singleSelection ? undefined : …` suppression the incoming side spelled there — `!singleSelection` is its first conjunct — so no gating is lost; a host with a real total but no `findParams` still gets no offer, which is the safety semantics winning the tie. Follow-through, ruled in both PRs: the composition test gains the full-path fan-out assertion #4510 made possible. The real ListView issues a real filtered query, the real grid offers the escalation off the host's `rowCount`, and the dispatched bulk action's fan-out is asserted to replay ListView's own params verbatim — measured against what went on the wire, not a hand-written literal. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3
This was referenced Aug 13, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #4501
The hazard
resolveBulkRowsre-issues the view's query in 500-record pages so a bulk action receives the whole match set rather than the visible window. The query it replayed came fromlastFindParamsRef, whose only writer is ObjectGrid's own data loader.Under a host that fetches the rows itself — ListView passing
data+manualPagination+rowCount, i.e. the console — that loader never runs (if (hasInlineData) return), so the ref is not the query behind the rows on screen: absent, or stale from an earlier own-fetch. Either way the?? {}default let the fan-out ask the server for the whole object — no$filter, no$orderby, no$search— and hand up to 5000 unmatched records to a destructive executor (onBulkDelete) while the bar read "All N matching records are selected".Reachability changed with #4464: before it the cross-page banner could never appear on that path. PR #4503 is held in draft behind this guard — landing order is the safety mechanism.
The fix — both halves of the #4501 ruling
1. Host params prop (the contract fix). ListView hoists the params object out of its
findcall — one object, one query, no second literal free to drift from what was actually asked — records it past the stale-request guard so it is always the query that produced the rows on screen, and hands it down asfindParamsin the same handoff block asrowCount/page/onPageChange. ObjectGrid reads whichever side owns the fetch:hasInlineDatais the loader's own guard, which makes it the precise test for "this grid did not issue the query behind these rows". There is deliberately no grid-side?? {}fallback — that tolerant-consumer shape is exactly what produced the unfiltered fan-out.2. The abstain floor (the permanent guard). With no query available for the current data path the escalation is not offered at all — one
canOfferSelectAllMatching, consumed by bothBulkActionBarsites, so the offer and the thing it promises cannot disagree. A host that forgetsfindParamsloses the affordance rather than silently collecting the whole object. That is what makes the unfiltered fan-out structurally unreachable rather than merely currently-wired-right. The same single source is re-checked at the point of consumption, so the gate and the action cannot drift.Clause 2 rides along. A changed
findParamsresets the escalation, mirroring thesetSelectAllMatching(false)the internal loader runs next to its own params write. Compared by content, not identity, so a host re-render that rebuilds an equal object does not drop the user's escalation.The internal-loader path is untouched: with the ref populated the fan-out issues the same params it always did, and the
selection.type: 'single'suppression is unchanged.Red-first
Both source files reverted to
origin/mainwith the tests in place — 7 of 11 red, restored byte-identical afterwards (sha256-verified). The clause-1 fan-out issued, verbatim:$filter,$orderbyand$searchall absent — the whole-object read. And the floor case found the affordance offered where it must not be:(Angle brackets spaced in that excerpt only — GitHub's body sanitizer eats
<followed by a letter at rest, code fence included.)"Select all 40 matching" over a 26-record host window. The 4 that stayed green either side are the negative controls: both internal-loader must-not-change cases, the single-selection suppression, and the equal-params re-render (which must not reset).
Verification
plugin-grid+plugin-listfull suitestype-checkboth packages (src + tests)...@object-ui/plugin-grid/-listeslintboth packagescheck:control-byteschangeset:check+ presenceDependency-closure build ran first, so no gate read a stale
.d.ts.Grading
.d.tsmeasured both ways againstorigin/main:plugin-grid— one new optional published member on the exportedObjectGridExternalPaginationProps:findParams?: Record< string, unknown > | null. Additive; nothing removed or narrowed. Reverse-verified that it is genuinely typed (a probe assigning a number is rejectedTS2322). A new published props member is minor per the precedent the ruling cites — graded accordingly, never major.plugin-list— emitted.d.tsis byte-identical. No public API change, so patch.Surface
packages/plugin-grid/src/ObjectGrid.tsx,packages/plugin-list/src/ListView.tsx, tests in both packages, two changesets — the mutual-exclusion surface the ruling set, nothing else.BulkActionBar, app-shell'sObjectViewandconsole/**are untouched. Regions are disjoint from PR #4503's; this branch is offorigin/main.Follow-through, not blocking: once #4503 lands, its test file gains the full-path assertion (the host-only path can reach the escalation there, so the count the bar shows becomes assertable too).
Generated by Claude Code