You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by di...@apache.org on 2024/02/19 15:28:18 UTC
(superset) branch master updated: feat(Alerts and Reports): Modal redesign (#26202)
This is an automated email from the ASF dual-hosted git repository.
diegopucci pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 601e62a2ee feat(Alerts and Reports): Modal redesign (#26202)
601e62a2ee is described below
commit 601e62a2ee7deab8fa480152e732e40ef722b3af
Author: Ross Mabbett <92...@users.noreply.github.com>
AuthorDate: Mon Feb 19 10:28:10 2024 -0500
feat(Alerts and Reports): Modal redesign (#26202)
Signed-off-by: dependabot[bot] <su...@github.com>
Co-authored-by: fisjac <jf...@gmail.com>
Co-authored-by: Corbin <co...@gmail.com>
Co-authored-by: Lily Kuang <li...@preset.io>
Co-authored-by: Michael S. Molina <70...@users.noreply.github.com>
Co-authored-by: dependabot[bot] <49...@users.noreply.github.com>
---
superset-frontend/src/GlobalStyles.tsx | 1 +
superset-frontend/src/components/Button/index.tsx | 11 +-
.../src/components/CronPicker/CronPicker.tsx | 115 +-
superset-frontend/src/components/Modal/Modal.tsx | 3 +
.../src/features/alerts/AlertReportModal.test.jsx | 367 ------
.../src/features/alerts/AlertReportModal.test.tsx | 632 +++++++++-
.../src/features/alerts/AlertReportModal.tsx | 1296 +++++++++++---------
.../alerts/buildErrorTooltipMessage.test.tsx | 70 ++
.../features/alerts/buildErrorTooltipMessage.tsx | 49 +
.../components/AlertReportCronScheduler.test.tsx | 153 ---
.../alerts/components/AlertReportCronScheduler.tsx | 96 +-
.../alerts/components/NotificationMethod.tsx | 31 +-
.../src/features/alerts/components/NumberInput.tsx | 52 +
.../src/features/alerts/components/StyledPanel.tsx | 75 ++
.../alerts/components/ValidatedPanelHeader.tsx | 53 +
superset-frontend/src/features/alerts/types.ts | 18 +
.../src/features/reports/ReportModal/index.tsx | 9 +-
superset/config.py | 2 +-
18 files changed, 1799 insertions(+), 1234 deletions(-)
diff --git a/superset-frontend/src/GlobalStyles.tsx b/superset-frontend/src/GlobalStyles.tsx
index f983c262d7..eceefaff62 100644
--- a/superset-frontend/src/GlobalStyles.tsx
+++ b/superset-frontend/src/GlobalStyles.tsx
@@ -20,6 +20,7 @@ import React from 'react';
import { css } from '@superset-ui/core';
import { Global } from '@emotion/react';
import { mix } from 'polished';
+import 'react-js-cron/dist/styles.css';
export const GlobalStyles = () => (
<Global
diff --git a/superset-frontend/src/components/Button/index.tsx b/superset-frontend/src/components/Button/index.tsx
index 1aa541a577..389cb31faf 100644
--- a/superset-frontend/src/components/Button/index.tsx
+++ b/superset-frontend/src/components/Button/index.tsx
@@ -16,8 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
-import React, { Children, ReactElement } from 'react';
-import { kebabCase } from 'lodash';
+import React, { Children, ReactElement, ReactNode } from 'react';
import { mix } from 'polished';
import cx from 'classnames';
import { AntdButton } from 'src/components';
@@ -43,7 +42,7 @@ export type ButtonSize = 'default' | 'small' | 'xsmall';
export type ButtonProps = Omit<AntdButtonProps, 'css'> &
Pick<TooltipProps, 'placement'> & {
- tooltip?: string;
+ tooltip?: ReactNode;
className?: string;
buttonSize?: ButtonSize;
buttonStyle?: ButtonStyle;
@@ -214,11 +213,7 @@ export default function Button(props: ButtonProps) {
if (tooltip) {
return (
- <Tooltip
- placement={placement}
- id={`${kebabCase(tooltip)}-tooltip`}
- title={tooltip}
- >
+ <Tooltip placement={placement} title={tooltip}>
{/* wrap the button in a span so that the tooltip shows up
when the button is disabled. */}
{disabled ? (
diff --git a/superset-frontend/src/components/CronPicker/CronPicker.tsx b/superset-frontend/src/components/CronPicker/CronPicker.tsx
index 385062e0ce..4fe6cbb1fd 100644
--- a/superset-frontend/src/components/CronPicker/CronPicker.tsx
+++ b/superset-frontend/src/components/CronPicker/CronPicker.tsx
@@ -44,7 +44,7 @@ export const LOCALE: Locale = {
prefixMonths: t('in'),
prefixMonthDays: t('on'),
prefixWeekDays: t('on'),
- prefixWeekDaysForMonthAndYearPeriod: t('and'),
+ prefixWeekDaysForMonthAndYearPeriod: t('or'),
prefixHours: t('at'),
prefixMinutes: t(':'),
prefixMinutesForHourPeriod: t('at'),
@@ -110,22 +110,99 @@ export const CronPicker = styled((props: CronProps) => (
<ReactCronPicker locale={LOCALE} {...props} />
</ConfigProvider>
))`
- .react-js-cron-field {
- margin-bottom: 0px;
- }
- .react-js-cron-select:not(.react-js-cron-custom-select) > div:first-of-type,
- .react-js-cron-custom-select {
- border-radius: ${({ theme }) => theme.gridUnit}px;
- background-color: ${({ theme }) =>
- theme.colors.grayscale.light4} !important;
- }
- .react-js-cron-custom-select > div:first-of-type {
- border-radius: ${({ theme }) => theme.gridUnit}px;
- }
- .react-js-cron-custom-select .ant-select-selection-placeholder {
- flex: auto;
- }
- .react-js-cron-custom-select .ant-select-selection-overflow-item {
- align-self: center;
- }
+ ${({ theme }) => `
+
+ /* Boilerplate styling for ReactCronPicker imported explicitly in GlobalStyles.tsx */
+
+ /* When year period is selected */
+
+ :has(.react-js-cron-months) {
+ display: grid !important;
+ grid-template-columns: repeat(2, 50%);
+ column-gap: ${theme.gridUnit}px;
+ row-gap: ${theme.gridUnit * 2}px;
+ div:has(.react-js-cron-hours) {
+ grid-column: span 2;
+ display: flex;
+ justify-content: space-between;
+ .react-js-cron-field {
+ width: 50%;
+ }
+ }
+ }
+
+ /* When month period is selected */
+
+ :not(:has(.react-js-cron-months)) {
+ display: grid;
+ grid-template-columns: repeat(2, 50%);
+ column-gap: ${theme.gridUnit}px;
+ row-gap: ${theme.gridUnit * 2}px;
+ .react-js-cron-period {
+ grid-column: span 2;
+ }
+ div:has(.react-js-cron-hours) {
+ grid-column: span 2;
+ display: flex;
+ justify-content: space-between;
+ .react-js-cron-field {
+ width: 50%;
+ }
+ }
+ }
+
+ /* When week period is selected */
+
+ :not(:has(.react-js-cron-month-days)) {
+ .react-js-cron-week-days {
+ grid-column: span 2;
+ }
+ }
+
+ /* For proper alignment of inputs and span elements */
+
+ :not(div:has(.react-js-cron-hours)) {
+ display: flex;
+ flex-wrap: nowrap;
+ }
+
+ div:has(.react-js-cron-hours) {
+ width: 100%;
+ }
+
+ .react-js-cron-minutes > span {
+ padding-left: ${theme.gridUnit}px;
+ }
+
+ /* Sizing of select container */
+
+ .react-js-cron-select.ant-select {
+ width: 100%;
+ .ant-select-selector {
+ flex-wrap: nowrap;
+ }
+ }
+
+ .react-js-cron-field {
+ width: 100%;
+ margin-bottom: 0px;
+ > span {
+ margin-left: 0px;
+ }
+ }
+
+ .react-js-cron-custom-select .ant-select-selection-placeholder {
+ flex: auto;
+ border-radius: ${theme.gridUnit}px;
+ }
+
+ .react-js-cron-custom-select .ant-select-selection-overflow-item {
+ align-self: center;
+ }
+
+ .react-js-cron-select > div:first-of-type,
+ .react-js-cron-custom-select {
+ border-radius: ${theme.gridUnit}px;
+ }
+ `}
`;
diff --git a/superset-frontend/src/components/Modal/Modal.tsx b/superset-frontend/src/components/Modal/Modal.tsx
index 331f34c1d2..831e8ab77a 100644
--- a/superset-frontend/src/components/Modal/Modal.tsx
+++ b/superset-frontend/src/components/Modal/Modal.tsx
@@ -41,6 +41,7 @@ export interface ModalProps {
className?: string;
children: ReactNode;
disablePrimaryButton?: boolean;
+ primaryTooltipMessage?: ReactNode;
primaryButtonLoading?: boolean;
onHide: () => void;
onHandledPrimaryAction?: () => void;
@@ -232,6 +233,7 @@ const defaultResizableConfig = (hideFooter: boolean | undefined) => ({
const CustomModal = ({
children,
disablePrimaryButton = false,
+ primaryTooltipMessage,
primaryButtonLoading = false,
onHide,
onHandledPrimaryAction,
@@ -274,6 +276,7 @@ const CustomModal = ({
key="submit"
buttonStyle={primaryButtonType}
disabled={disablePrimaryButton}
+ tooltip={primaryTooltipMessage}
loading={primaryButtonLoading}
onClick={onHandledPrimaryAction}
cta
diff --git a/superset-frontend/src/features/alerts/AlertReportModal.test.jsx b/superset-frontend/src/features/alerts/AlertReportModal.test.jsx
deleted file mode 100644
index a91e789920..0000000000
--- a/superset-frontend/src/features/alerts/AlertReportModal.test.jsx
+++ /dev/null
@@ -1,367 +0,0 @@
-/**
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements. See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership. The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied. See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-import React from 'react';
-import { Provider } from 'react-redux';
-import thunk from 'redux-thunk';
-import configureStore from 'redux-mock-store';
-import fetchMock from 'fetch-mock';
-import { act } from 'react-dom/test-utils';
-import Modal from 'src/components/Modal';
-import { Select, AsyncSelect } from 'src/components';
-import { Switch } from 'src/components/Switch';
-import { Radio } from 'src/components/Radio';
-import TextAreaControl from 'src/explore/components/controls/TextAreaControl';
-import waitForComponentToPaint from 'spec/helpers/waitForComponentToPaint';
-import { styledMount as mount } from 'spec/helpers/theming';
-import AlertReportModal from './AlertReportModal';
-
-const mockData = {
- active: true,
- id: 1,
- name: 'test report',
- description: 'test report description',
- chart: { id: 1, slice_name: 'test chart', viz_type: 'table' },
- database: { id: 1, database_name: 'test database' },
- sql: 'SELECT NaN',
-};
-const FETCH_REPORT_ENDPOINT = 'glob:*/api/v1/report/*';
-const REPORT_PAYLOAD = { result: mockData };
-
-fetchMock.get(FETCH_REPORT_ENDPOINT, REPORT_PAYLOAD);
-
-const mockStore = configureStore([thunk]);
-const store = mockStore({});
-
-// Report mock is default for testing
-const mockedProps = {
- addDangerToast: () => {},
- onAdd: jest.fn(() => []),
- onHide: () => {},
- show: true,
- isReport: true,
-};
-
-// Related mocks
-const ownersEndpoint = 'glob:*/api/v1/alert/related/owners?*';
-const databaseEndpoint = 'glob:*/api/v1/alert/related/database?*';
-const dashboardEndpoint = 'glob:*/api/v1/alert/related/dashboard?*';
-const chartEndpoint = 'glob:*/api/v1/alert/related/chart?*';
-
-fetchMock.get(ownersEndpoint, {
- result: [],
-});
-
-fetchMock.get(databaseEndpoint, {
- result: [],
-});
-
-fetchMock.get(dashboardEndpoint, {
- result: [],
-});
-
-fetchMock.get(chartEndpoint, {
- result: [{ text: 'table chart', value: 1 }],
-});
-
-async function mountAndWait(props = mockedProps) {
- const mounted = mount(
- <Provider store={store}>
- <AlertReportModal show {...props} />
- </Provider>,
- {
- context: { store },
- },
- );
-
- await waitForComponentToPaint(mounted);
-
- return mounted;
-}
-
-describe('AlertReportModal', () => {
- let wrapper;
-
- beforeAll(async () => {
- wrapper = await mountAndWait();
- });
-
- it('renders', () => {
- expect(wrapper.find(AlertReportModal)).toExist();
- });
-
- it('renders a Modal', () => {
- expect(wrapper.find(Modal)).toExist();
- });
-
- it('render a empty modal', () => {
- expect(wrapper.find('input[name="name"]').text()).toEqual('');
- expect(wrapper.find('input[name="description"]').text()).toEqual('');
- });
-
- it('renders add header for report when no alert is included, and isReport is true', async () => {
- const addWrapper = await mountAndWait();
-
- expect(
- addWrapper.find('[data-test="alert-report-modal-title"]').text(),
- ).toEqual('Add Report');
- });
-
- it('renders add header for alert when no alert is included, and isReport is false', async () => {
- const props = {
- ...mockedProps,
- isReport: false,
- };
-
- const addWrapper = await mountAndWait(props);
-
- expect(
- addWrapper.find('[data-test="alert-report-modal-title"]').text(),
- ).toEqual('Add Alert');
- });
-
- it('renders edit modal', async () => {
- const props = {
- ...mockedProps,
- alert: mockData,
- };
-
- const editWrapper = await mountAndWait(props);
- expect(
- editWrapper.find('[data-test="alert-report-modal-title"]').text(),
- ).toEqual('Edit Report');
- expect(editWrapper.find('input[name="name"]').props().value).toEqual(
- 'test report',
- );
- expect(editWrapper.find('input[name="description"]').props().value).toEqual(
- 'test report description',
- );
- });
-
- it('renders async select with value in alert edit modal', async () => {
- const props = {
- ...mockedProps,
- alert: mockData,
- isReport: false,
- };
-
- const editWrapper = await mountAndWait(props);
- expect(
- editWrapper.find('[aria-label="Database"]').at(0).props().value,
- ).toEqual({
- value: 1,
- label: 'test database',
- });
- expect(
- editWrapper.find('[aria-label="Chart"]').at(0).props().value,
- ).toEqual({
- value: 1,
- label: 'test chart',
- });
- });
-
- // Fields
- it('renders input element for name', () => {
- expect(wrapper.find('input[name="name"]')).toExist();
- });
-
- it('renders four select elements when in report mode', () => {
- expect(wrapper.find(Select)).toExist();
- expect(wrapper.find(AsyncSelect)).toExist();
- expect(wrapper.find(Select)).toHaveLength(2);
- expect(wrapper.find(AsyncSelect)).toHaveLength(2);
- });
-
- it('renders Switch element', () => {
- expect(wrapper.find(Switch)).toExist();
- });
-
- it('renders input element for description', () => {
- expect(wrapper.find('input[name="description"]')).toExist();
- });
-
- it('renders input element for sql in alert mode only', async () => {
- const props = {
- ...mockedProps,
- isReport: false,
- };
-
- const addWrapper = await mountAndWait(props);
-
- expect(wrapper.find(TextAreaControl)).toHaveLength(0);
- expect(addWrapper.find(TextAreaControl)).toExist();
- });
-
- it('renders input element for sql with NaN', async () => {
- const props = {
- ...mockedProps,
- alert: mockData,
- isReport: false,
- };
-
- const editWrapper = await mountAndWait(props);
- const input = editWrapper.find(TextAreaControl);
- expect(input).toExist();
- expect(input.props().initialValue).toEqual('SELECT NaN');
- });
-
- it('renders four select element when in report mode', () => {
- expect(wrapper.find(Select)).toExist();
- expect(wrapper.find(AsyncSelect)).toExist();
- expect(wrapper.find(Select)).toHaveLength(2);
- expect(wrapper.find(AsyncSelect)).toHaveLength(2);
- });
-
- it('renders six select elements when in alert mode', async () => {
- const props = {
- ...mockedProps,
- isReport: false,
- };
-
- const addWrapper = await mountAndWait(props);
-
- expect(addWrapper.find(Select)).toExist();
- expect(addWrapper.find(AsyncSelect)).toExist();
- expect(addWrapper.find(Select)).toHaveLength(3);
- expect(addWrapper.find(AsyncSelect)).toHaveLength(3);
- });
-
- it('renders value input element when in alert mode', async () => {
- const props = {
- ...mockedProps,
- isReport: false,
- };
-
- const addWrapper = await mountAndWait(props);
-
- expect(wrapper.find('input[name="threshold"]')).toHaveLength(0);
- expect(addWrapper.find('input[name="threshold"]')).toExist();
- });
-
- it('renders two radio buttons', () => {
- expect(wrapper.find(Radio)).toExist();
- expect(wrapper.find(Radio)).toHaveLength(2);
- });
-
- it('renders text option for text-based charts', async () => {
- const props = {
- ...mockedProps,
- alert: mockData,
- };
- const textWrapper = await mountAndWait(props);
-
- const chartOption = textWrapper.find('input[value="chart"]');
- act(() => {
- chartOption.props().onChange({ target: { value: 'chart' } });
- });
- await waitForComponentToPaint(textWrapper);
-
- expect(textWrapper.find('input[value="TEXT"]')).toExist();
- });
-
- it('renders input element for working timeout', () => {
- expect(wrapper.find('input[name="working_timeout"]')).toExist();
- });
-
- it('renders input element for grace period for alert only', async () => {
- const props = {
- ...mockedProps,
- isReport: false,
- };
-
- const addWrapper = await mountAndWait(props);
-
- expect(addWrapper.find('input[name="grace_period"]')).toExist();
- expect(wrapper.find('input[name="grace_period"]')).toHaveLength(0);
- });
-
- it('only allows grace period values > 1', async () => {
- const props = {
- ...mockedProps,
- isReport: false,
- };
-
- const addWrapper = await mountAndWait(props);
-
- const input = addWrapper.find('input[name="grace_period"]');
-
- input.simulate('change', { target: { name: 'grace_period', value: 7 } });
- expect(input.instance().value).toEqual('7');
-
- input.simulate('change', { target: { name: 'grace_period', value: 0 } });
- expect(input.instance().value).toEqual('');
-
- input.simulate('change', { target: { name: 'grace_period', value: -1 } });
- expect(input.instance().value).toEqual('1');
- });
-
- it('only allows working timeout values > 1', () => {
- const input = wrapper.find('input[name="working_timeout"]');
-
- input.simulate('change', { target: { name: 'working_timeout', value: 7 } });
- expect(input.instance().value).toEqual('7');
-
- input.simulate('change', { target: { name: 'working_timeout', value: 0 } });
- expect(input.instance().value).toEqual('');
-
- input.simulate('change', {
- target: { name: 'working_timeout', value: -1 },
- });
- expect(input.instance().value).toEqual('1');
- });
-
- it('allows to add notification method', async () => {
- const button = wrapper.find('[data-test="notification-add"]');
- act(() => {
- button.props().onClick();
- });
- await waitForComponentToPaint(wrapper);
-
- // use default config: only show Email as notification option.
- expect(
- wrapper.find('[data-test="notification-add"]').props().status,
- ).toEqual('hidden');
- act(() => {
- wrapper
- .find('[data-test="select-delivery-method"]')
- .last()
- .props()
- .onSelect('Email');
- });
- 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/features/alerts/AlertReportModal.test.tsx b/superset-frontend/src/features/alerts/AlertReportModal.test.tsx
index e4f7454fec..ee9504286d 100644
--- a/superset-frontend/src/features/alerts/AlertReportModal.test.tsx
+++ b/superset-frontend/src/features/alerts/AlertReportModal.test.tsx
@@ -17,68 +17,606 @@
* under the License.
*/
import React from 'react';
-import { render, screen, waitFor } from 'spec/helpers/testing-library';
import userEvent from '@testing-library/user-event';
-import AlertReportModal from './AlertReportModal';
+import fetchMock from 'fetch-mock';
+import { render, screen, waitFor, within } from 'spec/helpers/testing-library';
+import { buildErrorTooltipMessage } from './buildErrorTooltipMessage';
+import AlertReportModal, { AlertReportModalProps } from './AlertReportModal';
+import { AlertObject } from './types';
-jest.mock('src/components/AsyncAceEditor', () => ({
- ...jest.requireActual('src/components/AsyncAceEditor'),
- TextAreaEditor: () => <div data-test="react-ace" />,
+jest.mock('@superset-ui/core', () => ({
+ ...jest.requireActual('@superset-ui/core'),
+ isFeatureEnabled: () => true,
}));
-const onHide = jest.fn();
-
-test('allows change to None in log retention', async () => {
- render(<AlertReportModal show onHide={onHide} />, { useRedux: true });
- // open the log retention select
- userEvent.click(screen.getByText('90 days'));
- // change it to 30 days
- userEvent.click(await screen.findByText('30 days'));
- // open again
- userEvent.click(screen.getAllByText('30 days')[0]);
- // change it to None
- userEvent.click(await screen.findByText('None'));
- // get the selected item
- const selectedItem = await waitFor(() =>
- screen
- .getAllByLabelText('Log retention')[0]
- .querySelector('.ant-select-selection-item'),
+jest.mock('src/features/databases/state.ts', () => ({
+ useCommonConf: () => ({
+ ALERT_REPORTS_NOTIFICATION_METHODS: ['Email', 'Slack'],
+ }),
+}));
+
+const generateMockPayload = (dashboard = true) => {
+ const mockPayload = {
+ active: false,
+ context_markdown: 'string',
+ creation_method: 'alerts_reports',
+ crontab: '0 0 * * *',
+ custom_width: null,
+ database: {
+ database_name: 'examples',
+ id: 1,
+ },
+ description: 'Some description',
+ extra: {},
+ force_screenshot: true,
+ grace_period: 14400,
+ id: 1,
+ last_eval_dttm: null,
+ last_state: 'Not triggered',
+ last_value: null,
+ last_value_row_json: null,
+ log_retention: 90,
+ name: 'Test Alert',
+ owners: [
+ {
+ first_name: 'Superset',
+ id: 1,
+ last_name: 'Admin',
+ },
+ ],
+ recipients: [
+ {
+ id: 1,
+ recipient_config_json: '{"target": "test@user.com"}',
+ type: 'Email',
+ },
+ ],
+ report_format: 'PNG',
+ sql: 'Select * From DB',
+ timezone: 'America/Rainy_River',
+ type: 'Alert',
+ validator_config_json: '{"threshold": 10.0, "op": "<"}',
+ validator_type: 'operator',
+ working_timeout: 3600,
+ };
+ if (dashboard) {
+ return {
+ ...mockPayload,
+ dashboard: { id: 1, dashboard_title: 'Test Dashboard' },
+ };
+ }
+ return {
+ ...mockPayload,
+ chart: {
+ id: 1,
+ slice_name: 'Test Chart',
+ viz_type: 'table',
+ },
+ };
+};
+
+// mocking resource endpoints
+const FETCH_DASHBOARD_ENDPOINT = 'glob:*/api/v1/report/1';
+const FETCH_CHART_ENDPOINT = 'glob:*/api/v1/report/2';
+
+fetchMock.get(FETCH_DASHBOARD_ENDPOINT, { result: generateMockPayload(true) });
+fetchMock.get(FETCH_CHART_ENDPOINT, { result: generateMockPayload(false) });
+
+// Related mocks
+const ownersEndpoint = 'glob:*/api/v1/alert/related/owners?*';
+const databaseEndpoint = 'glob:*/api/v1/alert/related/database?*';
+const dashboardEndpoint = 'glob:*/api/v1/alert/related/dashboard?*';
+const chartEndpoint = 'glob:*/api/v1/alert/related/chart?*';
+
+fetchMock.get(ownersEndpoint, { result: [] });
+fetchMock.get(databaseEndpoint, { result: [] });
+fetchMock.get(dashboardEndpoint, { result: [] });
+fetchMock.get(chartEndpoint, { result: [{ text: 'table chart', value: 1 }] });
+
+// Create a valid alert with all required fields entered for validation check
+
+// @ts-ignore will add id in factory function
+const validAlert: AlertObject = {
+ active: false,
+ changed_on_delta_humanized: 'now',
+ created_on: '2023-12-12T22:33:25.927764',
+ creation_method: 'alerts_reports',
+ crontab: '0 0 * * *',
+ dashboard_id: 0,
+ chart_id: 0,
+ force_screenshot: false,
+ last_state: 'Not triggered',
+ name: 'Test Alert',
+ owners: [
+ {
+ first_name: 'Superset',
+ id: 1,
+ last_name: 'Admin',
+ },
+ ],
+ recipients: [
+ {
+ type: 'Email',
+ recipient_config_json: { target: 'test@user.com' },
+ },
+ ],
+ timezone: 'America/Rainy_River',
+ type: 'Alert',
+};
+
+jest.mock('./buildErrorTooltipMessage', () => ({
+ buildErrorTooltipMessage: jest.fn(),
+}));
+
+const generateMockedProps = (
+ isReport = false,
+ useValidAlert = false,
+ useDashboard = true,
+): AlertReportModalProps => {
+ let alert;
+ // switching ids for endpoint when testing dashboard vs chart edits
+ if (useDashboard) {
+ alert = { ...validAlert, id: 1 };
+ } else {
+ alert = { ...validAlert, id: 2 };
+ }
+
+ return {
+ addDangerToast: () => {},
+ addSuccessToast: () => {},
+ onAdd: jest.fn(() => []),
+ onHide: jest.fn(),
+ alert: useValidAlert ? alert : null,
+ show: true,
+ isReport,
+ };
+};
+
+// combobox selector for mocking user input
+const comboboxSelect = async (
+ element: HTMLElement,
+ value: string,
+ newElementQuery: Function,
+) => {
+ expect(element).toBeInTheDocument();
+ userEvent.type(element, `${value}{enter}`);
+ await waitFor(() => {
+ const element = newElementQuery();
+ expect(element).toBeInTheDocument();
+ });
+};
+
+// --------------- TEST SECTION ------------------
+test('properly renders add alert text', () => {
+ const addAlertProps = generateMockedProps();
+ render(<AlertReportModal {...addAlertProps} />, { useRedux: true });
+ const addAlertHeading = screen.getByRole('heading', { name: /add alert/i });
+ expect(addAlertHeading).toBeInTheDocument();
+ const addButton = screen.getByRole('button', { name: /add/i });
+ expect(addButton).toBeInTheDocument();
+});
+
+test('properly renders edit alert text', async () => {
+ render(<AlertReportModal {...generateMockedProps(false, true)} />, {
+ useRedux: true,
+ });
+ const editAlertHeading = screen.getByRole('heading', {
+ name: /edit alert/i,
+ });
+ expect(editAlertHeading).toBeInTheDocument();
+ const saveButton = screen.getByRole('button', { name: /save/i });
+ expect(saveButton).toBeInTheDocument();
+});
+
+test('properly renders add report text', () => {
+ render(<AlertReportModal {...generateMockedProps(true)} />, {
+ useRedux: true,
+ });
+ const addReportHeading = screen.getByRole('heading', {
+ name: /add report/i,
+ });
+ expect(addReportHeading).toBeInTheDocument();
+ const addButton = screen.getByRole('button', { name: /add/i });
+ expect(addButton).toBeInTheDocument();
+});
+
+test('properly renders edit report text', async () => {
+ render(<AlertReportModal {...generateMockedProps(true, true)} />, {
+ useRedux: true,
+ });
+
+ const editReportHeading = screen.getByRole('heading', {
+ name: /edit report/i,
+ });
+ expect(editReportHeading).toBeInTheDocument();
+ const saveButton = screen.getByRole('button', { name: /save/i });
+ expect(saveButton).toBeInTheDocument();
+});
+
+test('renders 4 sections for reports', () => {
+ render(<AlertReportModal {...generateMockedProps(true)} />, {
+ useRedux: true,
+ });
+ const sections = screen.getAllByRole('tab');
+ expect(sections.length).toBe(4);
+});
+
+test('renders 5 sections for alerts', () => {
+ render(<AlertReportModal {...generateMockedProps(false)} />, {
+ useRedux: true,
+ });
+
+ const sections = screen.getAllByRole('tab');
+ expect(sections.length).toBe(5);
+});
+
+// Validation
+test('renders 5 checkmarks for a valid alert', async () => {
+ render(<AlertReportModal {...generateMockedProps(false, true, false)} />, {
+ useRedux: true,
+ });
+ const checkmarks = await screen.findAllByRole('img', {
+ name: /check-circle/i,
+ });
+ expect(checkmarks.length).toEqual(5);
+});
+
+test('renders single checkmarks when creating a new alert', async () => {
+ render(<AlertReportModal {...generateMockedProps(false, false, false)} />, {
+ useRedux: true,
+ });
+ const checkmarks = await screen.findAllByRole('img', {
+ name: /check-circle/i,
+ });
+ expect(checkmarks.length).toEqual(1);
+});
+
+test('disables save when validation fails', () => {
+ render(<AlertReportModal {...generateMockedProps(false, false, false)} />, {
+ useRedux: true,
+ });
+
+ expect(screen.getByRole('button', { name: /add/i })).toBeDisabled();
+});
+
+test('calls build tooltip', async () => {
+ render(<AlertReportModal {...generateMockedProps(false, false, false)} />, {
+ useRedux: true,
+ });
+ expect(buildErrorTooltipMessage).toHaveBeenCalled();
+ expect(buildErrorTooltipMessage).toHaveBeenLastCalledWith({
+ alertConditionSection: {
+ errors: ['database', 'sql', 'alert condition'],
+ name: 'Alert condition',
+ hasErrors: true,
+ },
+ contentSection: {
+ errors: ['content type'],
+ name: 'Alert contents',
+ hasErrors: true,
+ },
+ generalSection: {
+ errors: ['name'],
+ name: 'General information',
+ hasErrors: true,
+ },
+ notificationSection: {
+ errors: ['recipients'],
+ name: 'Notification method',
+ hasErrors: true,
+ },
+ scheduleSection: { errors: [], name: 'Schedule', hasErrors: false },
+ });
+});
+
+// General Section
+test('opens General Section on render', async () => {
+ render(<AlertReportModal {...generateMockedProps(false, true, false)} />, {
+ useRedux: true,
+ });
+ const general_header = within(
+ screen.getByRole('tab', { expanded: true }),
+ ).queryByText(/general information/i);
+ expect(general_header).toBeInTheDocument();
+});
+
+test('renders all fields in General Section', () => {
+ render(<AlertReportModal {...generateMockedProps(false, true, false)} />, {
+ useRedux: true,
+ });
+ const name = screen.getByPlaceholderText(/enter alert name/i);
+ const owners = screen.getByTestId('owners-select');
+ const description = screen.getByPlaceholderText(
+ /include description to be sent with alert/i,
+ );
+ const activeSwitch = screen.getByRole('switch');
+
+ expect(name).toBeInTheDocument();
+ expect(owners).toBeInTheDocument();
+ expect(description).toBeInTheDocument();
+ expect(activeSwitch).toBeInTheDocument();
+});
+
+// Alert Condition Section
+/* A Note on textbox total numbers:
+ Because the General Info panel is open by default, the Name and Description textboxes register as being in the document on all tests, thus the total number of textboxes in each subsequent panel's tests will always be n+2. This is most significant in the Alert Condition panel tests because the nature of the SQL field as a TextAreaContol component may only be queried by role */
+test('opens Alert Condition Section on click', async () => {
+ render(<AlertReportModal {...generateMockedProps(false, true, false)} />, {
+ useRedux: true,
+ });
+ userEvent.click(screen.getByTestId('alert-condition-panel'));
+ const alertConditionHeader = within(
+ screen.getByRole('tab', { expanded: true }),
+ ).queryByText(/alert condition/i);
+ expect(alertConditionHeader).toBeInTheDocument();
+});
+test('renders all Alert Condition fields', async () => {
+ render(<AlertReportModal {...generateMockedProps(false, true, false)} />, {
+ useRedux: true,
+ });
+ userEvent.click(screen.getByTestId('alert-condition-panel'));
+ const database = screen.getByRole('combobox', { name: /database/i });
+ const sql = screen.getAllByRole('textbox')[2];
+ const condition = screen.getByRole('combobox', { name: /condition/i });
+ const threshold = screen.getByRole('spinbutton');
+ expect(database).toBeInTheDocument();
+ expect(sql).toBeInTheDocument();
+ expect(condition).toBeInTheDocument();
+ expect(threshold).toBeInTheDocument();
+});
+test('disables condition threshold if not null condition is selected', async () => {
+ render(<AlertReportModal {...generateMockedProps(false, true, false)} />, {
+ useRedux: true,
+ });
+ userEvent.click(screen.getByTestId('alert-condition-panel'));
+ await screen.findByText(/smaller than/i);
+ const condition = screen.getByRole('combobox', { name: /condition/i });
+ await comboboxSelect(
+ condition,
+ 'not null',
+ () => screen.getAllByText(/not null/i)[0],
);
- // check if None is selected
- expect(selectedItem).toHaveTextContent('None');
+ expect(screen.getByRole('spinbutton')).toBeDisabled();
});
-test('renders the appropriate dropdown in Message Content section', async () => {
- render(<AlertReportModal show onHide={onHide} />, { useRedux: true });
+// Content Section
+test('opens Contents Section on click', async () => {
+ render(<AlertReportModal {...generateMockedProps(false, true, false)} />, {
+ useRedux: true,
+ });
+ userEvent.click(screen.getByTestId('contents-panel'));
+ const contentsHeader = within(
+ screen.getByRole('tab', { expanded: true }),
+ ).queryByText(/contents/i);
+ expect(contentsHeader).toBeInTheDocument();
+});
+
+test('renders screenshot options 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('combobox', { name: /select content type/i }),
+ ).toBeInTheDocument();
+ expect(
+ screen.getByRole('combobox', { name: /dashboard/i }),
+ ).toBeInTheDocument();
+ expect(screen.getByRole('spinbutton')).toBeInTheDocument();
+ expect(
+ screen.getByRole('checkbox', {
+ name: /ignore cache when generating report/i,
+ }),
+ ).toBeInTheDocument();
+});
- const chartRadio = screen.getByRole('radio', { name: /chart/i });
+test('changes to content options 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.getByRole('combobox', { name: /chart/i }),
+ );
+ expect(
+ screen.getByRole('combobox', {
+ name: /select format/i,
+ }),
+ ).toBeInTheDocument();
+});
- // Dashboard is initially checked by default
+test('removes ignore cache 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);
expect(
- await screen.findByRole('radio', {
- name: /dashboard/i,
+ screen.getByRole('checkbox', {
+ name: /ignore cache when generating report/i,
}),
- ).toBeChecked();
- expect(chartRadio).not.toBeChecked();
- // Only the dashboard dropdown should show
- expect(screen.getByRole('combobox', { name: /dashboard/i })).toBeVisible();
+ ).toBeInTheDocument();
+ const contentTypeSelector = screen.getByRole('combobox', {
+ name: /select content type/i,
+ });
+ await comboboxSelect(
+ contentTypeSelector,
+ 'Chart',
+ () => screen.getAllByText(/select chart/i)[0],
+ );
expect(
- screen.queryByRole('combobox', { name: /chart/i }),
- ).not.toBeInTheDocument();
+ screen.queryByRole('checkbox', {
+ name: /ignore cache when generating report/i,
+ }),
+ ).toBe(null);
+});
+
+test('does not show screenshot width when csv is selected', async () => {
+ render(<AlertReportModal {...generateMockedProps(false, true, false)} />, {
+ useRedux: true,
+ });
+ userEvent.click(screen.getByTestId('contents-panel'));
+ await screen.findByText(/test chart/i);
+ const contentTypeSelector = screen.getByRole('combobox', {
+ name: /select content type/i,
+ });
+ await comboboxSelect(contentTypeSelector, 'Chart', () =>
+ screen.getByText(/select chart/i),
+ );
+ const reportFormatSelector = screen.getByRole('combobox', {
+ name: /select format/i,
+ });
+ await comboboxSelect(
+ reportFormatSelector,
+ 'CSV',
+ () => screen.getAllByText(/Send as CSV/i)[0],
+ );
+ expect(screen.queryByRole('spinbutton')).not.toBeInTheDocument();
+});
- // Click the chart radio option
- userEvent.click(chartRadio);
+// Schedule Section
+test('opens Schedule Section on click', async () => {
+ render(<AlertReportModal {...generateMockedProps(false, true, false)} />, {
+ useRedux: true,
+ });
+ userEvent.click(screen.getByTestId('schedule-panel'));
+ const scheduleHeader = within(
+ screen.getByRole('tab', { expanded: true }),
+ ).queryAllByText(/schedule/i)[0];
+ expect(scheduleHeader).toBeInTheDocument();
+});
+test('renders default Schedule fields', async () => {
+ render(<AlertReportModal {...generateMockedProps(false, false, false)} />, {
+ useRedux: true,
+ });
+ userEvent.click(screen.getByTestId('schedule-panel'));
+ const scheduleType = screen.getByRole('combobox', {
+ name: /schedule type/i,
+ });
+ const timezone = screen.getByRole('combobox', {
+ name: /timezone selector/i,
+ });
+ const logRetention = screen.getByRole('combobox', {
+ name: /log retention/i,
+ });
+ const gracePeriod = screen.getByPlaceholderText(/time in seconds/i);
+ expect(scheduleType).toBeInTheDocument();
+ expect(timezone).toBeInTheDocument();
+ expect(logRetention).toBeInTheDocument();
+ expect(gracePeriod).toBeInTheDocument();
+});
+test('renders working timout as report', async () => {
+ render(<AlertReportModal {...generateMockedProps(true, false, false)} />, {
+ useRedux: true,
+ });
+ userEvent.click(screen.getByTestId('schedule-panel'));
+ expect(screen.getByText(/working timeout/i)).toBeInTheDocument();
+});
+test('renders grace period as alert', async () => {
+ render(<AlertReportModal {...generateMockedProps(false, false, false)} />, {
+ useRedux: true,
+ });
+ userEvent.click(screen.getByTestId('schedule-panel'));
+ expect(screen.getByText(/grace period/i)).toBeInTheDocument();
+});
+test('shows CRON Expression when CRON is selected', async () => {
+ render(<AlertReportModal {...generateMockedProps(true, false, false)} />, {
+ useRedux: true,
+ });
+ userEvent.click(screen.getByTestId('schedule-panel'));
+ await comboboxSelect(
+ screen.getByRole('combobox', { name: /schedule type/i }),
+ 'cron schedule',
+ () => screen.getByPlaceholderText(/cron expression/i),
+ );
+ expect(screen.getByPlaceholderText(/cron expression/i)).toBeInTheDocument();
+});
+test('defaults to day when CRON is not selected', async () => {
+ render(<AlertReportModal {...generateMockedProps(true, false, false)} />, {
+ useRedux: true,
+ });
+ userEvent.click(screen.getByTestId('schedule-panel'));
+ const days = screen.getAllByTitle(/day/i, { exact: true });
+ expect(days.length).toBe(2);
+});
- await waitFor(() => expect(chartRadio).toBeChecked());
+// Notification Method Section
+test('opens Notification Method Section on click', async () => {
+ render(<AlertReportModal {...generateMockedProps(false, false, false)} />, {
+ useRedux: true,
+ });
+ userEvent.click(screen.getByTestId('notification-method-panel'));
+ const notificationMethodHeader = within(
+ screen.getByRole('tab', { expanded: true }),
+ ).queryAllByText(/notification method/i)[0];
+ expect(notificationMethodHeader).toBeInTheDocument();
+});
+test('renders all notification fields', async () => {
+ render(<AlertReportModal {...generateMockedProps(false, false, false)} />, {
+ useRedux: true,
+ });
+ userEvent.click(screen.getByTestId('notification-method-panel'));
+ const notificationMethod = screen.getByRole('combobox', {
+ name: /delivery method/i,
+ });
+ const recipients = screen.getByTestId('recipients');
+ const addNotificationMethod = screen.getByText(
+ /add another notification method/i,
+ );
+ expect(notificationMethod).toBeInTheDocument();
+ expect(recipients).toBeInTheDocument();
+ expect(addNotificationMethod).toBeInTheDocument();
+});
+test('adds another notification method section after clicking add notification method', async () => {
+ render(<AlertReportModal {...generateMockedProps(false, false, false)} />, {
+ useRedux: true,
+ });
+ userEvent.click(screen.getByTestId('notification-method-panel'));
+ const addNotificationMethod = screen.getByText(
+ /add another notification method/i,
+ );
+ userEvent.click(addNotificationMethod);
expect(
- await screen.findByRole('radio', {
- name: /dashboard/i,
- }),
- ).not.toBeChecked();
- // Now that chart is checked, only the chart dropdown should show
- expect(screen.getByRole('combobox', { name: /chart/i })).toBeVisible();
+ screen.getAllByRole('combobox', {
+ name: /delivery method/i,
+ }).length,
+ ).toBe(2);
+ await comboboxSelect(
+ screen.getAllByRole('combobox', {
+ name: /delivery method/i,
+ })[1],
+ 'Slack',
+ () => screen.getAllByRole('textbox')[1],
+ );
+ expect(screen.getAllByTestId('recipients').length).toBe(2);
+});
+
+test('removes notification method on clicking trash can', async () => {
+ render(<AlertReportModal {...generateMockedProps(false, false, false)} />, {
+ useRedux: true,
+ });
+ userEvent.click(screen.getByTestId('notification-method-panel'));
+ const addNotificationMethod = screen.getByText(
+ /add another notification method/i,
+ );
+ userEvent.click(addNotificationMethod);
+ await comboboxSelect(
+ screen.getAllByRole('combobox', {
+ name: /delivery method/i,
+ })[1],
+ 'Email',
+ () => screen.getAllByRole('textbox')[1],
+ );
+ const images = screen.getAllByRole('img');
+ const trash = images[images.length - 1];
+ userEvent.click(trash);
expect(
- screen.queryByRole('combobox', { name: /dashboard/i }),
- ).not.toBeInTheDocument();
+ screen.getAllByRole('combobox', { name: /delivery method/i }).length,
+ ).toBe(1);
});
diff --git a/superset-frontend/src/features/alerts/AlertReportModal.tsx b/superset-frontend/src/features/alerts/AlertReportModal.tsx
index 490755fa13..c9123869f2 100644
--- a/superset-frontend/src/features/alerts/AlertReportModal.tsx
+++ b/superset-frontend/src/features/alerts/AlertReportModal.tsx
@@ -22,6 +22,7 @@ import React, {
useEffect,
useMemo,
useCallback,
+ ReactNode,
} from 'react';
import {
css,
@@ -35,19 +36,17 @@ import {
import rison from 'rison';
import { useSingleViewResource } from 'src/views/CRUD/hooks';
-import Icons from 'src/components/Icons';
import { Input } from 'src/components/Input';
import { Switch } from 'src/components/Switch';
import Modal from 'src/components/Modal';
+import Collapse from 'src/components/Collapse';
import TimezoneSelector from 'src/components/TimezoneSelector';
-import { Radio } from 'src/components/Radio';
import { propertyComparator } from 'src/components/Select/utils';
import withToasts from 'src/components/MessageToasts/withToasts';
import Owner from 'src/types/Owner';
import { AntdCheckbox, AsyncSelect, Select } from 'src/components';
import TextAreaControl from 'src/explore/components/controls/TextAreaControl';
import { useCommonConf } from 'src/features/databases/state';
-import { CustomWidthHeaderStyle } from 'src/features/reports/ReportModal/styles';
import { InfoTooltipWithTrigger } from '@superset-ui/chart-controls';
import {
NotificationMethodOption,
@@ -59,11 +58,17 @@ import {
Operator,
Recipient,
AlertsReportsConfig,
+ ValidationObject,
+ Sections,
} from 'src/features/alerts/types';
import { useSelector } from 'react-redux';
import { UserWithPermissionsAndRoles } from 'src/types/bootstrapTypes';
+import NumberInput from './components/NumberInput';
import { AlertReportCronScheduler } from './components/AlertReportCronScheduler';
import { NotificationMethod } from './components/NotificationMethod';
+import ValidatedPanelHeader from './components/ValidatedPanelHeader';
+import StyledPanel from './components/StyledPanel';
+import { buildErrorTooltipMessage } from './buildErrorTooltipMessage';
const TIMEOUT_MIN = 1;
const TEXT_BASED_VISUALIZATION_TYPES = [
@@ -77,7 +82,7 @@ type SelectValue = {
label: string;
};
-interface AlertReportModalProps {
+export interface AlertReportModalProps {
addSuccessToast: (msg: string) => void;
addDangerToast: (msg: string) => void;
alert?: AlertObject | null;
@@ -88,7 +93,7 @@ interface AlertReportModalProps {
}
const DEFAULT_WORKING_TIMEOUT = 3600;
-const DEFAULT_CRON_VALUE = '0 * * * *'; // every hour
+const DEFAULT_CRON_VALUE = '0 0 * * *'; // every day
const DEFAULT_RETENTION = 90;
const DEFAULT_NOTIFICATION_METHODS: NotificationMethodOption[] = ['Email'];
@@ -143,62 +148,50 @@ const RETENTION_OPTIONS = [
},
];
-const StyledModal = styled(Modal)`
- max-width: 1200px;
- width: 100%;
-
- .ant-modal-body {
- overflow: initial;
- }
-`;
-
-const StyledTooltip = styled(InfoTooltipWithTrigger)`
- margin-left: ${({ theme }) => theme.gridUnit}px;
-`;
+const CONTENT_TYPE_OPTIONS = [
+ {
+ label: t('Dashboard'),
+ value: 'dashboard',
+ },
+ {
+ label: t('Chart'),
+ value: 'chart',
+ },
+];
+const FORMAT_OPTIONS = {
+ png: {
+ label: t('Send as PNG'),
+ value: 'PNG',
+ },
+ csv: {
+ label: t('Send as CSV'),
+ value: 'CSV',
+ },
+ txt: {
+ label: t('Send as text'),
+ value: 'TEXT',
+ },
+};
-const StyledIcon = (theme: SupersetTheme) => css`
- margin: auto ${theme.gridUnit * 2}px auto 0;
- color: ${theme.colors.grayscale.base};
+// Apply to final text input components of each collapse panel
+const noMarginBottom = css`
+ margin-bottom: 0;
`;
-const StyledSectionContainer = styled.div`
- display: flex;
- flex-direction: column;
+/*
+Height of modal body defined here, total width defined at component invocation as antd prop.
+ */
+const StyledModal = styled(Modal)`
+ .ant-modal-body {
+ height: 720px;
+ }
.control-label {
margin-top: ${({ theme }) => theme.gridUnit}px;
}
- .header-section {
- display: flex;
- flex: 0 0 auto;
- align-items: center;
- width: 100%;
- padding: ${({ theme }) => theme.gridUnit * 4}px;
- border-bottom: 1px solid ${({ theme }) => theme.colors.grayscale.light2};
- }
-
- .column-section {
- display: flex;
- flex: 1 1 auto;
-
- .column {
- flex: 1 1 auto;
- min-width: calc(33.33% - ${({ theme }) => theme.gridUnit * 8}px);
- padding: ${({ theme }) => theme.gridUnit * 4}px;
-
- .async-select {
- margin: 10px 0 20px;
- }
-
- &.condition {
- border-right: 1px solid ${({ theme }) => theme.colors.grayscale.light2};
- }
-
- &.message {
- border-left: 1px solid ${({ theme }) => theme.colors.grayscale.light2};
- }
- }
+ .ant-collapse > .ant-collapse-item {
+ border-bottom: none;
}
.inline-container {
@@ -212,34 +205,6 @@ const StyledSectionContainer = styled.div`
> div {
flex: 1 1 auto;
}
-
- &.add-margin {
- margin-bottom: 5px;
- }
-
- .styled-input {
- margin: 0 0 0 10px;
-
- input {
- flex: 0 0 auto;
- }
- }
- }
-`;
-
-const StyledSectionTitle = styled.div`
- display: flex;
- align-items: center;
- margin: ${({ theme }) => theme.gridUnit * 2}px auto
- ${({ theme }) => theme.gridUnit * 4}px auto;
-
- h4 {
- margin: 0;
- }
-
- .required {
- margin-left: ${({ theme }) => theme.gridUnit}px;
- color: ${({ theme }) => theme.colors.error.base};
}
`;
@@ -254,113 +219,112 @@ const StyledSwitchContainer = styled.div`
`;
export const StyledInputContainer = styled.div`
- flex: 1;
- margin-top: 0;
+ ${({ theme }) => css`
+ flex: 1;
+ margin-top: 0px;
+ margin-bottom: ${theme.gridUnit * 4}px;
+
+ input::-webkit-outer-spin-button,
+ input::-webkit-inner-spin-button {
+ -webkit-appearance: none;
+ margin: 0;
+ }
+ input[type='number'] {
+ -moz-appearance: textfield;
+ }
- input::-webkit-outer-spin-button,
- input::-webkit-inner-spin-button {
- -webkit-appearance: none;
- margin: 0;
- }
- input[type='number'] {
- -moz-appearance: textfield;
- }
+ .helper {
+ display: block;
+ color: ${theme.colors.grayscale.base};
+ font-size: ${theme.typography.sizes.s}px;
+ padding: ${theme.gridUnit}px 0;
+ text-align: left;
+ }
- .helper {
- display: block;
- color: ${({ theme }) => theme.colors.grayscale.base};
- font-size: ${({ theme }) => theme.typography.sizes.s}px;
- padding: ${({ theme }) => theme.gridUnit}px 0;
- text-align: left;
- }
+ .required {
+ margin-left: ${theme.gridUnit / 2}px;
+ color: ${theme.colors.error.base};
+ }
- .required {
- margin-left: ${({ theme }) => theme.gridUnit / 2}px;
- color: ${({ theme }) => theme.colors.error.base};
- }
+ .input-container {
+ display: flex;
+ align-items: center;
- .input-container {
- display: flex;
- align-items: center;
+ > div {
+ width: 100%;
+ }
- > div {
- width: 100%;
- }
+ label {
+ display: flex;
+ margin-right: ${theme.gridUnit * 2}px;
+ }
- label {
- display: flex;
- margin-right: ${({ theme }) => theme.gridUnit * 2}px;
+ i {
+ margin: 0 ${theme.gridUnit}px;
+ }
}
- i {
- margin: 0 ${({ theme }) => theme.gridUnit}px;
+ input,
+ textarea {
+ flex: 1 1 auto;
}
- }
- input,
- textarea {
- flex: 1 1 auto;
- }
-
- input[disabled] {
- color: ${({ theme }) => theme.colors.grayscale.base};
- }
+ input[disabled] {
+ color: ${theme.colors.grayscale.base};
+ }
- textarea {
- height: 300px;
- resize: none;
- }
+ textarea {
+ height: 300px;
+ resize: none;
+ }
- input::placeholder,
- textarea::placeholder {
- color: ${({ theme }) => theme.colors.grayscale.light1};
- }
+ input::placeholder,
+ textarea::placeholder {
+ color: ${theme.colors.grayscale.light1};
+ }
- textarea,
- input[type='text'],
- input[type='number'] {
- padding: ${({ theme }) => theme.gridUnit}px
- ${({ theme }) => theme.gridUnit * 2}px;
- border-style: none;
- border: 1px solid ${({ theme }) => theme.colors.grayscale.light2};
- border-radius: ${({ theme }) => theme.gridUnit}px;
+ textarea,
+ input[type='text'],
+ input[type='number'] {
+ padding: ${theme.gridUnit}px ${theme.gridUnit * 2}px;
+ border-style: none;
+ border: 1px solid ${theme.colors.grayscale.light2};
+ border-radius: ${theme.gridUnit}px;
- &[name='description'] {
- flex: 1 1 auto;
+ &[name='description'] {
+ flex: 1 1 auto;
+ }
}
- }
- .input-label {
- margin-left: 10px;
- }
-`;
-
-const StyledRadio = styled(Radio)`
- display: block;
- line-height: ${({ theme }) => theme.gridUnit * 7}px;
+ .input-label {
+ margin-left: 10px;
+ }
+ `}
`;
-const StyledRadioGroup = styled(Radio.Group)`
- margin-left: ${({ theme }) => theme.gridUnit * 5.5}px;
+const StyledCheckbox = styled(AntdCheckbox)`
+ margin-top: ${({ theme }) => theme.gridUnit * 0}px;
`;
-const StyledCheckbox = styled(AntdCheckbox)`
- margin-top: ${({ theme }) => theme.gridUnit * 2}px;
+const StyledTooltip = styled(InfoTooltipWithTrigger)`
+ margin-left: ${({ theme }) => theme.gridUnit}px;
`;
// Notification Method components
const StyledNotificationAddButton = styled.div`
- color: ${({ theme }) => theme.colors.primary.dark1};
- cursor: pointer;
+ ${({ theme }) => css`
+ color: ${theme.colors.primary.dark1};
+ cursor: pointer;
- i {
- margin-right: ${({ theme }) => theme.gridUnit * 2}px;
- }
+ i {
+ margin-right: ${theme.gridUnit * 2}px;
+ }
- &.disabled {
- color: ${({ theme }) => theme.colors.grayscale.light1};
- cursor: default;
- }
+ &.disabled {
+ color: ${theme.colors.grayscale.light1};
+ cursor: default;
+ }
+ `}
`;
const StyledNotificationMethodWrapper = styled.div`
@@ -369,10 +333,6 @@ const StyledNotificationMethodWrapper = styled.div`
}
`;
-const timezoneHeaderStyle = (theme: SupersetTheme) => css`
- margin: ${theme.gridUnit * 3}px 0;
-`;
-
const inputSpacer = (theme: SupersetTheme) => css`
margin-right: ${theme.gridUnit * 3}px;
`;
@@ -385,47 +345,26 @@ interface NotificationMethodAddProps {
}
export const TRANSLATIONS = {
- ADD_NOTIFICATION_METHOD_TEXT: t('Add notification method'),
- ADD_DELIVERY_METHOD_TEXT: t('Add delivery method'),
- SAVE_TEXT: t('Save'),
- ADD_TEXT: t('Add'),
- EDIT_REPORT_TEXT: t('Edit Report'),
- EDIT_ALERT_TEXT: t('Edit Alert'),
- ADD_REPORT_TEXT: t('Add Report'),
- ADD_ALERT_TEXT: t('Add Alert'),
- REPORT_NAME_TEXT: t('Report name'),
- ALERT_NAME_TEXT: t('Alert name'),
- OWNERS_TEXT: t('Owners'),
- DESCRIPTION_TEXT: t('Description'),
- ACTIVE_TEXT: t('Active'),
- ALERT_CONDITION_TEXT: t('Alert condition'),
- DATABASE_TEXT: t('Database'),
- SQL_QUERY_TEXT: t('SQL Query'),
- SQL_QUERY_TOOLTIP: t(
- 'The result of this query should be a numeric-esque value',
+ // Panel titles
+ GENERAL_TITLE: t('General information'),
+ ALERT_CONDITION_TITLE: t('Alert condition'),
+ ALERT_CONTENTS_TITLE: t('Alert contents'),
+ REPORT_CONTENTS_TITLE: t('Report contents'),
+ SCHEDULE_TITLE: t('Schedule'),
+ NOTIFICATION_TITLE: t('Notification method'),
+ // Error text
+ NAME_ERROR_TEXT: t('name'),
+ OWNERS_ERROR_TEXT: t('owners'),
+ CONTENT_ERROR_TEXT: t('content type'),
+ DATABASE_ERROR_TEXT: t('database'),
+ SQL_ERROR_TEXT: t('sql'),
+ ALERT_CONDITION_ERROR_TEXT: t('alert condition'),
+ CRONTAB_ERROR_TEXT: t('crontab'),
+ WORKING_TIMEOUT_ERROR_TEXT: t('working timeout'),
+ RECIPIENTS_ERROR_TEXT: t('recipients'),
+ ERROR_TOOLTIP_MESSAGE: t(
+ 'Not all required fields are complete. Please provide the following:',
),
- TRIGGER_ALERT_IF_TEXT: t('Trigger Alert If...'),
- CONDITION_TEXT: t('Condition'),
- VALUE_TEXT: t('Value'),
- REPORT_SCHEDULE_TEXT: t('Report schedule'),
- ALERT_CONDITION_SCHEDULE_TEXT: t('Alert condition schedule'),
- TIMEZONE_TEXT: t('Timezone'),
- SCHEDULE_SETTINGS_TEXT: t('Schedule settings'),
- LOG_RETENTION_TEXT: t('Log retention'),
- WORKING_TIMEOUT_TEXT: t('Working timeout'),
- TIME_IN_SECONDS_TEXT: t('Time in seconds'),
- SECONDS_TEXT: t('seconds'),
- GRACE_PERIOD_TEXT: t('Grace period'),
- MESSAGE_CONTENT_TEXT: t('Message content'),
- DASHBOARD_TEXT: t('Dashboard'),
- CHART_TEXT: t('Chart'),
- SEND_AS_PNG_TEXT: t('Send as PNG'),
- SEND_AS_CSV_TEXT: t('Send as CSV'),
- SEND_AS_TEXT: t('Send as text'),
- IGNORE_CACHE_TEXT: t('Ignore cache when generating report'),
- CUSTOM_SCREENSHOT_WIDTH_TEXT: t('Screenshot width'),
- CUSTOM_SCREENSHOT_WIDTH_PLACEHOLDER_TEXT: t('Input custom width in pixels'),
- NOTIFICATION_METHOD_TEXT: t('Notification method'),
};
const NotificationMethodAdd: FunctionComponent<NotificationMethodAddProps> = ({
@@ -446,8 +385,8 @@ const NotificationMethodAdd: FunctionComponent<NotificationMethodAddProps> = ({
<StyledNotificationAddButton className={status} onClick={checkStatus}>
<i className="fa fa-plus" />{' '}
{status === 'active'
- ? TRANSLATIONS.ADD_NOTIFICATION_METHOD_TEXT
- : TRANSLATIONS.ADD_DELIVERY_METHOD_TEXT}
+ ? t('Add another notification method')
+ : t('Add delivery method')}
</StyledNotificationAddButton>
);
};
@@ -470,11 +409,13 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
const currentUser = useSelector<any, UserWithPermissionsAndRoles>(
state => state.user,
);
+ // Check config for alternate notification methods setting
const conf = useCommonConf();
const allowedNotificationMethods: NotificationMethodOption[] =
conf?.ALERT_REPORTS_NOTIFICATION_METHODS || DEFAULT_NOTIFICATION_METHODS;
const [disableSave, setDisableSave] = useState<boolean>(true);
+
const [currentAlert, setCurrentAlert] =
useState<Partial<AlertObject> | null>();
const [isHidden, setIsHidden] = useState<boolean>(true);
@@ -497,10 +438,53 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
const [sourceOptions, setSourceOptions] = useState<MetaObject[]>([]);
const [dashboardOptions, setDashboardOptions] = useState<MetaObject[]>([]);
const [chartOptions, setChartOptions] = useState<MetaObject[]>([]);
+ // Validation
+ const [validationStatus, setValidationStatus] = useState<ValidationObject>({
+ [Sections.General]: {
+ hasErrors: false,
+ name: TRANSLATIONS.GENERAL_TITLE,
+ errors: [],
+ },
+ [Sections.Content]: {
+ hasErrors: false,
+ name: isReport
+ ? TRANSLATIONS.REPORT_CONTENTS_TITLE
+ : TRANSLATIONS.ALERT_CONTENTS_TITLE,
+ errors: [],
+ },
+ [Sections.Alert]: {
+ hasErrors: false,
+ name: TRANSLATIONS.ALERT_CONDITION_TITLE,
+ errors: [],
+ },
+ [Sections.Schedule]: {
+ hasErrors: false,
+ name: TRANSLATIONS.SCHEDULE_TITLE,
+ errors: [],
+ },
+ [Sections.Notification]: {
+ hasErrors: false,
+ name: TRANSLATIONS.NOTIFICATION_TITLE,
+ errors: [],
+ },
+ });
+ const [errorTooltipMessage, setErrorTooltipMessage] = useState<ReactNode>('');
+
+ const updateValidationStatus = (section: Sections, errors: string[]) => {
+ setValidationStatus(currentValidationData => ({
+ ...currentValidationData,
+ [section]: {
+ hasErrors: errors.length > 0,
+ name: currentValidationData[section].name,
+ errors,
+ },
+ }));
+ };
// Chart metadata
const [chartVizType, setChartVizType] = useState<string>('');
+ const reportOrAlert = isReport ? 'report' : 'alert';
const isEditMode = alert !== null;
const formatOptionEnabled =
contentType === 'chart' &&
@@ -508,13 +492,12 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
const [notificationAddState, setNotificationAddState] =
useState<NotificationAddStatus>('active');
+
const [notificationSettings, setNotificationSettings] = useState<
NotificationSetting[]
>([]);
-
const onNotificationAdd = () => {
const settings: NotificationSetting[] = notificationSettings.slice();
-
settings.push({
recipients: '',
options: allowedNotificationMethods,
@@ -966,18 +949,14 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
updateAlertState('timezone', timezone);
};
- const onContentTypeChange = (event: any) => {
- const { target } = event;
+ const onContentTypeChange = (value: string) => {
// When switch content type, reset force_screenshot to false
setForceScreenshot(false);
- // Gives time to close the select before changing the type
- setTimeout(() => setContentType(target.value), 200);
+ setContentType(value);
};
- const onFormatChange = (event: any) => {
- const { target } = event;
-
- setReportFormat(target.value);
+ const onFormatChange = (value: string) => {
+ setReportFormat(value);
};
const onForceScreenshotChange = (event: any) => {
@@ -1001,32 +980,89 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
return hasInfo;
};
- const validate = () => {
+ const validateGeneralSection = () => {
+ const errors = [];
+ if (!currentAlert?.name?.length) {
+ errors.push(TRANSLATIONS.NAME_ERROR_TEXT);
+ }
+ if (!currentAlert?.owners?.length) {
+ errors.push(TRANSLATIONS.OWNERS_ERROR_TEXT);
+ }
+ updateValidationStatus(Sections.General, errors);
+ };
+ const validateContentSection = () => {
+ const errors = [];
if (
- currentAlert?.name?.length &&
- currentAlert?.owners?.length &&
- currentAlert?.crontab?.length &&
- currentAlert?.working_timeout !== undefined &&
- ((contentType === 'dashboard' && !!currentAlert?.dashboard) ||
- (contentType === 'chart' && !!currentAlert?.chart)) &&
- checkNotificationSettings()
+ !(
+ (contentType === 'dashboard' && !!currentAlert?.dashboard) ||
+ (contentType === 'chart' && !!currentAlert?.chart)
+ )
) {
- if (isReport) {
- setDisableSave(false);
- } else if (
- !!currentAlert.database &&
- currentAlert.sql?.length &&
- (conditionNotNull || !!currentAlert.validator_config_json?.op) &&
+ errors.push(TRANSLATIONS.CONTENT_ERROR_TEXT);
+ }
+ updateValidationStatus(Sections.Content, errors);
+ };
+ const validateAlertSection = () => {
+ const errors = [];
+ if (!currentAlert?.database) {
+ errors.push(TRANSLATIONS.DATABASE_ERROR_TEXT);
+ }
+ if (!currentAlert?.sql?.length) {
+ errors.push(TRANSLATIONS.SQL_ERROR_TEXT);
+ }
+ if (
+ !(
+ (conditionNotNull || !!currentAlert?.validator_config_json?.op) &&
(conditionNotNull ||
- currentAlert.validator_config_json?.threshold !== undefined)
- ) {
- setDisableSave(false);
- } else {
- setDisableSave(true);
- }
- } else {
- setDisableSave(true);
+ currentAlert?.validator_config_json?.threshold !== undefined)
+ )
+ ) {
+ errors.push(TRANSLATIONS.ALERT_CONDITION_ERROR_TEXT);
}
+ updateValidationStatus(Sections.Alert, errors);
+ };
+
+ const validateScheduleSection = () => {
+ const errors = [];
+ if (!currentAlert?.crontab?.length) {
+ errors.push(TRANSLATIONS.CRONTAB_ERROR_TEXT);
+ }
+ if (!currentAlert?.working_timeout) {
+ errors.push(TRANSLATIONS.WORKING_TIMEOUT_ERROR_TEXT);
+ }
+
+ updateValidationStatus(Sections.Schedule, errors);
+ };
+
+ const validateNotificationSection = () => {
+ const hasErrors = !checkNotificationSettings();
+ const errors = hasErrors ? [TRANSLATIONS.RECIPIENTS_ERROR_TEXT] : [];
+ updateValidationStatus(Sections.Notification, errors);
+ };
+
+ const validateAll = () => {
+ validateGeneralSection();
+ validateContentSection();
+ if (!isReport) validateAlertSection();
+ validateScheduleSection();
+ validateNotificationSection();
+ };
+
+ const enforceValidation = () => {
+ const sections = [
+ Sections.General,
+ Sections.Content,
+ isReport ? undefined : Sections.Alert,
+ Sections.Schedule,
+ Sections.Notification,
+ ];
+
+ const hasErrors = sections.some(
+ section => section && validationStatus[section].hasErrors,
+ );
+ const tooltip = hasErrors ? buildErrorTooltipMessage(validationStatus) : '';
+ setErrorTooltipMessage(tooltip);
+ setDisableSave(hasErrors);
};
// Initialize
@@ -1054,7 +1090,13 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
]
: [],
});
- setNotificationSettings([]);
+ setNotificationSettings([
+ {
+ recipients: '',
+ options: allowedNotificationMethods,
+ method: 'Email',
+ },
+ ]);
setNotificationAddState('active');
}
}, [alert]);
@@ -1139,7 +1181,7 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
// Validation
const currentAlertSafe = currentAlert || {};
useEffect(() => {
- validate();
+ validateAll();
}, [
currentAlertSafe.name,
currentAlertSafe.owners,
@@ -1154,403 +1196,495 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
notificationSettings,
conditionNotNull,
]);
+ useEffect(() => {
+ enforceValidation();
+ }, [validationStatus]);
// Show/hide
if (isHidden && show) {
setIsHidden(false);
}
+ const getTitleText = () => {
+ let titleText;
+
+ switch (true) {
+ case isEditMode && isReport:
+ titleText = t('Edit Report');
+ break;
+ case isEditMode:
+ titleText = t('Edit Alert');
+ break;
+ case isReport:
+ titleText = t('Add Report');
+ break;
+ default:
+ titleText = t('Add Alert');
+ break;
+ }
+
+ return titleText;
+ };
+
return (
<StyledModal
className="no-content-padding"
responsive
disablePrimaryButton={disableSave}
+ primaryTooltipMessage={errorTooltipMessage}
onHandledPrimaryAction={onSave}
onHide={hide}
- primaryButtonName={
- isEditMode ? TRANSLATIONS.SAVE_TEXT : TRANSLATIONS.ADD_TEXT
- }
+ primaryButtonName={isEditMode ? t('Save') : t('Add')}
show={show}
- width="100%"
- maxWidth="1450px"
- title={
- <h4 data-test="alert-report-modal-title">
- {isEditMode ? (
- <Icons.EditAlt css={StyledIcon} />
- ) : (
- <Icons.PlusLarge css={StyledIcon} />
- )}
- {isEditMode && isReport
- ? TRANSLATIONS.EDIT_REPORT_TEXT
- : isEditMode
- ? TRANSLATIONS.EDIT_ALERT_TEXT
- : isReport
- ? TRANSLATIONS.ADD_REPORT_TEXT
- : TRANSLATIONS.ADD_ALERT_TEXT}
- </h4>
- }
+ width="500px"
+ centered
+ title={<h4 data-test="alert-report-modal-title">{getTitleText()}</h4>}
>
- <StyledSectionContainer>
- <div className="header-section">
- <StyledInputContainer>
- <div className="control-label">
- {isReport
- ? TRANSLATIONS.REPORT_NAME_TEXT
- : TRANSLATIONS.ALERT_NAME_TEXT}
- <span className="required">*</span>
- </div>
- <div className="input-container">
- <input
- type="text"
- name="name"
- value={currentAlert ? currentAlert.name : ''}
- placeholder={
- isReport
- ? TRANSLATIONS.REPORT_NAME_TEXT
- : TRANSLATIONS.ALERT_NAME_TEXT
- }
- onChange={onInputChange}
- css={inputSpacer}
- />
- </div>
- </StyledInputContainer>
- <StyledInputContainer>
- <div className="control-label">
- {TRANSLATIONS.OWNERS_TEXT}
- <span className="required">*</span>
- </div>
- <div data-test="owners-select" className="input-container">
- <AsyncSelect
- ariaLabel={TRANSLATIONS.OWNERS_TEXT}
- allowClear
- name="owners"
- mode="multiple"
- value={
- (currentAlert?.owners as {
- label: string;
- value: number;
- }[]) || []
- }
- options={loadOwnerOptions}
- onChange={onOwnersChange}
- css={inputSpacer}
- />
- </div>
- </StyledInputContainer>
- <StyledInputContainer>
- <div className="control-label">{TRANSLATIONS.DESCRIPTION_TEXT}</div>
- <div className="input-container">
- <input
- type="text"
- name="description"
- value={currentAlert ? currentAlert.description || '' : ''}
- placeholder={TRANSLATIONS.DESCRIPTION_TEXT}
- onChange={onInputChange}
- css={inputSpacer}
- />
- </div>
- </StyledInputContainer>
- <StyledSwitchContainer>
- <Switch
- onChange={onActiveSwitch}
- checked={currentAlert ? currentAlert.active : true}
+ <Collapse
+ expandIconPosition="right"
+ defaultActiveKey="general"
+ accordion
+ css={css`
+ border: 'none';
+ `}
+ >
+ <StyledPanel
+ header={
+ <ValidatedPanelHeader
+ title={TRANSLATIONS.GENERAL_TITLE}
+ subtitle={t(
+ 'Set up basic details, such as name and description.',
+ )}
+ validateCheckStatus={
+ !validationStatus[Sections.General].hasErrors
+ }
+ testId="general-information-panel"
/>
- <div className="switch-label">{TRANSLATIONS.ACTIVE_TEXT}</div>
- </StyledSwitchContainer>
- </div>
- <div className="column-section">
- {!isReport && (
- <div className="column condition">
- <StyledSectionTitle>
- <h4>{TRANSLATIONS.ALERT_CONDITION_TEXT}</h4>
- </StyledSectionTitle>
- <StyledInputContainer>
- <div className="control-label">
- {TRANSLATIONS.DATABASE_TEXT}
- <span className="required">*</span>
- </div>
- <div className="input-container">
- <AsyncSelect
- ariaLabel={TRANSLATIONS.DATABASE_TEXT}
- name="source"
- value={
- currentAlert?.database?.label &&
- currentAlert?.database?.value
- ? {
- value: currentAlert.database.value,
- label: currentAlert.database.label,
- }
- : undefined
- }
- options={loadSourceOptions}
- onChange={onSourceChange}
- />
- </div>
- </StyledInputContainer>
- <StyledInputContainer>
- <div className="control-label">
- {TRANSLATIONS.SQL_QUERY_TEXT}
- <StyledTooltip tooltip={TRANSLATIONS.SQL_QUERY_TOOLTIP} />
- <span className="required">*</span>
- </div>
- <TextAreaControl
- name="sql"
- language="sql"
- offerEditInModal={false}
- minLines={15}
- maxLines={15}
- onChange={onSQLChange}
- readOnly={false}
- initialValue={resource?.sql}
- key={currentAlert?.id}
+ }
+ key="general"
+ >
+ <div className="header-section">
+ <StyledInputContainer>
+ <div className="control-label">
+ {isReport ? t('Report name') : t('Alert name')}
+ <span className="required">*</span>
+ </div>
+ <div className="input-container">
+ <input
+ type="text"
+ name="name"
+ value={currentAlert ? currentAlert.name : ''}
+ placeholder={
+ isReport ? t('Enter report name') : t('Enter alert name')
+ }
+ onChange={onInputChange}
/>
- </StyledInputContainer>
- <div className="inline-container wrap">
- <StyledInputContainer>
- <div className="control-label" css={inputSpacer}>
- {TRANSLATIONS.TRIGGER_ALERT_IF_TEXT}
- <span className="required">*</span>
- </div>
- <div className="input-container">
- <Select
- ariaLabel={TRANSLATIONS.CONDITION_TEXT}
- onChange={onConditionChange}
- placeholder="Condition"
- value={
- currentAlert?.validator_config_json?.op || undefined
- }
- options={CONDITIONS}
- css={inputSpacer}
- />
- </div>
- </StyledInputContainer>
- <StyledInputContainer>
- <div className="control-label">
- {TRANSLATIONS.VALUE_TEXT}
- <span className="required">*</span>
- </div>
- <div className="input-container">
- <input
- type="number"
- name="threshold"
- disabled={conditionNotNull}
- value={
- currentAlert?.validator_config_json?.threshold !==
- undefined
- ? currentAlert.validator_config_json.threshold
- : ''
- }
- placeholder={TRANSLATIONS.VALUE_TEXT}
- onChange={onThresholdChange}
- />
- </div>
- </StyledInputContainer>
</div>
- </div>
- )}
- <div className="column schedule">
- <StyledSectionTitle>
- <h4>
- {isReport
- ? TRANSLATIONS.REPORT_SCHEDULE_TEXT
- : TRANSLATIONS.ALERT_CONDITION_SCHEDULE_TEXT}
- </h4>
- <span className="required">*</span>
- </StyledSectionTitle>
- <AlertReportCronScheduler
- value={currentAlert?.crontab || ALERT_REPORTS_DEFAULT_CRON_VALUE}
- onChange={newVal => updateAlertState('crontab', newVal)}
- />
- <div className="control-label">{TRANSLATIONS.TIMEZONE_TEXT}</div>
- <div
- className="input-container"
- css={(theme: SupersetTheme) => timezoneHeaderStyle(theme)}
- >
- <TimezoneSelector
- onTimezoneChange={onTimezoneChange}
- timezone={currentAlert?.timezone}
- minWidth="100%"
- />
- </div>
- <StyledSectionTitle>
- <h4>{TRANSLATIONS.SCHEDULE_SETTINGS_TEXT}</h4>
- </StyledSectionTitle>
+ </StyledInputContainer>
<StyledInputContainer>
<div className="control-label">
- {TRANSLATIONS.LOG_RETENTION_TEXT}
+ {t('Owners')}
<span className="required">*</span>
</div>
- <div className="input-container">
- <Select
- ariaLabel={TRANSLATIONS.LOG_RETENTION_TEXT}
- placeholder={TRANSLATIONS.LOG_RETENTION_TEXT}
- onChange={onLogRetentionChange}
+ <div data-test="owners-select" className="input-container">
+ <AsyncSelect
+ ariaLabel={t('Owners')}
+ allowClear
+ name="owners"
+ mode="multiple"
+ placeholder={t('Select owners')}
value={
- typeof currentAlert?.log_retention === 'number'
- ? currentAlert?.log_retention
- : ALERT_REPORTS_DEFAULT_RETENTION
+ (currentAlert?.owners as {
+ label: string;
+ value: number;
+ }[]) || []
}
- options={RETENTION_OPTIONS}
- sortComparator={propertyComparator('value')}
+ options={loadOwnerOptions}
+ onChange={onOwnersChange}
/>
</div>
</StyledInputContainer>
+ <StyledInputContainer>
+ <div className="control-label">{t('Description')}</div>
+ <div className="input-container">
+ <input
+ type="text"
+ name="description"
+ value={currentAlert ? currentAlert.description || '' : ''}
+ placeholder={t(
+ 'Include description to be sent with %s',
+ reportOrAlert,
+ )}
+ onChange={onInputChange}
+ />
+ </div>
+ </StyledInputContainer>
+ <StyledSwitchContainer>
+ <Switch
+ checked={currentAlert ? currentAlert.active : false}
+ defaultChecked
+ onChange={onActiveSwitch}
+ />
+ <div className="switch-label">
+ {isReport ? t('Report is active') : t('Alert is active')}
+ </div>
+ </StyledSwitchContainer>
+ </div>
+ </StyledPanel>
+ {!isReport && (
+ <StyledPanel
+ header={
+ <ValidatedPanelHeader
+ title={TRANSLATIONS.ALERT_CONDITION_TITLE}
+ subtitle={t(
+ 'Define the database, SQL query, and triggering conditions for alert.',
+ )}
+ validateCheckStatus={
+ !validationStatus[Sections.Alert].hasErrors
+ }
+ testId="alert-condition-panel"
+ />
+ }
+ key="condition"
+ >
<StyledInputContainer>
<div className="control-label">
- {TRANSLATIONS.WORKING_TIMEOUT_TEXT}
+ {t('Database')}
<span className="required">*</span>
</div>
<div className="input-container">
- <input
- type="number"
- min="1"
- name="working_timeout"
- value={currentAlert?.working_timeout || ''}
- placeholder={TRANSLATIONS.TIME_IN_SECONDS_TEXT}
- onChange={onTimeoutVerifyChange}
+ <AsyncSelect
+ ariaLabel={t('Database')}
+ name="source"
+ placeholder={t('Select database')}
+ value={
+ currentAlert?.database?.label &&
+ currentAlert?.database?.value
+ ? {
+ value: currentAlert.database.value,
+ label: currentAlert.database.label,
+ }
+ : undefined
+ }
+ options={loadSourceOptions}
+ onChange={onSourceChange}
/>
- <span className="input-label">{TRANSLATIONS.SECONDS_TEXT}</span>
</div>
</StyledInputContainer>
- {!isReport && (
- <StyledInputContainer>
+ <StyledInputContainer>
+ <div className="control-label">
+ {t('SQL Query')}
+ <StyledTooltip
+ tooltip={t(
+ 'The result of this query must be a value capable of numeric interpretation e.g. 1, 1.0, or "1" (compatible with Python\'s float() function).',
+ )}
+ />
+ <span className="required">*</span>
+ </div>
+ <TextAreaControl
+ name="sql"
+ language="sql"
+ offerEditInModal={false}
+ minLines={15}
+ maxLines={15}
+ onChange={onSQLChange}
+ readOnly={false}
+ initialValue={resource?.sql}
+ key={currentAlert?.id}
+ />
+ </StyledInputContainer>
+ <div className="inline-container wrap">
+ <StyledInputContainer css={noMarginBottom}>
+ <div className="control-label" css={inputSpacer}>
+ {t('Trigger Alert If...')}
+ <span className="required">*</span>
+ </div>
+ <div className="input-container">
+ <Select
+ ariaLabel={t('Condition')}
+ onChange={onConditionChange}
+ placeholder={t('Condition')}
+ value={currentAlert?.validator_config_json?.op || undefined}
+ options={CONDITIONS}
+ css={inputSpacer}
+ />
+ </div>
+ </StyledInputContainer>
+ <StyledInputContainer css={noMarginBottom}>
<div className="control-label">
- {TRANSLATIONS.GRACE_PERIOD_TEXT}
+ {t('Value')} <span className="required">*</span>
</div>
<div className="input-container">
<input
type="number"
- min="1"
- name="grace_period"
- value={currentAlert?.grace_period || ''}
- placeholder={TRANSLATIONS.TIME_IN_SECONDS_TEXT}
- onChange={onTimeoutVerifyChange}
+ name="threshold"
+ disabled={conditionNotNull}
+ value={
+ currentAlert?.validator_config_json?.threshold !==
+ undefined
+ ? currentAlert.validator_config_json.threshold
+ : ''
+ }
+ placeholder={t('Value')}
+ onChange={onThresholdChange}
/>
- <span className="input-label">
- {TRANSLATIONS.SECONDS_TEXT}
- </span>
</div>
</StyledInputContainer>
- )}
- </div>
- <div className="column message">
- <StyledSectionTitle>
- <h4>{TRANSLATIONS.MESSAGE_CONTENT_TEXT}</h4>
+ </div>
+ </StyledPanel>
+ )}
+ <StyledPanel
+ header={
+ <ValidatedPanelHeader
+ title={
+ isReport
+ ? TRANSLATIONS.REPORT_CONTENTS_TITLE
+ : TRANSLATIONS.ALERT_CONTENTS_TITLE
+ }
+ subtitle={t('Customize data source, filters, and layout.')}
+ validateCheckStatus={
+ !validationStatus[Sections.Content].hasErrors
+ }
+ testId="contents-panel"
+ />
+ }
+ key="contents"
+ >
+ <StyledInputContainer>
+ <div className="control-label">
+ {t('Content type')}
<span className="required">*</span>
- </StyledSectionTitle>
- <Radio.Group onChange={onContentTypeChange} value={contentType}>
- <StyledRadio value="dashboard">
- {TRANSLATIONS.DASHBOARD_TEXT}
- </StyledRadio>
- <StyledRadio value="chart">{TRANSLATIONS.CHART_TEXT}</StyledRadio>
- </Radio.Group>
+ </div>
+ <Select
+ ariaLabel={t('Select content type')}
+ onChange={onContentTypeChange}
+ value={contentType}
+ options={CONTENT_TYPE_OPTIONS}
+ placeholder={t('Select content type')}
+ />
+ </StyledInputContainer>
+ <StyledInputContainer>
{contentType === 'chart' ? (
- <AsyncSelect
- ariaLabel={TRANSLATIONS.CHART_TEXT}
- name="chart"
- value={
- currentAlert?.chart?.label && currentAlert?.chart?.value
- ? {
- value: currentAlert.chart.value,
- label: currentAlert.chart.label,
- }
- : undefined
- }
- options={loadChartOptions}
- onChange={onChartChange}
- />
+ <>
+ <div className="control-label">
+ {t('Select chart')}
+ <span className="required">*</span>
+ </div>
+ <AsyncSelect
+ ariaLabel={t('Chart')}
+ name="chart"
+ value={
+ currentAlert?.chart?.label && currentAlert?.chart?.value
+ ? {
+ value: currentAlert.chart.value,
+ label: currentAlert.chart.label,
+ }
+ : undefined
+ }
+ options={loadChartOptions}
+ onChange={onChartChange}
+ placeholder={t('Select chart to use')}
+ />
+ </>
) : (
- <AsyncSelect
- ariaLabel={TRANSLATIONS.DASHBOARD_TEXT}
- name="dashboard"
- value={
- currentAlert?.dashboard?.label &&
- currentAlert?.dashboard?.value
- ? {
- value: currentAlert.dashboard.value,
- label: currentAlert.dashboard.label,
- }
- : undefined
- }
- options={loadDashboardOptions}
- onChange={onDashboardChange}
- />
+ <>
+ <div className="control-label">
+ {t('Select dashboard')}
+ <span className="required">*</span>
+ </div>
+ <AsyncSelect
+ ariaLabel={t('Dashboard')}
+ name="dashboard"
+ value={
+ currentAlert?.dashboard?.label &&
+ currentAlert?.dashboard?.value
+ ? {
+ value: currentAlert.dashboard.value,
+ label: currentAlert.dashboard.label,
+ }
+ : undefined
+ }
+ options={loadDashboardOptions}
+ onChange={onDashboardChange}
+ placeholder={t('Select dashboard to use')}
+ />
+ </>
)}
+ </StyledInputContainer>
+ <StyledInputContainer
+ css={['TEXT', 'CSV'].includes(reportFormat) && noMarginBottom}
+ >
{formatOptionEnabled && (
<>
- <div className="inline-container">
- <StyledRadioGroup
- onChange={onFormatChange}
- value={reportFormat}
- >
- <StyledRadio value="PNG">
- {TRANSLATIONS.SEND_AS_PNG_TEXT}
- </StyledRadio>
- <StyledRadio value="CSV">
- {TRANSLATIONS.SEND_AS_CSV_TEXT}
- </StyledRadio>
- {TEXT_BASED_VISUALIZATION_TYPES.includes(chartVizType) && (
- <StyledRadio value="TEXT">
- {TRANSLATIONS.SEND_AS_TEXT}
- </StyledRadio>
- )}
- </StyledRadioGroup>
+ <div className="control-label">
+ {t('Content format')}
+ <span className="required">*</span>
</div>
+ <Select
+ ariaLabel={t('Select format')}
+ onChange={onFormatChange}
+ value={reportFormat}
+ options={
+ /* If chart is of text based viz type: show text
+ format option */
+ TEXT_BASED_VISUALIZATION_TYPES.includes(chartVizType)
+ ? Object.values(FORMAT_OPTIONS)
+ : ['png', 'csv'].map(key => FORMAT_OPTIONS[key])
+ }
+ placeholder={t('Select format')}
+ />
</>
)}
- {isScreenshot && (
- <StyledInputContainer>
- <div className="control-label" css={CustomWidthHeaderStyle}>
- {TRANSLATIONS.CUSTOM_SCREENSHOT_WIDTH_TEXT}
+ </StyledInputContainer>
+ {isScreenshot && (
+ <StyledInputContainer
+ css={!isReport && contentType === 'chart' && noMarginBottom}
+ >
+ <div className="control-label">{t('Screenshot width')}</div>
+ <div className="input-container">
+ <Input
+ type="number"
+ name="custom_width"
+ value={currentAlert?.custom_width || ''}
+ placeholder={t('Input custom width in pixels')}
+ onChange={onInputChange}
+ />
+ </div>
+ </StyledInputContainer>
+ )}
+ {(isReport || contentType === 'dashboard') && (
+ <div className="inline-container">
+ <StyledCheckbox
+ data-test="bypass-cache"
+ className="checkbox"
+ checked={forceScreenshot}
+ onChange={onForceScreenshotChange}
+ >
+ {t('Ignore cache when generating report')}
+ </StyledCheckbox>
+ </div>
+ )}
+ </StyledPanel>
+ <StyledPanel
+ header={
+ <ValidatedPanelHeader
+ title={TRANSLATIONS.SCHEDULE_TITLE}
+ subtitle={t(
+ 'Define delivery schedule, timezone, and frequency settings.',
+ )}
+ validateCheckStatus={
+ !validationStatus[Sections.Schedule].hasErrors
+ }
+ testId="schedule-panel"
+ />
+ }
+ key="schedule"
+ >
+ <AlertReportCronScheduler
+ value={currentAlert?.crontab || ALERT_REPORTS_DEFAULT_CRON_VALUE}
+ onChange={newVal => updateAlertState('crontab', newVal)}
+ />
+ <StyledInputContainer>
+ <div className="control-label">
+ {t('Timezone')} <span className="required">*</span>
+ </div>
+ <TimezoneSelector
+ onTimezoneChange={onTimezoneChange}
+ timezone={currentAlert?.timezone}
+ minWidth="100%"
+ />
+ </StyledInputContainer>
+ <StyledInputContainer>
+ <div className="control-label">
+ {t('Log retention')}
+ <span className="required">*</span>
+ </div>
+ <div className="input-container">
+ <Select
+ ariaLabel={t('Log retention')}
+ placeholder={t('Log retention')}
+ onChange={onLogRetentionChange}
+ value={
+ typeof currentAlert?.log_retention === 'number'
+ ? currentAlert?.log_retention
+ : ALERT_REPORTS_DEFAULT_RETENTION
+ }
+ options={RETENTION_OPTIONS}
+ sortComparator={propertyComparator('value')}
+ />
+ </div>
+ </StyledInputContainer>
+ <StyledInputContainer css={noMarginBottom}>
+ {isReport ? (
+ <>
+ <div className="control-label">
+ {t('Working timeout')}
+ <span className="required">*</span>
</div>
<div className="input-container">
- <Input
- type="number"
- name="custom_width"
- value={currentAlert?.custom_width || ''}
- placeholder={
- TRANSLATIONS.CUSTOM_SCREENSHOT_WIDTH_PLACEHOLDER_TEXT
- }
- onChange={onInputChange}
+ <NumberInput
+ min={1}
+ name="working_timeout"
+ value={currentAlert?.working_timeout || ''}
+ placeholder={t('Time in seconds')}
+ onChange={onTimeoutVerifyChange}
+ timeUnit={t('seconds')}
/>
</div>
- </StyledInputContainer>
- )}
- {(isReport || contentType === 'dashboard') && (
- <div className="inline-container">
- <StyledCheckbox
- data-test="bypass-cache"
- className="checkbox"
- checked={forceScreenshot}
- onChange={onForceScreenshotChange}
- >
- {TRANSLATIONS.IGNORE_CACHE_TEXT}
- </StyledCheckbox>
- </div>
+ </>
+ ) : (
+ <>
+ <div className="control-label">{t('Grace period')}</div>
+ <div className="input-container">
+ <NumberInput
+ min={1}
+ name="grace_period"
+ value={currentAlert?.grace_period || ''}
+ placeholder={t('Time in seconds')}
+ onChange={onTimeoutVerifyChange}
+ timeUnit={t('seconds')}
+ />
+ </div>
+ </>
)}
- <StyledSectionTitle>
- <h4>{TRANSLATIONS.NOTIFICATION_METHOD_TEXT}</h4>
- <span className="required">*</span>
- </StyledSectionTitle>
- {notificationSettings.map((notificationSetting, i) => (
- <StyledNotificationMethodWrapper>
- <NotificationMethod
- setting={notificationSetting}
- index={i}
- key={`NotificationMethod-${i}`}
- onUpdate={updateNotificationSetting}
- onRemove={removeNotificationSetting}
- />
- </StyledNotificationMethodWrapper>
- ))}
- <NotificationMethodAdd
- data-test="notification-add"
- status={notificationAddState}
- onClick={onNotificationAdd}
+ </StyledInputContainer>
+ </StyledPanel>
+ <StyledPanel
+ header={
+ <ValidatedPanelHeader
+ title={TRANSLATIONS.NOTIFICATION_TITLE}
+ subtitle={t('Choose notification method and recipients.')}
+ validateCheckStatus={
+ !validationStatus[Sections.Notification].hasErrors
+ }
+ testId="notification-method-panel"
/>
- </div>
- </div>
- </StyledSectionContainer>
+ }
+ key="notification"
+ >
+ {notificationSettings.map((notificationSetting, i) => (
+ <StyledNotificationMethodWrapper>
+ <NotificationMethod
+ setting={notificationSetting}
+ index={i}
+ key={`NotificationMethod-${i}`}
+ onUpdate={updateNotificationSetting}
+ onRemove={removeNotificationSetting}
+ />
+ </StyledNotificationMethodWrapper>
+ ))}
+ {
+ // Prohibit 'add notification method' button if only one present
+ allowedNotificationMethods.length > notificationSettings.length && (
+ <NotificationMethodAdd
+ data-test="notification-add"
+ status={notificationAddState}
+ onClick={onNotificationAdd}
+ />
+ )
+ }
+ </StyledPanel>
+ </Collapse>
</StyledModal>
);
};
diff --git a/superset-frontend/src/features/alerts/buildErrorTooltipMessage.test.tsx b/superset-frontend/src/features/alerts/buildErrorTooltipMessage.test.tsx
new file mode 100644
index 0000000000..c2463ac069
--- /dev/null
+++ b/superset-frontend/src/features/alerts/buildErrorTooltipMessage.test.tsx
@@ -0,0 +1,70 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { render, screen } from 'spec/helpers/testing-library';
+import { buildErrorTooltipMessage } from './buildErrorTooltipMessage';
+import { SectionValidationObject } from './types';
+
+const noErrors: SectionValidationObject = {
+ hasErrors: false,
+ errors: [],
+ name: 'No Errors',
+};
+
+const singleError: SectionValidationObject = {
+ hasErrors: true,
+ errors: ['first error'],
+ name: 'Single Error',
+};
+
+const threeErrors: SectionValidationObject = {
+ hasErrors: true,
+ errors: ['first error', 'second error', 'third error'],
+ name: 'Triple Error',
+};
+
+const validation = { noErrors, singleError, threeErrors };
+
+test('builds with proper heading', () => {
+ render(buildErrorTooltipMessage(validation));
+ const heading = screen.getByText(
+ /not all required fields are complete\. please provide the following:/i,
+ );
+ expect(heading).toBeInTheDocument();
+});
+
+test('only builds sections that have errors', async () => {
+ render(buildErrorTooltipMessage(validation));
+ const noErrors = screen.queryByText(/no errors: /i);
+ const singleError = screen.getByText(/single error:/i);
+ const tripleError = screen.getByText(/triple error:/i);
+ expect(noErrors).not.toBeInTheDocument();
+ expect(singleError).toBeInTheDocument();
+ expect(tripleError).toBeInTheDocument();
+});
+
+test('properly concatenates errors', async () => {
+ render(buildErrorTooltipMessage(validation));
+ const singleError = screen.getByText(/single error: first error/i);
+ const tripleError = screen.getByText(
+ /triple error: first error, second error, third error/i,
+ );
+ expect(singleError).toBeInTheDocument();
+ expect(tripleError).toBeInTheDocument();
+});
diff --git a/superset-frontend/src/features/alerts/buildErrorTooltipMessage.tsx b/superset-frontend/src/features/alerts/buildErrorTooltipMessage.tsx
new file mode 100644
index 0000000000..2011f5fe31
--- /dev/null
+++ b/superset-frontend/src/features/alerts/buildErrorTooltipMessage.tsx
@@ -0,0 +1,49 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import React from 'react';
+import { styled } from '@superset-ui/core';
+import { ValidationObject } from './types';
+import { TRANSLATIONS } from './AlertReportModal';
+
+const StyledList = styled.ul`
+ margin-left: ${({ theme }) => theme.gridUnit * 2}px;
+ padding-inline-start: ${({ theme }) => theme.gridUnit * 3}px;
+`;
+
+export const buildErrorTooltipMessage = (
+ validationStatus: ValidationObject,
+) => {
+ const sectionErrors: string[] = [];
+ Object.values(validationStatus).forEach(section => {
+ if (section.hasErrors) {
+ const sectionTitle = `${section.name}: `;
+ sectionErrors.push(sectionTitle + section.errors.join(', '));
+ }
+ });
+ return (
+ <div>
+ {TRANSLATIONS.ERROR_TOOLTIP_MESSAGE}
+ <StyledList>
+ {sectionErrors.map(err => (
+ <li key={err}>{err}</li>
+ ))}
+ </StyledList>
+ </div>
+ );
+};
diff --git a/superset-frontend/src/features/alerts/components/AlertReportCronScheduler.test.tsx b/superset-frontend/src/features/alerts/components/AlertReportCronScheduler.test.tsx
deleted file mode 100644
index 5d36c2994d..0000000000
--- a/superset-frontend/src/features/alerts/components/AlertReportCronScheduler.test.tsx
+++ /dev/null
@@ -1,153 +0,0 @@
-/**
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements. See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership. The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied. See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-import React from 'react';
-import { render, screen, waitFor, within } from 'spec/helpers/testing-library';
-import userEvent from '@testing-library/user-event';
-import { act } from 'react-dom/test-utils';
-
-import {
- AlertReportCronScheduler,
- AlertReportCronSchedulerProps,
-} from './AlertReportCronScheduler';
-
-const createProps = (props: Partial<AlertReportCronSchedulerProps> = {}) => ({
- onChange: jest.fn(),
- value: '* * * * *',
- ...props,
-});
-
-test('should render', () => {
- const props = createProps();
- render(<AlertReportCronScheduler {...props} />);
-
- // Text found in the first radio option
- expect(screen.getByText('Every')).toBeInTheDocument();
- // Text found in the second radio option
- expect(screen.getByText('CRON Schedule')).toBeInTheDocument();
-});
-
-test('only one radio option should be enabled at a time', () => {
- const props = createProps();
- const { container } = render(<AlertReportCronScheduler {...props} />);
-
- expect(screen.getByTestId('picker')).toBeChecked();
- expect(screen.getByTestId('input')).not.toBeChecked();
-
- const pickerContainer = container.querySelector(
- '.react-js-cron-select',
- ) as HTMLElement;
- const inputContainer = screen.getByTestId('input-content');
-
- expect(within(pickerContainer).getAllByRole('combobox')[0]).toBeEnabled();
- expect(inputContainer.querySelector('input[name="crontab"]')).toBeDisabled();
-
- userEvent.click(screen.getByTestId('input'));
-
- expect(within(pickerContainer).getAllByRole('combobox')[0]).toBeDisabled();
- expect(inputContainer.querySelector('input[name="crontab"]')).toBeEnabled();
-
- userEvent.click(screen.getByTestId('picker'));
-
- expect(within(pickerContainer).getAllByRole('combobox')[0]).toBeEnabled();
- expect(inputContainer.querySelector('input[name="crontab"]')).toBeDisabled();
-});
-
-test('picker mode updates correctly', async () => {
- const onChangeCallback = jest.fn();
- const props = createProps({
- onChange: onChangeCallback,
- });
-
- const { container } = render(<AlertReportCronScheduler {...props} />);
-
- expect(screen.getByTestId('picker')).toBeChecked();
-
- const pickerContainer = container.querySelector(
- '.react-js-cron-select',
- ) as HTMLElement;
-
- const firstSelect = within(pickerContainer).getAllByRole('combobox')[0];
- act(() => {
- userEvent.click(firstSelect);
- });
-
- expect(await within(pickerContainer).findByText('day')).toBeInTheDocument();
- act(() => {
- userEvent.click(within(pickerContainer).getByText('day'));
- });
-
- expect(onChangeCallback).toHaveBeenLastCalledWith('* * * * *');
-
- const secondSelect = container.querySelector(
- '.react-js-cron-hours .ant-select-selector',
- ) as HTMLElement;
- await waitFor(() => {
- expect(secondSelect).toBeInTheDocument();
- });
-
- act(() => {
- userEvent.click(secondSelect);
- });
-
- expect(await screen.findByText('9')).toBeInTheDocument();
- act(() => {
- userEvent.click(screen.getByText('9'));
- });
-
- await waitFor(() => {
- expect(onChangeCallback).toHaveBeenLastCalledWith('* 9 * * *');
- });
-});
-
-test('input mode updates correctly', async () => {
- const onChangeCallback = jest.fn();
- const props = createProps({
- onChange: onChangeCallback,
- });
-
- render(<AlertReportCronScheduler {...props} />);
-
- const inputContainer = screen.getByTestId('input-content');
- userEvent.click(screen.getByTestId('input'));
-
- const input = inputContainer.querySelector(
- 'input[name="crontab"]',
- ) as HTMLElement;
- await waitFor(() => {
- expect(input).toBeEnabled();
- });
-
- userEvent.clear(input);
- expect(input).toHaveValue('');
-
- const value = '* 10 2 * *';
- await act(async () => {
- await userEvent.type(input, value, { delay: 1 });
- });
-
- await waitFor(() => {
- expect(input).toHaveValue(value);
- });
-
- act(() => {
- userEvent.click(inputContainer);
- });
-
- expect(onChangeCallback).toHaveBeenLastCalledWith(value);
-});
diff --git a/superset-frontend/src/features/alerts/components/AlertReportCronScheduler.tsx b/superset-frontend/src/features/alerts/components/AlertReportCronScheduler.tsx
index 17652af510..821a91f8e6 100644
--- a/superset-frontend/src/features/alerts/components/AlertReportCronScheduler.tsx
+++ b/superset-frontend/src/features/alerts/components/AlertReportCronScheduler.tsx
@@ -19,9 +19,8 @@
import React, { useState, useCallback, useRef, FocusEvent } from 'react';
import { t, useTheme } from '@superset-ui/core';
-import { AntdInput, RadioChangeEvent } from 'src/components';
+import { AntdInput, Select } from 'src/components';
import { Input } from 'src/components/Input';
-import { Radio } from 'src/components/Radio';
import { CronPicker, CronError } from 'src/components/CronPicker';
import { StyledInputContainer } from '../AlertReportModal';
@@ -30,18 +29,29 @@ export interface AlertReportCronSchedulerProps {
onChange: (change: string) => any;
}
+enum ScheduleType {
+ Picker = 'picker',
+ Input = 'input',
+}
+
+const SCHEDULE_TYPE_OPTIONS = [
+ {
+ label: t('Recurring (every)'),
+ value: ScheduleType.Picker,
+ },
+ {
+ label: t('CRON Schedule'),
+ value: ScheduleType.Input,
+ },
+];
+
export const AlertReportCronScheduler: React.FC<
AlertReportCronSchedulerProps
> = ({ value, onChange }) => {
const theme = useTheme();
const inputRef = useRef<AntdInput>(null);
- const [scheduleFormat, setScheduleFormat] = useState<'picker' | 'input'>(
- 'picker',
- );
-
- const handleRadioButtonChange = useCallback(
- (e: RadioChangeEvent) => setScheduleFormat(e.target.value),
- [],
+ const [scheduleFormat, setScheduleFormat] = useState<ScheduleType>(
+ ScheduleType.Picker,
);
const customSetValue = useCallback(
@@ -67,40 +77,52 @@ export const AlertReportCronScheduler: React.FC<
return (
<>
- <Radio.Group onChange={handleRadioButtonChange} value={scheduleFormat}>
- <div className="inline-container add-margin">
- <Radio data-test="picker" value="picker" />
+ <StyledInputContainer>
+ <div className="control-label">
+ {t('Schedule type')}
+ <span className="required">*</span>
+ </div>
+ <div className="input-container">
+ <Select
+ ariaLabel={t('Schedule type')}
+ placeholder={t('Schedule type')}
+ onChange={(e: ScheduleType) => {
+ setScheduleFormat(e);
+ }}
+ value={scheduleFormat}
+ options={SCHEDULE_TYPE_OPTIONS}
+ />
+ </div>
+ </StyledInputContainer>
+
+ <StyledInputContainer data-test="input-content" className="styled-input">
+ <div className="control-label">
+ {t('Schedule')}
+ <span className="required">*</span>
+ </div>
+ {scheduleFormat === ScheduleType.Input && (
+ <Input
+ type="text"
+ name="crontab"
+ ref={inputRef}
+ style={error ? { borderColor: theme.colors.error.base } : {}}
+ placeholder={t('CRON expression')}
+ value={value}
+ onBlur={handleBlur}
+ onChange={e => customSetValue(e.target.value)}
+ onPressEnter={handlePressEnter}
+ />
+ )}
+ {scheduleFormat === ScheduleType.Picker && (
<CronPicker
clearButton={false}
value={value}
setValue={customSetValue}
- disabled={scheduleFormat !== 'picker'}
- displayError={scheduleFormat === 'picker'}
+ displayError={scheduleFormat === ScheduleType.Picker}
onError={onError}
/>
- </div>
- <div className="inline-container add-margin">
- <Radio data-test="input" value="input" />
- <span className="input-label">{t('CRON Schedule')}</span>
- <StyledInputContainer
- data-test="input-content"
- className="styled-input"
- >
- <div className="input-container">
- <Input
- type="text"
- name="crontab"
- ref={inputRef}
- style={error ? { borderColor: theme.colors.error.base } : {}}
- placeholder={t('CRON expression')}
- disabled={scheduleFormat !== 'input'}
- onBlur={handleBlur}
- onPressEnter={handlePressEnter}
- />
- </div>
- </StyledInputContainer>
- </div>
- </Radio.Group>
+ )}
+ </StyledInputContainer>
</>
);
};
diff --git a/superset-frontend/src/features/alerts/components/NotificationMethod.tsx b/superset-frontend/src/features/alerts/components/NotificationMethod.tsx
index 528c7011d7..1708c76f76 100644
--- a/superset-frontend/src/features/alerts/components/NotificationMethod.tsx
+++ b/superset-frontend/src/features/alerts/components/NotificationMethod.tsx
@@ -35,10 +35,6 @@ const StyledNotificationMethod = styled.div`
.inline-container {
margin-bottom: 10px;
- .input-container {
- margin-left: 10px;
- }
-
> div {
margin: 0;
}
@@ -119,6 +115,7 @@ export const NotificationMethod: FunctionComponent<NotificationMethodProps> = ({
<StyledNotificationMethod>
<div className="inline-container">
<StyledInputContainer>
+ <div className="control-label">{t('Notification Method')}</div>
<div className="input-container">
<Select
ariaLabel={t('Delivery method')}
@@ -133,25 +130,29 @@ export const NotificationMethod: FunctionComponent<NotificationMethodProps> = ({
)}
value={method}
/>
+ {method !== undefined && index !== 0 && !!onRemove ? (
+ <span
+ role="button"
+ tabIndex={0}
+ className="delete-button"
+ onClick={() => onRemove(index)}
+ >
+ <Icons.Trash iconColor={theme.colors.grayscale.base} />
+ </span>
+ ) : null}
</div>
</StyledInputContainer>
- {method !== undefined && !!onRemove ? (
- <span
- role="button"
- tabIndex={0}
- className="delete-button"
- onClick={() => onRemove(index)}
- >
- <Icons.Trash iconColor={theme.colors.grayscale.base} />
- </span>
- ) : null}
</div>
{method !== undefined ? (
<StyledInputContainer>
- <div className="control-label">{t(method)}</div>
+ <div className="control-label">
+ {t('%s recipients', method)}
+ <span className="required">*</span>
+ </div>
<div className="input-container">
<textarea
name="recipients"
+ data-test="recipients"
value={recipientValue}
onChange={onRecipientsChange}
/>
diff --git a/superset-frontend/src/features/alerts/components/NumberInput.tsx b/superset-frontend/src/features/alerts/components/NumberInput.tsx
new file mode 100644
index 0000000000..196335a588
--- /dev/null
+++ b/superset-frontend/src/features/alerts/components/NumberInput.tsx
@@ -0,0 +1,52 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import React, { useState, ChangeEvent } from 'react';
+
+interface NumberInputProps {
+ timeUnit: string;
+ min: number;
+ name: string;
+ value: string | number;
+ placeholder: string;
+ onChange: (event: ChangeEvent<HTMLInputElement>) => void;
+}
+
+export default function NumberInput({
+ timeUnit,
+ min,
+ name,
+ value,
+ placeholder,
+ onChange,
+}: NumberInputProps) {
+ const [isFocused, setIsFocused] = useState<boolean>(false);
+
+ return (
+ <input
+ type="text"
+ min={min}
+ name={name}
+ value={value ? `${value}${!isFocused ? ` ${timeUnit}` : ''}` : ''}
+ placeholder={placeholder}
+ onFocus={() => setIsFocused(true)}
+ onBlur={() => setIsFocused(false)}
+ onChange={onChange}
+ />
+ );
+}
diff --git a/superset-frontend/src/features/alerts/components/StyledPanel.tsx b/superset-frontend/src/features/alerts/components/StyledPanel.tsx
new file mode 100644
index 0000000000..339a6e1763
--- /dev/null
+++ b/superset-frontend/src/features/alerts/components/StyledPanel.tsx
@@ -0,0 +1,75 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import React from 'react';
+import { css, SupersetTheme } from '@superset-ui/core';
+import { Collapse as AntdCollapse } from 'antd';
+import { CollapsePanelProps } from 'antd/lib/collapse';
+
+const anticonHeight = 12;
+const antdPanelStyles = (theme: SupersetTheme) => css`
+ .ant-collapse-header {
+ display: flex;
+ flex-direction: column;
+ justify-content: center;
+ align-items: flex-start;
+ padding: 0px ${theme.gridUnit * 4}px;
+
+ .anticon.anticon-right.ant-collapse-arrow {
+ padding: 0;
+ top: calc(50% - ${anticonHeight / 2}px);
+ }
+
+ .collapse-panel-title {
+ font-size: ${theme.gridUnit * 4}px;
+ font-weight: ${theme.typography.weights.bold};
+ line-height: 130%;
+ }
+
+ .collapse-panel-subtitle {
+ color: ${theme.colors.grayscale.base};
+ font-size: ${theme.typography.sizes.s}px;
+ font-weight: ${theme.typography.weights.normal};
+ line-height: 150%;
+ margin-bottom: 0;
+ padding-top: ${theme.gridUnit}px;
+ }
+
+ .collapse-panel-asterisk {
+ color: var(--semantic-error-base, ${theme.colors.warning.dark1});
+ }
+ .validation-checkmark {
+ width: ${theme.gridUnit * 4}px;
+ height: ${theme.gridUnit * 4}px;
+ margin-left: ${theme.gridUnit}px;
+ color: ${theme.colors.success.base};
+ }
+ }
+`;
+
+export interface PanelProps extends CollapsePanelProps {
+ children?: React.ReactNode;
+}
+const StyledPanel = (props: PanelProps) => (
+ <AntdCollapse.Panel
+ css={(theme: SupersetTheme) => antdPanelStyles(theme)}
+ {...props}
+ />
+);
+
+export default StyledPanel;
diff --git a/superset-frontend/src/features/alerts/components/ValidatedPanelHeader.tsx b/superset-frontend/src/features/alerts/components/ValidatedPanelHeader.tsx
new file mode 100644
index 0000000000..016a387e6b
--- /dev/null
+++ b/superset-frontend/src/features/alerts/components/ValidatedPanelHeader.tsx
@@ -0,0 +1,53 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import React from 'react';
+import { t } from '@superset-ui/core';
+import { CheckCircleOutlined } from '@ant-design/icons';
+
+const ValidatedPanelHeader = ({
+ title,
+ subtitle,
+ validateCheckStatus,
+ testId,
+}: {
+ title: string;
+ subtitle: string;
+ validateCheckStatus: boolean;
+ testId?: string;
+}): JSX.Element => {
+ const checkmark = <CheckCircleOutlined />;
+
+ return (
+ <div className="collapse-panel-header">
+ <div className="collapse-panel-title" data-test={testId}>
+ <span>{t(title)}</span>
+ {validateCheckStatus ? (
+ <span className="validation-checkmark">{checkmark}</span>
+ ) : (
+ <span className="collapse-panel-asterisk">{' *'}</span>
+ )}
+ </div>
+ <p className="collapse-panel-subtitle">
+ {subtitle ? t(subtitle) : undefined}
+ </p>
+ </div>
+ );
+};
+
+export default ValidatedPanelHeader;
diff --git a/superset-frontend/src/features/alerts/types.ts b/superset-frontend/src/features/alerts/types.ts
index d8b166302c..c4264c7912 100644
--- a/superset-frontend/src/features/alerts/types.ts
+++ b/superset-frontend/src/features/alerts/types.ts
@@ -123,3 +123,21 @@ export interface AlertsReportsConfig {
ALERT_REPORTS_DEFAULT_RETENTION: number;
ALERT_REPORTS_DEFAULT_CRON_VALUE: string;
}
+
+export type SectionValidationObject = {
+ hasErrors: boolean;
+ errors: string[];
+ name: string;
+};
+
+export interface ValidationObject {
+ [key: string]: SectionValidationObject;
+}
+
+export enum Sections {
+ General = 'generalSection',
+ Content = 'contentSection',
+ Alert = 'alertConditionSection',
+ Schedule = 'scheduleSection',
+ Notification = 'notificationSection',
+}
diff --git a/superset-frontend/src/features/reports/ReportModal/index.tsx b/superset-frontend/src/features/reports/ReportModal/index.tsx
index 37833fedc8..e75d55302b 100644
--- a/superset-frontend/src/features/reports/ReportModal/index.tsx
+++ b/superset-frontend/src/features/reports/ReportModal/index.tsx
@@ -45,10 +45,7 @@ import {
NotificationFormats,
} from 'src/features/reports/types';
import { reportSelector } from 'src/views/CRUD/hooks';
-import {
- TRANSLATIONS,
- StyledInputContainer,
-} from 'src/features/alerts/AlertReportModal';
+import { StyledInputContainer } from 'src/features/alerts/AlertReportModal';
import { CreationMethod } from './HeaderReportDropdown';
import {
antDErrorAlertStyles,
@@ -270,14 +267,14 @@ function ReportModal({
const renderCustomWidthSection = (
<StyledInputContainer>
<div className="control-label" css={CustomWidthHeaderStyle}>
- {TRANSLATIONS.CUSTOM_SCREENSHOT_WIDTH_TEXT}
+ {t('Screenshot width')}
</div>
<div className="input-container">
<Input
type="number"
name="custom_width"
value={currentReport?.custom_width || ''}
- placeholder={TRANSLATIONS.CUSTOM_SCREENSHOT_WIDTH_PLACEHOLDER_TEXT}
+ placeholder={t('Input custom width in pixels')}
onChange={(event: React.ChangeEvent<HTMLInputElement>) => {
setCurrentReport({
custom_width: parseInt(event.target.value, 10) || null,
diff --git a/superset/config.py b/superset/config.py
index e8d98dbd1b..197e4bac42 100644
--- a/superset/config.py
+++ b/superset/config.py
@@ -1293,7 +1293,7 @@ ALERT_REPORTS_WORKING_SOFT_TIME_OUT_LAG = int(timedelta(seconds=1).total_seconds
# Default values that user using when creating alert
ALERT_REPORTS_DEFAULT_WORKING_TIMEOUT = 3600
ALERT_REPORTS_DEFAULT_RETENTION = 90
-ALERT_REPORTS_DEFAULT_CRON_VALUE = "0 * * * *" # every hour
+ALERT_REPORTS_DEFAULT_CRON_VALUE = "0 0 * * *" # every day
# If set to true no notification is sent, the worker will just log a message.
# Useful for debugging
ALERT_REPORTS_NOTIFICATION_DRY_RUN = False