-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Connection Form Refactor - Part Two #16748
Changes from 144 commits
10b4ff1
2f0e83f
183087a
b3bc5ff
85fe161
0e063b1
d7ad611
468d840
8834c04
b99d1b3
25f28ba
8272377
e85d301
bf6a647
33caf95
68a6ad8
9bf2c73
8af1a41
4ed21ec
eb24031
1d4b3a7
e41a42f
dfcd4cc
eb54ec2
34434d2
be4001b
9d95504
b53abb8
50bbb3f
65584ca
3da331e
e841140
92b7c0c
8c5558b
d390833
3d094fb
3ec2cf2
0452458
df444fe
aec7b10
6aabc3b
8af9bd2
c9ca5b5
f809c68
61251a0
208cf78
c7f9afa
cc91839
e89f762
96b12a5
3395828
4c17657
498743a
ed26655
955848c
a0beff7
e823d60
e2dc6e1
7f4916a
19f051c
cc5b939
e0a2811
30d81f5
aa56b25
cecb46d
3f948d8
cec5034
5ac6baa
4961461
184732f
50df6de
c9aeb58
c375056
a45d98a
c2c9f96
c9c7fec
dfcee9d
31d0ccf
793edd0
20ecdd1
e648c16
9862aed
b93ad8e
858609a
efcfcc2
bcc2bdf
3d0c43e
1f5c04e
6fd2098
664a14a
bcfc57b
33ce0cc
c9baa9a
cdcb20d
4203136
7ac8eb1
a0a9ff5
f84e214
f05a936
5b269fb
5c5c3b7
4578ddd
6997f45
6839074
93aaf68
5896e66
89b6e45
599128d
4b02323
5b95466
621c186
8a8a084
bc31bde
2764a78
a0277af
d696b0c
023b502
c2e36eb
ab504ca
4aaa6e3
9e70086
aaaeaae
38bb9b6
f3c3281
6dcbbae
27f790f
e791029
4dc6be4
4e3b9f7
22397e6
f80e8e6
ddc1573
ca576d4
0b33c07
f9432bd
ea58502
ca2d9e4
9510f76
330a151
82905e7
d1442ba
fb8fb88
e95bb38
562af75
4abbec4
55e0372
a0c5d9f
07a46df
ce49bdf
9812234
5c89063
c7e760e
7d8685f
e25f953
6443920
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
@use "../../scss/variables"; | ||
|
||
.connectionFormContainer { | ||
width: 100%; | ||
padding: 0 variables.$spacing-xl; | ||
|
||
> form { | ||
display: flex; | ||
flex-direction: column; | ||
gap: variables.$spacing-md; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,76 @@ | ||
/* eslint-disable @typescript-eslint/no-non-null-assertion */ | ||
import { act, render, RenderResult } from "@testing-library/react"; | ||
import mockConnection from "test-utils/mock-data/mockConnection.json"; | ||
import mockDest from "test-utils/mock-data/mockDestinationDefinition.json"; | ||
import { TestWrapper } from "test-utils/testutils"; | ||
|
||
import { AirbyteCatalog } from "core/request/AirbyteClient"; | ||
import * as sourceHook from "hooks/services/useSourceHook"; | ||
|
||
import { CreateConnectionForm } from "./CreateConnectionForm"; | ||
|
||
jest.mock("services/connector/DestinationDefinitionSpecificationService", () => ({ | ||
useGetDestinationDefinitionSpecification: () => mockDest, | ||
})); | ||
|
||
jest.mock("services/workspaces/WorkspacesService", () => ({ | ||
useCurrentWorkspace: () => ({}), | ||
useCurrentWorkspaceId: () => "workspace-id", | ||
})); | ||
|
||
describe("CreateConnectionForm", () => { | ||
const Wrapper: React.FC = ({ children }) => <TestWrapper>{children}</TestWrapper>; | ||
|
||
const baseUseDiscoverSchema = { | ||
schemaErrorStatus: null, | ||
isLoading: false, | ||
schema: mockConnection.syncCatalog as AirbyteCatalog, | ||
catalogId: "", | ||
onDiscoverSchema: () => Promise.resolve(), | ||
}; | ||
|
||
it("should render", async () => { | ||
let renderResult: RenderResult; | ||
jest.spyOn(sourceHook, "useDiscoverSchema").mockImplementationOnce(() => baseUseDiscoverSchema); | ||
await act(async () => { | ||
renderResult = render( | ||
<Wrapper> | ||
<CreateConnectionForm source={mockConnection.source} destination={mockConnection.destination} /> | ||
</Wrapper> | ||
); | ||
}); | ||
expect(renderResult!.container).toMatchSnapshot(); | ||
expect(renderResult!.queryByText("Please wait a little bit more…")).toBeFalsy(); | ||
}); | ||
|
||
it("should render when loading", async () => { | ||
let renderResult: RenderResult; | ||
jest | ||
.spyOn(sourceHook, "useDiscoverSchema") | ||
.mockImplementationOnce(() => ({ ...baseUseDiscoverSchema, isLoading: true })); | ||
await act(async () => { | ||
renderResult = render( | ||
<Wrapper> | ||
<CreateConnectionForm source={mockConnection.source} destination={mockConnection.destination} /> | ||
</Wrapper> | ||
); | ||
}); | ||
expect(renderResult!.container).toMatchSnapshot(); | ||
}); | ||
|
||
it("should render with an error", async () => { | ||
let renderResult: RenderResult; | ||
jest.spyOn(sourceHook, "useDiscoverSchema").mockImplementationOnce(() => ({ | ||
...baseUseDiscoverSchema, | ||
schemaErrorStatus: new Error("Test Error") as sourceHook.SchemaError, | ||
})); | ||
await act(async () => { | ||
renderResult = render( | ||
<Wrapper> | ||
<CreateConnectionForm source={mockConnection.source} destination={mockConnection.destination} /> | ||
</Wrapper> | ||
); | ||
}); | ||
expect(renderResult!.container).toMatchSnapshot(); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,159 @@ | ||
import { Form, Formik, FormikHelpers } from "formik"; | ||
import React, { Suspense, useCallback, useState } from "react"; | ||
import { useNavigate } from "react-router-dom"; | ||
|
||
import LoadingSchema from "components/LoadingSchema"; | ||
|
||
import { DestinationRead, SourceRead } from "core/request/AirbyteClient"; | ||
import { | ||
ConnectionFormServiceProvider, | ||
tidyConnectionFormValues, | ||
useConnectionFormService, | ||
} from "hooks/services/ConnectionForm/ConnectionFormService"; | ||
import { useFormChangeTrackerService } from "hooks/services/FormChangeTracker"; | ||
import { useCreateConnection } from "hooks/services/useConnectionHook"; | ||
import { SchemaError as SchemaErrorType, useDiscoverSchema } from "hooks/services/useSourceHook"; | ||
import { useCurrentWorkspaceId } from "services/workspaces/WorkspacesService"; | ||
import CreateControls from "views/Connection/ConnectionForm/components/CreateControls"; | ||
import { OperationsSection } from "views/Connection/ConnectionForm/components/OperationsSection"; | ||
import { ConnectionFormFields } from "views/Connection/ConnectionForm/ConnectionFormFields"; | ||
import { connectionValidationSchema, FormikConnectionFormValues } from "views/Connection/ConnectionForm/formConfig"; | ||
|
||
import styles from "./CreateConnectionForm.module.scss"; | ||
import { CreateConnectionNameField } from "./CreateConnectionNameField"; | ||
import { SchemaError } from "./SchemaError"; | ||
|
||
interface CreateConnectionProps { | ||
source: SourceRead; | ||
destination: DestinationRead; | ||
afterSubmitConnection?: () => void; | ||
} | ||
|
||
interface CreateConnectionPropsInner extends Pick<CreateConnectionProps, "afterSubmitConnection"> { | ||
schemaError: SchemaErrorType; | ||
} | ||
|
||
const CreateConnectionFormInner: React.FC<CreateConnectionPropsInner> = ({ schemaError, afterSubmitConnection }) => { | ||
const navigate = useNavigate(); | ||
|
||
const { mutateAsync: createConnection } = useCreateConnection(); | ||
|
||
const { clearFormChange } = useFormChangeTrackerService(); | ||
|
||
const workspaceId = useCurrentWorkspaceId(); | ||
|
||
const { connection, initialValues, mode, formId, getErrorMessage, setSubmitError } = useConnectionFormService(); | ||
const [editingTransformation, setEditingTransformation] = useState(false); | ||
|
||
const onFormSubmit = useCallback( | ||
async (formValues: FormikConnectionFormValues, formikHelpers: FormikHelpers<FormikConnectionFormValues>) => { | ||
const values = tidyConnectionFormValues(formValues, workspaceId, mode); | ||
|
||
try { | ||
const createdConnection = await createConnection({ | ||
values, | ||
source: connection.source, | ||
destination: connection.destination, | ||
sourceDefinition: { | ||
sourceDefinitionId: connection.source?.sourceDefinitionId ?? "", | ||
}, | ||
destinationDefinition: { | ||
name: connection.destination?.name ?? "", | ||
destinationDefinitionId: connection.destination?.destinationDefinitionId ?? "", | ||
}, | ||
sourceCatalogId: connection.catalogId, | ||
}); | ||
|
||
formikHelpers.resetForm(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Curious, why was this call added? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is carried over from previous iterations of the create connection's submit function. It might not be necessary but I've never removed it to check. |
||
// We need to clear the form changes otherwise the dirty form intercept service will prevent navigation | ||
clearFormChange(formId); | ||
|
||
if (afterSubmitConnection) { | ||
afterSubmitConnection(); | ||
} else { | ||
navigate(`../../connections/${createdConnection.connectionId}`); | ||
} | ||
} catch (e) { | ||
setSubmitError(e); | ||
} | ||
}, | ||
[ | ||
workspaceId, | ||
mode, | ||
createConnection, | ||
connection.source, | ||
connection.destination, | ||
connection.catalogId, | ||
clearFormChange, | ||
formId, | ||
afterSubmitConnection, | ||
navigate, | ||
setSubmitError, | ||
] | ||
); | ||
|
||
if (schemaError) { | ||
return <SchemaError schemaError={schemaError} />; | ||
} | ||
|
||
return ( | ||
<Suspense fallback={<LoadingSchema />}> | ||
<div className={styles.connectionFormContainer}> | ||
<Formik | ||
initialValues={initialValues} | ||
validationSchema={connectionValidationSchema(mode)} | ||
onSubmit={onFormSubmit} | ||
> | ||
{({ values, isSubmitting, isValid, dirty }) => ( | ||
<Form> | ||
<CreateConnectionNameField /> | ||
<ConnectionFormFields values={values} isSubmitting={isSubmitting} dirty={dirty} /> | ||
<OperationsSection | ||
onStartEditTransformation={() => setEditingTransformation(true)} | ||
onEndEditTransformation={() => setEditingTransformation(false)} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removed toggle. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Prone to errors in this use case |
||
/> | ||
<CreateControls | ||
isSubmitting={isSubmitting} | ||
isValid={isValid && !editingTransformation} | ||
errorMessage={getErrorMessage(isValid, dirty)} | ||
/> | ||
</Form> | ||
)} | ||
</Formik> | ||
</div> | ||
</Suspense> | ||
); | ||
}; | ||
|
||
export const CreateConnectionForm: React.FC<CreateConnectionProps> = ({ | ||
source, | ||
destination, | ||
afterSubmitConnection, | ||
}) => { | ||
const { schema, isLoading, schemaErrorStatus, catalogId, onDiscoverSchema } = useDiscoverSchema( | ||
source.sourceId, | ||
true | ||
); | ||
|
||
const partialConnection = { | ||
syncCatalog: schema, | ||
destination, | ||
source, | ||
catalogId, | ||
}; | ||
|
||
return ( | ||
<ConnectionFormServiceProvider | ||
connection={partialConnection} | ||
mode="create" | ||
refreshSchema={onDiscoverSchema} | ||
schemaError={schemaErrorStatus} | ||
> | ||
{isLoading ? ( | ||
<LoadingSchema /> | ||
) : ( | ||
<CreateConnectionFormInner afterSubmitConnection={afterSubmitConnection} schemaError={schemaErrorStatus} /> | ||
)} | ||
</ConnectionFormServiceProvider> | ||
); | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the best way to handle needing to change the implementation that I've found.