mikebridge opened a new pull request, #42539:
URL: https://github.com/apache/superset/pull/42539
### SUMMARY
Semantic views and regular datasets have **independent numeric-id
sequences**, so a semantic view with `id=3` and a regular dataset with `id=3`
both exist. Two surfaces resolved their bound datasource by id alone and
silently read the wrong object:
1. **Dynamic group by display control** — the FilterBar loader fetched
`/api/v1/dataset/<id>` while ignoring the `datasourceType` already persisted on
`targets[0]`, so the dropdown offered the colliding *dataset's* columns with no
error shown.
2. **Drill to detail** — `useDatasetDrillInfo` parsed the `<id>__<type>`
composite but discarded the type, so the drill modal's name and `verbose_map`
came from the colliding dataset.
Both are extensions of the type-aware pattern already shipped in #40475,
applied at a shared resolution point rather than as a fourth and fifth inline
copy:
- **Extract** `fetchSemanticViewStructure` /
`semanticViewDimensionsToColumns` from the two existing inline copies
(`ColumnSelect.tsx`, `FiltersConfigForm.tsx`) and migrate both onto them.
Behaviour-preserving; existing suites unchanged.
- **New `useDisplayControlDatasource` hook** — type-branched resolution with
a monotonic request-id stale-response guard (a slow `/dataset/3` must not
overwrite a fast `/semantic_view/3/structure` after a type flip), inert when
the control is unbound. The regular-dataset branch preserves the legacy request
byte-for-byte; characterization tests for that path were authored against the
*unmodified* component first so the claim is falsifiable.
- **Preserve `datasourceType` at both target-rewrite sites**
(`handleColumnChange` and the FilterBar clear-customizations path). Without
this the fix self-reverts: the first select-and-apply rebuilt the target
without the type, silently degrading the binding back to the colliding dataset.
- **`useDatasetDrillInfo`** — the `semantic_view` branch short-circuits
*before* the `load.drillby.options` extension check, because that extension
receives only the numeric id and would resolve the collision itself.
Non-semantic types keep byte-identical behaviour including extension precedence.
- Dropped the display-control effect's dead `dependencies` coupling (it was
in the dep array but the request never used it).
**Known, deliberate degradation:** the drill modal's metadata bar renders
its explicit "Not available" state for Last modified / Modified by on semantic
views — the structure endpoint carries no `changed_on`/owners metadata. This is
pinned by test rather than papered over.
Everything here is behind `FeatureFlag.SemanticLayers`, which is off by
default.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Not applicable — the visible difference requires a semantic-layer provider
configured with an id collision against a regular dataset. Covered by tests
instead (below).
### TESTING INSTRUCTIONS
**Automated** (100 tests across 6 suites):
```bash
cd superset-frontend
npm run test -- \
src/dashboard/components/nativeFilters/FilterBar/FilterControls/GroupByFilterCard.test.tsx
\
src/dashboard/components/nativeFilters/useDisplayControlDatasource.test.ts
\
src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/utils.test.ts
\
src/hooks/apiResources/datasets.test.ts \
src/components/Chart/DrillDetail/DrillDetailPane.test.tsx \
src/components/Chart/ChartContextMenu/ChartContextMenu.test.tsx
```
Notable cases: the collision fixture (dataset 306 vs semantic view 306)
asserting the dataset endpoint is *never* called; the select-and-apply
persistence guard for the self-revert; the stale-response discard; and the four
pre-change characterization tests pinning the regular-dataset path.
**Manual** (requires a semantic-layer provider —
`FeatureFlag.SemanticLayers` enabled and a configured extension):
1. Create a semantic view whose numeric id collides with an existing regular
dataset (the sequences are independent, so this happens naturally as both
tables grow).
2. Add a chart on that semantic view to a dashboard, then add a **Dynamic
group by** display control bound to it.
3. The dropdown must list the semantic view's *dimensions*, not the
colliding dataset's columns.
4. Select a dimension, apply, then reopen the filter bar — the list must
still show dimensions (this is the path that previously regressed after the
first interaction).
5. Clear the customization and confirm the binding survives.
6. Right-click a chart on the semantic view → **Drill to detail**. The modal
header must show the semantic view's name; the metadata bar will show "Not
available" for Last modified / Modified by, which is expected.
Regression check on regular datasets: repeat steps 2-6 against a normal
dataset-backed chart; behaviour should be indistinguishable from master.
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [x] Required feature flags: `SemanticLayers`
- [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]
