Skip to content

Commit

Permalink
feat: Allow users to bust cache in report dashboard + alerts charts +…
Browse files Browse the repository at this point in the history
… 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 8c52fe3)
  • Loading branch information
hughhhh authored and villebro committed Apr 3, 2022
1 parent 6178f05 commit fb5d77e
Show file tree
Hide file tree
Showing 6 changed files with 87 additions and 17 deletions.
6 changes: 4 additions & 2 deletions superset-frontend/src/chart/Chart.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down
4 changes: 4 additions & 0 deletions superset-frontend/src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,10 @@ export const URL_PARAMS = {
name: 'dataset_id',
type: 'string',
},
force: {
name: 'force',
type: 'boolean',
},
} as const;

/**
Expand Down
18 changes: 18 additions & 0 deletions superset-frontend/src/views/CRUD/alert/AlertReportModal.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
});
24 changes: 12 additions & 12 deletions superset-frontend/src/views/CRUD/alert/AlertReportModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -940,7 +940,6 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
(!currentAlert || currentAlert.id || (isHidden && show))
) {
setCurrentAlert({ ...DEFAULT_ALERT });
setForceScreenshot(contentType === 'chart' && !isReport);
setNotificationSettings([]);
setNotificationAddState('active');
}
Expand Down Expand Up @@ -1369,19 +1368,20 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
)}
</StyledRadioGroup>
</div>
{isReport && (
<div className="inline-container">
<StyledCheckbox
className="checkbox"
checked={forceScreenshot}
onChange={onForceScreenshotChange}
>
Ignore cache when generating screenshot
</StyledCheckbox>
</div>
)}
</>
)}
{(isReport || contentType === 'dashboard') && (
<div className="inline-container">
<StyledCheckbox
data-test="bypass-cache"
className="checkbox"
checked={forceScreenshot}
onChange={onForceScreenshotChange}
>
Ignore cache when generating screenshot
</StyledCheckbox>
</div>
)}
<StyledSectionTitle>
<h4>{t('Notification method')}</h4>
<span className="required">*</span>
Expand Down
3 changes: 1 addition & 2 deletions superset/reports/commands/execute.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
)

Expand Down
49 changes: 48 additions & 1 deletion tests/integration_tests/reports/commands_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -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="[email protected]", 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():
Expand Down Expand Up @@ -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"
)
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit fb5d77e

Please sign in to comment.