From aca7fe811a46ec97ddf2e19b3416e101562d9c12 Mon Sep 17 00:00:00 2001 From: Travis Jenkins Date: Fri, 24 Jan 2025 16:21:02 -0500 Subject: [PATCH 1/6] Making the function to fetch nullable type shared --- src/services/jsonforms/index.ts | 46 ++++++++++++++++++++++----------- 1 file changed, 31 insertions(+), 15 deletions(-) diff --git a/src/services/jsonforms/index.ts b/src/services/jsonforms/index.ts index 6bdb86942..9e165b676 100644 --- a/src/services/jsonforms/index.ts +++ b/src/services/jsonforms/index.ts @@ -143,21 +143,15 @@ const isAdvancedConfig = (schema: JsonSchema): boolean => { return schema[ADVANCED] === true; }; -// Nullable is only supported for anyOf and oneOf. This is manually checked -// because allOf will also return true for a combinator check. After that we only -// support when there is exactly two types. This is mainly here to help render -// pydantic inputs better. -const getNullableType = (schema: JsonSchema): null | string => { - const combinatorVal = schema.anyOf ?? schema.oneOf ?? null; - - if (combinatorVal === null) { - return null; - } - - const types = combinatorVal.map(({ type }) => type); - - if (types.length === 2 && types.includes('null')) { - const response = types.filter((val) => { +const getTypeOtherThanNull = ( + fieldTypes: (string | string[] | undefined)[] +): null | string => { + if ( + Array.isArray(fieldTypes) && + fieldTypes.length === 2 && + fieldTypes.includes('null') + ) { + const response = fieldTypes.filter((val) => { if (!val || Array.isArray(val)) { return false; } @@ -175,6 +169,20 @@ const getNullableType = (schema: JsonSchema): null | string => { return null; }; +// Nullable is only supported for anyOf and oneOf. This is manually checked +// because allOf will also return true for a combinator check. After that we only +// support when there is exactly two types. This is mainly here to help render +// pydantic inputs better. +const getNullableType = (schema: JsonSchema): null | string => { + const combinatorVal = schema.anyOf ?? schema.oneOf ?? null; + + if (combinatorVal === null) { + return null; + } + + return getTypeOtherThanNull(combinatorVal.map(({ type }) => type)); +}; + const isOAuthConfig = (schema: JsonSchema): boolean => Object.hasOwn(schema, Annotations.oAuthProvider); @@ -555,6 +563,14 @@ const generateUISchema = ( if (types.length > 1) { const controlObject: ControlElement = createControlElement(currentRef); + + // Some fields are 1 type OR null so check this so we + // can use our custom renderer to clear these values properly + const nullableType = getTypeOtherThanNull(types); + if (nullableType) { + addNullableField(controlObject, nullableType); + } + schemaElements.push(controlObject); return controlObject; } From 3fced6a5d22896623195fe881a868a626ac935ee Mon Sep 17 00:00:00 2001 From: Travis Jenkins Date: Fri, 24 Jan 2025 16:31:23 -0500 Subject: [PATCH 2/6] Starting to wire up the new change handler --- src/forms/renderers/nullable/Control.tsx | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/forms/renderers/nullable/Control.tsx b/src/forms/renderers/nullable/Control.tsx index c8e36d556..01303f594 100644 --- a/src/forms/renderers/nullable/Control.tsx +++ b/src/forms/renderers/nullable/Control.tsx @@ -18,7 +18,7 @@ export const nullableControlTester: RankedTester = rankWith( ); const NullableControlRenderer = (props: any) => { - const { uischema } = props; + const { handleChange, uischema } = props; const { options } = uischema; const InputComponent = useMemo(() => { @@ -45,7 +45,14 @@ const NullableControlRenderer = (props: any) => { return null; } - return ; + return ( + { + handleChange(path, val ?? null); + }} + /> + ); }; export const NullableControl = withJsonFormsControlProps( From 942a9617f9ee8bf4d2b117a189d55597d23523f1 Mon Sep 17 00:00:00 2001 From: Travis Jenkins Date: Mon, 3 Feb 2025 12:33:31 -0500 Subject: [PATCH 3/6] Cleaning up some logging Updating how we check formats No longer want to check the string type as we need to check nullableType as well. So just easier to wrap all these in a single check Adding some commenting explaining stuff Adding nullable to any input near the bottom --- src/forms/renderers/nullable/Control.tsx | 2 + src/services/jsonforms/index.ts | 98 ++++++++++++------------ 2 files changed, 49 insertions(+), 51 deletions(-) diff --git a/src/forms/renderers/nullable/Control.tsx b/src/forms/renderers/nullable/Control.tsx index 01303f594..7ce097768 100644 --- a/src/forms/renderers/nullable/Control.tsx +++ b/src/forms/renderers/nullable/Control.tsx @@ -21,6 +21,8 @@ const NullableControlRenderer = (props: any) => { const { handleChange, uischema } = props; const { options } = uischema; + console.log('>>>>> NullableControlRenderer', uischema); + const InputComponent = useMemo(() => { const nullableType: AllowedNullable = options ? options[Options.nullable] diff --git a/src/services/jsonforms/index.ts b/src/services/jsonforms/index.ts index 9e165b676..3616850a5 100644 --- a/src/services/jsonforms/index.ts +++ b/src/services/jsonforms/index.ts @@ -79,47 +79,32 @@ const addTitle = ( return group; }; -const isMultilineText = (schema: JsonSchema): boolean => { - if (schema.type === 'string' && Object.hasOwn(schema, 'multiline')) { +type DateTimeFormats = 'date' | 'date-time' | 'time'; +const schemaHasFormat = ( + format: DateTimeFormats, + schema: JsonSchema +): boolean => { + if (Object.hasOwn(schema, 'format')) { // eslint-disable-next-line @typescript-eslint/dot-notation - return schema['multiline'] === true; - } else { - return false; - } -}; - -const isDateText = (schema: JsonSchema): boolean => { - if (schema.type === 'string' && Object.hasOwn(schema, 'format')) { - // eslint-disable-next-line @typescript-eslint/dot-notation - return schema['format'] === 'date'; - } else { - return false; - } -}; - -const isDateTimeText = (schema: JsonSchema): boolean => { - if (schema.type === 'string' && Object.hasOwn(schema, 'format')) { - // eslint-disable-next-line @typescript-eslint/dot-notation - return schema['format'] === 'date-time'; + return schema['format'] === format; } else { return false; } }; -const isTimeText = (schema: JsonSchema): boolean => { - if (schema.type === 'string' && Object.hasOwn(schema, 'format')) { +const schemaHasMultilineProp = (schema: JsonSchema): boolean => { + if (Object.hasOwn(schema, 'multiline')) { // eslint-disable-next-line @typescript-eslint/dot-notation - return schema['format'] === 'time'; + return schema['multiline'] === true; } else { return false; } }; -const isSecretText = (schema: JsonSchema): boolean => { +const schemaHasSecretProp = (schema: JsonSchema): boolean => { if ( - schema.type === 'string' && - (Object.hasOwn(schema, 'secret') || - Object.hasOwn(schema, 'airbyte_secret')) + Object.hasOwn(schema, 'secret') || + Object.hasOwn(schema, 'airbyte_secret') ) { // eslint-disable-next-line @typescript-eslint/dot-notation return schema['secret'] === true || schema['airbyte_secret'] === true; @@ -169,7 +154,7 @@ const getTypeOtherThanNull = ( return null; }; -// Nullable is only supported for anyOf and oneOf. This is manually checked +// This is only supported for anyOf and oneOf. This is manually checked // because allOf will also return true for a combinator check. After that we only // support when there is exactly two types. This is mainly here to help render // pydantic inputs better. @@ -561,16 +546,15 @@ const generateUISchema = ( }; } - if (types.length > 1) { - const controlObject: ControlElement = createControlElement(currentRef); - - // Some fields are 1 type OR null so check this so we - // can use our custom renderer to clear these values properly - const nullableType = getTypeOtherThanNull(types); - if (nullableType) { - addNullableField(controlObject, nullableType); - } + // We're here so it means we are not rendering a combinator and rendering a control + // We need to fetch the nullableType. This only fetched the type if the array length + // is exactly 2 and we can pull a proper type off one of the values + const nullableType = getTypeOtherThanNull(types); + // If we have multiple types and one is not null then go ahead and render a control + // that way JSONForms can handle rendering the two types + if (types.length > 1 && !nullableType) { + const controlObject: ControlElement = createControlElement(currentRef); schemaElements.push(controlObject); return controlObject; } @@ -652,22 +636,34 @@ const generateUISchema = ( // After that we check if it is just multiline. const controlObject: ControlElement = createControlElement(currentRef); - if (isSecretText(jsonSchema)) { - addOption(controlObject, Options.format, Formats.password); - if (isMultilineText(jsonSchema)) { - addOption(controlObject, Options.multiLineSecret, true); + + // See if we need to mark the control as nullable. This is separate from the + // big block because this could apply to anything + if (nullableType) { + addNullableField(controlObject, nullableType); + } + + // Now check if the string needs any special handling when rendering the control + if (nullableType === 'string' || jsonSchema.type === 'string') { + if (schemaHasSecretProp(jsonSchema)) { + addOption(controlObject, Options.format, Formats.password); + if (schemaHasMultilineProp(jsonSchema)) { + addOption(controlObject, Options.multiLineSecret, true); + } + } else if (schemaHasMultilineProp(jsonSchema)) { + addOption(controlObject, Options.multi, true); + } else if (schemaHasFormat('date-time', jsonSchema)) { + addOption(controlObject, Options.format, Formats.dateTime); + } else if (schemaHasFormat('date', jsonSchema)) { + addOption(controlObject, Options.format, Formats.date); + } else if (schemaHasFormat('time', jsonSchema)) { + addOption(controlObject, Options.format, Formats.time); } - } else if (isMultilineText(jsonSchema)) { - addOption(controlObject, Options.multi, true); - } else if (isDateTimeText(jsonSchema)) { - addOption(controlObject, Options.format, Formats.dateTime); - } else if (isDateText(jsonSchema)) { - addOption(controlObject, Options.format, Formats.date); - } else if (isTimeText(jsonSchema)) { - addOption(controlObject, Options.format, Formats.time); } - switch (types[0]) { + // First see if we have a nullableType so that we do not need to worry about + // sorting the types array when it can be nullable + switch (nullableType ?? types[0]) { case 'object': // object items will be handled by the object control itself /* falls through */ case 'array': // array items will be handled by the array control itself From 9d5d1c03406c697390a7f7be608781ad6be5aee3 Mon Sep 17 00:00:00 2001 From: Travis Jenkins Date: Mon, 3 Feb 2025 16:15:27 -0500 Subject: [PATCH 4/6] reverting changes that are no longer needed --- src/forms/renderers/nullable/Control.tsx | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/src/forms/renderers/nullable/Control.tsx b/src/forms/renderers/nullable/Control.tsx index 7ce097768..c8e36d556 100644 --- a/src/forms/renderers/nullable/Control.tsx +++ b/src/forms/renderers/nullable/Control.tsx @@ -18,11 +18,9 @@ export const nullableControlTester: RankedTester = rankWith( ); const NullableControlRenderer = (props: any) => { - const { handleChange, uischema } = props; + const { uischema } = props; const { options } = uischema; - console.log('>>>>> NullableControlRenderer', uischema); - const InputComponent = useMemo(() => { const nullableType: AllowedNullable = options ? options[Options.nullable] @@ -47,14 +45,7 @@ const NullableControlRenderer = (props: any) => { return null; } - return ( - { - handleChange(path, val ?? null); - }} - /> - ); + return ; }; export const NullableControl = withJsonFormsControlProps( From 34b44e61de70760364d7ed89eab078deac4220ca Mon Sep 17 00:00:00 2001 From: Travis Jenkins Date: Mon, 3 Feb 2025 16:34:26 -0500 Subject: [PATCH 5/6] Adding better handling for multi line secrets --- src/services/jsonforms/index.ts | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/src/services/jsonforms/index.ts b/src/services/jsonforms/index.ts index 3616850a5..75e442d2b 100644 --- a/src/services/jsonforms/index.ts +++ b/src/services/jsonforms/index.ts @@ -634,15 +634,8 @@ const generateUISchema = ( // Then we check if it is password. If it is we set the proper format. While inside that we check for // multi line and set the format option so the MutliLineSecret renderer will pick it up. // After that we check if it is just multiline. - const controlObject: ControlElement = createControlElement(currentRef); - // See if we need to mark the control as nullable. This is separate from the - // big block because this could apply to anything - if (nullableType) { - addNullableField(controlObject, nullableType); - } - // Now check if the string needs any special handling when rendering the control if (nullableType === 'string' || jsonSchema.type === 'string') { if (schemaHasSecretProp(jsonSchema)) { @@ -661,6 +654,18 @@ const generateUISchema = ( } } + // See if we need to mark the control as nullable. This is separate from the + // big block because this could apply to almost anything + if ( + nullableType && + // "Nullable" fields only render with default renderers so we need to know if + // the control is a multi line secret so we can skip adding the "nullable" option + // and just use our custom renderer + !Boolean(controlObject.options?.[Options.multiLineSecret]) + ) { + addNullableField(controlObject, nullableType); + } + // First see if we have a nullableType so that we do not need to worry about // sorting the types array when it can be nullable switch (nullableType ?? types[0]) { From c1a53c3eff76bb00f183319c58dcee7c72e09568 Mon Sep 17 00:00:00 2001 From: Travis Jenkins Date: Mon, 3 Feb 2025 16:42:08 -0500 Subject: [PATCH 6/6] removing comment --- src/services/jsonforms/index.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/services/jsonforms/index.ts b/src/services/jsonforms/index.ts index 75e442d2b..3ce378bd0 100644 --- a/src/services/jsonforms/index.ts +++ b/src/services/jsonforms/index.ts @@ -666,8 +666,6 @@ const generateUISchema = ( addNullableField(controlObject, nullableType); } - // First see if we have a nullableType so that we do not need to worry about - // sorting the types array when it can be nullable switch (nullableType ?? types[0]) { case 'object': // object items will be handled by the object control itself /* falls through */