Skip to content

Commit

Permalink
fix(native-filters): ignore unset filter box time range (apache#16854)
Browse files Browse the repository at this point in the history
  • Loading branch information
villebro authored and Emmanuel Bavoux committed Nov 14, 2021
1 parent cb87a76 commit 1e8233a
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 7 deletions.
4 changes: 2 additions & 2 deletions superset/utils/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -1175,7 +1175,7 @@ def get_filter_key(f: Dict[str, Any]) -> str:
time_extra = date_options.get(filter_column)
if time_extra:
time_extra_value = filtr.get("val")
if time_extra_value:
if time_extra_value and time_extra_value != NO_TIME_RANGE:
form_data[time_extra] = time_extra_value
applied_time_extras[filter_column] = time_extra_value
elif filtr["val"]:
Expand Down Expand Up @@ -1640,7 +1640,7 @@ def get_time_filter_status(
)

time_range = applied_time_extras.get(ExtraFiltersTimeColumnType.TIME_RANGE)
if time_range and time_range != NO_TIME_RANGE:
if time_range:
# are there any temporal columns to assign the time grain to?
if temporal_columns:
applied.append({"column": ExtraFiltersTimeColumnType.TIME_RANGE})
Expand Down
44 changes: 43 additions & 1 deletion tests/integration_tests/utils_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
convert_legacy_filters_into_adhoc,
create_ssl_cert_file,
DTTM_ALIAS,
extract_dataframe_dtypes,
format_timedelta,
GenericDataType,
get_form_data_token,
Expand All @@ -60,10 +61,10 @@
merge_extra_filters,
merge_extra_form_data,
merge_request_params,
NO_TIME_RANGE,
normalize_dttm_col,
parse_ssl_cert,
parse_js_uri_path_item,
extract_dataframe_dtypes,
split,
TimeRangeEndpoint,
validate_json,
Expand Down Expand Up @@ -850,6 +851,47 @@ def test_merge_extra_filters_with_no_extras(self):
form_data, {"time_range": "Last 10 days", "adhoc_filters": [],},
)

def test_merge_extra_filters_with_unset_legacy_time_range(self):
"""
Make sure native filter is applied if filter box time range is unset.
"""
form_data = {
"time_range": "Last 10 days",
"extra_filters": [
{"col": "__time_range", "op": "==", "val": NO_TIME_RANGE},
],
"extra_form_data": {"time_range": "Last year"},
}
merge_extra_filters(form_data)
self.assertEqual(
form_data,
{
"time_range": "Last year",
"applied_time_extras": {},
"adhoc_filters": [],
},
)

def test_merge_extra_filters_with_conflicting_time_ranges(self):
"""
Make sure filter box takes precedence if both native filter and filter box
time ranges are set.
"""
form_data = {
"time_range": "Last 10 days",
"extra_filters": [{"col": "__time_range", "op": "==", "val": "Last week"}],
"extra_form_data": {"time_range": "Last year",},
}
merge_extra_filters(form_data)
self.assertEqual(
form_data,
{
"time_range": "Last week",
"applied_time_extras": {"__time_range": "Last week"},
"adhoc_filters": [],
},
)

def test_merge_extra_filters_with_extras(self):
form_data = {
"time_range": "Last 10 days",
Expand Down
4 changes: 0 additions & 4 deletions tests/unit_tests/core_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,10 +116,6 @@ def test_get_time_filter_status_time_col():
def test_get_time_filter_status_time_range():
dataset = get_dataset_mock()

assert get_time_filter_status(
dataset, {ExtraFiltersTimeColumnType.TIME_RANGE: NO_TIME_RANGE}
) == ([], [])

assert get_time_filter_status(
dataset, {ExtraFiltersTimeColumnType.TIME_RANGE: "1 year ago"}
) == ([{"column": ExtraFiltersTimeColumnType.TIME_RANGE}], [])
Expand Down

0 comments on commit 1e8233a

Please sign in to comment.