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

[add] Save filters to dashboard #3183

Merged
merged 12 commits into from
Aug 11, 2017
Merged

Conversation

roganw
Copy link
Contributor

@roganw roganw commented Jul 26, 2017

Save the filters of filter box to current dashboard.
2017-07-26 13 41 05

@coveralls
Copy link

coveralls commented Jul 26, 2017

Coverage Status

Coverage decreased (-0.04%) to 69.348% when pulling 581f610 on RoganW:default_filters into 6045063 on apache:master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.04%) to 69.348% when pulling 581f610 on RoganW:default_filters into 6045063 on apache:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.04%) to 69.348% when pulling 581f610 on RoganW:default_filters into 6045063 on apache:master.

@coveralls
Copy link

coveralls commented Jul 26, 2017

Coverage Status

Coverage decreased (-0.04%) to 69.348% when pulling 10e366e on RoganW:default_filters into 6045063 on apache:master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.04%) to 69.348% when pulling 10e366e on RoganW:default_filters into 6045063 on apache:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.04%) to 69.348% when pulling 10e366e on RoganW:default_filters into 6045063 on apache:master.

@coveralls
Copy link

coveralls commented Jul 26, 2017

Coverage Status

Coverage decreased (-0.04%) to 69.348% when pulling 5df2088 on RoganW:default_filters into 6045063 on apache:master.

@coveralls
Copy link

coveralls commented Jul 30, 2017

Coverage Status

Coverage decreased (-0.07%) to 69.313% when pulling b043d7e on RoganW:default_filters into 6045063 on apache:master.

filters = ''
if isinstance(default_filters, str):
filters = urllib.quote(default_filters)
elif isinstance(default_filters, unicode):
Copy link
Member

Choose a reason for hiding this comment

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

is this py3 compatible? can we add test coverage?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I will check these later.

@@ -195,6 +199,13 @@ export function dashboardContainer(dashboard, datasources, userid) {
return f;
},
addFilter(sliceId, col, vals, merge = true, refresh = true) {
// If slice doesn't exist, remove the related parameters in preselect_filters.
Copy link
Member

Choose a reason for hiding this comment

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

this function calls this.updateFilterParamsInUrl 3 times. Perhaps clearer logic would be an if statement that validates that there is a slice and column, put all the logic within that block, and call this.updateFilterParamsInUrl once at the very end

Copy link
Member

@mistercrunch mistercrunch left a comment

Choose a reason for hiding this comment

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

see comments

optimize the js code
fix the compatibility issue
@coveralls
Copy link

coveralls commented Aug 10, 2017

Coverage Status

Coverage decreased (-0.2%) to 69.194% when pulling b38bac4 on RoganW:default_filters into 6045063 on apache:master.

@coveralls
Copy link

coveralls commented Aug 10, 2017

Coverage Status

Coverage decreased (-0.03%) to 69.358% when pulling 4bbe05d on RoganW:default_filters into 6045063 on apache:master.

@coveralls
Copy link

coveralls commented Aug 10, 2017

Coverage Status

Coverage decreased (-0.07%) to 69.318% when pulling 5d815b2 on RoganW:default_filters into 6045063 on apache:master.

1 similar comment
@coveralls
Copy link

coveralls commented Aug 10, 2017

Coverage Status

Coverage decreased (-0.07%) to 69.318% when pulling 5d815b2 on RoganW:default_filters into 6045063 on apache:master.

@coveralls
Copy link

coveralls commented Aug 10, 2017

Coverage Status

Coverage decreased (-0.07%) to 69.318% when pulling adfd694 on RoganW:default_filters into 6045063 on apache:master.

@coveralls
Copy link

coveralls commented Aug 11, 2017

Coverage Status

Coverage decreased (-0.07%) to 69.318% when pulling 396e375 on RoganW:default_filters into 6045063 on apache:master.

@coveralls
Copy link

coveralls commented Aug 11, 2017

Coverage Status

Coverage decreased (-0.04%) to 69.348% when pulling f46ea38 on RoganW:default_filters into 6045063 on apache:master.

@roganw
Copy link
Contributor Author

roganw commented Aug 11, 2017

@mistercrunch Thanks, code optimized.

@mistercrunch
Copy link
Member

This is great work, thanks for this feature!

@mistercrunch mistercrunch merged commit a5320a0 into apache:master Aug 11, 2017
mistercrunch added a commit to mistercrunch/superset that referenced this pull request Aug 18, 2017
apache#3183 disabled the
ability of a filterbox to get filtered by another filterbox
mistercrunch added a commit that referenced this pull request Aug 18, 2017
* [dashboard] re-enabling cascading filters

#3183 disabled the
ability of a filterbox to get filtered by another filterbox

* linting
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.19.1 labels Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.19.1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants