feat(native-filters): Time native filter #12992

simcha90 · 2021-02-07T16:41:38Z

SUMMARY

This PR is continue of @agatapst PR: #12946

Here description of ☝️ PR:

The aim of this PR was to create foundations for new native filters' feature: time filters.
User can choose between one more filter type, which is Time.

That's the basics for this feature. What should be added in next PRs:

- UI improvements (if we want to show modal in modal, how to display very long time "pills" in Filter Bar etc.)
- Improve Filter Config Modal validation - right now datasource is required, but in case of time filters it should not
- Implement additional filtering: i.e. inverse
- Might also take a look on performance - it take more time to show time filters result than select filters result
- Filter indicator logic needs to be updated

In addition this PR fix some edge cases.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TEST PLAN

ADDITIONAL INFORMATION

codecov-io · 2021-02-07T16:53:45Z

Codecov Report

Merging #12992 (a71d35b) into master (2ce7982) will increase coverage by 14.02%.
The diff coverage is 31.11%.

@@             Coverage Diff             @@
##           master   #12992       +/-   ##
===========================================
+ Coverage   53.06%   67.08%   +14.02%     
===========================================
  Files         489      491        +2     
  Lines       17314    28947    +11633     
  Branches     4482        0     -4482     
===========================================
+ Hits         9187    19419    +10232     
- Misses       8127     9528     +1401

Flag	Coverage Δ
cypress	`?`
python	`67.08% <31.11%> (?)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
superset/examples/birth_names.py	`73.19% <ø> (ø)`
...43f2fdb_add_granularity_to_charts_where_missing.py	`0.00% <0.00%> (ø)`
...s/260bf0649a77_migrate_x_dateunit_in_time_range.py	`0.00% <0.00%> (ø)`
...ons/versions/41ce8799acc3_rename_pie_label_type.py	`0.00% <0.00%> (ø)`
superset/connectors/sqla/views.py	`62.43% <4.34%> (ø)`
superset/views/datasource.py	`88.70% <16.66%> (ø)`
superset/charts/commands/exceptions.py	`92.85% <77.77%> (ø)`
superset/utils/core.py	`87.99% <82.05%> (ø)`
superset/db_engine_specs/elasticsearch.py	`89.74% <93.75%> (ø)`
superset/config.py	`90.68% <100.00%> (ø)`
... and 936 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 956f276...a71d35b. Read the comment docs.

amitmiran137 · 2021-02-07T17:41:47Z

cc: @junlincc

villebro

The native filter changes look good, but I refer the review of time picker changes to @zhaoyongjie .

zhaoyongjie · 2021-02-08T11:45:45Z

Hi @simcha90
control layout issue on the latest Firefox

zhaoyongjie · 2021-02-08T12:09:26Z

set native filter in dashboard
apply this filter in dashboard. select last day in time filter (no such records in cleaned sales data), expected display no data in each chart in the dashboard.
not apply filter

� Conflicts: � superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBar.tsx � superset-frontend/src/dashboard/components/nativeFilters/types.ts � superset-frontend/src/dashboard/components/nativeFilters/utils.ts

zhaoyongjie · 2021-02-08T12:38:49Z

superset-frontend/src/explore/components/controls/DateFilterControl/DateFilterControl.tsx

  onChange: (timeRange: string) => void;
+  onTimeRangeChange?: (textValue?: string, timeRange?: string) => void;


onChange and onTimeRangeChange seem to have the same function, can we merge these props?

superset-frontend/src/filters/components/Time/AntdTimeFilter.tsx

zhaoyongjie · 2021-02-08T13:04:02Z

superset-frontend/src/explore/components/controls/DateFilterControl/DateFilterControl.tsx

-      response?.json?.result?.until || '',
-    );
+    const { since = '', until = '' } = response?.json?.result || {};
+    const timeRangeString = buildTimeRangeString(since, until);


NEED to double check here.
timeRangeString is Absolute time range.
eg:
if today is 2021-2-8, then last day, generate "2021-02-07 : 2021-02-08".
when the 2021-3-8, then last day generate "2021-03-07 : 2021-03-08"

zhaoyongjie

need double check

junlincc · 2021-02-08T15:27:08Z

@simcha90 thanks for taking over the project! really appreciate you work

tested and the basic functionality is working, can not fully test until the filter is connected to the indicator. @zhaoyongjie we should either skip the time column select or pre-select dttm column for your user, or simply narrow it down to show date column only in select dropdown.
Also there's noticeable layout issue of regular filter on the left bar.
Priority fo next steps -

Layout - make sure user can change filter value of regular filter (reason: we need to keep this feature in minimal demo-able state)

connect to filter indicator and magnifier
Improve Filter Config Modal validation, populate only date/datetime/time columns in select
Performance
Inverse

Screen.Recording.2021-02-08.at.7.17.12.AM.mov

junlincc · 2021-02-08T15:28:58Z

need double check

@villebro @simcha90 @zhaoyongjie what is the scope of this PR?
i listed my priority in last comment, can we make incremental changes based on the order?

junlincc · 2021-02-09T20:46:46Z

we also lost the indicator hook that we Agata previously implemented for the regular filter type. @simcha90 could you please double check? this is not ready to merge yet. @villebro

simcha90 · 2021-02-10T12:11:14Z

@junlincc I think scope of this PR - is ability to set TimeFilter as native filter and also set default value for it. All other additional features should be implemented in other PRs, and can be done in different times after this PR will be merged, because this PR is base PR for all other features. cc @villebro

zhaoyongjie · 2021-02-10T13:23:50Z

Hi, @simcha90, @villebro, @junlincc

The implementation of this PR still needs to be considered. Now the filter passed by the native filter to each chart is the specific time. For example, if you choose, Last week, it will transform to 2021-01-01 : 2021-01-02. This applies to The chart filter will not change. Imagine a scenario, this will cause a problem. After you select Last week, set the Dashboard to refresh by every minute. Then the chart will not update the filter.

superset-frontend/src/filters/components/Time/index.ts

simcha90 · 2021-02-11T15:26:19Z

superset-frontend/src/filters/components/Time/index.ts

+    const metadata = new ChartMetadata({
+      name: t('Time range filter plugin'),
+      description: 'Custom time filter plugin',
+      behaviors: [Behavior.NATIVE_FILTER],


it can be also CROSS_FILTER

simcha90 · 2021-02-11T15:38:03Z

@villebro

I think it have some inconsistencies, like if I choose Custom default value it shows like this
If I choose previous week it shows dates in filter bar instead of previous week, but after reload it shows previous week

zhaoyongjie · 2021-02-12T04:40:10Z

superset-frontend/src/filters/components/Time/AntdTimeFilter.tsx

+    <Styles width={width}>
+      <DateFilterControl
+        value={value}
+        name=""


one nit:
could we set a default name for DateFilterControl ?

zhaoyongjie

LGTM, thanks ville.

junlincc · 2021-02-12T15:22:33Z

LGTM! ✅
will be connecting to filter indicators and adding unit test in follow up PRs.

* Add Time Filter component * Improve Time Filter component * Fix import errors * Display Time Filter * Remove console logs * Change Control Panel * Remove unnecessary files * Use time range override * test: fix tests * feat: re run pipeline * fix: fix some case for Time filter * fix: merge with master * use original time range * fix height * add cross filter behavior * apply filters on initialization * add applied filter to overrides * add unit tests for merge_extra_form_data Co-authored-by: Agata Stawarz-Pastewska <[email protected]> Co-authored-by: Ville Brofeldt <[email protected]>

* master: (30 commits) refactor(native-filters): decouple params from filter config modal (first phase) (apache#13021) fix(native-filters): set currentValue null when empty (apache#13000) Custom superset_config.py + secret envs (apache#13096) Update http error code from 400 to 403 (apache#13061) feat(native-filters): add storybook entry for select filter (apache#13005) feat(native-filters): Time native filter (apache#12992) Force pod restart on config changes (apache#13056) feat(cross-filters): add cross filters (apache#12662) fix(explore): Enable selecting an option not included in suggestions (apache#13029) Improves RTL configuration (apache#13079) Added a note about the ! prefix for breaking changes to CONTRIBUTING.md (apache#13083) chore: lock down npm to v6 (apache#13069) fix: API tests, make them possible to run independently again (apache#13076) fix: add config to disable dataset ownership on the old api (apache#13051) add required * indicator to message content/notif method (apache#12931) fix: Retroactively add granularity param to charts (apache#12960) fix(ci): multiline regex in change detection (apache#13075) feat(style): hide dashboard header by url parameter (apache#12918) fix(explore): pie chart label bugs (apache#13052) fix: Disabled state button transition time (apache#13008) ...

agatapst and others added 11 commits February 3, 2021 14:41

Add Time Filter component

a29c6d0

Improve Time Filter component

1c3f891

Fix import errors

a7af2e9

Display Time Filter

4ed0cb5

Remove console logs

82a0320

Change Control Panel

a727bfe

Remove unnecessary files

c2ba1c7

Use time range override

ff28359

test: fix tests

158d86b

feat: re run pipeline

5260fca

fix: fix some case for Time filter

56c5fc4

simcha90 changed the title ~~feat(native-filters) Time native filter~~ feat(native-filters): Time native filter Feb 7, 2021

junlincc added the dashboard:native-filters Related to the native filters of the Dashboard label Feb 8, 2021

junlincc requested review from suddjian and zhaoyongjie February 8, 2021 04:22

villebro approved these changes Feb 8, 2021

View reviewed changes

villebro mentioned this pull request Feb 8, 2021

feat(native-filters): Add time filters #12946

Closed

6 tasks

zhaoyongjie reviewed Feb 8, 2021

View reviewed changes

fix: merge with master

a0619c0

zhaoyongjie reviewed Feb 8, 2021

View reviewed changes

superset-frontend/src/filters/components/Time/AntdTimeFilter.tsx Show resolved Hide resolved

zhaoyongjie reviewed Feb 8, 2021

View reviewed changes

junlincc added the hold:testing! On hold for testing label Feb 9, 2021

villebro added 2 commits February 11, 2021 14:06

Merge branch 'master' into time-cont

e3de8a6

use original time range

111b2d0

simcha90 commented Feb 11, 2021

View reviewed changes

superset-frontend/src/filters/components/Time/index.ts Outdated Show resolved Hide resolved

fix height

c52617c

villebro force-pushed the time-cont branch from 7bb75d2 to c52617c Compare February 11, 2021 13:30

Merge branch 'master' into time-cont

639dac2

simcha90 commented Feb 11, 2021

View reviewed changes

zhaoyongjie reviewed Feb 12, 2021

View reviewed changes

zhaoyongjie approved these changes Feb 12, 2021

View reviewed changes

villebro added 4 commits February 12, 2021 08:48

add cross filter behavior

a67b0fe

apply filters on initialization

0659f3f

Merge branch 'master' into time-cont

7063b2e

add applied filter to overrides

23592ea

villebro force-pushed the time-cont branch from 72613cc to 23592ea Compare February 12, 2021 10:27

junlincc self-requested a review February 12, 2021 15:29

junlincc approved these changes Feb 12, 2021

View reviewed changes

add unit tests for merge_extra_form_data

a71d35b

villebro merged commit d6fc720 into apache:master Feb 13, 2021

junlincc removed the hold:testing! On hold for testing label Mar 12, 2021

mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.2.0 labels Mar 12, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(native-filters): Time native filter #12992

feat(native-filters): Time native filter #12992

simcha90 commented Feb 7, 2021 •

edited

Loading

codecov-io commented Feb 7, 2021 •

edited

Loading

amitmiran137 commented Feb 7, 2021

villebro left a comment

zhaoyongjie commented Feb 8, 2021 •

edited

Loading

zhaoyongjie commented Feb 8, 2021 •

edited

Loading

zhaoyongjie Feb 8, 2021

zhaoyongjie Feb 8, 2021

zhaoyongjie left a comment

junlincc commented Feb 8, 2021 •

edited

Loading

junlincc commented Feb 8, 2021

junlincc commented Feb 9, 2021 •

edited

Loading

simcha90 commented Feb 10, 2021 •

edited

Loading

zhaoyongjie commented Feb 10, 2021 •

edited

Loading

simcha90 Feb 11, 2021

simcha90 commented Feb 11, 2021 •

edited

Loading

zhaoyongjie Feb 12, 2021 •

edited

Loading

zhaoyongjie left a comment

junlincc commented Feb 12, 2021 •

edited

Loading

		onChange: (timeRange: string) => void;
		onTimeRangeChange?: (textValue?: string, timeRange?: string) => void;

feat(native-filters): Time native filter #12992

feat(native-filters): Time native filter #12992

Conversation

simcha90 commented Feb 7, 2021 • edited Loading

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TEST PLAN

ADDITIONAL INFORMATION

codecov-io commented Feb 7, 2021 • edited Loading

Codecov Report

amitmiran137 commented Feb 7, 2021

villebro left a comment

Choose a reason for hiding this comment

zhaoyongjie commented Feb 8, 2021 • edited Loading

zhaoyongjie commented Feb 8, 2021 • edited Loading

zhaoyongjie Feb 8, 2021

Choose a reason for hiding this comment

zhaoyongjie Feb 8, 2021

Choose a reason for hiding this comment

zhaoyongjie left a comment

Choose a reason for hiding this comment

junlincc commented Feb 8, 2021 • edited Loading

junlincc commented Feb 8, 2021

junlincc commented Feb 9, 2021 • edited Loading

simcha90 commented Feb 10, 2021 • edited Loading

zhaoyongjie commented Feb 10, 2021 • edited Loading

simcha90 Feb 11, 2021

Choose a reason for hiding this comment

simcha90 commented Feb 11, 2021 • edited Loading

zhaoyongjie Feb 12, 2021 • edited Loading

Choose a reason for hiding this comment

zhaoyongjie left a comment

Choose a reason for hiding this comment

junlincc commented Feb 12, 2021 • edited Loading

simcha90 commented Feb 7, 2021 •

edited

Loading

codecov-io commented Feb 7, 2021 •

edited

Loading

zhaoyongjie commented Feb 8, 2021 •

edited

Loading

zhaoyongjie commented Feb 8, 2021 •

edited

Loading

junlincc commented Feb 8, 2021 •

edited

Loading

junlincc commented Feb 9, 2021 •

edited

Loading

simcha90 commented Feb 10, 2021 •

edited

Loading

zhaoyongjie commented Feb 10, 2021 •

edited

Loading

simcha90 commented Feb 11, 2021 •

edited

Loading

zhaoyongjie Feb 12, 2021 •

edited

Loading

junlincc commented Feb 12, 2021 •

edited

Loading