Skip to content

Commit 47e85d1

Browse files
lmossmanlmossman
committed
🪟 🎉 Better connector setup error messages (#9725)
1 parent 962061b commit 47e85d1

File tree

7 files changed

+66
-49
lines changed

7 files changed

+66
-49
lines changed

‎airbyte-webapp/src/components/connectorBuilder/StreamTestingPanel/StreamTestingPanel.tsx

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import React, { useMemo } from "react";
2-
import { FormattedMessage } from "react-intl";
2+
import { FormattedMessage, useIntl } from "react-intl";
33
import { ValidationError } from "yup";
44

55
import { Heading } from "components/ui/Heading";
@@ -27,11 +27,13 @@ import { useBuilderWatch } from "../types";
2727
const EMPTY_SCHEMA = {};
2828

2929
function useTestInputJsonErrors(testInputJson: ConnectorConfig | undefined, spec?: Spec): number {
30+
const { formatMessage } = useIntl();
31+
3032
return useMemo(() => {
3133
try {
3234
const jsonSchema = spec && spec.connection_specification ? spec.connection_specification : EMPTY_SCHEMA;
3335
const formFields = jsonSchemaToFormBlock(jsonSchema);
34-
const validationSchema = buildYupFormForJsonSchema(jsonSchema, formFields);
36+
const validationSchema = buildYupFormForJsonSchema(jsonSchema, formFields, formatMessage);
3537
validationSchema.validateSync(testInputJson, { abortEarly: false });
3638
return 0;
3739
} catch (e) {
@@ -40,7 +42,7 @@ function useTestInputJsonErrors(testInputJson: ConnectorConfig | undefined, spec
4042
}
4143
return 1;
4244
}
43-
}, [testInputJson, spec]);
45+
}, [spec, formatMessage, testInputJson]);
4446
}
4547

4648
export const StreamTestingPanel: React.FC<unknown> = () => {

‎airbyte-webapp/src/core/form/schemaToYup.test.ts

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { MessageDescriptor } from "react-intl";
12
import * as yup from "yup";
23

34
import { AirbyteJSONSchema } from "core/jsonSchema/types";
@@ -6,6 +7,10 @@ import { jsonSchemaToFormBlock } from "./schemaToFormBlock";
67
import { buildYupFormForJsonSchema } from "./schemaToYup";
78
import { FORM_PATTERN_ERROR } from "./types";
89

10+
const formatMessage = (message: MessageDescriptor, values?: Record<string, string | number>) => {
11+
return `${message.id}${values ? `: ${JSON.stringify(values)}` : ""}`;
12+
};
13+
914
// Note: We have to check yup schema with JSON.stringify
1015
// as exactly same objects throw now equality due to `Received: serializes to the same string` error
1116
it("should build schema for simple case", () => {
@@ -46,7 +51,7 @@ it("should build schema for simple case", () => {
4651
},
4752
additionalProperties: false,
4853
};
49-
const yupSchema = buildYupFormForJsonSchema(schema, jsonSchemaToFormBlock(schema));
54+
const yupSchema = buildYupFormForJsonSchema(schema, jsonSchemaToFormBlock(schema), formatMessage);
5055

5156
const expectedSchema = yup.object().shape({
5257
host: yup.string().trim().required("form.empty.error").transform(String),
@@ -116,7 +121,11 @@ const simpleConditionalSchema: AirbyteJSONSchema = {
116121
};
117122

118123
it("should build correct mixed schema structure for conditional case", () => {
119-
const yupSchema = buildYupFormForJsonSchema(simpleConditionalSchema, jsonSchemaToFormBlock(simpleConditionalSchema));
124+
const yupSchema = buildYupFormForJsonSchema(
125+
simpleConditionalSchema,
126+
jsonSchemaToFormBlock(simpleConditionalSchema),
127+
formatMessage
128+
);
120129

121130
const expectedSchema = yup.object().shape({
122131
start_date: yup.string().trim().required("form.empty.error").transform(String),
@@ -143,7 +152,12 @@ it("should build correct mixed schema structure for conditional case", () => {
143152

144153
// These tests check whether the built yup schema validates as expected, it is not checking the structure
145154
describe("yup schema validations", () => {
146-
const yupSchema = buildYupFormForJsonSchema(simpleConditionalSchema, jsonSchemaToFormBlock(simpleConditionalSchema));
155+
const yupSchema = buildYupFormForJsonSchema(
156+
simpleConditionalSchema,
157+
jsonSchemaToFormBlock(simpleConditionalSchema),
158+
formatMessage
159+
);
160+
147161
it("enforces required props for selected condition", () => {
148162
expect(() => {
149163
yupSchema.validateSync({
@@ -184,7 +198,7 @@ describe("yup schema validations", () => {
184198
api_key: "X",
185199
},
186200
});
187-
}).toThrow(FORM_PATTERN_ERROR);
201+
}).toThrow(`${FORM_PATTERN_ERROR}: {"pattern":"\\\\w{5}"}`);
188202
});
189203

190204
it("strips out properties belonging to other conditions", () => {

‎airbyte-webapp/src/core/form/schemaToYup.ts

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,17 @@
1-
import { JSONSchema7, JSONSchema7Type } from "json-schema";
1+
import { JSONSchema7Type } from "json-schema";
2+
import { MessageDescriptor } from "react-intl";
23
import * as yup from "yup";
34

45
import { FormBlock, FormGroupItem, FormObjectArrayItem, FormConditionItem, FORM_PATTERN_ERROR } from "core/form/types";
6+
import { AirbyteJSONSchema } from "core/jsonSchema/types";
7+
import { getPatternDescriptor } from "views/Connector/ConnectorForm/utils";
58

69
import { FormBuildError } from "./FormBuildError";
710

811
/**
912
* Returns yup.schema for validation
1013
*
11-
* This method builds yup schema based on jsonSchema ${@link JSONSchema7} and the derived ${@link FormBlock}.
14+
* This method builds yup schema based on jsonSchema ${@link AirbyteJSONSchema} and the derived ${@link FormBlock}.
1215
* Every property is walked through recursively in case it is condition | object | array.
1316
*
1417
* @param jsonSchema
@@ -18,9 +21,10 @@ import { FormBuildError } from "./FormBuildError";
1821
* @param propertyPath constructs path of property
1922
*/
2023
export const buildYupFormForJsonSchema = (
21-
jsonSchema: JSONSchema7,
24+
jsonSchema: AirbyteJSONSchema,
2225
formField: FormBlock,
23-
parentSchema?: JSONSchema7,
26+
formatMessage: (message: MessageDescriptor, values?: Record<string, string | number>) => string,
27+
parentSchema?: AirbyteJSONSchema,
2428
propertyKey?: string,
2529
propertyPath: string | undefined = propertyKey
2630
): yup.AnySchema => {
@@ -69,6 +73,7 @@ export const buildYupFormForJsonSchema = (
6973
: buildYupFormForJsonSchema(
7074
prop,
7175
selectionFormField.properties[propertyIndex],
76+
formatMessage,
7277
condition,
7378
key,
7479
propertyPath ? `${propertyPath}.${propertyKey}` : propertyKey
@@ -127,7 +132,7 @@ export const buildYupFormForJsonSchema = (
127132
}
128133

129134
// Otherwise require that all oneOfs have an option selected, as the user has no way to unselect an option.
130-
return oneOfSchema.required("form.empty.error");
135+
return oneOfSchema.required(formatMessage({ id: "form.empty.error" }));
131136
}
132137

133138
switch (jsonSchema.type) {
@@ -138,7 +143,10 @@ export const buildYupFormForJsonSchema = (
138143
.trim();
139144

140145
if (jsonSchema?.pattern !== undefined) {
141-
schema = schema.matches(new RegExp(jsonSchema.pattern), FORM_PATTERN_ERROR);
146+
schema = schema.matches(
147+
new RegExp(jsonSchema.pattern),
148+
formatMessage({ id: FORM_PATTERN_ERROR }, { pattern: getPatternDescriptor(jsonSchema) ?? jsonSchema.pattern })
149+
);
142150
}
143151

144152
break;
@@ -150,11 +158,11 @@ export const buildYupFormForJsonSchema = (
150158
schema = yup.number().transform((value) => (isNaN(value) ? undefined : value));
151159

152160
if (jsonSchema?.minimum !== undefined) {
153-
schema = schema.min(jsonSchema?.minimum);
161+
schema = schema.min(jsonSchema.minimum, (value) => formatMessage({ id: "form.min.error" }, { min: value.min }));
154162
}
155163

156164
if (jsonSchema?.maximum !== undefined) {
157-
schema = schema.max(jsonSchema?.maximum);
165+
schema = schema.max(jsonSchema.maximum, (value) => formatMessage({ id: "form.max.error" }, { max: value.max }));
158166
}
159167
break;
160168
case "array":
@@ -165,6 +173,7 @@ export const buildYupFormForJsonSchema = (
165173
buildYupFormForJsonSchema(
166174
jsonSchema.items,
167175
(formField as FormObjectArrayItem).properties,
176+
formatMessage,
168177
jsonSchema,
169178
propertyKey,
170179
propertyPath ? `${propertyPath}.${propertyKey}` : propertyKey
@@ -188,6 +197,7 @@ export const buildYupFormForJsonSchema = (
188197
? buildYupFormForJsonSchema(
189198
propertySchema,
190199
correspondingFormField,
200+
formatMessage,
191201
jsonSchema,
192202
propertyKey,
193203
propertyPath ? `${propertyPath}.${propertyKey}` : propertyKey
@@ -217,7 +227,7 @@ export const buildYupFormForJsonSchema = (
217227
parentSchema.required.find((item) => item === propertyKey);
218228

219229
if (schema && isRequired) {
220-
schema = schema.required("form.empty.error");
230+
schema = schema.required(formatMessage({ id: "form.empty.error" }));
221231
}
222232
}
223233

‎airbyte-webapp/src/locales/en.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,8 @@
159159
"form.wait": "Please wait a little bit more…",
160160
"form.optional": "Optional",
161161
"form.optionalFields": "Optional fields",
162+
"form.min.error": "Must be greater than or equal to {min}",
163+
"form.max.error": "Must be less than or equal to {max}",
162164

163165
"setupForm.check.loading": "Checking the security of your installation...",
164166
"setupForm.check.unsecured.title": "Your Airbyte installation is unsecured",

‎airbyte-webapp/src/views/Connector/ConnectorForm/components/Sections/PropertySection.tsx

Lines changed: 7 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import classNames from "classnames";
22
import uniq from "lodash/uniq";
33
import React from "react";
44
import { FieldError, UseFormGetFieldState, useController, useFormContext } from "react-hook-form";
5-
import { FormattedMessage } from "react-intl";
65

76
import { LabeledSwitch } from "components";
87
import { FlexContainer } from "components/ui/Flex";
@@ -24,24 +23,11 @@ interface PropertySectionProps {
2423
disabled?: boolean;
2524
}
2625

27-
const ErrorMessage = ({ error, property }: { error?: string; property: FormBaseItem }) => {
26+
const ErrorMessage = ({ error }: { error?: string }) => {
2827
if (!error) {
2928
return null;
3029
}
31-
return (
32-
<PropertyError>
33-
<FormattedMessage
34-
id={error}
35-
values={
36-
error === FORM_PATTERN_ERROR
37-
? {
38-
pattern: getPatternDescriptor(property) ?? property.pattern,
39-
}
40-
: undefined
41-
}
42-
/>
43-
</PropertyError>
44-
);
30+
return <PropertyError>{error}</PropertyError>;
4531
};
4632

4733
const FormatBlock = ({
@@ -58,12 +44,12 @@ const FormatBlock = ({
5844
return null;
5945
}
6046

61-
const hasPatternError = isPatternError(fieldMeta.error);
47+
const hasError = !!fieldMeta.error;
6248

6349
const patternStatus =
64-
value !== undefined && hasPatternError && fieldMeta.isTouched
50+
value !== undefined && hasError && fieldMeta.isTouched
6551
? "error"
66-
: value !== undefined && !hasPatternError && property.pattern !== undefined
52+
: value !== undefined && !hasError && property.pattern !== undefined
6753
? "success"
6854
: "none";
6955

@@ -115,11 +101,11 @@ export const PropertySection: React.FC<PropertySectionProps> = ({ property, path
115101
const errorMessage = Array.isArray(meta.error) ? (
116102
<FlexContainer direction="column" gap="none">
117103
{uniq(meta.error.map((error) => error?.message).filter(Boolean)).map((errorMessage, index) => {
118-
return <ErrorMessage key={index} error={errorMessage} property={property} />;
104+
return <ErrorMessage key={index} error={errorMessage} />;
119105
})}
120106
</FlexContainer>
121107
) : (
122-
<ErrorMessage error={meta.error?.message} property={property} />
108+
<ErrorMessage error={meta.error?.message} />
123109
);
124110

125111
return (

‎airbyte-webapp/src/views/Connector/ConnectorForm/useBuildForm.tsx

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,10 @@ export function useBuildForm(
124124
throw new FormBuildError("connectorForm.error.topLevelNonObject");
125125
}
126126

127-
const validationSchema = useMemo(() => buildYupFormForJsonSchema(jsonSchema, formBlock), [formBlock, jsonSchema]);
127+
const validationSchema = useMemo(
128+
() => buildYupFormForJsonSchema(jsonSchema, formBlock, formatMessage),
129+
[formBlock, formatMessage, jsonSchema]
130+
);
128131

129132
const startValues = useMemo<ConnectorFormValues>(() => {
130133
let baseValues = {

‎airbyte-webapp/src/views/Connector/ConnectorForm/utils.ts

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import toLower from "lodash/toLower";
22

3-
import { FormBaseItem, FormBlock } from "core/form/types";
3+
import { FormBlock } from "core/form/types";
44
import { AdvancedAuth } from "core/request/AirbyteClient";
55
import { naturalComparator } from "core/utils/objects";
66

@@ -63,29 +63,29 @@ export function OrderComparator(a: FormBlock, b: FormBlock): number {
6363
}
6464
}
6565

66-
export function getPatternDescriptor(formItem: FormBaseItem): string | undefined {
67-
if (formItem.pattern_descriptor !== undefined) {
68-
return formItem.pattern_descriptor;
66+
export function getPatternDescriptor(schema: { pattern?: string; pattern_descriptor?: string }): string | undefined {
67+
if (schema.pattern_descriptor !== undefined) {
68+
return schema.pattern_descriptor;
6969
}
70-
if (formItem.pattern === undefined) {
70+
if (schema.pattern === undefined) {
7171
return undefined;
7272
}
73-
if (formItem.pattern.includes("^[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}Z$")) {
73+
if (schema.pattern.includes("^[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}Z$")) {
7474
return "YYYY-MM-DDTHH:mm:ssZ";
7575
}
76-
if (formItem.pattern.includes("^[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}$")) {
76+
if (schema.pattern.includes("^[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}$")) {
7777
return "YYYY-MM-DDTHH:mm:ss";
7878
}
79-
if (formItem.pattern.includes("^[0-9]{4}-[0-9]{2}-[0-9]{2} [0-9]{2}:[0-9]{2}:[0-9]{2}$")) {
79+
if (schema.pattern.includes("^[0-9]{4}-[0-9]{2}-[0-9]{2} [0-9]{2}:[0-9]{2}:[0-9]{2}$")) {
8080
return "YYYY-MM-DD HH:mm:ss";
8181
}
82-
if (formItem.pattern.includes("^[0-9]{4}-[0-9]{2}-[0-9]{2}$")) {
82+
if (schema.pattern.includes("^[0-9]{4}-[0-9]{2}-[0-9]{2}$")) {
8383
return "YYYY-MM-DD";
8484
}
85-
if (formItem.pattern.includes("^[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}.[0-9]{3}Z$")) {
85+
if (schema.pattern.includes("^[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}.[0-9]{3}Z$")) {
8686
return "YYYY-MM-DDTHH:mm:ss.SSSZ";
8787
}
88-
if (formItem.pattern.includes("^[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}.[0-9]{6}Z$")) {
88+
if (schema.pattern.includes("^[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}.[0-9]{6}Z$")) {
8989
return "YYYY-MM-DDTHH:mm:ss.SSSSSSZ";
9090
}
9191
return undefined;

0 commit comments

Comments
 (0)