Filter Enhancement (#933) - Filter Value Dropdown #1061

the-dcruz · 2016-09-02T19:34:42Z

Overview

This PR address the issue #933, which is to replace the filter TextField with a FreeFormSelect to make it easier for the user to know what types of values are possible for a filter.

If their desired value is not in the list, they can still type in their custom value. (Especially common for regex operation)

The feature must be enabled on the table/datasource.

Technical Changes

Added new API endpoint /filter/<datasource_type>/<datasource_id>/<column>/
Added new function values_for_column to models.DruidDatasource and models.SqlaTable (migration script included)
- values_for_column uses the same querystring as query
- Currently limited to 500 values
Added new boolean column to datasources and tables to indicate allowing filter value select
add_filter in explore.jsx will now fix the filter IDs upon render instead of by prepForm on query submit

Screenshots

How filter looks:

Option on table:

TODO for Future PR

Entire filter can be redone in React
Cache filter endpoint

@mistercrunch

the-dcruz · 2016-09-02T19:35:55Z

This PR is the same as #948

Sorry I closed that PR while I was fixing issues after rebasing and my force push didn't allow me to re-open it.

kkalyan · 2016-09-06T14:46:11Z

Great enhancement, thanks @the-dcruz!

Is there a way to select multiple options ?
Currently comma separated values works as OR. May be switch to Select2 widget ?

mistercrunch · 2016-09-11T14:47:03Z

It needs to be backward compatible with the existing multi-value filters.

the-dcruz · 2016-09-12T17:42:18Z

@kkalyan @mistercrunch
I currently have this implemented with a freeform select since it is likely that not all possible values for a given column will be displayed (limit=500 currently). My idea was to have this select act as a guide for the user to see a sample of the possible values for the column. The user can then type the values with commas as before.

Maybe a better implementation I can try would be a freeform multiselect?

the-dcruz · 2016-09-12T20:28:31Z

Updated PR to now use freeform multiselect.

ShengyaoQian · 2016-09-15T16:59:42Z

This would be a very helpful enhancement!

the-dcruz · 2016-09-15T17:56:30Z

@mistercrunch I was wondering if there was still a chance of this PR being looked into. I've periodically rebased it but will hold off until I know it will be looked at. Thanks

mistercrunch · 2016-09-21T15:51:29Z

Oh nice! I didn't see your updates since my last comment. I'm a bit overwhelmed with the amount of notifications and communication channels around me.

This looks good, I'll fetch your branch and do some testing on our staging environment today.

mistercrunch · 2016-09-21T15:54:24Z

Quick question before I test, we recently had a PR to fix a bug around filtering on strings that include commas, is that handled on your side?

mistercrunch · 2016-09-21T15:55:42Z

@vera-liu , since you're familiar with this feature and this part of the code, would you mind doing some testing/reviewing here?

the-dcruz · 2016-09-21T15:57:16Z

@mistercrunch Doesn't ring a bell. Could you point me to the PR? I'll try to get that integrated. Thanks for taking a look btw!

And no worries, I understand it can't be easy managing multiple popular open source projects 😄

vera-liu · 2016-09-21T21:00:50Z

Hi! I was trying to test this on energy_usage example database but somehow the values are not showing up for me. Am I on the right user flow?

Toggle 'Enable Filter Select' in Edit mode of energy_usage
Go to explore view and click on plus-sign of Filters

the-dcruz · 2016-09-21T21:15:55Z

@vera-liu I use the same time parameters for this query as specified for the slice. Can you show me which time params you selected?

vera-liu · 2016-09-21T21:18:43Z

These ones?

the-dcruz · 2016-09-21T21:19:25Z

Yes. Does 7 Days ago actually return any data to display in the chart?

vera-liu · 2016-09-21T21:21:36Z

Yes:

But the filter values do not show up in the dropdown:

the-dcruz · 2016-09-21T21:30:43Z

@vera-liu I see my issue. Energy usage does not have any time column.

I guess I'll ignore time in my query if there is no time column. I'll add that fix in a bit. Could you try with something time sensitive like "birth names"?

the-dcruz · 2016-09-21T22:02:54Z

@vera-liu I pushed the fix so it should work for the energy table now. I'll rebase as soon as you think things look good. Thanks!

vera-liu · 2016-09-21T23:11:01Z

It's looking good :). We used to have a bug with comma in filter values:
#1045

Solved in this PR: #1084

We should include that fix in this PR

vera-liu · 2016-09-21T23:19:51Z

caravel/models.py

+                timestamp >= text(dttm_col.dttm_sql_literal(from_dttm)),
+                timestamp <= text(dttm_col.dttm_sql_literal(to_dttm)),
+            ]
+            qry = qry.where(and_(*time_filter)) if granularity else qry


Why do we need if granularity else qry here? If this is inside if granularity: it should always be true right?

Good catch. Forgot to remove this.

the-dcruz · 2016-09-22T00:03:33Z

@vera-liu I decided that the most consistent way to integrate your fix is to wrap all the filter choices with the single quotes.

This seems reasonable to me since this is a dropdown containing column values instead of column names like many of the other dropdowns.

vera-liu · 2016-09-22T18:09:08Z

Do you mind rebasing on latest master since it has python test for filter values with commas?

the-dcruz · 2016-09-22T18:16:49Z

@vera-liu Sure. Done

the-dcruz · 2016-09-22T18:18:25Z

Looks like another migration file was introduced after rebase, causing the tests to fail. I'll update my migration file and rebase again.

the-dcruz · 2016-09-22T19:10:44Z

Tests look good after rebase @vera-liu

vera-liu · 2016-09-26T22:22:10Z

So Sorry for the delay, I think it's looking good! I'll test it against prod db and then confirm. Could you resolve the conflicts and bump the migration in the meantime? Thank you for the patience! @the-dcruz

mistercrunch · 2016-10-07T17:35:13Z

please rebase so that we can merge this!

kkalyan · 2016-11-04T15:40:02Z

@the-dcruz can't wait to see this in master, please rebase!

the-dcruz · 2016-11-08T01:42:43Z

@kkalyan Sorry about the wait! I had abandoned this PR thinking it was going to be waived off until exploreV2.

@mistercrunch I've rebased and retested this.

…taining possible column values

ascott · 2016-11-17T02:45:17Z

@vera-liu would you make sure this gets added to the filters component in V2?

vera-liu · 2016-11-18T18:39:21Z

Seems like caravel-superset renaming caused conflicts in this PR, I will try to include this feature in explore V2, but feel free to merge into V1 if you would like @the-dcruz, it's an awesome feature to have!

mistercrunch · 2017-06-12T20:46:47Z

This is done in master

Bumps [ssri](https://github.com/npm/ssri) from 6.0.1 to 6.0.2. **This update includes a security fix.** - [Release notes](https://github.com/npm/ssri/releases) - [Changelog](https://github.com/npm/ssri/blob/v6.0.2/CHANGELOG.md) - [Commits](npm/ssri@v6.0.1...v6.0.2) Signed-off-by: dependabot-preview[bot] <support@dependabot.com> Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com>

the-dcruz force-pushed the filter-value-selector branch from 88c37de to 18cd7c1 Compare September 12, 2016 20:25

the-dcruz force-pushed the filter-value-selector branch from 18cd7c1 to 1d6923d Compare September 12, 2016 20:40

xrmx mentioned this pull request Sep 15, 2016

Getting distinct values for filter fields #1111

Closed

vera-liu reviewed Sep 21, 2016

View reviewed changes

the-dcruz force-pushed the filter-value-selector branch from 73b1553 to b6e0b25 Compare September 21, 2016 23:55

the-dcruz force-pushed the filter-value-selector branch from 1623d7b to c872f7a Compare September 22, 2016 01:29

the-dcruz force-pushed the filter-value-selector branch 2 times, most recently from 7ded4c4 to e539882 Compare September 22, 2016 18:12

the-dcruz force-pushed the filter-value-selector branch from e539882 to 91c2280 Compare September 22, 2016 18:25

vera-liu mentioned this pull request Sep 27, 2016

Explore control panel - Chart control, TimeFilter, GroupBy, Filters #1205

Merged

the-dcruz force-pushed the filter-value-selector branch 3 times, most recently from 32cd717 to a8c3bbb Compare November 8, 2016 01:35

the-dcruz force-pushed the filter-value-selector branch from a8c3bbb to 8191486 Compare November 8, 2016 03:09

Filter Enhancement (apache#933) - Replaced text field with select con…

9f6f544

…taining possible column values

the-dcruz force-pushed the filter-value-selector branch from 8191486 to 9f6f544 Compare November 8, 2016 05:10

mistercrunch closed this Jun 12, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Filter Enhancement (#933) - Filter Value Dropdown #1061

Filter Enhancement (#933) - Filter Value Dropdown #1061

the-dcruz commented Sep 2, 2016

the-dcruz commented Sep 2, 2016

kkalyan commented Sep 6, 2016

mistercrunch commented Sep 11, 2016

the-dcruz commented Sep 12, 2016

the-dcruz commented Sep 12, 2016

ShengyaoQian commented Sep 15, 2016

the-dcruz commented Sep 15, 2016 •

edited

Loading

mistercrunch commented Sep 21, 2016

mistercrunch commented Sep 21, 2016

mistercrunch commented Sep 21, 2016

the-dcruz commented Sep 21, 2016

vera-liu commented Sep 21, 2016

the-dcruz commented Sep 21, 2016

vera-liu commented Sep 21, 2016

the-dcruz commented Sep 21, 2016

vera-liu commented Sep 21, 2016

the-dcruz commented Sep 21, 2016

the-dcruz commented Sep 21, 2016

vera-liu commented Sep 21, 2016 •

edited

Loading

vera-liu Sep 21, 2016

the-dcruz Sep 21, 2016

the-dcruz commented Sep 22, 2016

vera-liu commented Sep 22, 2016 •

edited

Loading

the-dcruz commented Sep 22, 2016

the-dcruz commented Sep 22, 2016

the-dcruz commented Sep 22, 2016

vera-liu commented Sep 26, 2016 •

edited

Loading

mistercrunch commented Oct 7, 2016

kkalyan commented Nov 4, 2016

the-dcruz commented Nov 8, 2016 •

edited

Loading

ascott commented Nov 17, 2016

vera-liu commented Nov 18, 2016

mistercrunch commented Jun 12, 2017

Filter Enhancement (#933) - Filter Value Dropdown #1061

Filter Enhancement (#933) - Filter Value Dropdown #1061

Conversation

the-dcruz commented Sep 2, 2016

Overview

Technical Changes

Screenshots

TODO for Future PR

the-dcruz commented Sep 2, 2016

kkalyan commented Sep 6, 2016

mistercrunch commented Sep 11, 2016

the-dcruz commented Sep 12, 2016

the-dcruz commented Sep 12, 2016

ShengyaoQian commented Sep 15, 2016

the-dcruz commented Sep 15, 2016 • edited Loading

mistercrunch commented Sep 21, 2016

mistercrunch commented Sep 21, 2016

mistercrunch commented Sep 21, 2016

the-dcruz commented Sep 21, 2016

vera-liu commented Sep 21, 2016

the-dcruz commented Sep 21, 2016

vera-liu commented Sep 21, 2016

the-dcruz commented Sep 21, 2016

vera-liu commented Sep 21, 2016

the-dcruz commented Sep 21, 2016

the-dcruz commented Sep 21, 2016

vera-liu commented Sep 21, 2016 • edited Loading

vera-liu Sep 21, 2016

Choose a reason for hiding this comment

the-dcruz Sep 21, 2016

Choose a reason for hiding this comment

the-dcruz commented Sep 22, 2016

vera-liu commented Sep 22, 2016 • edited Loading

the-dcruz commented Sep 22, 2016

the-dcruz commented Sep 22, 2016

the-dcruz commented Sep 22, 2016

vera-liu commented Sep 26, 2016 • edited Loading

mistercrunch commented Oct 7, 2016

kkalyan commented Nov 4, 2016

the-dcruz commented Nov 8, 2016 • edited Loading

ascott commented Nov 17, 2016

vera-liu commented Nov 18, 2016

mistercrunch commented Jun 12, 2017

the-dcruz commented Sep 15, 2016 •

edited

Loading

vera-liu commented Sep 21, 2016 •

edited

Loading

vera-liu commented Sep 22, 2016 •

edited

Loading

vera-liu commented Sep 26, 2016 •

edited

Loading

the-dcruz commented Nov 8, 2016 •

edited

Loading