Lock Edit report source and Input Validation (opendistro-for-elastics…

…earch#225)
zhongnansu · Dec 4, 2020 · ba3ee0d · ba3ee0d
1 parent 38e63ee
commit ba3ee0d
Show file tree

Hide file tree

Showing 6 changed files with 178 additions and 104 deletions.
diff --git a/kibana-reports/public/components/report_definitions/create/create_report_definition.tsx b/kibana-reports/public/components/report_definitions/create/create_report_definition.tsx
@@ -28,13 +28,12 @@ import { ReportSettings } from '../report_settings';
 import { ReportDelivery } from '../delivery';
 import { ReportTrigger } from '../report_trigger';
 import { generateReportFromDefinitionId } from '../../main/main_utils';
-import { isValidCron } from 'cron-validator';
 import { converter } from '../utils';
-import moment from 'moment';
 import {
   permissionsMissingToast,
   permissionsMissingActions,
 } from '../../utils/utils';
+import { definitionInputValidation } from '../utils/utils';
 
 interface reportParamsType {
   report_name: string;
@@ -209,80 +208,6 @@ export function CreateReport(props) {
     timeTo: new Date(),
   };
 
-  const definitionInputValidation = async (metadata, error) => {
-    // check report name
-    // allow a-z, A-Z, 0-9, (), [], ',' - and _ and spaces
-    let regexp = /^[\w\-\s\(\)\[\]\,\_\-+]+$/;
-    if (metadata.report_params.report_name.search(regexp) === -1) {
-      setShowSettingsReportNameError(true);
-      if (metadata.report_params.report_name === '') {
-        setSettingsReportNameErrorMessage('Name must not be empty.');
-      } else {
-        setSettingsReportNameErrorMessage('Invalid characters in report name.');
-      }
-      error = true;
-    }
-
-    // if recurring by interval and input is not a number
-    if (
-      metadata.trigger.trigger_type === 'Schedule' &&
-      metadata.trigger.trigger_params.schedule_type === 'Recurring'
-    ) {
-      let interval = parseInt(
-        metadata.trigger.trigger_params.schedule.interval.period
-      );
-      if (isNaN(interval)) {
-        setShowTriggerIntervalNaNError(true);
-        error = true;
-      }
-    }
-
-    // if time range is invalid
-    const nowDate = new Date(moment.now());
-    if (timeRange.timeFrom > timeRange.timeTo || timeRange.timeTo > nowDate) {
-      setShowTimeRangeError(true);
-      error = true;
-    }
-
-    // if cron based and cron input is invalid
-    if (
-      metadata.trigger.trigger_type === 'Schedule' &&
-      metadata.trigger.trigger_params.schedule_type === 'Cron based'
-    ) {
-      if (
-        !isValidCron(metadata.trigger.trigger_params.schedule.cron.expression)
-      ) {
-        setShowCronError(true);
-        error = true;
-      }
-    }
-    // if email delivery
-    if (metadata.delivery.delivery_type === 'Channel') {
-      // no recipients are listed
-      if (metadata.delivery.delivery_params.recipients.length === 0) {
-        setShowEmailRecipientsError(true);
-        setEmailRecipientsErrorMessage(
-          'Email recipients list cannot be empty.'
-        );
-        error = true;
-      }
-      // recipients have invalid email addresses: regexp checks format xxxxx@yyyy.zzz
-      let emailRegExp = /\S+@\S+\.\S+/;
-      let index;
-      let recipients = metadata.delivery.delivery_params.recipients;
-      for (index = 0; index < recipients.length; ++index) {
-        if (recipients[0].search(emailRegExp) === -1) {
-          setShowEmailRecipientsError(true);
-          setEmailRecipientsErrorMessage(
-            'Invalid email addresses in recipients list.'
-          );
-          error = true;
-        }
-      }
-    }
-    return error;
-  };
-
   const createNewReportDefinition = async (
     metadata: reportDefinitionParams,
     timeRange: timeRangeParams
@@ -298,7 +223,18 @@ export function CreateReport(props) {
     }
 
     let error = false;
-    await definitionInputValidation(metadata, error).then((response) => {
+    await definitionInputValidation(
+      metadata, 
+      error,
+      setShowSettingsReportNameError,
+      setSettingsReportNameErrorMessage,
+      setShowTriggerIntervalNaNError,
+      timeRange,
+      setShowTimeRangeError,
+      setShowCronError,
+      setShowEmailRecipientsError,
+      setEmailRecipientsErrorMessage
+    ).then((response) => {
       error = response;
     });
     if (error) {

diff --git a/kibana-reports/public/components/report_definitions/edit/edit_report_definition.tsx b/kibana-reports/public/components/report_definitions/edit/edit_report_definition.tsx
@@ -34,12 +34,36 @@ import {
   permissionsMissingToast,
   permissionsMissingActions,
 } from '../../utils/utils';
+import { definitionInputValidation } from '../utils/utils';
 
 export function EditReportDefinition(props) {
   const [toasts, setToasts] = useState([]);
   const [comingFromError, setComingFromError] = useState(false);
   const [preErrorData, setPreErrorData] = useState({});
 
+  const [
+    showSettingsReportNameError,
+    setShowSettingsReportNameError,
+  ] = useState(false);
+  const [
+    settingsReportNameErrorMessage,
+    setSettingsReportNameErrorMessage,
+  ] = useState('');
+  const [
+    showTriggerIntervalNaNError,
+    setShowTriggerIntervalNaNError,
+  ] = useState(false);
+  const [showCronError, setShowCronError] = useState(false);
+  const [
+    showEmailRecipientsError, 
+    setShowEmailRecipientsError
+  ] = useState(false);
+  const [
+    emailRecipientsErrorMessage,
+    setEmailRecipientsErrorMessage,
+  ] = useState('');
+  const [showTimeRangeError, setShowTimeRangeError] = useState(false);
+
   const addPermissionsMissingViewEditPageToastHandler = (errorType: string) => {
     let toast = {};
     if (errorType === 'permissions') {
@@ -151,7 +175,6 @@ export function EditReportDefinition(props) {
 
   const callUpdateAPI = async (metadata) => {
     const { httpClient } = props;
-
     httpClient
       .put(`../api/reporting/reportDefinitions/${reportDefinitionId}`, {
         body: JSON.stringify(metadata),
@@ -175,7 +198,6 @@ export function EditReportDefinition(props) {
   };
 
   const editReportDefinition = async (metadata) => {
-    const { httpClient } = props;
     if ('header' in metadata.report_params.core_params) {
       metadata.report_params.core_params.header = converter.makeHtml(
         metadata.report_params.core_params.header
@@ -186,30 +208,27 @@ export function EditReportDefinition(props) {
         metadata.report_params.core_params.footer
       );
     }
-    /*
-      we check if this editing updates the trigger type from Schedule to On demand. 
-      If so, need to first delete the reportDefinition along with the scheduled job first, by calling the delete
-      report definition API
-    */
-    const {
-      trigger: { trigger_type: triggerType },
-    } = reportDefinition;
-    if (
-      triggerType !== 'On demand' &&
-      metadata.trigger.trigger_type === 'On demand'
-    ) {
-      httpClient
-        .delete(`../api/reporting/reportDefinitions/${reportDefinitionId}`)
-        .then(async () => {
-          await callUpdateAPI(metadata);
-        })
-        .catch((error) => {
-          console.log(
-            'error when deleting old scheduled report definition:',
-            error
-          );
-          handleErrorDeletingReportDefinitionToast();
-        });
+
+    // client-side input validation
+    let error = false;
+    await definitionInputValidation(
+      metadata,
+      error,
+      setShowSettingsReportNameError,
+      setSettingsReportNameErrorMessage,
+      setShowTriggerIntervalNaNError,
+      timeRange,
+      setShowTimeRangeError,
+      setShowCronError,
+      setShowEmailRecipientsError,
+      setEmailRecipientsErrorMessage
+    ).then((response) => {
+      error = response;
+    });
+    if (error) {
+      handleInputValidationErrorToast();
+      setPreErrorData(metadata);
+      setComingFromError(true);
     } else {
       await callUpdateAPI(metadata);
     }
@@ -273,6 +292,9 @@ export function EditReportDefinition(props) {
           reportDefinitionRequest={editReportDefinitionRequest}
           httpClientProps={props['httpClient']}
           timeRange={timeRange}
+          showSettingsReportNameError={showSettingsReportNameError}
+          settingsReportNameErrorMessage={settingsReportNameErrorMessage}
+          showTimeRangeError={showTimeRangeError}
         />
         <EuiSpacer />
         <ReportTrigger
@@ -281,6 +303,8 @@ export function EditReportDefinition(props) {
           reportDefinitionRequest={editReportDefinitionRequest}
           httpClientProps={props['httpClient']}
           timeRange={timeRange}
+          showTriggerIntervalNaNError={showTriggerIntervalNaNError}
+          showCronError={showCronError}
         />
         <EuiSpacer />
         <ReportDelivery
@@ -289,6 +313,8 @@ export function EditReportDefinition(props) {
           reportDefinitionRequest={editReportDefinitionRequest}
           httpClientProps={props['httpClient']}
           timeRange={timeRange}
+          showEmailRecipientsError={showEmailRecipientsError}
+          emailRecipientsErrorMessage={emailRecipientsErrorMessage}
         />
         <EuiSpacer />
         <EuiFlexGroup justifyContent="flexEnd">

diff --git a/.../report_definitions/report_settings/__tests__/__snapshots__/report_settings.test.tsx.snap b/.../report_definitions/report_settings/__tests__/__snapshots__/report_settings.test.tsx.snap
@@ -2748,6 +2748,7 @@ exports[`<ReportSettings /> panel render edit, dashboard source 1`] = `
             <input
               checked=""
               class="euiRadio__input"
+              disabled=""
               id="dashboardReportSource"
               type="radio"
               value=""
@@ -2767,6 +2768,7 @@ exports[`<ReportSettings /> panel render edit, dashboard source 1`] = `
           >
             <input
               class="euiRadio__input"
+              disabled=""
               id="visualizationReportSource"
               type="radio"
               value=""
@@ -2786,6 +2788,7 @@ exports[`<ReportSettings /> panel render edit, dashboard source 1`] = `
           >
             <input
               class="euiRadio__input"
+              disabled=""
               id="savedSearchReportSource"
               type="radio"
               value=""
@@ -3248,6 +3251,7 @@ exports[`<ReportSettings /> panel render edit, saved search source 1`] = `
             <input
               checked=""
               class="euiRadio__input"
+              disabled=""
               id="dashboardReportSource"
               type="radio"
               value=""
@@ -3267,6 +3271,7 @@ exports[`<ReportSettings /> panel render edit, saved search source 1`] = `
           >
             <input
               class="euiRadio__input"
+              disabled=""
               id="visualizationReportSource"
               type="radio"
               value=""
@@ -3286,6 +3291,7 @@ exports[`<ReportSettings /> panel render edit, saved search source 1`] = `
           >
             <input
               class="euiRadio__input"
+              disabled=""
               id="savedSearchReportSource"
               type="radio"
               value=""
@@ -3748,6 +3754,7 @@ exports[`<ReportSettings /> panel render edit, visualization source 1`] = `
             <input
               checked=""
               class="euiRadio__input"
+              disabled=""
               id="dashboardReportSource"
               type="radio"
               value=""
@@ -3767,6 +3774,7 @@ exports[`<ReportSettings /> panel render edit, visualization source 1`] = `
           >
             <input
               class="euiRadio__input"
+              disabled=""
               id="visualizationReportSource"
               type="radio"
               value=""
@@ -3786,6 +3794,7 @@ exports[`<ReportSettings /> panel render edit, visualization source 1`] = `
           >
             <input
               class="euiRadio__input"
+              disabled=""
               id="savedSearchReportSource"
               type="radio"
               value=""

diff --git a/kibana-reports/public/components/report_definitions/report_settings/report_settings.tsx b/kibana-reports/public/components/report_definitions/report_settings/report_settings.tsx
@@ -672,6 +672,7 @@ export function ReportSettings(props: ReportSettingProps) {
             options={REPORT_SOURCE_RADIOS}
             idSelected={reportSourceId}
             onChange={handleReportSource}
+            disabled={edit}
           />
         </EuiFormRow>
         <EuiSpacer />

diff --git a/kibana-reports/public/components/report_definitions/report_settings/time_range.tsx b/kibana-reports/public/components/report_definitions/report_settings/time_range.tsx
@@ -71,7 +71,7 @@ export function TimeRangeSelect(props) {
       if (
         !timeRangeMoment ||
         !timeRangeMoment.isValid() ||
-        timeRangeMoment > moment()
+        timeRangeMoment > moment.now()
       ) {
         handleInvalidTimeRangeToast();
       }