sadpandajoe commented on code in PR #40919:
URL: https://github.com/apache/superset/pull/40919#discussion_r4418970208
##########
superset-frontend/src/utils/common.ts:
##########
@@ -18,22 +18,17 @@
*/
import {
SupersetClient,
+ getConstantLabel,
getTimeFormatter,
+ EMPTY_STRING,
+ FALSE_STRING,
+ NULL_STRING,
Review Comment:
`NULL_STRING` is now only imported here and no longer exported from
`src/utils/common`, but
`src/explore/components/PartitionPruningIndicator/index.tsx:25` still does
`import { NULL_STRING } from 'src/utils/common'`. That breaks frontend
type-checking (and the currently failing lint/frontend checks), and without it
`value === NULL_STRING` in that file compares against `undefined`, so the
`<NULL>` sentinel is no longer recognized. Should that consumer import from
`@superset-ui/core` instead (or should the re-export stay)?
##########
superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts:
##########
@@ -730,7 +731,7 @@ export function extractSeries(
});
normalized[xAxis] =
datum[xAxis] === null && xAxisType === AxisType.Category
- ? NULL_STRING
+ ? getConstantLabel(NULL_STRING)
Review Comment:
This now replaces the actual categorical x-axis value with the translated
label, and `EchartsTimeseries` copies that value straight into cross-filter and
drill predicates. With `TRANSLATE_SHARED_CONSTANT_LABELS` on and `<NULL>`
customized to `N/A`, clicking the null bar of a categorical chart emits `IN
['N/A']`; the backend only treats the invariant `<NULL>` token as SQL NULL, so
the target charts filter for the literal string instead of null rows (before
this change it emitted `<NULL>`). The same happens in `formatSeriesName` below
(line 823): Pie uses it as both display name and `labelMap` key, so rows
`[null, 'N/A']` collapse into one entry and the null slice filters on `N/A`.
Could the series data keep the invariant token and translate only in the
display formatters?
##########
superset/constants.py:
##########
@@ -21,12 +21,47 @@
from collections.abc import Iterable
from enum import Enum
+from flask import current_app as app
+from flask_babel import lazy_gettext as __
+
+from superset.translations.utils import N_
from superset.utils.backports import StrEnum
DEFAULT_USER_AGENT = "Apache Superset"
-NULL_STRING = "<NULL>"
-EMPTY_STRING = "<empty string>"
+# Shared (back/front) constants registry.
+# Keep in sync with @superset-ui/core/src/utils/sharedConstants.ts.
+EMPTY_STRING = N_("<empty string>")
+FALSE_STRING = N_("FALSE")
+NULL_STRING = N_("<NULL>")
+TRUE_STRING = N_("TRUE")
+
+# select-filter masks and dataMask hydration use this exact predicate to
recognize
+# an explicit clear.
+EMPTY_FILTER_SQL_EXPRESSION = "1 = 0"
+
+# List of shared constants that have a label.
+SHARED_CONSTANT_LABELS: set[str] = {
+ EMPTY_STRING,
+ FALSE_STRING,
+ NULL_STRING,
+ TRUE_STRING,
+}
+
+
+def get_constant_label(value: str | None = None) -> str | dict[str, str]:
+ """
+ Get the translated label for a given constant,
+ or all shared constant labels if value is None.
+ """
+ if app.config.get("TRANSLATE_SHARED_CONSTANT_LABELS", False):
Review Comment:
Neither branch of `get_constant_label` (or its use in
`common_bootstrap_payload`) has a test: the Python test changes only swap
literals for the imported constants. If the flag check were dropped, or the
bootstrap `constant_labels` update removed, nothing would fail. Could you add a
unit test that patches `TRANSLATE_SHARED_CONSTANT_LABELS` and a translation
returning `N/A` for `<NULL>`, asserting the bootstrap payload carries
`{"<NULL>": "N/A", ...}` with the flag on and `{}` with it off, while the
constants themselves stay invariant?
--
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]
