You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by el...@apache.org on 2026/07/15 22:22:30 UTC
(superset) branch poc/per-chart-dashboard-reports updated: feat(reports): per-report toggle for single-column per-chart dashboard capture
This is an automated email from the ASF dual-hosted git repository.
eschutho pushed a commit to branch poc/per-chart-dashboard-reports
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/poc/per-chart-dashboard-reports by this push:
new 5ec96a39dab feat(reports): per-report toggle for single-column per-chart dashboard capture
5ec96a39dab is described below
commit 5ec96a39dab34b3e72b2d5095f6af11b77542139
Author: Elizabeth Thompson <es...@gmail.com>
AuthorDate: Wed Jul 15 22:22:22 2026 +0000
feat(reports): per-report toggle for single-column per-chart dashboard capture
Replace the workspace-wide behavior of PER_CHART_DASHBOARD_REPORTS with a
per-report-schedule opt-in, mirroring Looker's "arrange dashboard tiles in
a single column" option.
The feature flag now only gates visibility of a new checkbox in the report
modal — "Arrange dashboard charts in a single column" — shown for
dashboard reports with a screenshot format (PNG/PDF). The choice is stored
as extra.per_chart_dashboard on the report schedule, and the executor uses
per-chart capture only when both the flag and the report's own toggle are
enabled.
Also fix updateAnchorState to preserve unrelated keys in `extra` (it
previously rebuilt the object with only the dashboard state, which would
have silently dropped the new toggle when a tab was selected).
Backend:
- ReportScheduleExtra gains optional per_chart_dashboard (total=False)
- _get_screenshots() requires flag AND per-report toggle
Frontend:
- FeatureFlag.PerChartDashboardReports enum entry
- Checkbox + tooltip in the report contents panel, persisted via extra
- Extra type gains per_chart_dashboard
Tests: new backend case for flag-on/toggle-off; 3 new RTL tests covering
render, removal when chart content is selected, and toggling.
Co-Authored-By: Claude Fable 5 <no...@anthropic.com>
---
.../superset-ui-core/src/utils/featureFlags.ts | 1 +
.../src/features/alerts/AlertReportModal.test.tsx | 56 +++++++++++++++++++++-
.../src/features/alerts/AlertReportModal.tsx | 39 +++++++++++++++
superset-frontend/src/features/alerts/types.ts | 1 +
superset/commands/report/execute.py | 2 +-
superset/config.py | 10 ++--
superset/reports/types.py | 6 ++-
tests/unit_tests/commands/report/execute_test.py | 24 +++++++++-
8 files changed, 130 insertions(+), 9 deletions(-)
diff --git a/superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts b/superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts
index 35c0a37d562..8266f1768ea 100644
--- a/superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts
+++ b/superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts
@@ -60,6 +60,7 @@ export enum FeatureFlag {
GranularExportControls = 'GRANULAR_EXPORT_CONTROLS',
ListviewsDefaultCardView = 'LISTVIEWS_DEFAULT_CARD_VIEW',
Matrixify = 'MATRIXIFY',
+ PerChartDashboardReports = 'PER_CHART_DASHBOARD_REPORTS',
ScheduledQueries = 'SCHEDULED_QUERIES',
SemanticLayers = 'SEMANTIC_LAYERS',
SqllabBackendPersistence = 'SQLLAB_BACKEND_PERSISTENCE',
diff --git a/superset-frontend/src/features/alerts/AlertReportModal.test.tsx b/superset-frontend/src/features/alerts/AlertReportModal.test.tsx
index 06b10f11e30..5edcb5a8031 100644
--- a/superset-frontend/src/features/alerts/AlertReportModal.test.tsx
+++ b/superset-frontend/src/features/alerts/AlertReportModal.test.tsx
@@ -679,6 +679,54 @@ test('removes ignore cache checkbox when chart is selected', async () => {
).not.toBeInTheDocument();
});
+test('renders single column checkbox when dashboard is selected', async () => {
+ render(<AlertReportModal {...generateMockedProps(false, true, true)} />, {
+ useRedux: true,
+ });
+ userEvent.click(screen.getByTestId('contents-panel'));
+ await screen.findByText(/test dashboard/i);
+ expect(
+ screen.getByRole('checkbox', {
+ name: /arrange dashboard charts in a single column/i,
+ }),
+ ).toBeInTheDocument();
+});
+
+test('removes single column checkbox when chart is selected', async () => {
+ render(<AlertReportModal {...generateMockedProps(false, true, true)} />, {
+ useRedux: true,
+ });
+ userEvent.click(screen.getByTestId('contents-panel'));
+ await screen.findByText(/test dashboard/i);
+ const contentTypeSelector = screen.getByRole('combobox', {
+ name: /select content type/i,
+ });
+ await comboboxSelect(
+ contentTypeSelector,
+ 'Chart',
+ () => screen.getAllByText(/select chart/i)[0],
+ );
+ expect(
+ screen.queryByRole('checkbox', {
+ name: /arrange dashboard charts in a single column/i,
+ }),
+ ).not.toBeInTheDocument();
+});
+
+test('toggles single column checkbox', async () => {
+ render(<AlertReportModal {...generateMockedProps(false, true, true)} />, {
+ useRedux: true,
+ });
+ userEvent.click(screen.getByTestId('contents-panel'));
+ await screen.findByText(/test dashboard/i);
+ const checkbox = screen.getByRole('checkbox', {
+ name: /arrange dashboard charts in a single column/i,
+ });
+ expect(checkbox).not.toBeChecked();
+ await userEvent.click(checkbox);
+ expect(checkbox).toBeChecked();
+});
+
test('open chart button opens explore with slice_id', async () => {
// Render with an existing alert that has a chart selected
render(<AlertReportModal {...generateMockedProps(false, true, false)} />, {
@@ -694,7 +742,9 @@ test('open chart button opens explore with slice_id', async () => {
});
expect(openChartButton).toBeInTheDocument();
- const navSpy = jest.spyOn(navigationUtils, 'navigateTo').mockImplementation(() => null);
+ const navSpy = jest
+ .spyOn(navigationUtils, 'navigateTo')
+ .mockImplementation(() => null);
try {
await userEvent.click(openChartButton);
expect(navSpy).toHaveBeenCalledWith(
@@ -721,7 +771,9 @@ test('open dashboard button opens dashboard url', async () => {
});
expect(openDashButton).toBeInTheDocument();
- const navSpy = jest.spyOn(navigationUtils, 'navigateTo').mockImplementation(() => null);
+ const navSpy = jest
+ .spyOn(navigationUtils, 'navigateTo')
+ .mockImplementation(() => null);
try {
await userEvent.click(openDashButton);
expect(navSpy).toHaveBeenCalledWith(
diff --git a/superset-frontend/src/features/alerts/AlertReportModal.tsx b/superset-frontend/src/features/alerts/AlertReportModal.tsx
index df18f3a0e8e..920492308e2 100644
--- a/superset-frontend/src/features/alerts/AlertReportModal.tsx
+++ b/superset-frontend/src/features/alerts/AlertReportModal.tsx
@@ -628,6 +628,9 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
isFeatureEnabled(FeatureFlag.AlertsAttachReports) || isReport;
const tabsEnabled = isFeatureEnabled(FeatureFlag.AlertReportTabs);
const filtersEnabled = isFeatureEnabled(FeatureFlag.AlertReportsFilter);
+ const perChartEnabled = isFeatureEnabled(
+ FeatureFlag.PerChartDashboardReports,
+ );
const [notificationAddState, setNotificationAddState] =
useState<NotificationAddStatus>('active');
@@ -861,6 +864,7 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
setCurrentAlert(currentAlertData => {
const dashboardState = currentAlertData?.extra?.dashboard;
const extra = {
+ ...currentAlertData?.extra,
dashboard: {
...dashboardState,
anchor: value,
@@ -1488,6 +1492,17 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
setForceScreenshot(e.target.checked);
};
+ const onPerChartDashboardChange = (e: CheckboxChangeEvent) => {
+ const { checked } = e.target;
+ setCurrentAlert(currentAlertData => ({
+ ...currentAlertData,
+ extra: {
+ ...currentAlertData?.extra,
+ per_chart_dashboard: checked,
+ },
+ }));
+ };
+
const onChangeDashboardFilter = (idx: number, nativeFilterId: string) => {
if (
!nativeFilterId ||
@@ -2652,6 +2667,30 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
</Checkbox>
</div>
)}
+ {perChartEnabled &&
+ isScreenshot &&
+ contentType === ContentType.Dashboard && (
+ <div className="inline-container">
+ <Checkbox
+ data-test="per-chart-dashboard"
+ checked={
+ currentAlert?.extra?.per_chart_dashboard || false
+ }
+ onChange={onPerChartDashboardChange}
+ >
+ {t('Arrange dashboard charts in a single column')}
+ </Checkbox>
+ <InfoTooltip
+ tooltip={t(
+ 'Each chart is captured individually and stacked ' +
+ 'vertically in the report. Dashboard filters ' +
+ 'are applied to all charts. Charts that fail ' +
+ 'to load are skipped instead of blocking the ' +
+ 'report.',
+ )}
+ />
+ </div>
+ )}
</>
),
},
diff --git a/superset-frontend/src/features/alerts/types.ts b/superset-frontend/src/features/alerts/types.ts
index 7a518f5394a..fae31c3b728 100644
--- a/superset-frontend/src/features/alerts/types.ts
+++ b/superset-frontend/src/features/alerts/types.ts
@@ -110,6 +110,7 @@ export type ExtraNativeFilter = {
export type Extra = {
dashboard?: DashboardState;
+ per_chart_dashboard?: boolean;
};
export type Operator = '<' | '>' | '<=' | '>=' | '==' | '!=' | 'not null';
diff --git a/superset/commands/report/execute.py b/superset/commands/report/execute.py
index c75ec635bda..a7d4004eb74 100644
--- a/superset/commands/report/execute.py
+++ b/superset/commands/report/execute.py
@@ -465,7 +465,7 @@ class BaseReportState:
]
per_chart_reports = feature_flag_manager.is_feature_enabled(
"PER_CHART_DASHBOARD_REPORTS"
- )
+ ) and bool((self._report_schedule.extra or {}).get("per_chart_dashboard"))
try:
imges = []
for screenshot in screenshots:
diff --git a/superset/config.py b/superset/config.py
index 2c6d81494c4..cfa036bf7e0 100644
--- a/superset/config.py
+++ b/superset/config.py
@@ -732,10 +732,12 @@ DEFAULT_FEATURE_FLAGS: dict[str, bool] = {
# When impersonating a user, use the email prefix instead of username
# @lifecycle: testing
"IMPERSONATE_WITH_EMAIL_PREFIX": False,
- # Capture dashboard reports chart-by-chart and arrange the images in a
- # single column, instead of one full-dashboard screenshot. Each chart
- # waits for its own loading spinner, so one slow chart cannot block or
- # blank the entire report. Requires PLAYWRIGHT_REPORTS_AND_THUMBNAILS.
+ # Expose a per-report option to capture dashboard reports chart-by-chart
+ # and arrange the images in a single column, instead of one
+ # full-dashboard screenshot. Each chart waits for its own loading
+ # spinner, so one slow chart cannot block or blank the entire report.
+ # Enabled per report via the "Arrange dashboard charts in a single
+ # column" checkbox. Requires PLAYWRIGHT_REPORTS_AND_THUMBNAILS.
# @lifecycle: testing
"PER_CHART_DASHBOARD_REPORTS": False,
# Replace Selenium with Playwright for reports and thumbnails.
diff --git a/superset/reports/types.py b/superset/reports/types.py
index d487e3ad237..e0a298b5a53 100644
--- a/superset/reports/types.py
+++ b/superset/reports/types.py
@@ -19,5 +19,9 @@ from typing import TypedDict
from superset.dashboards.permalink.types import DashboardPermalinkState
-class ReportScheduleExtra(TypedDict):
+class ReportScheduleExtra(TypedDict, total=False):
dashboard: DashboardPermalinkState
+ # Capture each chart individually and arrange the images in a single
+ # column, instead of one full-dashboard screenshot. Only honored when
+ # the PER_CHART_DASHBOARD_REPORTS feature flag is enabled.
+ per_chart_dashboard: bool
diff --git a/tests/unit_tests/commands/report/execute_test.py b/tests/unit_tests/commands/report/execute_test.py
index 8c1e4c53bf3..507e0983df6 100644
--- a/tests/unit_tests/commands/report/execute_test.py
+++ b/tests/unit_tests/commands/report/execute_test.py
@@ -1724,7 +1724,9 @@ def test_get_url_for_csv_uses_post_processed_type(
)
-def _make_dashboard_report_state(mocker: MockerFixture) -> BaseReportState:
+def _make_dashboard_report_state(
+ mocker: MockerFixture, per_chart_dashboard: bool = True
+) -> BaseReportState:
"""Build a BaseReportState for a dashboard report with screenshot mocks."""
mock_report_schedule: ReportSchedule = mocker.Mock(spec=ReportSchedule)
mock_report_schedule.chart = False
@@ -1732,6 +1734,9 @@ def _make_dashboard_report_state(mocker: MockerFixture) -> BaseReportState:
mock_report_schedule.dashboard.digest = "digest"
mock_report_schedule.custom_width = None
mock_report_schedule.custom_height = None
+ mock_report_schedule.extra = (
+ {"per_chart_dashboard": True} if per_chart_dashboard else {}
+ )
mocker.patch(
"superset.commands.report.execute.get_executor",
@@ -1804,3 +1809,20 @@ def test_get_screenshots_per_chart_mode_disabled(mocker: MockerFixture, app) ->
assert result == [b"full_dashboard"]
per_chart.assert_not_called()
+
+
+@with_feature_flags(PER_CHART_DASHBOARD_REPORTS=True)
+def test_get_screenshots_per_chart_report_toggle_off(
+ mocker: MockerFixture, app
+) -> None:
+ """The flag alone is not enough; the report's own toggle must be on."""
+ class_instance = _make_dashboard_report_state(mocker, per_chart_dashboard=False)
+ per_chart = mocker.patch.object(DashboardScreenshot, "get_per_chart_screenshots")
+ mocker.patch.object(
+ DashboardScreenshot, "get_screenshot", return_value=b"full_dashboard"
+ )
+
+ result = class_instance._get_screenshots()
+
+ assert result == [b"full_dashboard"]
+ per_chart.assert_not_called()