From 76177efb7b698abe22c39307359a249c6a0e3668 Mon Sep 17 00:00:00 2001 From: fisjac Date: Thu, 9 Nov 2023 14:45:25 -0600 Subject: [PATCH 1/2] creating StyledPanel and CustomValidationHeader components for alerts&reports --- .../alerts/components/StyledPanel.tsx | 56 +++++++++++++++++++ .../components/ValidatedPanelHeader.tsx | 40 +++++++++++++ 2 files changed, 96 insertions(+) create mode 100644 superset-frontend/src/features/alerts/components/StyledPanel.tsx create mode 100644 superset-frontend/src/features/alerts/components/ValidatedPanelHeader.tsx 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 0000000000000..f9b81ab4259fa --- /dev/null +++ b/superset-frontend/src/features/alerts/components/StyledPanel.tsx @@ -0,0 +1,56 @@ +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; + } + + .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) => ( + 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 0000000000000..068c55b2b1214 --- /dev/null +++ b/superset-frontend/src/features/alerts/components/ValidatedPanelHeader.tsx @@ -0,0 +1,40 @@ +import React from 'react'; +import { t } from '@superset-ui/core'; +import { CheckCircleOutlined } from '@ant-design/icons'; + +const ValidatedPanelHeader = ({ + title, + subtitle, + required, + validateCheckStatus, +}: { + title: string; + subtitle: string; + required: boolean; + validateCheckStatus: boolean; +}): JSX.Element => { + let asterisk; + if (required) { + asterisk = ' *'; + } + + let checkmark; + if (validateCheckStatus) { + checkmark = ; + } + + return ( +
+
+ {t(title)} + {asterisk} + {checkmark} +
+

+ {subtitle ? t(subtitle) : undefined} +

+
+ ); +}; + +export default ValidatedPanelHeader; From 413a2b19f549c5f2269811f2982d1b27f1eb0834 Mon Sep 17 00:00:00 2001 From: fisjac Date: Wed, 15 Nov 2023 22:31:01 -0600 Subject: [PATCH 2/2] Splitting into sections and adding validation --- .../src/components/Button/index.tsx | 8 +- .../src/components/Modal/Modal.tsx | 3 + .../src/features/alerts/AlertReportModal.tsx | 592 ++++++++++++------ .../src/features/alerts/types.ts | 22 + 4 files changed, 422 insertions(+), 203 deletions(-) diff --git a/superset-frontend/src/components/Button/index.tsx b/superset-frontend/src/components/Button/index.tsx index 05a1a3ad79881..f3145112b0eb8 100644 --- a/superset-frontend/src/components/Button/index.tsx +++ b/superset-frontend/src/components/Button/index.tsx @@ -16,8 +16,8 @@ * 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 { kebabCase } from 'lodash'; import { mix } from 'polished'; import cx from 'classnames'; import { AntdButton } from 'src/components'; @@ -43,7 +43,7 @@ export type ButtonSize = 'default' | 'small' | 'xsmall'; export type ButtonProps = Omit & Pick & { - tooltip?: string; + tooltip?: ReactNode; className?: string; buttonSize?: ButtonSize; buttonStyle?: ButtonStyle; @@ -209,7 +209,7 @@ export default function Button(props: ButtonProps) { return ( {/* wrap the button in a span so that the tooltip shows up diff --git a/superset-frontend/src/components/Modal/Modal.tsx b/superset-frontend/src/components/Modal/Modal.tsx index fc9b82168397e..c037036eeb8c7 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, @@ -272,6 +274,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.tsx b/superset-frontend/src/features/alerts/AlertReportModal.tsx index 571c7b1b2a815..ecf64cf9cd69c 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, @@ -59,11 +60,16 @@ import { Operator, Recipient, AlertsReportsConfig, + ValidationObject, + Sections, } from 'src/features/alerts/types'; import { useSelector } from 'react-redux'; import { UserWithPermissionsAndRoles } from 'src/types/bootstrapTypes'; +import Collapse from 'src/components/Collapse'; import { AlertReportCronScheduler } from './components/AlertReportCronScheduler'; import { NotificationMethod } from './components/NotificationMethod'; +import StyledPanel from './components/StyledPanel'; +import ValidatedPanelHeader from './components/ValidatedPanelHeader'; const TIMEOUT_MIN = 1; const TEXT_BASED_VISUALIZATION_TYPES = [ @@ -469,6 +475,7 @@ const AlertReportModal: FunctionComponent = ({ conf?.ALERT_REPORTS_NOTIFICATION_METHODS || DEFAULT_NOTIFICATION_METHODS; const [disableSave, setDisableSave] = useState(true); + const [currentAlert, setCurrentAlert] = useState | null>(); const [isHidden, setIsHidden] = useState(true); @@ -491,7 +498,51 @@ const AlertReportModal: FunctionComponent = ({ const [sourceOptions, setSourceOptions] = useState([]); const [dashboardOptions, setDashboardOptions] = useState([]); const [chartOptions, setChartOptions] = useState([]); + // Validation + const [validationStatus, setValidationStatus] = useState({ + generalSection: { status: false, name: 'General information', errors: [] }, + contentSection: { status: false, name: 'Report contents', errors: [] }, + alertConditionSection: { + status: false, + name: 'Alert condition', + errors: [], + }, + scheduleSection: { status: false, name: 'Schedule', errors: [] }, + notificationSection: { + status: false, + name: 'Notification methods', + errors: [], + }, + }); + const [errorTooltipMessage, setErrorTooltipMessage] = useState(''); + const updateValidationStatus = ( + section: Sections, + status: boolean, + errors?: string[], + ) => { + if (status || (section === Sections.ALERT && isReport)) { + // clear set true and clear errors + setValidationStatus(currentValidationData => ({ + ...currentValidationData, + [section]: { + status: true, + name: currentValidationData[section].name, + errors: [], + }, + })); + } else { + // push errors + setValidationStatus(currentValidationData => ({ + ...currentValidationData, + [section]: { + status: false, + name: currentValidationData[section].name, + errors, + }, + })); + } + }; // Chart metadata const [chartVizType, setChartVizType] = useState(''); @@ -995,30 +1046,126 @@ const AlertReportModal: FunctionComponent = ({ return hasInfo; }; - const validate = () => { + const validateGeneralSection = () => { + const errors = []; + if (!currentAlert?.name?.length) { + errors.push('name'); + } + if (!currentAlert?.owners?.length) { + errors.push('owners'); + } + if (errors.length) { + updateValidationStatus(Sections.GENERAL, false, errors); + } else { + updateValidationStatus(Sections.GENERAL, true); + } + }; + 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('content type'); + } + if (errors.length) { + updateValidationStatus(Sections.CONTENT, false, errors); + } else { + updateValidationStatus(Sections.CONTENT, true); + } + }; + const validateAlertSection = () => { + const errors = []; + if (!currentAlert?.database) { + errors.push('database'); + } + if (!currentAlert?.sql?.length) { + errors.push('sql'); + } + if ( + !( + (conditionNotNull || !!currentAlert?.validator_config_json?.op) && (conditionNotNull || - currentAlert.validator_config_json?.threshold !== undefined) - ) { - setDisableSave(false); - } else { - setDisableSave(true); - } + currentAlert?.validator_config_json?.threshold !== undefined) + ) + ) { + errors.push('alert condition'); + } + if (errors.length) { + updateValidationStatus(Sections.ALERT, false, errors); } else { + updateValidationStatus(Sections.ALERT, true); + } + }; + const validateScheduleSection = () => { + const errors = []; + if (!currentAlert?.crontab?.length) { + errors.push('crontab'); + } + if (!currentAlert?.working_timeout) { + errors.push('working timeout'); + } + + if (errors.length) { + updateValidationStatus(Sections.SCHEDULE, false, errors); + } else { + updateValidationStatus(Sections.SCHEDULE, true); + } + }; + const validateNotificationSection = () => { + if (checkNotificationSettings()) { + updateValidationStatus(Sections.NOTIFICATION, true); + } else { + updateValidationStatus(Sections.NOTIFICATION, false, ['recipients']); + } + }; + + const validateAll = () => { + validateGeneralSection(); + validateContentSection(); + validateAlertSection(); + validateScheduleSection(); + validateNotificationSection(); + }; + + const buildErrorTooltipMessage = (build = true) => { + if (build) { + const sectionErrors: string[] = []; + Object.values(validationStatus).forEach(validationData => { + if (!validationData.status) { + const sectionTitle = `${validationData.name}: `; + sectionErrors.push(sectionTitle + validationData.errors.join(', ')); + } + }); + setErrorTooltipMessage( +
+ Not all required fields are complete. Please provide the following: +
    + {sectionErrors.map(err => ( +
  • {err}
    • + ))} +
+
, + ); + } else { + setErrorTooltipMessage(''); + } + }; + + const enforceValidation = () => { + if ( + validationStatus[Sections.GENERAL].status && + validationStatus[Sections.CONTENT].status && + (isReport || validationStatus[Sections.ALERT].status) && + validationStatus[Sections.SCHEDULE].status && + validationStatus[Sections.NOTIFICATION].status + ) { + buildErrorTooltipMessage(false); + setDisableSave(false); + } else { + buildErrorTooltipMessage(); setDisableSave(true); } }; @@ -1133,7 +1280,7 @@ const AlertReportModal: FunctionComponent = ({ // Validation const currentAlertSafe = currentAlert || {}; useEffect(() => { - validate(); + validateAll(); }, [ currentAlertSafe.name, currentAlertSafe.owners, @@ -1148,6 +1295,9 @@ const AlertReportModal: FunctionComponent = ({ notificationSettings, conditionNotNull, ]); + useEffect(() => { + enforceValidation(); + }, [validationStatus]); // Show/hide if (isHidden && show) { @@ -1159,6 +1309,7 @@ const AlertReportModal: FunctionComponent = ({ className="no-content-padding" responsive disablePrimaryButton={disableSave} + primaryTooltipMessage={errorTooltipMessage} onHandledPrimaryAction={onSave} onHide={hide} primaryButtonName={ @@ -1184,76 +1335,100 @@ const AlertReportModal: FunctionComponent = ({ } > - -
- -
- {isReport - ? TRANSLATIONS.REPORT_NAME_TEXT - : TRANSLATIONS.ALERT_NAME_TEXT} - * -
-
- -
- - -
- {TRANSLATIONS.OWNERS_TEXT} - * -
-
- + + } + key="1" + > +
+ +
+ {isReport + ? TRANSLATIONS.REPORT_NAME_TEXT + : TRANSLATIONS.ALERT_NAME_TEXT} + * +
+
+ +
+ + +
+ {TRANSLATIONS.OWNERS_TEXT} + * +
+
+ +
+ + +
+ {TRANSLATIONS.DESCRIPTION_TEXT} +
+
+ +
+ + + -
- - -
{TRANSLATIONS.DESCRIPTION_TEXT}
-
- {TRANSLATIONS.ACTIVE_TEXT}
+ +
+ + {!isReport && ( + -
- - - -
{TRANSLATIONS.ACTIVE_TEXT}
- - -
- {!isReport && ( + } + key="2" + >

{TRANSLATIONS.ALERT_CONDITION_TEXT}

@@ -1343,97 +1518,20 @@ const AlertReportModal: FunctionComponent = ({
- )} -
- -

- {isReport - ? TRANSLATIONS.REPORT_SCHEDULE_TEXT - : TRANSLATIONS.ALERT_CONDITION_SCHEDULE_TEXT} -

- * - - updateAlertState('crontab', newVal)} + + )} + -
{TRANSLATIONS.TIMEZONE_TEXT}
-
timezoneHeaderStyle(theme)} - > - -
- -

{TRANSLATIONS.SCHEDULE_SETTINGS_TEXT}

- - -
- {TRANSLATIONS.LOG_RETENTION_TEXT} - * -
-
- - {TRANSLATIONS.SECONDS_TEXT} -
- - {!isReport && ( - -
- {TRANSLATIONS.GRACE_PERIOD_TEXT} -
-
- - - {TRANSLATIONS.SECONDS_TEXT} - -
- - )} -
+ } + key="3" + >
- -

{TRANSLATIONS.MESSAGE_CONTENT_TEXT}

- * - {TRANSLATIONS.DASHBOARD_TEXT} @@ -1524,29 +1622,125 @@ const AlertReportModal: FunctionComponent = ({
)} - -

{TRANSLATIONS.NOTIFICATION_METHOD_TEXT}

- * - - {notificationSettings.map((notificationSetting, i) => ( - - - - ))} - + + + } + key="4" + > +
+ updateAlertState('crontab', newVal)} + /> +
{TRANSLATIONS.TIMEZONE_TEXT}
+
timezoneHeaderStyle(theme)} + > + +
+ +
+ {TRANSLATIONS.LOG_RETENTION_TEXT} + * +
+
+ + {TRANSLATIONS.SECONDS_TEXT} +
+ + {!isReport && ( + +
+ {TRANSLATIONS.GRACE_PERIOD_TEXT} +
+
+ + + {TRANSLATIONS.SECONDS_TEXT} + +
+ + )}
- - + + + } + key="5" + > + {notificationSettings.map((notificationSetting, i) => ( + + + + ))} + + + ); }; diff --git a/superset-frontend/src/features/alerts/types.ts b/superset-frontend/src/features/alerts/types.ts index e01cf6f438d07..77e09fd6f78c4 100644 --- a/superset-frontend/src/features/alerts/types.ts +++ b/superset-frontend/src/features/alerts/types.ts @@ -123,3 +123,25 @@ export interface AlertsReportsConfig { ALERT_REPORTS_DEFAULT_RETENTION: number; ALERT_REPORTS_DEFAULT_CRON_VALUE: string; } + +export type SectionValidationObject = { + status: boolean; + errors: string[]; + name: string; +}; + +export interface ValidationObject { + generalSection: SectionValidationObject; + contentSection: SectionValidationObject; + alertConditionSection: SectionValidationObject; + scheduleSection: SectionValidationObject; + notificationSection: SectionValidationObject; +} + +export enum Sections { + GENERAL = 'generalSection', + CONTENT = 'contentSection', + ALERT = 'alertConditionSection', + SCHEDULE = 'scheduleSection', + NOTIFICATION = 'notificationSection', +}