rusackas commented on code in PR #36951:
URL: https://github.com/apache/superset/pull/36951#discussion_r3455914381
##########
superset/config.py:
##########
@@ -1008,9 +1019,10 @@ class D3TimeFormat(TypedDict, total=False):
"brandLogoAlt": "Apache Superset",
"brandLogoUrl": APP_ICON,
"brandLogoMargin": "18px 0",
- "brandLogoHref": "/",
+ "brandLogoHref": LOGO_TARGET_PATH or "/",
Review Comment:
Yep, you're right... THEME_DEFAULT is built way up top, long before
superset_config.py loads at the bottom of config.py, so setting just
LOGO_TARGET_PATH never took. Pushed a fix (26d5129f9c) that re-syncs the logo
link from the final LOGO_TARGET_PATH after the overrides are applied (covers
THEME_DARK too). Confirmed an override actually propagates now.
##########
tests/unit_tests/config_test.py:
##########
@@ -312,3 +312,24 @@ def test_full_setting(
assert dttm_col.is_dttm
assert dttm_col.python_date_format == "epoch_s"
assert dttm_col.expression == "CAST(dttm as INTEGER)"
+
+
+def test_theme_default_logo_target_path() -> None:
+ """
+ Test that THEME_DEFAULT properly uses LOGO_TARGET_PATH for brandLogoHref.
+
+ When LOGO_TARGET_PATH is None (default), brandLogoHref should be "/".
+ When LOGO_TARGET_PATH is set, it should be used instead.
+ """
+ from superset import config
+
+ # Verify default behavior: LOGO_TARGET_PATH is None, so brandLogoHref is
"/"
+ assert config.LOGO_TARGET_PATH is None
+ assert config.THEME_DEFAULT["token"]["brandLogoHref"] == "/"
+
+ # Verify the expression logic works correctly for custom values
+ custom_path = "https://example.com";
+ assert (custom_path or "/") == custom_path
Review Comment:
Good call, that assertion wasn't really testing anything. Replaced it with
one that exercises the wiring itself (sync_theme_logo_href with a custom path,
and a None no-op) in 26d5129f9c.
--
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]
