Skip to content

[FilterBox] dashboard date range filtering #1165

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.

Sign up for GitHub

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

Merged
mistercrunch merged 2 commits into from
Sep 22, 2016
Merged

[FilterBox] dashboard date range filtering #1165

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
39e262a
[FilterBox] dashboard date range filtering
mistercrunch Sep 21, 2016
d3c6dcd
Addressing comments
mistercrunch Sep 22, 2016
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
13 changes: 8 additions & 5 deletions caravel/assets/.eslintignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
node_modules/*
vendor/*
**/*{.,-}min.js
**/*.sh
coverage/**
dist/*
stylesheets/*
images/*
node_modules/*
node_modules*/*
spec/*
coverage/**
**/*{.,-}min.js
stylesheets/*
vendor/*
2 changes: 1 addition & 1 deletion caravel/assets/javascripts/modules/caravel.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ const px = function () {
const containerId = data.token + '_con';
const selector = '#' + containerId;
const container = $(selector);
const sliceId = data.sliceId;
const sliceId = data.slice_id;
Copy link

@ascott ascott Sep 21, 2016

Choose a reason for hiding this comment

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

/facepalm

Copy link
Member Author

@mistercrunch mistercrunch Sep 21, 2016

Choose a reason for hiding this comment

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

that's the last one!

Copy link

@ascott ascott Sep 22, 2016

Choose a reason for hiding this comment

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

🎉

let dttm = 0;
const stopwatch = function () {
dttm += 10;
Expand Down
16 changes: 7 additions & 9 deletions caravel/assets/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,7 @@
"cover": "babel-node ./node_modules/.bin/istanbul cover _mocha -- --require spec/helpers/browser.js --recursive spec/**/*_spec.*",
"dev": "NODE_ENV=dev webpack -d --watch --colors --progress",
"prod": "NODE_ENV=production webpack -p --colors --progress",
"lint": "npm run --silent lint:js",
"lint:js": "eslint --ignore-path=.eslintignore --ext .js,.jsx ."
"lint": "eslint --ignore-path=.eslintignore --ext .js,.jsx ."
},
"repository": {
"type": "git",
Expand All @@ -39,6 +38,7 @@
"dependencies": {
"autobind-decorator": "^1.3.3",
"babel-cli": "^6.14.0",
"babel-preset-es2015": "^6.14.0",
"bootstrap": "^3.3.6",
"bootstrap-datepicker": "^1.6.0",
"brace": "^0.7.0",
Expand All @@ -62,11 +62,11 @@
"moments": "0.0.2",
"mustache": "^2.2.1",
"nvd3": "1.8.4",
"react": "^15.2.1",
"react": "^15.3.2",
Copy link

@ascott ascott Sep 22, 2016

Choose a reason for hiding this comment

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

👍

"react-ace": "^3.4.1",
"react-bootstrap": "^0.30.3",
"react-bootstrap-table": "^2.3.8",
"react-dom": "^0.14.8",
"react-dom": "^15.3.2",
"react-draggable": "^2.1.2",
"react-grid-layout": "^0.13.1",
"react-map-gl": "^1.0.0-beta-10",
Expand All @@ -89,8 +89,7 @@
"babel": "^6.3.26",
"babel-core": "^6.10.4",
"babel-loader": "^6.2.4",
"babel-preset-airbnb": "^1.1.1",
"babel-preset-es2015": "^6.9.0",
"babel-preset-airbnb": "^2.0.0",
"babel-preset-react": "^6.11.1",
"chai": "^3.5.0",
"codeclimate-test-reporter": "^0.3.3",
Expand All @@ -99,7 +98,7 @@
"eslint": "^2.13.1",
"eslint-config-airbnb": "^9.0.1",
"eslint-plugin-import": "^1.11.1",
"eslint-plugin-jsx-a11y": "^2.0.1",
"eslint-plugin-jsx-a11y": "^1.2.0",
"eslint-plugin-react": "^5.2.2",
"exports-loader": "^0.6.3",
"istanbul": "^1.0.0-alpha",
Expand All @@ -110,8 +109,7 @@
"less": "^2.6.1",
"less-loader": "^2.2.2",
"mocha": "^2.4.5",
"react-addons-test-utils": "^0.14.8",
"react-dom": "^0.14.8",
"react-addons-test-utils": "^15.3.2",
"style-loader": "^0.13.0",
"transform-loader": "^0.2.3",
"url-loader": "^0.5.7",
Expand Down
10 changes: 10 additions & 0 deletions caravel/assets/visualizations/constants.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
export const TIME_CHOICES = [
'1 hour ago',
'12 hours ago',
'1 day ago',
'7 days ago',
'28 days ago',
'90 days ago',
'1 year ago',
];

3 changes: 3 additions & 0 deletions caravel/assets/visualizations/filter_box.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,6 @@ ul.select2-results div.filter_box{
.filter_box:hover {
z-index: 1000;
}
.m-b-5 {
margin-bottom: 5px;
}
79 changes: 57 additions & 22 deletions caravel/assets/visualizations/filter_box.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,21 @@ import Select from 'react-select';
import '../stylesheets/react-select/select.less';

import './filter_box.css';
import { TIME_CHOICES } from './constants.js';

const propTypes = {
Copy link

@ascott ascott Sep 22, 2016

Choose a reason for hiding this comment

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

i like how you moved these to the top of the file. it's a pattern i use too & find super useful when reading a component file. 👌

filtersChoices: React.PropTypes.object,
Copy link

@ascott ascott Sep 22, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

is this a required prop? if so we should mark it isRequired

Copy link
Member Author

@mistercrunch mistercrunch Sep 22, 2016

Choose a reason for hiding this comment

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

it's not required, I added a default value for it

onChange: React.PropTypes.func,
origSelectedValues: React.PropTypes.object,
showDateFilter: React.PropTypes.bool,
};

const defaultProps = {
filtersChoices: {},
onChange: () => {},
origSelectedValues: {},
showDateFilter: false,
};

class FilterBox extends React.Component {
Copy link

@ascott ascott Sep 22, 2016

Choose a reason for hiding this comment

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

i think this component should be in it's own file in the components directory, and imported for use in this visualization file.

Copy link
Member Author

@mistercrunch mistercrunch Sep 22, 2016

Choose a reason for hiding this comment

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

I'd like to keep all of the visualization-specific as together as possible hoping that one day they can be standalone, or distributed outside of Caravel.

constructor(props) {
Expand All @@ -17,15 +32,50 @@ class FilterBox extends React.Component {
selectedValues: props.origSelectedValues,
};
}
changeFilter(filter, options) {
let vals = null;
if (options) {
if (Array.isArray(options)) {
Copy link

@ascott ascott Sep 22, 2016

Choose a reason for hiding this comment

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

why are options sometimes arrays and sometimes objects?

Copy link
Member Author

@mistercrunch mistercrunch Sep 22, 2016

Choose a reason for hiding this comment

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

because we use a mix of Select multiple and Select one... The time ones are single choice

vals = options.map((opt) => opt.value);
} else {
vals = options.value;
}
}
const selectedValues = Object.assign({}, this.state.selectedValues);
selectedValues[filter] = vals;
this.setState({ selectedValues });
this.props.onChange(filter, vals);
}
render() {
let dateFilter;
if (this.props.showDateFilter) {
dateFilter = ['__from', '__to'].map((field) => {
const val = this.state.selectedValues[field];
const choices = TIME_CHOICES.slice();
if (!choices.includes(val)) {
choices.push(val);
}
const options = choices.map((s) => ({ value: s, label: s }));
return (
<div className="m-b-5">
{field.replace('__', '')}
<Select.Creatable
options={options}
value={this.state.selectedValues[field]}
onChange={this.changeFilter.bind(this, field)}
/>
</div>
);
});
}
const filters = Object.keys(this.props.filtersChoices).map((filter) => {
const data = this.props.filtersChoices[filter];
const maxes = {};
maxes[filter] = d3.max(data, function (d) {
return d.metric;
});
return (
<div>
<div key={filter} className="m-b-5">
{filter}
<Select
placeholder={`Select [${filter}]`}
Expand All @@ -44,38 +94,21 @@ class FilterBox extends React.Component {
};
return { value: opt.id, label: opt.id, style };
})}
onChange={(selectedOptions) => {
let vals;
if (selectedOptions) {
vals = selectedOptions.map((opt) => opt.value);
} else {
vals = null;
}
const selectedValues = this.state.selectedValues;
selectedValues[filter] = vals;
this.setState({ selectedValues });
this.props.onChange(filter, vals);
}}
onChange={this.changeFilter.bind(this, filter)}
/>
</div>
);
});
return (
<div>
{dateFilter}
{filters}
</div>
);
}
}
FilterBox.propTypes = {
origSelectedValues: React.PropTypes.objectOf(React.PropTypes.array),
filtersChoices: React.PropTypes.objectOf(React.PropTypes.array),
onChange: React.PropTypes.function,
};
FilterBox.defaultProps = {
origSelectedValues: {},
onChange() {},
};
FilterBox.propTypes = propTypes;
FilterBox.defaultProps = defaultProps;

function filterBox(slice) {
const d3token = d3.select(slice.selector);
Expand All @@ -86,6 +119,7 @@ function filterBox(slice) {
// filter box should ignore the dashboard's filters
const url = slice.jsonEndpoint({ extraFilters: false });
$.getJSON(url, (payload) => {
const fd = payload.form_data;
const filtersChoices = {};
// Making sure the ordering of the fields matches the setting in the
// dropdown as it may have been shuffled while serialized to json
Expand All @@ -96,6 +130,7 @@ function filterBox(slice) {
<FilterBox
filtersChoices={filtersChoices}
onChange={slice.setFilter}
showDateFilter={fd.date_filter}
origSelectedValues={slice.getFilters() || {}}
/>,
document.getElementById(slice.containerId)
Expand Down
5 changes: 5 additions & 0 deletions caravel/forms.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -749,6 +749,11 @@ def __init__(self, viz):
"description": _(
"Whether to display the time range interactive selector")
}),
'date_filter': (BetterBooleanField, {
"label": _("Date Filter"),
"default": False,
"description": _("Whether to include a time filter")
}),
'show_datatable': (BetterBooleanField, {
"label": _("Data Table"),
"default": False,
Expand Down
45 changes: 30 additions & 15 deletions caravel/viz.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,17 @@ def form(self):
def form_class(self):
return FormFactory(self).get_form()

def get_extra_filters(self):
extra_filters = self.form_data.get('extra_filters')
if not extra_filters:
return {}
extra_filters = json.loads(extra_filters)
# removing per-slice details
summary = {}
for flt in extra_filters.values():
summary.update(flt)
return summary

def query_filters(self, is_having_filter=False):
"""Processes the filters for the query"""
form_data = self.form_data
Expand All @@ -205,36 +216,39 @@ def query_filters(self, is_having_filter=False):
if col and op and eq is not None:
filters.append((col, op, eq))

if is_having_filter:
Copy link

@ascott ascott Sep 22, 2016

Choose a reason for hiding this comment

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

is this a python-ism? would has_filter be more clear?

Copy link
Member Author

@mistercrunch mistercrunch Sep 22, 2016

Choose a reason for hiding this comment

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

it has to do with the SQL HAVING clause

return filters

# Extra filters (coming from dashboard)
extra_filters = form_data.get('extra_filters')
if extra_filters and not is_having_filter:
extra_filters = json.loads(extra_filters)
for slice_filters in extra_filters.values():
for col, vals in slice_filters.items():
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))]
for col, vals in self.get_extra_filters().items():
if not (col and vals):
continue
elif col in self.datasource.filterable_column_names:
# Quote values with comma to avoid conflict
vals = ["'{}'".format(x) if "," in x else x for x in vals]
filters += [(col, 'in', ",".join(vals))]
return filters

def query_obj(self):
"""Building a query object"""
form_data = self.form_data
groupby = form_data.get("groupby") or []
metrics = form_data.get("metrics") or ['count']
granularity = \
extra_filters = self.get_extra_filters()
granularity = (
form_data.get("granularity") or form_data.get("granularity_sqla")
)
limit = int(form_data.get("limit", 0))
row_limit = int(
form_data.get("row_limit", config.get("ROW_LIMIT")))
since = form_data.get("since", "1 year ago")
since = (
extra_filters.get('__from') or form_data.get("since", "1 year ago")
)
from_dttm = utils.parse_human_datetime(since)
now = datetime.now()
if from_dttm > now:
from_dttm = now - (from_dttm - now)
until = form_data.get("until", "now")
until = extra_filters.get('__to') or form_data.get("until", "now")
to_dttm = utils.parse_human_datetime(until)
if from_dttm > to_dttm:
flasher("The date range doesn't seem right.", "danger")
Expand All @@ -245,7 +259,7 @@ def query_obj(self):
extras = {
'where': form_data.get("where", ''),
'having': form_data.get("having", ''),
'having_druid': self.query_filters(True),
'having_druid': self.query_filters(is_having_filter=True),
'time_grain_sqla': form_data.get("time_grain_sqla", ''),
'druid_time_origin': form_data.get("druid_time_origin", ''),
}
Expand Down Expand Up @@ -1634,6 +1648,7 @@ class FilterBoxViz(BaseViz):
fieldsets = ({
'label': None,
'fields': (
('date_filter', None),
'groupby',
'metric',
)
Expand Down