You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "durgaprasadml (via GitHub)" <gi...@apache.org> on 2026/06/04 19:09:45 UTC

[PR] fix(theme): embedded method overrides dashboard level config [superset]

durgaprasadml opened a new pull request, #40777:
URL: https://github.com/apache/superset/pull/40777

   ## Summary
   
   Fix unstable dashboard theme/color scheme behavior introduced in Superset 6.0.0, especially affecting embedded dashboards and guest-token iframe environments.
   
   This PR improves theme consistency by ensuring explicit SDK theme overrides are respected, preventing stale cached theme data, and adding more reliable theme fetching behavior for embedded use cases.
   
   ## Changes
   
   ### Frontend
   - Prevent nested ThemeProvider instances from overriding explicit SDK theme configuration
   - Improve embedded dashboard theme precedence behavior
   - Add guest-token aware fallback handling for theme fetching
   
   ### Backend
   - Add automatic cache invalidation for cached bootstrap/theme data on Theme model create/update/delete
   - Prevent stale dashboard theme rendering caused by cached bootstrap responses
   
   ### Testing
   - Added frontend regression coverage
   - Added backend cache invalidation tests
   - Updated assertions for dynamic options handling
   
   ## Validation
   
   Verified successfully with:
   
   - Frontend Jest tests
   - Backend unit tests
   - pre-commit checks
   - plugins:build
   
   ## Problem
   
   After upgrading to Superset 6.0.0, embedded dashboards could inconsistently apply dashboard color schemes/themes, especially:
   - inside iframes
   - with guest token authentication
   - after theme updates
   - when nested providers overrode explicit SDK settings
   
   This resulted in unstable or stale theme rendering behavior.
   
   ## Root Cause
   
   The issue was caused by a combination of:
   - nested theme providers shadowing explicit embedded SDK overrides
   - cached bootstrap theme data not being invalidated after theme updates
   - unreliable theme fetch behavior in guest-token environments
   
   ## Related Issue
   
   Fixes #40754


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(theme): embedded method overrides dashboard level config [superset]

Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #40777:
URL: https://github.com/apache/superset/pull/40777#discussion_r3358303631


##########
superset-frontend/src/components/CrudThemeProvider.tsx:
##########
@@ -83,7 +91,7 @@ export default function CrudThemeProvider({
     };
   }, [dashboardTheme, fontUrls]);
 
-  if (!dashboardTheme) {
+  if (!dashboardTheme || hasThemeConfigOverride) {
     return <>{children}</>;

Review Comment:
   **Suggestion:** The new early return skips rendering the dashboard `SupersetThemeProvider` when an SDK override is active, but it does not stop or clean up dashboard font injection, so dashboard `fontUrls` can still leak into the document and override typography despite the SDK override. Include the override flag in the font-injection effect logic/dependencies and ensure styles are removed when the override becomes active. [logic error]
   
   <details>
   <summary><b>Severity Level:</b> Major ⚠️</summary>
   
   ```mdx
   - ⚠️ Embedded dashboards use dashboard fonts instead of SDK fonts.
   - ⚠️ SDK theme changes leave stale dashboard font CSS active.
   - ⚠️ Branding/typography inconsistent across embedded dashboard views.
   ```
   </details>
   <details>
   <summary><b>Steps of Reproduction ✅ </b></summary>
   
   ```mdx
   1. Configure a dashboard with a custom theme that includes `fontUrls` in its JSON config
   (e.g., `{ token: { fontUrls: [...] } }`), which is read and normalized in
   `superset-frontend/src/components/CrudThemeProvider.tsx:52-70` via
   `JSON.parse(theme.json_data)` and `Theme.fromConfig`, returning `{ dashboardTheme,
   fontUrls }`.
   
   2. Run the app under the global `SupersetThemeProvider`
   (`superset-frontend/src/theme/ThemeProvider.tsx:43-57`) and apply an Embedded SDK theme
   override by calling `ThemeController.setThemeConfig(...)` (see
   `superset-frontend/src/theme/ThemeController.ts:67-88`), which sets
   `this.themeConfigOverride = true` and causes `hasThemeConfigOverride` in the context to
   become `true` via the effect at `ThemeProvider.tsx:59-75`.
   
   3. Navigate to a dashboard route so that `DashboardPage` renders
   (`superset-frontend/src/dashboard/containers/DashboardPage.tsx:11-13`), which wraps
   `DashboardContainer` in `CrudThemeProvider` and passes the dashboard theme from the API;
   inside `CrudThemeProvider` the `useEffect` at
   `superset-frontend/src/components/CrudThemeProvider.tsx:77-92` still sees a non-null
   `dashboardTheme` and non-empty `fontUrls` and injects a `<style data-superset-fonts>` tag
   with `@import url(...)` rules into `document.head`, even though `hasThemeConfigOverride`
   is already `true`.
   
   4. Because the early return at `CrudThemeProvider.tsx:94-96` (`if (!dashboardTheme ||
   hasThemeConfigOverride) { return <>{children}</>; }`) skips the dashboard
   `SupersetThemeProvider` wrapper but the font-injection effect does not depend on
   `hasThemeConfigOverride` and never checks it, the injected dashboard font CSS remains
   active; when the SDK override is active (either from initial load or toggled later), the
   global SDK theme is used for colors but the stale dashboard `fontUrls` stylesheet
   continues to affect typography, overriding or conflicting with the SDK-provided fonts.
   ```
   </details>
   
   [Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=08e81653209840e08b882f7b63bea3a8&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) | [Fix in VSCode Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=08e81653209840e08b882f7b63bea3a8&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
   
   *(Use Cmd/Ctrl + Click for best experience)*
   <details>
   <summary><b>Prompt for AI Agent 🤖 </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** superset-frontend/src/components/CrudThemeProvider.tsx
   **Line:** 94:95
   **Comment:**
   	*Logic Error: The new early return skips rendering the dashboard `SupersetThemeProvider` when an SDK override is active, but it does not stop or clean up dashboard font injection, so dashboard `fontUrls` can still leak into the document and override typography despite the SDK override. Include the override flag in the font-injection effect logic/dependencies and ensure styles are removed when the override becomes active.
   
   Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
   Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
   ```
   </details>
   <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40777&comment_hash=49c061d119c72369296cc3696e0764e77b30eff2c0957174d7a703cff6a0c3e2&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40777&comment_hash=49c061d119c72369296cc3696e0764e77b30eff2c0957174d7a703cff6a0c3e2&reaction=dislike'>👎</a>



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(theme): embedded method overrides dashboard level config [superset]

Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas merged PR #40777:
URL: https://github.com/apache/superset/pull/40777


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(theme): embedded method overrides dashboard level config [superset]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #40777:
URL: https://github.com/apache/superset/pull/40777#discussion_r3364300651


##########
superset-frontend/src/theme/ThemeController.ts:
##########
@@ -1015,32 +1030,81 @@ export class ThemeController {
    */
   private async fetchSystemDefaultTheme(): Promise<AnyThemeConfig | null> {
     try {
-      // Try to fetch theme marked as system default (is_system_default=true)
-      const defaultResponse = await fetch(
-        '/api/v1/theme/?q=(filters:!((col:is_system_default,opr:eq,value:!t)))',
-      );
-      if (defaultResponse.ok) {
-        const data = await defaultResponse.json();
-        if (data.result?.length > 0) {
-          const themeConfig = JSON.parse(data.result[0].json_data);
+      // Try to use SupersetClient first if it has been configured
+      try {
+        const response = await SupersetClient.get({

Review Comment:
   The note above `fetchSystemDefaultTheme()` says this method uses raw `fetch()` instead of `SupersetClient`, but the updated implementation now tries `SupersetClient.get()` first. Updating the note will prevent future confusion when debugging embedded/guest-token flows.



##########
superset/models/core.py:
##########
@@ -146,6 +146,22 @@ class Theme(AuditMixinNullable, ImportExportMixin, Model):
     export_fields = ["theme_name", "json_data"]
 
 
+# Event listeners to clear the memoized bootstrap data cache when a theme is modified
+@sqla.event.listens_for(Theme, "after_insert")
+@sqla.event.listens_for(Theme, "after_update")
+@sqla.event.listens_for(Theme, "after_delete")
+def clear_bootstrap_cache(
+    mapper: sqla.orm.Mapper, connection: sqla.engine.Connection, target: Theme
+) -> None:

Review Comment:
   `clear_bootstrap_cache` doesn't use its SQLAlchemy event parameters, which will trigger `pylint`'s `unused-argument` on changed Python files. Rename the parameters to `_mapper/_connection/_target` (or disable `unused-argument`) to keep the listener signature but satisfy linting.



##########
superset-frontend/src/components/CrudThemeProvider.tsx:
##########
@@ -83,7 +91,7 @@ export default function CrudThemeProvider({
     };
   }, [dashboardTheme, fontUrls]);
 
-  if (!dashboardTheme) {
+  if (!dashboardTheme || hasThemeConfigOverride) {
     return <>{children}</>;

Review Comment:
   When `hasThemeConfigOverride` is active, the component returns children without the dashboard ThemeProvider, but the font `@import` effect can still run and inject dashboard-provided fonts. That can still influence the rendered UI and undermines the intent of skipping the dashboard theme. Gate the effect on `hasThemeConfigOverride` and include it in the dependency array so existing injected styles are cleaned up if the override becomes active after initial render.



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(theme): embedded method overrides dashboard level config [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40777:
URL: https://github.com/apache/superset/pull/40777#issuecomment-4625294433

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(theme): embedded method overrides dashboard level config [superset]

Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #40777:
URL: https://github.com/apache/superset/pull/40777#issuecomment-4637519577

   ### <span aria-hidden="true">✅</span> Deploy Preview for *superset-docs-preview* ready!
   
   
   |  Name | Link |
   |:-:|------------------------|
   |<span aria-hidden="true">🔨</span> Latest commit | 3781b39f8436c932ea171840df4210427a2940d8 |
   |<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6a23aef5e7899d000979bf09 |
   |<span aria-hidden="true">😎</span> Deploy Preview | [https://deploy-preview-40777--superset-docs-preview.netlify.app](https://deploy-preview-40777--superset-docs-preview.netlify.app) |
   |<span aria-hidden="true">📱</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br />![QR Code](https://app.netlify.com/qr-code/eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJ1cmwiOiJodHRwczovL2RlcGxveS1wcmV2aWV3LTQwNzc3LS1zdXBlcnNldC1kb2NzLXByZXZpZXcubmV0bGlmeS5hcHAifQ.W7_7gYcuYhl5ba2tZuebVVMgJ6Vi0ZrTrSK-3tYJn58)<br /><br />_Use your smartphone camera to open QR code link._</details> |
   |<span aria-hidden="true">🤖</span> Make changes | [Run an agent on this branch](https://app.netlify.com/projects/superset-docs-preview/agent-runs#DcoxDoAgDADAv3SWpNEo1h_4jGJLMBEhyKAx_l22G-6FXFLMFRaADlzhcwvNfr9NDRrVaHQqomKEr-ASF2lPNB_pWaXNifuB1Y9qZyJBRLLkPBJ8Pw) |
   ---
   <!-- [superset-docs-preview Preview](https://deploy-preview-40777--superset-docs-preview.netlify.app) -->
   _To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(theme): embedded method overrides dashboard level config [superset]

Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #40777:
URL: https://github.com/apache/superset/pull/40777#issuecomment-4625327006

   ## [Codecov](https://app.codecov.io/gh/apache/superset/pull/40777?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   :x: Patch coverage is `80.00000%` with `2 lines` in your changes missing coverage. Please review.
   :white_check_mark: Project coverage is 55.41%. Comparing base ([`75d94ff`](https://app.codecov.io/gh/apache/superset/commit/75d94ff466aa6735bc1f4cc801bd0c26b2a8d889?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`727dbf6`](https://app.codecov.io/gh/apache/superset/commit/727dbf6ef7b9a9e8456a1e982a241e5937849d02?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
   :warning: Report is 4 commits behind head on master.
   
   | [Files with missing lines](https://app.codecov.io/gh/apache/superset/pull/40777?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
   |---|---|---|
   | [superset/models/core.py](https://app.codecov.io/gh/apache/superset/pull/40777?src=pr&el=tree&filepath=superset%2Fmodels%2Fcore.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | 80.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/40777?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   > :exclamation:  There is a different number of reports uploaded between BASE (75d94ff) and HEAD (727dbf6). Click for more details.
   > 
   > <details><summary>HEAD has 8 uploads less than BASE</summary>
   >
   >| Flag | BASE (75d94ff) | HEAD (727dbf6) |
   >|------|------|------|
   >|python|10|2|
   ></details>
   
   <details><summary>Additional details and impacted files</summary>
   
   
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #40777      +/-   ##
   ==========================================
   - Coverage   64.04%   55.41%   -8.64%     
   ==========================================
     Files        2663     2664       +1     
     Lines      143650   143785     +135     
     Branches    33042    33066      +24     
   ==========================================
   - Hits        92003    79674   -12329     
   - Misses      50045    63395   +13350     
   + Partials     1602      716     -886     
   ```
   
   | [Flag](https://app.codecov.io/gh/apache/superset/pull/40777/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
   |---|---|---|
   | [hive](https://app.codecov.io/gh/apache/superset/pull/40777/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `39.77% <80.00%> (+0.10%)` | :arrow_up: |
   | [mysql](https://app.codecov.io/gh/apache/superset/pull/40777/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [postgres](https://app.codecov.io/gh/apache/superset/pull/40777/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [presto](https://app.codecov.io/gh/apache/superset/pull/40777/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `41.36% <80.00%> (+0.10%)` | :arrow_up: |
   | [python](https://app.codecov.io/gh/apache/superset/pull/40777/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `41.42% <80.00%> (-18.52%)` | :arrow_down: |
   | [sqlite](https://app.codecov.io/gh/apache/superset/pull/40777/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [unit](https://app.codecov.io/gh/apache/superset/pull/40777/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
   </details>
   
   [:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/superset/pull/40777?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).   
   :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   <details><summary> :rocket: New features to boost your workflow: </summary>
   
   - :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
   - :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
   </details>


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(theme): embedded method overrides dashboard level config [superset]

Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #40777:
URL: https://github.com/apache/superset/pull/40777#discussion_r3358309562


##########
superset-frontend/src/theme/ThemeController.ts:
##########
@@ -1015,32 +1030,81 @@ export class ThemeController {
    */
   private async fetchSystemDefaultTheme(): Promise<AnyThemeConfig | null> {
     try {
-      // Try to fetch theme marked as system default (is_system_default=true)
-      const defaultResponse = await fetch(
-        '/api/v1/theme/?q=(filters:!((col:is_system_default,opr:eq,value:!t)))',
-      );
-      if (defaultResponse.ok) {
-        const data = await defaultResponse.json();
-        if (data.result?.length > 0) {
-          const themeConfig = JSON.parse(data.result[0].json_data);
+      // Try to use SupersetClient first if it has been configured
+      try {
+        const response = await SupersetClient.get({
+          endpoint:
+            '/api/v1/theme/?q=(filters:!((col:is_system_default,opr:eq,value:!t)))',
+        });
+        if (response.json?.result?.length > 0) {
+          const themeConfig = JSON.parse(response.json.result[0].json_data);
           if (themeConfig && typeof themeConfig === 'object') {
             return themeConfig;
           }
         }
+      } catch (clientError) {
+        // If SupersetClient is not configured yet or request fails, fall back to native fetch
+        const headers: Record<string, string> = {};
+        try {
+          const guestToken = SupersetClient.getGuestToken();
+          if (guestToken) {
+            headers['X-GuestToken'] = guestToken;

Review Comment:
   **Suggestion:** The fallback `fetch` path hardcodes `X-GuestToken`, but embedded mode supports a configurable guest-token header name (`GUEST_TOKEN_HEADER_NAME`). In deployments that override this header, the fallback API requests will be unauthenticated and theme recovery will silently fail. Read the configured header name (or reuse SupersetClient's configured request headers) instead of hardcoding the header key. [api mismatch]
   
   <details>
   <summary><b>Severity Level:</b> Major ⚠️</summary>
   
   ```mdx
   - ❌ Embedded dashboards cannot recover themes via authenticated fallback API.
   - ⚠️ Theme recovery uses stale or built-in defaults, not server.
   - ⚠️ Behavior diverges when GUEST_TOKEN_HEADER_NAME is customized.
   ```
   </details>
   <details>
   <summary><b>Steps of Reproduction ✅ </b></summary>
   
   ```mdx
   1. In an embedded deployment, configure a non-default guest token header name via backend
   config so `bootstrapData.config.GUEST_TOKEN_HEADER_NAME` is not `'X-GuestToken'` (this
   value is read and passed into the embedded Superset client in
   `superset-frontend/src/embedded/index.tsx:18-23`, where `setupGuestClient` calls
   `setupClient({ appRoot: applicationRoot(), guestToken, guestTokenHeaderName:
   bootstrapData.config?.GUEST_TOKEN_HEADER_NAME, ... })`).
   
   2. Load an embedded dashboard so that the embedded app bootstraps using
   `EmbeddedContextProviders`
   (`superset-frontend/src/embedded/EmbeddedContextProviders.tsx:14-31`), which creates a
   shared `ThemeController` instance and wires it into `SupersetThemeProvider`, meaning theme
   changes and recovery go through `ThemeController`.
   
   3. Trigger a theme application error or an invalid theme configuration so that
   `ThemeController.updateTheme()` throws inside `applyTheme` (see
   `superset-frontend/src/theme/ThemeController.ts:23-38` for `updateTheme`, and `852-25` for
   `applyTheme`), causing the `catch` block in `updateTheme` to execute and call `await
   this.fallbackToDefaultMode()` at `superset-frontend/src/theme/ThemeController.ts:39-48`.
   
   4. In `fallbackToDefaultMode()` (`superset-frontend/src/theme/ThemeController.ts:56-60`),
   the controller calls `fetchSystemDefaultTheme()`, which first tries
   `SupersetClient.get(...)` and then falls back to native `fetch` on failure (`52-91`). In
   that fallback, it obtains the guest token via `SupersetClient.getGuestToken()` at line
   `1049` and unconditionally sends it as `headers['X-GuestToken'] = guestToken;` at line
   `1051`, ignoring the configured `guestTokenHeaderName`. In deployments where
   `guestTokenHeaderName !== 'X-GuestToken'`, the backend will treat this request as
   unauthenticated, `defaultResponse.ok` will be false, `fetchSystemDefaultTheme()` returns
   `null` (`135`), and `fallbackToDefaultMode()` is forced to skip server theme recovery and
   instead use only the cached/built-in default theme (`600-37`), silently failing to recover
   the correct system theme for embedded dashboards.
   ```
   </details>
   
   [Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=b08aa244ce1b4db1ad77eec8a0e965eb&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) | [Fix in VSCode Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=b08aa244ce1b4db1ad77eec8a0e965eb&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
   
   *(Use Cmd/Ctrl + Click for best experience)*
   <details>
   <summary><b>Prompt for AI Agent 🤖 </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** superset-frontend/src/theme/ThemeController.ts
   **Line:** 1049:1051
   **Comment:**
   	*Api Mismatch: The fallback `fetch` path hardcodes `X-GuestToken`, but embedded mode supports a configurable guest-token header name (`GUEST_TOKEN_HEADER_NAME`). In deployments that override this header, the fallback API requests will be unauthenticated and theme recovery will silently fail. Read the configured header name (or reuse SupersetClient's configured request headers) instead of hardcoding the header key.
   
   Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
   Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
   ```
   </details>
   <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40777&comment_hash=8dd5b4683349b7289c96eafe55bd7958ca05c5c43e0719625c478c62d6a865d7&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40777&comment_hash=8dd5b4683349b7289c96eafe55bd7958ca05c5c43e0719625c478c62d6a865d7&reaction=dislike'>👎</a>



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(theme): embedded method overrides dashboard level config [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40777:
URL: https://github.com/apache/superset/pull/40777#issuecomment-4637962498

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
   
   <details>
   <summary>
   Bito Usage Guide
   </summary>
   
   **Commands**
         
   Type the following command in the pull request comment and save the comment.
         
   - `/review` - Manually triggers a full AI review.
   
   - `/pause` - Pauses automatic reviews on this pull request.
   - `/resume` - Resumes automatic reviews.
   - `/resolve` - Marks all Bito-posted review comments as resolved.
   - `/abort` - Cancels all in-progress reviews.
   
         
   Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
         
   **Configuration**
   
   This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
         
   **Documentation & Help**
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
   
   </details>
   


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org