From fb5d77e4047dee1e064b6c2dc3cb1bba01b01273 Mon Sep 17 00:00:00 2001 From: "Hugh A. Miles II" Date: Fri, 4 Mar 2022 12:30:40 -0800 Subject: [PATCH] feat: Allow users to bust cache in report dashboard + alerts charts + alert dashboards (#18795) * wip * add force cahce bypass option to alerts * remove default for alerts to bypass cache * save for now * save for now * fix * commenting out for now * fix linting * remove link * add back force id test * add frontend test * address (cherry picked from commit 8c52fe347699b4d529997ea7600c72874d49f905) --- superset-frontend/src/chart/Chart.jsx | 6 ++- superset-frontend/src/constants.ts | 4 ++ .../CRUD/alert/AlertReportModal.test.jsx | 18 +++++++ .../src/views/CRUD/alert/AlertReportModal.tsx | 24 ++++----- superset/reports/commands/execute.py | 3 +- .../reports/commands_tests.py | 49 ++++++++++++++++++- 6 files changed, 87 insertions(+), 17 deletions(-) diff --git a/superset-frontend/src/chart/Chart.jsx b/superset-frontend/src/chart/Chart.jsx index 7fb5f47808f7c..734ebb8ce9177 100644 --- a/superset-frontend/src/chart/Chart.jsx +++ b/superset-frontend/src/chart/Chart.jsx @@ -27,6 +27,8 @@ import Loading from 'src/components/Loading'; import { EmptyStateBig } from 'src/components/EmptyState'; import ErrorBoundary from 'src/components/ErrorBoundary'; import { Logger, LOG_ACTIONS_RENDER_CHART } from 'src/logger/LogUtils'; +import { URL_PARAMS } from 'src/constants'; +import { getUrlParam } from 'src/utils/urlUtils'; import ChartRenderer from './ChartRenderer'; import { ChartErrorMessage } from './ChartErrorMessage'; @@ -157,7 +159,7 @@ class Chart extends React.PureComponent { // Load saved chart with a GET request this.props.actions.getSavedChart( this.props.formData, - this.props.force, + this.props.force || getUrlParam(URL_PARAMS.force), // allow override via url params force=true this.props.timeout, this.props.chartId, this.props.dashboardId, @@ -167,7 +169,7 @@ class Chart extends React.PureComponent { // Create chart with POST request this.props.actions.postChartFormData( this.props.formData, - this.props.force, + this.props.force || getUrlParam(URL_PARAMS.force), // allow override via url params force=true this.props.timeout, this.props.chartId, this.props.dashboardId, diff --git a/superset-frontend/src/constants.ts b/superset-frontend/src/constants.ts index 4444e8ea1f4a8..b54fc1173c28f 100644 --- a/superset-frontend/src/constants.ts +++ b/superset-frontend/src/constants.ts @@ -67,6 +67,10 @@ export const URL_PARAMS = { name: 'dataset_id', type: 'string', }, + force: { + name: 'force', + type: 'boolean', + }, } as const; /** diff --git a/superset-frontend/src/views/CRUD/alert/AlertReportModal.test.jsx b/superset-frontend/src/views/CRUD/alert/AlertReportModal.test.jsx index fb58c13043d43..1598e5a926f32 100644 --- a/superset-frontend/src/views/CRUD/alert/AlertReportModal.test.jsx +++ b/superset-frontend/src/views/CRUD/alert/AlertReportModal.test.jsx @@ -340,4 +340,22 @@ describe('AlertReportModal', () => { await waitForComponentToPaint(wrapper); expect(wrapper.find('textarea[name="recipients"]')).toHaveLength(1); }); + + it('renders bypass cache checkbox', async () => { + const bypass = wrapper.find('[data-test="bypass-cache"]'); + expect(bypass).toExist(); + }); + + it('renders no bypass cache checkbox when alert', async () => { + const props = { + ...mockedProps, + alert: mockData, + isReport: false, + }; + + const alertWrapper = await mountAndWait(props); + + const bypass = alertWrapper.find('[data-test="bypass-cache"]'); + expect(bypass).not.toExist(); + }); }); diff --git a/superset-frontend/src/views/CRUD/alert/AlertReportModal.tsx b/superset-frontend/src/views/CRUD/alert/AlertReportModal.tsx index 19543005df090..41198813f13a6 100644 --- a/superset-frontend/src/views/CRUD/alert/AlertReportModal.tsx +++ b/superset-frontend/src/views/CRUD/alert/AlertReportModal.tsx @@ -940,7 +940,6 @@ const AlertReportModal: FunctionComponent = ({ (!currentAlert || currentAlert.id || (isHidden && show)) ) { setCurrentAlert({ ...DEFAULT_ALERT }); - setForceScreenshot(contentType === 'chart' && !isReport); setNotificationSettings([]); setNotificationAddState('active'); } @@ -1369,19 +1368,20 @@ const AlertReportModal: FunctionComponent = ({ )} - {isReport && ( -
- - Ignore cache when generating screenshot - -
- )} )} + {(isReport || contentType === 'dashboard') && ( +
+ + Ignore cache when generating screenshot + +
+ )}

{t('Notification method')}

* diff --git a/superset/reports/commands/execute.py b/superset/reports/commands/execute.py index 7e7eeaceaccd6..2a75d2e738ff3 100644 --- a/superset/reports/commands/execute.py +++ b/superset/reports/commands/execute.py @@ -147,7 +147,6 @@ def _get_url( Get the url for this report schedule: chart or dashboard """ force = "true" if self._report_schedule.force_screenshot else "false" - if self._report_schedule.chart: if result_format in { ChartDataResultFormat.CSV, @@ -173,7 +172,7 @@ def _get_url( user_friendly=user_friendly, dashboard_id_or_slug=self._report_schedule.dashboard_id, standalone=DashboardStandaloneMode.REPORT.value, - # force=force, TODO (betodealmeida) implement this + force=force, **kwargs, ) diff --git a/tests/integration_tests/reports/commands_tests.py b/tests/integration_tests/reports/commands_tests.py index 68effd2c7fcb6..f914941e919e9 100644 --- a/tests/integration_tests/reports/commands_tests.py +++ b/tests/integration_tests/reports/commands_tests.py @@ -290,6 +290,18 @@ def create_report_email_dashboard(): cleanup_report_schedule(report_schedule) +@pytest.fixture() +def create_report_email_dashboard_force_screenshot(): + with app.app_context(): + dashboard = db.session.query(Dashboard).first() + report_schedule = create_report_notification( + email_target="target@email.com", dashboard=dashboard, force_screenshot=True + ) + yield report_schedule + + cleanup_report_schedule(report_schedule) + + @pytest.fixture() def create_report_email_tabbed_dashboard(tabbed_dashboard): with app.app_context(): @@ -1002,6 +1014,41 @@ def test_email_dashboard_report_schedule( assert_log(ReportState.SUCCESS) +@pytest.mark.usefixtures( + "load_birth_names_dashboard_with_slices", + "create_report_email_dashboard_force_screenshot", +) +@patch("superset.reports.notifications.email.send_email_smtp") +@patch("superset.utils.screenshots.DashboardScreenshot.get_screenshot") +def test_email_dashboard_report_schedule_force_screenshot( + screenshot_mock, email_mock, create_report_email_dashboard_force_screenshot +): + """ + ExecuteReport Command: Test dashboard email report schedule + """ + # setup screenshot mock + screenshot_mock.return_value = SCREENSHOT_FILE + + with freeze_time("2020-01-01T00:00:00Z"): + AsyncExecuteReportScheduleCommand( + TEST_ID, + create_report_email_dashboard_force_screenshot.id, + datetime.utcnow(), + ).run() + + notification_targets = get_target_from_report_schedule( + create_report_email_dashboard_force_screenshot + ) + + # 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_report_slack_chart" ) @@ -1772,7 +1819,7 @@ def test_when_tabs_are_selected_it_takes_screenshots_for_every_tabs( assert dashboard_screenshot_mock.call_count == 2 for index, tab in enumerate(tabs): assert dashboard_screenshot_mock.call_args_list[index].args == ( - f"http://0.0.0.0:8080/superset/dashboard/{dashboard.id}/?standalone=3#{tab}", + f"http://0.0.0.0:8080/superset/dashboard/{dashboard.id}/?standalone=3&force=false#{tab}", f"{dashboard.digest}", ) assert send_email_smtp_mock.called is True