From 94b4ef3e5a12c327ec7845e228f1199c00c08d54 Mon Sep 17 00:00:00 2001 From: Jared White Date: Wed, 23 Oct 2024 14:38:40 -0700 Subject: [PATCH] feat: improve client-side field validation on Partners listings (#4331) * feat: improve client-side field validation on Partners listings * feat: additional error reporting * fix: make lottery dates/times required * fix: use time error values for time fields * test: update demographics queries not to use title attribute --- shared-helpers/package.json | 2 +- sites/partners/package.json | 2 +- .../sections/FormApplicationData.tsx | 2 + .../PaperListingForm/OpenHouseForm.tsx | 5 +- .../listings/PaperListingForm/index.tsx | 70 ++++++++++--------- .../sections/ApplicationAddress.tsx | 4 ++ .../sections/ApplicationDates.tsx | 6 +- .../sections/RankingsAndResults.tsx | 17 +++-- .../applications/review/demographics.test.tsx | 16 ++--- sites/public/package.json | 2 +- yarn.lock | 8 +-- 11 files changed, 80 insertions(+), 54 deletions(-) diff --git a/shared-helpers/package.json b/shared-helpers/package.json index 6bdc738f99..161c6640eb 100644 --- a/shared-helpers/package.json +++ b/shared-helpers/package.json @@ -16,7 +16,7 @@ "prettier": "prettier --write \"**/*.ts\"" }, "dependencies": { - "@bloom-housing/ui-components": "12.1.0", + "@bloom-housing/ui-components": "12.4.0", "@bloom-housing/ui-seeds": "1.17.0", "@heroicons/react": "^2.1.1", "axios-cookiejar-support": "4.0.6", diff --git a/sites/partners/package.json b/sites/partners/package.json index c30654962a..dbf0fbd24a 100644 --- a/sites/partners/package.json +++ b/sites/partners/package.json @@ -30,7 +30,7 @@ }, "dependencies": { "@bloom-housing/shared-helpers": "^7.7.1", - "@bloom-housing/ui-components": "12.1.0", + "@bloom-housing/ui-components": "12.4.0", "@bloom-housing/ui-seeds": "1.17.0", "@heroicons/react": "^2.1.1", "@mapbox/mapbox-sdk": "^0.13.0", diff --git a/sites/partners/src/components/applications/PaperApplicationForm/sections/FormApplicationData.tsx b/sites/partners/src/components/applications/PaperApplicationForm/sections/FormApplicationData.tsx index 9f75eec5ed..ba7c6105cb 100644 --- a/sites/partners/src/components/applications/PaperApplicationForm/sections/FormApplicationData.tsx +++ b/sites/partners/src/components/applications/PaperApplicationForm/sections/FormApplicationData.tsx @@ -37,6 +37,7 @@ const FormApplicationData = () => { register={register} error={errors?.dateSubmitted} watch={watch} + setValue={setValue} label={t("application.add.dateSubmitted")} errorMessage={t("errors.dateError")} required={!!isDateRequired} @@ -50,6 +51,7 @@ const FormApplicationData = () => { name="timeSubmitted" label={t("application.add.timeSubmitted")} register={register} + setValue={setValue} watch={watch} error={!!errors?.timeSubmitted} disabled={!isDateFilled} diff --git a/sites/partners/src/components/listings/PaperListingForm/OpenHouseForm.tsx b/sites/partners/src/components/listings/PaperListingForm/OpenHouseForm.tsx index 1918befc65..5d85909aa0 100644 --- a/sites/partners/src/components/listings/PaperListingForm/OpenHouseForm.tsx +++ b/sites/partners/src/components/listings/PaperListingForm/OpenHouseForm.tsx @@ -61,7 +61,7 @@ const OpenHouseForm = ({ onSubmit, currentEvent }: OpenHouseFormProps) => { })() // eslint-disable-next-line @typescript-eslint/unbound-method - const { register, watch, trigger, getValues, errors } = useForm({ + const { register, setValue, watch, trigger, getValues, errors } = useForm({ defaultValues, }) @@ -107,6 +107,7 @@ const OpenHouseForm = ({ onSubmit, currentEvent }: OpenHouseFormProps) => { name="date" id="date" register={register} + setValue={setValue} watch={watch} error={errors?.date} errorMessage={t("errors.requiredFieldError")} @@ -120,6 +121,7 @@ const OpenHouseForm = ({ onSubmit, currentEvent }: OpenHouseFormProps) => { name="startTime" id="startTime" register={register} + setValue={setValue} watch={watch} error={!!errors?.startTime} required @@ -132,6 +134,7 @@ const OpenHouseForm = ({ onSubmit, currentEvent }: OpenHouseFormProps) => { name="endTime" id="endTime" register={register} + setValue={setValue} watch={watch} error={!!errors?.endTime} required diff --git a/sites/partners/src/components/listings/PaperListingForm/index.tsx b/sites/partners/src/components/listings/PaperListingForm/index.tsx index 9c65917439..1a92dc083b 100644 --- a/sites/partners/src/components/listings/PaperListingForm/index.tsx +++ b/sites/partners/src/components/listings/PaperListingForm/index.tsx @@ -190,44 +190,50 @@ const ListingForm = ({ listing, editMode, setListingName }: ListingFormProps) => try { setLoading(true) clearErrors() - - const dataPipeline = new ListingDataPipeline(formData, { - preferences, - programs, - units, - openHouseEvents, - profile: profile, - latLong, - customMapPositionChosen, - }) - const formattedData = await dataPipeline.run() - let result - if (editMode) { - result = await listingsService.update({ - id: listing.id, - body: { id: listing.id, ...(formattedData as unknown as ListingUpdate) }, - }) - } else { - result = await listingsService.create({ - body: formattedData as unknown as ListingCreate, + const successful = await formMethods.trigger() + + if (successful) { + const dataPipeline = new ListingDataPipeline(formData, { + preferences, + programs, + units, + openHouseEvents, + profile: profile, + latLong, + customMapPositionChosen, }) - } + const formattedData = await dataPipeline.run() + let result + if (editMode) { + result = await listingsService.update({ + id: listing.id, + body: { id: listing.id, ...(formattedData as unknown as ListingUpdate) }, + }) + } else { + result = await listingsService.create({ + body: formattedData as unknown as ListingCreate, + }) + } - reset(formData) + reset(formData) - if (result) { - addToast(getToast(listing, listing?.status, formattedData?.status), { - variant: "success", - }) + if (result) { + addToast(getToast(listing, listing?.status, formattedData?.status), { + variant: "success", + }) - if (continueEditing) { - setAlert(null) - setListingName(result.name) - } else { - await router.push(`/listings/${result.id}`) + if (continueEditing) { + setAlert(null) + setListingName(result.name) + } else { + await router.push(`/listings/${result.id}`) + } } + setLoading(false) + } else { + setLoading(false) + setAlert("form") } - setLoading(false) } catch (err) { reset(formData) setLoading(false) diff --git a/sites/partners/src/components/listings/PaperListingForm/sections/ApplicationAddress.tsx b/sites/partners/src/components/listings/PaperListingForm/sections/ApplicationAddress.tsx index 4817f6c2d3..e566c557e5 100644 --- a/sites/partners/src/components/listings/PaperListingForm/sections/ApplicationAddress.tsx +++ b/sites/partners/src/components/listings/PaperListingForm/sections/ApplicationAddress.tsx @@ -705,7 +705,9 @@ const ApplicationAddress = ({ listing }: ApplicationAddressProps) => { name={"postmarkByDateDateField"} id={"postmarkByDateDateField"} register={register} + setValue={setValue} watch={watch} + error={errors?.postmarkByDateDateField} defaultDate={{ month: listing?.postmarkedApplicationsReceivedByDate ? dayjsDate.format("MM") @@ -728,7 +730,9 @@ const ApplicationAddress = ({ listing }: ApplicationAddressProps) => { name={"postmarkByDateTimeField"} id={"postmarkByDateTimeField"} register={register} + setValue={setValue} watch={watch} + error={errors?.postmarkByDateTimeField} defaultValues={{ hours: listing?.postmarkedApplicationsReceivedByDate ? dayjsDate.format("hh") diff --git a/sites/partners/src/components/listings/PaperListingForm/sections/ApplicationDates.tsx b/sites/partners/src/components/listings/PaperListingForm/sections/ApplicationDates.tsx index dce373bcaf..55425a0a67 100644 --- a/sites/partners/src/components/listings/PaperListingForm/sections/ApplicationDates.tsx +++ b/sites/partners/src/components/listings/PaperListingForm/sections/ApplicationDates.tsx @@ -69,7 +69,7 @@ const ApplicationDates = ({ const formMethods = useFormContext() // eslint-disable-next-line @typescript-eslint/unbound-method - const { register, watch } = formMethods + const { errors, register, setValue, watch } = formMethods const [drawerOpenHouse, setDrawerOpenHouse] = useState(false) const [modalDeleteOpenHouse, setModalDeleteOpenHouse] = useState(null) @@ -108,7 +108,9 @@ const ApplicationDates = ({ name={"applicationDueDateField"} id={"applicationDueDateField"} register={register} + setValue={setValue} watch={watch} + error={errors?.applicationDueDateField} note={t("listings.whenApplicationsClose")} defaultDate={{ month: listing?.applicationDueDate @@ -129,7 +131,9 @@ const ApplicationDates = ({ name={"applicationDueTimeField"} id={"applicationDueTimeField"} register={register} + setValue={setValue} watch={watch} + error={errors?.applicationDueTimeField} defaultValues={{ hours: listing?.applicationDueDate ? dayjs(new Date(listing?.applicationDueDate)).format("hh") diff --git a/sites/partners/src/components/listings/PaperListingForm/sections/RankingsAndResults.tsx b/sites/partners/src/components/listings/PaperListingForm/sections/RankingsAndResults.tsx index be5adf052b..005404ee91 100644 --- a/sites/partners/src/components/listings/PaperListingForm/sections/RankingsAndResults.tsx +++ b/sites/partners/src/components/listings/PaperListingForm/sections/RankingsAndResults.tsx @@ -23,7 +23,7 @@ const RankingsAndResults = ({ listing, isAdmin }: RankingsAndResultsProps) => { const formMethods = useFormContext() // eslint-disable-next-line @typescript-eslint/unbound-method - const { register, watch, control, errors } = formMethods + const { register, setValue, watch, control, errors } = formMethods const lotteryEvent = getLotteryEvent(listing as unknown as Listing) @@ -61,6 +61,7 @@ const RankingsAndResults = ({ listing, isAdmin }: RankingsAndResultsProps) => { value: YesNoEnum.no, }, ] + return ( <> { name={"lotteryDate"} id={"lotteryDate"} register={register} + required + setValue={setValue} watch={watch} error={ errors?.lotteryDate @@ -181,10 +184,12 @@ const RankingsAndResults = ({ listing, isAdmin }: RankingsAndResultsProps) => { name={"lotteryStartTime"} id={"lotteryStartTime"} register={register} + required + setValue={setValue} watch={watch} - error={errors?.lotteryDate ? true : false} + error={errors?.lotteryStartTime ? true : false} strings={{ - timeError: errors?.lotteryDate ? t("errors.dateError") : null, + timeError: errors?.lotteryStartTime ? t("errors.timeError") : null, }} defaultValues={ errors?.lotteryDate @@ -211,10 +216,12 @@ const RankingsAndResults = ({ listing, isAdmin }: RankingsAndResultsProps) => { name={"lotteryEndTime"} id={"lotteryEndTime"} register={register} + required + setValue={setValue} watch={watch} - error={errors?.lotteryDate ? true : false} + error={errors?.lotteryEndTime ? true : false} strings={{ - timeError: errors?.lotteryDate ? t("errors.dateError") : null, + timeError: errors?.lotteryEndTime ? t("errors.timeError") : null, }} defaultValues={ errors?.lotteryDate diff --git a/sites/public/__tests__/pages/applications/review/demographics.test.tsx b/sites/public/__tests__/pages/applications/review/demographics.test.tsx index 708cbfbec7..b41e7710a7 100644 --- a/sites/public/__tests__/pages/applications/review/demographics.test.tsx +++ b/sites/public/__tests__/pages/applications/review/demographics.test.tsx @@ -75,25 +75,25 @@ describe("applications pages", () => { }) it("should show other text fields when other options are checked", async () => { - const { getByText, queryByTitle, findByTitle } = render() + const { getByText, queryByTestId, findAllByTestId } = render() - expect(queryByTitle("asian-otherAsian")).not.toBeInTheDocument() + expect(queryByTestId("asian-otherAsian")).not.toBeInTheDocument() fireEvent.click(getByText("Asian")) fireEvent.click(getByText("Other Asian")) - expect(await findByTitle("asian-otherAsian")).toBeInTheDocument() + expect(await findAllByTestId("asian-otherAsian")).toHaveLength(2) expect( - queryByTitle("nativeHawaiianOtherPacificIslander-otherPacificIslander") + queryByTestId("nativeHawaiianOtherPacificIslander-otherPacificIslander") ).not.toBeInTheDocument() fireEvent.click(getByText("Native Hawaiian / Other Pacific Islander")) fireEvent.click(getByText("Other Pacific Islander")) expect( - await findByTitle("nativeHawaiianOtherPacificIslander-otherPacificIslander") - ).toBeInTheDocument() + await findAllByTestId("nativeHawaiianOtherPacificIslander-otherPacificIslander") + ).toHaveLength(2) - expect(queryByTitle("otherMultiracial")).not.toBeInTheDocument() + expect(await findAllByTestId("otherMultiracial")).toHaveLength(1) fireEvent.click(getByText("Other / Multiracial")) - expect(await findByTitle("otherMultiracial")).toBeInTheDocument() + expect(await findAllByTestId("otherMultiracial")).toHaveLength(2) }) }) }) diff --git a/sites/public/package.json b/sites/public/package.json index 3377b96276..9482e3fdf0 100644 --- a/sites/public/package.json +++ b/sites/public/package.json @@ -30,7 +30,7 @@ }, "dependencies": { "@bloom-housing/shared-helpers": "^7.7.1", - "@bloom-housing/ui-components": "12.1.0", + "@bloom-housing/ui-components": "12.4.0", "@bloom-housing/ui-seeds": "1.17.0", "@heroicons/react": "^2.1.1", "@mapbox/mapbox-sdk": "^0.13.0", diff --git a/yarn.lock b/yarn.lock index d18a30f1b9..9acb6f1791 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1985,10 +1985,10 @@ resolved "https://registry.npmjs.org/@bcoe/v8-coverage/-/v8-coverage-0.2.3.tgz" integrity sha512-0hYQ8SB4Db5zvZB4axdMHGwEaQjkZzFjQiN9LVYvIFB2nSUHW9tYpxWriPrWDASIxiaXax83REcLxuSdnGPZtw== -"@bloom-housing/ui-components@12.1.0": - version "12.1.0" - resolved "https://registry.yarnpkg.com/@bloom-housing/ui-components/-/ui-components-12.1.0.tgz#c6066ed097887820bf2195c748ac2862fb171eef" - integrity sha512-xFW4IlPw9v9OJCkEQiKdIiN4csJyHuuaz1GmZGD33+m2phU79f5he2zBw0/qZogqpmP8aYsuzzKHAigHKXNhdQ== +"@bloom-housing/ui-components@12.4.0": + version "12.4.0" + resolved "https://registry.yarnpkg.com/@bloom-housing/ui-components/-/ui-components-12.4.0.tgz#bc36172c4d500ecd632d440d58af4183904cf882" + integrity sha512-c67UsaZZF8LG3J5AzdbBcNfRf70+rLaQjjQxbH+dTHnH8GvPffKF9kL/vICldPHIm+qO+5Uc7paYXITzKFZp+w== dependencies: "@fortawesome/fontawesome-svg-core" "^6.1.1" "@fortawesome/free-regular-svg-icons" "^6.1.1"