fix: Schema Diff filter chip toggle broadcasting stale empty selection#10172
fix: Schema Diff filter chip toggle broadcasting stale empty selection#10172kundansable wants to merge 1 commit into
Conversation
selectFilterOption() read newOptions from inside the functional
setSelectedFilters(prev => {...}) updater and used it to dispatch
TRIGGER_CHANGE_FILTER synchronously in the same tick. React's
functional updater runs lazily during render, so newOptions was still
its initial [] value when the event fired — every chip toggle
broadcast an empty filter, so prepareRows() returned nothing and the
results tree showed "No difference found" until the whole comparison
was re-run.
Compute the new selection synchronously from the current
selectedFilters state and use that same array for both the state
update and the dispatched event, so the filter chips and the results
grid stay in sync on every toggle.
Fixes pgadmin-org#10102
WalkthroughSchema Diff filter toggling now computes the next selected filter array synchronously, uses it for the state update, and emits the same selection in the filter-change event. ChangesSchema Diff filter behavior
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
web/pgadmin/tools/schema_diff/static/js/components/SchemaDiffButtonComponent.jsx (1)
115-130: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd regression coverage for filter-toggle synchronization.
The implementation correctly sends the synchronously computed selection, but add tests covering off→on restoration, multiple toggle orders, correct hierarchy counts, and ensuring
TRIGGER_COMPARE_DIFFis not emitted during filter changes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/pgadmin/tools/schema_diff/static/js/components/SchemaDiffButtonComponent.jsx` around lines 115 - 130, Add regression tests for the filter-toggle flow around the SchemaDiffButtonComponent selection handler, covering off-to-on restoration, multiple toggle orders, and accurate hierarchy counts. Also assert that filter changes emit TRIGGER_CHANGE_FILTER with the computed selection without emitting TRIGGER_COMPARE_DIFF.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@web/pgadmin/tools/schema_diff/static/js/components/SchemaDiffButtonComponent.jsx`:
- Around line 115-130: Add regression tests for the filter-toggle flow around
the SchemaDiffButtonComponent selection handler, covering off-to-on restoration,
multiple toggle orders, and accurate hierarchy counts. Also assert that filter
changes emit TRIGGER_CHANGE_FILTER with the computed selection without emitting
TRIGGER_COMPARE_DIFF.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: cb86f777-ed03-4034-a3e3-6e5e03eb2b5c
📒 Files selected for processing (1)
web/pgadmin/tools/schema_diff/static/js/components/SchemaDiffButtonComponent.jsx
Summary
Fixes #10102 — in Tools → Schema Diff, toggling a result-status filter chip off and back on shows "No difference found" even when real differences exist, until the whole comparison is re-run.
Root Cause
selectFilterOption()computed the new chip selection (newOptions) inside the functional form ofsetSelectedFilters(prev => {...}), then immediately used that samenewOptionsvariable to dispatchTRIGGER_CHANGE_FILTERsynchronously in the same tick. React's functional state updater runs lazily during the next render — so at the point the event was dispatched,newOptionswas still its initial empty array. Every chip toggle therefore broadcast an empty filter,prepareRows()had nothing to render, and the tree went blank. The initial "Compare" click worked fine only because it readsselectedFiltersstate directly rather than going through this buggy path.Fix
Compute the new filter selection synchronously from the current
selectedFiltersstate (not from inside thesetSelectedFiltersupdater callback), and use that same synchronously-computed array both to update state and to dispatch the change event — so the event and the state always agree.Test Steps
Summary by CodeRabbit