From 261a844b274e182dc90ed82a58ac313882d7a8f6 Mon Sep 17 00:00:00 2001
From: Phillip Burch <phillip.burch@live.com>
Date: Mon, 11 May 2020 15:01:38 -0500
Subject: [PATCH] Cleanup prefill and edit flow.

---
 .../components/expression.tsx                 | 90 ++++++++++---------
 .../infra/server/lib/alerting/common/utils.ts |  5 ++
 2 files changed, 53 insertions(+), 42 deletions(-)

diff --git a/x-pack/plugins/infra/public/alerting/metric_threshold/components/expression.tsx b/x-pack/plugins/infra/public/alerting/metric_threshold/components/expression.tsx
index 406f9c7602d35..8fdba86f233d4 100644
--- a/x-pack/plugins/infra/public/alerting/metric_threshold/components/expression.tsx
+++ b/x-pack/plugins/infra/public/alerting/metric_threshold/components/expression.tsx
@@ -74,6 +74,7 @@ export const Expressions: React.FC<Props> = props => {
     fetch: alertsContext.http.fetch,
     toastWarning: alertsContext.toastNotifications.addWarning,
   });
+
   const [timeSize, setTimeSize] = useState<number | undefined>(1);
   const [timeUnit, setTimeUnit] = useState<TimeUnit>('m');
   const derivedIndexPattern = useMemo(() => createDerivedIndexPattern('metrics'), [
@@ -173,52 +174,57 @@ export const Expressions: React.FC<Props> = props => {
     [alertParams.criteria, setAlertParams]
   );
 
-  useEffect(() => {
+  const preFillAlertCriteria = useCallback(() => {
     const md = alertsContext.metadata;
-    if (md) {
-      if (md.currentOptions?.metrics) {
-        setAlertParams(
-          'criteria',
-          md.currentOptions.metrics.map(metric => ({
-            metric: metric.field,
-            comparator: Comparator.GT,
-            threshold: [],
-            timeSize,
-            timeUnit,
-            aggType: metric.aggregation,
-          }))
-        );
-      } else {
-        setAlertParams('criteria', [defaultExpression]);
-      }
+    if (md && md.currentOptions?.metrics) {
+      setAlertParams(
+        'criteria',
+        md.currentOptions.metrics.map(metric => ({
+          metric: metric.field,
+          comparator: Comparator.GT,
+          threshold: [],
+          timeSize,
+          timeUnit,
+          aggType: metric.aggregation,
+        }))
+      );
+    } else {
+      setAlertParams('criteria', [defaultExpression]);
+    }
+  }, [alertsContext.metadata, setAlertParams, timeSize, timeUnit]);
 
-      if (md.currentOptions) {
-        if (md.currentOptions.filterQuery) {
-          setAlertParams('filterQueryText', md.currentOptions.filterQuery);
-          setAlertParams(
-            'filterQuery',
-            convertKueryToElasticSearchQuery(md.currentOptions.filterQuery, derivedIndexPattern) ||
-              ''
-          );
-        } else if (md.currentOptions.groupBy && md.series) {
-          const filter = `${md.currentOptions.groupBy}: "${md.series.id}"`;
-          setAlertParams('filterQueryText', filter);
-          setAlertParams(
-            'filterQuery',
-            convertKueryToElasticSearchQuery(filter, derivedIndexPattern) || ''
-          );
-        }
+  const preFillAlertFilter = useCallback(() => {
+    const md = alertsContext.metadata;
+    if (md && md.currentOptions?.filterQuery) {
+      setAlertParams('filterQueryText', md.currentOptions.filterQuery);
+      setAlertParams(
+        'filterQuery',
+        convertKueryToElasticSearchQuery(md.currentOptions.filterQuery, derivedIndexPattern) || ''
+      );
+    } else if (md && md.currentOptions?.groupBy && md.series) {
+      const filter = `${md.currentOptions?.groupBy}: "${md.series.id}"`;
+      setAlertParams('filterQueryText', filter);
+      setAlertParams(
+        'filterQuery',
+        convertKueryToElasticSearchQuery(filter, derivedIndexPattern) || ''
+      );
+    }
+  }, [alertsContext.metadata, derivedIndexPattern, setAlertParams]);
 
-        setAlertParams('groupBy', md.currentOptions.groupBy);
-      }
-      setAlertParams('sourceId', source?.id);
+  useEffect(() => {
+    if (alertParams.criteria && alertParams.criteria.length) {
+      setTimeSize(alertParams.criteria[0].timeSize);
+      setTimeUnit(alertParams.criteria[0].timeUnit);
     } else {
-      if (!alertParams.criteria) {
-        setAlertParams('criteria', [defaultExpression]);
-      }
-      if (!alertParams.sourceId) {
-        setAlertParams('sourceId', source?.id || 'default');
-      }
+      preFillAlertCriteria();
+    }
+
+    if (!alertParams.filterQuery) {
+      preFillAlertFilter();
+    }
+
+    if (!alertParams.sourceId) {
+      setAlertParams('sourceId', source?.id || 'default');
     }
   }, [alertsContext.metadata, defaultExpression, source]); // eslint-disable-line react-hooks/exhaustive-deps
 
diff --git a/x-pack/plugins/infra/server/lib/alerting/common/utils.ts b/x-pack/plugins/infra/server/lib/alerting/common/utils.ts
index a2c5b27c38fd6..5ca65b667ae11 100644
--- a/x-pack/plugins/infra/server/lib/alerting/common/utils.ts
+++ b/x-pack/plugins/infra/server/lib/alerting/common/utils.ts
@@ -13,6 +13,11 @@ export const oneOfLiterals = (arrayOfLiterals: Readonly<string[]>) =>
   });
 
 export const validateIsStringElasticsearchJSONFilter = (value: string) => {
+  if (value === '') {
+    // Allow clearing the filter.
+    return;
+  }
+
   const errorMessage = 'filterQuery must be a valid Elasticsearch filter expressed in JSON';
   try {
     const parsedValue = JSON.parse(value);