diff --git a/.gitignore b/.gitignore index 929abbd5e8..2c553ad1db 100644 --- a/.gitignore +++ b/.gitignore @@ -40,6 +40,7 @@ dump.rdb env env_py3 envpy3 +env36 local_config.py superset_config.py superset.egg-info/ diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 5db5e98c8c..36ed573376 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -520,6 +520,12 @@ Run Cypress tests: cd /superset/superset/assets npm run build npm run cypress run + +# run tests from a specific file +npm run cypress run -- --spec cypress/integration/explore/link.test.js + +# run specific file with video capture +npm run cypress run -- --spec cypress/integration/dashboard/index.test.js --config video=true ``` ## Translating diff --git a/superset/assets/cypress/integration/explore/control.test.js b/superset/assets/cypress/integration/explore/control.test.js index 711ab78342..d20cb46e21 100644 --- a/superset/assets/cypress/integration/explore/control.test.js +++ b/superset/assets/cypress/integration/explore/control.test.js @@ -29,7 +29,7 @@ describe('Groupby', () => { cy.route('GET', '/superset/explore_json/**').as('getJson'); cy.route('POST', '/superset/explore_json/**').as('postJson'); cy.visitChartByName('Num Births Trend'); - cy.verifySliceSuccess({ waitAlias: '@getJson' }); + cy.verifySliceSuccess({ waitAlias: '@postJson' }); cy.get('[data-test=groupby]').within(() => { cy.get('.Select-control').click(); @@ -53,7 +53,7 @@ describe('AdhocMetrics', () => { const metricName = 'Girl Births'; cy.visitChartByName('Num Births Trend'); - cy.verifySliceSuccess({ waitAlias: '@getJson' }); + cy.verifySliceSuccess({ waitAlias: '@postJson' }); cy.get('[data-test=metrics]').within(() => { cy.get('.select-clear').click(); @@ -86,7 +86,7 @@ describe('AdhocMetrics', () => { const metric = 'SUM(num)/COUNT(DISTINCT name)'; cy.visitChartByName('Num Births Trend'); - cy.verifySliceSuccess({ waitAlias: '@getJson' }); + cy.verifySliceSuccess({ waitAlias: '@postJson' }); cy.get('[data-test=metrics]').within(() => { cy.get('.select-clear').click(); @@ -115,7 +115,7 @@ describe('AdhocMetrics', () => { it('Switch between simple and custom sql tabs', () => { cy.visitChartByName('Num Births Trend'); - cy.verifySliceSuccess({ waitAlias: '@getJson' }); + cy.verifySliceSuccess({ waitAlias: '@postJson' }); cy.get('[data-test=metrics]').within(() => { cy.get('.select-clear').click(); @@ -155,7 +155,7 @@ describe('AdhocFilters', () => { it('Set simple adhoc filter', () => { cy.visitChartByName('Num Births Trend'); - cy.verifySliceSuccess({ waitAlias: '@getJson' }); + cy.verifySliceSuccess({ waitAlias: '@postJson' }); cy.get('[data-test=adhoc_filters]').within(() => { cy.get('.Select-control').click({ force: true }); @@ -187,7 +187,7 @@ describe('AdhocFilters', () => { it('Set custom adhoc filter', () => { cy.visitChartByName('Num Births Trend'); - cy.verifySliceSuccess({ waitAlias: '@getJson' }); + cy.verifySliceSuccess({ waitAlias: '@postJson' }); cy.get('[data-test=adhoc_filters]').within(() => { cy.get('.Select-control').click({ force: true }); @@ -226,7 +226,7 @@ describe('Advanced analytics', () => { it('Create custom time compare', () => { cy.visitChartByName('Num Births Trend'); - cy.verifySliceSuccess({ waitAlias: '@getJson' }); + cy.verifySliceSuccess({ waitAlias: '@postJson' }); cy.get('span') .contains('Advanced Analytics') @@ -247,7 +247,7 @@ describe('Advanced analytics', () => { cy.wait('@postJson'); cy.reload(); cy.verifySliceSuccess({ - waitAlias: '@getJson', + waitAlias: '@postJson', chartSelector: 'svg', }); @@ -267,7 +267,7 @@ describe('Annotations', () => { it('Create formula annotation y-axis goal line', () => { cy.visitChartByName('Num Births Trend'); - cy.verifySliceSuccess({ waitAlias: '@getJson' }); + cy.verifySliceSuccess({ waitAlias: '@postJson' }); cy.get('[data-test=annotation_layers]').within(() => { cy.get('button').click(); diff --git a/superset/assets/cypress/integration/explore/link.test.js b/superset/assets/cypress/integration/explore/link.test.js index 36b56ce5d5..9f5f82d59f 100644 --- a/superset/assets/cypress/integration/explore/link.test.js +++ b/superset/assets/cypress/integration/explore/link.test.js @@ -32,7 +32,7 @@ describe('Test explore links', () => { it('Open and close view query modal', () => { cy.visitChartByName('Growth Rate'); - cy.verifySliceSuccess({ waitAlias: '@getJson' }); + cy.verifySliceSuccess({ waitAlias: '@postJson' }); cy.get('button#query').click(); cy.get('span').contains('View query').parent().click(); @@ -48,7 +48,7 @@ describe('Test explore links', () => { cy.route('POST', 'r/shortner/').as('getShortUrl'); cy.visitChartByName('Growth Rate'); - cy.verifySliceSuccess({ waitAlias: '@getJson' }); + cy.verifySliceSuccess({ waitAlias: '@postJson' }); cy.get('[data-test=short-link-button]').click(); @@ -61,12 +61,12 @@ describe('Test explore links', () => { .then((text) => { cy.visit(text); }); - cy.verifySliceSuccess({ waitAlias: '@getJson' }); + cy.verifySliceSuccess({ waitAlias: '@postJson' }); }); it('Test iframe link', () => { cy.visitChartByName('Growth Rate'); - cy.verifySliceSuccess({ waitAlias: '@getJson' }); + cy.verifySliceSuccess({ waitAlias: '@postJson' }); cy.get('[data-test=embed-code-button]').click(); cy.get('#embed-code-popover').within(() => { @@ -94,14 +94,14 @@ describe('Test explore links', () => { cy.url().should('eq', url); cy.visitChartByName(newChartName); - cy.verifySliceSuccess({ waitAlias: '@getJson' }); + cy.verifySliceSuccess({ waitAlias: '@postJson' }); }); }); xit('Test chart save', () => { const chartName = 'Test chart'; cy.visitChartByName(chartName); - cy.verifySliceSuccess({ waitAlias: '@getJson' }); + cy.verifySliceSuccess({ waitAlias: '@postJson' }); cy.get('[data-test=groupby]').within(() => { cy.get('span.select-clear-zone').click(); @@ -118,7 +118,7 @@ describe('Test explore links', () => { it('Test chart save as and add to new dashboard', () => { cy.visitChartByName('Growth Rate'); - cy.verifySliceSuccess({ waitAlias: '@getJson' }); + cy.verifySliceSuccess({ waitAlias: '@postJson' }); const dashboardTitle = 'Test dashboard'; cy.get('button[data-target="#save_modal"]').click(); @@ -128,7 +128,7 @@ describe('Test explore links', () => { cy.get('input[placeholder="[dashboard name]"]').type(dashboardTitle); cy.get('button#btn_modal_save').click(); }); - cy.verifySliceSuccess({ waitAlias: '@getJson' }); + cy.verifySliceSuccess({ waitAlias: '@postJson' }); cy.request(`/dashboard/api/read?_flt_3_dashboard_title=${dashboardTitle}`).then((response) => { expect(response.body.pks[0]).not.equals(null); }); @@ -136,7 +136,7 @@ describe('Test explore links', () => { it('Test chart save as and add to existing dashboard', () => { cy.visitChartByName('Most Populated Countries'); - cy.verifySliceSuccess({ waitAlias: '@getJson' }); + cy.verifySliceSuccess({ waitAlias: '@postJson' }); const chartName = 'New Most Populated Countries'; const dashboardTitle = 'Test dashboard'; @@ -151,7 +151,7 @@ describe('Test explore links', () => { }); cy.get('button#btn_modal_save').click(); }); - cy.verifySliceSuccess({ waitAlias: '@getJson' }); + cy.verifySliceSuccess({ waitAlias: '@postJson' }); cy.request(`/chart/api/read?_flt_3_slice_name=${chartName}`).then((response) => { cy.request('DELETE', `/chart/api/delete/${response.body.pks[0]}`); }); diff --git a/superset/assets/src/chart/Chart.jsx b/superset/assets/src/chart/Chart.jsx index 8cb615361e..c0d391673f 100644 --- a/superset/assets/src/chart/Chart.jsx +++ b/superset/assets/src/chart/Chart.jsx @@ -18,6 +18,8 @@ */ import PropTypes from 'prop-types'; import React from 'react'; +import { Alert } from 'react-bootstrap'; + import { Logger, LOG_ACTIONS_RENDER_CHART_CONTAINER } from '../logger/LogUtils'; import Loading from '../components/Loading'; import RefreshChartOverlay from '../components/RefreshChartOverlay'; @@ -133,7 +135,9 @@ class Chart extends React.PureComponent { if (chartStatus === 'failed') { return this.renderStackTraceMessage(); } - + if (errorMessage) { + return {errorMessage}; + } return (
{this.renderTooltip()} diff --git a/superset/assets/src/explore/components/Control.jsx b/superset/assets/src/explore/components/Control.jsx index 31942e9f28..b8babc709b 100644 --- a/superset/assets/src/explore/components/Control.jsx +++ b/superset/assets/src/explore/components/Control.jsx @@ -37,7 +37,6 @@ const propTypes = { description: PropTypes.string, tooltipOnClick: PropTypes.func, places: PropTypes.number, - validators: PropTypes.array, validationErrors: PropTypes.array, renderTrigger: PropTypes.bool, rightNode: PropTypes.node, @@ -54,7 +53,6 @@ const propTypes = { const defaultProps = { renderTrigger: false, - validators: [], hidden: false, validationErrors: [], }; @@ -63,45 +61,14 @@ export default class Control extends React.PureComponent { constructor(props) { super(props); this.state = { hovered: false }; - this.validate = this.validate.bind(this); this.onChange = this.onChange.bind(this); } - componentDidMount() { - this.validateAndSetValue(this.props.value, []); - } onChange(value, errors) { - this.validateAndSetValue(value, errors); + this.props.actions.setControlValue(this.props.name, value, errors); } setHover(hovered) { this.setState({ hovered }); } - validateAndSetValue(value, errors) { - let validationErrors = this.props.validationErrors; - let currentErrors = this.validate(value); - if (errors && errors.length > 0) { - currentErrors = validationErrors.concat(errors); - } - if (validationErrors.length + currentErrors.length > 0) { - validationErrors = currentErrors; - } - - if (value !== this.props.value || validationErrors !== this.props.validationErrors) { - this.props.actions.setControlValue(this.props.name, value, validationErrors); - } - } - validate(value) { - const validators = this.props.validators; - const validationErrors = []; - if (validators && validators.length > 0) { - validators.forEach((f) => { - const v = f(value); - if (v) { - validationErrors.push(v); - } - }); - } - return validationErrors; - } render() { const ControlType = controlMap[this.props.type]; const divStyle = this.props.hidden ? { display: 'none' } : null; diff --git a/superset/assets/src/explore/components/ExploreViewContainer.jsx b/superset/assets/src/explore/components/ExploreViewContainer.jsx index 91777e45e2..68d8fe1adc 100644 --- a/superset/assets/src/explore/components/ExploreViewContainer.jsx +++ b/superset/assets/src/explore/components/ExploreViewContainer.jsx @@ -21,6 +21,7 @@ import React from 'react'; import PropTypes from 'prop-types'; import { bindActionCreators } from 'redux'; import { connect } from 'react-redux'; +import { t } from '@superset-ui/translation'; import ExploreChartPanel from './ExploreChartPanel'; import ControlPanelsContainer from './ControlPanelsContainer'; @@ -100,6 +101,12 @@ class ExploreViewContainer extends React.Component { document.addEventListener('keydown', this.handleKeydown); this.addHistory({ isReplace: true }); this.props.actions.logEvent(LOG_ACTIONS_MOUNT_EXPLORER); + + // Trigger the chart if there are no errors + const { chart } = this.props; + if (!this.hasErrors()) { + this.props.actions.triggerQuery(true, this.props.chart.id); + } } componentWillReceiveProps(nextProps) { @@ -272,12 +279,13 @@ class ExploreViewContainer extends React.Component { renderErrorMessage() { // Returns an error message as a node if any errors are in the store const errors = []; + const ctrls = this.props.controls; for (const controlName in this.props.controls) { const control = this.props.controls[controlName]; if (control.validationErrors && control.validationErrors.length > 0) { errors.push(
- {`[ ${control.label} ] `} + {t('Control labeled ')}{` "${control.label}" `} {control.validationErrors.join('. ')}
, ); diff --git a/superset/assets/src/explore/reducers/exploreReducer.js b/superset/assets/src/explore/reducers/exploreReducer.js index 3823888812..6f6a1a9e6b 100644 --- a/superset/assets/src/explore/reducers/exploreReducer.js +++ b/superset/assets/src/explore/reducers/exploreReducer.js @@ -17,7 +17,8 @@ * under the License. */ /* eslint camelcase: 0 */ -import { getControlsState, getFormDataFromControls } from '../store'; +import { validateControl, getControlsState, getFormDataFromControls } from '../store'; +import controls from '../controls'; import * as actions from '../actions/exploreActions'; export default function exploreReducer(state = {}, action) { @@ -75,24 +76,28 @@ export default function exploreReducer(state = {}, action) { }; }, [actions.SET_FIELD_VALUE]() { - const controls = Object.assign({}, state.controls); - const control = Object.assign({}, controls[action.controlName]); - control.value = action.value; - control.validationErrors = action.validationErrors; - controls[action.controlName] = control; - const changes = { - controls, + // These errors are reported from the Control components + let errors = action.validationErrors || []; + let control = { + ...controls[action.controlName], + value: action.value, }; - if (control.renderTrigger) { - changes.triggerRender = true; - } else { - changes.triggerRender = false; - } - const newState = { + control = validateControl(control); + + // These errors are based on control config `validators` + errors = errors.concat(control.validationErrors || []); + const hasErrors = errors && errors.length > 0; + return { ...state, - ...changes, + triggerRender: control.renderTrigger && !hasErrors, + controls: { + ...state.controls, + [action.controlName]: { + ...control, + validationErrors: errors, + }, + }, }; - return newState; }, [actions.SET_EXPLORE_CONTROLS]() { return { diff --git a/superset/assets/src/explore/reducers/getInitialState.js b/superset/assets/src/explore/reducers/getInitialState.js index 48c85c7579..98b979914a 100644 --- a/superset/assets/src/explore/reducers/getInitialState.js +++ b/superset/assets/src/explore/reducers/getInitialState.js @@ -52,14 +52,14 @@ export default function getInitialState(bootstrapData) { [chartKey]: { id: chartKey, chartAlert: null, - chartStatus: 'loading', + chartStatus: null, chartUpdateEndTime: null, chartUpdateStartTime: 0, latestQueryFormData: getFormDataFromControls(controls), sliceFormData, queryController: null, queryResponse: null, - triggerQuery: true, + triggerQuery: false, lastRendered: 0, }, }, diff --git a/superset/assets/src/explore/store.js b/superset/assets/src/explore/store.js index be2be7b32a..df456c29eb 100644 --- a/superset/assets/src/explore/store.js +++ b/superset/assets/src/explore/store.js @@ -29,6 +29,24 @@ export function getFormDataFromControls(controlsState) { return formData; } +export function validateControl(control) { + const validators = control.validators; + const validationErrors = []; + if (validators && validators.length > 0) { + validators.forEach((f) => { + const v = f(control.value); + if (v) { + validationErrors.push(v); + } + }); + } + if (validationErrors.length > 0) { + return { ...control, validationErrors }; + } + return control; +} + + export function getControlNames(vizType, datasourceType) { const controlNames = []; sectionsToRender(vizType, datasourceType).forEach( @@ -109,7 +127,7 @@ export function getControlsState(state, form_data) { ) { control.value = formData[k]; } - controlsState[k] = control; + controlsState[k] = validateControl(control); }); if (viz.onInit) { return viz.onInit(controlsState);