You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by be...@apache.org on 2025/07/18 03:29:22 UTC
(superset) 04/09: WIP
This is an automated email from the ASF dual-hosted git repository.
beto pushed a commit to branch dbt-metricflow
in repository https://gitbox.apache.org/repos/asf/superset.git
commit e51b352cfffedbc0c533cdb644c4db821a69fd13
Author: Beto Dealmeida <ro...@dealmeida.net>
AuthorDate: Thu Jul 17 21:30:27 2025 -0400
WIP
---
.../src/shared-controls/dndControls.tsx | 1 +
.../DndColumnSelectControl/ColumnSelectPopover.tsx | 125 ++++++++++++++++++--
.../controls/SemanticLayerVerification.tsx | 131 +++++++++++++++++----
.../components/controls/withAsyncVerification.tsx | 9 +-
4 files changed, 234 insertions(+), 32 deletions(-)
diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/dndControls.tsx b/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/dndControls.tsx
index 4604d49d4c..1056a5414f 100644
--- a/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/dndControls.tsx
+++ b/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/dndControls.tsx
@@ -166,6 +166,7 @@ function enhanceControlWithSemanticLayer(
return {
...originalProps,
needAsyncVerification: needsVerification,
+ skipEffectVerification: true, // Custom flag to skip useEffect verification
form_data: state.form_data,
datasource: state.datasource, // Pass datasource to verification function
};
diff --git a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/ColumnSelectPopover.tsx b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/ColumnSelectPopover.tsx
index 089ae98e2c..d02dddbe37 100644
--- a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/ColumnSelectPopover.tsx
+++ b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/ColumnSelectPopover.tsx
@@ -26,7 +26,7 @@ import {
useRef,
useState,
} from 'react';
-import { useSelector } from 'react-redux';
+import { useSelector, useStore } from 'react-redux';
import {
AdhocColumn,
isAdhocColumn,
@@ -131,6 +131,7 @@ const ColumnSelectPopover = ({
const formData = useSelector<ExplorePageState, any>(
state => state.explore.form_data,
);
+ const store = useStore();
// Check if this is a semantic layer dataset
const isSemanticLayer = useMemo(() => {
@@ -166,6 +167,7 @@ const ColumnSelectPopover = ({
const [selectedTab, setSelectedTab] = useState<string | null>(null);
const [validDimensions, setValidDimensions] = useState<string[] | null>(null);
const [isLoadingValidDimensions, setIsLoadingValidDimensions] = useState(false);
+ const previousFormDataRef = useRef<string>('');
const [resizeButton, width, height] = useResizeButton(
POPOVER_INITIAL_WIDTH,
@@ -262,13 +264,77 @@ const ColumnSelectPopover = ({
}, [defaultActiveTabKey, getCurrentTab, setSelectedTab]);
// Fetch valid dimensions for semantic layer datasets
+ // Only trigger when actually needed (tab is Simple or modal opens after delay)
useEffect(() => {
- if (isSemanticLayer && formData && datasource) {
- setIsLoadingValidDimensions(true);
+ console.log('=== COLUMN MODAL EFFECT TRIGGER ===');
+ console.log('isSemanticLayer:', isSemanticLayer);
+ console.log('formData exists:', !!formData);
+ console.log('datasource exists:', !!datasource);
+ console.log('selectedTab:', selectedTab);
+ console.log('TABS_KEYS.SIMPLE:', TABS_KEYS.SIMPLE);
+ console.log('Should trigger API?', isSemanticLayer && formData && datasource &&
+ (selectedTab === TABS_KEYS.SIMPLE || selectedTab === null));
+
+ // Temporarily disable column modal API calls to isolate main verification timing issue
+ if (false && isSemanticLayer && formData && datasource &&
+ (selectedTab === TABS_KEYS.SIMPLE || selectedTab === null)) {
const fetchValidDimensions = async () => {
+ setIsLoadingValidDimensions(true);
+
try {
- const queryFields = collectQueryFields(formData);
+ // Wait for Redux state to settle after drag-and-drop operations
+ await new Promise(resolve => setTimeout(resolve, 500));
+
+ // Get the most current form data from store
+ const currentState = store.getState() as ExplorePageState;
+ let currentFormData = currentState.explore.form_data;
+
+ // If we're in a table and don't have metrics/dimensions, try to get from controls state
+ if ((!currentFormData.metrics && !currentFormData.groupby && !currentFormData.all_columns) ||
+ (Array.isArray(currentFormData.metrics) && currentFormData.metrics.length === 0 &&
+ Array.isArray(currentFormData.groupby) && currentFormData.groupby.length === 0)) {
+
+ // Try to get from the controls state instead
+ const controlsState = (currentState as any).explore?.controls;
+ if (controlsState) {
+ const enhancedFormData = { ...currentFormData };
+
+ // Get metrics from controls
+ if (controlsState.metrics?.value) {
+ enhancedFormData.metrics = controlsState.metrics.value;
+ }
+ if (controlsState.percent_metrics?.value) {
+ enhancedFormData.percent_metrics = controlsState.percent_metrics.value;
+ }
+
+ // Get dimensions from controls
+ if (controlsState.groupby?.value) {
+ enhancedFormData.groupby = controlsState.groupby.value;
+ }
+ if (controlsState.all_columns?.value) {
+ enhancedFormData.all_columns = controlsState.all_columns.value;
+ }
+
+ console.log('=== ENHANCED FORM DATA FROM CONTROLS ===');
+ console.log('Controls state metrics:', controlsState.metrics?.value);
+ console.log('Controls state groupby:', controlsState.groupby?.value);
+ console.log('Controls state all_columns:', controlsState.all_columns?.value);
+ console.log('Enhanced form data:', enhancedFormData);
+
+ currentFormData = enhancedFormData;
+ }
+ }
+
+ console.log('=== COLUMN MODAL DEBUG ===');
+ console.log('Column modal Redux state keys:', Object.keys(currentFormData));
+ console.log('Column modal form data:', currentFormData);
+ console.log('Column modal has metrics?', 'metrics' in currentFormData, currentFormData.metrics);
+ console.log('Column modal has groupby?', 'groupby' in currentFormData, currentFormData.groupby);
+ console.log('Column modal collected query fields:', collectQueryFields(currentFormData));
+ console.log('=== END COLUMN MODAL DEBUG ===');
+
+ const queryFields = collectQueryFields(currentFormData);
const validationResult = await callValidationAPI(
datasource,
queryFields.dimensions,
@@ -287,15 +353,58 @@ const ColumnSelectPopover = ({
}
};
- // Make it non-blocking by using setTimeout
- setTimeout(() => {
+ // Trigger API call after a delay to ensure state is current
+ const timeoutId = setTimeout(() => {
fetchValidDimensions();
- }, 0);
+ }, 100);
+
+ return () => clearTimeout(timeoutId);
} else {
setValidDimensions(null);
setIsLoadingValidDimensions(false);
}
- }, [isSemanticLayer, formData, datasource]);
+ }, [isSemanticLayer, selectedTab, datasource, store]);
+
+ // Also trigger when form data changes (for subsequent updates)
+ useEffect(() => {
+ if (isSemanticLayer && validDimensions !== null && formData && datasource) {
+ const currentFormDataString = JSON.stringify(formData);
+
+ // Only make API call if form data actually changed and we already have loaded once
+ if (currentFormDataString !== previousFormDataRef.current) {
+ previousFormDataRef.current = currentFormDataString;
+
+ const fetchValidDimensions = async () => {
+ setIsLoadingValidDimensions(true);
+
+ try {
+ await new Promise(resolve => setTimeout(resolve, 200));
+
+ const currentState = store.getState() as ExplorePageState;
+ const currentFormData = currentState.explore.form_data;
+
+ const queryFields = collectQueryFields(currentFormData);
+ const validationResult = await callValidationAPI(
+ datasource,
+ queryFields.dimensions,
+ queryFields.metrics,
+ );
+ if (validationResult) {
+ setValidDimensions(validationResult.dimensions);
+ }
+ } catch (error) {
+ console.warn('Failed to fetch valid dimensions:', error);
+ } finally {
+ setIsLoadingValidDimensions(false);
+ }
+ };
+
+ setTimeout(() => {
+ fetchValidDimensions();
+ }, 100);
+ }
+ }
+ }, [isSemanticLayer, formData, datasource, store, validDimensions]);
useEffect(() => {
/* if the adhoc column is not set (because it was never edited) but the
diff --git a/superset-frontend/src/explore/components/controls/SemanticLayerVerification.tsx b/superset-frontend/src/explore/components/controls/SemanticLayerVerification.tsx
index ca38fd2625..0c2022e381 100644
--- a/superset-frontend/src/explore/components/controls/SemanticLayerVerification.tsx
+++ b/superset-frontend/src/explore/components/controls/SemanticLayerVerification.tsx
@@ -47,6 +47,13 @@ export function collectQueryFields(formData: any): {
: [formData.columns]),
);
}
+ if (formData.all_columns) {
+ dimensions.push(
+ ...(Array.isArray(formData.all_columns)
+ ? formData.all_columns
+ : [formData.all_columns]),
+ );
+ }
if (formData.series_columns) {
dimensions.push(
...(Array.isArray(formData.series_columns)
@@ -216,17 +223,58 @@ export function createMetricsVerification(controlName?: string): AsyncVerify {
return null;
}
- // Create updated form data with the current value
- const updatedFormData = { ...form_data };
+ // Stop trying to get "current" state - just use the value we have and minimal other state
+ // The issue is that verification happens immediately while Redux is updating
+ // So let's just send the minimal needed data: current control value + some basic context
- // Update the appropriate field based on the control name
- // Handle both addition and removal of values (including empty arrays)
- if (controlName) {
- updatedFormData[controlName] = value;
+ const queryFields = {
+ dimensions: [] as string[],
+ metrics: [] as string[]
+ };
+
+ // Add the current value being set
+ if (controlName && value !== undefined && value !== null) {
+ if (['metrics', 'metric', 'metric_2', 'percent_metrics', 'timeseries_limit_metric', 'x', 'y', 'size', 'secondary_metric'].includes(controlName)) {
+ if (Array.isArray(value)) {
+ queryFields.metrics.push(...value.filter(v => v != null).map(v => typeof v === 'string' ? v : (v as any)?.metric_name || String(v)));
+ } else {
+ const metricName = typeof value === 'string' ? value : (value as any)?.metric_name || String(value);
+ if (metricName) queryFields.metrics.push(metricName);
+ }
+ } else if (['groupby', 'columns', 'all_columns', 'series_columns', 'series', 'entity', 'x_axis'].includes(controlName)) {
+ if (Array.isArray(value)) {
+ queryFields.dimensions.push(...value.filter(v => v != null).map(v => typeof v === 'string' ? v : (v as any)?.column_name || String(v)));
+ } else {
+ const dimensionName = typeof value === 'string' ? value : (value as any)?.column_name || String(value);
+ if (dimensionName) queryFields.dimensions.push(dimensionName);
+ }
+ }
}
- // Extract current form fields from updated form data
- const queryFields = collectQueryFields(updatedFormData || {});
+ // Only add a minimal set of other values - don't try to be complete since that's causing timing issues
+ // Just include basic metric/dimension fields that are commonly used
+ if (form_data) {
+ // Add basic metrics that are commonly present
+ if (form_data.metrics && controlName !== 'metrics') {
+ const metrics = Array.isArray(form_data.metrics) ? form_data.metrics : [form_data.metrics];
+ queryFields.metrics.push(...metrics.filter(v => v != null).map(v => typeof v === 'string' ? v : (v as any)?.metric_name || String(v)));
+ }
+
+ // Add basic dimensions that are commonly present
+ if (form_data.groupby && controlName !== 'groupby') {
+ const dimensions = Array.isArray(form_data.groupby) ? form_data.groupby : [form_data.groupby];
+ queryFields.dimensions.push(...dimensions.filter(v => v != null).map(v => typeof v === 'string' ? v : (v as any)?.column_name || String(v)));
+ }
+ }
+
+ // Remove duplicates
+ queryFields.dimensions = [...new Set(queryFields.dimensions)];
+ queryFields.metrics = [...new Set(queryFields.metrics)];
+
+ console.log('=== METRICS VERIFICATION SIMPLIFIED ===');
+ console.log('Control name:', controlName);
+ console.log('Current value:', value);
+ console.log('Simplified query fields:', queryFields);
// Call validation API
console.log('Metrics verification API call:', {
@@ -329,27 +377,65 @@ export function createColumnsVerification(controlName?: string): AsyncVerify {
return null;
}
- // Create updated form data with the current value
- const updatedFormData = { ...form_data };
+ // Stop trying to get "current" state - just use the value we have and minimal other state
+ // The issue is that verification happens immediately while Redux is updating
+ // So let's just send the minimal needed data: current control value + some basic context
- // Update the appropriate field based on the control name
- // Handle both addition and removal of values (including empty arrays)
- if (controlName) {
- updatedFormData[controlName] = value;
+ const queryFields = {
+ dimensions: [] as string[],
+ metrics: [] as string[]
+ };
+
+ // Add the current value being set
+ if (controlName && value !== undefined && value !== null) {
+ if (['metrics', 'metric', 'metric_2', 'percent_metrics', 'timeseries_limit_metric', 'x', 'y', 'size', 'secondary_metric'].includes(controlName)) {
+ if (Array.isArray(value)) {
+ queryFields.metrics.push(...value.filter(v => v != null).map(v => typeof v === 'string' ? v : (v as any)?.metric_name || String(v)));
+ } else {
+ const metricName = typeof value === 'string' ? value : (value as any)?.metric_name || String(value);
+ if (metricName) queryFields.metrics.push(metricName);
+ }
+ } else if (['groupby', 'columns', 'all_columns', 'series_columns', 'series', 'entity', 'x_axis'].includes(controlName)) {
+ if (Array.isArray(value)) {
+ queryFields.dimensions.push(...value.filter(v => v != null).map(v => typeof v === 'string' ? v : (v as any)?.column_name || String(v)));
+ } else {
+ const dimensionName = typeof value === 'string' ? value : (value as any)?.column_name || String(value);
+ if (dimensionName) queryFields.dimensions.push(dimensionName);
+ }
+ }
+ }
+
+ // Only add a minimal set of other values - don't try to be complete since that's causing timing issues
+ // Just include basic metric/dimension fields that are commonly used
+ if (form_data) {
+ // Add basic metrics that are commonly present
+ if (form_data.metrics && controlName !== 'metrics') {
+ const metrics = Array.isArray(form_data.metrics) ? form_data.metrics : [form_data.metrics];
+ queryFields.metrics.push(...metrics.filter(v => v != null).map(v => typeof v === 'string' ? v : (v as any)?.metric_name || String(v)));
+ }
+
+ // Add basic dimensions that are commonly present
+ if (form_data.groupby && controlName !== 'groupby') {
+ const dimensions = Array.isArray(form_data.groupby) ? form_data.groupby : [form_data.groupby];
+ queryFields.dimensions.push(...dimensions.filter(v => v != null).map(v => typeof v === 'string' ? v : (v as any)?.column_name || String(v)));
+ }
}
- // Extract current form fields from updated form data
- const queryFields = collectQueryFields(updatedFormData || {});
+ // Remove duplicates
+ queryFields.dimensions = [...new Set(queryFields.dimensions)];
+ queryFields.metrics = [...new Set(queryFields.metrics)];
+
+ console.log('=== COLUMNS VERIFICATION SIMPLIFIED ===');
+ console.log('Control name:', controlName);
+ console.log('Current value:', value);
+ console.log('Current value type:', typeof value, Array.isArray(value));
+ console.log('Current value length:', Array.isArray(value) ? value.length : 'N/A');
+ console.log('Simplified query fields:', queryFields);
+ console.log('Stack trace:', new Error().stack);
// Call validation API
console.log('Columns verification API call:', {
controlName,
- originalFormData: form_data,
- updatedFormData,
- value,
- valueType: typeof value,
- isArray: Array.isArray(value),
- valueLength: Array.isArray(value) ? value.length : 'N/A',
dimensions: queryFields.dimensions,
metrics: queryFields.metrics,
});
@@ -516,6 +602,7 @@ export const SEMANTIC_LAYER_CONTROL_FIELDS = [
// Dimension controls
'groupby',
'columns',
+ 'all_columns',
'series_columns',
'series',
'entity',
diff --git a/superset-frontend/src/explore/components/controls/withAsyncVerification.tsx b/superset-frontend/src/explore/components/controls/withAsyncVerification.tsx
index 5c45427247..92daa2ec73 100644
--- a/superset-frontend/src/explore/components/controls/withAsyncVerification.tsx
+++ b/superset-frontend/src/explore/components/controls/withAsyncVerification.tsx
@@ -54,6 +54,10 @@ export type FullControlProps = ControlPropsWithExtras & {
* An extra flag for triggering async verification. Set it in mapStateToProps.
*/
needAsyncVerification?: boolean;
+ /**
+ * Whether to skip useEffect verification and only use onChange verification.
+ */
+ skipEffectVerification?: boolean;
/**
* Whether to show loading state when verification is still loading.
*/
@@ -136,6 +140,7 @@ export default function withAsyncVerification({
hovered,
onChange: basicOnChange,
needAsyncVerification = false,
+ skipEffectVerification = false,
isLoading: initialIsLoading = false,
showLoadingState = defaultShowLoadingState,
verify = defaultVerify,
@@ -212,7 +217,7 @@ export default function withAsyncVerification({
);
useEffect(() => {
- if (needAsyncVerification && verify) {
+ if (needAsyncVerification && verify && !skipEffectVerification) {
// Skip verification if it was just triggered by onChange
if (verificationTriggeredByChange.current) {
verificationTriggeredByChange.current = false;
@@ -220,7 +225,7 @@ export default function withAsyncVerification({
}
verifyProps(verify, otherProps);
}
- }, [needAsyncVerification, verify, otherProps, verifyProps]);
+ }, [needAsyncVerification, verify, otherProps, verifyProps, skipEffectVerification]);
return (
<ControlComponent