gabotorresruiz commented on code in PR #45552:
URL: https://github.com/apache/superset/pull/45552#discussion_r4338680373
##########
superset/connectors/sqla/models.py:
##########
@@ -652,6 +734,126 @@ def _extract_query_context_columns(
return column_names or None
+ @staticmethod
+ def _query_output_references(
+ query_context: str | None,
+ ) -> ChartDisplayReferences:
+ """Collect query outputs for full-access metadata, including sort
fields.
+
+ These references must not authorize chart labels: viewers can
regenerate
+ the query context, and plugins add dependencies that they do not
display.
+ """
+ columns: list[DisplayReference] = []
+ metrics: list[DisplayReference] = []
+ try:
+ payload = json.loads(query_context or "{}")
+ except (TypeError, ValueError):
+ return columns, metrics
+ if not isinstance(payload, dict) or not isinstance(
+ queries := payload.get("queries"), list
+ ):
+ return columns, metrics
+ for query in queries:
+ if not isinstance(query, dict):
+ continue
+ for name, target in (
+ ("columns", columns),
+ ("groupby", columns),
+ ("metrics", metrics),
+ ):
+ if isinstance(references := query.get(name), list):
+ target.extend(
+ item for item in references if isinstance(item, (str,
dict))
+ )
+ return columns, metrics
+
+ @staticmethod
+ def _active_display_form_data(
+ form_data: dict[str, Any], viz_type: str | None
+ ) -> dict[str, Any]:
+ """Exclude table controls retained while their query mode is hidden."""
+ if viz_type not in ("table", "ag-grid-table", "handlebars"):
+ return form_data
+ mode = form_data.get("query_mode")
+ if viz_type == "handlebars" and mode not in ("raw", "aggregate"):
+ # Handlebars' extractQueryFields retains both modes when
unspecified.
+ params: tuple[str, ...] = ("all_columns", "columns", "groupby",
"metrics")
+ return {name: form_data[name] for name in params if name in
form_data}
+ if mode not in ("raw", "aggregate"):
+ # Match both table plugins' getQueryMode for legacy saved charts.
Review Comment:
Question on the scope claim. This prunes retained controls only for `table`,
`ag-grid-table` and `handlebars`, so for every other viz type
`_form_display_references` reads every name in the `DISPLAY_*` tuples whether
or not that viz type owns the control. On this branch a chart saved with
`viz_type: pie`, `groupby: ["region"]` and a leftover `all_columns:
["cost_center"]` returns `{"cost_center": "Cost center", "region": "Region"}`,
so a restricted viewer and an embedded guest get the display name of a column
the pie never renders.
I did try to talk myself out of it. `StandardizedFormData.transform`
rebuilds `form_data` from the target control panel, so a viz switch in Explore
drops the abandoned keys, and the only `visibility` gated controls among the
`DISPLAY_*` names across the shipped plugins are the `query_mode` ones you
already handle. What is left is `params` written outside Explore: older saved
charts, and anything created through the API or an import bundle.
The backend has no view of the control panel registry, so I do not see a
cheap gate here and this may simply be the intended limit. If it is, could the
body and the embedding doc say "saved display controls" rather than "active
saved display controls"? The narrower wording is what a reader will hold you to
for a guest visible field.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
