Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Single quote filter values with comma #1084

Merged
merged 3 commits into from
Sep 12, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Binary file modified caravel/data/energy.json.gz
Binary file not shown.
13 changes: 9 additions & 4 deletions caravel/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
from __future__ import division
from __future__ import print_function
from __future__ import unicode_literals
import re

import functools
import json
Expand Down Expand Up @@ -54,6 +55,7 @@
config = app.config

QueryResult = namedtuple('namedtuple', ['df', 'query', 'duration'])
FillterPattern = re.compile(r'''((?:[^,"']|"[^"]*"|'[^']*')+)''')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please add a unit test



class JavascriptPostAggregator(Postaggregator):
Expand Down Expand Up @@ -839,7 +841,8 @@ def query( # sqla
for col, op, eq in filter:
col_obj = cols[col]
if op in ('in', 'not in'):
values = eq.split(",")
splitted = FillterPattern.split(eq)[1::2]
values = [types.replace("'", '').strip() for types in splitted]
cond = col_obj.sqla_col.in_(values)
if op == 'not in':
cond = ~cond
Expand Down Expand Up @@ -1597,9 +1600,11 @@ def get_filters(raw_filters):
cond = ~(Dimension(col) == eq)
elif op in ('in', 'not in'):
fields = []
splitted = eq.split(',')
if len(splitted) > 1:
for s in eq.split(','):
# Distinguish quoted values with regular value types
splitted = FillterPattern.split(eq)[1::2]
values = [types.replace("'", '') for types in splitted]
if len(values) > 1:
for s in values:
s = s.strip()
fields.append(Dimension(col) == s)
cond = Filter(type="or", fields=fields)
Expand Down
7 changes: 5 additions & 2 deletions caravel/templates/caravel/explore.html
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,11 @@
<div class="panel-title">
{{ _("Filters") }}
<i class="fa fa-question-circle-o" data-toggle="tooltip"
data-placement="bottom"
title="{{_("Filters are defined using comma delimited strings as in 'US,FR,Other'")}}. {{_("Leave the value field empty to filter empty strings or nulls")}}">
data-placement="right"
title="{{_("Filters are defined using comma delimited strings as in <US,FR,Other>")}}.
{{_("Leave the value field empty to filter empty strings or nulls")}}.
{{_("For filters with comma in values, wrap them in single quotes,
as in <NY, 'Tahoe, CA', DC>")}}">
</i>
</div>
</div>
Expand Down
9 changes: 6 additions & 3 deletions caravel/viz.py
Original file line number Diff line number Diff line change
Expand Up @@ -214,9 +214,12 @@ def query_filters(self, is_having_filter=False):
extra_filters = json.loads(extra_filters)
for slice_filters in extra_filters.values():
for col, vals in slice_filters.items():
if col and vals:
if col in self.datasource.filterable_column_names:
filters += [(col, 'in', ",".join(vals))]
if not (col and vals):
continue
elif col in self.datasource.filterable_column_names:
# Quote values with comma to avoid conflict
vals = ["'%s'" % x if "," in x else x for x in vals]
filters += [(col, 'in', ",".join(vals))]
return filters

def query_obj(self):
Expand Down
10 changes: 10 additions & 0 deletions tests/core_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,16 @@ def test_filter_druid_datasource(self):
assert 'datasource_for_gamma' in resp.data.decode('utf-8')
assert 'datasource_not_for_gamma' not in resp.data.decode('utf-8')

def test_add_filter(self, username='admin'):
# navigate to energy_usage slice with "Electricity,heat" in filter values
data = (
"/caravel/explore/table/1/?viz_type=table&groupby=source&metric=count&flt_col_1=source&flt_op_1=in&flt_eq_1=%27Electricity%2Cheat%27"
"&userid=1&datasource_name=energy_usage&datasource_id=1&datasource_type=tablerdo_save=saveas")
resp = self.client.get(
data,
follow_redirects=True)
assert ("source" in resp.data.decode('utf-8'))

def test_gamma(self):
self.login(username='gamma')
resp = self.client.get('/slicemodelview/list/')
Expand Down