diff --git a/superset-frontend/.eslintrc.js b/superset-frontend/.eslintrc.js index 469709528089..5b79778ef525 100644 --- a/superset-frontend/.eslintrc.js +++ b/superset-frontend/.eslintrc.js @@ -104,6 +104,7 @@ module.exports = { 'no-mixed-operators': 0, 'no-multi-assign': 0, 'no-multi-spaces': 0, + 'no-nested-ternary': 0, 'no-prototype-builtins': 0, 'no-restricted-properties': 0, 'no-restricted-imports': [ @@ -226,6 +227,7 @@ module.exports = { 'no-mixed-operators': 0, 'no-multi-assign': 0, 'no-multi-spaces': 0, + 'no-nested-ternary': 0, 'no-prototype-builtins': 0, 'no-restricted-properties': 0, 'no-restricted-imports': [ diff --git a/superset-frontend/cypress-base/cypress/integration/explore/advanced.test.ts b/superset-frontend/cypress-base/cypress/integration/explore/advanced_analytics.test.ts similarity index 63% rename from superset-frontend/cypress-base/cypress/integration/explore/advanced.test.ts rename to superset-frontend/cypress-base/cypress/integration/explore/advanced_analytics.test.ts index 066ae868a195..77ebfbe7f9a4 100644 --- a/superset-frontend/cypress-base/cypress/integration/explore/advanced.test.ts +++ b/superset-frontend/cypress-base/cypress/integration/explore/advanced_analytics.test.ts @@ -55,44 +55,3 @@ describe('Advanced analytics', () => { .contains('1 year'); }); }); - -describe('Annotations', () => { - beforeEach(() => { - cy.login(); - cy.intercept('GET', '/superset/explore_json/**').as('getJson'); - cy.intercept('POST', '/superset/explore_json/**').as('postJson'); - }); - - it('Create formula annotation y-axis goal line', () => { - cy.visitChartByName('Num Births Trend'); - cy.verifySliceSuccess({ waitAlias: '@postJson' }); - - cy.get('[data-test=annotation_layers]').within(() => { - cy.get('button').click(); - }); - - cy.get('[data-test="popover-content"]').within(() => { - cy.get('[data-test=annotation-layer-name-header]') - .siblings() - .first() - .within(() => { - cy.get('input').type('Goal line'); - }); - cy.get('[data-test=annotation-layer-value-header]') - .siblings() - .first() - .within(() => { - cy.get('input').type('y=1400000'); - }); - cy.get('button').contains('OK').click({ force: true }); - }); - - cy.get('button[data-test="run-query-button"]').click(); - cy.verifySliceSuccess({ - waitAlias: '@postJson', - chartSelector: 'svg', - }); - - cy.get('.nv-legend-text').should('have.length', 2); - }); -}); diff --git a/superset-frontend/cypress-base/cypress/integration/explore/annotations.test.ts b/superset-frontend/cypress-base/cypress/integration/explore/annotations.test.ts new file mode 100644 index 000000000000..ebbaddf72dbc --- /dev/null +++ b/superset-frontend/cypress-base/cypress/integration/explore/annotations.test.ts @@ -0,0 +1,59 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +describe('Annotations', () => { + beforeEach(() => { + cy.login(); + cy.intercept('GET', '/superset/explore_json/**').as('getJson'); + cy.intercept('POST', '/superset/explore_json/**').as('postJson'); + }); + + it('Create formula annotation y-axis goal line', () => { + cy.visitChartByName('Num Births Trend'); + cy.verifySliceSuccess({ waitAlias: '@postJson' }); + + const layerLabel = 'Goal line'; + + cy.get('[data-test=annotation_layers] button').click(); + + cy.get('[data-test="popover-content"]').within(() => { + cy.get('[data-test=annotation-layer-name-header]') + .siblings() + .first() + .within(() => { + cy.get('input').type(layerLabel); + }); + cy.get('[data-test=annotation-layer-value-header]') + .siblings() + .first() + .within(() => { + cy.get('input').type('y=1400000'); + }); + cy.get('button').contains('OK').click(); + }); + + cy.get('button[data-test="run-query-button"]').click(); + cy.get('[data-test=annotation_layers]').contains(layerLabel); + + cy.verifySliceSuccess({ + waitAlias: '@postJson', + chartSelector: 'svg', + }); + cy.get('.nv-legend-text').should('have.length', 2); + }); +}); diff --git a/superset-frontend/cypress-base/cypress/integration/explore/visualizations/pivot_table.test.js b/superset-frontend/cypress-base/cypress/integration/explore/visualizations/pivot_table.test.js index 1346b89bebef..14de08da7936 100644 --- a/superset-frontend/cypress-base/cypress/integration/explore/visualizations/pivot_table.test.js +++ b/superset-frontend/cypress-base/cypress/integration/explore/visualizations/pivot_table.test.js @@ -28,7 +28,7 @@ describe('Visualization > Pivot Table', () => { adhoc_filters: [], groupby: ['name'], columns: ['state'], - row_limit: 50000, + row_limit: 5000, pandas_aggfunc: 'sum', pivot_margins: true, number_format: '.3s', diff --git a/superset-frontend/spec/javascripts/explore/components/ControlPanelsContainer_spec.jsx b/superset-frontend/spec/javascripts/explore/components/ControlPanelsContainer_spec.tsx similarity index 86% rename from superset-frontend/spec/javascripts/explore/components/ControlPanelsContainer_spec.jsx rename to superset-frontend/spec/javascripts/explore/components/ControlPanelsContainer_spec.tsx index 35de5b6c5b34..9b2fe59f56f9 100644 --- a/superset-frontend/spec/javascripts/explore/components/ControlPanelsContainer_spec.jsx +++ b/superset-frontend/spec/javascripts/explore/components/ControlPanelsContainer_spec.tsx @@ -18,10 +18,17 @@ */ import React from 'react'; import { shallow } from 'enzyme'; -import { getChartControlPanelRegistry, t } from '@superset-ui/core'; +import { + DatasourceType, + getChartControlPanelRegistry, + t, +} from '@superset-ui/core'; import { defaultControls } from 'src/explore/store'; import { getFormDataFromControls } from 'src/explore/controlUtils'; -import { ControlPanelsContainer } from 'src/explore/components/ControlPanelsContainer'; +import { + ControlPanelsContainer, + ControlPanelsContainerProps, +} from 'src/explore/components/ControlPanelsContainer'; import Collapse from 'src/common/components/Collapse'; describe('ControlPanelsContainer', () => { @@ -78,15 +85,15 @@ describe('ControlPanelsContainer', () => { }); function getDefaultProps() { + const controls = defaultControls as ControlPanelsContainerProps['controls']; return { - datasource_type: 'table', + datasource_type: DatasourceType.Table, actions: {}, - controls: defaultControls, - // Note: default viz_type is table - form_data: getFormDataFromControls(defaultControls), + controls, + form_data: getFormDataFromControls(controls), isDatasourceMetaLoading: false, exploreState: {}, - }; + } as ControlPanelsContainerProps; } it('renders ControlPanelSections', () => { diff --git a/superset-frontend/src/constants.ts b/superset-frontend/src/constants.ts index 0a7f156099c2..ad7683681b93 100644 --- a/superset-frontend/src/constants.ts +++ b/superset-frontend/src/constants.ts @@ -17,7 +17,6 @@ * under the License. */ export const DATETIME_WITH_TIME_ZONE = 'YYYY-MM-DD HH:mm:ssZ'; - export const TIME_WITH_MS = 'HH:mm:ss.SSS'; export const BOOL_TRUE_DISPLAY = 'True'; diff --git a/superset-frontend/src/explore/actions/exploreActions.ts b/superset-frontend/src/explore/actions/exploreActions.ts index 4923ae3174d0..7ebfc45e4263 100644 --- a/superset-frontend/src/explore/actions/exploreActions.ts +++ b/superset-frontend/src/explore/actions/exploreActions.ts @@ -99,7 +99,7 @@ export const SET_FIELD_VALUE = 'SET_FIELD_VALUE'; export function setControlValue( controlName: string, value: any, - validationErrors: any[], + validationErrors?: any[], ) { return { type: SET_FIELD_VALUE, controlName, value, validationErrors }; } @@ -109,11 +109,6 @@ export function setExploreControls(formData: QueryFormData) { return { type: SET_EXPLORE_CONTROLS, formData }; } -export const REMOVE_CONTROL_PANEL_ALERT = 'REMOVE_CONTROL_PANEL_ALERT'; -export function removeControlPanelAlert() { - return { type: REMOVE_CONTROL_PANEL_ALERT }; -} - export const UPDATE_CHART_TITLE = 'UPDATE_CHART_TITLE'; export function updateChartTitle(sliceName: string) { return { type: UPDATE_CHART_TITLE, sliceName }; @@ -154,7 +149,6 @@ export const exploreActions = { saveFaveStar, setControlValue, setExploreControls, - removeControlPanelAlert, updateChartTitle, createNewSlice, sliceUpdated, diff --git a/superset-frontend/src/explore/components/ControlPanelsContainer.jsx b/superset-frontend/src/explore/components/ControlPanelsContainer.tsx similarity index 69% rename from superset-frontend/src/explore/components/ControlPanelsContainer.jsx rename to superset-frontend/src/explore/components/ControlPanelsContainer.tsx index cc61ab2f1ca7..fc738e9d216c 100644 --- a/superset-frontend/src/explore/components/ControlPanelsContainer.jsx +++ b/superset-frontend/src/explore/components/ControlPanelsContainer.tsx @@ -18,30 +18,47 @@ */ /* eslint camelcase: 0 */ import React from 'react'; -import PropTypes from 'prop-types'; import { bindActionCreators } from 'redux'; import { connect } from 'react-redux'; -import { t, styled, getChartControlPanelRegistry } from '@superset-ui/core'; +import { + t, + styled, + getChartControlPanelRegistry, + QueryFormData, + DatasourceType, +} from '@superset-ui/core'; import Tabs from 'src/common/components/Tabs'; -import Alert from 'src/components/Alert'; import Collapse from 'src/common/components/Collapse'; import { PluginContext } from 'src/components/DynamicPlugins'; import Loading from 'src/components/Loading'; -import { InfoTooltipWithTrigger } from '@superset-ui/chart-controls'; +import { + ControlPanelSectionConfig, + ControlState, + CustomControlItem, + ExpandedControlItem, + InfoTooltipWithTrigger, +} from '@superset-ui/chart-controls'; import ControlRow from './ControlRow'; import Control from './Control'; import { sectionsToRender } from '../controlUtils'; -import { exploreActions } from '../actions/exploreActions'; +import { ExploreActions, exploreActions } from '../actions/exploreActions'; +import { ExploreState } from '../reducers/getInitialState'; -const propTypes = { - actions: PropTypes.object.isRequired, - alert: PropTypes.string, - datasource_type: PropTypes.string.isRequired, - exploreState: PropTypes.object.isRequired, - controls: PropTypes.object.isRequired, - form_data: PropTypes.object.isRequired, - isDatasourceMetaLoading: PropTypes.bool.isRequired, +export type ControlPanelsContainerProps = { + actions: ExploreActions; + datasource_type: DatasourceType; + exploreState: Record; + controls: Record; + form_data: QueryFormData; + isDatasourceMetaLoading: boolean; +}; + +export type ExpandedControlPanelSectionConfig = Omit< + ControlPanelSectionConfig, + 'controlSetRows' +> & { + controlSetRows: ExpandedControlItem[][]; }; const Styles = styled.div` @@ -50,9 +67,6 @@ const Styles = styled.div` overflow: auto; overflow-x: visible; overflow-y: auto; - .remove-alert { - cursor: pointer; - } #controlSections { min-height: 100%; overflow: visible; @@ -80,60 +94,34 @@ const ControlPanelsTabs = styled(Tabs)` height: 100%; } `; -class ControlPanelsContainer extends React.Component { + +class ControlPanelsContainer extends React.Component { // trigger updates to the component when async plugins load static contextType = PluginContext; - constructor(props) { + constructor(props: ControlPanelsContainerProps) { super(props); - - this.removeAlert = this.removeAlert.bind(this); this.renderControl = this.renderControl.bind(this); this.renderControlPanelSection = this.renderControlPanelSection.bind(this); } - componentDidUpdate(prevProps) { - const { - actions: { setControlValue }, - } = this.props; - if (this.props.form_data.datasource !== prevProps.form_data.datasource) { - const defaultValues = [ - 'MetricsControl', - 'AdhocFilterControl', - 'TextControl', - 'SelectControl', - 'CheckboxControl', - 'AnnotationLayerControl', - ]; - Object.entries(this.props.controls).forEach(([controlName, control]) => { - const { type, default: defaultValue } = control; - if (defaultValues.includes(type)) { - setControlValue(controlName, defaultValue); - } - }); - } - } - - sectionsToRender() { + sectionsToRender(): ExpandedControlPanelSectionConfig[] { return sectionsToRender( this.props.form_data.viz_type, this.props.datasource_type, ); } - sectionsToExpand(sections) { + sectionsToExpand(sections: ControlPanelSectionConfig[]) { return sections.reduce( - (acc, cur) => (cur.expanded ? [...acc, cur.label] : acc), - [], + (acc, section) => + section.expanded ? [...acc, String(section.label)] : acc, + [] as string[], ); } - removeAlert() { - this.props.actions.removeControlPanelAlert(); - } - - renderControl({ name, config }) { - const { actions, controls, form_data: formData } = this.props; + renderControl({ name, config }: CustomControlItem) { + const { actions, controls } = this.props; const { visibility } = config; // If the control item is not an object, we have to look up the control data from @@ -144,11 +132,9 @@ class ControlPanelsContainer extends React.Component { ...controls[name], name, }; - const { - validationErrors, - provideFormDataToProps, - ...restProps - } = controlData; + const { validationErrors, ...restProps } = controlData as ControlState & { + validationErrors?: any[]; + }; // if visibility check says the config is not visible, don't render it if (visibility && !visibility.call(config, this.props, controlData)) { @@ -160,30 +146,42 @@ class ControlPanelsContainer extends React.Component { name={name} validationErrors={validationErrors} actions={actions} - formData={provideFormDataToProps ? formData : null} - datasource={formData?.datasource} {...restProps} /> ); } - renderControlPanelSection(section) { + renderControlPanelSection(section: ExpandedControlPanelSectionConfig) { const { controls } = this.props; const { label, description } = section; + // Section label can be a ReactNode but in some places we want to + // have a string ID. Using forced type conversion for now, + // should probably add a `id` field to sections in the future. + const sectionId = String(label); + const hasErrors = section.controlSetRows.some(rows => - rows.some( - s => - controls[s] && - controls[s].validationErrors && - controls[s].validationErrors.length > 0, - ), + rows.some(item => { + const controlName = + typeof item === 'string' + ? item + : item && 'name' in item + ? item.name + : null; + return ( + controlName && + controlName in controls && + controls[controlName].validationErrors && + controls[controlName].validationErrors.length > 0 + ); + }), ); const PanelHeader = () => ( {label}{' '} {description && ( - + // label is only used in tooltip id (should probably call this prop `id`) + )} {hasErrors && ( {section.controlSetRows.map((controlSets, i) => { const renderedControls = controlSets @@ -229,7 +227,6 @@ class ControlPanelsContainer extends React.Component { return ( ); @@ -247,8 +244,8 @@ class ControlPanelsContainer extends React.Component { return ; } - const querySectionsToRender = []; - const displaySectionsToRender = []; + const querySectionsToRender: ExpandedControlPanelSectionConfig[] = []; + const displaySectionsToRender: ExpandedControlPanelSectionConfig[] = []; this.sectionsToRender().forEach(section => { // if at least one control in the section is not `renderTrigger` // or asks to be displayed at the Data tab @@ -258,6 +255,8 @@ class ControlPanelsContainer extends React.Component { rows.some( control => control && + typeof control === 'object' && + 'config' in control && control.config && (!control.config.renderTrigger || control.config.tabOverride === 'data'), @@ -277,14 +276,6 @@ class ControlPanelsContainer extends React.Component { ); return ( - {this.props.alert && ( - - )} - {props.controls.map((control, i) => ( + {controls.map((control, i) => (
{control}
@@ -37,6 +37,3 @@ function ControlSetRow(props) { ); } - -ControlSetRow.propTypes = propTypes; -export default ControlSetRow; diff --git a/superset-frontend/src/explore/components/ExploreViewContainer.jsx b/superset-frontend/src/explore/components/ExploreViewContainer.jsx index 71d163aa7265..cd72ac7214cb 100644 --- a/superset-frontend/src/explore/components/ExploreViewContainer.jsx +++ b/superset-frontend/src/explore/components/ExploreViewContainer.jsx @@ -222,10 +222,7 @@ function ExploreViewContainer(props) { } function onQuery() { - // remove alerts when query - props.actions.removeControlPanelAlert(); props.actions.triggerQuery(true, props.chart.id); - addHistory(); } diff --git a/superset-frontend/src/explore/components/controls/CollectionControl.jsx b/superset-frontend/src/explore/components/controls/CollectionControl.jsx index 46b096605f16..198df6d5cc51 100644 --- a/superset-frontend/src/explore/components/controls/CollectionControl.jsx +++ b/superset-frontend/src/explore/components/controls/CollectionControl.jsx @@ -69,12 +69,6 @@ export default class CollectionControl extends React.Component { this.onAdd = this.onAdd.bind(this); } - componentDidUpdate(prevProps) { - if (prevProps.datasource.name !== this.props.datasource.name) { - this.props.onChange([]); - } - } - onChange(i, value) { Object.assign(this.props.value[i], value); this.props.onChange(this.props.value); diff --git a/superset-frontend/src/explore/components/controls/DateFilterControl/DateFilterControl.tsx b/superset-frontend/src/explore/components/controls/DateFilterControl/DateFilterControl.tsx index 0e84bc3ee7b3..f15da37b5d15 100644 --- a/superset-frontend/src/explore/components/controls/DateFilterControl/DateFilterControl.tsx +++ b/superset-frontend/src/explore/components/controls/DateFilterControl/DateFilterControl.tsx @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -import React, { useState, useEffect } from 'react'; +import React, { useState, useEffect, useMemo } from 'react'; import rison from 'rison'; import { SupersetClient, @@ -25,6 +25,7 @@ import { t, TimeRangeEndpoints, } from '@superset-ui/core'; +import { DatasourceMeta } from '@superset-ui/chart-controls'; import { buildTimeRangeString, formatTimeRange, @@ -38,6 +39,8 @@ import { Divider } from 'src/common/components'; import Icon from 'src/components/Icon'; import { Select } from 'src/components/Select'; import { Tooltip } from 'src/common/components/Tooltip'; +import { DEFAULT_TIME_RANGE } from 'src/explore/constants'; + import { SelectOptionType, FrameType } from './types'; import { COMMON_RANGE_VALUES_SET, @@ -165,28 +168,27 @@ const IconWrapper = styled.span` } `; -interface DateFilterLabelProps { +interface DateFilterControlProps { name: string; onChange: (timeRange: string) => void; value?: string; endpoints?: TimeRangeEndpoints; - datasource?: string; + datasource?: DatasourceMeta; } -export default function DateFilterControl(props: DateFilterLabelProps) { - const { value = 'Last week', endpoints, onChange, datasource } = props; +export default function DateFilterControl(props: DateFilterControlProps) { + const { value = DEFAULT_TIME_RANGE, endpoints, onChange } = props; const [actualTimeRange, setActualTimeRange] = useState(value); const [show, setShow] = useState(false); - const [frame, setFrame] = useState(guessFrame(value)); - const [isMounted, setIsMounted] = useState(false); + const guessedFrame = useMemo(() => guessFrame(value), [value]); + const [frame, setFrame] = useState(guessedFrame); const [timeRangeValue, setTimeRangeValue] = useState(value); const [validTimeRange, setValidTimeRange] = useState(false); const [evalResponse, setEvalResponse] = useState(value); const [tooltipTitle, setTooltipTitle] = useState(value); useEffect(() => { - if (!isMounted) setIsMounted(true); fetchTimeRange(value, endpoints).then(({ value: actualRange, error }) => { if (error) { setEvalResponse(error || ''); @@ -205,9 +207,9 @@ export default function DateFilterControl(props: DateFilterLabelProps) { +--------------+------+----------+--------+----------+-----------+ */ if ( - frame === 'Common' || - frame === 'Calendar' || - frame === 'No filter' + guessedFrame === 'Common' || + guessedFrame === 'Calendar' || + guessedFrame === 'No filter' ) { setActualTimeRange(value); setTooltipTitle(actualRange || ''); @@ -220,14 +222,6 @@ export default function DateFilterControl(props: DateFilterLabelProps) { }); }, [value]); - useEffect(() => { - if (isMounted) { - onChange('Last week'); - setTimeRangeValue('Last week'); - setFrame(guessFrame('Last week')); - } - }, [datasource]); - useEffect(() => { fetchTimeRange(timeRangeValue, endpoints).then(({ value, error }) => { if (error) { @@ -247,13 +241,13 @@ export default function DateFilterControl(props: DateFilterLabelProps) { function onOpen() { setTimeRangeValue(value); - setFrame(guessFrame(value)); + setFrame(guessedFrame); setShow(true); } function onHide() { setTimeRangeValue(value); - setFrame(guessFrame(value)); + setFrame(guessedFrame); setShow(false); } @@ -265,7 +259,7 @@ export default function DateFilterControl(props: DateFilterLabelProps) { } }; - function onFrame(option: SelectOptionType) { + function onChangeFrame(option: SelectOptionType) { if (option.value === 'No filter') { setTimeRangeValue('No filter'); } @@ -278,7 +272,7 @@ export default function DateFilterControl(props: DateFilterLabelProps) {