From 413a2b19f549c5f2269811f2982d1b27f1eb0834 Mon Sep 17 00:00:00 2001 From: fisjac Date: Wed, 15 Nov 2023 22:31:01 -0600 Subject: [PATCH] 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', +}