mirror of
https://github.com/apache/superset.git
synced 2026-04-07 10:31:50 +00:00
fix(theme): migrate APP_NAME to brandAppName theme token with backward compatibility (#37370)
Co-authored-by: Rafael Benitez <rebenitez1802@gmail.com> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Beto Dealmeida <roberto@dealmeida.net> Co-authored-by: codeant-ai-for-open-source[bot] <244253245+codeant-ai-for-open-source[bot]@users.noreply.github.com>
This commit is contained in:
committed by
GitHub
parent
3ca8c998ab
commit
760227d630
24
UPDATING.md
24
UPDATING.md
@@ -163,6 +163,28 @@ See `superset/mcp_service/PRODUCTION.md` for deployment guides.
|
||||
- [35062](https://github.com/apache/superset/pull/35062): Changed the function signature of `setupExtensions` to `setupCodeOverrides` with options as arguments.
|
||||
|
||||
### Breaking Changes
|
||||
- [37370](https://github.com/apache/superset/pull/37370): The `APP_NAME` configuration variable no longer controls the browser window/tab title or other frontend branding. Application names should now be configured using the theme system with the `brandAppName` token. The `APP_NAME` config is still used for backend contexts (MCP service, logs, etc.) and serves as a fallback if `brandAppName` is not set.
|
||||
- **Migration:**
|
||||
```python
|
||||
# Before (Superset 5.x)
|
||||
APP_NAME = "My Custom App"
|
||||
|
||||
# After (Superset 6.x) - Option 1: Use theme system (recommended)
|
||||
THEME_DEFAULT = {
|
||||
"token": {
|
||||
"brandAppName": "My Custom App", # Window titles
|
||||
"brandLogoAlt": "My Custom App", # Logo alt text
|
||||
"brandLogoUrl": "/static/assets/images/custom_logo.png"
|
||||
}
|
||||
}
|
||||
|
||||
# After (Superset 6.x) - Option 2: Temporary fallback
|
||||
# Keep APP_NAME for now (will be used as fallback for brandAppName)
|
||||
APP_NAME = "My Custom App"
|
||||
# But you should migrate to THEME_DEFAULT.token.brandAppName
|
||||
```
|
||||
- **Note:** For dark mode, set the same tokens in `THEME_DARK` configuration.
|
||||
|
||||
- [36317](https://github.com/apache/superset/pull/36317): The `CUSTOM_FONT_URLS` configuration option has been removed. Use the new per-theme `fontUrls` token in `THEME_DEFAULT` or database-managed themes instead.
|
||||
- **Before:**
|
||||
```python
|
||||
@@ -177,7 +199,7 @@ See `superset/mcp_service/PRODUCTION.md` for deployment guides.
|
||||
"fontUrls": [
|
||||
"https://fonts.example.com/myfont.css",
|
||||
],
|
||||
# ... other tokens
|
||||
# ... other tokens
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
@@ -119,6 +119,7 @@ export interface SupersetSpecificTokens {
|
||||
|
||||
// Brand-related
|
||||
brandIconMaxWidth: number;
|
||||
brandAppName?: string;
|
||||
brandLogoAlt: string;
|
||||
brandLogoUrl: string;
|
||||
brandLogoMargin: string;
|
||||
|
||||
@@ -223,15 +223,28 @@ export const DashboardPage: FC<PageProps> = ({ idOrSlug }: PageProps) => {
|
||||
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||
}, [readyToRender]);
|
||||
|
||||
// Capture original title before any effects run
|
||||
const originalTitle = useMemo(() => document.title, []);
|
||||
|
||||
// Update document title when dashboard title changes
|
||||
useEffect(() => {
|
||||
if (dashboard_title) {
|
||||
document.title = dashboard_title;
|
||||
}
|
||||
return () => {
|
||||
document.title = 'Superset';
|
||||
};
|
||||
}, [dashboard_title]);
|
||||
|
||||
// Restore original title on unmount
|
||||
useEffect(
|
||||
() => () => {
|
||||
document.title =
|
||||
originalTitle ||
|
||||
theme?.brandAppName ||
|
||||
theme?.brandLogoAlt ||
|
||||
'Superset';
|
||||
},
|
||||
[originalTitle, theme?.brandAppName, theme?.brandLogoAlt],
|
||||
);
|
||||
|
||||
useEffect(() => {
|
||||
if (typeof css === 'string') {
|
||||
// returning will clean up custom css
|
||||
|
||||
@@ -291,15 +291,28 @@ function ExploreViewContainer(props) {
|
||||
|
||||
const theme = useTheme();
|
||||
|
||||
// Capture original title before any effects run
|
||||
const originalTitle = useMemo(() => document.title, []);
|
||||
|
||||
// Update document title when slice name changes
|
||||
useEffect(() => {
|
||||
if (props.sliceName) {
|
||||
document.title = props.sliceName;
|
||||
}
|
||||
return () => {
|
||||
document.title = 'Superset';
|
||||
};
|
||||
}, [props.sliceName]);
|
||||
|
||||
// Restore original title on unmount
|
||||
useEffect(
|
||||
() => () => {
|
||||
document.title =
|
||||
originalTitle ||
|
||||
theme?.brandAppName ||
|
||||
theme?.brandLogoAlt ||
|
||||
'Superset';
|
||||
},
|
||||
[originalTitle, theme?.brandAppName, theme?.brandLogoAlt],
|
||||
);
|
||||
|
||||
const addHistory = useCallback(
|
||||
async ({ isReplace = false, title } = {}) => {
|
||||
const formData = props.dashboardId
|
||||
|
||||
@@ -902,6 +902,8 @@ EXTRA_CATEGORICAL_COLOR_SCHEMES: list[dict[str, Any]] = []
|
||||
THEME_DEFAULT: Theme = {
|
||||
"token": {
|
||||
# Brand
|
||||
# Application name for window titles
|
||||
"brandAppName": APP_NAME,
|
||||
"brandLogoAlt": "Apache Superset",
|
||||
"brandLogoUrl": APP_ICON,
|
||||
"brandLogoMargin": "18px 0",
|
||||
|
||||
@@ -24,6 +24,8 @@
|
||||
{% block title %}
|
||||
{% if title %}
|
||||
{{ title }}
|
||||
{% else %}
|
||||
{{ default_title | default('Superset') }}
|
||||
{% endif %}
|
||||
{% endblock %}
|
||||
</title>
|
||||
|
||||
@@ -16,6 +16,7 @@
|
||||
# under the License.
|
||||
from __future__ import annotations
|
||||
|
||||
import copy
|
||||
import functools
|
||||
import logging
|
||||
import os
|
||||
@@ -567,10 +568,39 @@ def get_spa_template_context(
|
||||
"""
|
||||
payload = get_spa_payload(extra_bootstrap_data)
|
||||
|
||||
# Extract theme data for template access
|
||||
theme_data = get_theme_bootstrap_data().get("theme", {})
|
||||
# Deep copy theme data to avoid mutating cached bootstrap payload
|
||||
theme_data = copy.deepcopy(payload.get("common", {}).get("theme", {}))
|
||||
default_theme = theme_data.get("default", {})
|
||||
theme_tokens = default_theme.get("token", {})
|
||||
dark_theme = theme_data.get("dark", {})
|
||||
|
||||
# Apply brandAppName fallback to both default and dark themes
|
||||
# Priority: theme brandAppName > APP_NAME config > "Superset" default
|
||||
app_name_from_config = app.config.get("APP_NAME", "Superset")
|
||||
for theme_config in [default_theme, dark_theme]:
|
||||
if not theme_config:
|
||||
continue
|
||||
# Get or create token dict
|
||||
if "token" not in theme_config:
|
||||
theme_config["token"] = {}
|
||||
theme_tokens = theme_config["token"]
|
||||
|
||||
if (
|
||||
not theme_tokens.get("brandAppName")
|
||||
or theme_tokens.get("brandAppName") == "Superset"
|
||||
):
|
||||
# If brandAppName not set or is default, check if APP_NAME customized
|
||||
if app_name_from_config != "Superset":
|
||||
# User has customized APP_NAME, use it as brandAppName
|
||||
theme_tokens["brandAppName"] = app_name_from_config
|
||||
|
||||
# Write the modified theme data back to payload
|
||||
if "common" not in payload:
|
||||
payload["common"] = {}
|
||||
payload["common"]["theme"] = theme_data
|
||||
|
||||
# Extract theme tokens for template access (after fallback applied)
|
||||
# Use the direct reference to ensure we get the modified token dict
|
||||
theme_tokens = default_theme.get("token", {}) if default_theme else {}
|
||||
|
||||
# Determine spinner content with precedence: theme SVG > theme URL > default SVG
|
||||
spinner_svg = None
|
||||
@@ -581,6 +611,9 @@ def get_spa_template_context(
|
||||
# No custom URL either, use default SVG
|
||||
spinner_svg = get_default_spinner_svg()
|
||||
|
||||
# Determine default title using the (potentially updated) brandAppName
|
||||
default_title = theme_tokens.get("brandAppName", "Superset")
|
||||
|
||||
return {
|
||||
"entry": entry,
|
||||
"bootstrap_data": json.dumps(
|
||||
@@ -588,6 +621,7 @@ def get_spa_template_context(
|
||||
),
|
||||
"theme_tokens": theme_tokens,
|
||||
"spinner_svg": spinner_svg,
|
||||
"default_title": default_title,
|
||||
**template_kwargs,
|
||||
}
|
||||
|
||||
|
||||
@@ -530,3 +530,305 @@ class TestGetThemeBootstrapData:
|
||||
assert result["theme"]["enableUiThemeAdministration"] is False
|
||||
assert result["theme"]["default"] == {}
|
||||
assert result["theme"]["dark"] == {}
|
||||
|
||||
|
||||
class TestBrandAppNameFallback:
|
||||
"""Test brandAppName fallback mechanism for APP_NAME migration (issue #34865)"""
|
||||
|
||||
@patch("superset.views.base.get_spa_payload")
|
||||
@patch("superset.views.base.app")
|
||||
def test_brandappname_uses_theme_value_when_set(self, mock_app, mock_payload):
|
||||
"""Test that explicit brandAppName in theme takes precedence"""
|
||||
from superset.views.base import get_spa_template_context
|
||||
|
||||
# Use a plain dict for config to mirror Flask's config mapping behavior
|
||||
mock_app.config = {"APP_NAME": "Fallback App Name"}
|
||||
|
||||
# Mock payload with theme data that has custom brandAppName
|
||||
mock_payload.return_value = {
|
||||
"common": {
|
||||
"theme": {
|
||||
"default": {
|
||||
"token": {
|
||||
"brandAppName": "My Custom App",
|
||||
"brandLogoAlt": "Logo Alt",
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
result = get_spa_template_context("app")
|
||||
|
||||
# Should use the theme's brandAppName
|
||||
assert result["default_title"] == "My Custom App"
|
||||
# Theme tokens should have brandAppName
|
||||
theme_tokens = result["theme_tokens"]
|
||||
assert theme_tokens["brandAppName"] == "My Custom App"
|
||||
|
||||
@patch("superset.views.base.get_spa_payload")
|
||||
@patch("superset.views.base.app")
|
||||
def test_brandappname_falls_back_to_app_name_config(self, mock_app, mock_payload):
|
||||
"""Test fallback to APP_NAME config when brandAppName not in theme"""
|
||||
from superset.views.base import get_spa_template_context
|
||||
|
||||
mock_app.config = MagicMock()
|
||||
mock_app.config.get.side_effect = lambda k, d=None: {
|
||||
"APP_NAME": "My Test Analytics Platform",
|
||||
}.get(k, d)
|
||||
|
||||
# Mock payload with default "Superset" brandAppName
|
||||
mock_payload.return_value = {
|
||||
"common": {
|
||||
"theme": {
|
||||
"default": {
|
||||
"token": {
|
||||
"brandAppName": "Superset", # Default value
|
||||
"brandLogoAlt": "Apache Superset",
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
result = get_spa_template_context("app")
|
||||
|
||||
# Should fall back to APP_NAME config
|
||||
assert result["default_title"] == "My Test Analytics Platform"
|
||||
# Theme tokens should be updated with APP_NAME value
|
||||
theme_tokens = result["theme_tokens"]
|
||||
assert theme_tokens["brandAppName"] == "My Test Analytics Platform"
|
||||
|
||||
@patch("superset.views.base.get_spa_payload")
|
||||
@patch("superset.views.base.app")
|
||||
def test_brandappname_uses_superset_default_when_nothing_set(
|
||||
self, mock_app, mock_payload
|
||||
):
|
||||
"""Test fallback to 'Superset' when neither is customized"""
|
||||
from superset.views.base import get_spa_template_context
|
||||
|
||||
mock_app.config = MagicMock()
|
||||
mock_app.config.get.side_effect = lambda k, d=None: {
|
||||
"APP_NAME": "Superset", # Default value
|
||||
}.get(k, d)
|
||||
|
||||
# Mock payload with default "Superset" brandAppName
|
||||
mock_payload.return_value = {
|
||||
"common": {
|
||||
"theme": {
|
||||
"default": {
|
||||
"token": {
|
||||
"brandAppName": "Superset", # Default value
|
||||
"brandLogoAlt": "Apache Superset",
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
result = get_spa_template_context("app")
|
||||
|
||||
# Should use default "Superset"
|
||||
assert result["default_title"] == "Superset"
|
||||
# Theme tokens should keep "Superset"
|
||||
theme_tokens = result["theme_tokens"]
|
||||
assert theme_tokens["brandAppName"] == "Superset"
|
||||
|
||||
@patch("superset.views.base.get_spa_payload")
|
||||
@patch("superset.views.base.app")
|
||||
def test_brandappname_empty_string_falls_back(self, mock_app, mock_payload):
|
||||
"""Test that empty string brandAppName triggers fallback"""
|
||||
from superset.views.base import get_spa_template_context
|
||||
|
||||
mock_app.config = MagicMock()
|
||||
mock_app.config.get.side_effect = lambda k, d=None: {
|
||||
"APP_NAME": "Custom App",
|
||||
}.get(k, d)
|
||||
|
||||
# Mock payload with empty brandAppName
|
||||
mock_payload.return_value = {
|
||||
"common": {
|
||||
"theme": {
|
||||
"default": {
|
||||
"token": {
|
||||
"brandAppName": "", # Empty string
|
||||
"brandLogoAlt": "Logo",
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
result = get_spa_template_context("app")
|
||||
|
||||
# Should fall back to APP_NAME
|
||||
assert result["default_title"] == "Custom App"
|
||||
theme_tokens = result["theme_tokens"]
|
||||
assert theme_tokens["brandAppName"] == "Custom App"
|
||||
|
||||
@patch("superset.views.base.get_spa_payload")
|
||||
@patch("superset.views.base.app")
|
||||
def test_brandappname_none_falls_back(self, mock_app, mock_payload):
|
||||
"""Test that missing brandAppName triggers fallback"""
|
||||
from superset.views.base import get_spa_template_context
|
||||
|
||||
mock_app.config = MagicMock()
|
||||
mock_app.config.get.side_effect = lambda k, d=None: {
|
||||
"APP_NAME": "Analytics Dashboard",
|
||||
}.get(k, d)
|
||||
|
||||
# Mock payload without brandAppName
|
||||
mock_payload.return_value = {
|
||||
"common": {"theme": {"default": {"token": {"brandLogoAlt": "Logo"}}}}
|
||||
}
|
||||
|
||||
result = get_spa_template_context("app")
|
||||
|
||||
# Should fall back to APP_NAME
|
||||
assert result["default_title"] == "Analytics Dashboard"
|
||||
theme_tokens = result["theme_tokens"]
|
||||
assert theme_tokens["brandAppName"] == "Analytics Dashboard"
|
||||
|
||||
@patch("superset.views.base.get_spa_payload")
|
||||
@patch("superset.views.base.app")
|
||||
def test_brandappname_updates_both_default_and_dark_themes(
|
||||
self, mock_app, mock_payload
|
||||
):
|
||||
"""Test that brandAppName fallback applies to both default and dark themes"""
|
||||
from superset.views.base import get_spa_template_context
|
||||
|
||||
mock_app.config = MagicMock()
|
||||
mock_app.config.get.side_effect = lambda k, d=None: {
|
||||
"APP_NAME": "Multi Theme App",
|
||||
}.get(k, d)
|
||||
|
||||
# Mock payload with both themes missing brandAppName
|
||||
mock_payload.return_value = {
|
||||
"common": {
|
||||
"theme": {
|
||||
"default": {
|
||||
"token": {
|
||||
"brandAppName": "Superset", # Default value
|
||||
"colorPrimary": "#111",
|
||||
}
|
||||
},
|
||||
"dark": {
|
||||
"token": {
|
||||
# Missing brandAppName
|
||||
"colorPrimary": "#222",
|
||||
}
|
||||
},
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
result = get_spa_template_context("app")
|
||||
|
||||
# Should update both themes
|
||||
assert result["default_title"] == "Multi Theme App"
|
||||
# Verify default theme was updated
|
||||
theme_tokens = result["theme_tokens"]
|
||||
assert theme_tokens["brandAppName"] == "Multi Theme App"
|
||||
assert theme_tokens["colorPrimary"] == "#111" # Preserved
|
||||
|
||||
@patch("superset.views.base.get_spa_payload")
|
||||
@patch("superset.views.base.app")
|
||||
def test_brandappname_does_not_mutate_cached_payload(self, mock_app, mock_payload):
|
||||
"""Test that brandAppName fallback doesn't mutate the cached payload"""
|
||||
from superset.views.base import get_spa_template_context
|
||||
|
||||
mock_app.config = MagicMock()
|
||||
mock_app.config.get.side_effect = lambda k, d=None: {
|
||||
"APP_NAME": "Test App",
|
||||
}.get(k, d)
|
||||
|
||||
# Create a payload that simulates cached data
|
||||
original_theme_data = {
|
||||
"default": {
|
||||
"token": {
|
||||
"brandAppName": "Superset",
|
||||
"colorPrimary": "#333",
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
mock_payload.return_value = {"common": {"theme": original_theme_data}}
|
||||
|
||||
# Call get_spa_template_context
|
||||
result = get_spa_template_context("app")
|
||||
|
||||
# Verify the function result has the updated brandAppName
|
||||
assert result["default_title"] == "Test App"
|
||||
theme_tokens = result["theme_tokens"]
|
||||
assert theme_tokens["brandAppName"] == "Test App"
|
||||
|
||||
# Verify the original mock payload structure wasn't mutated
|
||||
# (the function should deep copy before mutating)
|
||||
# Note: We can't easily test the cached payload immutability
|
||||
# without more complex mocking, but we've verified the result is correct
|
||||
assert result["default_title"] == "Test App"
|
||||
|
||||
@patch("superset.views.base.get_spa_payload")
|
||||
@patch("superset.views.base.app")
|
||||
def test_brandappname_handles_empty_theme_config(self, mock_app, mock_payload):
|
||||
"""Test that empty theme configs are skipped gracefully"""
|
||||
from superset.views.base import get_spa_template_context
|
||||
|
||||
mock_app.config = {"APP_NAME": "Test App"}
|
||||
|
||||
# Mock payload with empty dark theme
|
||||
mock_payload.return_value = {
|
||||
"common": {
|
||||
"theme": {
|
||||
"default": {"token": {"brandAppName": "Superset"}},
|
||||
"dark": {}, # Empty theme config
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
result = get_spa_template_context("app")
|
||||
|
||||
# Should handle empty theme gracefully and still update default
|
||||
assert result["default_title"] == "Test App"
|
||||
|
||||
@patch("superset.views.base.get_spa_payload")
|
||||
@patch("superset.views.base.app")
|
||||
def test_brandappname_creates_token_dict_when_missing(self, mock_app, mock_payload):
|
||||
"""Test that token dict is created when missing from theme config"""
|
||||
from superset.views.base import get_spa_template_context
|
||||
|
||||
mock_app.config = {"APP_NAME": "Token Test App"}
|
||||
|
||||
# Mock payload with theme missing token dict
|
||||
mock_payload.return_value = {
|
||||
"common": {
|
||||
"theme": {
|
||||
"default": {"algorithm": "default"}, # No token dict
|
||||
"dark": {"algorithm": "dark"}, # No token dict
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
result = get_spa_template_context("app")
|
||||
|
||||
# Should create token dict and set brandAppName
|
||||
assert result["default_title"] == "Token Test App"
|
||||
assert result["theme_tokens"]["brandAppName"] == "Token Test App"
|
||||
|
||||
@patch("superset.views.base.get_spa_payload")
|
||||
@patch("superset.views.base.app")
|
||||
def test_brandappname_handles_missing_common_in_payload(
|
||||
self, mock_app, mock_payload
|
||||
):
|
||||
"""Test handling when common dict is missing from payload"""
|
||||
from superset.views.base import get_spa_template_context
|
||||
|
||||
mock_app.config = {"APP_NAME": "Superset"}
|
||||
|
||||
# Mock payload without common dict
|
||||
mock_payload.return_value = {}
|
||||
|
||||
result = get_spa_template_context("app")
|
||||
|
||||
# Should handle gracefully and use default title
|
||||
assert result["default_title"] == "Superset"
|
||||
|
||||
Reference in New Issue
Block a user