diff --git a/superset-frontend/src/explore/components/controls/AnnotationLayer.jsx b/superset-frontend/src/explore/components/controls/AnnotationLayer.jsx index b0f0569dcbf..3388fea03d5 100644 --- a/superset-frontend/src/explore/components/controls/AnnotationLayer.jsx +++ b/superset-frontend/src/explore/components/controls/AnnotationLayer.jsx @@ -120,8 +120,8 @@ export default class AnnotationLayer extends React.PureComponent { intervalEndColumn, } = props; - const overridesKeys = Object.keys(overrides); - if (overridesKeys.includes('since') || overridesKeys.includes('until')) { + // Only allow override whole time_range + if ('since' in overrides || 'until' in overrides) { overrides.time_range = null; delete overrides.since; delete overrides.until; @@ -130,7 +130,6 @@ export default class AnnotationLayer extends React.PureComponent { this.state = { // base name, - oldName: !this.props.name ? null : name, annotationType, sourceType, value, @@ -149,10 +148,9 @@ export default class AnnotationLayer extends React.PureComponent { showMarkers, hideLine, // refData - isNew: !this.props.name, + isNew: !name, isLoadingOptions: true, valueOptions: [], - validationErrors: {}, }; this.submitAnnotation = this.submitAnnotation.bind(this); this.deleteAnnotation = this.deleteAnnotation.bind(this); @@ -237,7 +235,6 @@ export default class AnnotationLayer extends React.PureComponent { this.setState({ annotationType, sourceType: null, - validationErrors: {}, value: null, }); } @@ -246,12 +243,7 @@ export default class AnnotationLayer extends React.PureComponent { const { sourceType: prevSourceType } = this.state; if (prevSourceType !== sourceType) { - this.setState({ - sourceType, - isLoadingOptions: true, - validationErrors: {}, - value: null, - }); + this.setState({ sourceType, value: null, isLoadingOptions: true }); } } @@ -267,7 +259,7 @@ export default class AnnotationLayer extends React.PureComponent { } fetchOptions(annotationType, sourceType, isLoadingOptions) { - if (isLoadingOptions === true) { + if (isLoadingOptions) { if (sourceType === ANNOTATION_SOURCE_TYPES.NATIVE) { SupersetClient.get({ endpoint: '/annotationlayermodelview/api/read?', @@ -310,28 +302,43 @@ export default class AnnotationLayer extends React.PureComponent { } deleteAnnotation() { + this.props.removeAnnotationLayer(); this.props.close(); - if (!this.state.isNew) { - this.props.removeAnnotationLayer(this.state); - } } applyAnnotation() { - if (this.state.name.length) { - const annotation = {}; - Object.keys(this.state).forEach(k => { - if (this.state[k] !== null) { - annotation[k] = this.state[k]; + if (this.isValidForm()) { + const annotationFields = [ + 'name', + 'annotationType', + 'sourceType', + 'color', + 'opacity', + 'style', + 'width', + 'showMarkers', + 'hideLine', + 'value', + 'overrides', + 'show', + 'titleColumn', + 'descriptionColumns', + 'timeColumn', + 'intervalEndColumn', + ]; + const newAnnotation = {}; + annotationFields.forEach(field => { + if (this.state[field] !== null) { + newAnnotation[field] = this.state[field]; } }); - delete annotation.isNew; - delete annotation.valueOptions; - delete annotation.isLoadingOptions; - delete annotation.validationErrors; - annotation.color = - annotation.color === AUTOMATIC_COLOR ? null : annotation.color; - this.props.addAnnotationLayer(annotation); - this.setState(prevState => ({ isNew: false, oldName: prevState.name })); + + if (newAnnotation.color === AUTOMATIC_COLOR) { + newAnnotation.color = null; + } + + this.props.addAnnotationLayer(newAnnotation); + this.setState({ isNew: false }); } } @@ -363,7 +370,7 @@ export default class AnnotationLayer extends React.PureComponent { label = 'Annotation Layer'; description = 'Select the Annotation Layer you would like to use.'; } else { - label = label = t('Chart'); + label = t('Chart'); description = `Use a pre defined Superset Chart as a source for annotations and overlays. your chart must be one of these visualization types: [${this.getSupportedSourceTypes(annotationType) @@ -502,7 +509,7 @@ export default class AnnotationLayer extends React.PureComponent { label="Override time range" description={`This controls whether the "time_range" field from the current view should be passed down to the chart containing the annotation data.`} - value={!!Object.keys(overrides).find(x => x === 'time_range')} + value={'time_range' in overrides} onChange={v => { delete overrides.time_range; if (v) { @@ -520,9 +527,7 @@ export default class AnnotationLayer extends React.PureComponent { label="Override time grain" description={`This controls whether the time grain field from the current view should be passed down to the chart containing the annotation data.`} - value={ - !!Object.keys(overrides).find(x => x === 'time_grain_sqla') - } + value={'time_grain_sqla' in overrides} onChange={v => { delete overrides.time_grain_sqla; delete overrides.granularity; @@ -710,7 +715,7 @@ export default class AnnotationLayer extends React.PureComponent { value={annotationType} onChange={this.handleAnnotationType} /> - {!!supportedSourceTypes.length && ( + {supportedSourceTypes.length > 0 && (
- + {isNew ? ( + + ) : ( + + )}