fix(GAT-9237): bug in the onboarding form for tools (#1579)

Author: calmacx

src/app/[locale]/account/team/[teamId]/(withoutLeftNav)/datasets/components/CreateDataset/CreateDataset.tsx

@@ -21,10 +21,7 @@ import {
     NewDataset,
     StructuralMetadata,
 } from "@/interfaces/Dataset";
-import {
-    FormHydrationSchema,
-    FormHydrationValidation,
-} from "@/interfaces/FormHydration";
+import { FormHydrationSchema } from "@/interfaces/FormHydration";
 import { LegendItem } from "@/interfaces/FormLegend";
 import { Team } from "@/interfaces/Team";
 import { Defs } from "@/interfaces/V4Schema";
@@ -78,7 +75,7 @@ import {
     isLastSection,
     mapFormFieldsForSubmission,
     renderFormHydrationField,
-    formatValidationItems,
+    generateValidationRules,
 } from "@/utils/formHydration";
 import { capitalise, decodeHtmlEntity, splitCamelcase } from "@/utils/general";
 import IntroScreen from "../IntroScreen";
@@ -272,40 +269,6 @@ const CreateDataset = ({
         setExistingFormData(mappedFormData);
     }, [dataset, isLoading]);
 
-    const generateValidationRules = useMemo(
-        () => (validationFields: FormHydrationValidation[]) => {
-            const transformedObject: Record<
-                string,
-                Omit<FormHydrationValidation, "title">
-            > = {};
-
-            validationFields.forEach(field => {
-                const { title, items, required, of, ...rest } = field;
-
-                if (items && Array.isArray(items)) {
-                    // When field has an items array, convert to formatted object (used for field arrays)
-                    transformedObject[title] = {
-                        ...rest,
-                        required,
-                        items: formatValidationItems(items),
-                    };
-                } else if (of && required) {
-                    // Ensure required array of enums requires at least 1 value
-                    transformedObject[title] = {
-                        ...rest,
-                        required,
-                        min: 1,
-                    };
-                } else {
-                    transformedObject[title] = { ...rest, required };
-                }
-            });
-
-            return transformedObject;
-        },
-        []
-    );
-
     const [selectedFormSection, setSelectedFormSection] = useState<string>("");
     const [isSaving, setIsSaving] = useState<boolean>(false);
     const [guidanceText, setGuidanceText] = useState<string>();
@@ -333,7 +296,7 @@ const CreateDataset = ({
                     ? generateValidationRules(currentFormJSON.validation)
                     : {},
             },
-        [currentFormJSON.validation, generateValidationRules, isDraft]
+        [currentFormJSON.validation, isDraft]
     );
 
     const yupSchema = buildYup(generatedYupValidation);

src/utils/formHydration.test.tsx

@@ -1,4 +1,17 @@
-import { formGetFieldsCompletedCount } from "./formHydration";
+import { buildYup } from "schema-to-yup";
+import { FormHydrationValidation } from "@/interfaces/FormHydration";
+import {
+    formGenerateLegendItems,
+    formGetFieldsCompletedCount,
+    generateValidationRules,
+} from "./formHydration";
+
+const buildYupSchema = (validationFields: FormHydrationValidation[]) =>
+    buildYup({
+        title: "Metadata form",
+        type: "object",
+        properties: generateValidationRules(validationFields),
+    });
 
 const schemaFields = [
     {
@@ -84,3 +97,118 @@ describe("formGetFieldsCompletedCount", () => {
         );
     });
 });
+
+describe("generateValidationRules", () => {
+    // Shape confirmed against the real HDRUK 4.0.0 form_hydration response for
+    // "Tools" (an optional list of URL strings) - see GAT-9237.
+    const toolsValidation: FormHydrationValidation[] = [
+        {
+            title: "Tools",
+            type: "array",
+            required: false,
+            of: {
+                title: "Tools",
+                type: "string",
+                required: false,
+                format: "url",
+            },
+        },
+    ];
+
+    it("rejects a non-URL value in an optional url-format array field", () => {
+        const schema = buildYupSchema(toolsValidation);
+
+        expect(() =>
+            schema.validateSync({ Tools: ["das"] })
+        ).toThrow(/must be a valid URL/);
+    });
+
+    it("accepts a valid URL in an optional url-format array field", () => {
+        const schema = buildYupSchema(toolsValidation);
+
+        expect(() =>
+            schema.validateSync({ Tools: ["https://example.com"] })
+        ).not.toThrow();
+    });
+
+    it("accepts an empty array for an optional url-format array field", () => {
+        const schema = buildYupSchema(toolsValidation);
+
+        expect(() => schema.validateSync({ Tools: [] })).not.toThrow();
+    });
+
+    it("still enforces at least one value for a required array field", () => {
+        const requiredEnumValidation: FormHydrationValidation[] = [
+            {
+                title: "Category",
+                type: "array",
+                required: true,
+                of: {
+                    title: "Category",
+                    type: "string",
+                    required: true,
+                    enum: ["A", "B"],
+                },
+            },
+        ];
+        const schema = buildYupSchema(requiredEnumValidation);
+
+        expect(() => schema.validateSync({ Category: [] })).toThrow();
+        expect(() =>
+            schema.validateSync({ Category: ["A"] })
+        ).not.toThrow();
+    });
+});
+
+describe("formGenerateLegendItems", () => {
+    const section = "enrichmentAndLinkage";
+    const sectionFields = [
+        {
+            title: "Tools",
+            is_array_form: false,
+            description: "",
+            location: "enrichmentAndLinkage.tools",
+            guidance: "",
+            field: {
+                component: "Autocomplete",
+                name: "Tools",
+                required: false,
+                hidden: false,
+            },
+        },
+    ];
+
+    // activeSectionName === section short-circuits formGetSectionStatus to
+    // ACTIVE, so this test only exercises the clearErrors behaviour we care about.
+    const runLegend = async (dirtyFields: Record<string, boolean>) => {
+        const clearErrors = jest.fn();
+        const trigger = jest.fn(() => Promise.resolve(false));
+        const getValues = jest.fn();
+
+        await formGenerateLegendItems(
+            [section],
+            section,
+            true,
+            sectionFields,
+            clearErrors,
+            getValues,
+            trigger,
+            false,
+            dirtyFields
+        );
+
+        return clearErrors;
+    };
+
+    it("does not clear the error for a field the user has already touched", async () => {
+        const clearErrors = await runLegend({ Tools: true });
+
+        expect(clearErrors).not.toHaveBeenCalled();
+    });
+
+    it("clears the probe-validation error for a field the user hasn't touched", async () => {
+        const clearErrors = await runLegend({});
+
+        expect(clearErrors).toHaveBeenCalledWith(["Tools"]);
+    });
+});

src/utils/formHydration.tsx

@@ -180,18 +180,25 @@ const formGetSectionStatus = (
     return LegendStatus.VALID;
 };
 
-const formValidateSection = async (
+const getSectionFieldNames = (
     schemaFields: FormHydration[],
-    section: string,
-    trigger: UseFormTrigger<FieldValues>
-) => {
+    section: string
+): string | string[] => {
     const allSectionFields = formGetAllSectionFields(schemaFields, section);
     const isArrayForm = allSectionFields[0]?.is_array_form;
-    const fields = isArrayForm
+    return isArrayForm
         ? allSectionFields[0].title
         : allSectionFields
               .filter(schemaField => !schemaField?.field?.hidden)
               .map(field => field.title);
+};
+
+const formValidateSection = async (
+    schemaFields: FormHydration[],
+    section: string,
+    trigger: UseFormTrigger<FieldValues>
+) => {
+    const fields = getSectionFieldNames(schemaFields, section);
 
     return await trigger(fields, { shouldFocus: false });
 };
@@ -227,9 +234,22 @@ const formGenerateLegendItems = async (
                 ? await formValidateSection(schemaFields, section, trigger)
                 : false;
 
-            // Reset form error state
+            // Reset probe-validation errors, but keep errors for fields the
+            // user has actually touched (dirty) so they don't vanish on nav
             if (!submissionRequested) {
-                clearErrors();
+                const sectionFieldNames = getSectionFieldNames(
+                    schemaFields,
+                    section
+                );
+                const namesToClear = (
+                    Array.isArray(sectionFieldNames)
+                        ? sectionFieldNames
+                        : [sectionFieldNames]
+                ).filter(name => !dirtyFields[name]);
+
+                if (namesToClear.length) {
+                    clearErrors(namesToClear);
+                }
             }
 
             // Get status of section
@@ -340,6 +360,38 @@ const formatValidationItems = (items: Partial<FormHydrationValidation>[]) => ({
         ),
 });
 
+const generateValidationRules = (validationFields: FormHydrationValidation[]) => {
+    const transformedObject: Record<
+        string,
+        Omit<FormHydrationValidation, "title">
+    > = {};
+
+    validationFields.forEach(field => {
+        const { title, items, required, of, ...rest } = field;
+
+        if (items && Array.isArray(items)) {
+            // When field has an items array, convert to formatted object (used for field arrays)
+            transformedObject[title] = {
+                ...rest,
+                required,
+                items: formatValidationItems(items),
+            };
+        } else if (of && required) {
+            // Ensure required array of enums requires at least 1 value
+            transformedObject[title] = {
+                ...rest,
+                required,
+                of,
+                min: 1,
+            };
+        } else {
+            transformedObject[title] = { ...rest, required, of };
+        }
+    });
+
+    return transformedObject;
+};
+
 const convertRevisionsToArray = (data: {
     revisions: Revision | Revision[] | null;
 }) => {
@@ -583,6 +635,7 @@ export {
     hasVisibleFieldsForLocation,
     renderFormHydrationField,
     formatValidationItems,
+    generateValidationRules,
     formGetFieldsCompletedCount,
     mapFormFieldsForSubmission,
     mapExistingDatasetToFormFields,