mikebridge commented on code in PR #44851:
URL: https://github.com/apache/superset/pull/44851#discussion_r4167847085
##########
superset-frontend/src/explore/actions/exploreActions.ts:
##########
@@ -223,8 +233,8 @@ export function fetchCompatibility(
selected_dimensions: selectedDimensions,
},
});
- if (requestSeq !== compatibilityRequestSeq) {
- return;
+ if (requestSeq !== compatibilityRequestSeq || !isCurrent()) {
Review Comment:
Good catch, thanks. Fixed in `eb88f4ee8e`: the compatibility request is now
tied to the active datasource instead of the editor session, so its response
settles after the modal closes; a datasource change or a newer request still
retires it. The deferred-POST tests close the editor after loading starts and
cover both the success and the failure outcome; both failed before the change.
This is verified in Jest against a real store, not in a browser.
##########
superset-frontend/src/explore/actions/exploreActions.ts:
##########
@@ -272,6 +284,75 @@ export function syncDatasourceMetadata(datasource:
Dataset) {
return { type: SYNC_DATASOURCE_METADATA, datasource };
}
+export const SYNC_SEMANTIC_METADATA = 'explore/SYNC_SEMANTIC_METADATA';
+/** Rebuild metadata-derived controls without recording a chart edit. */
+export function syncSemanticMetadata(
+ datasource: Dataset,
+ formData: QueryFormData,
+): {
+ type: typeof SYNC_SEMANTIC_METADATA;
+ datasource: Dataset;
+ formData: QueryFormData;
+} {
+ return { type: SYNC_SEMANTIC_METADATA, datasource, formData };
+}
+
+/** Refresh the active view's metadata without saving or running the chart. */
+export function refreshSemanticMetadata(
+ viewId: number,
+ sessionIsCurrent: () => boolean,
+) {
+ return async (
+ dispatch: Dispatch,
+ getState: () => Pick<ExplorePageState, 'explore'>,
+ ) => {
+ const isCurrent = () => {
+ const { datasource } = getState().explore;
+ return (
+ sessionIsCurrent() &&
+ Number(datasource.id) === viewId &&
+ String(datasource.type) === 'semantic_view'
+ );
+ };
+ if (!isCurrent()) return;
+ // A pre-sync compatibility response cannot replace a post-sync answer.
+ compatibilityRequestSeq += 1;
+ const { json } = await SupersetClient.get({
+ endpoint:
`/fetch_datasource_metadata?datasourceKey=${viewId}__semantic_view`,
+ });
+ if (!isCurrent()) return;
+ const formData = getFormDataFromControls(getState().explore.controls);
+ // Rebuild the controls against fresh fields using their existing values
and
+ // normal removed-member validation, without rewriting form_data or
querying.
+ dispatch(syncSemanticMetadata(json as Dataset, formData));
+ const selectedMetrics = [
+ ...new Set(
+ [...ensureIsArray(formData.metrics), formData.metric].filter(
Review Comment:
Agreed. `eb88f4ee8e` moves the selection extraction into one helper
(`getCompatibilitySelection`) used by both the metadata sync and the ongoing
Explore effect, and the effect now also depends on the singleton `metric`. A
real-store test clears metric A on a Big Number chart: before the change no
request was sent; after it an empty-selection request goes out and B is
selectable again. Not walked through against a live provider.
##########
superset-frontend/src/explore/actions/exploreActions.ts:
##########
@@ -272,6 +284,75 @@ export function syncDatasourceMetadata(datasource:
Dataset) {
return { type: SYNC_DATASOURCE_METADATA, datasource };
}
+export const SYNC_SEMANTIC_METADATA = 'explore/SYNC_SEMANTIC_METADATA';
+/** Rebuild metadata-derived controls without recording a chart edit. */
+export function syncSemanticMetadata(
+ datasource: Dataset,
+ formData: QueryFormData,
+): {
+ type: typeof SYNC_SEMANTIC_METADATA;
+ datasource: Dataset;
+ formData: QueryFormData;
+} {
+ return { type: SYNC_SEMANTIC_METADATA, datasource, formData };
+}
+
+/** Refresh the active view's metadata without saving or running the chart. */
+export function refreshSemanticMetadata(
+ viewId: number,
+ sessionIsCurrent: () => boolean,
+) {
+ return async (
+ dispatch: Dispatch,
+ getState: () => Pick<ExplorePageState, 'explore'>,
+ ) => {
+ const isCurrent = () => {
+ const { datasource } = getState().explore;
+ return (
+ sessionIsCurrent() &&
+ Number(datasource.id) === viewId &&
+ String(datasource.type) === 'semantic_view'
+ );
+ };
+ if (!isCurrent()) return;
+ // A pre-sync compatibility response cannot replace a post-sync answer.
+ compatibilityRequestSeq += 1;
+ const { json } = await SupersetClient.get({
+ endpoint:
`/fetch_datasource_metadata?datasourceKey=${viewId}__semantic_view`,
+ });
+ if (!isCurrent()) return;
+ const formData = getFormDataFromControls(getState().explore.controls);
+ // Rebuild the controls against fresh fields using their existing values
and
+ // normal removed-member validation, without rewriting form_data or
querying.
+ dispatch(syncSemanticMetadata(json as Dataset, formData));
+ const selectedMetrics = [
+ ...new Set(
+ [...ensureIsArray(formData.metrics), formData.metric].filter(
+ (value): value is string => typeof value === 'string',
+ ),
+ ),
+ ];
+ const selectedDimensions = [
+ ...new Set(
+ [
+ ...ensureIsArray(formData.groupby),
+ ...ensureIsArray(formData.columns),
+ formData.x_axis,
+ ].filter((value): value is string => typeof value === 'string'),
+ ),
+ ];
+ const verified = await fetchCompatibility(
+ 'semantic_view',
+ viewId,
+ selectedMetrics,
+ selectedDimensions,
+ isCurrent,
+ )(dispatch);
+ if (verified === false && isCurrent())
Review Comment:
Thanks, added in `eb88f4ee8e` as you described: `SupersetClient.post`
rejects and `refreshSemanticMetadata(7, () => true)(...)` must reject. It
passes with the check, fails with "Received promise resolved instead of
rejected" when the check is deleted, and passes again once restored.
--
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]
