-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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: Add manual schema #20862
Changes from 109 commits
d2ed762
f474a1c
5b0521f
af659fd
1cd3bbb
807c436
6c76e0f
05c3107
eee4303
7d6f977
0fe11f0
34d4814
67d62c5
afea882
c7e7475
cc7f65f
a1c6cbd
9be8337
37cee8f
5686241
7b73364
7058d26
dd7e82a
6d4c7d3
d624792
36cf941
b7a6322
72b602a
5e47a4a
6c71028
809625f
10446eb
4a7a885
2024b60
ee30092
d91df38
93fe558
cbc023a
ec54d38
47152ea
6ec0e10
1aa7829
7b90088
fbefbdd
6b3e5e2
9aa3f80
c765003
5cd77ec
5dfbc22
a6aa741
95717d2
40d162d
b9384ee
582a5f2
18d6ce8
60c1f1c
9cb595d
fe74a45
5c00ad0
bd0dffc
1ff8c13
c6ee526
05bbd6f
fff4703
94ba866
97bb0a7
633b9b2
e47517f
e931956
6889f66
7b60406
d5e56eb
35a74ba
02c8096
526adfc
7d49233
c5da764
6c0bf9d
477977f
200cfe7
c7fd310
e0e3796
e23a6d8
610e668
e68ff49
9ccf294
ae9a229
a9e1d09
1965efe
c8f3920
e4f7bba
16c71ae
54c070d
a8b9aae
2e20fa6
65199d0
92722e0
718c233
c303a5d
6a8be0b
753241d
16ce351
25d6586
64a5eb9
1db62dd
2a3776f
abb9c0b
08e6d99
12db741
7088b2c
5423192
daae429
4a7ffc6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,7 @@ | |
display: flex; | ||
flex-direction: column; | ||
gap: variables.$spacing-md; | ||
height: 100%; | ||
} | ||
|
||
.heading { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,13 @@ | ||
import { faTrashCan, faCopy } from "@fortawesome/free-regular-svg-icons"; | ||
import { FontAwesomeIcon } from "@fortawesome/react-fontawesome"; | ||
import classNames from "classnames"; | ||
import { useField } from "formik"; | ||
import { useIntl } from "react-intl"; | ||
import { FormikErrors, useField } from "formik"; | ||
import { useState } from "react"; | ||
import { FormattedMessage, useIntl } from "react-intl"; | ||
|
||
import Indicator from "components/Indicator"; | ||
import { CodeEditor } from "components/ui/CodeEditor"; | ||
import { Text } from "components/ui/Text"; | ||
|
||
import { useConfirmationModalService } from "hooks/services/ConfirmationModal"; | ||
import { BuilderView, useConnectorBuilderState } from "services/connectorBuilder/ConnectorBuilderStateService"; | ||
|
@@ -23,14 +28,22 @@ interface StreamConfigViewProps { | |
} | ||
|
||
export const StreamConfigView: React.FC<StreamConfigViewProps> = ({ streamNum }) => { | ||
const streamPath = `streams[${streamNum}]`; | ||
const streamFieldPath = (fieldPath: string) => `${streamPath}.${fieldPath}`; | ||
|
||
const { formatMessage } = useIntl(); | ||
const [field, , helpers] = useField<BuilderStream[]>("streams"); | ||
const [field, meta, helpers] = useField<BuilderStream[]>("streams"); | ||
const currentStreamErrors = meta.error?.[streamNum] as FormikErrors<BuilderStream>; | ||
const hasSchemaErrors = Boolean(currentStreamErrors?.schema); | ||
const hasConfigErrors = Boolean( | ||
Object.keys(currentStreamErrors || {}).filter((errorKey) => errorKey !== "schema").length > 0 | ||
); | ||
const [selectedTab, setSelectedTab] = useState<"configuration" | "schema">( | ||
hasSchemaErrors && !hasConfigErrors ? "schema" : "configuration" | ||
); | ||
const { openConfirmationModal, closeConfirmationModal } = useConfirmationModalService(); | ||
const { setSelectedView, setTestStreamIndex } = useConnectorBuilderState(); | ||
|
||
const streamPath = `streams[${streamNum}]`; | ||
const streamFieldPath = (fieldPath: string) => `${streamPath}.${fieldPath}`; | ||
|
||
const handleDelete = () => { | ||
openConfirmationModal({ | ||
text: "connectorBuilder.deleteStreamModal.text", | ||
|
@@ -53,6 +66,18 @@ export const StreamConfigView: React.FC<StreamConfigViewProps> = ({ streamNum }) | |
{/* Not using intl for the labels and tooltips in this component in order to keep maintainence simple */} | ||
<BuilderTitle path={streamFieldPath("name")} label="Stream Name" size="md" /> | ||
<div className={styles.controls}> | ||
<StreamTab | ||
label={formatMessage({ id: "connectorBuilder.streamConfiguration" })} | ||
selected={selectedTab === "configuration"} | ||
onSelect={() => setSelectedTab("configuration")} | ||
showErrorIndicator={hasConfigErrors} | ||
/> | ||
<StreamTab | ||
label={formatMessage({ id: "connectorBuilder.streamSchema" })} | ||
selected={selectedTab === "schema"} | ||
onSelect={() => setSelectedTab("schema")} | ||
showErrorIndicator={hasSchemaErrors} | ||
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. Doesn't need to be done in this PR, but it would be nice if clicking the Test or Download Config buttons would cause the schema view to be focused if it has any errors. Currently it auto-focuses the Stream view, but it doesn't open the Schema tab automatically 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. Restructured this a bit and I think I found an acceptable way to handle this and the comment about showing an error indicator for the configuration tab. It's always selecting the view that has errors if there is one by default. 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. nit: should we also show this error indicator on the configuration tab if there are any errors in there? Maybe not necessary since we already show it on the sidebar |
||
/> | ||
<AddStreamButton | ||
onAddStream={(addedStreamNum) => { | ||
setSelectedView(addedStreamNum); | ||
|
@@ -69,53 +94,100 @@ export const StreamConfigView: React.FC<StreamConfigViewProps> = ({ streamNum }) | |
<FontAwesomeIcon icon={faTrashCan} /> | ||
</button> | ||
</div> | ||
<BuilderCard> | ||
<BuilderField | ||
type="string" | ||
path={streamFieldPath("urlPath")} | ||
label="Path URL" | ||
tooltip="Path of the endpoint that this stream represents." | ||
/> | ||
<BuilderField | ||
type="enum" | ||
path={streamFieldPath("httpMethod")} | ||
options={["GET", "POST"]} | ||
label="HTTP Method" | ||
tooltip="HTTP method to use for requests sent to the API" | ||
/> | ||
<BuilderField | ||
type="array" | ||
path={streamFieldPath("fieldPointer")} | ||
label="Record selector" | ||
tooltip="Pointer into the response that should be extracted as the final record" | ||
/> | ||
<BuilderField | ||
type="array" | ||
path={streamFieldPath("primaryKey")} | ||
label="Primary key" | ||
tooltip="Pointer into the response that should be used as the primary key when deduplicating records in the destination" | ||
optional | ||
/> | ||
</BuilderCard> | ||
<PaginationSection streamFieldPath={streamFieldPath} /> | ||
<StreamSlicerSection streamFieldPath={streamFieldPath} /> | ||
<BuilderCard> | ||
<KeyValueListField | ||
path={streamFieldPath("requestOptions.requestParameters")} | ||
label="Request Parameters" | ||
tooltip="Parameters to attach to API requests" | ||
/> | ||
<KeyValueListField | ||
path={streamFieldPath("requestOptions.requestHeaders")} | ||
label="Request Headers" | ||
tooltip="Headers to attach to API requests" | ||
/> | ||
<KeyValueListField | ||
path={streamFieldPath("requestOptions.requestBody")} | ||
label="Request Body" | ||
tooltip="Body to attach to API requests as url-encoded form values" | ||
/> | ||
</BuilderCard> | ||
{selectedTab === "configuration" ? ( | ||
<> | ||
<BuilderCard> | ||
<BuilderField | ||
type="string" | ||
path={streamFieldPath("urlPath")} | ||
label="Path URL" | ||
tooltip="Path of the endpoint that this stream represents." | ||
/> | ||
<BuilderField | ||
type="enum" | ||
path={streamFieldPath("httpMethod")} | ||
options={["GET", "POST"]} | ||
label="HTTP Method" | ||
tooltip="HTTP method to use for requests sent to the API" | ||
/> | ||
<BuilderField | ||
type="array" | ||
path={streamFieldPath("fieldPointer")} | ||
label="Record selector" | ||
tooltip="Pointer into the response that should be extracted as the final record" | ||
/> | ||
<BuilderField | ||
type="array" | ||
path={streamFieldPath("primaryKey")} | ||
label="Primary key" | ||
tooltip="Pointer into the response that should be used as the primary key when deduplicating records in the destination" | ||
optional | ||
/> | ||
</BuilderCard> | ||
<PaginationSection streamFieldPath={streamFieldPath} /> | ||
<StreamSlicerSection streamFieldPath={streamFieldPath} /> | ||
<BuilderCard> | ||
<KeyValueListField | ||
path={streamFieldPath("requestOptions.requestParameters")} | ||
label="Request Parameters" | ||
tooltip="Parameters to attach to API requests" | ||
/> | ||
<KeyValueListField | ||
path={streamFieldPath("requestOptions.requestHeaders")} | ||
label="Request Headers" | ||
tooltip="Headers to attach to API requests" | ||
/> | ||
<KeyValueListField | ||
path={streamFieldPath("requestOptions.requestBody")} | ||
label="Request Body" | ||
tooltip="Body to attach to API requests as url-encoded form values" | ||
/> | ||
</BuilderCard> | ||
</> | ||
) : ( | ||
<BuilderCard className={styles.schemaEditor}> | ||
<SchemaEditor streamFieldPath={streamFieldPath} /> | ||
</BuilderCard> | ||
)} | ||
</BuilderConfigView> | ||
); | ||
}; | ||
|
||
const StreamTab = ({ | ||
selected, | ||
label, | ||
onSelect, | ||
showErrorIndicator, | ||
}: { | ||
selected: boolean; | ||
label: string; | ||
onSelect: () => void; | ||
showErrorIndicator?: boolean; | ||
}) => ( | ||
<button type="button" className={classNames(styles.tab, { [styles.selectedTab]: selected })} onClick={onSelect}> | ||
{label} | ||
{showErrorIndicator && <Indicator />} | ||
</button> | ||
); | ||
|
||
const SchemaEditor = ({ streamFieldPath }: { streamFieldPath: (fieldPath: string) => string }) => { | ||
const [field, meta, helpers] = useField<string | undefined>(streamFieldPath("schema")); | ||
|
||
return ( | ||
<> | ||
<CodeEditor | ||
value={field.value || ""} | ||
language="json" | ||
theme="airbyte-light" | ||
onChange={(val: string | undefined) => { | ||
helpers.setValue(val); | ||
}} | ||
/> | ||
{meta.error && ( | ||
<Text className={styles.errorMessage}> | ||
<FormattedMessage id={meta.error} /> | ||
</Text> | ||
)} | ||
</> | ||
); | ||
}; |
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.
One small issue with just looking at
meta.error
directly is that fields which have errors but are untouched cause the error indicator to show on the tab, but the error is. not shown on the field because it is untouched.Could we use the
useBuilderErrors
custom hook instead to ignore untouched fields, so that the error indicator is only ever shown if the error is also being shown on the field?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.
Ah, right, forgot about that. We can't use useBuilderErrors directly because it can't split out the
schema
bit so we can have separate error indicators for the configuration and the schema tabs. I could try to get that into the existing logic, but we already have the issue to refactor this to make it easier to work, I would rather like to do that first.I reverted my attempt from yesterday and going to merge like that for now and open a follow-up PR.