mirror of
https://github.com/apache/superset.git
synced 2026-04-19 08:04:53 +00:00
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>
This commit is contained in:
@@ -14,7 +14,8 @@
|
||||
# KIND, either express or implied. See the License for the
|
||||
# specific language governing permissions and limitations
|
||||
# under the License.
|
||||
from typing import Any, Optional
|
||||
import logging
|
||||
from typing import Any, Iterable, Optional
|
||||
|
||||
from flask import current_app as app
|
||||
|
||||
@@ -27,6 +28,8 @@ from superset.daos.datasource import DatasourceDAO
|
||||
from superset.utils.core import QueryStatus
|
||||
from superset.views.datasource.schemas import SamplesPayloadSchema
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
def get_limit_clause(page: Optional[int], per_page: Optional[int]) -> dict[str, int]:
|
||||
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,
|
||||
return {"row_offset": offset, "row_limit": limit}
|
||||
|
||||
|
||||
def replace_verbose_with_column(
|
||||
filters: list[dict[str, Any]],
|
||||
columns: Iterable[Any],
|
||||
verbose_attr: str = "verbose_name",
|
||||
column_attr: str = "column_name",
|
||||
) -> None:
|
||||
"""
|
||||
Replace filter 'col' values that match column verbose_name with the column_name.
|
||||
Operates in-place on the filters list
|
||||
|
||||
Args:
|
||||
filters: List of filter dicts, each must have 'col' key.
|
||||
columns: Iterable of column objects with verbose_name and column_name.
|
||||
verbose_attr: Attribute name for verbose/label.
|
||||
column_attr: Attribute name for actual column name.
|
||||
"""
|
||||
for f in filters:
|
||||
col_value = f.get("col")
|
||||
if col_value is None:
|
||||
logger.warning("Filter missing 'col' key: %s", f)
|
||||
continue
|
||||
|
||||
match = None
|
||||
for col in columns:
|
||||
if not hasattr(col, verbose_attr) or not hasattr(col, column_attr):
|
||||
logger.warning(
|
||||
"Column object %s missing expected attributes '%s' or '%s'",
|
||||
col,
|
||||
verbose_attr,
|
||||
column_attr,
|
||||
)
|
||||
continue
|
||||
|
||||
if getattr(col, verbose_attr) == col_value:
|
||||
match = getattr(col, column_attr)
|
||||
break
|
||||
|
||||
if match:
|
||||
f["col"] = match
|
||||
|
||||
|
||||
def get_samples( # pylint: disable=too-many-arguments
|
||||
datasource_type: str,
|
||||
datasource_id: int,
|
||||
@@ -72,6 +116,9 @@ def get_samples( # pylint: disable=too-many-arguments
|
||||
force=force,
|
||||
)
|
||||
else:
|
||||
# Use column names replacing verbose column names(Label)
|
||||
replace_verbose_with_column(payload.get("filters", []), datasource.columns)
|
||||
|
||||
# constructing drill detail query
|
||||
# When query_type == 'samples' the `time filter` will be removed,
|
||||
# so it is not applicable drill detail query
|
||||
|
||||
@@ -0,0 +1,83 @@
|
||||
# Licensed to the Apache Software Foundation (ASF) under one
|
||||
# or more contributor license agreements. See the NOTICE file
|
||||
# distributed with this work for additional information
|
||||
# regarding copyright ownership. The ASF licenses this file
|
||||
# to you under the Apache License, Version 2.0 (the
|
||||
# "License"); you may not use this file except in compliance
|
||||
# with the License. You may obtain a copy of the License at
|
||||
#
|
||||
# http://www.apache.org/licenses/LICENSE-2.0
|
||||
#
|
||||
# Unless required by applicable law or agreed to in writing,
|
||||
# software distributed under the License is distributed on an
|
||||
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
|
||||
# KIND, either express or implied. See the License for the
|
||||
# specific language governing permissions and limitations
|
||||
# under the License.
|
||||
|
||||
import pytest
|
||||
|
||||
from superset.views.datasource.utils import replace_verbose_with_column
|
||||
|
||||
|
||||
class Column:
|
||||
def __init__(self, column_name, verbose_name):
|
||||
self.column_name = column_name
|
||||
self.verbose_name = verbose_name
|
||||
|
||||
|
||||
class IncompleteColumn:
|
||||
"""A column missing required attributes."""
|
||||
|
||||
def __init__(self, only_name):
|
||||
self.only_name = only_name
|
||||
|
||||
|
||||
# Test dataset and filters
|
||||
columns = [
|
||||
Column("col1", "Column 1"),
|
||||
Column("col3", "Column 3"),
|
||||
]
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"filters, expected",
|
||||
[
|
||||
# Normal match, should be replaced with the actual column_name
|
||||
([{"col": "Column 1"}], [{"col": "col1"}]),
|
||||
# Multiple filters, should correctly replace all matching columns
|
||||
(
|
||||
[{"col": "Column 1"}, {"col": "Column 3"}],
|
||||
[{"col": "col1"}, {"col": "col3"}],
|
||||
),
|
||||
# No matching case, the original value should remain unchanged
|
||||
([{"col": "Non-existent"}], [{"col": "Non-existent"}]),
|
||||
# Empty filters, no changes should be made
|
||||
([], []),
|
||||
],
|
||||
)
|
||||
def test_replace_verbose_with_column(filters, expected):
|
||||
filters_copy = [dict(f) for f in filters]
|
||||
replace_verbose_with_column(filters_copy, columns)
|
||||
assert filters_copy == expected
|
||||
|
||||
|
||||
def test_replace_verbose_with_column_missing_col_key(caplog):
|
||||
"""Filter dict missing 'col' should trigger a warning and be skipped."""
|
||||
filters = [{"op": "=="}] # missing "col"
|
||||
with caplog.at_level("WARNING"):
|
||||
replace_verbose_with_column(filters, columns)
|
||||
assert "Filter missing 'col' key:" in caplog.text
|
||||
# filter should remain unchanged
|
||||
assert filters == [{"op": "=="}]
|
||||
|
||||
|
||||
def test_replace_verbose_with_column_missing_column_attrs(caplog):
|
||||
"""Column missing expected attributes should trigger a warning."""
|
||||
filters = [{"col": "whatever"}]
|
||||
bad_columns = [IncompleteColumn("broken")]
|
||||
with caplog.at_level("WARNING"):
|
||||
replace_verbose_with_column(filters, bad_columns)
|
||||
assert "missing expected attributes" in caplog.text
|
||||
# filter should remain unchanged
|
||||
assert filters == [{"col": "whatever"}]
|
||||
Reference in New Issue
Block a user