geido commented on code in PR #43719:
URL: https://github.com/apache/superset/pull/43719#discussion_r3916107899
##########
superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx:
##########
@@ -196,13 +196,22 @@ const SliceHeaderControls = (
.get(props.slice.viz_type)
?.behaviors?.includes(Behavior.InteractiveChart);
const canExplore = props.supersetCanExplore;
- const { canDrillToDetail, canViewQuery, canViewTable } = usePermissions();
+ const { canDrillToDetail, canGetDrillInfo, canViewQuery, canViewTable } =
+ usePermissions();
+ // Single predicate for the "View as table" entry, so the fetch that feeds
its
+ // column headers cannot drift from the set of users who can open it.
+ const canViewResultsTable = canExplore || canViewTable;
+
+ // The dataset's verbose map resolves friendly Labels for both the
drill-to-detail
+ // pane and the results grid, and those are separate permissions — so fetch
it for
+ // either one, as long as the drill_info endpoint itself is readable (it is
gated
+ // by `can_get_drill_info` on Dataset).
const datasetResource = useDatasetDrillInfo(
props.slice.datasource,
props.dashboardId,
props.formData,
Review Comment:
Verified and fixed — and it's wider than View as table. Both
`DrillDetailModal` call sites get their column headers from this same
`verbose_map` (`DrillDetailPane` does `dataset?.verbose_map?.[column] ||
column`), so in extension deployments drill-to-detail headers were incomplete
too.
Fixed in the hook rather than at this call site, so both consumers get it.
The extension keeps supplying drill-by options; labels always come from the
REST endpoint:
```ts
if (loadDrillByOptionsExtension && formData) {
const response = await loadDrillByOptionsExtension(numericDatasetId,
formData);
result = response?.json?.result;
try {
labelSource = await fetchDrillInfo();
} catch {
labelSource = result;
}
} else {
result = await fetchDrillInfo();
labelSource = result;
}
const verbose_map = createVerboseMap(labelSource);
```
I went with your "fallback" option rather than a hard separate fetch, in
that direction: a deployment may register the extension *because* the REST
endpoint isn't reachable for it, and drill-by works there today. So a failed
label fetch degrades to the extension-derived map instead of failing the
resource and taking drill-by down with it.
Cost: in extension deployments this is one additional REST call per dataset
per dashboard load. In non-extension deployments nothing changes — same single
call, still deduped by `cachedSupersetGet`'s endpoint key.
Three tests in `datasets.test.ts`:
- `labels come from the API even when the extension supplies drill-by
options` — extension returns dimensions only, API returns metrics plus a
non-dimension column; asserts `verbose_map` covers all three while `columns`
still come from the extension
- `keeps extension drill-by options when the label fetch fails` — API
rejects; asserts the resource stays `complete` with the extension's columns
- the existing extension test lost its
`expect(mockedCachedSupersetGet).not.toHaveBeenCalled()`, since that assertion
encoded exactly the behaviour you flagged
First two verified failing against the previous code.
--
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]
