rusackas commented on code in PR #41780:
URL: https://github.com/apache/superset/pull/41780#discussion_r3567822697
##########
superset/views/core.py:
##########
@@ -129,6 +130,11 @@
SqlResults = dict[str, Any]
+# Matches the locale codes Superset actually ships: a 2-3 letter language
+# subtag, optionally followed by a 2-letter region subtag ("pt_BR") or a
+# 4-letter script subtag ("sr_Latn").
+LANGUAGE_CODE_RE = re.compile(r"^[a-z]{2,3}(_[A-Z]{2}|_[A-Z][a-z]{3})?$")
Review Comment:
Done — annotated the constant as `re.Pattern[str]`.
##########
tests/unit_tests/views/test_bootstrap_auth.py:
##########
@@ -156,71 +156,106 @@ def test_bootstrap_does_not_crash_without_recaptcha_key(
assert "RECAPTCHA_PUBLIC_KEY" not in payload["conf"]
-# --- language_pack injection --------------------------------------------
+# --- language pack delivery ----------------------------------------------
#
-# The Jed pack is injected by `common_bootstrap_payload` (outside the
-# memoized `cached_common_bootstrap_data`) using the shared
-# `superset.translations.utils.get_language_pack`. Tests here cover the
-# wrapper to confirm the pack lands on the payload for non-English
-# locales and is None for English.
+# The pack is NOT embedded in the bootstrap payload; spa.html loads it via a
+# content-addressed script tag whose URL comes from
+# `get_language_pack_template_context`. A pack supplied through
+# COMMON_BOOTSTRAP_OVERRIDES_FUNC still rides the payload and suppresses the
+# script tag.
-def test_common_bootstrap_payload_includes_language_pack_for_non_english(
+def test_common_bootstrap_payload_does_not_embed_language_pack(
app_context: None,
) -> None:
- """common.language_pack carries the shared utility's pack for non-en."""
- fake_pack = {"domain": "superset", "locale_data": {"superset": {}}}
+ """The payload stays small: no full pack even for non-English locales."""
with (
patch(
"superset.views.base.cached_common_bootstrap_data",
return_value={"locale": "fr"},
),
- patch(
- "superset.views.base.get_language_pack",
- return_value=fake_pack,
- ) as mock_get,
patch("superset.views.base.utils.get_user_id", return_value=1),
patch("superset.views.base.get_locale", return_value="fr"),
):
- payload = common_bootstrap_payload()
+ payload: dict[str, Any] = common_bootstrap_payload()
- assert payload["language_pack"] == fake_pack
- mock_get.assert_called_once_with("fr")
+ assert payload["language_pack"] is None
-def test_common_bootstrap_payload_skips_pack_for_english(
+def test_common_bootstrap_payload_preserves_override_pack(
app_context: None,
) -> None:
- """English short-circuits: pack is None and the utility is not called."""
+ """A pack from COMMON_BOOTSTRAP_OVERRIDES_FUNC is passed through."""
+ fake_pack: dict[str, Any] = {"domain": "superset", "locale_data":
{"superset": {}}}
with (
patch(
"superset.views.base.cached_common_bootstrap_data",
- return_value={"locale": "en"},
+ return_value={"locale": "fr", "language_pack": fake_pack},
),
- patch("superset.views.base.get_language_pack") as mock_get,
patch("superset.views.base.utils.get_user_id", return_value=1),
- patch("superset.views.base.get_locale", return_value="en"),
+ patch("superset.views.base.get_locale", return_value="fr"),
):
- payload = common_bootstrap_payload()
+ payload: dict[str, Any] = common_bootstrap_payload()
- assert payload["language_pack"] is None
- mock_get.assert_not_called()
+ assert payload["language_pack"] == fake_pack
def test_common_bootstrap_payload_does_not_mutate_memoized_dict(
app_context: None,
) -> None:
- """Injecting language_pack must not write back into the memoize cache."""
+ """Defaulting language_pack must not write back into the memoize cache."""
cached: dict[str, Any] = {"locale": "fr"}
with (
patch(
"superset.views.base.cached_common_bootstrap_data",
return_value=cached,
),
- patch("superset.views.base.get_language_pack", return_value={"x": 1}),
patch("superset.views.base.utils.get_user_id", return_value=1),
patch("superset.views.base.get_locale", return_value="fr"),
):
common_bootstrap_payload()
assert "language_pack" not in cached
+
+
+def _language_pack_context(
+ locale: str, payload_extra: dict[str, Any]
+) -> dict[str, Any]:
+ from superset.views.base import get_language_pack_template_context
+
+ with (
+ patch(
+ "superset.views.base.get_language_pack_version",
+ return_value="abc123def456",
+ ),
+ patch(
+ "superset.views.base.url_for",
+ return_value="/language_pack/fr/abc123def456/script.js",
+ ),
+ ):
+ return get_language_pack_template_context({"locale": locale,
**payload_extra})
+
+
+def test_language_pack_template_context_versioned_src_for_non_english(
+ app_context: None,
+) -> None:
+ context = _language_pack_context("fr", {})
Review Comment:
Done — annotated as `dict[str, Any]`.
##########
tests/unit_tests/views/test_bootstrap_auth.py:
##########
@@ -156,71 +156,106 @@ def test_bootstrap_does_not_crash_without_recaptcha_key(
assert "RECAPTCHA_PUBLIC_KEY" not in payload["conf"]
-# --- language_pack injection --------------------------------------------
+# --- language pack delivery ----------------------------------------------
#
-# The Jed pack is injected by `common_bootstrap_payload` (outside the
-# memoized `cached_common_bootstrap_data`) using the shared
-# `superset.translations.utils.get_language_pack`. Tests here cover the
-# wrapper to confirm the pack lands on the payload for non-English
-# locales and is None for English.
+# The pack is NOT embedded in the bootstrap payload; spa.html loads it via a
+# content-addressed script tag whose URL comes from
+# `get_language_pack_template_context`. A pack supplied through
+# COMMON_BOOTSTRAP_OVERRIDES_FUNC still rides the payload and suppresses the
+# script tag.
-def test_common_bootstrap_payload_includes_language_pack_for_non_english(
+def test_common_bootstrap_payload_does_not_embed_language_pack(
app_context: None,
) -> None:
- """common.language_pack carries the shared utility's pack for non-en."""
- fake_pack = {"domain": "superset", "locale_data": {"superset": {}}}
+ """The payload stays small: no full pack even for non-English locales."""
with (
patch(
"superset.views.base.cached_common_bootstrap_data",
return_value={"locale": "fr"},
),
- patch(
- "superset.views.base.get_language_pack",
- return_value=fake_pack,
- ) as mock_get,
patch("superset.views.base.utils.get_user_id", return_value=1),
patch("superset.views.base.get_locale", return_value="fr"),
):
- payload = common_bootstrap_payload()
+ payload: dict[str, Any] = common_bootstrap_payload()
- assert payload["language_pack"] == fake_pack
- mock_get.assert_called_once_with("fr")
+ assert payload["language_pack"] is None
-def test_common_bootstrap_payload_skips_pack_for_english(
+def test_common_bootstrap_payload_preserves_override_pack(
app_context: None,
) -> None:
- """English short-circuits: pack is None and the utility is not called."""
+ """A pack from COMMON_BOOTSTRAP_OVERRIDES_FUNC is passed through."""
+ fake_pack: dict[str, Any] = {"domain": "superset", "locale_data":
{"superset": {}}}
with (
patch(
"superset.views.base.cached_common_bootstrap_data",
- return_value={"locale": "en"},
+ return_value={"locale": "fr", "language_pack": fake_pack},
),
- patch("superset.views.base.get_language_pack") as mock_get,
patch("superset.views.base.utils.get_user_id", return_value=1),
- patch("superset.views.base.get_locale", return_value="en"),
+ patch("superset.views.base.get_locale", return_value="fr"),
):
- payload = common_bootstrap_payload()
+ payload: dict[str, Any] = common_bootstrap_payload()
- assert payload["language_pack"] is None
- mock_get.assert_not_called()
+ assert payload["language_pack"] == fake_pack
def test_common_bootstrap_payload_does_not_mutate_memoized_dict(
app_context: None,
) -> None:
- """Injecting language_pack must not write back into the memoize cache."""
+ """Defaulting language_pack must not write back into the memoize cache."""
cached: dict[str, Any] = {"locale": "fr"}
with (
patch(
"superset.views.base.cached_common_bootstrap_data",
return_value=cached,
),
- patch("superset.views.base.get_language_pack", return_value={"x": 1}),
patch("superset.views.base.utils.get_user_id", return_value=1),
patch("superset.views.base.get_locale", return_value="fr"),
):
common_bootstrap_payload()
assert "language_pack" not in cached
+
+
+def _language_pack_context(
+ locale: str, payload_extra: dict[str, Any]
+) -> dict[str, Any]:
+ from superset.views.base import get_language_pack_template_context
+
+ with (
+ patch(
+ "superset.views.base.get_language_pack_version",
+ return_value="abc123def456",
+ ),
+ patch(
+ "superset.views.base.url_for",
+ return_value="/language_pack/fr/abc123def456/script.js",
+ ),
+ ):
+ return get_language_pack_template_context({"locale": locale,
**payload_extra})
+
+
+def test_language_pack_template_context_versioned_src_for_non_english(
+ app_context: None,
+) -> None:
+ context = _language_pack_context("fr", {})
+ assert context == {
+ "language_pack_src": "/language_pack/fr/abc123def456/script.js",
+ "language_pack_inline": False,
+ }
+
+
+def test_language_pack_template_context_none_for_english(
+ app_context: None,
+) -> None:
+ context = _language_pack_context("en", {})
Review Comment:
Done — annotated as `dict[str, Any]`.
##########
tests/unit_tests/views/test_bootstrap_auth.py:
##########
@@ -156,71 +156,106 @@ def test_bootstrap_does_not_crash_without_recaptcha_key(
assert "RECAPTCHA_PUBLIC_KEY" not in payload["conf"]
-# --- language_pack injection --------------------------------------------
+# --- language pack delivery ----------------------------------------------
#
-# The Jed pack is injected by `common_bootstrap_payload` (outside the
-# memoized `cached_common_bootstrap_data`) using the shared
-# `superset.translations.utils.get_language_pack`. Tests here cover the
-# wrapper to confirm the pack lands on the payload for non-English
-# locales and is None for English.
+# The pack is NOT embedded in the bootstrap payload; spa.html loads it via a
+# content-addressed script tag whose URL comes from
+# `get_language_pack_template_context`. A pack supplied through
+# COMMON_BOOTSTRAP_OVERRIDES_FUNC still rides the payload and suppresses the
+# script tag.
-def test_common_bootstrap_payload_includes_language_pack_for_non_english(
+def test_common_bootstrap_payload_does_not_embed_language_pack(
app_context: None,
) -> None:
- """common.language_pack carries the shared utility's pack for non-en."""
- fake_pack = {"domain": "superset", "locale_data": {"superset": {}}}
+ """The payload stays small: no full pack even for non-English locales."""
with (
patch(
"superset.views.base.cached_common_bootstrap_data",
return_value={"locale": "fr"},
),
- patch(
- "superset.views.base.get_language_pack",
- return_value=fake_pack,
- ) as mock_get,
patch("superset.views.base.utils.get_user_id", return_value=1),
patch("superset.views.base.get_locale", return_value="fr"),
):
- payload = common_bootstrap_payload()
+ payload: dict[str, Any] = common_bootstrap_payload()
- assert payload["language_pack"] == fake_pack
- mock_get.assert_called_once_with("fr")
+ assert payload["language_pack"] is None
-def test_common_bootstrap_payload_skips_pack_for_english(
+def test_common_bootstrap_payload_preserves_override_pack(
app_context: None,
) -> None:
- """English short-circuits: pack is None and the utility is not called."""
+ """A pack from COMMON_BOOTSTRAP_OVERRIDES_FUNC is passed through."""
+ fake_pack: dict[str, Any] = {"domain": "superset", "locale_data":
{"superset": {}}}
with (
patch(
"superset.views.base.cached_common_bootstrap_data",
- return_value={"locale": "en"},
+ return_value={"locale": "fr", "language_pack": fake_pack},
),
- patch("superset.views.base.get_language_pack") as mock_get,
patch("superset.views.base.utils.get_user_id", return_value=1),
- patch("superset.views.base.get_locale", return_value="en"),
+ patch("superset.views.base.get_locale", return_value="fr"),
):
- payload = common_bootstrap_payload()
+ payload: dict[str, Any] = common_bootstrap_payload()
- assert payload["language_pack"] is None
- mock_get.assert_not_called()
+ assert payload["language_pack"] == fake_pack
def test_common_bootstrap_payload_does_not_mutate_memoized_dict(
app_context: None,
) -> None:
- """Injecting language_pack must not write back into the memoize cache."""
+ """Defaulting language_pack must not write back into the memoize cache."""
cached: dict[str, Any] = {"locale": "fr"}
with (
patch(
"superset.views.base.cached_common_bootstrap_data",
return_value=cached,
),
- patch("superset.views.base.get_language_pack", return_value={"x": 1}),
patch("superset.views.base.utils.get_user_id", return_value=1),
patch("superset.views.base.get_locale", return_value="fr"),
):
common_bootstrap_payload()
assert "language_pack" not in cached
+
+
+def _language_pack_context(
+ locale: str, payload_extra: dict[str, Any]
+) -> dict[str, Any]:
+ from superset.views.base import get_language_pack_template_context
+
+ with (
+ patch(
+ "superset.views.base.get_language_pack_version",
+ return_value="abc123def456",
+ ),
+ patch(
+ "superset.views.base.url_for",
+ return_value="/language_pack/fr/abc123def456/script.js",
+ ),
+ ):
+ return get_language_pack_template_context({"locale": locale,
**payload_extra})
+
+
+def test_language_pack_template_context_versioned_src_for_non_english(
+ app_context: None,
+) -> None:
+ context = _language_pack_context("fr", {})
+ assert context == {
+ "language_pack_src": "/language_pack/fr/abc123def456/script.js",
+ "language_pack_inline": False,
+ }
+
+
+def test_language_pack_template_context_none_for_english(
+ app_context: None,
+) -> None:
+ context = _language_pack_context("en", {})
+ assert context == {"language_pack_src": None, "language_pack_inline":
False}
+
+
+def test_language_pack_template_context_inline_for_override_pack(
+ app_context: None,
+) -> None:
+ """An operator-supplied pack suppresses the script tag; spa.html
inlines."""
+ context = _language_pack_context("fr", {"language_pack": {"domain":
"superset"}})
Review Comment:
Done — annotated as `dict[str, Any]`.
--
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]
