Skip to content

Commit

Permalink
Smarter redirect on slice creation (#691)
Browse files Browse the repository at this point in the history
After ea8a7ec 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.
  • Loading branch information
xrmx authored and mistercrunch committed Aug 2, 2016
1 parent 862042b commit e7ce38b
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 9 deletions.
6 changes: 0 additions & 6 deletions caravel/templates/caravel/add_slice.html

This file was deleted.

23 changes: 22 additions & 1 deletion caravel/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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 <a href='/tablemodelview/list/'>here</a> "
"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",
Expand Down
25 changes: 23 additions & 2 deletions tests/core_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'

Expand Down Expand Up @@ -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/')
Expand Down Expand Up @@ -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')


Expand Down

0 comments on commit e7ce38b

Please sign in to comment.