From 4761a68e8938d61855af665fe1305b66c9a00e6d Mon Sep 17 00:00:00 2001 From: Riccardo Magliocchetti Date: Wed, 29 Jun 2016 11:12:37 +0200 Subject: [PATCH] Smarter redirect on slice creation After ea8a7ec1ba26393047f70bcacd335b9e22fc63b8 creating a slice started redirecting to druid datasource from sqlalchemy tables. That's quite painful for sqlalchemy tables users. Instead of hardcoding a choice just query the db, if we don't have any druid datasource fallback to sqlalchemy tables. Bonus points we remove hacky javascript and make the message translatable. While at it fix druid client test to not hardcode datasource id. --- caravel/templates/caravel/add_slice.html | 6 ------ caravel/views.py | 23 +++++++++++++++++++++- tests/core_tests.py | 25 ++++++++++++++++++++++-- 3 files changed, 45 insertions(+), 9 deletions(-) delete mode 100644 caravel/templates/caravel/add_slice.html diff --git a/caravel/templates/caravel/add_slice.html b/caravel/templates/caravel/add_slice.html deleted file mode 100644 index 6d74fca5bcbf8..0000000000000 --- a/caravel/templates/caravel/add_slice.html +++ /dev/null @@ -1,6 +0,0 @@ - diff --git a/caravel/views.py b/caravel/views.py index fe1984254d165..9851ba8fbcf4f 100755 --- a/caravel/views.py +++ b/caravel/views.py @@ -520,7 +520,6 @@ class DruidClusterModelView(CaravelModelView, DeleteMixin): # noqa class SliceModelView(CaravelModelView, DeleteMixin): # noqa datamodel = SQLAInterface(models.Slice) - add_template = "caravel/add_slice.html" can_add = False label_columns = { 'datasource_link': 'Datasource', @@ -568,6 +567,28 @@ def pre_update(self, obj): def pre_delete(self, obj): check_ownership(obj) + @expose('/add', methods=['GET', 'POST']) + @has_access + def add(self): + widget = self._add() + if not widget: + return redirect(self.get_redirect()) + + a_druid_datasource = db.session.query(models.DruidDatasource).first() + if a_druid_datasource is not None: + url = "/druiddatasourcemodelview/list/" + msg = _( + "Click on a datasource link to create a Slice, " + "or click on a table link here " + "to create a Slice for a table" + ) + else: + url = "/tablemodelview/list/" + msg = _("Click on a table link to create a Slice") + + redirect_url = "/r/msg/?url={}&msg={}".format(url, msg) + return redirect(redirect_url) + appbuilder.add_view( SliceModelView, "Slices", diff --git a/tests/core_tests.py b/tests/core_tests.py index b6f9154b38833..40e206c237972 100644 --- a/tests/core_tests.py +++ b/tests/core_tests.py @@ -17,7 +17,7 @@ import caravel from caravel import app, db, models, utils, appbuilder -from caravel.models import DruidCluster +from caravel.models import DruidCluster, DruidDatasource os.environ['CARAVEL_CONFIG'] = 'tests.caravel_test_config' @@ -218,6 +218,27 @@ def test_add_slices(self, username='admin'): assert new_slice in dash.slices assert len(set(dash.slices)) == len(dash.slices) + def test_add_slice_redirect_to_sqla(self, username='admin'): + self.login(username=username) + url = '/slicemodelview/add' + resp = self.client.get(url, follow_redirects=True) + assert "Click on a table link to create a Slice" in resp.data.decode('utf-8') + + def test_add_slice_redirect_to_druid(self, username='admin'): + datasource = DruidDatasource( + datasource_name="datasource_name", + ) + db.session.add(datasource) + db.session.commit() + + self.login(username=username) + url = '/slicemodelview/add' + resp = self.client.get(url, follow_redirects=True) + assert "Click on a datasource link to create a Slice" in resp.data.decode('utf-8') + + db.session.delete(datasource) + db.session.commit() + def test_gamma(self): self.login(username='gamma') resp = self.client.get('/slicemodelview/list/') @@ -409,7 +430,7 @@ def test_client(self, PyDruid): instance.export_pandas.return_value = df instance.query_dict = {} instance.query_builder.last_query.query_dict = {} - resp = self.client.get('/caravel/explore/druid/1/?viz_type=table&granularity=one+day&druid_time_origin=&since=7+days+ago&until=now&row_limit=5000&include_search=false&metrics=count&groupby=name&flt_col_0=dim1&flt_op_0=in&flt_eq_0=&slice_id=&slice_name=&collapsed_fieldsets=&action=&datasource_name=test_datasource&datasource_id=1&datasource_type=druid&previous_viz_type=table&json=true&force=true') + resp = self.client.get('/caravel/explore/druid/{}/?viz_type=table&granularity=one+day&druid_time_origin=&since=7+days+ago&until=now&row_limit=5000&include_search=false&metrics=count&groupby=name&flt_col_0=dim1&flt_op_0=in&flt_eq_0=&slice_id=&slice_name=&collapsed_fieldsets=&action=&datasource_name=test_datasource&datasource_id={}&datasource_type=druid&previous_viz_type=table&json=true&force=true'.format(datasource_id, datasource_id)) assert "Canada" in resp.data.decode('utf-8')