feat: bypass cache on screenshots for alerts (apache#17695)

* feat: bypass cache on screenshots for alerts

* Update existing tests

* Add backend test

* Add frontend test

superset-frontend/src/chart/Chart.jsx

@@ -51,6 +51,7 @@ const propTypes = {
   timeout: PropTypes.number,
   vizType: PropTypes.string.isRequired,
   triggerRender: PropTypes.bool,
+  force: PropTypes.bool,
   isFiltersInitialized: PropTypes.bool,
   isDeactivatedViz: PropTypes.bool,
   // state
@@ -84,6 +85,7 @@ const defaultProps = {
   dashboardId: null,
   chartStackTrace: null,
   isDeactivatedViz: false,
+  force: false,
 };
 
 const Styles = styled.div`
@@ -143,7 +145,7 @@ class Chart extends React.PureComponent {
       // Load saved chart with a GET request
       this.props.actions.getSavedChart(
         this.props.formData,
-        false,
+        this.props.force,
         this.props.timeout,
         this.props.chartId,
         this.props.dashboardId,
@@ -153,7 +155,7 @@ class Chart extends React.PureComponent {
       // Create chart with POST request
       this.props.actions.postChartFormData(
         this.props.formData,
-        false,
+        this.props.force,
         this.props.timeout,
         this.props.chartId,
         this.props.dashboardId,

superset-frontend/src/explore/components/ExploreChartPanel.jsx

@@ -52,6 +52,7 @@ const propTypes = {
   form_data: PropTypes.object,
   ownState: PropTypes.object,
   standalone: PropTypes.number,
+  force: PropTypes.bool,
   timeout: PropTypes.number,
   refreshOverlayVisible: PropTypes.bool,
   chart: chartPropShape,
@@ -131,7 +132,7 @@ const ExploreChartPanel = props => {
       if (slice && slice.query_context === null) {
         const queryContext = buildV1ChartDataPayload({
           formData: slice.form_data,
-          force: false,
+          force: props.force,
           resultFormat: 'json',
           resultType: 'full',
           setDataMask: null,
@@ -227,6 +228,7 @@ const ExploreChartPanel = props => {
           chartId={chart.id}
           chartStatus={chart.chartStatus}
           triggerRender={props.triggerRender}
+          force={props.force}
           datasource={props.datasource}
           errorMessage={props.errorMessage}
           formData={props.form_data}

superset-frontend/src/explore/components/ExploreViewContainer.jsx

@@ -73,6 +73,7 @@ const propTypes = {
   forcedHeight: PropTypes.string,
   form_data: PropTypes.object.isRequired,
   standalone: PropTypes.number.isRequired,
+  force: PropTypes.bool,
   timeout: PropTypes.number,
   impressionId: PropTypes.string,
   vizType: PropTypes.string,
@@ -202,6 +203,8 @@ function ExploreViewContainer(props) {
         formData,
         props.standalone ? URL_PARAMS.standalone.name : null,
         false,
+        {},
+        props.force,
       );
 
       try {
@@ -220,7 +223,7 @@ function ExploreViewContainer(props) {
         );
       }
     },
-    [props.form_data, props.standalone],
+    [props.form_data, props.standalone, props.force],
   );
 
   const handlePopstate = useCallback(() => {
@@ -229,7 +232,7 @@ function ExploreViewContainer(props) {
       props.actions.setExploreControls(formData);
       props.actions.postChartFormData(
         formData,
-        false,
+        props.force,
         props.timeout,
         props.chart.id,
       );
@@ -639,6 +642,7 @@ function mapStateToProps(state) {
     table_name: form_data.datasource_name,
     vizType: form_data.viz_type,
     standalone: explore.standalone,
+    force: explore.force,
     forcedHeight: explore.forced_height,
     chart,
     timeout: explore.common.conf.SUPERSET_WEBSERVER_TIMEOUT,

superset-frontend/src/explore/exploreUtils/getExploreLongUrl.test.ts

@@ -45,12 +45,27 @@ test('Get url when endpointType:standalone', () => {
   expect(
     getExploreLongUrl(
       params.formData,
-      params.endpointType,
+      'standalone',
       params.allowOverflow,
       params.extraSearch,
     ),
   ).toBe(
-    '/superset/explore/?same=any-string&form_data=%7B%22datasource%22%3A%22datasource%22%2C%22viz_type%22%3A%22viz_type%22%7D',
+    '/superset/explore/?same=any-string&form_data=%7B%22datasource%22%3A%22datasource%22%2C%22viz_type%22%3A%22viz_type%22%7D&standalone=1',
+  );
+});
+
+test('Get url when endpointType:standalone and force:true', () => {
+  const params = createParams();
+  expect(
+    getExploreLongUrl(
+      params.formData,
+      'standalone',
+      params.allowOverflow,
+      params.extraSearch,
+      true,
+    ),
+  ).toBe(
+    '/superset/explore/?same=any-string&form_data=%7B%22datasource%22%3A%22datasource%22%2C%22viz_type%22%3A%22viz_type%22%7D&force=1&standalone=1',
   );
 });
 

superset-frontend/src/explore/exploreUtils/index.js

@@ -99,6 +99,7 @@ export function getExploreLongUrl(
   endpointType,
   allowOverflow = true,
   extraSearch = {},
+  force = false,
 ) {
   if (!formData.datasource) {
     return null;
@@ -112,6 +113,9 @@ export function getExploreLongUrl(
   });
   search.form_data = safeStringify(formData);
   if (endpointType === URL_PARAMS.standalone.name) {
+    if (force) {
+      search.force = '1';
+    }
     search.standalone = DashboardStandaloneMode.HIDE_NAV;
   }
   const url = uri.directory(directory).search(search).toString();
@@ -120,9 +124,15 @@ export function getExploreLongUrl(
       datasource: formData.datasource,
       viz_type: formData.viz_type,
     };
-    return getExploreLongUrl(minimalFormData, endpointType, false, {
-      URL_IS_TOO_LONG_TO_SHARE: null,
-    });
+    return getExploreLongUrl(
+      minimalFormData,
+      endpointType,
+      false,
+      {
+        URL_IS_TOO_LONG_TO_SHARE: null,
+      },
+      force,
+    );
   }
   return url;
 }

superset-frontend/src/explore/reducers/getInitialState.ts

@@ -48,6 +48,7 @@ export interface ExlorePageBootstrapData extends JsonObject {
   form_data: QueryFormData;
   slice: Slice | null;
   standalone: boolean;
+  force: boolean;
   user: UserWithPermissionsAndRoles;
 }
 

superset/reports/commands/execute.py

@@ -72,6 +72,7 @@
     DashboardScreenshot,
 )
 from superset.utils.urls import get_url_path
+from superset.utils.webdriver import DashboardStandaloneMode
 
 logger = logging.getLogger(__name__)
 
@@ -145,6 +146,16 @@ def _get_url(
         """
         Get the url for this report schedule: chart or dashboard
         """
+        # For alerts we always want to send a fresh screenshot, bypassing
+        # the cache.
+        # TODO (betodealmeida): allow to specify per report if users want
+        # to bypass the cache as well.
+        force = (
+            "true"
+            if self._report_schedule.type == ReportScheduleType.ALERT
+            else "false"
+        )
+
         if self._report_schedule.chart:
             if result_format in {
                 ChartDataResultFormat.CSV,
@@ -155,17 +166,22 @@ def _get_url(
                     pk=self._report_schedule.chart_id,
                     format=result_format.value,
                     type=ChartDataResultType.POST_PROCESSED.value,
+                    force=force,
                 )
             return get_url_path(
-                "Superset.slice",
+                "Superset.explore",
                 user_friendly=user_friendly,
-                slice_id=self._report_schedule.chart_id,
+                form_data=json.dumps({"slice_id": self._report_schedule.chart_id}),
+                standalone="true",
+                force=force,
                 **kwargs,
             )
         return get_url_path(
             "Superset.dashboard",
             user_friendly=user_friendly,
             dashboard_id_or_slug=self._report_schedule.dashboard_id,
+            standalone=DashboardStandaloneMode.REPORT.value,
+            force=force,
             **kwargs,
         )
 
@@ -187,7 +203,7 @@ def _get_screenshot(self) -> bytes:
         """
         screenshot: Optional[BaseScreenshot] = None
         if self._report_schedule.chart:
-            url = self._get_url(standalone="true")
+            url = self._get_url()
             logger.info("Screenshotting chart at %s", url)
             screenshot = ChartScreenshot(
                 url,

superset/utils/core.py

@@ -303,7 +303,7 @@ class ReservedUrlParameters(str, Enum):
     @staticmethod
     def is_standalone_mode() -> Optional[bool]:
         standalone_param = request.args.get(ReservedUrlParameters.STANDALONE.value)
-        standalone: Optional[bool] = (
+        standalone: Optional[bool] = bool(
             standalone_param and standalone_param != "false" and standalone_param != "0"
         )
         return standalone

superset/utils/webdriver.py

@@ -21,7 +21,6 @@
 from typing import Any, Dict, Optional, Tuple, TYPE_CHECKING
 
 from flask import current_app
-from requests.models import PreparedRequest
 from selenium.common.exceptions import (
     StaleElementReferenceException,
     TimeoutException,
@@ -107,11 +106,6 @@ def destroy(driver: WebDriver, tries: int = 2) -> None:
     def get_screenshot(
         self, url: str, element_name: str, user: "User"
     ) -> Optional[bytes]:
-        params = {"standalone": DashboardStandaloneMode.REPORT.value}
-        req = PreparedRequest()
-        req.prepare_url(url, params)
-        url = req.url or ""
-
         driver = self.auth(user)
         driver.set_window_size(*self._window)
         driver.get(url)

superset/views/core.py

@@ -842,6 +842,7 @@ def explore(
                 query_context,
             )
         standalone_mode = ReservedUrlParameters.is_standalone_mode()
+        force = request.args.get("force") in {"force", "1", "true"}
         dummy_datasource_data: Dict[str, Any] = {
             "type": datasource_type,
             "name": datasource_name,
@@ -866,6 +867,7 @@ def explore(
             "datasource_type": datasource_type,
             "slice": slc.data if slc else None,
             "standalone": standalone_mode,
+            "force": force,
             "user": bootstrap_user_data(g.user, include_perms=True),
             "forced_height": request.args.get("height"),
             "common": common_bootstrap_payload(),

tests/integration_tests/reports/commands_tests.py

@@ -663,8 +663,47 @@ def test_email_chart_report_schedule(
         )
         # assert that the link sent is correct
         assert (
-            f'<a href="http://0.0.0.0:8080/superset/slice/'
-            f'{create_report_email_chart.chart.id}/">Explore in Superset</a>'
+            '<a href="http://0.0.0.0:8080/superset/explore/?'
+            "form_data=%7B%22slice_id%22%3A+"
+            f"{create_report_email_chart.chart.id}%7D&"
+            'standalone=true&force=false">Explore in Superset</a>'
+            in email_mock.call_args[0][2]
+        )
+        # Assert the email smtp address
+        assert email_mock.call_args[0][0] == notification_targets[0]
+        # Assert the email inline screenshot
+        smtp_images = email_mock.call_args[1]["images"]
+        assert smtp_images[list(smtp_images.keys())[0]] == SCREENSHOT_FILE
+        # Assert logs are correct
+        assert_log(ReportState.SUCCESS)
+
+
+@pytest.mark.usefixtures(
+    "load_birth_names_dashboard_with_slices", "create_alert_email_chart"
+)
+@patch("superset.reports.notifications.email.send_email_smtp")
+@patch("superset.utils.screenshots.ChartScreenshot.get_screenshot")
+def test_email_chart_alert_schedule(
+    screenshot_mock, email_mock, create_alert_email_chart,
+):
+    """
+    ExecuteReport Command: Test chart email alert schedule with screenshot
+    """
+    # setup screenshot mock
+    screenshot_mock.return_value = SCREENSHOT_FILE
+
+    with freeze_time("2020-01-01T00:00:00Z"):
+        AsyncExecuteReportScheduleCommand(
+            TEST_ID, create_alert_email_chart.id, datetime.utcnow()
+        ).run()
+
+        notification_targets = get_target_from_report_schedule(create_alert_email_chart)
+        # assert that the link sent is correct
+        assert (
+            '<a href="http://0.0.0.0:8080/superset/explore/?'
+            "form_data=%7B%22slice_id%22%3A+"
+            f"{create_alert_email_chart.chart.id}%7D&"
+            'standalone=true&force=true">Explore in Superset</a>'
             in email_mock.call_args[0][2]
         )
         # Assert the email smtp address
@@ -729,8 +768,10 @@ def test_email_chart_report_schedule_with_csv(
         )
         # assert that the link sent is correct
         assert (
-            f'<a href="http://0.0.0.0:8080/superset/slice/'
-            f'{create_report_email_chart_with_csv.chart.id}/">Explore in Superset</a>'
+            '<a href="http://0.0.0.0:8080/superset/explore/?'
+            "form_data=%7B%22slice_id%22%3A+"
+            f"{create_report_email_chart_with_csv.chart.id}%7D&"
+            'standalone=true&force=false">Explore in Superset</a>'
             in email_mock.call_args[0][2]
         )
         # Assert the email smtp address