a few fixes per code review.

- add annotation input sanity check before add and before update.
- make SelectAsyncControl component statelesis, and generic
- add annotation description in d3 tool tip
- use less variable to replace hard-coded color

superset/assets/javascripts/explore/components/controls/SelectAsyncControl.jsx

@@ -5,7 +5,12 @@ import Select from '../../../components/AsyncSelect';
 import { t } from '../../../locales';
 
 const propTypes = {
+  dataEndpoint: PropTypes.string.isRequired,
+  multi: PropTypes.bool,
+  mutator: PropTypes.func,
+  onAsyncErrorMessage: PropTypes.string,
   onChange: PropTypes.func,
+  placeholder: PropTypes.string,
   value: PropTypes.oneOfType([
     PropTypes.string,
     PropTypes.number,
@@ -15,46 +20,32 @@ const propTypes = {
 };
 
 const defaultProps = {
+  multi: true,
+  onAsyncErrorMessage: t('Error while fetching data'),
   onChange: () => {},
+  placeholder: t('Select ...'),
 };
 
-class SelectAsyncControl extends React.PureComponent {
-  constructor(props) {
-    super(props);
-    this.state = {
-      value: this.props.value,
-    };
-  }
-
-  onChange(options) {
+const SelectAsyncControl = ({ value, onChange, dataEndpoint,
+                              multi, mutator, placeholder, onAsyncErrorMessage }) => {
+  const onSelectionChange = (options) => {
     const optionValues = options.map(option => option.value);
-    this.setState({ value: optionValues });
-    this.props.onChange(optionValues);
-  }
-
-  mutator(data) {
-    if (!data || !data.result) {
-      return [];
-    }
-
-    return data.result.map(layer => ({ value: layer.id, label: layer.name }));
-  }
-
-  render() {
-    return (
-      <Select
-        dataEndpoint={'/annotationlayermodelview/api/read?'}
-        onChange={this.onChange.bind(this)}
-        onAsyncError={() => notify.error(t('Error while fetching annotation layers'))}
-        mutator={this.mutator}
-        multi
-        value={this.state.value}
-        placeholder={t('Select a annotation layer')}
-        valueRenderer={v => (<div>{v.label}</div>)}
-      />
-    );
-  }
-}
+    onChange(optionValues);
+  };
+
+  return (
+    <Select
+      dataEndpoint={dataEndpoint}
+      onChange={onSelectionChange}
+      onAsyncError={() => notify.error(onAsyncErrorMessage)}
+      mutator={mutator}
+      multi={multi}
+      value={value}
+      placeholder={placeholder}
+      valueRenderer={v => (<div>{v.label}</div>)}
+    />
+  );
+};
 
 SelectAsyncControl.propTypes = propTypes;
 SelectAsyncControl.defaultProps = defaultProps;

superset/assets/javascripts/explore/stores/controls.jsx

@@ -125,6 +125,15 @@ export const controls = {
     label: t('Annotation Layers'),
     default: [],
     description: t('Annotation layers to overlay on the visualization'),
+    dataEndpoint: '/annotationlayermodelview/api/read?',
+    placeholder: t('Select a annotation layer'),
+    onAsyncErrorMessage: t('Error while fetching annotation layers'),
+    mutator: (data) => {
+      if (!data || !data.result) {
+        return [];
+      }
+      return data.result.map(layer => ({ value: layer.id, label: layer.name }));
+    },
   },
 
   metric: {

superset/assets/stylesheets/less/index.less

@@ -3,3 +3,5 @@
 @import "~bootstrap/less/bootstrap.less";
 @import "./cosmo/variables.less";
 @import "./cosmo/bootswatch.less";
+
+@stroke-primary:  @brand-primary;

superset/assets/stylesheets/superset.less

@@ -1,3 +1,5 @@
+@import './less/index.less';
+
 body {
     margin: 0px !important;
 }
@@ -371,3 +373,15 @@ iframe {
 .float-right {
   float: right;
 }
+
+g.annotation-container {
+  line {
+    stroke: @stroke-primary;
+  }
+
+  rect.annotation {
+    stroke: @stroke-primary;
+    fill-opacity: 0.1;
+    stroke-width: 1;
+  }
+}

superset/assets/visualizations/nvd3_vis.js

@@ -484,7 +484,18 @@ function nvd3Vis(slice, payload) {
           .attr('class', 'd3-tip')
           .direction('n')
           .offset([-5, 0])
-          .html(d => (d && d.layer ? d.layer : ''));
+          .html((d) => {
+            if (!d || !d.layer) {
+              return '';
+            }
+
+            const title = d.short_descr ?
+              d.short_descr + ' - ' + d.layer :
+              d.layer;
+            const body = d.long_descr;
+            return '<div><strong>' + title + '</strong></div><br/>' +
+            '<div>' + body + '</div>';
+          });
 
         const hh = chart.yAxis.scale().range()[0];
 
@@ -517,7 +528,6 @@ function nvd3Vis(slice, payload) {
           .attr('height', 10)
           .attr('patternTransform', 'rotate(45 50 50)')
           .append('line')
-          .attr('stroke', '#00A699')
           .attr('stroke-width', 7)
           .attr('y2', 10);
 
@@ -534,9 +544,6 @@ function nvd3Vis(slice, payload) {
           })
           .attr('height', hh)
           .attr('fill', 'url(#diagonal)')
-          .attr('fill-opacity', 0.1)
-          .attr('stroke-width', 1)
-          .attr('stroke', '#00A699')
           .on('mouseover', tip.show)
           .on('mouseout', tip.hide);
 

superset/models/core.py

@@ -25,7 +25,7 @@
 
 from sqlalchemy import (
     Column, Integer, String, ForeignKey, Text, Boolean,
-    DateTime, Date, Table, Index,
+    DateTime, Date, Table,
     create_engine, MetaData, select
 )
 from sqlalchemy.orm import relationship

superset/views/annotations.py

@@ -18,6 +18,21 @@ class AnnotationModelView(SupersetModelView, DeleteMixin):  # noqa
         'layer', 'short_descr', 'long_descr', 'start_dttm', 'end_dttm']
     add_columns = edit_columns
 
+    def pre_add(self, obj):
+        if not obj.layer:
+            raise Exception("Annotation layer is required.")
+        if not obj.start_dttm and not obj.end_dttm:
+            raise Exception("Annotation start time or end time is required.")
+        elif not obj.start_dttm:
+            obj.start_dttm = obj.end_dttm
+        elif not obj.end_dttm:
+            obj.end_dttm = obj.start_dttm
+        elif obj.end_dttm < obj.start_dttm:
+            raise Exception("Annotation end time must be no earlier than start time.")
+
+    def pre_update(self, obj):
+        self.pre_add(obj)
+
 
 class AnnotationLayerModelView(SupersetModelView, DeleteMixin):
     datamodel = SQLAInterface(AnnotationLayer)

superset/viz.py

@@ -112,7 +112,6 @@ def get_extra_filters(self):
 
     def query_obj(self):
         """Building a query object"""
-        print("running query_obj============================")
         form_data = self.form_data
         gb = form_data.get("groupby") or []
         metrics = form_data.get("metrics") or []