fix(dashboard): prevent divider display controls from reverting on second save#40696
Conversation
…sc-85682) DividerConfigForm does not call handleModifyItem on form value changes the way FiltersConfigForm does via its onModifyFilter prop. As a result, editing a saved divider's title/description on a second modal open left the divider absent from changes.modified, keeping canSave false and causing resetForm to revert the displayed values back to the prior saved state after any save. Fix: detect divider field changes in handleValuesChange (Form.onValuesChange) and call handleModifyItem for the affected divider ID, ensuring it enters changes.modified so canSave becomes true and the save payload includes the updated values. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
dosubot
Bot
added
the
dashboard:native-filters
![bito-code-review[bot]](https://avatars.githubusercontent.com/in/1061978?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Agent Run #d51bc7
Actionable Suggestions - 1
-
superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.test.tsx - 1
- Incorrect timer advancement · Line 843-843
Review Details
-
Files reviewed - 2 · Commit Range:
1be9916..1be9916- superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.test.tsx
- superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.tsx
-
Files skipped - 0
-
Tools
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
Bito Usage Guide
Commands
Type the following command in the pull request comment and save the comment.
-
/review- Manually triggers a full AI review. -
/pause- Pauses automatic reviews on this pull request. -
/resume- Resumes automatic reviews. -
/resolve- Marks all Bito-posted review comments as resolved. -
/abort- Cancels all in-progress reviews.
Refer to the documentation for additional commands.
Configuration
This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.
Documentation & Help
AI Code Review powered by
| await userEvent.clear(titleInput); | ||
| await userEvent.type(titleInput, 'Second Edit'); | ||
|
|
||
| jest.advanceTimersByTime(1000); |
There was a problem hiding this comment.
Timer advances by 1000ms but Constants.SLOW_DEBOUNCE is 500ms. Use 500ms to match the actual debounce delay and avoid unnecessary waiting.
Code suggestion
Check the AI-generated fix before applying
--- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.test.tsx
+++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.test.tsx
@@ -840,7 +840,7 @@ test('enables save button and includes updated title when editing an existing d
await userEvent.type(titleInput, 'Second Edit');
- jest.advanceTimersByTime(1000);
+ jest.advanceTimersByTime(500);
jest.useRealTimers();
await waitFor(() =>
Code Review Run #d51bc7
Should Bito avoid suggestions like this for future reviews? (Manage Rules)
- Yes, avoid them
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #40696 +/- ##
==========================================
+ Coverage 64.13% 64.16% +0.03%
==========================================
Files 2666 2666
Lines 143979 143983 +4
Branches 33105 33107 +2
==========================================
+ Hits 92337 92383 +46
+ Misses 50034 49990 -44
- Partials 1608 1610 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Fixes a dashboard “display controls” modal bug where edits to divider title/description weren’t marked as modified, causing the Save button to remain disabled on reopen and (on subsequent saves) divider values to revert to previously-saved content.
Changes:
- Update
FiltersConfigModalonValuesChangehandling to detect divider field edits and callhandleModifyItemfor the divider ID. - Add a regression test ensuring editing an existing native filter divider enables Save and persists the updated title in the save payload.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.tsx | Marks divider items as modified when divider form fields change so the Save button enables and payload includes updated divider values. |
| superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.test.tsx | Adds regression coverage for editing an existing NATIVE_FILTER_DIVIDER-* item and verifying save payload contains the new title. |
| test('enables save button and includes updated title when editing an existing divider', async () => { | ||
| jest.useFakeTimers(); | ||
|
|
||
| const nativeFilterDividerConfig = [ | ||
| { |
…ge (sc-85682) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Code Review Agent Run #426018Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
|
Bito Automatic Review Skipped – PR Already Merged |
SUMMARY
Fixes a bug where editing a dashboard divider's title/description in the "Add or edit display controls" modal causes the panel to revert to the first-saved values on subsequent saves, and the Save button to start disabled on every reopen after the first save.
Root cause (two-part):
DividerConfigFormnever callshandleModifyItemwhen the user types — unlikeFiltersConfigForm, which calls it via itsformChanged/onModifyFilterpath. This caused two symptoms:Save button disabled on reopen. After the first save,
changes.modifiedis reset to[]. On modal reopen the save button'scanSavecheck requireschanges.modified.length > 0, which never becomes true because no code marks the divider modified when the user types.Values revert on save. When save is triggered,
resetForm(true)resets all antd fields back toinitialValue. Since the divider was never added tochanges.modified, its updated value is excluded from the save payload and the display reverts to the previously saved title.Fix: In
FiltersConfigModal.tsx, updatehandleValuesChange(wired toForm.onValuesChange) to detect when a changed field key belongs to a divider (NATIVE_FILTER_DIVIDER-*orCHART_CUSTOMIZATION_DIVIDER-*) and callhandleModifyItemfor that ID. This mirrors the existing pattern inFiltersConfigFormwithout requiring changes toDividerConfigFormor any renderer components.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A — logic fix, no visual change to the modal layout.
TESTING INSTRUCTIONS
Repeat with a chart customization divider (
CHART_CUSTOMIZATION_DIVIDER-*) to confirm both prefixes are covered.ADDITIONAL INFORMATION