🪟🎉 Connector builder: Improve testing panel UX on invalid configurati…

…on (#22061) * wup * fix builder field * use stream list response if available * error message for URL
airbytehq · Feb 2, 2023 · a5452a6 · a5452a6
1 parent 6a10ae3
commit a5452a6
Show file tree

Hide file tree

Showing 11 changed files with 98 additions and 46 deletions.
diff --git a/airbyte-webapp/src/components/connectorBuilder/Builder/BuilderField.tsx b/airbyte-webapp/src/components/connectorBuilder/Builder/BuilderField.tsx
@@ -117,6 +117,7 @@ const InnerBuilderField: React.FC<BuilderFieldProps & FastFieldProps<unknown>> =
           readOnly={readOnly}
           adornment={adornment}
           onBlur={(e) => {
+            field.onBlur(e);
             props.onBlur?.(e.target.value);
           }}
         />

diff --git a/airbyte-webapp/src/components/connectorBuilder/StreamTestingPanel/StreamSelector.tsx b/airbyte-webapp/src/components/connectorBuilder/StreamTestingPanel/StreamSelector.tsx
@@ -34,7 +34,10 @@ export const StreamSelector: React.FC<StreamSelectorProps> = ({ className }) =>
   const analyticsService = useAnalyticsService();
   const { formatMessage } = useIntl();
   const { selectedView, setSelectedView } = useConnectorBuilderFormState();
-  const { streams, testStreamIndex, setTestStreamIndex } = useConnectorBuilderTestState();
+  const { testStreamIndex, setTestStreamIndex } = useConnectorBuilderTestState();
+
+  const streams = useStreamNames();
+
   const options = streams.map((stream) => {
     const label =
       stream.name && stream.name.trim() ? capitalize(stream.name) : formatMessage({ id: "connectorBuilder.emptyName" });
@@ -60,10 +63,25 @@ export const StreamSelector: React.FC<StreamSelectorProps> = ({ className }) =>
     <ListBox
       className={classNames(className, styles.container)}
       options={options}
-      selectedValue={streams[testStreamIndex]?.name}
+      selectedValue={streams[testStreamIndex]?.name ?? formatMessage({ id: "connectorBuilder.noStreamSelected" })}
       onSelect={handleStreamSelect}
       buttonClassName={styles.button}
       controlButton={ControlButton}
     />
   );
 };
+
+function useStreamNames() {
+  const { builderFormValues, editorView, formValuesValid } = useConnectorBuilderFormState();
+  const { streams: testStreams, isFetchingStreamList, streamListErrorMessage } = useConnectorBuilderTestState();
+
+  let streams: Array<{ name: string }> = editorView === "ui" ? builderFormValues.streams : testStreams;
+
+  const testStreamListUpToDate = formValuesValid && !isFetchingStreamList && !streamListErrorMessage;
+
+  if (editorView === "ui" && testStreamListUpToDate) {
+    streams = streams.map((stream, index) => ({ name: testStreams[index]?.name || stream.name }));
+  }
+
+  return streams;
+}
diff --git a/airbyte-webapp/src/components/connectorBuilder/StreamTestingPanel/StreamTestButton.tsx b/airbyte-webapp/src/components/connectorBuilder/StreamTestingPanel/StreamTestButton.tsx
@@ -15,12 +15,14 @@ import { useBuilderErrors } from "../useBuilderErrors";
 interface StreamTestButtonProps {
   readStream: () => void;
   hasTestInputJsonErrors: boolean;
+  hasStreamListErrors: boolean;
   setTestInputOpen: (open: boolean) => void;
 }
 
 export const StreamTestButton: React.FC<StreamTestButtonProps> = ({
   readStream,
   hasTestInputJsonErrors,
+  hasStreamListErrors,
   setTestInputOpen,
 }) => {
   const { editorView, yamlIsValid } = useConnectorBuilderFormState();
@@ -52,6 +54,9 @@ export const StreamTestButton: React.FC<StreamTestButtonProps> = ({
   if ((editorView === "ui" && hasErrors(false)) || hasTestInputJsonErrors) {
     showWarningIcon = true;
     tooltipContent = <FormattedMessage id="connectorBuilder.configErrorsTest" />;
+  } else if (hasStreamListErrors) {
+    // only disable the button on stream list errors if there are no user-fixable errors
+    buttonDisabled = true;
   }
 
   const testButton = (

diff --git a/airbyte-webapp/src/components/connectorBuilder/StreamTestingPanel/StreamTester.module.scss b/airbyte-webapp/src/components/connectorBuilder/StreamTestingPanel/StreamTester.module.scss
@@ -5,7 +5,6 @@
   flex: 1;
   display: flex;
   flex-direction: column;
-  align-items: center;
   gap: variables.$spacing-lg;
   min-height: 0;
   width: 100%;
@@ -31,3 +30,12 @@
 .fetchingSpinner {
   margin: auto;
 }
+
+.listErrorDisplay {
+  padding: variables.$spacing-lg;
+  display: flex;
+  flex-direction: column;
+  gap: variables.$spacing-md;
+  background-color: colors.$blue-50;
+  border-radius: variables.$border-radius-sm;
+}
diff --git a/airbyte-webapp/src/components/connectorBuilder/StreamTestingPanel/StreamTester.tsx b/airbyte-webapp/src/components/connectorBuilder/StreamTestingPanel/StreamTester.tsx
@@ -1,13 +1,15 @@
 import { useEffect, useState } from "react";
-import { useIntl } from "react-intl";
+import { FormattedMessage, useIntl } from "react-intl";
 
 import { ResizablePanels } from "components/ui/ResizablePanels";
 import { Spinner } from "components/ui/Spinner";
 import { Text } from "components/ui/Text";
 
 import { Action, Namespace } from "core/analytics";
+import { StreamsListReadStreamsItem } from "core/request/ConnectorBuilderClient";
 import { useAnalyticsService } from "hooks/services/Analytics";
 import { useConnectorBuilderTestState } from "services/connectorBuilder/ConnectorBuilderStateService";
+import { links } from "utils/links";
 
 import { LogsDisplay } from "./LogsDisplay";
 import { ResultDisplay } from "./ResultDisplay";
@@ -22,6 +24,8 @@ export const StreamTester: React.FC<{
   const {
     streams,
     testStreamIndex,
+    isFetchingStreamList,
+    streamListErrorMessage,
     streamRead: {
       data: streamReadData,
       refetch: readStream,
@@ -78,11 +82,24 @@ export const StreamTester: React.FC<{
     }
   }, [analyticsService, errorMessage, isFetchedAfterMount, streamName, dataUpdatedAt, errorUpdatedAt]);
 
+  const currentStream = streams[testStreamIndex] as StreamsListReadStreamsItem | undefined;
   return (
     <div className={styles.container}>
-      <Text className={styles.url} size="lg">
-        {streams[testStreamIndex]?.url}
-      </Text>
+      {currentStream && (
+        <Text className={styles.url} centered size="lg">
+          {currentStream.url}
+        </Text>
+      )}
+      {!currentStream && isFetchingStreamList && (
+        <Text size="lg" centered>
+          <FormattedMessage id="connectorBuilder.loadingStreamList" />
+        </Text>
+      )}
+      {!currentStream && streamListErrorMessage && (
+        <Text size="lg" centered>
+          <FormattedMessage id="connectorBuilder.streamListUrlError" />
+        </Text>
+      )}
 
       <StreamTestButton
         readStream={() => {
@@ -93,9 +110,30 @@ export const StreamTester: React.FC<{
           });
         }}
         hasTestInputJsonErrors={hasTestInputJsonErrors}
+        hasStreamListErrors={Boolean(streamListErrorMessage)}
         setTestInputOpen={setTestInputOpen}
       />
 
+      {streamListErrorMessage !== undefined && (
+        <div className={styles.listErrorDisplay}>
+          <Text>
+            <FormattedMessage id="connectorBuilder.couldNotDetectStreams" />
+          </Text>
+          <Text bold>{streamListErrorMessage}</Text>
+          <Text>
+            <FormattedMessage
+              id="connectorBuilder.ensureProperYaml"
+              values={{
+                a: (node: React.ReactNode) => (
+                  <a href={links.lowCodeYamlDescription} target="_blank" rel="noreferrer">
+                    {node}
+                  </a>
+                ),
+              }}
+            />
+          </Text>
+        </div>
+      )}
       {isFetching && (
         <div className={styles.fetchingSpinner}>
           <Spinner />

diff --git a/...-webapp/src/components/connectorBuilder/StreamTestingPanel/StreamTestingPanel.module.scss b/...-webapp/src/components/connectorBuilder/StreamTestingPanel/StreamTestingPanel.module.scss
@@ -28,18 +28,6 @@
   align-items: center;
 }
 
-.listErrorDisplay {
-  padding: variables.$spacing-lg;
-  display: flex;
-  flex-direction: column;
-  gap: variables.$spacing-md;
-  background-color: colors.$blue-50;
-  border-radius: variables.$border-radius-sm;
-
-  // leave room for config button
-  margin-top: 50px;
-}
-
 .loadingSpinner {
   height: 100%;
   width: 100%;

diff --git a/airbyte-webapp/src/components/connectorBuilder/StreamTestingPanel/StreamTestingPanel.tsx b/airbyte-webapp/src/components/connectorBuilder/StreamTestingPanel/StreamTestingPanel.tsx
@@ -4,7 +4,6 @@ import { ValidationError } from "yup";
 
 import { Heading } from "components/ui/Heading";
 import { Spinner } from "components/ui/Spinner";
-import { Text } from "components/ui/Text";
 
 import { jsonSchemaToFormBlock } from "core/form/schemaToFormBlock";
 import { buildYupFormForJsonSchema } from "core/form/schemaToYup";
@@ -14,7 +13,6 @@ import {
   useConnectorBuilderTestState,
   useConnectorBuilderFormState,
 } from "services/connectorBuilder/ConnectorBuilderStateService";
-import { links } from "utils/links";
 
 import { ConfigMenu } from "./ConfigMenu";
 import { StreamSelector } from "./StreamSelector";
@@ -43,7 +41,7 @@ function useTestInputJsonErrors(testInputJson: StreamReadRequestBodyConfig, spec
 export const StreamTestingPanel: React.FC<unknown> = () => {
   const [isTestInputOpen, setTestInputOpen] = useState(false);
   const { jsonManifest, yamlEditorIsMounted } = useConnectorBuilderFormState();
-  const { testInputJson, streamListErrorMessage } = useConnectorBuilderTestState();
+  const { testInputJson } = useConnectorBuilderTestState();
 
   const testInputJsonErrors = useTestInputJsonErrors(testInputJson, jsonManifest.spec);
 
@@ -73,32 +71,12 @@ export const StreamTestingPanel: React.FC<unknown> = () => {
           <img className={styles.logo} alt="" src="/images/octavia/pointing.svg" width={102} />
         </div>
       )}
-      {hasStreams && streamListErrorMessage === undefined && (
+      {hasStreams && (
         <div className={styles.selectAndTestContainer}>
           <StreamSelector className={styles.streamSelector} />
           <StreamTester hasTestInputJsonErrors={testInputJsonErrors > 0} setTestInputOpen={setTestInputOpen} />
         </div>
       )}
-      {hasStreams && streamListErrorMessage !== undefined && (
-        <div className={styles.listErrorDisplay}>
-          <Text>
-            <FormattedMessage id="connectorBuilder.couldNotDetectStreams" />
-          </Text>
-          <Text bold>{streamListErrorMessage}</Text>
-          <Text>
-            <FormattedMessage
-              id="connectorBuilder.ensureProperYaml"
-              values={{
-                a: (node: React.ReactNode) => (
-                  <a href={links.lowCodeYamlDescription} target="_blank" rel="noreferrer">
-                    {node}
-                  </a>
-                ),
-              }}
-            />
-          </Text>
-        </div>
-      )}
     </div>
   );
 };
diff --git a/airbyte-webapp/src/components/connectorBuilder/useBuilderErrors.ts b/airbyte-webapp/src/components/connectorBuilder/useBuilderErrors.ts
@@ -14,7 +14,9 @@ export const useBuilderErrors = () => {
   const invalidViews = useCallback(
     (ignoreUntouched: boolean, limitToViews?: BuilderView[], inputErrors?: FormikErrors<BuilderFormValues>) => {
       const errorsToCheck = inputErrors !== undefined ? inputErrors : errors;
-      const errorKeys = Object.keys(errorsToCheck);
+      const errorKeys = Object.keys(errorsToCheck).filter((errorKey) =>
+        Boolean(errorsToCheck[errorKey as keyof BuilderFormValues])
+      );
 
       const invalidViews: BuilderView[] = [];
 
@@ -33,7 +35,9 @@ export const useBuilderErrors = () => {
       }
 
       if (errorKeys.includes("streams")) {
-        const errorStreamNums = Object.keys(errorsToCheck.streams ?? {});
+        const errorStreamNums = Object.keys(errorsToCheck.streams ?? {}).filter((errorKey) =>
+          Boolean(errorsToCheck.streams?.[Number(errorKey)])
+        );
 
         if (ignoreUntouched) {
           if (errorsToCheck.streams && touched.streams) {

diff --git a/airbyte-webapp/src/locales/en.json b/airbyte-webapp/src/locales/en.json
@@ -668,7 +668,7 @@
   "connectorBuilder.invalidYamlTest": "Cannot test stream while YAML content is invalid.",
   "connectorBuilder.unknownError": "An unknown error has occurred",
   "connectorBuilder.testConnector": "TEST YOUR CONNECTOR",
-  "connectorBuilder.couldNotDetectStreams": "Could not detect streams in the YAML editor:",
+  "connectorBuilder.couldNotDetectStreams": "Could not verify streams:",
   "connectorBuilder.ensureProperYaml": "In order to test a stream, ensure that the YAML is structured as described in <a>the docs</a>.",
   "connectorBuilder.addStreamModal.title": "New stream",
   "connectorBuilder.addStreamModal.streamNameLabel": "Stream name",
@@ -747,6 +747,9 @@
   "connectorBuilder.copyFromSlicerTitle": "Import slicing settings from...",
   "connectorBuilder.inputsButton": "Inputs",
   "connectorBuilder.interUserInputValue": "Insert a user input value",
+  "connectorBuilder.loadingStreamList": "Loading",
+  "connectorBuilder.noStreamSelected": "No stream selected",
+  "connectorBuilder.streamListUrlError": "Could not load URL",
 
   "jobs.noAttemptsFailure": "Failed to start job.",
 

diff --git a/airbyte-webapp/src/pages/ConnectorBuilderPage/ConnectorBuilderPage.tsx b/airbyte-webapp/src/pages/ConnectorBuilderPage/ConnectorBuilderPage.tsx
@@ -36,6 +36,7 @@ const ConnectorBuilderPageInner: React.FC = React.memo(() => {
   const switchToYaml = useCallback(() => setEditorView("yaml"), [setEditorView]);
 
   const initialFormValues = useRef(builderFormValues);
+
   return useMemo(
     () => (
       <Formik

diff --git a/airbyte-webapp/src/services/connectorBuilder/ConnectorBuilderStateService.tsx b/airbyte-webapp/src/services/connectorBuilder/ConnectorBuilderStateService.tsx
@@ -30,6 +30,7 @@ export type BuilderView = "global" | "inputs" | number;
 
 interface FormStateContext {
   builderFormValues: BuilderFormValues;
+  formValuesValid: boolean;
   jsonManifest: ConnectorManifest;
   lastValidJsonManifest: DeclarativeComponentSchema | undefined;
   yamlManifest: string;
@@ -53,6 +54,7 @@ interface TestStateContext {
   setTestStreamIndex: (streamIndex: number) => void;
   testStreamIndex: number;
   streamRead: UseQueryResult<StreamRead, unknown>;
+  isFetchingStreamList: boolean;
 }
 
 export const ConnectorBuilderFormStateContext = React.createContext<FormStateContext | null>(null);
@@ -65,6 +67,8 @@ export const ConnectorBuilderFormStateProvider: React.FC<React.PropsWithChildren
     DEFAULT_BUILDER_FORM_VALUES
   );
 
+  const [formValuesValid, setFormValuesValid] = useState(true);
+
   const lastValidBuilderFormValuesRef = useRef<BuilderFormValues>(storedBuilderFormValues as BuilderFormValues);
   const currentBuilderFormValuesRef = useRef<BuilderFormValues>(storedBuilderFormValues as BuilderFormValues);
 
@@ -75,6 +79,7 @@ export const ConnectorBuilderFormStateProvider: React.FC<React.PropsWithChildren
         lastValidBuilderFormValuesRef.current = values;
       }
       currentBuilderFormValuesRef.current = values;
+      setFormValuesValid(isValid);
       setStoredBuilderFormValues(values);
     },
     [setStoredBuilderFormValues]
@@ -141,6 +146,7 @@ export const ConnectorBuilderFormStateProvider: React.FC<React.PropsWithChildren
 
   const ctx = {
     builderFormValues,
+    formValuesValid,
     jsonManifest: derivedJsonManifest,
     lastValidJsonManifest,
     yamlManifest,
@@ -173,6 +179,7 @@ export const ConnectorBuilderTestStateProvider: React.FC<React.PropsWithChildren
     data: streamListRead,
     isError: isStreamListError,
     error: streamListError,
+    isFetching: isFetchingStreamList,
   } = useListStreams({ manifest, config: testInputJson });
   const unknownErrorMessage = formatMessage({ id: "connectorBuilder.unknownError" });
   const streamListErrorMessage = isStreamListError
@@ -205,6 +212,7 @@ export const ConnectorBuilderTestStateProvider: React.FC<React.PropsWithChildren
     testStreamIndex,
     setTestStreamIndex,
     streamRead,
+    isFetchingStreamList,
   };
 
   return <ConnectorBuilderTestStateContext.Provider value={ctx}>{children}</ConnectorBuilderTestStateContext.Provider>;