From 020aa9adddf6a310ee54a65e9248272ec12b72d4 Mon Sep 17 00:00:00 2001
From: Eric P Green <ericpgreen2@gmail.com>
Date: Thu, 29 Feb 2024 14:08:52 -0800
Subject: [PATCH 1/4] Handle incomplete criteria

---
 .../src/features/alerts/PreviewEmpty.svelte   | 16 ++++
 .../AlertDialogCriteriaTab.svelte             |  4 +-
 .../alerts/criteria-tab/AlertPreview.svelte   | 55 ++++++++++++
 .../alerts/data-tab/AlertDialogDataTab.svelte |  6 +-
 .../DataPreview.svelte}                       | 23 ++---
 .../dashboards/stores/filter-utils.spec.ts    | 85 +++++++++++++++++++
 .../dashboards/stores/filter-utils.ts         | 47 +++++++++-
 7 files changed, 215 insertions(+), 21 deletions(-)
 create mode 100644 web-common/src/features/alerts/PreviewEmpty.svelte
 create mode 100644 web-common/src/features/alerts/criteria-tab/AlertPreview.svelte
 rename web-common/src/features/alerts/{AlertDataPreview.svelte => data-tab/DataPreview.svelte} (62%)
 create mode 100644 web-common/src/features/dashboards/stores/filter-utils.spec.ts

diff --git a/web-common/src/features/alerts/PreviewEmpty.svelte b/web-common/src/features/alerts/PreviewEmpty.svelte
new file mode 100644
index 00000000000..1f69e656f72
--- /dev/null
+++ b/web-common/src/features/alerts/PreviewEmpty.svelte
@@ -0,0 +1,16 @@
+<script lang="ts">
+  import TableIcon from "../../components/icons/TableIcon.svelte";
+
+  export let topLine: string;
+  export let bottomLine: string;
+</script>
+
+<div class="pt-5 pb-10 flex flex-col justify-center items-center gap-1">
+  <TableIcon size="32px" className="text-slate-300" />
+  <div class="flex flex-col justify-center items-center text-sm">
+    <div class="text-gray-600 font-semibold">{topLine}</div>
+    <div class="text-gray-500 font-normal">
+      {bottomLine}
+    </div>
+  </div>
+</div>
diff --git a/web-common/src/features/alerts/criteria-tab/AlertDialogCriteriaTab.svelte b/web-common/src/features/alerts/criteria-tab/AlertDialogCriteriaTab.svelte
index fb00303de64..a7cfc7ceccb 100644
--- a/web-common/src/features/alerts/criteria-tab/AlertDialogCriteriaTab.svelte
+++ b/web-common/src/features/alerts/criteria-tab/AlertDialogCriteriaTab.svelte
@@ -1,8 +1,8 @@
 <script lang="ts">
   import FormSection from "@rilldata/web-common/components/forms/FormSection.svelte";
   import { translateFilter } from "@rilldata/web-common/features/alerts/alert-filter-utils";
+  import AlertPreview from "@rilldata/web-common/features/alerts/criteria-tab/AlertPreview.svelte";
   import CriteriaGroup from "@rilldata/web-common/features/alerts/criteria-tab/CriteriaGroup.svelte";
-  import AlertDataPreview from "@rilldata/web-common/features/alerts/AlertDataPreview.svelte";
 
   export let formState: any; // svelte-forms-lib's FormState
 
@@ -17,7 +17,7 @@
     <CriteriaGroup {formState} />
   </FormSection>
   <FormSection title="Alert Preview">
-    <AlertDataPreview
+    <AlertPreview
       criteria={translateFilter($form["criteria"], $form["criteriaOperation"])}
       measure={$form["measure"]}
       metricsViewName={$form["metricsViewName"]}
diff --git a/web-common/src/features/alerts/criteria-tab/AlertPreview.svelte b/web-common/src/features/alerts/criteria-tab/AlertPreview.svelte
new file mode 100644
index 00000000000..58a7fdfe740
--- /dev/null
+++ b/web-common/src/features/alerts/criteria-tab/AlertPreview.svelte
@@ -0,0 +1,55 @@
+<script lang="ts">
+  import { getAlertPreviewData } from "@rilldata/web-common/features/alerts/alert-preview-data";
+  import Spinner from "@rilldata/web-common/features/entity-management/Spinner.svelte";
+  import { EntityStatus } from "@rilldata/web-common/features/entity-management/types";
+  import { useQueryClient } from "@tanstack/svelte-query";
+  import PreviewTable from "../../../components/preview-table/PreviewTable.svelte";
+  import type { V1Expression, V1TimeRange } from "../../../runtime-client";
+  import { isExpressionIncomplete } from "../../dashboards/stores/filter-utils";
+  import PreviewEmpty from "../PreviewEmpty.svelte";
+
+  export let metricsViewName: string;
+  export let measure: string;
+  export let splitByDimension: string;
+  export let splitByTimeGrain: string;
+  export let whereFilter: V1Expression;
+  export let criteria: V1Expression;
+  export let timeRange: V1TimeRange;
+
+  const queryClient = useQueryClient();
+
+  $: alertPreviewQuery = getAlertPreviewData(queryClient, {
+    metricsViewName,
+    measure,
+    splitByDimension,
+    splitByTimeGrain,
+    whereFilter,
+    criteria,
+    timeRange,
+  });
+
+  $: isCriteriaEmpty = isExpressionIncomplete(criteria);
+</script>
+
+{#if $alertPreviewQuery.isFetching}
+  <div class="p-2 flex flex-col justify-center">
+    <Spinner status={EntityStatus.Running} />
+  </div>
+{:else if isCriteriaEmpty || !$alertPreviewQuery.data}
+  <PreviewEmpty
+    topLine="No criteria selected"
+    bottomLine="Select criteria to see a preview"
+  />
+{:else if $alertPreviewQuery.data.rows.length > 0}
+  <div class="max-h-64 overflow-auto">
+    <PreviewTable
+      rows={$alertPreviewQuery.data.rows}
+      columnNames={$alertPreviewQuery.data.schema}
+    />
+  </div>
+{:else}
+  <div>
+    Given the above criteria, this alert will not trigger for the current time
+    range.
+  </div>
+{/if}
diff --git a/web-common/src/features/alerts/data-tab/AlertDialogDataTab.svelte b/web-common/src/features/alerts/data-tab/AlertDialogDataTab.svelte
index 77b65dfa49a..e81b19502d8 100644
--- a/web-common/src/features/alerts/data-tab/AlertDialogDataTab.svelte
+++ b/web-common/src/features/alerts/data-tab/AlertDialogDataTab.svelte
@@ -1,7 +1,6 @@
 <script lang="ts">
-  import { translateFilter } from "@rilldata/web-common/features/alerts/alert-filter-utils";
+  import DataPreview from "@rilldata/web-common/features/alerts/data-tab/DataPreview.svelte";
   import { AlertIntervalOptions } from "@rilldata/web-common/features/alerts/data-tab/intervals";
-  import AlertDataPreview from "web-common/src/features/alerts/AlertDataPreview.svelte";
   import FormSection from "../../../components/forms/FormSection.svelte";
   import InputV2 from "../../../components/forms/InputV2.svelte";
   import Select from "../../../components/forms/Select.svelte";
@@ -91,8 +90,7 @@
     />
   </FormSection>
   <FormSection title="Data preview">
-    <AlertDataPreview
-      criteria={translateFilter($form["criteria"], $form["criteriaOperation"])}
+    <DataPreview
       measure={$form["measure"]}
       metricsViewName={$form["metricsViewName"]}
       splitByDimension={$form["splitByDimension"]}
diff --git a/web-common/src/features/alerts/AlertDataPreview.svelte b/web-common/src/features/alerts/data-tab/DataPreview.svelte
similarity index 62%
rename from web-common/src/features/alerts/AlertDataPreview.svelte
rename to web-common/src/features/alerts/data-tab/DataPreview.svelte
index 3ca302b23cc..40b2babde31 100644
--- a/web-common/src/features/alerts/AlertDataPreview.svelte
+++ b/web-common/src/features/alerts/data-tab/DataPreview.svelte
@@ -3,16 +3,16 @@
   import Spinner from "@rilldata/web-common/features/entity-management/Spinner.svelte";
   import { EntityStatus } from "@rilldata/web-common/features/entity-management/types";
   import { useQueryClient } from "@tanstack/svelte-query";
-  import TableIcon from "../../components/icons/TableIcon.svelte";
-  import PreviewTable from "../../components/preview-table/PreviewTable.svelte";
-  import type { V1Expression, V1TimeRange } from "../../runtime-client";
+
+  import PreviewTable from "../../../components/preview-table/PreviewTable.svelte";
+  import type { V1Expression, V1TimeRange } from "../../../runtime-client";
+  import PreviewEmpty from "../PreviewEmpty.svelte";
 
   export let metricsViewName: string;
   export let measure: string;
   export let splitByDimension: string;
   export let splitByTimeGrain: string;
   export let whereFilter: V1Expression;
-  export let criteria: V1Expression | undefined = undefined;
   export let timeRange: V1TimeRange;
 
   const queryClient = useQueryClient();
@@ -23,7 +23,7 @@
     splitByDimension,
     splitByTimeGrain,
     whereFilter,
-    criteria,
+    criteria: undefined,
     timeRange,
   });
 </script>
@@ -33,15 +33,10 @@
     <Spinner status={EntityStatus.Running} />
   </div>
 {:else if !$alertPreviewQuery.data}
-  <div class="pt-5 pb-10 flex flex-col justify-center items-center gap-1">
-    <TableIcon size="32px" className="text-slate-300" />
-    <div class="flex flex-col justify-center items-center text-sm">
-      <div class="text-gray-600 font-semibold">No data to preview</div>
-      <div class="text-gray-500 font-normal">
-        To see preview, select measures above.
-      </div>
-    </div>
-  </div>
+  <PreviewEmpty
+    topLine="No data to preview"
+    bottomLine="To see a preview, select measures above."
+  />
 {:else}
   <div class="max-h-64 overflow-auto">
     <PreviewTable
diff --git a/web-common/src/features/dashboards/stores/filter-utils.spec.ts b/web-common/src/features/dashboards/stores/filter-utils.spec.ts
new file mode 100644
index 00000000000..58dbdf77733
--- /dev/null
+++ b/web-common/src/features/dashboards/stores/filter-utils.spec.ts
@@ -0,0 +1,85 @@
+import { describe, expect, it } from "vitest";
+import { V1Operation } from "../../../runtime-client";
+import { isExpressionIncomplete } from "./filter-utils";
+
+// Mock data for test cases
+const testCases = [
+  {
+    description: "Simple complete expression with a single condition",
+    criteria: {
+      cond: {
+        op: V1Operation.OPERATION_AND,
+        exprs: [{ val: "1", ident: "measure_0" }],
+      },
+    },
+    incomplete: false,
+  },
+  {
+    description: "Simple incomplete expression with a missing val",
+    criteria: {
+      cond: { op: V1Operation.OPERATION_AND, exprs: [{ ident: "measure_0" }] },
+    },
+    incomplete: true,
+  },
+  {
+    description: "Incomplete expression with an unspecified operation",
+    criteria: { cond: { exprs: [{ val: "1", ident: "measure_0" }] } },
+    incomplete: true,
+  },
+  {
+    description: "Incomplete expression with an empty string val",
+    criteria: {
+      cond: {
+        op: V1Operation.OPERATION_AND,
+        exprs: [{ val: "", ident: "measure_0" }],
+      },
+    },
+    incomplete: true,
+  },
+  {
+    description:
+      "Nested complete expression with one incomplete nested condition",
+    criteria: {
+      cond: {
+        op: V1Operation.OPERATION_AND,
+        exprs: [
+          {
+            cond: {
+              op: V1Operation.OPERATION_OR,
+              exprs: [{ val: "1", ident: "measure_0" }, { ident: "measure_1" }],
+            },
+          },
+        ],
+      },
+    },
+    incomplete: false,
+  },
+  {
+    description:
+      "Nested incomplete expression with unspecified operation in nested condition",
+    criteria: {
+      cond: {
+        op: V1Operation.OPERATION_AND,
+        exprs: [
+          {
+            cond: {
+              exprs: [{ val: "1", ident: "measure_0" }, { ident: "measure_1" }],
+            },
+          },
+        ],
+      },
+    },
+    incomplete: true,
+  },
+  // Add more test cases as needed
+];
+
+// Test suite
+describe("isExpressionIncomplete", () => {
+  testCases.forEach((testCase, index) => {
+    it(`Test case ${index + 1}: ${testCase.description}`, () => {
+      const result = isExpressionIncomplete(testCase.criteria);
+      expect(result).toBe(testCase.incomplete);
+    });
+  });
+});
diff --git a/web-common/src/features/dashboards/stores/filter-utils.ts b/web-common/src/features/dashboards/stores/filter-utils.ts
index 5a74ec7f903..53a3c0b7de8 100644
--- a/web-common/src/features/dashboards/stores/filter-utils.ts
+++ b/web-common/src/features/dashboards/stores/filter-utils.ts
@@ -1,6 +1,7 @@
 import {
-  type V1Expression,
+  V1Condition,
   V1Operation,
+  type V1Expression,
 } from "@rilldata/web-common/runtime-client";
 
 export function createLikeExpression(
@@ -253,3 +254,47 @@ export const sanitiseExpression = (
   if (!where?.cond?.exprs?.length) return undefined;
   return where;
 };
+
+export function isExpressionIncomplete(expression: V1Expression): boolean {
+  // Check if the operation is unspecified at any level of the condition.
+  function isOperationUnspecified(cond: V1Condition): boolean {
+    if (
+      cond.op === V1Operation.OPERATION_UNSPECIFIED ||
+      cond.op === undefined
+    ) {
+      return true;
+    }
+    // Check nested conditions
+    return (
+      cond.exprs?.some(
+        (expr) => expr.cond && isOperationUnspecified(expr.cond),
+      ) ?? false
+    );
+  }
+
+  // Check if the val is defined and non-empty at any level of the nested expressions.
+  function isValDefinedAndNonEmpty(expr: V1Expression): boolean {
+    if (expr.val !== undefined && expr.val !== "") {
+      return true; // val is defined and non-empty
+    }
+    // If there is a nested condition, check if any nested expression has a defined and non-empty val
+    return (
+      expr.cond?.exprs?.some((nestedExpr) =>
+        isValDefinedAndNonEmpty(nestedExpr),
+      ) ?? false
+    );
+  }
+
+  // Check the top-level expression's operation
+  if (expression.cond && isOperationUnspecified(expression.cond)) {
+    return true; // The top-level operation is unspecified, thus incomplete
+  }
+
+  // If there's no val at the top level, check nested expressions
+  if (!isValDefinedAndNonEmpty(expression)) {
+    return true; // No defined and non-empty val found in any expressions, thus incomplete
+  }
+
+  // If the operation is specified and a defined, non-empty val is found, the expression is complete
+  return false;
+}

From cbbdb438a5618bcae38bf1c985091690cb6a86aa Mon Sep 17 00:00:00 2001
From: Eric P Green <ericpgreen2@gmail.com>
Date: Thu, 29 Feb 2024 14:09:04 -0800
Subject: [PATCH 2/4] Capitalization

---
 .../src/features/alerts/criteria-tab/CriteriaForm.svelte    | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/web-common/src/features/alerts/criteria-tab/CriteriaForm.svelte b/web-common/src/features/alerts/criteria-tab/CriteriaForm.svelte
index ac24ab31f14..d5dae1fb942 100644
--- a/web-common/src/features/alerts/criteria-tab/CriteriaForm.svelte
+++ b/web-common/src/features/alerts/criteria-tab/CriteriaForm.svelte
@@ -2,9 +2,9 @@
   import InputV2 from "@rilldata/web-common/components/forms/InputV2.svelte";
   import Select from "@rilldata/web-common/components/forms/Select.svelte";
   import { CriteriaOperationOptions } from "@rilldata/web-common/features/alerts/criteria-tab/operations";
-  import { runtime } from "../../../runtime-client/runtime-store";
   import { useMetricsView } from "@rilldata/web-common/features/dashboards/selectors";
   import { debounce } from "@rilldata/web-common/lib/create-debouncer";
+  import { runtime } from "../../../runtime-client/runtime-store";
 
   export let formState: any; // svelte-forms-lib's FormState
   export let index: number;
@@ -51,9 +51,9 @@
   <Select
     id="compareWith"
     label=""
-    options={[{ value: "value" }]}
+    options={[{ value: "Value" }]}
     placeholder="compare with"
-    value={"value"}
+    value={"Value"}
   />
   <InputV2
     bind:value

From 2309a396c35c3a6771c32ee5d86c61ff8ed76700 Mon Sep 17 00:00:00 2001
From: Eric P Green <ericpgreen2@gmail.com>
Date: Fri, 1 Mar 2024 16:40:56 -0800
Subject: [PATCH 3/4] Move function definition outside their parent function

---
 .../dashboards/stores/filter-utils.ts         | 49 +++++++++----------
 1 file changed, 23 insertions(+), 26 deletions(-)

diff --git a/web-common/src/features/dashboards/stores/filter-utils.ts b/web-common/src/features/dashboards/stores/filter-utils.ts
index 53a3c0b7de8..cad7da5e501 100644
--- a/web-common/src/features/dashboards/stores/filter-utils.ts
+++ b/web-common/src/features/dashboards/stores/filter-utils.ts
@@ -255,36 +255,33 @@ export const sanitiseExpression = (
   return where;
 };
 
-export function isExpressionIncomplete(expression: V1Expression): boolean {
-  // Check if the operation is unspecified at any level of the condition.
-  function isOperationUnspecified(cond: V1Condition): boolean {
-    if (
-      cond.op === V1Operation.OPERATION_UNSPECIFIED ||
-      cond.op === undefined
-    ) {
-      return true;
-    }
-    // Check nested conditions
-    return (
-      cond.exprs?.some(
-        (expr) => expr.cond && isOperationUnspecified(expr.cond),
-      ) ?? false
-    );
+// Check if the operation is unspecified at any level of the condition.
+function isOperationUnspecified(cond: V1Condition): boolean {
+  if (cond.op === V1Operation.OPERATION_UNSPECIFIED || cond.op === undefined) {
+    return true;
   }
+  // Check nested conditions
+  return (
+    cond.exprs?.some(
+      (expr) => expr.cond && isOperationUnspecified(expr.cond),
+    ) ?? false
+  );
+}
 
-  // Check if the val is defined and non-empty at any level of the nested expressions.
-  function isValDefinedAndNonEmpty(expr: V1Expression): boolean {
-    if (expr.val !== undefined && expr.val !== "") {
-      return true; // val is defined and non-empty
-    }
-    // If there is a nested condition, check if any nested expression has a defined and non-empty val
-    return (
-      expr.cond?.exprs?.some((nestedExpr) =>
-        isValDefinedAndNonEmpty(nestedExpr),
-      ) ?? false
-    );
+// Check if the val is defined and non-empty at any level of the nested expressions.
+function isValDefinedAndNonEmpty(expr: V1Expression): boolean {
+  if (expr.val !== undefined && expr.val !== "") {
+    return true; // val is defined and non-empty
   }
+  // If there is a nested condition, check if any nested expression has a defined and non-empty val
+  return (
+    expr.cond?.exprs?.some((nestedExpr) =>
+      isValDefinedAndNonEmpty(nestedExpr),
+    ) ?? false
+  );
+}
 
+export function isExpressionIncomplete(expression: V1Expression): boolean {
   // Check the top-level expression's operation
   if (expression.cond && isOperationUnspecified(expression.cond)) {
     return true; // The top-level operation is unspecified, thus incomplete

From 6a30749e6ac186317e7485f778440141832030d6 Mon Sep 17 00:00:00 2001
From: Eric P Green <ericpgreen2@gmail.com>
Date: Fri, 1 Mar 2024 16:41:35 -0800
Subject: [PATCH 4/4] Sneak in a font color improvement

---
 .../notifications/PersistedLinkNotification.svelte          | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/web-common/src/components/notifications/PersistedLinkNotification.svelte b/web-common/src/components/notifications/PersistedLinkNotification.svelte
index 3af0b2e7acc..c7d97674ba4 100644
--- a/web-common/src/components/notifications/PersistedLinkNotification.svelte
+++ b/web-common/src/components/notifications/PersistedLinkNotification.svelte
@@ -23,7 +23,11 @@
     </span>
   </div>
   <div class="px-4 py-1.5 border-l border-gray-600 text-sm">
-    <a href={link.href} on:click={() => dispatch("clear")}>{link.text}</a>
+    <a
+      class="text-primary-300 hover:text-primary-200"
+      href={link.href}
+      on:click={() => dispatch("clear")}>{link.text}</a
+    >
   </div>
   <div class="px-2.5 py-1.5 border-l border-gray-600">
     <IconButton on:click={() => dispatch("clear")} bgDark>