fix: validate page3 using unregister

The schema is cleaner if we register / unregister the necessary fields.
This allows us to use a single top level undefined for a field that
should not be validate instead of writing custom validation logic.

apps/ui/components/application/ApplicationFormTextInput.tsx

@@ -0,0 +1,53 @@
+import React from "react";
+import { TextInput } from "hds-react";
+import { useTranslation } from "next-i18next";
+import { useFormContext } from "react-hook-form";
+import { type ApplicationPage3FormValues } from "./form";
+
+type TextFields =
+  | "organisation.name"
+  | "organisation.coreBusiness"
+  | "organisation.identifier"
+  | "organisation.address.streetAddress"
+  | "organisation.address.postCode"
+  | "organisation.address.city"
+  | "contactPerson.firstName"
+  | "contactPerson.lastName"
+  | "contactPerson.phoneNumber"
+  | "billingAddress.streetAddress"
+  | "billingAddress.postCode"
+  | "billingAddress.city"
+  | "additionalInformation";
+export function ApplicationFormTextInput({
+  name,
+  disabled,
+}: {
+  name: TextFields;
+  disabled?: boolean;
+}): JSX.Element {
+  const { t } = useTranslation();
+  const { register, getFieldState } =
+    useFormContext<ApplicationPage3FormValues>();
+
+  const translateError = (errorMsg?: string) =>
+    errorMsg ? t(`application:validation.${errorMsg}`) : "";
+  const state = getFieldState(name);
+
+  // avoid duplicating the translation key without polluting the props for a single case
+  const transformedLabel = name.replace(
+    "organisation.address.",
+    "billingAddress."
+  );
+
+  return (
+    <TextInput
+      {...register(name)}
+      label={t(`application:Page3.${transformedLabel}`)}
+      id={name}
+      required={!disabled}
+      disabled={disabled}
+      invalid={!!state.error?.message}
+      errorText={translateError(state.error?.message)}
+    />
+  );
+}

apps/ui/components/application/ApplicationPage.tsx

@@ -137,7 +137,9 @@ export function ApplicationPageWrapper({
   }));
 
   const handleStepClick = (i: number) => {
-    if (Number.isNaN(i) || i > 3) return; // invalid step
+    if (i < 0 || i > 3) {
+      return; // invalid step
+    }
     const targetPageIndex = i + 1;
     if (
       targetPageIndex === 4

apps/ui/components/application/BillingAddress.tsx

@@ -1,52 +1,19 @@
 import React from "react";
 import { useTranslation } from "next-i18next";
-import { TextInput } from "hds-react";
-import { useFormContext } from "react-hook-form";
 import { FormSubHeading } from "./styled";
-import { type ApplicationPage3FormValues } from "./form";
+import { ApplicationFormTextInput } from "./ApplicationFormTextInput";
 
 export function BillingAddress() {
   const { t } = useTranslation();
 
-  const {
-    register,
-    formState: { errors },
-  } = useFormContext<ApplicationPage3FormValues>();
-
-  const translateError = (errorMsg?: string) =>
-    errorMsg ? t(`application:validation.${errorMsg}`) : "";
-
   return (
     <>
       <FormSubHeading as="h2">
         {t("application:Page3.subHeading.billingAddress")}
       </FormSubHeading>
-      <TextInput
-        {...register("billingAddress.streetAddress")}
-        label={t("application:Page3.billingAddress.streetAddress")}
-        id="billingAddress.streetAddress"
-        required
-        invalid={!!errors.billingAddress?.streetAddress?.message}
-        errorText={translateError(
-          errors.billingAddress?.streetAddress?.message
-        )}
-      />
-      <TextInput
-        {...register("billingAddress.postCode")}
-        label={t("application:Page3.billingAddress.postCode")}
-        id="billingAddress.postCode"
-        required
-        invalid={!!errors.billingAddress?.postCode?.message}
-        errorText={translateError(errors.billingAddress?.postCode?.message)}
-      />
-      <TextInput
-        {...register("billingAddress.city")}
-        label={t("application:Page3.billingAddress.city")}
-        id="billingAddress.city"
-        required
-        invalid={!!errors.billingAddress?.city?.message}
-        errorText={translateError(errors.billingAddress?.city?.message)}
-      />
+      <ApplicationFormTextInput name="billingAddress.streetAddress" />
+      <ApplicationFormTextInput name="billingAddress.postCode" />
+      <ApplicationFormTextInput name="billingAddress.city" />
     </>
   );
 }

apps/ui/components/application/CompanyForm.tsx

@@ -1,11 +1,11 @@
 import React from "react";
-import { TextInput } from "hds-react";
 import { useTranslation } from "next-i18next";
 import { useFormContext } from "react-hook-form";
 import { FormSubHeading } from "./styled";
 import { BillingAddress } from "./BillingAddress";
 import { type ApplicationPage3FormValues } from "./form";
 import { ControlledCheckbox } from "common/src/components/form/ControlledCheckbox";
+import { ApplicationFormTextInput } from "./ApplicationFormTextInput";
 
 export function CompanyForm(): JSX.Element {
   const { t } = useTranslation();
@@ -51,42 +51,3 @@ export function ContactPersonSection(): JSX.Element {
     </>
   );
 }
-
-type TextFields =
-  | "organisation.name"
-  | "organisation.coreBusiness"
-  | "organisation.identifier"
-  | "organisation.address.streetAddress"
-  | "organisation.address.postCode"
-  | "organisation.address.city"
-  | "contactPerson.firstName"
-  | "contactPerson.lastName"
-  | "contactPerson.phoneNumber"
-  | "additionalInformation";
-export function ApplicationFormTextInput({
-  name,
-  disabled,
-}: {
-  name: TextFields;
-  disabled?: boolean;
-}): JSX.Element {
-  const { t } = useTranslation();
-  const { register, getFieldState } =
-    useFormContext<ApplicationPage3FormValues>();
-
-  const translateError = (errorMsg?: string) =>
-    errorMsg ? t(`application:validation.${errorMsg}`) : "";
-  const state = getFieldState(name);
-
-  return (
-    <TextInput
-      {...register(name)}
-      label={t(`application:Page3.${name}`)}
-      id={name}
-      required={!disabled}
-      disabled={disabled}
-      invalid={!!state.error?.message}
-      errorText={translateError(state.error?.message)}
-    />
-  );
-}

apps/ui/components/application/IndividualForm.tsx

@@ -2,7 +2,7 @@ import React from "react";
 import { useTranslation } from "next-i18next";
 import { BillingAddress } from "./BillingAddress";
 import { FormSubHeading, SpanFullRow } from "./styled";
-import { ApplicationFormTextInput } from "./CompanyForm";
+import { ApplicationFormTextInput } from "./ApplicationFormTextInput";
 
 export function IndividualForm(): JSX.Element {
   const { t } = useTranslation();

apps/ui/components/application/OrganisationForm.tsx

@@ -1,4 +1,4 @@
-import React, { useEffect } from "react";
+import React from "react";
 import { Checkbox } from "hds-react";
 import { useTranslation } from "next-i18next";
 import { useFormContext } from "react-hook-form";
@@ -8,7 +8,8 @@ import { type ApplicationPage3FormValues } from "./form";
 import { FormSubHeading } from "./styled";
 import { ControlledSelect } from "common/src/components/form";
 import { ControlledCheckbox } from "common/src/components/form/ControlledCheckbox";
-import { ApplicationFormTextInput, ContactPersonSection } from "./CompanyForm";
+import { ContactPersonSection } from "./CompanyForm";
+import { ApplicationFormTextInput } from "./ApplicationFormTextInput";
 
 type OptionType = {
   label: string;
@@ -22,8 +23,6 @@ export function OrganisationForm({ homeCityOptions }: Props): JSX.Element {
   const { t } = useTranslation();
 
   const {
-    register,
-    unregister,
     control,
     formState: { errors },
     watch,
@@ -34,26 +33,6 @@ export function OrganisationForm({ homeCityOptions }: Props): JSX.Element {
   const hasRegistration = applicantType === ApplicantTypeChoice.Association;
   const hasBillingAddress = watch("hasBillingAddress");
 
-  useEffect(() => {
-    if (hasRegistration) {
-      register("organisation.identifier", { required: true });
-    } else {
-      unregister("organisation.identifier");
-    }
-  }, [hasRegistration, register, unregister]);
-
-  useEffect(() => {
-    if (hasBillingAddress) {
-      register("billingAddress", { required: true });
-      register("billingAddress.postCode", { required: true });
-      register("billingAddress.city", { required: true });
-    } else {
-      unregister("billingAddress");
-      unregister("billingAddress.postCode");
-      unregister("billingAddress.city");
-    }
-  }, [hasBillingAddress, register, unregister]);
-
   const translateError = (errorMsg?: string) =>
     errorMsg ? t(`application:validation.${errorMsg}`) : "";
 

apps/ui/components/application/form.ts

@@ -444,22 +444,27 @@ export function ApplicationPage1SchemaRefined(round: {
   });
 }
 
-// TODO refine the form (different applicant types require different fields)
 // if applicantType === Organisation | Company => organisation.identifier is required
-// if hasBillingAddress | applicantType === Individual => billingAddress is required
+// if hasBillingAddress || applicantType === Individual => billingAddress is required
 export const ApplicationPage3Schema = z
   .object({
     pk: z.number(),
     applicantType: ApplicantTypeSchema.optional(),
     organisation: OrganisationFormValuesSchema.optional(),
-    contactPerson: PersonFormValuesSchema.optional(),
+    contactPerson: PersonFormValuesSchema,
     billingAddress: AddressFormValueSchema.optional(),
     // this is not submitted, we can use it to remove the billing address from submit without losing the frontend state
-    hasBillingAddress: z.boolean().optional(),
+    hasBillingAddress: z.boolean(),
     // TODO what is the max length for this?
     additionalInformation: z.string().optional(),
+    // why is this optional and for what cases?
     homeCity: z.number().optional(),
   })
+  // have to check at form level otherwise it forbids undefined initialization
+  .refine((val) => val.applicantType != null, {
+    message: "Required",
+    path: ["applicantType"],
+  })
   .superRefine((val, ctx) => {
     switch (val.applicantType) {
       case ApplicantTypeChoice.Association:
@@ -475,97 +480,6 @@ export const ApplicationPage3Schema = z
       default:
         break;
     }
-    if (val.applicantType !== ApplicantTypeChoice.Individual) {
-      if (val.organisation?.name == null) {
-        ctx.addIssue({
-          code: z.ZodIssueCode.custom,
-          path: ["organisation", "name"],
-          message: "Required",
-        });
-      }
-      if (val.organisation?.coreBusiness == null) {
-        ctx.addIssue({
-          code: z.ZodIssueCode.custom,
-          path: ["organisation", "coreBusiness"],
-          message: "Required",
-        });
-      }
-      if (!val.organisation?.address?.streetAddress) {
-        ctx.addIssue({
-          code: z.ZodIssueCode.custom,
-          path: ["organisation", "address", "streetAddress"],
-          message: "Required",
-        });
-      }
-      if (!val.organisation?.address?.city) {
-        ctx.addIssue({
-          code: z.ZodIssueCode.custom,
-          path: ["organisation", "address", "city"],
-          message: "Required",
-        });
-      }
-      if (!val.organisation?.address?.postCode) {
-        ctx.addIssue({
-          code: z.ZodIssueCode.custom,
-          path: ["organisation", "address", "postCode"],
-          message: "Required",
-        });
-      }
-    }
-    // TODO need to split
-    if (!val.contactPerson?.firstName) {
-      ctx.addIssue({
-        code: z.ZodIssueCode.custom,
-        path: ["contactPerson", "firstName"],
-        message: "Required",
-      });
-    }
-    if (!val.contactPerson?.lastName) {
-      ctx.addIssue({
-        code: z.ZodIssueCode.custom,
-        path: ["contactPerson", "lastName"],
-        message: "Required",
-      });
-    }
-    if (!val.contactPerson?.email) {
-      ctx.addIssue({
-        code: z.ZodIssueCode.custom,
-        path: ["contactPerson", "email"],
-        message: "Required",
-      });
-    }
-    if (!val.contactPerson?.phoneNumber) {
-      ctx.addIssue({
-        code: z.ZodIssueCode.custom,
-        path: ["contactPerson", "phoneNumber"],
-        message: "Required",
-      });
-    }
-
-    // TODO need to check the subfields of the address
-    if (val.hasBillingAddress) {
-      if (!val.billingAddress?.streetAddress) {
-        ctx.addIssue({
-          code: z.ZodIssueCode.custom,
-          path: ["billingAddress", "streetAddress"],
-          message: "Required",
-        });
-      }
-      if (!val.billingAddress?.city) {
-        ctx.addIssue({
-          code: z.ZodIssueCode.custom,
-          path: ["billingAddress", "city"],
-          message: "Required",
-        });
-      }
-      if (!val.billingAddress?.postCode) {
-        ctx.addIssue({
-          code: z.ZodIssueCode.custom,
-          path: ["billingAddress", "postCode"],
-          message: "Required",
-        });
-      }
-    }
   });
 
 export type ApplicationPage3FormValues = z.infer<typeof ApplicationPage3Schema>;
@@ -747,15 +661,21 @@ function transformOrganisation(org: OrganisationFormValues) {
 export function convertApplicationPage3(
   app?: Maybe<ApplicantFragment>
 ): ApplicationPage3FormValues {
+  const hasBillingAddress =
+    app?.applicantType !== ApplicantTypeChoice.Individual &&
+    app?.billingAddress?.streetAddressFi != null &&
+    app?.billingAddress?.streetAddressFi !== "";
   return {
     pk: app?.pk ?? 0,
     applicantType: app?.applicantType ?? undefined,
-    organisation: convertOrganisation(app?.organisation),
+    organisation: app?.organisation
+      ? convertOrganisation(app.organisation)
+      : undefined,
     contactPerson: convertPerson(app?.contactPerson),
-    billingAddress: convertAddress(app?.billingAddress),
-    hasBillingAddress:
-      app?.applicantType !== ApplicantTypeChoice.Individual &&
-      app?.billingAddress?.streetAddressFi != null,
+    billingAddress: hasBillingAddress
+      ? convertAddress(app.billingAddress)
+      : undefined,
+    hasBillingAddress,
     additionalInformation: app?.additionalInformation ?? "",
     homeCity: app?.homeCity?.pk ?? undefined,
   };