Implement query builder & select step validation error UI behavior

bruinenxyz · Mar 30, 2024 · 2759a40 · 2759a40
1 parent b72de47
commit 2759a40
Show file tree

Hide file tree

Showing 6 changed files with 146 additions and 16 deletions.
diff --git a/frontend/src/components/column-selectors/multi-column-selector/multi-column-selector.tsx b/frontend/src/components/column-selectors/multi-column-selector/multi-column-selector.tsx
@@ -90,7 +90,6 @@ export default function MultiColumnSelector({
           !!column.relation &&
           column.relation.as !== columnToRemove.relation.as),
     );
-    console.log("New selected:", newSelected);
     setSelected(newSelected);
   };
 

diff --git a/frontend/src/components/query/query-builder/invalid-step-popover.tsx b/frontend/src/components/query/query-builder/invalid-step-popover.tsx
@@ -0,0 +1,34 @@
+"use client";
+import { Callout, Colors, Icon, Popover, Text } from "@blueprintjs/core";
+import * as _ from "lodash";
+
+export default function InvalidStepPopover({ errors }: { errors: any[] }) {
+  return (
+    <Popover
+      content={
+        <div className="p-1">
+          <Callout
+            intent="warning"
+            icon="warning-sign"
+            title="This step is invalid"
+          >
+            {_.map(errors, (error: any, index: number) => {
+              return (
+                <Text key={index}>
+                  {index + 1}. {error.message}
+                </Text>
+              );
+            })}
+          </Callout>
+        </div>
+      }
+      interactionKind="hover"
+      placement="right"
+    >
+      <div className="flex flex-row items-center p-1 ml-2 border rounded-sm cursor-pointer border-bluprint-border-gray">
+        <Icon className="mr-1" icon="warning-sign" color={Colors.ORANGE3} />
+        <Text>Invalid step</Text>
+      </div>
+    </Popover>
+  );
+}
diff --git a/frontend/src/components/query/query-builder/query-builder.tsx b/frontend/src/components/query/query-builder/query-builder.tsx
@@ -10,8 +10,11 @@ import { Button, NonIdealState, Section } from "@blueprintjs/core";
 import StepTypeSelector from "./step-type-selector";
 import FromStepComponent from "./steps/from-step";
 import SelectStepComponent from "./steps/select-step";
+import { usePipelineSchema } from "@/data/use-user-query";
 import { useState, useEffect } from "react";
 import * as _ from "lodash";
+import Loading from "@/app/loading";
+import { ErrorDisplay } from "@/components/error-display";
 
 interface QueryBuilderProps {
   pipeline: Pipeline;
@@ -33,11 +36,30 @@ export default function QueryBuilder({
   const [editStepIndex, setEditStepIndex] = useState<number | null>(
     pipeline.from ? null : -1,
   );
+  const [validationErrorIndex, setValidationErrorIndex] = useState<
+    number | null
+  >(null);
+
+  const {
+    data: schema,
+    isLoading: isLoadingSchema,
+    error: schemaError,
+  } = usePipelineSchema(pipeline);
 
   useEffect(() => {
     setNewStepType(null);
   }, [pipeline]);
 
+  useEffect(() => {
+    if (schema) {
+      if (schema.success) {
+        setValidationErrorIndex(null);
+      } else {
+        setValidationErrorIndex(schema.error.issues[0].path[0]);
+      }
+    }
+  }, [schema]);
+
   function renderStep(
     stepType: StepIdentifier,
     step: Step | null,
@@ -86,7 +108,7 @@ export default function QueryBuilder({
   function renderSteps() {
     return (
       <>
-        {_.map(pipeline.steps, (step: Step, index) => {
+        {_.map(pipeline.steps, (step: Step, index: number) => {
           return (
             <>
               {newStepType && newStepType.index === index ? (
@@ -96,7 +118,12 @@ export default function QueryBuilder({
                   key={`selector-${index}`}
                   index={index}
                   setNewStepType={setNewStepType}
-                  disabled={newStepType !== null || editStepIndex !== null}
+                  disabled={
+                    newStepType !== null ||
+                    editStepIndex !== null ||
+                    (validationErrorIndex !== null &&
+                      validationErrorIndex < index)
+                  }
                 />
               )}
               {renderStep(step.type, step, index)}
@@ -109,13 +136,34 @@ export default function QueryBuilder({
           <StepTypeSelector
             index={pipeline.steps.length}
             setNewStepType={setNewStepType}
-            disabled={newStepType !== null || editStepIndex !== null}
+            disabled={
+              newStepType !== null ||
+              editStepIndex !== null ||
+              (validationErrorIndex !== null &&
+                validationErrorIndex <= pipeline.steps.length)
+            }
           />
         )}
       </>
     );
   }
 
+  if (isLoadingSchema) {
+    return (
+      <Section className={className}>
+        <Loading />
+      </Section>
+    );
+  }
+
+  if ((schemaError || !schema) && !!pipeline.from) {
+    return (
+      <Section className={className}>
+        <ErrorDisplay description={schemaError} />
+      </Section>
+    );
+  }
+
   return (
     <Section className={className}>
       <FromStepComponent

diff --git a/frontend/src/components/query/query-builder/steps/from-step.tsx b/frontend/src/components/query/query-builder/steps/from-step.tsx
@@ -49,7 +49,7 @@ export default function FromStepComponent({
     if (tables && pipeline.from) {
       setSelected(tables.find((table) => table.id === pipeline.from) || null);
     }
-  }, [tables]);
+  }, [tables, pipeline]);
 
   const renderTable: ItemRenderer<HydratedTable> = (
     table: HydratedTable,

diff --git a/frontend/src/components/query/query-builder/steps/select-step.tsx b/frontend/src/components/query/query-builder/steps/select-step.tsx
@@ -19,7 +19,8 @@ import Loading from "@/app/loading";
 import { ErrorDisplay } from "@/components/error-display";
 import MultiColumnSelector from "@/components/column-selectors/multi-column-selector/multi-column-selector";
 import InferredSchemaColumnTag from "@/components/column/inferred-schema-column-tag";
-import { useState } from "react";
+import InvalidStepPopover from "../invalid-step-popover";
+import { useEffect, useState } from "react";
 import { usePipelineSchema } from "@/data/use-user-query";
 import * as _ from "lodash";
 
@@ -50,20 +51,64 @@ export default function SelectStepComponent({
     !!step ? step.select : [],
   );
 
+  const {
+    data: inputSchema,
+    isLoading: isLoadingInputSchema,
+    error: inputSchemaError,
+  } = usePipelineSchema({
+    ...pipeline,
+    steps: _.slice(pipeline.steps, 0, index),
+  });
+
   const {
     data: schema,
     isLoading: isLoadingSchema,
     error: schemaError,
   } = usePipelineSchema({
     ...pipeline,
-    steps: _.slice(pipeline.steps, 0, index),
+    steps: _.slice(pipeline.steps, 0, index + 1),
   });
 
+  useEffect(() => {
+    if (step) {
+      setSelected(step.select);
+    } else {
+      setSelected([]);
+    }
+  }, [step]);
+
+  function getAdditionalClasses() {
+    if (inputSchema && !inputSchema.success) {
+      return "border-2 border-gold";
+    } else if (schema && !schema.success) {
+      return "border-2 border-error";
+    }
+  }
+
   function renderTitle() {
     if (edit || create || !step) {
       return <Text className="text-xl grow-0">Select:</Text>;
+    } else if (inputSchema && !inputSchema.success) {
+      return (
+        <div className="flex flex-row items-center">
+          <Text className="text-xl grow-0">Select:</Text>
+          <div className="flex flex-row flex-wrap gap-1 mx-2 grow">
+            {_.map(
+              step.select,
+              (column: InferredSchemaColumn, index: number) => {
+                return <InferredSchemaColumnTag key={index} column={column} />;
+              },
+            )}
+          </div>
+        </div>
+      );
     } else if (schema && !schema.success) {
-      // TODO: Handle schema validation errors
+      return (
+        <div className="flex flex-row items-center">
+          <Text className="text-xl grow-0">Select:</Text>
+          <InvalidStepPopover errors={schema!.error.issues} />
+        </div>
+      );
     } else {
       return (
         <div className="flex flex-row items-center">
@@ -136,6 +181,10 @@ export default function SelectStepComponent({
                 <MenuItem
                   icon="edit"
                   text="Edit step"
+                  disabled={
+                    (!!inputSchema && !inputSchema.success) ||
+                    (!!schema && !schema.success)
+                  }
                   onClick={() => setEditStepIndex(index)}
                 />
                 <MenuItem
@@ -159,27 +208,27 @@ export default function SelectStepComponent({
   }
 
   function renderContent() {
-    if (isLoadingSchema) {
+    if (isLoadingSchema || isLoadingInputSchema) {
       return <Loading />;
-    } else if (schemaError || !schema) {
-      return <ErrorDisplay description={schemaError} />;
-    } else if (!schema.success) {
-      // TODO: Handle schema validation errors
+    } else if (schemaError || inputSchemaError) {
+      return <ErrorDisplay description={schemaError || inputSchemaError} />;
+    } else if (inputSchema && !inputSchema.success) {
+      return null;
     } else if (edit || create || !step) {
       return (
         <MultiColumnSelector
           className="mx-3 my-2"
           selected={selected}
           setSelected={setSelected}
-          items={schema.data.columns}
+          items={inputSchema!.data.columns}
         />
       );
     }
   }
 
   return (
     <Section
-      className="flex-none w-full rounded-sm"
+      className={`flex-none w-full rounded-sm ${getAdditionalClasses()}`}
       title={renderTitle()}
       rightElement={<div className="flex flex-row">{renderRightElement()}</div>}
     >

diff --git a/frontend/src/utils/pipeline/validate-pipeline-step.tsx b/frontend/src/utils/pipeline/validate-pipeline-step.tsx
@@ -90,7 +90,7 @@ function createSelectStepValidator(
         ) {
           ctx.addIssue({
             code: z.ZodIssueCode.custom,
-            message: `Column '${column.name}' on table '${column.table}' ${column.relation && `via relation '${column.relation.as}' `}does not exist in input schema`,
+            message: `Column '${column.name}' on table '${column.table}' ${column.relation ? `via relation '${column.relation.as}' ` : ""}does not exist in input schema`,
             path: [
               stepIndex.toString(),
               `step ${stepIndex + 1} - Select`,