Skip to content

Commit

Permalink
🪟🚨 Refactor connector form code (#20146)
Browse files Browse the repository at this point in the history
* Remove uiWidgetsInfo state by finding the enum/const field defining the selected oneOf and adding it to the condition form block to check the selected value by looking at the formik state
* Only do default value calculation once before rendering the formik component the first time (remove PatchInitialValuesWithWidgetConfig hack)
* Build yup schema once by flattening the oneOf conditions into a single object and adding when conditions on the found condition key (remove RevalidateOnValidationSchemaChange hack)

The snowflake destination in version <=0.4.40 does not work together with the changes on this PR - existing connections will continue to work fine, but it's not possible to change the configuration. Please update the snowflake destination connector to 0.4.41.

oneOf properties not following the rules described in the documentation will stop working in the UI - the form will crash and a meaningful error is shown which also links to the documentation:
  • Loading branch information
Joe Reuter authored Jan 3, 2023
1 parent 2f2e530 commit 99905b2
Show file tree
Hide file tree
Showing 25 changed files with 781 additions and 898 deletions.
10 changes: 6 additions & 4 deletions airbyte-webapp/src/components/GroupControls/index.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,16 +38,18 @@ const propTwoFormBlock: FormBlock = {
};

const conditionFormField: FormConditionItem = {
conditions: {
ChoiceOne: {
conditions: [
{
isRequired: true,
_type: "formGroup",
fieldKey: "choice_one_key",
path: "section.conditional.choice_one",
jsonSchema: {},
properties: [propOneFormBlock, propTwoFormBlock],
},
},
],
selectionPath: "section.conditional.choice_one.type",
selectionKey: "type",
selectionConstValues: ["one"],
isRequired: true,
_type: "formCondition",
fieldKey: "field_key",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ import { NavigateFunction, useNavigate } from "react-router-dom";
import { useLocation } from "react-use";
import { LocationSensorState } from "react-use/lib/useLocation";

import { isFormBuildError } from "core/form/FormBuildError";
import { isVersionError } from "core/request/VersionError";
import { TrackErrorFn, useAppMonitoringService } from "hooks/services/AppMonitoringService";
import { ErrorOccurredView } from "views/common/ErrorOccurredView";
import { ResourceNotFoundErrorBoundary } from "views/common/ResorceNotFoundErrorBoundary";
import { StartOverErrorView } from "views/common/StartOverErrorView";
Expand All @@ -21,6 +23,7 @@ interface ApiErrorBoundaryState {

enum ErrorId {
VersionMismatch = "version.mismatch",
FormBuild = "form.build",
ServerUnavailable = "server.unavailable",
UnknownError = "unknown",
}
Expand All @@ -29,6 +32,7 @@ interface ApiErrorBoundaryHookProps {
location: LocationSensorState;
onRetry?: () => void;
navigate: NavigateFunction;
trackError: TrackErrorFn;
}

interface ApiErrorBoundaryProps {
Expand All @@ -51,6 +55,10 @@ class ApiErrorBoundaryComponent extends React.Component<
return { errorId: ErrorId.VersionMismatch, message: error.message };
}

if (isFormBuildError(error)) {
return { errorId: ErrorId.FormBuild, message: error.message };
}

const isNetworkBoundaryMessage = error.message === "Failed to fetch";
const is502 = error.status === 502;

Expand All @@ -72,6 +80,15 @@ class ApiErrorBoundaryComponent extends React.Component<
}
}

componentDidCatch(error: { message: string; status?: number; __type?: string }) {
if (isFormBuildError(error)) {
this.props.trackError(error, {
id: "formBuildError",
connectorDefinitionId: error.connectorDefinitionId,
});
}
}

retry = () => {
this.setState((state) => ({
didRetry: true,
Expand All @@ -89,6 +106,21 @@ class ApiErrorBoundaryComponent extends React.Component<
return <ErrorOccurredView message={message} />;
}

if (errorId === ErrorId.FormBuild) {
return (
<ErrorOccurredView
message={
<>
<FormattedMessage id={message} />
<br />
<FormattedMessage id="errorView.upgradeConnectors" />
</>
}
docLink="https://docs.airbyte.com/connector-development/connector-specification-reference/#airbyte-modifications-to-jsonschema"
/>
);
}

if (errorId === ErrorId.ServerUnavailable && !didRetry) {
return <ServerUnavailableView retryDelay={retryDelay || RETRY_DELAY} onRetryClick={this.retry} />;
}
Expand All @@ -111,9 +143,16 @@ export const ApiErrorBoundary: React.FC<React.PropsWithChildren<ApiErrorBoundary
const { reset } = useQueryErrorResetBoundary();
const location = useLocation();
const navigate = useNavigate();
const { trackError } = useAppMonitoringService();

return (
<ApiErrorBoundaryComponent {...props} location={location} navigate={navigate} onRetry={reset}>
<ApiErrorBoundaryComponent
{...props}
location={location}
navigate={navigate}
onRetry={reset}
trackError={trackError}
>
{children}
</ApiErrorBoundaryComponent>
);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { AirbyteJSONSchema } from "core/jsonSchema";
import { AirbyteJSONSchema } from "core/jsonSchema/types";

import { traverseSchemaToField } from "./traverseSchemaToField";

Expand Down
2 changes: 1 addition & 1 deletion airbyte-webapp/src/core/domain/connection/types.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { AirbyteJSONSchema } from "core/jsonSchema";
import { AirbyteJSONSchema } from "core/jsonSchema/types";

type ConnectionConfiguration = unknown;

Expand Down
11 changes: 11 additions & 0 deletions airbyte-webapp/src/core/form/FormBuildError.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
export class FormBuildError extends Error {
__type = "form.build";

constructor(public message: string, public connectorDefinitionId?: string) {
super(message);
}
}

export function isFormBuildError(error: { __type?: string }): error is FormBuildError {
return error.__type === "form.build";
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { FormGroupItem } from "core/form/types";
import { AirbyteJSONSchemaDefinition } from "core/jsonSchema/types";

import { jsonSchemaToUiWidget } from "./schemaToUiWidget";
import { AirbyteJSONSchemaDefinition } from "./types";
import { jsonSchemaToFormBlock } from "./schemaToFormBlock";

it("should reformat jsonSchema to internal widget representation", () => {
const schema: AirbyteJSONSchemaDefinition = {
Expand All @@ -26,40 +26,13 @@ it("should reformat jsonSchema to internal widget representation", () => {
},
};

const builtSchema = jsonSchemaToUiWidget(schema, "key");
const builtSchema = jsonSchemaToFormBlock(schema, "key");

const expected = {
_type: "formGroup",
path: "key",
fieldKey: "key",
isRequired: false,
jsonSchema: {
properties: {
dbname: {
description: "Name of the database.",
type: "string",
},
host: {
description: "Hostname of the database.",
type: "string",
},
password: {
airbyte_secret: true,
description: "Password associated with the username.",
type: "string",
},
port: {
description: "Port of the database.",
type: "integer",
},
user: {
description: "Username to use to access the database.",
type: "string",
},
},
required: ["host", "port", "user", "dbname"],
type: "object",
},
properties: [
{
_type: "formItem",
Expand Down Expand Up @@ -128,7 +101,7 @@ it("should turn single enum into const but keep multi value enum", () => {
},
};

const builtSchema = jsonSchemaToUiWidget(schema, "key");
const builtSchema = jsonSchemaToFormBlock(schema, "key");

const expectedProperties = [
{
Expand Down Expand Up @@ -176,7 +149,7 @@ it("should reformat jsonSchema to internal widget representation with parent sch
},
};

const builtSchema = jsonSchemaToUiWidget(schema, "key", undefined, {
const builtSchema = jsonSchemaToFormBlock(schema, "key", undefined, {
required: ["key"],
});

Expand All @@ -185,17 +158,6 @@ it("should reformat jsonSchema to internal widget representation with parent sch
fieldKey: "key",
path: "key",
isRequired: true,
jsonSchema: {
properties: {
host: {
description: "Hostname of the database.",
type: "string",
},
},
required: ["host", "port", "user", "dbname"],
title: "Postgres Source Spec",
type: "object",
},
properties: [
{
_type: "formItem",
Expand Down Expand Up @@ -235,6 +197,11 @@ it("should reformat jsonSchema to internal widget representation when has oneOf"
api_key: {
type: "string",
},
type: {
type: "string",
const: "api",
default: "api",
},
},
},
{
Expand All @@ -245,49 +212,24 @@ it("should reformat jsonSchema to internal widget representation when has oneOf"
type: "string",
examples: ["https://api.hubspot.com/"],
},
type: {
type: "string",
const: "oauth",
default: "oauth",
},
},
},
],
},
},
};

const builtSchema = jsonSchemaToUiWidget(schema, "key", undefined, {
const builtSchema = jsonSchemaToFormBlock(schema, "key", undefined, {
required: ["key"],
});

const expected = {
_type: "formGroup",
jsonSchema: {
type: "object",
required: ["start_date", "credentials"],
properties: {
start_date: { type: "string" },
credentials: {
type: "object",
description: "Credentials Condition Description",
title: "Credentials Condition",
order: 0,
oneOf: [
{
title: "api key",
required: ["api_key"],
properties: { api_key: { type: "string" } },
},
{
title: "oauth",
required: ["redirect_uri"],
properties: {
redirect_uri: {
type: "string",
examples: ["https://api.hubspot.com/"],
},
},
},
],
},
},
},
path: "key",
fieldKey: "key",
properties: [
Expand All @@ -307,16 +249,13 @@ it("should reformat jsonSchema to internal widget representation when has oneOf"
title: "Credentials Condition",
order: 0,
fieldKey: "credentials",
conditions: {
"api key": {
selectionConstValues: ["api", "oauth"],
selectionKey: "type",
selectionPath: "key.credentials.type",
conditions: [
{
title: "api key",
_type: "formGroup",
jsonSchema: {
title: "api key",
required: ["api_key"],
type: "object",
properties: { api_key: { type: "string" } },
},
path: "key.credentials",
fieldKey: "credentials",
properties: [
Expand All @@ -329,23 +268,24 @@ it("should reformat jsonSchema to internal widget representation when has oneOf"
multiline: false,
type: "string",
},
{
_type: "formItem",
const: "api",
default: "api",
fieldKey: "type",
format: undefined,
isRequired: false,
isSecret: false,
multiline: false,
path: "key.credentials.type",
type: "string",
},
],
isRequired: false,
},
oauth: {
{
title: "oauth",
_type: "formGroup",
jsonSchema: {
title: "oauth",
required: ["redirect_uri"],
type: "object",
properties: {
redirect_uri: {
type: "string",
examples: ["https://api.hubspot.com/"],
},
},
},
path: "key.credentials",
fieldKey: "credentials",
properties: [
Expand All @@ -359,10 +299,22 @@ it("should reformat jsonSchema to internal widget representation when has oneOf"
multiline: false,
type: "string",
},
{
_type: "formItem",
const: "oauth",
default: "oauth",
fieldKey: "type",
format: undefined,
isRequired: false,
isSecret: false,
multiline: false,
path: "key.credentials.type",
type: "string",
},
],
isRequired: false,
},
},
],
isRequired: true,
},
],
Expand Down
Loading

0 comments on commit 99905b2

Please sign in to comment.