Skip to content

Commit

Permalink
🪟🔧 Move connector loading state into connector card (#20127)
Browse files Browse the repository at this point in the history
* refactor loading state

* move loading state up

* remove isLoading references

* remove unused props and make fetch connector error work

* Update airbyte-webapp/src/views/Connector/ConnectorCard/ConnectorCard.module.scss

Co-authored-by: Tim Roes <tim@airbyte.io>

* remove undefined option for selected id

* remove unused prop

Co-authored-by: Tim Roes <tim@airbyte.io>
  • Loading branch information
Joe Reuter and timroes authored Dec 8, 2022
1 parent ce29361 commit 0e6e02f
Show file tree
Hide file tree
Showing 14 changed files with 89 additions and 127 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { LogsRequestError } from "core/request/LogsRequestError";
import { useExperiment } from "hooks/services/Experiment";
import { useGetDestinationDefinitionSpecificationAsync } from "services/connector/DestinationDefinitionSpecificationService";
import { ConnectorIds } from "utils/connectors";
import { generateMessageFromError, FormError } from "utils/errorStatusMessage";
import { FormError } from "utils/errorStatusMessage";
import { ConnectorCard } from "views/Connector/ConnectorCard";
import { ConnectorCardValues, FrequentlyUsedConnectors, StartWithDestination } from "views/Connector/ConnectorForm";

Expand Down Expand Up @@ -63,8 +63,6 @@ export const DestinationForm: React.FC<DestinationFormProps> = ({
});
};

const errorMessage = error ? generateMessageFromError(error) : null;

const frequentlyUsedDestinationIds = useExperiment("connector.frequentlyUsedDestinationIds", [
ConnectorIds.Destinations.BigQuery,
ConnectorIds.Destinations.Snowflake,
Expand All @@ -91,11 +89,11 @@ export const DestinationForm: React.FC<DestinationFormProps> = ({
description={<FormattedMessage id="destinations.description" />}
isLoading={isLoading}
hasSuccess={hasSuccess}
errorMessage={errorMessage}
fetchingConnectorError={destinationDefinitionError instanceof Error ? destinationDefinitionError : null}
availableConnectorDefinitions={destinationDefinitions}
onConnectorDefinitionSelect={onDropDownSelect}
selectedConnectorDefinitionSpecification={destinationDefinitionSpecification}
selectedConnectorDefinitionId={destinationDefinitionId}
onSubmit={onSubmitForm}
jobInfo={LogsRequestError.extractJobInfo(error)}
additionalSelectorComponent={frequentlyUsedDestinationsComponent}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { ConnectionConfiguration } from "core/domain/connection";
import { LogsRequestError } from "core/request/LogsRequestError";
import { SourceDefinitionReadWithLatestTag } from "services/connector/SourceDefinitionService";
import { useGetSourceDefinitionSpecificationAsync } from "services/connector/SourceDefinitionSpecificationService";
import { generateMessageFromError, FormError } from "utils/errorStatusMessage";
import { FormError } from "utils/errorStatusMessage";
import { ConnectorCard } from "views/Connector/ConnectorCard";
import { ConnectorCardValues } from "views/Connector/ConnectorForm/types";

Expand Down Expand Up @@ -54,20 +54,18 @@ export const SourceForm: React.FC<SourceFormProps> = ({ onSubmit, sourceDefiniti
});
};

const errorMessage = error ? generateMessageFromError(error) : null;

return (
<ConnectorCard
formType="source"
title={<FormattedMessage id="onboarding.sourceSetUp" />}
description={<FormattedMessage id="sources.description" />}
isLoading={isLoading}
hasSuccess={hasSuccess}
errorMessage={errorMessage}
fetchingConnectorError={sourceDefinitionError instanceof Error ? sourceDefinitionError : null}
availableConnectorDefinitions={sourceDefinitions}
onConnectorDefinitionSelect={onDropDownSelect}
selectedConnectorDefinitionSpecification={sourceDefinitionSpecification}
selectedConnectorDefinitionId={sourceDefinitionId}
onSubmit={onSubmitForm}
jobInfo={LogsRequestError.extractJobInfo(error)}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ const SourceSettings: React.FC<SourceSettingsProps> = ({ currentSource, connecti
formId={formId}
availableConnectorDefinitions={[sourceDefinition]}
selectedConnectorDefinitionSpecification={sourceDefinitionSpecification}
selectedConnectorDefinitionId={sourceDefinitionSpecification.sourceDefinitionId}
connector={currentSource}
onSubmit={onSubmit}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ export const DestinationSettingsPage: React.FC = () => {
formId={formId}
availableConnectorDefinitions={[destinationDefinition]}
selectedConnectorDefinitionSpecification={destinationSpecification}
selectedConnectorDefinitionId={destinationSpecification.destinationDefinitionId}
connector={destination}
onSubmit={onSubmitForm}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,15 @@
.connectorSelectControl {
margin-bottom: vars.$spacing-xl;
}

.loaderContainer {
display: flex;
justify-content: center;
align-items: center;
padding: vars.$spacing-2xl 0;
}

.loadingMessage {
margin-top: vars.$spacing-md;
margin-left: vars.$spacing-lg;
}
61 changes: 39 additions & 22 deletions airbyte-webapp/src/views/Connector/ConnectorCard/ConnectorCard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { FormattedMessage } from "react-intl";

import { JobItem } from "components/JobItem/JobItem";
import { Card } from "components/ui/Card";
import { Spinner } from "components/ui/Spinner";

import {
Connector,
Expand All @@ -19,6 +20,8 @@ import { ConnectorCardValues, ConnectorForm, ConnectorFormValues } from "views/C

import { useDocumentationPanelContext } from "../ConnectorDocumentationLayout/DocumentationPanelContext";
import { ConnectorDefinitionTypeControl } from "../ConnectorForm/components/Controls/ConnectorServiceTypeControl";
import { FetchingConnectorError } from "../ConnectorForm/components/TestingConnectionError";
import ShowLoadingMessage from "./components/ShowLoadingMessage";
import styles from "./ConnectorCard.module.scss";
import { useAnalyticsTrackFunctions } from "./useAnalyticsTrackFunctions";
import { useTestConnector } from "./useTestConnector";
Expand All @@ -38,14 +41,16 @@ interface ConnectorCardBaseProps {

// used in ConnectorCard and ConnectorForm
formType: "source" | "destination";
/**
* id of the selected connector definition id - might be available even if the specification is not loaded yet
* */
selectedConnectorDefinitionId: string | null;
selectedConnectorDefinitionSpecification?: ConnectorDefinitionSpecification;
isEditMode?: boolean;

// used in ConnectorForm
formId?: string;
fetchingConnectorError?: Error | null;
errorMessage?: React.ReactNode;
successMessage?: React.ReactNode;
hasSuccess?: boolean;
isLoading?: boolean;
}
Expand All @@ -70,6 +75,8 @@ export const ConnectorCard: React.FC<ConnectorCardCreateProps | ConnectorCardEdi
jobInfo,
onSubmit,
additionalSelectorComponent,
selectedConnectorDefinitionId,
fetchingConnectorError,
...props
}) => {
const [saved, setSaved] = useState(false);
Expand All @@ -96,7 +103,8 @@ export const ConnectorCard: React.FC<ConnectorCardCreateProps | ConnectorCardEdi
} = props;

const selectedConnectorDefinitionSpecificationId =
selectedConnectorDefinitionSpecification && ConnectorSpecification.id(selectedConnectorDefinitionSpecification);
selectedConnectorDefinitionId ||
(selectedConnectorDefinitionSpecification && ConnectorSpecification.id(selectedConnectorDefinitionSpecification));

const selectedConnectorDefinition = useMemo(
() => availableConnectorDefinitions.find((s) => Connector.id(s) === selectedConnectorDefinitionSpecificationId),
Expand Down Expand Up @@ -175,25 +183,34 @@ export const ConnectorCard: React.FC<ConnectorCardCreateProps | ConnectorCardEdi
</div>
{additionalSelectorComponent}
<div>
<ConnectorForm
// Causes the whole ConnectorForm to be unmounted and a new instance mounted whenever the connector type changes.
// That way we carry less state around inside it, preventing any state from one connector type from affecting another
// connector type's form in any way.
key={selectedConnectorDefinition && Connector.id(selectedConnectorDefinition)}
{...props}
selectedConnectorDefinition={selectedConnectorDefinition}
selectedConnectorDefinitionSpecification={selectedConnectorDefinitionSpecification}
isTestConnectionInProgress={isTestConnectionInProgress}
onStopTesting={onStopTesting}
testConnector={testConnector}
onSubmit={onHandleSubmit}
formValues={formValues}
errorMessage={props.errorMessage || (error && generateMessageFromError(error))}
successMessage={
props.successMessage || (saved && props.isEditMode && <FormattedMessage id="form.changesSaved" />)
}
connectorId={isEditMode ? getConnectorId(props.connector) : undefined}
/>
{props.isLoading && (
<div className={styles.loaderContainer}>
<Spinner />
<div className={styles.loadingMessage}>
<ShowLoadingMessage connector={selectedConnectorDefinition?.name} />
</div>
</div>
)}
{fetchingConnectorError && <FetchingConnectorError />}
{selectedConnectorDefinition && selectedConnectorDefinitionSpecification && (
<ConnectorForm
// Causes the whole ConnectorForm to be unmounted and a new instance mounted whenever the connector type changes.
// That way we carry less state around inside it, preventing any state from one connector type from affecting another
// connector type's form in any way.
key={selectedConnectorDefinition && Connector.id(selectedConnectorDefinition)}
{...props}
selectedConnectorDefinition={selectedConnectorDefinition}
selectedConnectorDefinitionSpecification={selectedConnectorDefinitionSpecification}
isTestConnectionInProgress={isTestConnectionInProgress}
onStopTesting={onStopTesting}
testConnector={testConnector}
onSubmit={onHandleSubmit}
formValues={formValues}
errorMessage={error && generateMessageFromError(error)}
successMessage={saved && props.isEditMode && <FormattedMessage id="form.changesSaved" />}
connectorId={isEditMode ? getConnectorId(props.connector) : undefined}
/>
)}
{/* Show the job log only if advanced mode is turned on or the actual job failed (not the check inside the job) */}
{job && (advancedMode || !job.succeeded) && <JobItem job={job} />}
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import React from "react";
import selectEvent from "react-select-event";
import { render, useMockIntersectionObserver } from "test-utils/testutils";

import { ConnectorDefinition } from "core/domain/connector";
import { AirbyteJSONSchema } from "core/jsonSchema";
import { DestinationDefinitionSpecificationRead } from "core/request/AirbyteClient";
import { ConnectorForm, ConnectorFormProps } from "views/Connector/ConnectorForm";
Expand Down Expand Up @@ -37,6 +38,11 @@ jest.mock("../ConnectorDocumentationLayout/DocumentationPanelContext", () => {

jest.setTimeout(10000);

const connectorDefinition = {
sourceDefinitionId: "1",
documentationUrl: "",
} as ConnectorDefinition;

const useAddPriceListItem = (container: HTMLElement) => {
const priceList = getByTestId(container, "connectionConfiguration.priceList");
let index = 0;
Expand Down Expand Up @@ -162,6 +168,7 @@ describe("Service Form", () => {
<ConnectorForm
formType="source"
onSubmit={handleSubmit}
selectedConnectorDefinition={connectorDefinition}
selectedConnectorDefinitionSpecification={
// @ts-expect-error Partial objects for testing
{
Expand Down Expand Up @@ -245,6 +252,7 @@ describe("Service Form", () => {
onSubmit={async (values) => {
result = values;
}}
selectedConnectorDefinition={connectorDefinition}
selectedConnectorDefinitionSpecification={
// @ts-expect-error Partial objects for testing
{
Expand Down Expand Up @@ -390,6 +398,7 @@ describe("Service Form", () => {

it("should render <CreateControls /> if connector is selected", async () => {
const { getByText } = await renderConnectorForm({
selectedConnectorDefinition: connectorDefinition,
selectedConnectorDefinitionSpecification:
// @ts-expect-error Partial objects for testing
connectorDefSpec as DestinationDefinitionSpecificationRead,
Expand All @@ -399,20 +408,9 @@ describe("Service Form", () => {
expect(getByText(/Set up destination/)).toBeInTheDocument();
});

it("should not render <CreateControls /> if connector is not selected", async () => {
const { container } = await renderConnectorForm({
selectedConnectorDefinitionSpecification: undefined,
formType: "destination",
onSubmit: onSubmitClb,
});

const submitBtn = container.querySelector('button[type="submit"]');

expect(submitBtn).toBeNull();
});

it("should render <EditControls /> if connector is selected", async () => {
const { getByText } = await renderConnectorForm({
selectedConnectorDefinition: connectorDefinition,
selectedConnectorDefinitionSpecification:
// @ts-expect-error Partial objects for testing
connectorDefSpec as DestinationDefinitionSpecificationRead,
Expand All @@ -423,18 +421,5 @@ describe("Service Form", () => {

expect(getByText(/Save changes and test/)).toBeInTheDocument();
});

it("should render <EditControls /> if connector is not selected", async () => {
const { container } = await renderConnectorForm({
selectedConnectorDefinitionSpecification: undefined,
formType: "destination",
onSubmit: onSubmitClb,
isEditMode: true,
});

const submitBtn = container.querySelector('button[type="submit"]');

expect(submitBtn).toBeInTheDocument();
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -87,14 +87,12 @@ const RevalidateOnValidationSchemaChange: React.FC<{ validationSchema: unknown }
export interface ConnectorFormProps {
formType: "source" | "destination";
formId?: string;
selectedConnectorDefinition?: ConnectorDefinition;
selectedConnectorDefinitionSpecification?: ConnectorDefinitionSpecification;
selectedConnectorDefinition: ConnectorDefinition;
selectedConnectorDefinitionSpecification: ConnectorDefinitionSpecification;
onSubmit: (values: ConnectorFormValues) => Promise<void>;
isLoading?: boolean;
isEditMode?: boolean;
formValues?: Partial<ConnectorFormValues>;
hasSuccess?: boolean;
fetchingConnectorError?: Error | null;
errorMessage?: React.ReactNode;
successMessage?: React.ReactNode;
connectorId?: string;
Expand All @@ -112,7 +110,6 @@ export const ConnectorForm: React.FC<ConnectorFormProps> = (props) => {
formType,
formValues,
onSubmit,
isLoading,
isEditMode,
isTestConnectionInProgress,
onStopTesting,
Expand All @@ -132,13 +129,13 @@ export const ConnectorForm: React.FC<ConnectorFormProps> = (props) => {
...(selectedConnectorDefinitionSpecification ? { name: { type: "string" } } : {}),
...Object.fromEntries(
Object.entries({
connectionConfiguration: isLoading ? null : specifications,
connectionConfiguration: specifications,
}).filter(([, v]) => !!v)
),
},
required: ["name"],
}),
[isLoading, selectedConnectorDefinitionSpecification, specifications]
[selectedConnectorDefinitionSpecification, specifications]
);

const { formFields, initialValues } = useBuildForm(jsonSchema, formValues);
Expand Down Expand Up @@ -199,7 +196,6 @@ export const ConnectorForm: React.FC<ConnectorFormProps> = (props) => {
selectedConnectorDefinition={selectedConnectorDefinition}
selectedConnectorDefinitionSpecification={selectedConnectorDefinitionSpecification}
isEditMode={isEditMode}
isLoadingSchema={isLoading}
validationSchema={validationSchema}
connectorId={connectorId}
>
Expand All @@ -209,7 +205,6 @@ export const ConnectorForm: React.FC<ConnectorFormProps> = (props) => {
<PatchInitialValuesWithWidgetConfig schema={jsonSchema} initialValues={initialValues} />
<FormRoot
{...props}
selectedConnector={selectedConnectorDefinitionSpecification}
formFields={formFields}
errorMessage={errorMessage}
isTestConnectionInProgress={isTestConnectionInProgress}
Expand Down

This file was deleted.

Loading

0 comments on commit 0e6e02f

Please sign in to comment.