From 255ea699770a98b8406a516d44fc7eac47d45ab6 Mon Sep 17 00:00:00 2001 From: michellethomas Date: Fri, 22 Sep 2017 09:49:13 -0700 Subject: [PATCH] Add metric warning (#3499) * Adding warning text to metrics * Adding javascript tests * Fixing downgrade script for warning_text * Adding merge migration --- .../javascripts/components/MetricOption.jsx | 8 ++++++ .../components/MetricOption_spec.jsx | 14 +++++++--- superset/connectors/base/models.py | 5 +++- superset/connectors/druid/views.py | 3 ++- superset/connectors/sqla/views.py | 7 ++--- ...19a814813610_adding_metric_warning_text.py | 26 +++++++++++++++++++ superset/migrations/versions/472d2f73dfd4_.py | 22 ++++++++++++++++ 7 files changed, 76 insertions(+), 9 deletions(-) create mode 100644 superset/migrations/versions/19a814813610_adding_metric_warning_text.py create mode 100644 superset/migrations/versions/472d2f73dfd4_.py diff --git a/superset/assets/javascripts/components/MetricOption.jsx b/superset/assets/javascripts/components/MetricOption.jsx index 842761199c64..b19043479d64 100644 --- a/superset/assets/javascripts/components/MetricOption.jsx +++ b/superset/assets/javascripts/components/MetricOption.jsx @@ -27,6 +27,14 @@ export default function MetricOption({ metric }) { tooltip={metric.expression} label={`expr-${metric.metric_name}`} /> + {metric.warning_text && + + } ); } MetricOption.propTypes = propTypes; diff --git a/superset/assets/spec/javascripts/components/MetricOption_spec.jsx b/superset/assets/spec/javascripts/components/MetricOption_spec.jsx index f3fc26e24317..952e9fa125b1 100644 --- a/superset/assets/spec/javascripts/components/MetricOption_spec.jsx +++ b/superset/assets/spec/javascripts/components/MetricOption_spec.jsx @@ -13,6 +13,7 @@ describe('MetricOption', () => { verbose_name: 'Foo', expression: 'SUM(foo)', description: 'Foo is the greatest metric of all', + warning_text: 'Be careful when using foo', }, }; @@ -31,17 +32,22 @@ describe('MetricOption', () => { expect(lbl).to.have.length(1); expect(lbl.first().text()).to.equal('Foo'); }); - it('shows 2 InfoTooltipWithTrigger', () => { - expect(wrapper.find(InfoTooltipWithTrigger)).to.have.length(2); + it('shows 3 InfoTooltipWithTrigger', () => { + expect(wrapper.find(InfoTooltipWithTrigger)).to.have.length(3); }); - it('shows only 1 InfoTooltipWithTrigger when no descr', () => { + it('shows only 2 InfoTooltipWithTrigger when no descr', () => { props.metric.description = null; wrapper = shallow(factory(props)); - expect(wrapper.find(InfoTooltipWithTrigger)).to.have.length(1); + expect(wrapper.find(InfoTooltipWithTrigger)).to.have.length(2); }); it('shows a label with metric_name when no verbose_name', () => { props.metric.verbose_name = null; wrapper = shallow(factory(props)); expect(wrapper.find('.option-label').first().text()).to.equal('foo'); }); + it('shows only 1 InfoTooltipWithTrigger when no descr and no warning', () => { + props.metric.warning_text = null; + wrapper = shallow(factory(props)); + expect(wrapper.find(InfoTooltipWithTrigger)).to.have.length(1); + }); }); diff --git a/superset/connectors/base/models.py b/superset/connectors/base/models.py index 593c722d42bb..14cac09aba78 100644 --- a/superset/connectors/base/models.py +++ b/superset/connectors/base/models.py @@ -265,6 +265,7 @@ class BaseMetric(AuditMixinNullable, ImportMixin): description = Column(Text) is_restricted = Column(Boolean, default=False, nullable=True) d3format = Column(String(128)) + warning_text = Column(Text) """ The interface should also declare a datasource relationship pointing @@ -289,5 +290,7 @@ def expression(self): @property def data(self): - attrs = ('metric_name', 'verbose_name', 'description', 'expression') + attrs = ( + 'metric_name', 'verbose_name', 'description', 'expression', + 'warning_text') return {s: getattr(self, s) for s in attrs} diff --git a/superset/connectors/druid/views.py b/superset/connectors/druid/views.py index a06bc391ff35..f64b6c185009 100644 --- a/superset/connectors/druid/views.py +++ b/superset/connectors/druid/views.py @@ -85,7 +85,7 @@ class DruidMetricInlineView(CompactCRUDMixin, SupersetModelView): # noqa list_columns = ['metric_name', 'verbose_name', 'metric_type'] edit_columns = [ 'metric_name', 'description', 'verbose_name', 'metric_type', 'json', - 'datasource', 'd3format', 'is_restricted'] + 'datasource', 'd3format', 'is_restricted', 'warning_text'] add_columns = edit_columns page_size = 500 validators_columns = { @@ -109,6 +109,7 @@ class DruidMetricInlineView(CompactCRUDMixin, SupersetModelView): # noqa 'metric_type': _("Type"), 'json': _("JSON"), 'datasource': _("Druid Datasource"), + 'warning_text': _("Warning Message"), } def post_add(self, metric): diff --git a/superset/connectors/sqla/views.py b/superset/connectors/sqla/views.py index c49d4d02a56d..32b70c4754f4 100644 --- a/superset/connectors/sqla/views.py +++ b/superset/connectors/sqla/views.py @@ -108,7 +108,7 @@ class SqlMetricInlineView(CompactCRUDMixin, SupersetModelView): # noqa list_columns = ['metric_name', 'verbose_name', 'metric_type'] edit_columns = [ 'metric_name', 'description', 'verbose_name', 'metric_type', - 'expression', 'table', 'd3format', 'is_restricted'] + 'expression', 'table', 'd3format', 'is_restricted', 'warning_text'] description_columns = { 'expression': utils.markdown( "a valid SQL expression as supported by the underlying backend. " @@ -135,7 +135,8 @@ class SqlMetricInlineView(CompactCRUDMixin, SupersetModelView): # noqa 'expression': _("SQL Expression"), 'table': _("Table"), 'd3format': _("D3 Format"), - 'is_restricted': _('Is Restricted') + 'is_restricted': _('Is Restricted'), + 'warning_text': _('Warning Message'), } def post_add(self, metric): @@ -151,7 +152,7 @@ def post_update(self, metric): class TableModelView(DatasourceModelView, DeleteMixin): # noqa datamodel = SQLAInterface(models.SqlaTable) - + list_title = _('List Tables') show_title = _('Show Table') add_title = _('Add Table') diff --git a/superset/migrations/versions/19a814813610_adding_metric_warning_text.py b/superset/migrations/versions/19a814813610_adding_metric_warning_text.py new file mode 100644 index 000000000000..cf39a0e63159 --- /dev/null +++ b/superset/migrations/versions/19a814813610_adding_metric_warning_text.py @@ -0,0 +1,26 @@ +"""Adding metric warning_text + +Revision ID: 19a814813610 +Revises: ca69c70ec99b +Create Date: 2017-09-15 15:09:40.495345 + +""" + +# revision identifiers, used by Alembic. +revision = '19a814813610' +down_revision = 'ca69c70ec99b' + +from alembic import op +import sqlalchemy as sa + + +def upgrade(): + op.add_column('metrics', sa.Column('warning_text', sa.Text(), nullable=True)) + op.add_column('sql_metrics', sa.Column('warning_text', sa.Text(), nullable=True)) + + +def downgrade(): + with op.batch_alter_table('sql_metrics') as batch_op_sql_metrics: + batch_op_sql_metrics.drop_column('warning_text') + with op.batch_alter_table('metrics') as batch_op_metrics: + batch_op_metrics.drop_column('warning_text') diff --git a/superset/migrations/versions/472d2f73dfd4_.py b/superset/migrations/versions/472d2f73dfd4_.py new file mode 100644 index 000000000000..d74fd03a7b7c --- /dev/null +++ b/superset/migrations/versions/472d2f73dfd4_.py @@ -0,0 +1,22 @@ +"""empty message + +Revision ID: 472d2f73dfd4 +Revises: ('19a814813610', 'a9c47e2c1547') +Create Date: 2017-09-21 18:37:30.844196 + +""" + +# revision identifiers, used by Alembic. +revision = '472d2f73dfd4' +down_revision = ('19a814813610', 'a9c47e2c1547') + +from alembic import op +import sqlalchemy as sa + + +def upgrade(): + pass + + +def downgrade(): + pass