Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🪟🎉 Connector builder: Improve testing panel UX on invalid configuration #22061

Merged
merged 7 commits into from
Feb 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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" });
Expand All @@ -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;
}
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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 = (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
flex: 1;
display: flex;
flex-direction: column;
align-items: center;
gap: variables.$spacing-lg;
min-height: 0;
width: 100%;
Expand All @@ -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;
}
Original file line number Diff line number Diff line change
@@ -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";
Expand All @@ -22,6 +24,8 @@ export const StreamTester: React.FC<{
const {
streams,
testStreamIndex,
isFetchingStreamList,
streamListErrorMessage,
streamRead: {
data: streamReadData,
refetch: readStream,
Expand Down Expand Up @@ -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={() => {
Expand All @@ -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 />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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%;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -14,7 +13,6 @@ import {
useConnectorBuilderTestState,
useConnectorBuilderFormState,
} from "services/connectorBuilder/ConnectorBuilderStateService";
import { links } from "utils/links";

import { ConfigMenu } from "./ConfigMenu";
import { StreamSelector } from "./StreamSelector";
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -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[] = [];

Expand All @@ -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) {
Expand Down
5 changes: 4 additions & 1 deletion airbyte-webapp/src/locales/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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.",

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ const ConnectorBuilderPageInner: React.FC = React.memo(() => {
const switchToYaml = useCallback(() => setEditorView("yaml"), [setEditorView]);

const initialFormValues = useRef(builderFormValues);

return useMemo(
() => (
<Formik
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ export type BuilderView = "global" | "inputs" | number;

interface FormStateContext {
builderFormValues: BuilderFormValues;
formValuesValid: boolean;
jsonManifest: ConnectorManifest;
lastValidJsonManifest: DeclarativeComponentSchema | undefined;
yamlManifest: string;
Expand All @@ -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);
Expand All @@ -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);

Expand All @@ -75,6 +79,7 @@ export const ConnectorBuilderFormStateProvider: React.FC<React.PropsWithChildren
lastValidBuilderFormValuesRef.current = values;
}
currentBuilderFormValuesRef.current = values;
setFormValuesValid(isValid);
setStoredBuilderFormValues(values);
},
[setStoredBuilderFormValues]
Expand Down Expand Up @@ -141,6 +146,7 @@ export const ConnectorBuilderFormStateProvider: React.FC<React.PropsWithChildren

const ctx = {
builderFormValues,
formValuesValid,
jsonManifest: derivedJsonManifest,
lastValidJsonManifest,
yamlManifest,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -205,6 +212,7 @@ export const ConnectorBuilderTestStateProvider: React.FC<React.PropsWithChildren
testStreamIndex,
setTestStreamIndex,
streamRead,
isFetchingStreamList,
};

return <ConnectorBuilderTestStateContext.Provider value={ctx}>{children}</ConnectorBuilderTestStateContext.Provider>;
Expand Down