fix(table-viz): table chart time column should use default (#10293)

superset-frontend/cypress-base/cypress/integration/explore/visualizations/line.test.ts

@@ -45,6 +45,13 @@ describe('Visualization > Line', () => {
     cy.get('.alert-warning').should('not.exist');
   });
 
+  it('should allow negative values in Y bounds', () => {
+    cy.get('#controlSections-tab-display').click();
+    cy.get('span').contains('Y Axis Bounds').scrollIntoView();
+    cy.get('input[placeholder="Min"]').type('-0.1', { delay: 100 });
+    cy.get('.alert-warning').should('not.exist');
+  });
+
   it('should work with adhoc metric', () => {
     const formData = { ...LINE_CHART_DEFAULTS, metrics: [NUM_METRIC] };
     cy.visitChartByParams(JSON.stringify(formData));
@@ -54,9 +61,7 @@ describe('Visualization > Line', () => {
   it('should work with groupby', () => {
     const metrics = ['count'];
     const groupby = ['gender'];
-
     const formData = { ...LINE_CHART_DEFAULTS, metrics, groupby };
-
     cy.visitChartByParams(JSON.stringify(formData));
     cy.verifySliceSuccess({ waitAlias: '@getJson', chartSelector: 'svg' });
   });
@@ -64,13 +69,11 @@ describe('Visualization > Line', () => {
   it('should work with simple filter', () => {
     const metrics = ['count'];
     const filters = [SIMPLE_FILTER];
-
     const formData = {
       ...LINE_CHART_DEFAULTS,
       metrics,
       adhoc_filters: filters,
     };
-
     cy.visitChartByParams(JSON.stringify(formData));
     cy.verifySliceSuccess({ waitAlias: '@getJson', chartSelector: 'svg' });
   });
@@ -83,7 +86,6 @@ describe('Visualization > Line', () => {
       groupby: ['name'],
       timeseries_limit_metric: NUM_METRIC,
     };
-
     cy.visitChartByParams(JSON.stringify(formData));
     cy.verifySliceSuccess({ waitAlias: '@getJson', chartSelector: 'svg' });
   });
@@ -97,28 +99,24 @@ describe('Visualization > Line', () => {
       timeseries_limit_metric: NUM_METRIC,
       order_desc: true,
     };
-
     cy.visitChartByParams(JSON.stringify(formData));
     cy.verifySliceSuccess({ waitAlias: '@getJson', chartSelector: 'svg' });
   });
 
   it('should work with rolling avg', () => {
     const metrics = [NUM_METRIC];
-
     const formData = {
       ...LINE_CHART_DEFAULTS,
       metrics,
       rolling_type: 'mean',
       rolling_periods: 10,
     };
-
     cy.visitChartByParams(JSON.stringify(formData));
     cy.verifySliceSuccess({ waitAlias: '@getJson', chartSelector: 'svg' });
   });
 
   it('should work with time shift 1 year', () => {
     const metrics = [NUM_METRIC];
-
     const formData = {
       ...LINE_CHART_DEFAULTS,
       metrics,
@@ -162,28 +160,24 @@ describe('Visualization > Line', () => {
 
   it('should work with time shift yoy', () => {
     const metrics = [NUM_METRIC];
-
     const formData = {
       ...LINE_CHART_DEFAULTS,
       metrics,
       time_compare: ['1+year'],
       comparison_type: 'ratio',
     };
-
     cy.visitChartByParams(JSON.stringify(formData));
     cy.verifySliceSuccess({ waitAlias: '@getJson', chartSelector: 'svg' });
   });
 
   it('should work with time shift percentage change', () => {
     const metrics = [NUM_METRIC];
-
     const formData = {
       ...LINE_CHART_DEFAULTS,
       metrics,
       time_compare: ['1+year'],
       comparison_type: 'percentage',
     };
-
     cy.visitChartByParams(JSON.stringify(formData));
     cy.verifySliceSuccess({ waitAlias: '@getJson', chartSelector: 'svg' });
   });
@@ -193,7 +187,6 @@ describe('Visualization > Line', () => {
       ...LINE_CHART_DEFAULTS,
       metrics: ['count'],
     };
-
     cy.visitChartByParams(JSON.stringify(formData));
     cy.verifySliceSuccess({ waitAlias: '@getJson', chartSelector: 'svg' });
     cy.get('text.nv-legend-text').contains('COUNT(*)');
@@ -224,7 +217,6 @@ describe('Visualization > Line', () => {
         },
       ],
     };
-
     cy.visitChartByParams(JSON.stringify(formData));
     cy.verifySliceSuccess({ waitAlias: '@getJson', chartSelector: 'svg' });
     cy.get('.slice_container').within(() => {
@@ -263,7 +255,6 @@ describe('Visualization > Line', () => {
         cy.visitChartByParams(JSON.stringify(formData));
       },
     );
-
     cy.verifySliceSuccess({ waitAlias: '@getJson', chartSelector: 'svg' });
     cy.get('.slice_container').within(() => {
       cy.get('.nv-event-annotation-layer-0')

superset-frontend/cypress-base/cypress/integration/explore/visualizations/table.test.ts

@@ -35,6 +35,15 @@ describe('Visualization > Table', () => {
     cy.route('POST', '/superset/explore_json/**').as('getJson');
   });
 
+  it('Use default time column', () => {
+    cy.visitChartByParams({
+      ...VIZ_DEFAULTS,
+      granularity_sqla: undefined,
+      metrics: ['count'],
+    });
+    cy.get('input[name="select-granularity_sqla"]').should('have.value', 'ds');
+  });
+
   it('Format non-numeric metrics correctly', () => {
     cy.visitChartByParams({
       ...VIZ_DEFAULTS,

superset-frontend/spec/javascripts/explore/controlUtils_spec.jsx

@@ -35,6 +35,7 @@ describe('controlUtils', () => {
       columns: ['a', 'b', 'c'],
       metrics: [{ metric_name: 'first' }, { metric_name: 'second' }],
     },
+    controls: {},
   };
 
   beforeAll(() => {
@@ -250,9 +251,10 @@ describe('controlUtils', () => {
     it('should not apply mapStateToProps when initializing', () => {
       const control = getControlState('metrics', 'table', {
         ...state,
-        isInitializing: true,
+        controls: undefined,
       });
-      expect(control.default).toEqual(null);
+      expect(typeof control.default).toBe('function');
+      expect(control.value).toBe(undefined);
     });
   });
 
@@ -261,6 +263,15 @@ describe('controlUtils', () => {
       const control = getControlState('metric', 'table', state, null);
       expect(control.validationErrors).toEqual(['cannot be empty']);
     });
+    it('should not validate if control panel is initializing', () => {
+      const control = getControlState(
+        'metric',
+        'table',
+        { ...state, controls: undefined },
+        undefined,
+      );
+      expect(control.validationErrors).toBeUndefined();
+    });
   });
 
   describe('queryFields', () => {

superset-frontend/src/explore/components/ControlPanelsContainer.jsx

@@ -66,15 +66,14 @@ class ControlPanelsContainer extends React.Component {
   renderControl({ name, config }) {
     const { actions, controls, exploreState, form_data: formData } = this.props;
     const { visibility } = config;
+
     // If the control item is not an object, we have to look up the control data from
     // the centralized controls file.
     // When it is an object we read control data straight from `config` instead
     const controlData = {
-      ...controls[name],
       ...config,
+      ...controls[name],
       name,
-      // apply current value in formData
-      value: formData[name],
     };
     const {
       validationErrors,

superset-frontend/src/explore/controlUtils.js

@@ -36,18 +36,17 @@ export function getFormDataFromControls(controlsState) {
 
 export function validateControl(control, processedState) {
   const validators = control.validators;
+  const validationErrors = [];
   if (validators && validators.length > 0) {
-    const validatedControl = { ...control };
-    const validationErrors = [];
     validators.forEach(f => {
       const v = f.call(control, control.value, processedState);
       if (v) {
         validationErrors.push(v);
       }
     });
-    return { ...validatedControl, validationErrors };
   }
-  return control;
+  // always reset validation errors even when there is no validator
+  return { ...control, validationErrors };
 }
 
 /**
@@ -88,17 +87,6 @@ export const getControlConfig = memoizeOne(function getControlConfig(
   return control?.config || control;
 });
 
-export function applyMapStateToPropsToControl(controlState, controlPanelState) {
-  const { mapStateToProps } = controlState;
-  if (mapStateToProps && controlPanelState) {
-    return {
-      ...controlState,
-      ...mapStateToProps(controlPanelState, controlState),
-    };
-  }
-  return controlState;
-}
-
 function handleMissingChoice(control) {
   // If the value is not valid anymore based on choices, clear it
   const value = control.value;
@@ -121,6 +109,36 @@ function handleMissingChoice(control) {
   return control;
 }
 
+export function applyMapStateToPropsToControl(controlState, controlPanelState) {
+  const { mapStateToProps } = controlState;
+  let { value } = controlState;
+  let state = { ...controlState };
+  if (mapStateToProps && controlPanelState) {
+    state = {
+      ...controlState,
+      ...mapStateToProps(controlPanelState, controlState),
+    };
+  }
+  // If default is a function, evaluate it
+  if (typeof state.default === 'function') {
+    state.default = state.default(state, controlPanelState);
+    // if default is still a function, discard
+    if (typeof state.default === 'function') {
+      delete state.default;
+    }
+  }
+  // If no current value, set it as default
+  if (state.default && value === undefined) {
+    value = state.default;
+  }
+  // If a choice control went from multi=false to true, wrap value in array
+  if (value && state.multi && !Array.isArray(value)) {
+    value = [value];
+  }
+  state.value = value;
+  return validateControl(handleMissingChoice(state), state);
+}
+
 export function getControlStateFromControlConfig(
   controlConfig,
   controlPanelState,
@@ -130,38 +148,16 @@ export function getControlStateFromControlConfig(
   if (!controlConfig) {
     return null;
   }
-  let controlState = { ...controlConfig };
+  const controlState = { ...controlConfig, value };
   // only apply mapStateToProps when control states have been initialized
+  // or when explicitly didn't provide control panel state (mostly for testing)
   if (
-    controlPanelState &&
-    (controlPanelState.controls || !controlPanelState.isInitializing)
+    (controlPanelState && controlPanelState.controls) ||
+    controlPanelState === null
   ) {
-    controlState = applyMapStateToPropsToControl(
-      controlState,
-      controlPanelState,
-    );
+    return applyMapStateToPropsToControl(controlState, controlPanelState);
   }
-
-  // If default is a function, evaluate it
-  if (typeof controlState.default === 'function') {
-    controlState.default = controlState.default(
-      controlState,
-      controlPanelState,
-    );
-    // if default is still a function, discard
-    if (typeof controlState.default === 'function') {
-      delete controlState.default;
-    }
-  }
-
-  // If a choice control went from multi=false to true, wrap value in array
-  const controlValue =
-    controlConfig.multi && value && !Array.isArray(value) ? [value] : value;
-
-  controlState.value =
-    typeof controlValue === 'undefined' ? controlState.default : controlValue;
-
-  return validateControl(handleMissingChoice(controlState), controlState);
+  return controlState;
 }
 
 export function getControlState(controlKey, vizType, state, value) {

superset-frontend/src/explore/reducers/getInitialState.js

@@ -41,7 +41,6 @@ export default function getInitialState(bootstrapData) {
     filterColumnOpts: [],
     isDatasourceMetaLoading: false,
     isStarred: false,
-    isInitializing: true,
   };
   const controls = getControlsState(bootstrappedState, rawFormData);
   bootstrappedState.controls = controls;
@@ -55,7 +54,6 @@ export default function getInitialState(bootstrapData) {
       bootstrappedState,
     );
   });
-  bootstrappedState.isInitializing = false;
 
   const sliceFormData = slice
     ? getFormDataFromControls(getControlsState(bootstrapData, slice.form_data))