From 3538a1c9f4109b4db9a1bcc14139254cc46dc82a Mon Sep 17 00:00:00 2001 From: laura stordeur Date: Tue, 20 Feb 2024 18:36:06 +0100 Subject: [PATCH] Enable adding new nested entity that are already part of a nested entity when the form is in edit-mode (Issue #5375, PR #5546) # Description Enabling the possibility to add new nested entities that are within embedded entities. closes *#5375* https://github.com/inmanta/web-console/assets/44098050/d8f2015e-be7e-4737-9a5c-c661f8f07e76 --- .../5375-nested-embedded-entity-edit.yml | 6 + .../EditInstance/UI/EditInstancePage.test.tsx | 2 + src/Test/Data/Field.ts | 22 +++ src/Test/Data/Service/EmbeddedEntity.ts | 4 +- src/UI/Components/Diagram/helpers.test.ts | 2 +- .../Components/FieldInput.tsx | 132 ++++++++++++++++-- .../Helpers/createFormState.ts | 38 +++-- .../ServiceInstanceForm.test.tsx | 94 +++++++++++++ .../ServiceInstanceForm.tsx | 64 +++++++-- 9 files changed, 332 insertions(+), 32 deletions(-) create mode 100644 changelogs/unreleased/5375-nested-embedded-entity-edit.yml diff --git a/changelogs/unreleased/5375-nested-embedded-entity-edit.yml b/changelogs/unreleased/5375-nested-embedded-entity-edit.yml new file mode 100644 index 000000000..53739e793 --- /dev/null +++ b/changelogs/unreleased/5375-nested-embedded-entity-edit.yml @@ -0,0 +1,6 @@ +description: Enable adding new nested entity that are already part of a nested entity when the form is in edit-mode +issue-nr: 5375 +change-type: patch +destination-branches: [master, iso7] +sections: + bugfix: "{{description}}" \ No newline at end of file diff --git a/src/Slices/EditInstance/UI/EditInstancePage.test.tsx b/src/Slices/EditInstance/UI/EditInstancePage.test.tsx index b5048bbf4..aa76b34eb 100644 --- a/src/Slices/EditInstance/UI/EditInstancePage.test.tsx +++ b/src/Slices/EditInstance/UI/EditInstancePage.test.tsx @@ -522,6 +522,7 @@ test("Given the EditInstance View When adding new nested embedded entity Then th const addedOptionalEmbedded = screen.getByLabelText( "DictListFieldInputItem-editableOptionalEmbedded_base.1", ); + //check if direct attributes are correctly displayed expect(within(addedOptionalEmbedded).queryByText("string")).toBeEnabled(); expect( @@ -646,6 +647,7 @@ test("Given the EditInstance View When adding new nested embedded entity Then th within(nested_editableOptionalEmbedded_base).getByText("Add"), ); }); + expect( within(nested_editableOptionalEmbedded_base).queryByText("Delete"), ).toBeEnabled(); diff --git a/src/Test/Data/Field.ts b/src/Test/Data/Field.ts index fdf580434..083fe7798 100644 --- a/src/Test/Data/Field.ts +++ b/src/Test/Data/Field.ts @@ -19,6 +19,17 @@ export const text: TextField = { type: "string?", }; +export const textDisabled: TextField = { + kind: "Text", + name: "text_field_disabled", + description: "description", + isOptional: true, + isDisabled: true, + defaultValue: "", + inputType: TextInputTypes.text, + type: "string?", +}; + export const bool: BooleanField = { kind: "Boolean", name: "boolean_field", @@ -114,6 +125,17 @@ export const dictList = (fields?: Field[]): DictListField => ({ fields: fields || [], }); +export const nestedDictList = (fields?: Field[]): DictListField => ({ + kind: "DictList", + name: "nested_dict_list_field", + description: "description", + isOptional: true, + isDisabled: false, + min: 1, + max: 4, + fields: [dictList(fields)] || [], +}); + export const nestedEditable: Field[] = [ { kind: "Text", diff --git a/src/Test/Data/Service/EmbeddedEntity.ts b/src/Test/Data/Service/EmbeddedEntity.ts index 3f8f80516..9945b4123 100644 --- a/src/Test/Data/Service/EmbeddedEntity.ts +++ b/src/Test/Data/Service/EmbeddedEntity.ts @@ -437,7 +437,7 @@ export const nestedEditable: EmbeddedEntity[] = [ attributes: [ { name: "attr4", - modifier: "rw+", + modifier: "rw", type: "int[]", default_value: null, default_value_set: false, @@ -448,7 +448,7 @@ export const nestedEditable: EmbeddedEntity[] = [ embedded_entities: [], name: "embedded_single", description: "description", - modifier: "rw+", + modifier: "rw", lower_limit: 0, upper_limit: 1, }, diff --git a/src/UI/Components/Diagram/helpers.test.ts b/src/UI/Components/Diagram/helpers.test.ts index 171e5ad25..6ddb933fb 100644 --- a/src/UI/Components/Diagram/helpers.test.ts +++ b/src/UI/Components/Diagram/helpers.test.ts @@ -195,7 +195,7 @@ describe("createConnectionRules", () => { name: "embedded_single", lowerLimit: null, upperLimit: 1, - modifier: "rw+", + modifier: "rw", kind: "Embedded", }, ], diff --git a/src/UI/Components/ServiceInstanceForm/Components/FieldInput.tsx b/src/UI/Components/ServiceInstanceForm/Components/FieldInput.tsx index b754867a2..e5ba1201f 100644 --- a/src/UI/Components/ServiceInstanceForm/Components/FieldInput.tsx +++ b/src/UI/Components/ServiceInstanceForm/Components/FieldInput.tsx @@ -29,19 +29,47 @@ interface Props { originalState: InstanceAttributeModel; getUpdate: GetUpdate; path: string | null; + isNew?: boolean; } +/** + * Type representing a function to update the state within the form. + * + * @param {string} path - The path within the form state to update. + * @param {unknown} value - The new value to set at the specified path. + * @param {boolean} [multi] - Optional flag indicating if the update is for multiple values. Default is false. + * @returns {void} + */ type GetUpdate = (path: string, value: unknown, multi?: boolean) => void; +/** + * Combines the current path with the next path segment to create a new path. + * + * @param {string | null} path - The current path (can be null). + * @param {string} next - The next path segment to append. + * @returns {string} The new combined path. + */ const makePath = (path: string | null, next: string): string => path === null ? next : `${path}.${next}`; +/** + * FieldInput component for managing form input related to a specific field. + * + * @param {Props} props - Props for the FieldInput component. + * @prop {Field} field - The field associated with the input. + * @prop {FormState} formState - The current form state. + * @prop {OriginalState} originalState - The original state of the field. + * @prop {Function} getUpdate - Function to get updates for the field. + * @prop {string} path - The path of the field within the form. + * @prop {boolean} isNew - Flag indicating whether the field is newly added. Default is false. + */ export const FieldInput: React.FC = ({ field, formState, originalState, getUpdate, path, + isNew = false, }) => { //callback was used to avoid re-render in useEffect used in SelectFormInput const getEnumUpdate = useCallback( @@ -50,6 +78,7 @@ export const FieldInput: React.FC = ({ }, [getUpdate, path, field.name], ); + switch (field.kind) { case "Boolean": return field.isOptional ? ( @@ -65,7 +94,8 @@ export const FieldInput: React.FC = ({ key={field.name} shouldBeDisabled={ field.isDisabled && - get(originalState, makePath(path, field.name)) !== undefined + get(originalState, makePath(path, field.name)) !== undefined && + !isNew } /> ) : ( @@ -80,7 +110,8 @@ export const FieldInput: React.FC = ({ key={field.name} shouldBeDisabled={ field.isDisabled && - get(originalState, makePath(path, field.name)) !== undefined + get(originalState, makePath(path, field.name)) !== undefined && + !isNew } /> ); @@ -97,7 +128,8 @@ export const FieldInput: React.FC = ({ shouldBeDisabled={ field.isDisabled && get(originalState, makePath(path, field.name).split(".")) !== - undefined + undefined && + !isNew } type={field.inputType} handleInputChange={(value, _event) => @@ -118,7 +150,8 @@ export const FieldInput: React.FC = ({ isOptional={field.isOptional} shouldBeDisabled={ field.isDisabled && - get(originalState, makePath(path, field.name)) !== undefined + get(originalState, makePath(path, field.name)) !== undefined && + !isNew } type={field.inputType} handleInputChange={(value, _event) => { @@ -140,7 +173,8 @@ export const FieldInput: React.FC = ({ isOptional={field.isOptional} shouldBeDisabled={ field.isDisabled && - get(originalState, makePath(path, field.name)) !== undefined + get(originalState, makePath(path, field.name)) !== undefined && + !isNew } type={field.inputType} handleInputChange={(value, _event) => @@ -183,7 +217,8 @@ export const FieldInput: React.FC = ({ key={field.name} shouldBeDisabled={ field.isDisabled && - get(originalState, makePath(path, field.name)) !== undefined + get(originalState, makePath(path, field.name)) !== undefined && + !isNew } /> ); @@ -230,6 +265,12 @@ export const FieldInput: React.FC = ({ } }; +/** + * Get a placeholder for the given data type. + * + * @param {string} typeName - The data type name. + * @returns {string | undefined} The placeholder string for the given data type, or undefined if not found. + */ const getPlaceholderForType = (typeName: string): string | undefined => { if (typeName === "int[]") { return words("inventory.form.placeholder.intList"); @@ -244,6 +285,12 @@ const getPlaceholderForType = (typeName: string): string | undefined => { return undefined; }; +/** + * Get a type hint for the given data type. + * + * @param {string} typeName - The data type name. + * @returns {string | undefined} The type hint string for the given data type, or undefined if not found. + */ const getTypeHintForType = (typeName: string): string | undefined => { if (typeName.endsWith("[]")) { return words("inventory.form.typeHint.list")( @@ -263,6 +310,17 @@ interface NestedProps { path: string | null; } +/** + * NestedFieldInput component with inner state for managing nested field input. + * + * @param {NestedProps} props - Props for the NestedFieldInput component. + * @prop {Field} field - The nested field. + * @prop {FormState} formState - The form state. + * @prop {OriginalState} originalState - The original state of the nested field. + * @prop {Function} getUpdate - Function to update and get updates for the nested field. + * @prop {string} path - The path of the nested field. + * @returns {JSX.Element} The rendered NestedFieldInput component. + */ const NestedFieldInput: React.FC = ({ field, formState, @@ -283,6 +341,7 @@ const NestedFieldInput: React.FC = ({ setShowList(false); return getUpdate(makePath(path, field.name), null); }; + return ( = ({ field, formState, @@ -353,22 +423,65 @@ const DictListFieldInput: React.FC = ({ path, }) => { const list = get(formState, makePath(path, field.name)) as Array; + const [addedItemsPaths, setAddedItemPaths] = useState([]); + /** + * Add a new formField group of the same type to the list. + * Stores the paths of the newly added elements. + * + * @returns void + */ const onAdd = () => { if (field.max && list.length >= field.max) { return; } + + get(originalState, makePath(path, field.name)); + setAddedItemPaths([ + ...addedItemsPaths, + `${makePath(path, field.name)}.${list.length}`, + ]); + getUpdate(makePath(path, field.name), [ ...list, createFormState(field.fields), ]); }; - const getOnDelete = (index: number) => () => + /** + * Delete method that also handles the update of the stored paths + * + * @param {index} number + * @returns void + */ + const getOnDelete = (index: number) => () => { + const newPaths: string[] = []; + + /** + * We need to update the stored paths after the deleted item, + * because paths are dynamically defined and not fixed. + * If the user deletes an item preceding the new items, + * we want to make sure the path refers to the same entity. + */ + addedItemsPaths.forEach((addedPath, indexPath) => { + const lastDigit: number = Number(addedPath.slice(-1)); + + if (indexPath < index) { + newPaths.push(addedPath); // add addedPath to newPath + } else if (lastDigit > index) { + const truncatedPath = addedPath.slice(0, -1); // truncate the last digit + const modifiedPath = `${truncatedPath}${lastDigit - 1}`; // deduce 1 from the index + newPaths.push(modifiedPath); + } + }); + + setAddedItemPaths([...newPaths]); + getUpdate(makePath(path, field.name), [ ...list.slice(0, index), ...list.slice(index + 1, list.length), ]); + }; return ( = ({ /> } > - {list.map((item, index) => ( + {list.map((_item, index) => ( = ({ originalState={originalState} getUpdate={getUpdate} path={makePath(path, `${field.name}.${index}`)} + isNew={addedItemsPaths.includes( + `${makePath(path, field.name)}.${index}`, + )} /> ))} diff --git a/src/UI/Components/ServiceInstanceForm/Helpers/createFormState.ts b/src/UI/Components/ServiceInstanceForm/Helpers/createFormState.ts index e06929ee4..c1df36feb 100644 --- a/src/UI/Components/ServiceInstanceForm/Helpers/createFormState.ts +++ b/src/UI/Components/ServiceInstanceForm/Helpers/createFormState.ts @@ -1,6 +1,12 @@ import { times, cloneDeep } from "lodash-es"; import { FieldLikeWithFormState, InstanceAttributeModel } from "@/Core"; +/** + * Create an form state based on the provided fields. + * + * @param {FieldLikeWithFormState[]} fields - Array of fields with form state information. + * @returns {InstanceAttributeModel} The instance attribute Model. + */ export const createFormState = ( fields: FieldLikeWithFormState[], ): InstanceAttributeModel => { @@ -57,14 +63,14 @@ export const createFormState = ( }; /** - * Creates Form State based on available fields and optional originalAttributes. - * If apiVersion "v2" then returns originalAttributes as second version endpoint replaces the whole edited instance - * and fields not necessarily cover that 1:1 + * Create an edit form state based on the provided fields, API version, and original attributes. + * If API version is "v2" and original attributes are provided, returns the original attributes as the edit form state. + * Otherwise, generates an edit form state by comparing the provided fields with the original attributes. * - * @param {FieldLikeWithFormState[]} fields - active fields that - * @param {"v1" | "v2"} apiVersion - version of endpoint which handles the edit - * @param {InstanceAttributeModel | null | undefined} originalAttributes - current state of Attributes - * @returns + * @param {FieldLikeWithFormState[]} fields - Array of fields with form state information. + * @param {"v1" | "v2"} apiVersion - API version ("v1" or "v2"). + * @param {InstanceAttributeModel | null | undefined} originalAttributes - Original attributes of the instance. + * @returns {InstanceAttributeModel} The edit form state. */ export const createEditFormState = ( fields: FieldLikeWithFormState[], @@ -74,6 +80,7 @@ export const createEditFormState = ( if (apiVersion === "v2" && originalAttributes) { return originalAttributes; } + return fields.reduce((acc, curr) => { if (originalAttributes?.[curr.name] !== undefined) { switch (curr.kind) { @@ -133,12 +140,15 @@ export const createEditFormState = ( } }, {}); }; + /** - * Creates Form State based on available fields and optional originalAttributes. + * Create a form state for duplicating an instance based on the provided fields and original attributes. + * If an original attribute is present, generates a duplicate form state by copying the values from the original instance. + * Otherwise, generates a default form state based on the provided fields. * - * @param {FieldLikeWithFormState[]} fields - active fields that - * @param {InstanceAttributeModel | null | undefined} originalAttributes - current state of Attributes - * @returns + * @param {FieldLikeWithFormState[]} fields - Array of fields with form state information. + * @param {InstanceAttributeModel | null | undefined} originalAttributes - Original attributes of the instance. + * @returns {InstanceAttributeModel} The duplicate form state. */ export const createDuplicateFormState = ( fields: FieldLikeWithFormState[], @@ -202,6 +212,12 @@ export const createDuplicateFormState = ( }, {}); }; +/** + * Convert a value to a JSON string, or return an empty string if the value is an empty string. + * + * @param {unknown} value - The value to stringify. + * @returns {string} The JSON string representation of the value, or an empty string if the value is an empty string. + */ function stringifyDict(value: unknown) { return value === "" ? "" : JSON.stringify(value); } diff --git a/src/UI/Components/ServiceInstanceForm/ServiceInstanceForm.test.tsx b/src/UI/Components/ServiceInstanceForm/ServiceInstanceForm.test.tsx index 35ea9483e..f36d69ba9 100644 --- a/src/UI/Components/ServiceInstanceForm/ServiceInstanceForm.test.tsx +++ b/src/UI/Components/ServiceInstanceForm/ServiceInstanceForm.test.tsx @@ -6,6 +6,7 @@ import { BooleanField, DictListField, EnumField, + InstanceAttributeModel, NestedField, TextField, } from "@/Core"; @@ -24,6 +25,8 @@ const setup = ( | EnumField )[], func: undefined | jest.Mock = undefined, + isEdit = false, + originalAttributes: InstanceAttributeModel | undefined = undefined, ) => { return ( @@ -35,6 +38,8 @@ const setup = ( fields={fields} onCancel={jest.fn()} onSubmit={func ? func : jest.fn()} + isEdit={isEdit} + originalAttributes={originalAttributes} /> } /> @@ -42,6 +47,7 @@ const setup = ( ); }; + test("GIVEN ServiceInstanceForm WHEN passed a TextField THEN shows that field", async () => { render(setup([Test.Field.text])); @@ -215,6 +221,94 @@ test("GIVEN ServiceInstanceForm and a DictListField WHEN clicking all toggles op ).toBeVisible(); }); +test("GIVEN ServiceInstanceForm and a nested DictListField WHEN in EDIT mode, new items should be enabled.", async () => { + const originalAttributes = { + nested_dict_list_field: [ + { dict_list_field: [{ text_field_disabled: "a" }] }, + ], + }; + + render( + setup( + [Test.Field.nestedDictList([Test.Field.textDisabled])], + undefined, + true, + originalAttributes, + ), + ); + + const group = screen.getByRole("group", { + name: "nested_dict_list_field", + }); + + expect(group).toBeVisible(); + + expect( + screen.queryByRole("textbox", { name: Test.Field.textDisabled.name }), + ).not.toBeInTheDocument(); + + await act(async () => { + await userEvent.click( + within(group).getByRole("button", { + name: "nested_dict_list_field", + }), + ); + }); + await act(async () => { + await userEvent.click(within(group).getByRole("button", { name: "0" })); + }); + + expect( + screen.queryByRole("textbox", { + name: `TextInput-${Test.Field.textDisabled.name}`, + }), + ).not.toBeInTheDocument(); + + const nestedGroup = screen.getByRole("group", { + name: "dict_list_field", + }); + + expect(nestedGroup).toBeVisible(); + + await act(async () => { + await userEvent.click( + within(nestedGroup).getByRole("button", { + name: "dict_list_field", + }), + ); + }); + await act(async () => { + await userEvent.click( + within(nestedGroup).getByRole("button", { name: "0" }), + ); + }); + + const disabledNestedTextField = within(nestedGroup).getByRole("textbox", { + name: `TextInput-${Test.Field.textDisabled.name}`, + }); + + expect(disabledNestedTextField).toBeDisabled(); + + await act(async () => { + await userEvent.click( + within(nestedGroup).getByRole("button", { name: "Add" }), + ); + }); + + await act(async () => { + await userEvent.click( + within(nestedGroup).getByRole("button", { name: "1" }), + ); + }); + + const nestedTextFields = screen.getAllByRole("textbox", { + name: `TextInput-${Test.Field.textDisabled.name}`, + }); + + expect(nestedTextFields[0]).toBeDisabled(); + expect(nestedTextFields[1]).toBeEnabled(); +}); + test("GIVEN ServiceInstanceForm WHEN clicking the submit button THEN callback is executed with formState", async () => { const nestedField = Test.Field.nested([ { ...Test.Field.text, name: "flat_field_text_2" }, diff --git a/src/UI/Components/ServiceInstanceForm/ServiceInstanceForm.tsx b/src/UI/Components/ServiceInstanceForm/ServiceInstanceForm.tsx index a18481a12..bcc1df44d 100644 --- a/src/UI/Components/ServiceInstanceForm/ServiceInstanceForm.tsx +++ b/src/UI/Components/ServiceInstanceForm/ServiceInstanceForm.tsx @@ -26,12 +26,22 @@ interface Props { isEdit?: boolean; } +/** + * Creates the form state. + * If the form is not in edit mode but has original attributes, it returns a state for a duplicated instance. + * + * @param {Fields} fields - Array of Fields. + * @param {string} apiVersion - API version ("v1" or "v2"). + * @param {InstanceAttributeModel} originalAttributes - The original state of the attributes. + * @param {boolean} [isEdit=false] - Whether the form is in edit mode. Default is false. + * @returns {InstanceAttributeModel} The calculated form state. + */ const getFormState = ( fields, apiVersion, originalAttributes, isEdit = false, -) => { +): InstanceAttributeModel => { if (isEdit) { return createEditFormState(fields, apiVersion, originalAttributes); } else if (originalAttributes) { @@ -41,6 +51,20 @@ const getFormState = ( } }; +/** + * ServiceInstanceForm Component. + * Supports editing, creating, and duplicating instances. + * + * @param {Props} props - Props for the ServiceInstanceForm component. + * @prop {Fields[]} fields - Array of Fields. + * @prop {function} onSubmit - Callback method to handle form submission. + * @prop {function} onCancel - Callback method to handle form cancellation. + * @prop {InstanceAttributeModel} originalAttributes - Original state of the attributes. + * @prop {boolean} isSubmitDisabled - Indicates whether the submit button is disabled. + * @prop {string} [apiVersion="v1"] - API version ("v1" or "v2"). Default is "v1". + * @prop {boolean} [isEdit=false] - Whether the form is in edit mode. Default is false. + * @returns {JSX.Element} The rendered ServiceInstanceForm component. + */ export const ServiceInstanceForm: React.FC = ({ fields, onSubmit, @@ -58,16 +82,25 @@ export const ServiceInstanceForm: React.FC = ({ getFormState(fields, apiVersion, originalAttributes, isEdit), ); - const [dirtyInputs, setDirtyInputs] = useState(false); + const [isDirty, setIsDirty] = useState(false); const [shouldPerformCancel, setShouldCancel] = useState(false); - usePrompt(words("notification.instanceForm.prompt"), dirtyInputs); + usePrompt(words("notification.instanceForm.prompt"), isDirty); - //callback was used to avoid re-render in useEffect used in SelectFormInput inside FieldInput + /** + * Get an update for the form state based on the provided path and value. + * + * callback was used to avoid re-render in useEffect used in SelectFormInput inside FieldInput + * + * @param {string} path - The path within the form state to update. + * @param {unknown} value - The new value to set at the specified path. + * @param {boolean} [multi=false] - Optional flag indicating if the update is for multiple values. Default is false. + * @returns {void} + */ const getUpdate = useCallback( (path: string, value: unknown, multi = false): void => { - if (!dirtyInputs) { - setDirtyInputs(true); + if (!isDirty) { + setIsDirty(true); } if (multi) { setFormState((prev) => { @@ -88,15 +121,26 @@ export const ServiceInstanceForm: React.FC = ({ }); } }, - [dirtyInputs], + [isDirty], ); + /** + * Prevent the default behavior of a React form event. + * + * @param {React.FormEvent} event - The React form event. + * @returns {void} + */ const preventDefault = (event: React.FormEvent) => { event.preventDefault(); }; + /** + * Handle confirmation action by triggering form submission and updating dirty state. + * + * @returns {void} + */ const onConfirm = () => - onSubmit(formState, (value: boolean) => setDirtyInputs(value)); + onSubmit(formState, (value: boolean) => setIsDirty(value)); useEffect(() => { if (shouldPerformCancel) { @@ -143,8 +187,8 @@ export const ServiceInstanceForm: React.FC = ({