mikebridge opened a new pull request, #45069:
URL: https://github.com/apache/superset/pull/45069
### SUMMARY
Native filters can't be configured on a dashboard whose charts use a
semantic view. When you add a filter, the datasource and column pickers spin
forever.
**Cause.** A new filter or display control picks its default datasource from
the chart datasource used most. That default kept only the numeric id, so the
type fell back to SQL dataset. The filter config then:
- requested `GET /api/v1/dataset/<semantic view id>`, which 404s;
- sent a default-value query against `<id>__table`;
- computed the filter's initial chart scope for the wrong type.
Datasets and semantic views have independent id sequences. If a readable SQL
dataset happened to share the view's id, the filter silently used that
dataset's columns, default values and scope.
The pickers themselves already handle semantic views correctly. They just
never received the right type for the default.
**What changes (frontend only):**
- **Typed default.** `mostUsedDataset` returns a typed `(id, type)` binding
from the dashboard's datasource entry.
- **One source for id and type.** The form resolves both together: first
from the form's selection (whose type comes from the selected value's own
`kind`), then from the saved target, then from the default. Saved targets with
no type keep resolving as SQL datasets, as they always have.
- **Type kept in sync.** The hidden type field is written to the type of the
datasource that loaded. Datasources that reach the dashboard after the modal
opens can then no longer pair a semantic view's id with the SQL type in
requests, default-value queries or the saved target.
- **No stale responses.** Responses for a datasource that is no longer
selected are ignored, including column responses that arrive after the picker
has changed datasource or unmounted, and the previous datasource's details are
cleared while the next one loads. Its columns or semantic selection version
can't be attached to another datasource.
- **Failure handling.** A failed datasource load, of either type, now:
- shows its error, which the form never dispatched before;
- leaves an empty, usable datasource select instead of an endless spinner;
- retries when you choose a datasource, including SQL datasets, whose
failed request is evicted from the request cache.
- **Switching back now works.** The memoized datasource select kept a stale
change handler, so switching back to a same-id datasource of the other type
kept the previous type. It now compares against the live selection.
- **Time-range pre-filter.** It looks up its datasource by type and id
rather than id alone.
Each datasource is fetched only through its own type's endpoint:
`/api/v1/dataset/<id>` or `/api/v1/semantic_view/<id>/structure`. That
endpoint's access checks therefore apply. No backend change.
**Relation to #44999** (manual range bounds for semantic views). That PR
builds on this one. Its semantic-view range behavior now also applies to a
semantic view that was picked as the default. It merges cleanly with this
branch.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
**Before:**
- *New filter on a semantic-view dashboard:* the datasource and column
pickers spin forever. Network shows `GET /api/v1/dataset/<view id>` → 404, and
a `chart/data` call against `<id>__table` → 404.
- *Saved filter whose datasource fails to load:* the field spins. Renaming
the filter and saving writes it with an empty target (`targets: [{}]`),
silently dropping its datasource and column.
**After:**
- *New filter on a semantic-view dashboard:* the semantic view is
pre-selected and its dimensions are listed. Network shows `GET
/api/v1/semantic_view/<view id>/structure`, with no `/api/v1/dataset/<view id>`
request.
- *Saved filter whose datasource fails to load:* the error is shown and the
datasource select is empty and usable. Save asks for a datasource ("Datasource
is required") instead of wiping the target. Re-pointing or removing the filter
resolves it.
_(Screenshots to follow from a provider-backed environment.)_
### TESTING INSTRUCTIONS
**Automated:** focused FiltersConfigModal suites pass (23 suites, 238
passed, 1 skipped).
- `npm run test --
src/dashboard/components/nativeFilters/FiltersConfigModal`. The new
`FiltersConfigModal.semanticDefault.test.tsx` starts from the real
default-selection path with dashboard state shaped like the
`/api/v1/dashboard/<id>/datasets` payload. It registers exact routes only, and
any request to `/api/v1/dataset/<view id>` fails the test.
- It covers:
- Value, Numerical range, Time column and Time grain filters, and display
controls;
- a same-id SQL dataset;
- mixed dashboards in both most-used directions;
- switching type;
- initial scope;
- late-loading datasources;
- slow responses;
- failures and retries;
- the saved-filter failure;
- stale column responses for a superseded datasource, including after the
picker unmounts;
- unchanged SQL-only request URLs.
Playwright coverage for semantic-view dashboards is a follow-up.
**Manual** (needs a working semantic-layer provider, because `/structure`
queries it):
1. **Semantic-view dashboard.** Open a dashboard whose charts use a semantic
view, then Filters → Add filter → Value.
- Expect the semantic view to be pre-selected and its dimensions listed.
- In DevTools, check there is no `/api/v1/dataset/<view id>` request.
- Save, reload, and confirm the filter applies.
2. **Other filter types.** Repeat for Numerical range, Time column, Time
grain and a Dynamic group-by display control.
3. **Mixed dashboard.** On a dashboard with both SQL-dataset and
semantic-view charts, the default follows the datasource used most. Switching
between a dataset and a view loads the right columns each time.
4. **SQL-only dashboard.** Behavior and requests are unchanged.
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [x] Required feature flags: existing `SEMANTIC_LAYERS`; no flag changes.
- [x] Changes UI
- [ ] Includes DB Migration (follow approval process in
[SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
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]
