Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(contextmenu): uncaught TypeError #28203

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
+65 −5
Open

fix(contextmenu): uncaught TypeError #28203

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
9c0d1f4
Resolve context menu crash on charts w/o dimensions
sowo Apr 8, 2024
f38bdd6
Fix context menu crash
sowo Apr 24, 2024
da81ff2
linting and unit test
sowo Apr 25, 2024
9a93421
linting and unit test
sowo Apr 25, 2024
13ac6c3
linting and unit test
sowo Apr 25, 2024
3d2c270
linting and unit test
sowo Apr 25, 2024
89118fc
linting
sowo May 3, 2024
d3c0338
more linting
sowo May 3, 2024
6a4364c
mypy fixes
sowo May 7, 2024
bd00154
fix issue with adhoc query for chart
sowo May 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 20 additions & 0 deletions superset/common/query_context_processor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@
get_column_names_from_columns,
get_column_names_from_metrics,
get_metric_names,
is_adhoc_metric,
is_adhoc_column,
get_xaxis_label,
normalize_dttm_col,
TIME_COMPARISON,
Expand Down Expand Up @@ -177,6 +179,24 @@ def get_df_payload(
]
for col in cache.df.columns.values
}
label_map.update({
unescape_separator(column_name): [
unescape_separator(col)
for col in re.split(r"(?<!\\),\s", query_obj.columns[idx]
if not is_adhoc_column(query_obj.columns[idx])
else query_obj.columns[idx]["sqlExpression"])
]
for idx, column_name in enumerate(query_obj.column_names)
})
label_map.update({
unescape_separator(metric_name): [
unescape_separator(metric)
for metric in re.split(r"(?<!\\),\s", query_obj.metrics[idx]
if not is_adhoc_metric(query_obj.metrics[idx])
else query_obj.metrics[idx]["sqlExpression"])
]
for idx, metric_name in enumerate(query_obj.metric_names) if query_obj
})
cache.df.columns = [unescape_separator(col) for col in cache.df.columns.values]

return {
Expand Down
2 changes: 1 addition & 1 deletion superset/common/query_object.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ def _move_deprecated_extra_fields(self, kwargs: dict[str, Any]) -> None:
@property
def metric_names(self) -> list[str]:
"""Return metrics names (labels), coerce adhoc metrics to strings."""
return get_metric_names(self.metrics or [])
return get_metric_names(self.metrics or [], self.datasource.verbose_map)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we add a small unit test to verify that metric_names will always return properly?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hughhhh I've added a unit test and some sanity checking. Not sure if the implementation of those checks (https://github.com/sowo/superset/blob/bd0015426aab9ed949259454e36572473b927c39/superset/common/query_context_processor.py#L187 and https://github.com/sowo/superset/blob/bd0015426aab9ed949259454e36572473b927c39/superset/common/query_context_processor.py#L197) is optimal or if there is a better way to handle AdhocColumns and AdhocMetrics. Please advise.


@property
def column_names(self) -> list[str]:
Expand Down
5 changes: 3 additions & 2 deletions superset/connectors/sqla/models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -420,7 +420,7 @@ def data_for_slices( # pylint: disable=too-many-locals
# pull out all required metrics from the form_data
for metric_param in METRIC_FORM_DATA_PARAMS:
for metric in utils.as_list(form_data.get(metric_param) or []):
metric_names.add(utils.get_metric_name(metric))
metric_names.add(utils.get_metric_name(metric,self.verbose_map))
if utils.is_adhoc_metric(metric):
column_ = metric.get("column") or {}
if column_name := column_.get("column_name"):
Expand Down Expand Up @@ -470,6 +470,7 @@ def data_for_slices( # pylint: disable=too-many-locals
metric
for metric in data["metrics"]
if metric["metric_name"] in metric_names
or metric["verbose_name"] in metric_names
]

filtered_columns: list[Column] = []
Expand Down Expand Up @@ -1472,7 +1473,7 @@ def adhoc_metric_to_sqla(
:rtype: sqlalchemy.sql.column
"""
expression_type = metric.get("expressionType")
label = utils.get_metric_name(metric)
label = utils.get_metric_name(metric,self.verbose_map)

if expression_type == utils.AdhocMetricExpressionType.SIMPLE:
metric_column = metric.get("column") or {}
Expand Down