sadpandajoe opened a new pull request, #43947:
URL: https://github.com/apache/superset/pull/43947
### SUMMARY
When a chart is opened from a dashboard, Explore inherits the dashboard's
filters (e.g. a time range or a value filter) and marks them as not saved to
the chart. However, editing one of these inherited filters in Explore and
saving did not actually keep the edit — the filter reverted to its pre-edit
state (or, for the time range specifically, silently kept the dashboard's
original value instead of the edited one).
### ROOT CAUSE
Explore tags every adhoc filter inherited from a dashboard with `isExtra:
true`, and the save path strips `isExtra` filters from the payload sent to the
chart (restoring the pre-dashboard value for time-range filters specifically).
`AdhocFilter.duplicateWith`, used by every edit handler in the Simple filter
tab (subject, operator, comparator, and the time-range date picker), copies the
existing `isExtra` flag forward unless the caller explicitly overrides it. So
editing an inherited filter changed it live in the query, but at save time the
edited filter still looked exactly like an untouched dashboard filter and was
discarded/reverted.
The fix adds `isExtra: false` to the five `duplicateWith(...)` calls in
`AdhocFilterEditPopoverSimpleTabContent`'s edit handlers, so a deliberate edit
is adopted as the chart's own filter rather than treated as still-inherited.
A related symptom from the original report — a non-time-range value filter
(e.g. a categorical column) getting silently persisted onto the chart even when
untouched — is already handled by the existing save-path logic that strips
non-temporal `isExtra` filters unconditionally, so it is out of scope here.
### BEFORE/AFTER SCREENSHOTS
UI QA (before/after video) is in progress and will be attached to this PR
once captured; this fix is validated by a new automated regression test in the
interim (see Testing Instructions), confirmed to fail before the fix and pass
after:
```
FAIL (pre-fix, at guard commit only): 2 failed, 39 skipped, 41 total
✕ editing a dashboard-inherited time range filter clears isExtra so the
new value is kept on save
✕ editing a dashboard-inherited filter comparator clears isExtra so the
new value is kept on save
PASS (post-fix): 41 passed, 41 total
```
### TESTING INSTRUCTIONS
1. Create a chart using any dataset with a temporal column, save it to a
dashboard.
2. On the dashboard, add a Time Range filter and apply a value.
3. Open the chart from the dashboard (click the chart title) — the inherited
time range filter is applied.
4. Change the time range filter's value in Explore, then Save.
5. Re-open the chart: the newly edited time range value is now kept
(previously it silently reverted to the dashboard's original value).
Automated coverage:
`superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/AdhocFilterEditPopoverSimpleTabContent.test.tsx`
### ADDITIONAL INFORMATION
- [ ] Has associated issue
- [ ] Required feature flags
- [ ] Changes UI
- [x] Includes tests
- [ ] Functional or Behavior Change
- [ ] Migrations
- [ ] Includes downloadable artifact (jar, wheel, etc)
- [ ] Changes API contracts (e.g. Adds new endpoint, changes an existing
endpoint)
- [ ] Confidential fields (e.g. add a new secret field to be used in
`expose_in_sqllab`, connections)
- [ ] Affects chart plugins (please describe)
- [ ] Includes an incomplete migration (unit test data cases missing)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
