integration tests for report tabs

superset-frontend/src/dashboard/containers/DashboardPage.tsx

@@ -147,7 +147,6 @@ export const DashboardPage: FC<PageProps> = ({ idOrSlug }: PageProps) => {
         if (!isDashboardHydrated.current) {
           isDashboardHydrated.current = true;
         }
-        console.log('active tabs:', activeTabs);
         dispatch(
           hydrateDashboard({
             history,

superset-frontend/src/features/alerts/AlertReportModal.tsx

@@ -219,6 +219,10 @@ const StyledModal = styled(Modal)`
   }
 `;
 
+const StyledTreeSelect = styled(TreeSelect)`
+  width: 100%;
+`;
+
 const StyledSwitchContainer = styled.div`
   display: flex;
   align-items: center;
@@ -599,7 +603,7 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
     setNotificationAddState('active');
   };
 
-  const updateAnchorState = (value: string) => {
+  const updateAnchorState = (value: any) => {
     setCurrentAlert(currentAlertData => {
       const dashboardState = currentAlertData?.extra?.dashboard;
       const extra = {
@@ -806,11 +810,16 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
       SupersetClient.get({
         endpoint: `/api/v1/dashboard/${dashboard.value}/tabs`,
       }).then(response => {
-        const tabs = response.json.result.tab_tree;
-        setTabOptions(tabs);
+        const { tab_tree: tabTree, all_tabs: allTabs } = response.json.result;
+        setTabOptions(tabTree);
+        if (currentAlert?.extra?.dashboard?.anchor) {
+          if (!(currentAlert?.extra?.dashboard?.anchor in allTabs)) {
+            updateAnchorState(undefined);
+          }
+        }
       });
     }
-  }, [dashboard, tabsEnabled]);
+  }, [dashboard, tabsEnabled, currentAlert?.extra]);
 
   const databaseLabel = currentAlert?.database && !currentAlert.database.label;
   useEffect(() => {
@@ -923,6 +932,28 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
       endpoint: `/api/v1/chart/${chart.value}`,
     }).then(response => setChartVizType(response.json.result.viz_type));
 
+  const updateEmailSubject = () => {
+    if (contentType === 'chart') {
+      if (currentAlert?.name || currentAlert?.chart?.label) {
+        setEmailSubject(
+          `${currentAlert?.name}: ${currentAlert?.chart?.label || ''}`,
+        );
+      } else {
+        setEmailSubject('');
+      }
+    } else if (contentType === 'dashboard') {
+      if (currentAlert?.name || currentAlert?.dashboard?.label) {
+        setEmailSubject(
+          `${currentAlert?.name}: ${currentAlert?.dashboard?.label || ''}`,
+        );
+      } else {
+        setEmailSubject('');
+      }
+    } else {
+      setEmailSubject('');
+    }
+  };
+
   // Handle input/textarea updates
   const onInputChange = (
     event: ChangeEvent<HTMLTextAreaElement | HTMLInputElement>,
@@ -1303,28 +1334,6 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
     return titleText;
   };
 
-  const updateEmailSubject = () => {
-    if (contentType === 'chart') {
-      if (currentAlert?.name || currentAlert?.chart?.label) {
-        setEmailSubject(
-          `${currentAlert?.name}: ${currentAlert?.chart?.label || ''}`,
-        );
-      } else {
-        setEmailSubject('');
-      }
-    } else if (contentType === 'dashboard') {
-      if (currentAlert?.name || currentAlert?.dashboard?.label) {
-        setEmailSubject(
-          `${currentAlert?.name}: ${currentAlert?.dashboard?.label || ''}`,
-        );
-      } else {
-        setEmailSubject('');
-      }
-    } else {
-      setEmailSubject('');
-    }
-  };
-
   const handleErrorUpdate = (hasError: boolean) => {
     setEmailError(hasError);
   };
@@ -1648,12 +1657,11 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
             <StyledInputContainer>
               <>
                 <div className="control-label">{t('Select tab')}</div>
-                <TreeSelect
+                <StyledTreeSelect
                   disabled={tabOptions?.length === 0}
                   treeData={tabOptions}
                   value={currentAlert?.extra?.dashboard?.anchor}
-                  onSelect={value => updateAnchorState(value)}
-                  style={{ width: '100%' }}
+                  onSelect={updateAnchorState}
                   placeholder={t('Select a tab')}
                 />
               </>

superset/commands/report/execute.py

@@ -168,15 +168,15 @@ def _get_url(
                 force=force,
                 **kwargs,
             )
-
         # If we need to render dashboard in a specific state, use stateful permalink
-        if dashboard_state := self._report_schedule.extra.get("dashboard"):
+        if (
+            dashboard_state := self._report_schedule.extra.get("dashboard")
+        ) and feature_flag_manager.is_feature_enabled("ALERT_REPORT_TABS"):
             permalink_key = CreateDashboardPermalinkCommand(
                 dashboard_id=str(self._report_schedule.dashboard.uuid),
                 state=dashboard_state,
             ).run()
             return get_url_path("Superset.dashboard_permalink", key=permalink_key)
-
         dashboard = self._report_schedule.dashboard
         dashboard_id_or_slug = (
             dashboard.uuid if dashboard and dashboard.uuid else dashboard.id

tests/integration_tests/reports/commands/execute_dashboard_report_tests.py

@@ -18,12 +18,14 @@
 from unittest.mock import MagicMock, patch
 from uuid import uuid4
 
+import pytest
 from flask import current_app
 
 from superset.commands.dashboard.permalink.create import CreateDashboardPermalinkCommand
 from superset.commands.report.execute import AsyncExecuteReportScheduleCommand
 from superset.models.dashboard import Dashboard
 from superset.reports.models import ReportSourceFormat
+from superset.utils.urls import get_url_path
 from tests.integration_tests.fixtures.tabbed_dashboard import (
     tabbed_dashboard,  # noqa: F401
 )
@@ -34,22 +36,21 @@
 @patch(
     "superset.commands.report.execute.DashboardScreenshot",
 )
-@patch(
-    "superset.commands.dashboard.permalink.create.CreateDashboardPermalinkCommand.run"
+@patch.dict(
+    "superset.extensions.feature_flag_manager._feature_flags", ALERT_REPORT_TABS=True
 )
+@pytest.mark.usefixtures("login_as_admin")
 def test_report_for_dashboard_with_tabs(
-    create_dashboard_permalink_mock: MagicMock,
     dashboard_screenshot_mock: MagicMock,
     send_email_smtp_mock: MagicMock,
     tabbed_dashboard: Dashboard,  # noqa: F811
 ) -> None:
-    create_dashboard_permalink_mock.return_value = "permalink"
     dashboard_screenshot_mock.get_screenshot.return_value = b"test-image"
     current_app.config["ALERT_REPORTS_NOTIFICATION_DRY_RUN"] = False
 
     with create_dashboard_report(
         dashboard=tabbed_dashboard,
-        extra={"active_tabs": ["TAB-L1B", "TAB-L2BB"]},
+        extra={"dashboard": {"active_tabs": ["TAB-L1B", "TAB-L2BB"]}},
         name="test report tabbed dashboard",
     ) as report_schedule:
         dashboard: Dashboard = report_schedule.dashboard
@@ -61,9 +62,12 @@ def test_report_for_dashboard_with_tabs(
             str(dashboard.id), dashboard_state
         ).run()
 
+        expected_url = get_url_path("Superset.dashboard_permalink", key=permalink_key)
+
         assert dashboard_screenshot_mock.call_count == 1
-        url = dashboard_screenshot_mock.call_args.args[0]
-        assert url.endswith(f"/superset/dashboard/p/{permalink_key}/")
+        called_url = dashboard_screenshot_mock.call_args.args[0]
+
+        assert called_url == expected_url
         assert send_email_smtp_mock.call_count == 1
         assert len(send_email_smtp_mock.call_args.kwargs["images"]) == 1
 
@@ -72,22 +76,21 @@ def test_report_for_dashboard_with_tabs(
 @patch(
     "superset.commands.report.execute.DashboardScreenshot",
 )
-@patch(
-    "superset.commands.dashboard.permalink.create.CreateDashboardPermalinkCommand.run"
+@patch.dict(
+    "superset.extensions.feature_flag_manager._feature_flags", ALERT_REPORT_TABS=True
 )
+@pytest.mark.usefixtures("login_as_admin")
 def test_report_with_header_data(
-    create_dashboard_permalink_mock: MagicMock,
     dashboard_screenshot_mock: MagicMock,
     send_email_smtp_mock: MagicMock,
     tabbed_dashboard: Dashboard,  # noqa: F811
 ) -> None:
-    create_dashboard_permalink_mock.return_value = "permalink"
     dashboard_screenshot_mock.get_screenshot.return_value = b"test-image"
     current_app.config["ALERT_REPORTS_NOTIFICATION_DRY_RUN"] = False
 
     with create_dashboard_report(
         dashboard=tabbed_dashboard,
-        extra={"active_tabs": ["TAB-L1B"]},
+        extra={"dashboard": {"active_tabs": ["TAB-L1B", "TAB-L2BB"]}},
         name="test report tabbed dashboard",
     ) as report_schedule:
         dashboard: Dashboard = report_schedule.dashboard
@@ -101,6 +104,7 @@ def test_report_with_header_data(
 
         assert dashboard_screenshot_mock.call_count == 1
         url = dashboard_screenshot_mock.call_args.args[0]
+
         assert url.endswith(f"/superset/dashboard/p/{permalink_key}/")
         assert send_email_smtp_mock.call_count == 1
         header_data = send_email_smtp_mock.call_args.kwargs["header_data"]

tests/integration_tests/reports/commands_tests.py

@@ -60,6 +60,7 @@
 )
 from superset.commands.report.log_prune import AsyncPruneReportScheduleLogCommand
 from superset.exceptions import SupersetException
+from superset.key_value.models import KeyValueEntry
 from superset.models.core import Database
 from superset.models.dashboard import Dashboard
 from superset.models.slice import Slice
@@ -82,6 +83,9 @@
     load_birth_names_dashboard_with_slices,  # noqa: F401
     load_birth_names_data,  # noqa: F401
 )
+from tests.integration_tests.fixtures.tabbed_dashboard import (
+    tabbed_dashboard,  # noqa: F401
+)
 from tests.integration_tests.fixtures.world_bank_dashboard import (
     load_world_bank_dashboard_with_slices_module_scope,  # noqa: F401
     load_world_bank_data,  # noqa: F401
@@ -91,6 +95,7 @@
     create_report_notification,
     CSV_FILE,
     DEFAULT_OWNER_EMAIL,
+    reset_key_values,
     SCREENSHOT_FILE,
     TEST_ID,
 )
@@ -1064,6 +1069,93 @@ def test_email_dashboard_report_schedule(
             statsd_mock.assert_called_once_with("reports.email.send.ok", 1)
 
 
+@pytest.mark.usefixtures("tabbed_dashboard")
+@patch("superset.utils.screenshots.DashboardScreenshot.get_screenshot")
+@patch("superset.reports.notifications.email.send_email_smtp")
+@patch.dict(
+    "superset.extensions.feature_flag_manager._feature_flags", ALERT_REPORT_TABS=True
+)
+def test_email_dashboard_report_schedule_with_tab_anchor(
+    _email_mock,
+    _screenshot_mock,
+):
+    """
+    ExecuteReport Command: Test dashboard email report schedule with tab metadata
+    """
+    with freeze_time("2020-01-01T00:00:00Z"):
+        with patch.object(current_app.config["STATS_LOGGER"], "gauge") as statsd_mock:
+            # get tabbed dashboard fixture
+            dashboard = db.session.query(Dashboard).all()[1]
+            # build report_schedule
+            report_schedule = create_report_notification(
+                email_target="[email protected]",
+                dashboard=dashboard,
+                extra={"dashboard": {"anchor": "TAB-L2AB"}},
+            )
+            AsyncExecuteReportScheduleCommand(
+                TEST_ID, report_schedule.id, datetime.utcnow()
+            ).run()
+
+            # Assert logs are correct
+            assert_log(ReportState.SUCCESS)
+            statsd_mock.assert_called_once_with("reports.email.send.ok", 1)
+
+            pl = (
+                db.session.query(KeyValueEntry)
+                .order_by(KeyValueEntry.id.desc())
+                .first()
+            )
+
+            value = json.loads(pl.value)
+            # test that report schedule extra json matches permalink state
+            assert report_schedule.extra["dashboard"] == value["state"]
+
+            # remove report_schedule
+            cleanup_report_schedule(report_schedule)
+            # remove permalink kvalues
+            reset_key_values()
+
+
+@pytest.mark.usefixtures("tabbed_dashboard")
+@patch("superset.utils.screenshots.DashboardScreenshot.get_screenshot")
+@patch("superset.reports.notifications.email.send_email_smtp")
+@patch.dict(
+    "superset.extensions.feature_flag_manager._feature_flags", ALERT_REPORT_TABS=False
+)
+def test_email_dashboard_report_schedule_disabled_tabs(
+    _email_mock,
+    _screenshot_mock,
+):
+    """
+    ExecuteReport Command: Test dashboard email report schedule with tab metadata
+    """
+    with freeze_time("2020-01-01T00:00:00Z"):
+        with patch.object(current_app.config["STATS_LOGGER"], "gauge") as statsd_mock:
+            # get tabbed dashboard fixture
+            dashboard = db.session.query(Dashboard).all()[1]
+            # build report_schedule
+            report_schedule = create_report_notification(
+                email_target="[email protected]",
+                dashboard=dashboard,
+                extra={"dashboard": {"anchor": "TAB-L2AB"}},
+            )
+            AsyncExecuteReportScheduleCommand(
+                TEST_ID, report_schedule.id, datetime.utcnow()
+            ).run()
+
+            # Assert logs are correct
+            assert_log(ReportState.SUCCESS)
+            statsd_mock.assert_called_once_with("reports.email.send.ok", 1)
+
+            permalinks = db.session.query(KeyValueEntry).all()
+
+            # test that report schedule extra json matches permalink state
+            assert len(permalinks) == 0
+
+            # remove report_schedule
+            cleanup_report_schedule(report_schedule)
+
+
 @pytest.mark.usefixtures(
     "load_birth_names_dashboard_with_slices",
     "create_report_email_dashboard_force_screenshot",

tests/integration_tests/reports/utils.py

@@ -22,6 +22,7 @@
 from flask_appbuilder.security.sqla.models import User
 
 from superset import db, security_manager
+from superset.key_value.models import KeyValueEntry
 from superset.models.core import Database
 from superset.models.dashboard import Dashboard
 from superset.models.slice import Slice
@@ -199,3 +200,8 @@ def create_dashboard_report(dashboard, extra, **kwargs):
 
     if error:
         raise error
+
+
+def reset_key_values() -> None:
+    db.session.query(KeyValueEntry).delete()
+    db.session.commit()