Skip to content

Commit 1ca0ad8

Browse files
Tim Roesakashkulk
authored andcommitted
🪟 🐛 Hotfix getValues failures in connector form (#19400)
* Hotfix getValues failures in connector form * Fix jest tests * Fix jest tests * Show errors somewhat properly * Fix jest tests
1 parent 40efe24 commit 1ca0ad8

File tree

8 files changed

+50
-12
lines changed

8 files changed

+50
-12
lines changed

airbyte-webapp/src/core/jsonSchema/schemaToYup.test.ts

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -46,19 +46,19 @@ it("should build schema for simple case", () => {
4646
const yupSchema = buildYupFormForJsonSchema(schema);
4747

4848
const expectedSchema = yup.object().shape({
49-
host: yup.string().trim().required("form.empty.error"),
49+
host: yup.string().trim().required("form.empty.error").transform(String),
5050
port: yup
5151
.number()
5252
.min(0)
5353
.max(65536)
5454
.required("form.empty.error")
5555
.transform((val) => (isNaN(val) ? undefined : val)),
56-
user: yup.string().trim().required("form.empty.error"),
56+
user: yup.string().trim().required("form.empty.error").transform(String),
5757
is_sandbox: yup.boolean().default(false),
5858
is_field_no_default: yup.boolean().required("form.empty.error"),
59-
dbname: yup.string().trim().required("form.empty.error"),
60-
password: yup.string().trim(),
61-
reports: yup.array().of(yup.string().trim()),
59+
dbname: yup.string().trim().required("form.empty.error").transform(String),
60+
password: yup.string().trim().transform(String),
61+
reports: yup.array().of(yup.string().trim().transform(String)),
6262
});
6363

6464
expect(JSON.stringify(yupSchema)).toEqual(JSON.stringify(expectedSchema));
@@ -103,9 +103,9 @@ it("should build schema for conditional case", () => {
103103
);
104104

105105
const expectedSchema = yup.object().shape({
106-
start_date: yup.string().trim().required("form.empty.error"),
106+
start_date: yup.string().trim().required("form.empty.error").transform(String),
107107
credentials: yup.object().shape({
108-
api_key: yup.string().trim().required("form.empty.error"),
108+
api_key: yup.string().trim().required("form.empty.error").transform(String),
109109
}),
110110
});
111111

@@ -151,7 +151,7 @@ it("should build schema for conditional case with inner schema and selected uiwi
151151

152152
const expectedSchema = yup.object().shape({
153153
credentials: yup.object().shape({
154-
redirect_uri: yup.string().trim().required("form.empty.error"),
154+
redirect_uri: yup.string().trim().required("form.empty.error").transform(String),
155155
}),
156156
});
157157

airbyte-webapp/src/core/jsonSchema/schemaToYup.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,10 @@ export const buildYupFormForJsonSchema = (
5454

5555
switch (jsonSchema.type) {
5656
case "string":
57-
schema = yup.string().trim();
57+
schema = yup
58+
.string()
59+
.transform((val) => String(val))
60+
.trim();
5861

5962
if (jsonSchema?.pattern !== undefined) {
6063
schema = schema.matches(new RegExp(jsonSchema.pattern), "form.pattern.error");
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
export const useAppMonitoringService = () => {
2+
return {
3+
trackError: jest.fn(),
4+
trackAction: jest.fn(),
5+
};
6+
};
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
export * from "./AppMonitoringService";

airbyte-webapp/src/views/Connector/ConnectorForm/ConnectorForm.test.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,7 @@ const schema: AirbyteJSONSchema = {
142142
},
143143
};
144144

145+
jest.mock("hooks/services/AppMonitoringService");
145146
jest.mock("hooks/services/Analytics");
146147

147148
jest.mock("hooks/services/useWorkspace", () => ({

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { useFormikContext, setIn } from "formik";
1+
import { useFormikContext, setIn, useField } from "formik";
22
import React, { useCallback, useMemo } from "react";
33

44
import GroupControls from "components/GroupControls";
@@ -15,7 +15,7 @@ import { GroupLabel } from "./GroupLabel";
1515

1616
interface ConditionSectionProps {
1717
formField: FormConditionItem;
18-
path?: string;
18+
path: string;
1919
disabled?: boolean;
2020
}
2121

@@ -26,6 +26,8 @@ export const ConditionSection: React.FC<ConditionSectionProps> = ({ formField, p
2626
const { widgetsInfo, setUiWidgetsInfo } = useConnectorForm();
2727
const { values, setValues } = useFormikContext<ConnectorFormValues>();
2828

29+
const [, meta] = useField(path);
30+
2931
const currentlySelectedCondition = widgetsInfo[formField.path]?.selectedItem;
3032

3133
const onOptionChange = useCallback(
@@ -73,6 +75,7 @@ export const ConditionSection: React.FC<ConditionSectionProps> = ({ formField, p
7375
value={currentlySelectedCondition}
7476
name={formField.path}
7577
isDisabled={disabled}
78+
error={typeof meta.error === "string" && !!meta.error}
7679
/>
7780
</>
7881
}

airbyte-webapp/src/views/Connector/ConnectorForm/useAuthentication.test.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { useAuthentication as useAuthenticationHook } from "./useAuthentication"
99
import { noPredicateAdvancedAuth, predicateInsideConditional } from "./useAuthentication.mocks";
1010
import { makeConnectionConfigurationPath } from "./utils";
1111

12+
jest.mock("hooks/services/AppMonitoringService");
1213
jest.mock("./connectorFormContext");
1314
jest.mock("formik", () => ({
1415
...jest.requireActual("formik"),

airbyte-webapp/src/views/Connector/ConnectorForm/useAuthentication.tsx

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ import { getIn, useFormikContext } from "formik";
22
import { JSONSchema7 } from "json-schema";
33
import { useCallback, useMemo } from "react";
44

5+
import { ConnectorSpecification } from "core/domain/connector";
6+
import { useAppMonitoringService } from "hooks/services/AppMonitoringService";
57
import { FeatureItem, useFeature } from "hooks/services/Feature";
68

79
import { useConnectorForm } from "./connectorFormContext";
@@ -130,6 +132,7 @@ interface AuthenticationHook {
130132
}
131133

132134
export const useAuthentication = (): AuthenticationHook => {
135+
const { trackError } = useAppMonitoringService();
133136
const { values, getFieldMeta, submitCount } = useFormikContext<ConnectorFormValues>();
134137
const { getValues, selectedConnectorDefinitionSpecification: connectorSpec } = useConnectorForm();
135138

@@ -140,7 +143,27 @@ export const useAuthentication = (): AuthenticationHook => {
140143

141144
const spec = connectorSpec?.connectionSpecification as JSONSchema7;
142145

143-
const valuesWithDefaults = useMemo(() => getValues(values), [getValues, values]);
146+
const getValuesSafe = useCallback(
147+
(values: ConnectorFormValues<unknown>) => {
148+
try {
149+
// We still see cases where calling `getValues` which will eventually use the yupSchema.cast method
150+
// crashes based on the passed in values. To temporarily make sure we're not crashing the form, we're
151+
// falling back to `values` in case the cast fails. This is a temporary patch, and we need to investigate
152+
// all the failures happening here properly.
153+
return getValues(values);
154+
} catch (e) {
155+
console.error(`getValues in useAuthentication failed.`, e);
156+
trackError(e, {
157+
id: "useAuthentication.getValues",
158+
connector: connectorSpec ? ConnectorSpecification.id(connectorSpec) : null,
159+
});
160+
return values;
161+
}
162+
},
163+
[connectorSpec, getValues, trackError]
164+
);
165+
166+
const valuesWithDefaults = useMemo(() => getValuesSafe(values), [getValuesSafe, values]);
144167

145168
const isAuthButtonVisible = useMemo(() => {
146169
const shouldShowAdvancedAuth =

0 commit comments

Comments
 (0)