Staging sync: stabilize DataControlHeader filtering flow and workspace auth logging#179
Merged
VibhavSetlur merged 4 commits intoModelSEED:stagingfrom May 5, 2026
Merged
Conversation
…nstraints Refactors DataControlHeader and server-driven table consumers to keep filter behavior consistent when using MUI DataGrid Community, where multiple column filtering is internally constrained. The toolbar now applies authoritative models explicitly, consumers guard against grid truncation callbacks, and filter editor interactions are hardened so user-applied filters remain stable and predictable. Also updates DataControlHeader e2e coverage with multi-filter regression scaffolding and stronger select interaction helpers to reflect the new toolbar-driven flow. Co-authored-by: Cursor <cursoragent@cursor.com>
Downgrades workspace 401 logging from error to info while preserving thrown errors and user-facing failure messages. This keeps local development and signed-out sessions from producing misleading high-severity console noise without masking real non-auth workspace failures. Co-authored-by: Cursor <cursoragent@cursor.com>
Cleans up changed filter consumers and toolbar search logic to satisfy strict React hook/lint rules without masking runtime behavior. Removes unused callback imports/params and debug noise in biochem pages, and keeps toolbar quick-search debounce behavior lint-compliant while preserving multi-filter application flow. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Pull request overview
This PR syncs staging with upstream staging, focusing on stabilizing DataControlHeader’s filter-model behavior under MUI DataGrid Community constraints (especially multi-filter + quick-search interactions) and reducing log severity for expected unauthenticated Workspace calls.
Changes:
- Refactors
DataControlHeaderto preserve multi-column filter intent via committed filter refs/registry and adds a toolbar→page callback (onApplyFilterModel) for server-backed grids. - Updates server-grid consumers (biochem reactions/compounds and PATRIC genomes) and adds Playwright e2e regression coverage for multi-filter persistence and logic operators.
- Downgrades Workspace 401 logging from
errortoinfowhile preserving error throwing for failed calls.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| components/layout/DataControlHeader.tsx | Introduces committed multi-filter tracking and toolbar callback wiring intended to bypass Community filter truncation. |
| app/(reference-data)/biochem/reactions/page.tsx | Adjusts server filtering handler and wires toolbar callback for authoritative filter updates. |
| app/(reference-data)/biochem/compounds/page.tsx | Same as reactions: updates filter handler and wires toolbar callback. |
| components/build-model/PatricGenomesTable.tsx | Adds logic intended to preserve multi-filter state across truncation events in server filtering. |
| tests/e2e/datacontrol-header.spec.ts | Adds helper refactors and new multi-filter regression tests (AND/OR, persistence). |
| lib/api/workspace.ts | Logs 401 unauthenticated responses as info instead of error while still throwing. |
Comments suppressed due to low confidence (3)
components/layout/DataControlHeader.tsx:227
- In server-side mode (when onApplyFilterModel is provided), this branch bypasses apiRef.setFilterModel entirely. That leaves the grid’s internal filterModel/quickFilterValues stale, which can break highlight behavior (GridHighlightText relies on gridFilterModelSelector), prevent draftQuick from ever clearing, and cause the filter editor to “lose” the current search term when saving filters. Consider also calling apiRef.setFilterModel with a grid-safe/truncated model (e.g. items.slice(0,1) but including quickFilterValues) while still calling onApplyFilterModel with the full multi-filter model.
const handleChange = useCallback(
(e: React.ChangeEvent<HTMLInputElement>) => {
const term = e.target.value;
setDraftQuick(term);
if (debounceRef.current) clearTimeout(debounceRef.current);
app/(reference-data)/biochem/reactions/page.tsx:362
- This console.debug in queryOpts will run on every dependency change and may leak noisy internal state (including potentially large filterModel JSON) into production logs. Please remove it or guard it behind a dev-only flag.
sort: sortModel[0]
? { field: sortModel[0].field, desc: sortModel[0].sort === 'desc' }
: { field: 'id' },
app/(reference-data)/biochem/reactions/page.tsx:411
- DataGrid is in server filterMode and the page’s Solr query is driven by the local filterModel state, but filterModel is no longer passed into the DataGrid. This can desync the grid UI (quick filter value, highlights, filter panel state) from the server query state. Consider keeping the grid controlled (pass filterModel={filterModel} or a grid-safe derived model) so gridFilterModelSelector consumers (e.g. GridHighlightText) and the toolbar’s committed value reflect the applied filters/search.
getRowHeight={() => 'auto'}
disableRowSelectionOnClick
hideFooter
disableColumnMenu
sx={{
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+616
to
+620
| // server query receives the complete multi-filter. The grid's setFilterModel | ||
| // silently truncates to 1 item (Community Edition), so we bypass it entirely. | ||
| if (typeof onApplyFilterModel === 'function') { | ||
| // Pass source:'toolbar' so handlers know this is an explicit user action, | ||
| // not a Community Edition truncation event from the grid itself. |
Comment on lines
+631
to
635
| apiRef.current.setFilterModel(gridModel); | ||
| } | ||
|
|
||
| allColumns.forEach((column) => { | ||
| const visible = draftColumnVisibilityModel[column.field] !== false; |
Comment on lines
268
to
272
| slots={{ toolbar: DataControlHeader }} | ||
| slotProps={{ toolbar: { onApplyFilterModel: handleToolbarApplyFilterModel } }} | ||
| getRowId={(row) => row.id} | ||
| getRowHeight={() => 'auto'} | ||
| disableRowSelectionOnClick |
Comment on lines
+156
to
+160
| const committed = committedFilterItemsRef.current; | ||
| const incomingItems = (next.items ?? []) as GridFilterItem[]; | ||
| const shouldKeepCommitted = | ||
| committed.length > 0 && incomingItems.length < committed.length; | ||
| return { |
Comment on lines
176
to
178
| showToolbar | ||
| slots={{ toolbar: DataControlHeader }} | ||
| hideFooter |
|
|
||
| async function waitForGridStable(page: Page): Promise<void> { | ||
| // Wait for loading indicator to disappear and row count to stabilise | ||
| await page.waitForTimeout(600); |
Comment on lines
+206
to
+210
| logicOperator, | ||
| quickFilterValues: term.trim() ? [term.trim()] : [], | ||
| quickFilterLogicOperator: | ||
| (current.quickFilterLogicOperator as GridLogicOperator | undefined) ?? | ||
| GridLogicOperator.And, | ||
| }); | ||
| quickFilterLogicOperator: GridLogicOperator.And, | ||
| }; | ||
|
|
| import { useState, useCallback, useMemo, useRef } from 'react'; | ||
| import { useQuery, keepPreviousData } from '@tanstack/react-query'; | ||
| import { DataGrid, GridColDef, GridPaginationModel, GridSortModel, GridFilterModel } from '@mui/x-data-grid'; | ||
| import Typography from '@mui/material/Typography'; |
…rolHeader - Added applyTwoFilters() helper for testing multi-filter scenarios - Added activeFilterCount() helper to read filter badge - Made fillFilterRow() row-index-aware - Added regression test: multiple filters persist after Save - Added OR-logic test to verify logic operator switching
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.
Summary
This PR syncs fork
stagingto upstreamstagingwith two atomic commits fromdevelop.Commit 1
fix(filters): preserve toolbar filter intent across community grid constraintsThis commit hardens DataGrid filtering behavior in areas using
DataControlHeader, especially under MUI DataGrid Community constraints where native multi-column filter behavior differs from Pro/Premium.What changed:
DataControlHeadertoolbar internals so filter application is explicit and stable.Why this matters:
Commit 2
chore(logging): treat unauthenticated workspace calls as expected noiseWhat changed:
lib/api/workspace.ts, 401 workspace responses now log asinfo(expected unauthenticated case) rather thanerror.Why this matters:
Files Touched
components/layout/DataControlHeader.tsxapp/(reference-data)/biochem/reactions/page.tsxapp/(reference-data)/biochem/compounds/page.tsxcomponents/build-model/PatricGenomesTable.tsxtests/e2e/datacontrol-header.spec.tslib/api/workspace.tsValidation Performed
npx tsc --noEmit(pass)developpushed to forkstagingfast-forward merged fromdevelopstagingpushed to forkRisk Notes
Requested Review Focus
Please focus review on:
DataControlHeaderfilter model flow and callback wiring,Made with Cursor