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

🪟🚨 Refactor connector form code #20146

Merged
merged 75 commits into from
Jan 3, 2023
Merged
Show file tree
Hide file tree
Changes from 63 commits
Commits
Show all changes
75 commits
Select commit Hold shift + click to select a range
d2ed762
improve some types
Dec 5, 2022
f474a1c
Merge remote-tracking branch 'upstream/master' into chore-improve-con…
Dec 5, 2022
5b0521f
improve further
Dec 5, 2022
af659fd
Merge remote-tracking branch 'upstream/master' into chore-improve-con…
Dec 6, 2022
1cd3bbb
clean up a bit more
Dec 6, 2022
807c436
refactor loading state
Dec 6, 2022
6c76e0f
Merge remote-tracking branch 'upstream/master' into flash1293/connect…
Dec 6, 2022
05c3107
move loading state up
Dec 6, 2022
eee4303
remove isLoading references
Dec 6, 2022
7d6f977
remove unused props and make fetch connector error work
Dec 6, 2022
0fe11f0
remove special component for name
Dec 6, 2022
34d4814
remove top level state for unifinished flows
Dec 6, 2022
67d62c5
Merge remote-tracking branch 'upstream/master' into flash1293/connect…
Dec 6, 2022
afea882
Merge branch 'flash1293/connector-form-name-override' into flash1293/…
Dec 6, 2022
c7e7475
Merge branch 'master' into flash1293/connector-form-loading-state
Dec 6, 2022
cc7f65f
Merge branch 'chore-improve-connector-form-types' into flash1293/conn…
Dec 6, 2022
a1c6cbd
start removing uiwidget
Dec 6, 2022
9be8337
Update airbyte-webapp/src/views/Connector/ConnectorCard/ConnectorCard…
Dec 6, 2022
37cee8f
Merge remote-tracking branch 'upstream/master' into flash1293/connect…
Dec 6, 2022
5686241
Merge branch 'flash1293/connector-form-loading-state' of github.com:a…
Dec 6, 2022
7b73364
remove undefined option for selected id
Dec 6, 2022
7058d26
Merge branch 'flash1293/connector-form-loading-state' into flash1293/…
Dec 6, 2022
dd7e82a
Merge branch 'flash1293/connector-form-name-override' into flash1293/…
Dec 6, 2022
6d4c7d3
Merge remote-tracking branch 'upstream/master' into flash1293/connect…
Dec 7, 2022
d624792
Merge branch 'flash1293/connector-form-loading-state' into flash1293/…
Dec 7, 2022
36cf941
Merge branch 'flash1293/connector-form-name-override' into flash1293/…
Dec 7, 2022
b7a6322
Merge remote-tracking branch 'upstream/master' into flash1293/connect…
Dec 7, 2022
72b602a
Merge branch 'flash1293/connector-form-loading-state' into flash1293/…
Dec 7, 2022
5e47a4a
Merge branch 'flash1293/connector-form-name-override' into flash1293/…
Dec 7, 2022
6c71028
Merge remote-tracking branch 'origin/master' into flash1293/connector…
Dec 8, 2022
809625f
remove unused prop
Dec 8, 2022
10446eb
Merge branch 'flash1293/connector-form-loading-state' into flash1293/…
Dec 8, 2022
4a7a885
Merge branch 'flash1293/connector-form-name-override' into flash1293/…
Dec 8, 2022
2024b60
Merge branch 'flash1293/connector-form-unifinished-flows' into flash1…
Dec 8, 2022
ee30092
fix types
Dec 8, 2022
d91df38
remove uiwidget state
Dec 8, 2022
93fe558
Merge remote-tracking branch 'origin/master' into flash1293/connector…
Dec 9, 2022
cbc023a
Merge branch 'flash1293/connector-form-name-override' into flash1293/…
Dec 9, 2022
ec54d38
Merge branch 'flash1293/connector-form-unifinished-flows' into flash1…
Dec 9, 2022
47152ea
clean up
Dec 9, 2022
6ec0e10
adjust comment
Dec 9, 2022
1aa7829
handle errors in a nice way
Dec 9, 2022
7b90088
do not respect default on oneOf fields
Dec 9, 2022
fbefbdd
rename to formblock
Dec 9, 2022
6b3e5e2
reduce re-renders
Dec 9, 2022
9aa3f80
Merge remote-tracking branch 'origin/master' into flash1293/connector…
Dec 12, 2022
c765003
Merge branch 'flash1293/connector-form-unifinished-flows' into flash1…
Dec 12, 2022
5cd77ec
pass error to secure inputs
Dec 12, 2022
5dfbc22
Merge remote-tracking branch 'origin/master' into flash1293/connector…
Dec 13, 2022
a6aa741
simplify and improve styling
Dec 13, 2022
95717d2
align top
Dec 13, 2022
40d162d
Merge branch 'flash1293/connector-form-unifinished-flows' into flash1…
Dec 13, 2022
b9384ee
Merge remote-tracking branch 'origin/master' into flash1293/connector…
Dec 13, 2022
582a5f2
code review
Dec 13, 2022
18d6ce8
remove comment
Dec 13, 2022
60c1f1c
Merge branch 'master' into flash1293/connector-form-remove-ui-widget-…
Dec 14, 2022
9cb595d
Merge remote-tracking branch 'origin/master' into flash1293/connector…
Dec 15, 2022
fe74a45
review comments
Dec 15, 2022
5c00ad0
rename file
Dec 15, 2022
bd0dffc
be strict about boolean values
Dec 15, 2022
1ff8c13
add example
Dec 15, 2022
c6ee526
track form error in error boundary
Dec 15, 2022
05bbd6f
Merge branch 'flash1293/connector-form-remove-ui-widget-state' of git…
Dec 15, 2022
fff4703
Merge remote-tracking branch 'origin/master' into flash1293/connector…
Dec 16, 2022
94ba866
review comments
Dec 16, 2022
97bb0a7
handle unexpected cases better
Dec 16, 2022
633b9b2
Merge branch 'master' into flash1293/connector-form-remove-ui-widget-…
Dec 16, 2022
e47517f
Merge branch 'master' into flash1293/connector-form-remove-ui-widget-…
Dec 16, 2022
6889f66
enrich error with connector id
Dec 19, 2022
d5e56eb
Merge branch 'master' into flash1293/connector-form-remove-ui-widget-…
Dec 19, 2022
526adfc
rename prop
Dec 20, 2022
c5da764
Merge remote-tracking branch 'origin/master' into flash1293/connector…
Dec 20, 2022
9ccf294
Merge branch 'master' into flash1293/connector-form-remove-ui-widget-…
Dec 22, 2022
16c71ae
Merge branch 'master' into flash1293/connector-form-remove-ui-widget-…
Jan 2, 2023
92722e0
Merge branch 'master' into flash1293/connector-form-remove-ui-widget-…
Jan 3, 2023
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
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,12 @@ class ApiErrorBoundaryComponent extends React.Component<
}
}

componentDidCatch(error: { message: string; status?: number; __type?: string }) {
if (isFormBuildError(error)) {
this.props.trackError(error);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice if this could include a second argument to add some metadata about these errors, so they are easier to discover in datadog. I'm thinking of something like how errors are tracked in the useAuthenticator hook:

trackError(e, {
id: "useAuthentication.getValues",
connector: connectorSpec ? ConnectorSpecification.id(connectorSpec) : null,
});

If this could include an id like "formBuildError", and also attach the connector ID to it, then we could see which connector the error occurred for. This seems important because I don't know how we would address a generic form build error that we see in datadog unless we know what connector it occurred for.

}
}

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

if (errorId === ErrorId.FormBuild) {
return (
<ErrorOccurredView
message={message}
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 +134,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
7 changes: 7 additions & 0 deletions airbyte-webapp/src/core/form/FormBuildError.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export class FormBuildError extends Error {
__type = "form.build";
}

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