EnxDev commented on code in PR #45176:
URL: https://github.com/apache/superset/pull/45176#discussion_r4403227123
##########
superset-frontend/src/pages/DatasetList/index.tsx:
##########
@@ -542,7 +546,7 @@ const DatasetList: FunctionComponent<DatasetListProps> = ({
sourceTypeFilter?.value && typeof sourceTypeFilter.value === 'object'
? (sourceTypeFilter.value as { value: string }).value
: (sourceTypeFilter?.value as string | undefined);
- if (sourceTypeValue) {
+ if (sourceTypeValue && semanticLayersEnabled) {
Review Comment:
This keeps `source_type` out of the request, but it stays in
`lastFetchConfig.filters`, so `currentSourceFilter` still reads
`semantic_layer`. In `fetchConnectionOptions` that sets `showDatabases` to
false, and with the flag off `showSemanticLayers` is false too. The Database
dropdown opens empty until some other filter change drops the URL value.
`clearFilterById('source_type')` can't help here, since there's no
configured filter with that id when the flag is off. Could
`currentSourceFilter` return `''` when `SEMANTIC_LAYERS` is off? The existing
saved-URL test could also assert that the Database select still lists databases.
##########
superset-frontend/src/pages/DatasetList/index.tsx:
##########
@@ -517,6 +517,10 @@ const DatasetList: FunctionComponent<DatasetListProps> = ({
setLastFetchConfig(config);
setLoading(true);
const { pageIndex, pageSize, sortBy, filters: filterValues } = config;
+ // The Source control and the semantic-layer connections only exist with
+ // SEMANTIC_LAYERS on, but a saved URL can still carry them. The dataset
API
+ // rejects both, and with the control hidden the user could not clear
them.
+ const semanticLayersEnabled = isFeatureEnabled(SEMANTIC_LAYERS_FLAG);
Review Comment:
A saved `Type = Semantic View` (`sql:semantic_view`) also reaches this point
with the flag off, and it goes straight through `otherFilters` to
`/api/v1/dataset/`. `DatasetIsNullOrEmptyFilter` reads the truthy string as
`true`, so I'd expect the list to show physical datasets under a Type pill
whose value isn't one of the two options.
Might be misreading how the select renders an unknown value, but it seems
worth dropping `semantic_view` here like the `sl:` connection, and covering it
with a third saved-URL test.
--
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]
