Skip to content

Commit a7d349a

Browse files
fix(drill-to-detail): ensure axis label filters map to original column names (#34694)
Co-authored-by: bito-code-review[bot] <188872107+bito-code-review[bot]@users.noreply.github.com>
1 parent 7a20a65 commit a7d349a

File tree

2 files changed

+131
-1
lines changed

2 files changed

+131
-1
lines changed

superset/views/datasource/utils.py

Lines changed: 48 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,8 @@
1414
# KIND, either express or implied. See the License for the
1515
# specific language governing permissions and limitations
1616
# under the License.
17-
from typing import Any, Optional
17+
import logging
18+
from typing import Any, Iterable, Optional
1819

1920
from flask import current_app as app
2021

@@ -27,6 +28,8 @@
2728
from superset.utils.core import QueryStatus
2829
from superset.views.datasource.schemas import SamplesPayloadSchema
2930

31+
logger = logging.getLogger(__name__)
32+
3033

3134
def get_limit_clause(page: Optional[int], per_page: Optional[int]) -> dict[str, int]:
3235
samples_row_limit = app.config.get("SAMPLES_ROW_LIMIT", 1000)
@@ -44,6 +47,47 @@ def get_limit_clause(page: Optional[int], per_page: Optional[int]) -> dict[str,
4447
return {"row_offset": offset, "row_limit": limit}
4548

4649

50+
def replace_verbose_with_column(
51+
filters: list[dict[str, Any]],
52+
columns: Iterable[Any],
53+
verbose_attr: str = "verbose_name",
54+
column_attr: str = "column_name",
55+
) -> None:
56+
"""
57+
Replace filter 'col' values that match column verbose_name with the column_name.
58+
Operates in-place on the filters list
59+
60+
Args:
61+
filters: List of filter dicts, each must have 'col' key.
62+
columns: Iterable of column objects with verbose_name and column_name.
63+
verbose_attr: Attribute name for verbose/label.
64+
column_attr: Attribute name for actual column name.
65+
"""
66+
for f in filters:
67+
col_value = f.get("col")
68+
if col_value is None:
69+
logger.warning("Filter missing 'col' key: %s", f)
70+
continue
71+
72+
match = None
73+
for col in columns:
74+
if not hasattr(col, verbose_attr) or not hasattr(col, column_attr):
75+
logger.warning(
76+
"Column object %s missing expected attributes '%s' or '%s'",
77+
col,
78+
verbose_attr,
79+
column_attr,
80+
)
81+
continue
82+
83+
if getattr(col, verbose_attr) == col_value:
84+
match = getattr(col, column_attr)
85+
break
86+
87+
if match:
88+
f["col"] = match
89+
90+
4791
def get_samples( # pylint: disable=too-many-arguments
4892
datasource_type: str,
4993
datasource_id: int,
@@ -72,6 +116,9 @@ def get_samples( # pylint: disable=too-many-arguments
72116
force=force,
73117
)
74118
else:
119+
# Use column names replacing verbose column names(Label)
120+
replace_verbose_with_column(payload.get("filters", []), datasource.columns)
121+
75122
# constructing drill detail query
76123
# When query_type == 'samples' the `time filter` will be removed,
77124
# so it is not applicable drill detail query
Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
# Licensed to the Apache Software Foundation (ASF) under one
2+
# or more contributor license agreements. See the NOTICE file
3+
# distributed with this work for additional information
4+
# regarding copyright ownership. The ASF licenses this file
5+
# to you under the Apache License, Version 2.0 (the
6+
# "License"); you may not use this file except in compliance
7+
# with the License. You may obtain a copy of the License at
8+
#
9+
# http://www.apache.org/licenses/LICENSE-2.0
10+
#
11+
# Unless required by applicable law or agreed to in writing,
12+
# software distributed under the License is distributed on an
13+
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
# KIND, either express or implied. See the License for the
15+
# specific language governing permissions and limitations
16+
# under the License.
17+
18+
import pytest
19+
20+
from superset.views.datasource.utils import replace_verbose_with_column
21+
22+
23+
class Column:
24+
def __init__(self, column_name, verbose_name):
25+
self.column_name = column_name
26+
self.verbose_name = verbose_name
27+
28+
29+
class IncompleteColumn:
30+
"""A column missing required attributes."""
31+
32+
def __init__(self, only_name):
33+
self.only_name = only_name
34+
35+
36+
# Test dataset and filters
37+
columns = [
38+
Column("col1", "Column 1"),
39+
Column("col3", "Column 3"),
40+
]
41+
42+
43+
@pytest.mark.parametrize(
44+
"filters, expected",
45+
[
46+
# Normal match, should be replaced with the actual column_name
47+
([{"col": "Column 1"}], [{"col": "col1"}]),
48+
# Multiple filters, should correctly replace all matching columns
49+
(
50+
[{"col": "Column 1"}, {"col": "Column 3"}],
51+
[{"col": "col1"}, {"col": "col3"}],
52+
),
53+
# No matching case, the original value should remain unchanged
54+
([{"col": "Non-existent"}], [{"col": "Non-existent"}]),
55+
# Empty filters, no changes should be made
56+
([], []),
57+
],
58+
)
59+
def test_replace_verbose_with_column(filters, expected):
60+
filters_copy = [dict(f) for f in filters]
61+
replace_verbose_with_column(filters_copy, columns)
62+
assert filters_copy == expected
63+
64+
65+
def test_replace_verbose_with_column_missing_col_key(caplog):
66+
"""Filter dict missing 'col' should trigger a warning and be skipped."""
67+
filters = [{"op": "=="}] # missing "col"
68+
with caplog.at_level("WARNING"):
69+
replace_verbose_with_column(filters, columns)
70+
assert "Filter missing 'col' key:" in caplog.text
71+
# filter should remain unchanged
72+
assert filters == [{"op": "=="}]
73+
74+
75+
def test_replace_verbose_with_column_missing_column_attrs(caplog):
76+
"""Column missing expected attributes should trigger a warning."""
77+
filters = [{"col": "whatever"}]
78+
bad_columns = [IncompleteColumn("broken")]
79+
with caplog.at_level("WARNING"):
80+
replace_verbose_with_column(filters, bad_columns)
81+
assert "missing expected attributes" in caplog.text
82+
# filter should remain unchanged
83+
assert filters == [{"col": "whatever"}]

0 commit comments

Comments
 (0)