Skip to content

fix(bindx-dataview): scope the export query the way the view loads - #105

Closed
matej21 wants to merge 4 commits into
mainfrom
fix/export-ignores-static-filter
Closed

fix(bindx-dataview): scope the export query the way the view loads#105
matej21 wants to merge 4 commits into
mainfrom
fix/export-ignores-static-filter

Conversation

@matej21

@matej21 matej21 commented Sep 9, 2026

Copy link
Copy Markdown
Member

The leak

DataViewExportTrigger built its unpaged export query from filtering.resolvedWhere only. The grid's static filter prop — the one that scopes every row the operator can see — never reached it, so the exported file contained rows from outside the grid's visible scope. Any statically scoped list ("non-archived contacts", "sessions of this program") leaked.

On HasManyDataGrid it degenerated further. The grid loads its rows through a GET on the parent record with a nested relation spec, but publishes entityType: targetEntityType into the context. The export took that at face value and issued a root list query over the whole target entity table, with no parent constraint at all.

useDataViewFetchAllData() had the same gap.

Why a context-level fetch-all, not an exposed filter

The obvious fix is to publish combinedFilter on the context and use it in the export. That fixes today's bug and leaves the class of bug open: a filter is only correct for views whose query shape is a root list. HasManyDataGrid proves it is not universal — no filter it could publish makes a root list query over the target table correct, because the parent constraint is not expressible there.

So the context now carries the operation, not the ingredient:

readonly fetchAllData: () => Promise<readonly Record<string, unknown>[] | null>

Each view implements it as the query it already loads with, minus paging:

  • DataGrid — its list query with setup.combinedFilter and the resolved orderBy, no limit/offset.
  • SelectDataView — the same, via the shared useListFetchAllData hook.
  • HasManyDataGrid — the same GET on the parent, no limit/offset. Load and export now share one buildHasManyRelationQuery builder, so they cannot drift.

A future view cannot reintroduce this bug by forgetting to combine a filter: it has to supply its own unpaged query or it does not compile.

DataViewExportTrigger and useDataViewFetchAllData stop building queries and just call the context. Their public props and return shapes are unchanged.

Toolbar slot

DataGridToolbarUI hard-coded <DataGridAutoExport /> with no way to replace or hide it, and the DataGridShowFiltersContext it feeds the mobile filter chips was not exported — so a consumer needing a different export control had to rebuild the toolbar and lose the mobile filters toggle.

  • DataGridToolbarUI gains exportControl?: ReactNode | null — default <DataGridAutoExport />, null hides it.
  • DataGridShowFiltersContext is now exported from @contember/bindx-ui, so a custom toolbar keeps the mobile filters toggle.

Verification

  • tests/react/dataview/exportIgnoresStaticFilter.test.tsx — the reporter's repro, cherry-picked with their authorship. Confirmed failing before any source change (export query filter was {}, not published).
  • Added the has-many half in the same file: a HasManyDataGrid export must read through the parent record. It asserts on what the adapter actually receives — zero root list queries on the target entity, a GET on Author by id whose relation field is unpaged and carries the static filter, and exported rows containing only this parent's matching row.
  • Both new tests were re-run against the pre-fix export.tsx and fail there: the static-filter one on the empty filter, the has-many one on the root list query it issues (1, expected 0).
  • bun run typecheck — clean.
  • bun run test — 2028 pass, 0 fail, 212 files. (tests/browser excluded as usual; it needs a live playground.)
  • bun run lint — 0 errors; the 17 warnings are pre-existing and untouched.

No as casts, any, or ts-ignores added; a couple were removed along the way (the redundant ListQueryResult casts in the export path and an as unknown[] on the has-many alias params).

Fixes #67

🤖 Generated with Claude Code

https://claude.ai/code/session_01Euwkf2wtutqvtE4YRutU5R

vparys and others added 4 commits September 9, 2026 15:47
DataViewExportTrigger built its unpaged query from filtering.resolvedWhere
only, so a grid's static `filter` prop never reached the export and the file
carried rows outside the grid's visible scope. On HasManyDataGrid it went
further: a root list query over the whole target entity table, with no parent
constraint at all.

The context now carries `fetchAllData`, and each view implements it as the
query it already loads with, minus paging — DataGrid and SelectDataView as
their list query with the combined filter, HasManyDataGrid as the same GET on
the parent record. Exposing a filter instead would leave the next view free to
forget to combine it; owning the query shape cannot be forgotten.

Fixes #67

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Euwkf2wtutqvtE4YRutU5R
The reporter's repro covers the static filter on a root DataGrid. This adds the
has-many half: the export must read the relation through the parent record, so
it can never reach another parent's rows.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Euwkf2wtutqvtE4YRutU5R
DataGridToolbarUI hard-coded <DataGridAutoExport />, and the context feeding
its mobile filter chips was private, so swapping the export meant rebuilding
the toolbar and losing the mobile filters toggle. Adds an `exportControl` slot
(null hides it) and exports DataGridShowFiltersContext.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Euwkf2wtutqvtE4YRutU5R
@matej21

matej21 commented Sep 10, 2026

Copy link
Copy Markdown
Member Author

Consolidated into #109, merged there.

@matej21 matej21 closed this Sep 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

DataViewExportTrigger ignores the grid's static filter (and parent scope in HasManyDataGrid); toolbar export control is not replaceable

2 participants