kgabryje commented on code in PR #26202:
URL: https://github.com/apache/superset/pull/26202#discussion_r1494625377
##########
superset-frontend/src/features/alerts/AlertReportModal.tsx:
##########
@@ -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}
Review Comment:
Nit: instead of all that logic for adding `noMarginBottom` styles, could we
maybe just use something like `:last-child` in `StyledInputContainer`? That
would simplify the logic and we could get rid of `noMarginBottom` everywhere.
If you agree, I'm ok with doing that as a cleanup follow up
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
