Skip to content

fix(reports): preserve dashboard state in tab permalinks#39708

Open
ayush-sharaf wants to merge 1 commit intoapache:masterfrom
ayush-sharaf:report-schedule-datamask-permalink
Open

fix(reports): preserve dashboard state in tab permalinks#39708
ayush-sharaf wants to merge 1 commit intoapache:masterfrom
ayush-sharaf:report-schedule-datamask-permalink

Conversation

@ayush-sharaf
Copy link
Copy Markdown

@ayush-sharaf ayush-sharaf commented Apr 28, 2026

SUMMARY

Preserve dashboard state from ReportSchedule.extra.dashboard when generating per-tab report permalinks for scheduled reports.

This fixes a bug where report generation was dropping stored dashboard state while building tab-specific permalinks. In particular, dataMask was not being forwarded, which meant scheduled reports could ignore native
filter state already persisted on the report schedule.

This is important for report schedules that rely on date filter state stored in extra.dashboard.dataMask. Without this change, the permalink created during execution does not reflect the scheduled filter context, so
generated screenshots can show unfiltered or incorrect dashboard results.

With this change, report generation reuses the parent dashboard state and preserves:

  • dataMask
  • activeTabs
  • urlParams

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

N/A

TESTING INSTRUCTIONS

  1. Create a dashboard with multiple tabs and native filters.
  2. Store dashboard filter state in ReportSchedule.extra.dashboard, including dataMask.
  3. Enable ALERT_REPORT_TABS.
  4. Run scheduled report generation for the dashboard.
  5. Verify the generated tab permalinks preserve dataMask, activeTabs, and urlParams.
  6. Verify the screenshots reflect the expected filtered dashboard state.

ADDITIONAL INFORMATION

  • Has associated issue:
  • Fixes Scheduled report permalinks do not preserve dashboard dataMask state #39709
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

@dosubot dosubot Bot added alert-reports Namespace | Anything related to the Alert & Reports feature change:backend Requires changing the backend dashboard:tab Related to the usage of tabs in the Dashboard labels Apr 28, 2026
Comment on lines 368 to +372
{
"anchor": tab_anchor,
"dataMask": None,
"activeTabs": None,
"urlParams": [
["native_filters", native_filter_params] # type: ignore
],
"dataMask": parent_state.get("dataMask"),
"activeTabs": parent_state.get("activeTabs"),
"urlParams": parent_state.get("urlParams")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: The tab permalink state now copies urlParams only from stored dashboard state and ignores the native_filter_params argument, so tab-based report runs can drop computed native_filters when extra.dashboard.urlParams is empty or missing. This breaks filter application for schedules that rely on nativeFilters generation, causing tab screenshots to render unfiltered data. Merge native_filter_params into urlParams for each tab (while still preserving existing parent urlParams) just like the non-tab permalink path does. [logic error]

Severity Level: Critical 🚨
- ❌ Multi-tab dashboard report screenshots omit native filter context.
- ❌ get_dashboard_urls() ignores computed native_filters for tab anchors.
- ⚠️ Alert/report recipients can receive misleading unfiltered dashboards.
Steps of Reproduction ✅
1. Configure a dashboard report schedule whose `extra["dashboard"]` contains multiple tab
anchors and native filters, e.g. in `superset/reports/models.py:193-53` the
`ReportSchedule.get_native_filters_params()` method reads
`extra["dashboard"]["nativeFilters"]` to generate `native_filter_params`, and in
`superset/commands/report/execute.py:274-281` `get_dashboard_urls()` expects
`extra["dashboard"]["anchor"]` to be a JSON list of tab ids.

2. Enable the `ALERT_REPORT_TABS` feature flag so that `get_dashboard_urls()` in
`superset/commands/report/execute.py:266-268` takes the tabbed permalink path, and ensure
`extra["dashboard"]` does not define a `urlParams` entry with a `("native_filters", ...)`
tuple (only `nativeFilters` is set, which is the normal case given how
`get_native_filters_params()` is implemented).

3. Trigger dashboard report execution so `_get_screenshots()` in
`superset/commands/report/execute.py:379-47` is called for a dashboard (no chart); it
calls `get_dashboard_urls()` (line 33) which obtains `native_filter_params` from
`ReportSchedule.get_native_filters_params()` (lines 269-271) and, because `anchor` is a
list, calls `_get_tabs_urls(anchor_list, native_filter_params=native_filter_params,
user_friendly=...)` at lines 281-285.

4. Inside `_get_tabs_urls()` in `superset/commands/report/execute.py:356-377`, the per-tab
state passed to `CreateDashboardPermalinkCommand` is built from `parent_state =
self._report_schedule.extra.get("dashboard") or {}` at line 365 and the literal dict at
lines 368-372, which includes `"anchor"`, `"dataMask"`, `"activeTabs"`, and `"urlParams":
parent_state.get("urlParams")`, but never uses the `native_filter_params` argument from
the function signature (line 359); if `parent_state["urlParams"]` is empty or lacks a
`("native_filters", ...)` entry, the generated permalinks for each tab do not contain the
`native_filters` URL param, so the filters encoded by `get_native_filters_params()` are
not applied when the dashboard is rendered, and the resulting tab screenshots show
unfiltered data compared to the intended filtered state.

Fix in Cursor | Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** superset/commands/report/execute.py
**Line:** 368:372
**Comment:**
	*Logic Error: The tab permalink state now copies `urlParams` only from stored dashboard state and ignores the `native_filter_params` argument, so tab-based report runs can drop computed `native_filters` when `extra.dashboard.urlParams` is empty or missing. This breaks filter application for schedules that rely on `nativeFilters` generation, causing tab screenshots to render unfiltered data. Merge `native_filter_params` into `urlParams` for each tab (while still preserving existing parent `urlParams`) just like the non-tab permalink path does.

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
👍 | 👎

@bito-code-review
Copy link
Copy Markdown
Contributor

The flagged issue is correct—the change ignores native_filter_params for tab permalinks, potentially dropping filters when parent_state lacks urlParams. To resolve, merge native_filter_params into existing urlParams while preserving parent values. I've implemented a concise fix below.

superset/commands/report/execute.py

parent_state = self._report_schedule.extra.get("dashboard") or {}
        return [
            self._get_tab_url(
                {
                    "anchor": tab_anchor,
                    "dataMask": parent_state.get("dataMask"),
                    "activeTabs": parent_state.get("activeTabs"),
                    "urlParams": self._merge_native_filters(parent_state.get("urlParams"), native_filter_params)
                },
                user_friendly=user_friendly,
            )
            for tab_anchor in anchor_list
        ]

superset/commands/report/execute.py

def _merge_native_filters(self, existing_url_params, native_filter_params):
        url_params = existing_url_params or []
        if not any(p[0] == "native_filters" for p in url_params):
            url_params.append(["native_filters", native_filter_params])
        return url_params

Comment on lines +365 to 373
parent_state = self._report_schedule.extra.get("dashboard") or {}
return [
self._get_tab_url(
{
"anchor": tab_anchor,
"dataMask": None,
"activeTabs": None,
"urlParams": [
["native_filters", native_filter_params] # type: ignore
],
"dataMask": parent_state.get("dataMask"),
"activeTabs": parent_state.get("activeTabs"),
"urlParams": parent_state.get("urlParams")
},
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Architect Review — HIGH

In the multi-tab path, _get_tabs_urls now ignores the computed native_filter_params and only forwards parent_state["urlParams"], so per-tab permalinks no longer inject or refresh the native_filters URL param based on extra.dashboard.nativeFilters. For schedules that store filters in extra.dashboard.nativeFilters (with urlParams missing or stale), tab-specific permalinks will omit or retain outdated native filter params, and generated screenshots can reflect the wrong filter context compared with the scheduled state.

Suggestion: Keep reusing the parent dashboard state, but in _get_tabs_urls merge native_filter_params into each tab state's urlParams (dropping any existing native_filters entry and appending the fresh one), mirroring the merge logic used in the single-tab branch so both nativeFilters-based and precomputed urlParams storage formats remain supported.

Fix in Cursor | Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖
This is an **Architect / Logical Review** comment left during a code review. These reviews are first-class, important findings — not optional suggestions. Do NOT dismiss this as a 'big architectural change' just because the title says architect review; most of these can be resolved with a small, localized fix once the intent is understood.

**Path:** superset/commands/report/execute.py
**Line:** 365:373
**Comment:**
	*HIGH: In the multi-tab path, `_get_tabs_urls` now ignores the computed `native_filter_params` and only forwards `parent_state["urlParams"]`, so per-tab permalinks no longer inject or refresh the `native_filters` URL param based on `extra.dashboard.nativeFilters`. For schedules that store filters in `extra.dashboard.nativeFilters` (with `urlParams` missing or stale), tab-specific permalinks will omit or retain outdated native filter params, and generated screenshots can reflect the wrong filter context compared with the scheduled state.

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.
If a suggested approach is provided above, use it as the authoritative instruction. If no explicit code suggestion is given, you MUST still draft and apply your own minimal, localized fix — do not punt back with 'no suggestion provided, review manually'. Keep the change as small as possible: add a guard clause, gate on a loading state, reorder an await, wrap in a conditional, etc. Do not refactor surrounding code or expand scope beyond the finding.
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

Copy link
Copy Markdown
Contributor

@bito-code-review bito-code-review Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Agent Run #f2f448

Actionable Suggestions - 1
  • superset/commands/report/execute.py - 1
    • Functional bug: missing native filter merge · Line 365-374
Review Details
  • Files reviewed - 1 · Commit Range: 4d6652d..4d6652d
    • superset/commands/report/execute.py
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful
    • MyPy (Static Code Analysis) - ✔︎ Successful
    • Astral Ruff (Static Code Analysis) - ✔︎ Successful

Bito Usage Guide

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 documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

Comment on lines +365 to 374
parent_state = self._report_schedule.extra.get("dashboard") or {}
return [
self._get_tab_url(
{
"anchor": tab_anchor,
"dataMask": None,
"activeTabs": None,
"urlParams": [
["native_filters", native_filter_params] # type: ignore
],
"dataMask": parent_state.get("dataMask"),
"activeTabs": parent_state.get("activeTabs"),
"urlParams": parent_state.get("urlParams")
},
user_friendly=user_friendly,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Functional bug: missing native filter merge

The native_filter_params parameter is passed to _get_tabs_urls but not used in the logic. In the single tab case, it's merged into urlParams to avoid overwriting existing params. This change should do the same for tabs to ensure native filters are included in tab URLs.

Code Review Run #f2f448


Should Bito avoid suggestions like this for future reviews? (Manage Rules)

  • Yes, avoid them

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

alert-reports Namespace | Anything related to the Alert & Reports feature change:backend Requires changing the backend dashboard:tab Related to the usage of tabs in the Dashboard size/XS

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Scheduled report permalinks do not preserve dashboard dataMask state

1 participant