From 4a78c27b4b6b5e2dce799ed6acd0649a5ed1cf40 Mon Sep 17 00:00:00 2001 From: Diego Cohen Date: Mon, 2 Dec 2024 12:49:56 -0500 Subject: [PATCH 01/50] Initialize HoldPageStatusMessage component --- pages/hold/request/[id]/edd.tsx | 4 +-- pages/hold/request/[id]/index.tsx | 1 - .../HoldPages/HoldPageStatusMessages.tsx | 23 ++++++++++++ .../HoldPages/HoldRequestBanner.tsx | 3 +- src/types/holdPageTypes.ts | 7 +++- src/utils/holdPageUtils.ts | 35 +++++++++++-------- 6 files changed, 53 insertions(+), 20 deletions(-) create mode 100644 src/components/HoldPages/HoldPageStatusMessages.tsx diff --git a/pages/hold/request/[id]/edd.tsx b/pages/hold/request/[id]/edd.tsx index 5991e992a..5762d55e8 100644 --- a/pages/hold/request/[id]/edd.tsx +++ b/pages/hold/request/[id]/edd.tsx @@ -35,7 +35,7 @@ import type { DiscoveryItemResult } from "../../../../src/types/itemTypes" import type { EDDRequestParams, - EDDPageStatus, + HoldPageStatus, } from "../../../../src/types/holdPageTypes" interface EDDRequestPropsType { @@ -44,7 +44,7 @@ interface EDDRequestPropsType { patronId: string isAuthenticated?: boolean eddRequestable?: boolean - pageStatus?: EDDPageStatus + pageStatus?: HoldPageStatus } /** diff --git a/pages/hold/request/[id]/index.tsx b/pages/hold/request/[id]/index.tsx index 2feb9554b..c46d81ce6 100644 --- a/pages/hold/request/[id]/index.tsx +++ b/pages/hold/request/[id]/index.tsx @@ -213,7 +213,6 @@ export async function getServerSideProps({ params, req, res }) { try { const patronId = patronTokenResponse?.decodedPatron?.sub - // TODO: implement this function const holdRequestEligibility = await fetchHoldRequestEligibility(patronId) console.log("holdRequestEligibility", holdRequestEligibility) diff --git a/src/components/HoldPages/HoldPageStatusMessages.tsx b/src/components/HoldPages/HoldPageStatusMessages.tsx new file mode 100644 index 000000000..568ad4917 --- /dev/null +++ b/src/components/HoldPages/HoldPageStatusMessages.tsx @@ -0,0 +1,23 @@ +import { Text } from "@nypl/design-system-react-components" + +import type { + HoldPageStatus, + PatronEligibilityStatus, +} from "../../types/holdPageTypes" + +interface HoldIneligibilityStatusProps { + status: HoldPageStatus + eligibilityStatus?: PatronEligibilityStatus +} + +/** + * The HoldPageStatusMessages component renders the correct copy for different patron hold ineligibility statuses. + */ +const HoldPageStatusMessages = ({ + status, + eligibilityStatus, +}: HoldIneligibilityStatusProps) => { + return <> +} + +export default HoldPageStatusMessages diff --git a/src/components/HoldPages/HoldRequestBanner.tsx b/src/components/HoldPages/HoldRequestBanner.tsx index afaaac058..9c7c6cd35 100644 --- a/src/components/HoldPages/HoldRequestBanner.tsx +++ b/src/components/HoldPages/HoldRequestBanner.tsx @@ -5,13 +5,14 @@ import { FeedbackContext } from "../../../src/context/FeedbackContext" import type { ItemMetadata } from "../../../src/types/itemTypes" import type Item from "../../../src/models/Item" import RCLink from "../Links/RCLink/RCLink" +import type { HoldPageStatus } from "../../types/holdPageTypes" interface HoldRequestBannerProps { item: Item heading: string errorMessage: string errorDetail?: string - pageStatus?: string + pageStatus?: HoldPageStatus } /** diff --git a/src/types/holdPageTypes.ts b/src/types/holdPageTypes.ts index 60e9fa2d0..a904c0c76 100644 --- a/src/types/holdPageTypes.ts +++ b/src/types/holdPageTypes.ts @@ -59,7 +59,12 @@ export interface DiscoveryHoldPostParams { docDeliveryData?: EDDRequestParams } -export type EDDPageStatus = null | "failed" | "unavailable" | "invalid" +export type HoldPageStatus = + | null + | "failed" + | "eddUnavailable" + | "invalid" + | "patronIneligible" export interface EDDStatusMessage { heading?: string diff --git a/src/utils/holdPageUtils.ts b/src/utils/holdPageUtils.ts index 02209ffa1..561019353 100644 --- a/src/utils/holdPageUtils.ts +++ b/src/utils/holdPageUtils.ts @@ -1,7 +1,7 @@ import type { EDDRequestParams, EDDFormValidatedField, - EDDPageStatus, + HoldPageStatus, EDDStatusMessage, } from "../types/holdPageTypes" @@ -23,20 +23,25 @@ export const initialEDDFormState: EDDRequestParams = { pickupLocation: "edd", } -export const EDDPageStatusMessages: Record = { - failed: { - heading: "Request failed", - message: "We were unable to process your request at this time.", - }, - unavailable: { - heading: "Electronic delivery unavailable", - message: - "Electronic delivery options for this item are currently unavailable.", - }, - invalid: { - message: "Some fields contain errors. Please correct and submit again.", - }, -} +export const HoldPageStatusMessages: Record = + { + failed: { + heading: "Request failed", + message: "We were unable to process your request at this time.", + }, + eddUnavailable: { + heading: "Electronic delivery unavailable", + message: + "Electronic delivery options for this item are currently unavailable.", + }, + invalid: { + message: "Some fields contain errors. Please correct and submit again.", + }, + patronIneligible: { + heading: "There is a problem with your library account.", + message: "This is because:", + }, + } // Initial state for invalid fields in the EDD form to keep track of the first invalid field for focus on submit export const initialEDDInvalidFields: EDDFormValidatedField[] = [ From 01bc09954d702cd8fb4548757dbfffbdf547af6f Mon Sep 17 00:00:00 2001 From: Diego Cohen Date: Mon, 2 Dec 2024 13:10:31 -0500 Subject: [PATCH 02/50] Rename to HoldPageStatusMessage --- .../HoldPages/HoldPageStatusMessage.tsx | 21 +++++++++++++++++ .../HoldPages/HoldPageStatusMessages.tsx | 23 ------------------- src/types/holdPageTypes.ts | 2 +- 3 files changed, 22 insertions(+), 24 deletions(-) create mode 100644 src/components/HoldPages/HoldPageStatusMessage.tsx delete mode 100644 src/components/HoldPages/HoldPageStatusMessages.tsx diff --git a/src/components/HoldPages/HoldPageStatusMessage.tsx b/src/components/HoldPages/HoldPageStatusMessage.tsx new file mode 100644 index 000000000..ffbce0335 --- /dev/null +++ b/src/components/HoldPages/HoldPageStatusMessage.tsx @@ -0,0 +1,21 @@ +import type { + HoldPageStatus, + PatronEligibility, +} from "../../types/holdPageTypes" + +interface HoldPageStatusMessageProps { + status: HoldPageStatus + patronEligibility?: PatronEligibility +} + +/** + * The HoldPageStatusMessage component renders the correct copy for different patron hold ineligibility statuses. + */ +const HoldPageStatusMessage = ({ + status, + patronEligibility, +}: HoldPageStatusMessageProps) => { + return <> +} + +export default HoldPageStatusMessage diff --git a/src/components/HoldPages/HoldPageStatusMessages.tsx b/src/components/HoldPages/HoldPageStatusMessages.tsx deleted file mode 100644 index 568ad4917..000000000 --- a/src/components/HoldPages/HoldPageStatusMessages.tsx +++ /dev/null @@ -1,23 +0,0 @@ -import { Text } from "@nypl/design-system-react-components" - -import type { - HoldPageStatus, - PatronEligibilityStatus, -} from "../../types/holdPageTypes" - -interface HoldIneligibilityStatusProps { - status: HoldPageStatus - eligibilityStatus?: PatronEligibilityStatus -} - -/** - * The HoldPageStatusMessages component renders the correct copy for different patron hold ineligibility statuses. - */ -const HoldPageStatusMessages = ({ - status, - eligibilityStatus, -}: HoldIneligibilityStatusProps) => { - return <> -} - -export default HoldPageStatusMessages diff --git a/src/types/holdPageTypes.ts b/src/types/holdPageTypes.ts index a904c0c76..8008d6684 100644 --- a/src/types/holdPageTypes.ts +++ b/src/types/holdPageTypes.ts @@ -37,7 +37,7 @@ export interface HoldDetailsResult { errorMessage?: string } -export interface PatronEligibilityStatus { +export interface PatronEligibility { eligibility: boolean expired?: boolean blocked?: boolean From 2ba3e42f4285b155e28e8a663d6f4a834e59449e Mon Sep 17 00:00:00 2001 From: Diego Cohen Date: Mon, 2 Dec 2024 13:20:31 -0500 Subject: [PATCH 03/50] Update renamed variable references --- __test__/pages/hold/eddRequestPage.test.tsx | 8 ++++---- pages/hold/request/[id]/edd.tsx | 6 +++--- src/components/HoldPages/EDDRequestForm.tsx | 7 +++++-- src/types/holdPageTypes.ts | 2 +- 4 files changed, 13 insertions(+), 10 deletions(-) diff --git a/__test__/pages/hold/eddRequestPage.test.tsx b/__test__/pages/hold/eddRequestPage.test.tsx index d1ec932fa..d8c3b9d55 100644 --- a/__test__/pages/hold/eddRequestPage.test.tsx +++ b/__test__/pages/hold/eddRequestPage.test.tsx @@ -21,7 +21,7 @@ import { EDD_FORM_FIELD_COPY, } from "../../../src/config/constants" import { fetchDeliveryLocations } from "../../../src/server/api/hold" -import { EDDPageStatusMessages } from "../../../src/utils/holdPageUtils" +import { HoldPageStatusMessages } from "../../../src/utils/holdPageUtils" jest.mock("../../../src/server/auth") jest.mock("../../../src/server/api/bib") @@ -311,7 +311,7 @@ describe("EDD Request page", () => { /> ) expect( - screen.getByText(EDDPageStatusMessages.unavailable.heading) + screen.getByText(HoldPageStatusMessages.unavailable.heading) ).toBeInTheDocument() }) it("shows a failed error message when the page loads with an failed status", async () => { @@ -325,7 +325,7 @@ describe("EDD Request page", () => { /> ) expect( - screen.getByText(EDDPageStatusMessages.failed.heading) + screen.getByText(HoldPageStatusMessages.failed.heading) ).toBeInTheDocument() }) it("shows an invalid error message when the page loads with an invalid status", async () => { @@ -339,7 +339,7 @@ describe("EDD Request page", () => { /> ) expect( - screen.getByText(EDDPageStatusMessages.invalid.message) + screen.getByText(HoldPageStatusMessages.invalid.message) ).toBeInTheDocument() }) }) diff --git a/pages/hold/request/[id]/edd.tsx b/pages/hold/request/[id]/edd.tsx index 5762d55e8..3fc593a0e 100644 --- a/pages/hold/request/[id]/edd.tsx +++ b/pages/hold/request/[id]/edd.tsx @@ -18,7 +18,7 @@ import useLoading from "../../../../src/hooks/useLoading" import { fetchBib } from "../../../../src/server/api/bib" import { fetchDeliveryLocations } from "../../../../src/server/api/hold" import { - EDDPageStatusMessages, + HoldPageStatusMessages, initialEDDFormState, } from "../../../../src/utils/holdPageUtils" @@ -144,8 +144,8 @@ export default function EDDRequestPage({ {pageStatus && ( )} diff --git a/src/components/HoldPages/EDDRequestForm.tsx b/src/components/HoldPages/EDDRequestForm.tsx index 0a7fd3dc8..effa2de44 100644 --- a/src/components/HoldPages/EDDRequestForm.tsx +++ b/src/components/HoldPages/EDDRequestForm.tsx @@ -21,13 +21,16 @@ import { initialEDDInvalidFields, isInvalidField, } from "../../utils/holdPageUtils" -import type { EDDRequestParams, EDDPageStatus } from "../../types/holdPageTypes" +import type { + EDDRequestParams, + HoldPageStatus, +} from "../../types/holdPageTypes" interface EDDRequestFormProps { eddFormState: EDDRequestParams setEddFormState: React.Dispatch> handleSubmit: (eddParams: EDDRequestParams) => void - setPageStatus: (status: EDDPageStatus) => void + setPageStatus: (status: HoldPageStatus) => void holdId: string } diff --git a/src/types/holdPageTypes.ts b/src/types/holdPageTypes.ts index 8008d6684..a904c0c76 100644 --- a/src/types/holdPageTypes.ts +++ b/src/types/holdPageTypes.ts @@ -37,7 +37,7 @@ export interface HoldDetailsResult { errorMessage?: string } -export interface PatronEligibility { +export interface PatronEligibilityStatus { eligibility: boolean expired?: boolean blocked?: boolean From 584843d84c671a825cf809a29e96bda5fb232256 Mon Sep 17 00:00:00 2001 From: Diego Cohen Date: Mon, 2 Dec 2024 13:25:01 -0500 Subject: [PATCH 04/50] More reference name changes --- __test__/pages/hold/eddRequestPage.test.tsx | 4 ++-- pages/hold/request/[id]/edd.tsx | 4 ++-- src/components/HoldPages/HoldPageStatusMessage.tsx | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/__test__/pages/hold/eddRequestPage.test.tsx b/__test__/pages/hold/eddRequestPage.test.tsx index d8c3b9d55..80b3de6a4 100644 --- a/__test__/pages/hold/eddRequestPage.test.tsx +++ b/__test__/pages/hold/eddRequestPage.test.tsx @@ -307,11 +307,11 @@ describe("EDD Request page", () => { discoveryItemResult={bibWithItems.resource.items[0]} patronId="123" isAuthenticated={true} - pageStatus="unavailable" + pageStatus="eddUnavailable" /> ) expect( - screen.getByText(HoldPageStatusMessages.unavailable.heading) + screen.getByText(HoldPageStatusMessages.eddUnavailable.heading) ).toBeInTheDocument() }) it("shows a failed error message when the page loads with an failed status", async () => { diff --git a/pages/hold/request/[id]/edd.tsx b/pages/hold/request/[id]/edd.tsx index 3fc593a0e..7dbf4590a 100644 --- a/pages/hold/request/[id]/edd.tsx +++ b/pages/hold/request/[id]/edd.tsx @@ -156,7 +156,7 @@ export default function EDDRequestPage({ {isLoading || formPosting ? ( - ) : pageStatus !== "unavailable" ? ( + ) : pageStatus !== "eddUnavailable" ? ( Date: Tue, 3 Dec 2024 09:58:03 -0500 Subject: [PATCH 05/50] Start error message refactor --- __test__/pages/hold/eddRequestPage.test.tsx | 12 ++--- pages/hold/request/[id]/edd.tsx | 12 +---- .../HoldPages/HoldPageStatusMessage.tsx | 21 --------- .../HoldPages/HoldRequestBanner.tsx | 13 ++---- src/utils/holdPageUtils.ts | 45 +++++++++++-------- 5 files changed, 37 insertions(+), 66 deletions(-) delete mode 100644 src/components/HoldPages/HoldPageStatusMessage.tsx diff --git a/__test__/pages/hold/eddRequestPage.test.tsx b/__test__/pages/hold/eddRequestPage.test.tsx index 80b3de6a4..3150d7714 100644 --- a/__test__/pages/hold/eddRequestPage.test.tsx +++ b/__test__/pages/hold/eddRequestPage.test.tsx @@ -21,7 +21,7 @@ import { EDD_FORM_FIELD_COPY, } from "../../../src/config/constants" import { fetchDeliveryLocations } from "../../../src/server/api/hold" -import { HoldPageStatusMessages } from "../../../src/utils/holdPageUtils" +import { HoldPageErrorHeadings } from "../../../src/utils/holdPageUtils" jest.mock("../../../src/server/auth") jest.mock("../../../src/server/api/bib") @@ -311,7 +311,7 @@ describe("EDD Request page", () => { /> ) expect( - screen.getByText(HoldPageStatusMessages.eddUnavailable.heading) + screen.getByText(HoldPageErrorHeadings.eddUnavailable) ).toBeInTheDocument() }) it("shows a failed error message when the page loads with an failed status", async () => { @@ -324,9 +324,7 @@ describe("EDD Request page", () => { pageStatus="failed" /> ) - expect( - screen.getByText(HoldPageStatusMessages.failed.heading) - ).toBeInTheDocument() + expect(screen.getByText(HoldPageErrorHeadings.failed)).toBeInTheDocument() }) it("shows an invalid error message when the page loads with an invalid status", async () => { render( @@ -339,7 +337,9 @@ describe("EDD Request page", () => { /> ) expect( - screen.getByText(HoldPageStatusMessages.invalid.message) + screen.getByText( + "Some fields contain errors. Please correct and submit again." + ) ).toBeInTheDocument() }) }) diff --git a/pages/hold/request/[id]/edd.tsx b/pages/hold/request/[id]/edd.tsx index 7dbf4590a..b66241350 100644 --- a/pages/hold/request/[id]/edd.tsx +++ b/pages/hold/request/[id]/edd.tsx @@ -17,10 +17,7 @@ import useLoading from "../../../../src/hooks/useLoading" import { fetchBib } from "../../../../src/server/api/bib" import { fetchDeliveryLocations } from "../../../../src/server/api/hold" -import { - HoldPageStatusMessages, - initialEDDFormState, -} from "../../../../src/utils/holdPageUtils" +import { initialEDDFormState } from "../../../../src/utils/holdPageUtils" import initializePatronTokenAuth, { doRedirectBasedOnNyplAccountRedirects, @@ -142,12 +139,7 @@ export default function EDDRequestPage({ dynamically rendered notification for focus management */} {pageStatus && ( - + )} diff --git a/src/components/HoldPages/HoldPageStatusMessage.tsx b/src/components/HoldPages/HoldPageStatusMessage.tsx deleted file mode 100644 index eedfbcc7c..000000000 --- a/src/components/HoldPages/HoldPageStatusMessage.tsx +++ /dev/null @@ -1,21 +0,0 @@ -import type { - HoldPageStatus, - PatronEligibilityStatus, -} from "../../types/holdPageTypes" - -interface HoldPageStatusMessageProps { - status: HoldPageStatus - patronEligibility?: PatronEligibilityStatus -} - -/** - * The HoldPageStatusMessage component renders the correct copy for different patron hold ineligibility statuses. - */ -const HoldPageStatusMessage = ({ - status, - patronEligibility, -}: HoldPageStatusMessageProps) => { - return <> -} - -export default HoldPageStatusMessage diff --git a/src/components/HoldPages/HoldRequestBanner.tsx b/src/components/HoldPages/HoldRequestBanner.tsx index 9c7c6cd35..cbf1da6a8 100644 --- a/src/components/HoldPages/HoldRequestBanner.tsx +++ b/src/components/HoldPages/HoldRequestBanner.tsx @@ -1,6 +1,7 @@ import { useContext } from "react" import { Box, Banner, Button } from "@nypl/design-system-react-components" +import { HoldPageErrorHeadings } from "../../utils/holdPageUtils" import { FeedbackContext } from "../../../src/context/FeedbackContext" import type { ItemMetadata } from "../../../src/types/itemTypes" import type Item from "../../../src/models/Item" @@ -9,9 +10,6 @@ import type { HoldPageStatus } from "../../types/holdPageTypes" interface HoldRequestBannerProps { item: Item - heading: string - errorMessage: string - errorDetail?: string pageStatus?: HoldPageStatus } @@ -21,9 +19,6 @@ interface HoldRequestBannerProps { */ const HoldRequestBanner = ({ item, - heading, - errorMessage, - errorDetail, pageStatus = "failed", }: HoldRequestBannerProps) => { const { onOpen, setItemMetadata } = useContext(FeedbackContext) @@ -36,7 +31,7 @@ const HoldRequestBanner = ({ return ( - {errorMessage} - {pageStatus !== "invalid" ? ( + {pageStatus !== "invalid" && pageStatus !== "patronIneligible" ? ( <> {" Please try again, "} {" "} - for assistance, or{" "} - start a new search. - - ) : null} + + + {HOLD_PAGE_CONTACT_PREFIXES?.[errorStatus] ? ( + <> + {HOLD_PAGE_CONTACT_PREFIXES?.[errorStatus]} + {" Please try again, "} + {" "} + for assistance, or{" "} + start a new search. + + ) : null} + {(() => { switch (errorStatus) { case "invalid": @@ -122,7 +130,7 @@ const PatronErrors = ({ return ( - This is because: + This is because: Date: Tue, 3 Dec 2024 16:34:10 -0500 Subject: [PATCH 19/50] Add eligibility check to api route --- pages/api/hold/request/[id]/index.ts | 18 +++++++++++------- pages/hold/request/[id]/index.tsx | 16 ++++++++++------ 2 files changed, 21 insertions(+), 13 deletions(-) diff --git a/pages/api/hold/request/[id]/index.ts b/pages/api/hold/request/[id]/index.ts index fe15161a3..c3a65d1c7 100644 --- a/pages/api/hold/request/[id]/index.ts +++ b/pages/api/hold/request/[id]/index.ts @@ -1,4 +1,5 @@ import type { NextApiRequest, NextApiResponse } from "next" +import { NextResponse } from "next/server" import { postHoldRequest, @@ -22,20 +23,22 @@ async function handler(req: NextApiRequest, res: NextApiResponse) { const holdId = req.query.id as string const [, itemId] = holdId.split("-") - const patronEligibility = await fetchHoldRequestEligibility(patronId) + const patronEligibilityStatus = await fetchHoldRequestEligibility(patronId) - if (!patronEligibility?.eligibility) { + if (patronEligibilityStatus && !patronEligibilityStatus?.eligibility) { switch (jsEnabled) { case true: return res.status(401).json({ - patronEligibility, + patronEligibilityStatus, }) // Server side redirect on ineligibility when Js is disabled // TODO: Move this to seaprate API route + // TODO: Parse these query params in getServerSideProps default: - res.redirect( - `${BASE_URL}${PATHS.HOLD_REQUEST}/${holdId}?patronEligibility=${patronEligibility}` + return NextResponse.redirect( + `${BASE_URL}${PATHS.HOLD_REQUEST}/${holdId}?patronEligibility=${patronEligibilityStatus}`, + 307 ) } } @@ -64,8 +67,9 @@ async function handler(req: NextApiRequest, res: NextApiResponse) { } // Server side redirect in case user has JS disabled - res.redirect( - `${BASE_URL}${PATHS.HOLD_CONFIRMATION}/${holdId}?pickupLocation=${pickupLocationFromResponse}?requestId=${requestId}` + return NextResponse.redirect( + `${BASE_URL}${PATHS.HOLD_CONFIRMATION}/${holdId}?pickupLocation=${pickupLocationFromResponse}?requestId=${requestId}`, + 307 ) } catch (error) { const { statusText } = error as Response diff --git a/pages/hold/request/[id]/index.tsx b/pages/hold/request/[id]/index.tsx index 1c0091d13..015c3dd14 100644 --- a/pages/hold/request/[id]/index.tsx +++ b/pages/hold/request/[id]/index.tsx @@ -60,7 +60,7 @@ export default function HoldRequestPage({ patronId, isAuthenticated, errorStatus: defaultErrorStatus, - patronEligibilityStatus, + patronEligibilityStatus: defaultEligibilityStatus, }: HoldRequestPropsType) { const metadataTitle = `Item Request | ${SITE_NAME}` @@ -70,6 +70,9 @@ export default function HoldRequestPage({ const holdId = `${item.bibId}-${item.id}` const [errorStatus, setErrorStatus] = useState(defaultErrorStatus) + const [patronEligibilityStatus, setPatronEligibilityStatus] = useState( + defaultEligibilityStatus + ) const [formPosting, setFormPosting] = useState(false) const bannerContainerRef = useRef() @@ -98,28 +101,29 @@ export default function HoldRequestPage({ }), }) const responseJson = await response.json() - const { pickupLocation: pickupLocationFromResponse, requestId } = - responseJson switch (response.status) { + // Patron is ineligible to place holds case 401: - setErrorStatus("patronIneligible") setFormPosting(false) + setErrorStatus("patronIneligible") + setPatronEligibilityStatus(responseJson.patronEligibilityStatus) bannerContainerRef.current.focus() break + // Server side error placing the hold request case 500: + setFormPosting(false) console.error( "HoldRequestPage: Error in hold request api response", responseJson.error ) setErrorStatus("failed") - setFormPosting(false) break default: setFormPosting(false) // Success state await router.push( - `${PATHS.HOLD_CONFIRMATION}/${holdId}?pickupLocation=${pickupLocationFromResponse}&requestId=${requestId}` + `${PATHS.HOLD_CONFIRMATION}/${holdId}?pickupLocation=${responseJson.pickupLocation}&requestId=${responseJson.requestId}` ) } } catch (error) { From 87d35282ee285e2b3618f19fdfd66983bd035b58 Mon Sep 17 00:00:00 2001 From: Diego Cohen Date: Wed, 4 Dec 2024 12:24:23 -0500 Subject: [PATCH 20/50] Add variable with failed edd server responses --- pages/hold/request/[id]/edd.tsx | 18 ++++++++++-------- pages/hold/request/[id]/index.tsx | 14 ++++++++------ 2 files changed, 18 insertions(+), 14 deletions(-) diff --git a/pages/hold/request/[id]/edd.tsx b/pages/hold/request/[id]/edd.tsx index 865496fa9..65006d1b1 100644 --- a/pages/hold/request/[id]/edd.tsx +++ b/pages/hold/request/[id]/edd.tsx @@ -243,20 +243,22 @@ export async function getServerSideProps({ params, req, res, query }) { ) } + const locationOrEligibilityFetchFailed = + locationStatus !== 200 || !patronEligibilityStatus + return { props: { discoveryBibResult, discoveryItemResult, patronId, isAuthenticated, - errorStatus: - locationStatus !== 200 || !patronEligibilityStatus - ? "failed" - : !patronEligibilityStatus.eligibility - ? "patronIneligible" - : !isEddAvailable - ? "eddUnavailable" - : null, + errorStatus: locationOrEligibilityFetchFailed + ? "failed" + : patronEligibilityStatus.eligibility === false + ? "patronIneligible" + : !isEddAvailable + ? "eddUnavailable" + : null, }, } } catch (error) { diff --git a/pages/hold/request/[id]/index.tsx b/pages/hold/request/[id]/index.tsx index 015c3dd14..09ab6ed2c 100644 --- a/pages/hold/request/[id]/index.tsx +++ b/pages/hold/request/[id]/index.tsx @@ -263,6 +263,9 @@ export async function getServerSideProps({ params, req, res }) { ) } + const locationOrEligibilityFetchFailed = + locationStatus !== 200 || !patronEligibilityStatus + return { props: { discoveryBibResult, @@ -271,12 +274,11 @@ export async function getServerSideProps({ params, req, res }) { patronId, isAuthenticated, patronEligibilityStatus, - errorStatus: - locationStatus !== 200 || !patronEligibilityStatus - ? "failed" - : !patronEligibilityStatus.eligibility - ? "patronIneligible" - : null, + errorStatus: locationOrEligibilityFetchFailed + ? "failed" + : patronEligibilityStatus.eligibility === false + ? "patronIneligible" + : null, }, } } catch (error) { From 2911eb9b63ced22382b593ef638f4f4785bff03f Mon Sep 17 00:00:00 2001 From: Diego Cohen Date: Wed, 4 Dec 2024 12:42:14 -0500 Subject: [PATCH 21/50] Don't show reasons list when none is active --- pages/hold/confirmation/[id].tsx | 2 ++ pages/hold/request/[id]/index.tsx | 2 ++ src/components/HoldPages/HoldRequestErrorBanner.tsx | 9 +++++++++ 3 files changed, 13 insertions(+) diff --git a/pages/hold/confirmation/[id].tsx b/pages/hold/confirmation/[id].tsx index 5304df2cc..74a3713dc 100644 --- a/pages/hold/confirmation/[id].tsx +++ b/pages/hold/confirmation/[id].tsx @@ -99,6 +99,8 @@ export async function getServerSideProps({ req, res, query }) { const patronId = patronTokenResponse?.decodedPatron?.sub try { + const confirmationResponse = await fetchHoldDetails(requestId) + console.log(confirmationResponse) const { patronId: patronIdFromResponse } = await fetchHoldDetails(requestId) if (patronId !== patronIdFromResponse) { diff --git a/pages/hold/request/[id]/index.tsx b/pages/hold/request/[id]/index.tsx index 09ab6ed2c..2c3a1f768 100644 --- a/pages/hold/request/[id]/index.tsx +++ b/pages/hold/request/[id]/index.tsx @@ -63,6 +63,8 @@ export default function HoldRequestPage({ patronEligibilityStatus: defaultEligibilityStatus, }: HoldRequestPropsType) { const metadataTitle = `Item Request | ${SITE_NAME}` + console.log("defaultEligibilityStatus", defaultEligibilityStatus) + console.log("defaultErrorStatus", defaultErrorStatus) const bib = new Bib(discoveryBibResult) const item = new Item(discoveryItemResult, bib) diff --git a/src/components/HoldPages/HoldRequestErrorBanner.tsx b/src/components/HoldPages/HoldRequestErrorBanner.tsx index 6be538681..7da516518 100644 --- a/src/components/HoldPages/HoldRequestErrorBanner.tsx +++ b/src/components/HoldPages/HoldRequestErrorBanner.tsx @@ -128,6 +128,12 @@ const PatronErrors = ({ const { expired, moneyOwed, ptypeDisallowsHolds, reachedHoldLimit } = patronEligibilityStatus + const hasSpecificReason = + expired || moneyOwed || ptypeDisallowsHolds || reachedHoldLimit + + // Generic patron error displayed in heading, don't show reasons list if there isn't one + if (hasSpecificReason) return null + return ( This is because: @@ -135,6 +141,9 @@ const PatronErrors = ({ type="ul" margin={0} listItems={[ + ...(ptypeDisallowsHolds + ? ["Your card does not permit placing holds on ReCAP materials."] + : []), ...(expired ? [ <> From 3872336a6d68e17a56d4ae7783fbe22332b5256d Mon Sep 17 00:00:00 2001 From: Diego Cohen Date: Fri, 6 Dec 2024 13:08:43 -0500 Subject: [PATCH 22/50] Don't return reason when there isn't a specific eligibility reason --- pages/hold/confirmation/[id].tsx | 34 +++++++++++++++---- pages/hold/request/[id]/edd.tsx | 2 +- .../HoldPages/HoldRequestErrorBanner.tsx | 13 +++---- 3 files changed, 34 insertions(+), 15 deletions(-) diff --git a/pages/hold/confirmation/[id].tsx b/pages/hold/confirmation/[id].tsx index 74a3713dc..864641124 100644 --- a/pages/hold/confirmation/[id].tsx +++ b/pages/hold/confirmation/[id].tsx @@ -13,6 +13,7 @@ import HoldConfirmationFAQ from "../../../src/components/HoldPages/HoldConfirmat // import HoldItemDetails from "../../../src/components/HoldPages/HoldItemDetails" import { fetchHoldDetails } from "../../../src/server/api/hold" +import { fetchBib } from "../../../src/server/api/bib" import initializePatronTokenAuth, { doRedirectBasedOnNyplAccountRedirects, @@ -65,8 +66,10 @@ export default function HoldConfirmationPage({ ) } -export async function getServerSideProps({ req, res, query }) { - const { pickupLocation, requestId } = query +export async function getServerSideProps({ params, req, res, query }) { + const { id } = params + const { requestId } = query + // authentication redirect const patronTokenResponse = await initializePatronTokenAuth(req.cookies) const isAuthenticated = patronTokenResponse.isTokenValid @@ -85,7 +88,7 @@ export async function getServerSideProps({ req, res, query }) { ) const redirect = getLoginRedirect( req, - `${PATHS.HOLD_CONFIRMATION}?pickupLocation=${pickupLocation}&requestId=${requestId}` + `${PATHS.HOLD_CONFIRMATION}?requestId=${requestId}` ) return { @@ -99,9 +102,11 @@ export async function getServerSideProps({ req, res, query }) { const patronId = patronTokenResponse?.decodedPatron?.sub try { - const confirmationResponse = await fetchHoldDetails(requestId) - console.log(confirmationResponse) - const { patronId: patronIdFromResponse } = await fetchHoldDetails(requestId) + const { + patronId: patronIdFromResponse, + pickupLocation, + status: responseStatus, + } = await fetchHoldDetails(requestId) if (patronId !== patronIdFromResponse) { throw new Error( @@ -109,6 +114,23 @@ export async function getServerSideProps({ req, res, query }) { ) } + // TODO: Determine error state when there are errors in hold details endpoint reponse + if (responseStatus !== 200) { + throw new Error( + "Hold Confirmation Page - Bad response from hold confirmation request" + ) + } + + // fetch bib and item + const [bibId, itemId] = id.split("-") + + if (!itemId) { + throw new Error("No item id in url") + } + const { discoveryBibResult } = await fetchBib(bibId, {}, itemId) + const discoveryItemResult = discoveryBibResult?.items?.[0] + console.log(discoveryItemResult) + return { props: { isEDD: pickupLocation === "edd", diff --git a/pages/hold/request/[id]/edd.tsx b/pages/hold/request/[id]/edd.tsx index 65006d1b1..36303d8ae 100644 --- a/pages/hold/request/[id]/edd.tsx +++ b/pages/hold/request/[id]/edd.tsx @@ -168,7 +168,7 @@ export default function EDDRequestPage({ ) } -export async function getServerSideProps({ params, req, res, query }) { +export async function getServerSideProps({ params, req, res }) { const { id } = params // authentication redirect diff --git a/src/components/HoldPages/HoldRequestErrorBanner.tsx b/src/components/HoldPages/HoldRequestErrorBanner.tsx index 7da516518..3c5c54286 100644 --- a/src/components/HoldPages/HoldRequestErrorBanner.tsx +++ b/src/components/HoldPages/HoldRequestErrorBanner.tsx @@ -60,8 +60,8 @@ const HoldRequestErrorBanner = ({ content={ <> - - {HOLD_PAGE_CONTACT_PREFIXES?.[errorStatus] ? ( + {HOLD_PAGE_CONTACT_PREFIXES?.[errorStatus] ? ( + <> {HOLD_PAGE_CONTACT_PREFIXES?.[errorStatus]} {" Please try again, "} @@ -96,8 +96,8 @@ const HoldRequestErrorBanner = ({ for assistance, or{" "} start a new search. - ) : null} - + + ) : null} {(() => { switch (errorStatus) { case "invalid": @@ -132,7 +132,7 @@ const PatronErrors = ({ expired || moneyOwed || ptypeDisallowsHolds || reachedHoldLimit // Generic patron error displayed in heading, don't show reasons list if there isn't one - if (hasSpecificReason) return null + if (!hasSpecificReason) return null return ( @@ -141,9 +141,6 @@ const PatronErrors = ({ type="ul" margin={0} listItems={[ - ...(ptypeDisallowsHolds - ? ["Your card does not permit placing holds on ReCAP materials."] - : []), ...(expired ? [ <> From cf3bda6d5ebfa388d7e5ad67d51db68c9a82fe6c Mon Sep 17 00:00:00 2001 From: Diego Cohen Date: Fri, 6 Dec 2024 13:09:39 -0500 Subject: [PATCH 23/50] Add top margin to reasons box --- src/components/HoldPages/HoldRequestErrorBanner.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/HoldPages/HoldRequestErrorBanner.tsx b/src/components/HoldPages/HoldRequestErrorBanner.tsx index 3c5c54286..ee7288b4e 100644 --- a/src/components/HoldPages/HoldRequestErrorBanner.tsx +++ b/src/components/HoldPages/HoldRequestErrorBanner.tsx @@ -135,7 +135,7 @@ const PatronErrors = ({ if (!hasSpecificReason) return null return ( - + This is because: Date: Fri, 6 Dec 2024 14:10:31 -0500 Subject: [PATCH 24/50] Render item details on confirmation page --- pages/hold/confirmation/[id].tsx | 87 +++++++++++++++++++++++++++----- 1 file changed, 73 insertions(+), 14 deletions(-) diff --git a/pages/hold/confirmation/[id].tsx b/pages/hold/confirmation/[id].tsx index 864641124..6513a54c0 100644 --- a/pages/hold/confirmation/[id].tsx +++ b/pages/hold/confirmation/[id].tsx @@ -9,10 +9,19 @@ import { PATHS, } from "../../../src/config/constants" +import Bib from "../../../src/models/Bib" +import Item from "../../../src/models/Item" + +import RCLink from "../../../src/components/Links/RCLink/RCLink" +import ExternalLink from "../../../src/components/Links/RCLink/RCLink" + import HoldConfirmationFAQ from "../../../src/components/HoldPages/HoldConfirmationFAQ" -// import HoldItemDetails from "../../../src/components/HoldPages/HoldItemDetails" +import HoldItemDetails from "../../../src/components/HoldPages/HoldItemDetails" -import { fetchHoldDetails } from "../../../src/server/api/hold" +import { + fetchHoldDetails, + fetchDeliveryLocations, +} from "../../../src/server/api/hold" import { fetchBib } from "../../../src/server/api/bib" import initializePatronTokenAuth, { @@ -20,8 +29,14 @@ import initializePatronTokenAuth, { getLoginRedirect, } from "../../../src/server/auth" +import type { DiscoveryItemResult } from "../../../src/types/itemTypes" +import type { DiscoveryBibResult } from "../../../src/types/bibTypes" + interface HoldConfirmationPageProps { isEDD?: boolean + pickupLocationLabel?: string + discoveryBibResult: DiscoveryBibResult + discoveryItemResult: DiscoveryItemResult } /** @@ -30,8 +45,15 @@ interface HoldConfirmationPageProps { */ export default function HoldConfirmationPage({ isEDD = false, + pickupLocationLabel, + discoveryBibResult, + discoveryItemResult, }: HoldConfirmationPageProps) { const metadataTitle = `Request Confirmation | ${SITE_NAME}` + console.log(discoveryBibResult) + const bib = new Bib(discoveryBibResult) + const item = new Item(discoveryItemResult, bib) + return ( <> @@ -48,16 +70,39 @@ export default function HoldConfirmationPage({ {isEDD ? EDD_PAGE_HEADING : HOLD_PAGE_HEADING} - {/* */} + - You're all set! We have received your on-site request for - TODO replace with bib link - + <> + + You're all set! We have received your request for{" "} + + {item.bibTitle} + + + {isEDD ? ( + + The item will be delivered to the email address you provided. + + ) : pickupLocationLabel ? ( + + The item will be delivered to: {pickupLocationLabel}. + + ) : ( + + please{" "} + + email us + {" "} + or call 917-ASK-NYPL ( + 917-275-6975) for your + delivery location. + + )} + } /> @@ -68,7 +113,7 @@ export default function HoldConfirmationPage({ export async function getServerSideProps({ params, req, res, query }) { const { id } = params - const { requestId } = query + const { requestId, pickupLocation } = query // authentication redirect const patronTokenResponse = await initializePatronTokenAuth(req.cookies) @@ -102,11 +147,8 @@ export async function getServerSideProps({ params, req, res, query }) { const patronId = patronTokenResponse?.decodedPatron?.sub try { - const { - patronId: patronIdFromResponse, - pickupLocation, - status: responseStatus, - } = await fetchHoldDetails(requestId) + const { patronId: patronIdFromResponse, status: responseStatus } = + await fetchHoldDetails(requestId) if (patronId !== patronIdFromResponse) { throw new Error( @@ -129,11 +171,28 @@ export async function getServerSideProps({ params, req, res, query }) { } const { discoveryBibResult } = await fetchBib(bibId, {}, itemId) const discoveryItemResult = discoveryBibResult?.items?.[0] - console.log(discoveryItemResult) + if (!discoveryItemResult) { + throw new Error("Hold Confirmation Page - Item not found") + } + + const bib = new Bib(discoveryBibResult) + const item = new Item(discoveryItemResult, bib) + + const { deliveryLocations } = await fetchDeliveryLocations( + item.barcode, + patronId + ) + const pickupLocationLabel = deliveryLocations.find( + (location) => location.value === pickupLocation + )?.label + console.log(pickupLocation) return { props: { isEDD: pickupLocation === "edd", + pickupLocationLabel: pickupLocationLabel || null, + discoveryBibResult, + discoveryItemResult, }, } } catch (error) { From 8d9b937a07992ae5c5f8f793949dcfe0dbad2435 Mon Sep 17 00:00:00 2001 From: Diego Cohen Date: Fri, 6 Dec 2024 14:13:15 -0500 Subject: [PATCH 25/50] Remove console logs --- pages/hold/confirmation/[id].tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pages/hold/confirmation/[id].tsx b/pages/hold/confirmation/[id].tsx index 6513a54c0..ce63b0f44 100644 --- a/pages/hold/confirmation/[id].tsx +++ b/pages/hold/confirmation/[id].tsx @@ -50,7 +50,7 @@ export default function HoldConfirmationPage({ discoveryItemResult, }: HoldConfirmationPageProps) { const metadataTitle = `Request Confirmation | ${SITE_NAME}` - console.log(discoveryBibResult) + const bib = new Bib(discoveryBibResult) const item = new Item(discoveryItemResult, bib) @@ -186,7 +186,7 @@ export async function getServerSideProps({ params, req, res, query }) { const pickupLocationLabel = deliveryLocations.find( (location) => location.value === pickupLocation )?.label - console.log(pickupLocation) + return { props: { isEDD: pickupLocation === "edd", From 580b160ccd38cf223d383c4fb21a0c6cf5b2665e Mon Sep 17 00:00:00 2001 From: Diego Cohen Date: Mon, 9 Dec 2024 09:13:38 -0500 Subject: [PATCH 26/50] Render back to search link --- pages/hold/confirmation/[id].tsx | 18 ++++++++++++++++-- src/components/BibPage/BibDetail.tsx | 2 +- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/pages/hold/confirmation/[id].tsx b/pages/hold/confirmation/[id].tsx index ce63b0f44..b0ab3eab2 100644 --- a/pages/hold/confirmation/[id].tsx +++ b/pages/hold/confirmation/[id].tsx @@ -78,7 +78,8 @@ export default function HoldConfirmationPage({ content={ <> - You're all set! We have received your request for{" "} + You're all set! We have received your{" "} + {isEDD ? "scan " : ""}request for{" "} {item.bibTitle} @@ -106,6 +107,19 @@ export default function HoldConfirmationPage({ } /> + + Start a new search + ) @@ -183,7 +197,7 @@ export async function getServerSideProps({ params, req, res, query }) { item.barcode, patronId ) - const pickupLocationLabel = deliveryLocations.find( + const pickupLocationLabel = deliveryLocations?.find( (location) => location.value === pickupLocation )?.label diff --git a/src/components/BibPage/BibDetail.tsx b/src/components/BibPage/BibDetail.tsx index dd8f9ddf9..73571c1d8 100644 --- a/src/components/BibPage/BibDetail.tsx +++ b/src/components/BibPage/BibDetail.tsx @@ -34,7 +34,7 @@ const BibDetails = ({ details, heading }: BibDetailsProps) => { noStyling type="dl" showRowDividers={false} - className={styles.bibDetails + styles.inBibPage} + className={`${styles.bibDetails} ${styles.inBibPage}`} > {details.map( (detail: BibDetail | LinkedBibDetail | SubjectHeadingDetail) => { From 45b8db3c3033644ff9e669640fcf068d8c5a67dd Mon Sep 17 00:00:00 2001 From: Diego Cohen Date: Mon, 9 Dec 2024 09:14:57 -0500 Subject: [PATCH 27/50] Add margin to search link --- pages/hold/confirmation/[id].tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pages/hold/confirmation/[id].tsx b/pages/hold/confirmation/[id].tsx index b0ab3eab2..868c6f1ff 100644 --- a/pages/hold/confirmation/[id].tsx +++ b/pages/hold/confirmation/[id].tsx @@ -116,7 +116,7 @@ export default function HoldConfirmationPage({ type="standalone" fontWeight="bold" isUnderlined={false} - mt="m" + my="l" > Start a new search From 50e4b4e4fc5638f7b14b9acc2e88246e9d828705 Mon Sep 17 00:00:00 2001 From: Diego Cohen Date: Mon, 9 Dec 2024 09:23:48 -0500 Subject: [PATCH 28/50] Add separate item table for confirmation page --- pages/hold/confirmation/[id].tsx | 30 ++++-------- pages/hold/request/[id]/edd.tsx | 4 +- pages/hold/request/[id]/index.tsx | 4 +- .../HoldPages/HoldConfirmationItemDetails.tsx | 47 +++++++++++++++++++ ...Details.tsx => HoldRequestItemDetails.tsx} | 4 +- 5 files changed, 61 insertions(+), 28 deletions(-) create mode 100644 src/components/HoldPages/HoldConfirmationItemDetails.tsx rename src/components/HoldPages/{HoldItemDetails.tsx => HoldRequestItemDetails.tsx} (90%) diff --git a/pages/hold/confirmation/[id].tsx b/pages/hold/confirmation/[id].tsx index 868c6f1ff..b718be1a6 100644 --- a/pages/hold/confirmation/[id].tsx +++ b/pages/hold/confirmation/[id].tsx @@ -16,7 +16,7 @@ import RCLink from "../../../src/components/Links/RCLink/RCLink" import ExternalLink from "../../../src/components/Links/RCLink/RCLink" import HoldConfirmationFAQ from "../../../src/components/HoldPages/HoldConfirmationFAQ" -import HoldItemDetails from "../../../src/components/HoldPages/HoldItemDetails" +import HoldItemDetails from "../../../src/components/HoldPages/HoldRequestItemDetails" import { fetchHoldDetails, @@ -31,6 +31,7 @@ import initializePatronTokenAuth, { import type { DiscoveryItemResult } from "../../../src/types/itemTypes" import type { DiscoveryBibResult } from "../../../src/types/bibTypes" +import HoldConfirmationItemDetails from "../../../src/components/HoldPages/HoldConfirmationItemDetails" interface HoldConfirmationPageProps { isEDD?: boolean @@ -70,10 +71,10 @@ export default function HoldConfirmationPage({ {isEDD ? EDD_PAGE_HEADING : HOLD_PAGE_HEADING} - + @@ -84,28 +85,13 @@ export default function HoldConfirmationPage({ {item.bibTitle} - {isEDD ? ( - - The item will be delivered to the email address you provided. - - ) : pickupLocationLabel ? ( - - The item will be delivered to: {pickupLocationLabel}. - - ) : ( - - please{" "} - - email us - {" "} - or call 917-ASK-NYPL ( - 917-275-6975) for your - delivery location. - - )} } /> + Request scan - + {isLoading || formPosting ? ( ) : errorStatus !== "eddUnavailable" ? ( diff --git a/pages/hold/request/[id]/index.tsx b/pages/hold/request/[id]/index.tsx index 2c3a1f768..8bfb0ecde 100644 --- a/pages/hold/request/[id]/index.tsx +++ b/pages/hold/request/[id]/index.tsx @@ -11,7 +11,7 @@ import Layout from "../../../../src/components/Layout/Layout" import HoldRequestForm from "../../../../src/components/HoldPages/HoldRequestForm" import HoldRequestErrorBanner from "../../../../src/components/HoldPages/HoldRequestErrorBanner" -import HoldItemDetails from "../../../../src/components/HoldPages/HoldItemDetails" +import HoldRequestItemDetails from "../../../../src/components/HoldPages/HoldRequestItemDetails" import { SITE_NAME, BASE_URL, PATHS } from "../../../../src/config/constants" import useLoading from "../../../../src/hooks/useLoading" @@ -165,7 +165,7 @@ export default function HoldRequestPage({ Request for on-site use - + {isLoading || formPosting ? ( { + return ( + + {pickupLocationLabel ? ( + + ) : ( + <> + )} + + {item.barcode ? ( + + ) : ( + <> + )} + + ) +} + +export default HoldConfirmationItemDetails diff --git a/src/components/HoldPages/HoldItemDetails.tsx b/src/components/HoldPages/HoldRequestItemDetails.tsx similarity index 90% rename from src/components/HoldPages/HoldItemDetails.tsx rename to src/components/HoldPages/HoldRequestItemDetails.tsx index 265df35ec..8a8370fb2 100644 --- a/src/components/HoldPages/HoldItemDetails.tsx +++ b/src/components/HoldPages/HoldRequestItemDetails.tsx @@ -1,10 +1,10 @@ import { List } from "@nypl/design-system-react-components" -import type Item from "../../../src/models/Item" +import type Item from "../../models/Item" import { LinkedDetailElement, PlainTextElement } from "../BibPage/BibDetail" import bibDetailStyles from "../../../styles/components/BibDetails.module.scss" -import { PATHS } from "../../../src/config/constants" +import { PATHS } from "../../config/constants" interface HoldItemDetailsProps { item: Item From ff5e32de7907da6583b91e0f23c84770d23ffdd2 Mon Sep 17 00:00:00 2001 From: Diego Cohen Date: Mon, 9 Dec 2024 09:57:42 -0500 Subject: [PATCH 29/50] Remove redundant item result prop --- __test__/pages/hold/confirmation.test.tsx | 5 ++++- pages/hold/confirmation/[id].tsx | 9 ++------- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/__test__/pages/hold/confirmation.test.tsx b/__test__/pages/hold/confirmation.test.tsx index 546473a9a..4634112bd 100644 --- a/__test__/pages/hold/confirmation.test.tsx +++ b/__test__/pages/hold/confirmation.test.tsx @@ -1,10 +1,13 @@ import HoldConfirmationPage from "../../../pages/hold/confirmation/[id]" import { render, screen } from "../../../src/utils/testUtils" +import { bibWithItems } from "../../fixtures/bibFixtures" describe("Hold Confirmation page", () => { describe("Hold Confirmation page UI", () => { beforeEach(() => { - render() + render( + + ) }) it("renders an H2", () => { diff --git a/pages/hold/confirmation/[id].tsx b/pages/hold/confirmation/[id].tsx index b718be1a6..9635e1588 100644 --- a/pages/hold/confirmation/[id].tsx +++ b/pages/hold/confirmation/[id].tsx @@ -16,7 +16,7 @@ import RCLink from "../../../src/components/Links/RCLink/RCLink" import ExternalLink from "../../../src/components/Links/RCLink/RCLink" import HoldConfirmationFAQ from "../../../src/components/HoldPages/HoldConfirmationFAQ" -import HoldItemDetails from "../../../src/components/HoldPages/HoldRequestItemDetails" +import HoldConfirmationItemDetails from "../../../src/components/HoldPages/HoldConfirmationItemDetails" import { fetchHoldDetails, @@ -29,15 +29,12 @@ import initializePatronTokenAuth, { getLoginRedirect, } from "../../../src/server/auth" -import type { DiscoveryItemResult } from "../../../src/types/itemTypes" import type { DiscoveryBibResult } from "../../../src/types/bibTypes" -import HoldConfirmationItemDetails from "../../../src/components/HoldPages/HoldConfirmationItemDetails" interface HoldConfirmationPageProps { isEDD?: boolean pickupLocationLabel?: string discoveryBibResult: DiscoveryBibResult - discoveryItemResult: DiscoveryItemResult } /** @@ -48,12 +45,11 @@ export default function HoldConfirmationPage({ isEDD = false, pickupLocationLabel, discoveryBibResult, - discoveryItemResult, }: HoldConfirmationPageProps) { const metadataTitle = `Request Confirmation | ${SITE_NAME}` const bib = new Bib(discoveryBibResult) - const item = new Item(discoveryItemResult, bib) + const item = bib.getItemsFromResult[0] return ( <> @@ -192,7 +188,6 @@ export async function getServerSideProps({ params, req, res, query }) { isEDD: pickupLocation === "edd", pickupLocationLabel: pickupLocationLabel || null, discoveryBibResult, - discoveryItemResult, }, } } catch (error) { From 3fa04f5b38fb3699597feb90e2a4a7204c4b08a8 Mon Sep 17 00:00:00 2001 From: Diego Cohen Date: Mon, 9 Dec 2024 10:02:01 -0500 Subject: [PATCH 30/50] Remove query from mocked gerServerSideProps calls --- __test__/pages/hold/eddRequestPage.test.tsx | 6 ------ 1 file changed, 6 deletions(-) diff --git a/__test__/pages/hold/eddRequestPage.test.tsx b/__test__/pages/hold/eddRequestPage.test.tsx index bf4759ebe..56023a55c 100644 --- a/__test__/pages/hold/eddRequestPage.test.tsx +++ b/__test__/pages/hold/eddRequestPage.test.tsx @@ -74,14 +74,12 @@ describe("EDD Request page", () => { params: { id }, req: mockReq, res: mockRes, - query: {}, }) expect(responseWithZeroRedirects.redirect).toBeDefined() const responseWithTwoRedirects = await getServerSideProps({ params: { id: "123-456" }, req: { ...mockReq, cookies: { nyplAccountRedirects: 2 } }, res: mockRes, - query: {}, }) expect(responseWithTwoRedirects.redirect).toBeDefined() }) @@ -98,7 +96,6 @@ describe("EDD Request page", () => { params: { id }, req: mockReq, res: mockRes, - query: {}, }) expect(responseWithoutRedirect.redirect).not.toBeDefined() }) @@ -107,7 +104,6 @@ describe("EDD Request page", () => { params: { id }, req: mockReq, res: mockRes, - query: {}, }) expect(response.redirect).toBeUndefined() }) @@ -123,7 +119,6 @@ describe("EDD Request page", () => { params: { id }, res: mockRes, req: mockReq, - query: {}, }) expect(mockRes.setHeader.mock.calls[0]).toStrictEqual([ "Set-Cookie", @@ -149,7 +144,6 @@ describe("EDD Request page", () => { params: { id }, res: mockRes, req: mockReq, - query: {}, }) expect(responseWithAeonRedirect.redirect).toStrictEqual({ destination: bibWithSingleAeonItem.resource.items[0].aeonUrl[0], From 6fcc965a766276a77c4ab9e1e79e2785d38db5bf Mon Sep 17 00:00:00 2001 From: Diego Cohen Date: Mon, 9 Dec 2024 10:45:12 -0500 Subject: [PATCH 31/50] Fix bib item fetching call --- pages/hold/confirmation/[id].tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pages/hold/confirmation/[id].tsx b/pages/hold/confirmation/[id].tsx index 9635e1588..75e61f89a 100644 --- a/pages/hold/confirmation/[id].tsx +++ b/pages/hold/confirmation/[id].tsx @@ -49,7 +49,7 @@ export default function HoldConfirmationPage({ const metadataTitle = `Request Confirmation | ${SITE_NAME}` const bib = new Bib(discoveryBibResult) - const item = bib.getItemsFromResult[0] + const item = bib?.items[0] return ( <> From 76d283753aa7bfa9184bd15fbf1cb2a26df3da88 Mon Sep 17 00:00:00 2001 From: Diego Cohen Date: Mon, 9 Dec 2024 12:09:30 -0500 Subject: [PATCH 32/50] Update prop names on hold item details components --- src/components/HoldPages/HoldConfirmationItemDetails.tsx | 4 ++-- src/components/HoldPages/HoldRequestItemDetails.tsx | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/components/HoldPages/HoldConfirmationItemDetails.tsx b/src/components/HoldPages/HoldConfirmationItemDetails.tsx index 39e5adca8..d8d7c82e4 100644 --- a/src/components/HoldPages/HoldConfirmationItemDetails.tsx +++ b/src/components/HoldPages/HoldConfirmationItemDetails.tsx @@ -5,7 +5,7 @@ import { PlainTextElement } from "../BibPage/BibDetail" import bibDetailStyles from "../../../styles/components/BibDetails.module.scss" -interface HoldItemDetailsProps { +interface HoldConfirmationItemDetailsProps { item: Item pickupLocationLabel?: string } @@ -16,7 +16,7 @@ interface HoldItemDetailsProps { const HoldConfirmationItemDetails = ({ item, pickupLocationLabel, -}: HoldItemDetailsProps) => { +}: HoldConfirmationItemDetailsProps) => { return ( { +const HoldRequestItemDetails = ({ item }: HoldRequestItemDetailsProps) => { return ( { ) } -export default HoldItemDetails +export default HoldRequestItemDetails From fdd8d6b4fc3ba8bfac7282c77f9f56332361d596 Mon Sep 17 00:00:00 2001 From: Diego Cohen Date: Mon, 9 Dec 2024 12:13:26 -0500 Subject: [PATCH 33/50] Remove duplicate item initialization --- pages/hold/confirmation/[id].tsx | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/pages/hold/confirmation/[id].tsx b/pages/hold/confirmation/[id].tsx index 75e61f89a..dfdfd1e25 100644 --- a/pages/hold/confirmation/[id].tsx +++ b/pages/hold/confirmation/[id].tsx @@ -166,14 +166,13 @@ export async function getServerSideProps({ params, req, res, query }) { throw new Error("No item id in url") } const { discoveryBibResult } = await fetchBib(bibId, {}, itemId) - const discoveryItemResult = discoveryBibResult?.items?.[0] - if (!discoveryItemResult) { + if (!discoveryBibResult?.items?.length) { throw new Error("Hold Confirmation Page - Item not found") } const bib = new Bib(discoveryBibResult) - const item = new Item(discoveryItemResult, bib) + const item = bib.items[0] const { deliveryLocations } = await fetchDeliveryLocations( item.barcode, From 2ed9b4c62cfe71f59e2da27af151957866041e05 Mon Sep 17 00:00:00 2001 From: Diego Cohen Date: Mon, 9 Dec 2024 12:45:27 -0500 Subject: [PATCH 34/50] Remove NextResponse from hold request api handler --- pages/api/hold/request/[id]/index.ts | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/pages/api/hold/request/[id]/index.ts b/pages/api/hold/request/[id]/index.ts index c3a65d1c7..e3ba184a3 100644 --- a/pages/api/hold/request/[id]/index.ts +++ b/pages/api/hold/request/[id]/index.ts @@ -1,5 +1,4 @@ import type { NextApiRequest, NextApiResponse } from "next" -import { NextResponse } from "next/server" import { postHoldRequest, @@ -36,9 +35,8 @@ async function handler(req: NextApiRequest, res: NextApiResponse) { // TODO: Move this to seaprate API route // TODO: Parse these query params in getServerSideProps default: - return NextResponse.redirect( - `${BASE_URL}${PATHS.HOLD_REQUEST}/${holdId}?patronEligibility=${patronEligibilityStatus}`, - 307 + return res.redirect( + `${BASE_URL}${PATHS.HOLD_REQUEST}/${holdId}?patronEligibility=${patronEligibilityStatus}` ) } } @@ -67,14 +65,13 @@ async function handler(req: NextApiRequest, res: NextApiResponse) { } // Server side redirect in case user has JS disabled - return NextResponse.redirect( - `${BASE_URL}${PATHS.HOLD_CONFIRMATION}/${holdId}?pickupLocation=${pickupLocationFromResponse}?requestId=${requestId}`, - 307 + res.redirect( + `${BASE_URL}${PATHS.HOLD_CONFIRMATION}/${holdId}?pickupLocation=${pickupLocationFromResponse}?requestId=${requestId}` ) } catch (error) { const { statusText } = error as Response - return res.status(500).json({ + res.status(500).json({ title: "Error posting hold request to Discovery API", status: 500, detail: statusText || error.message, From fa7dfa3ac68f0aa0eabe66089ae582cabc286d5e Mon Sep 17 00:00:00 2001 From: Diego Cohen Date: Mon, 9 Dec 2024 15:11:47 -0500 Subject: [PATCH 35/50] Rename fetchHoldRequestEligibility to fetchPatronEligibility --- pages/api/hold/request/[id]/index.ts | 4 +- pages/hold/request/[id]/edd.tsx | 4 +- pages/hold/request/[id]/index.tsx | 4 +- src/server/api/hold.ts | 106 +++++++++++++-------------- 4 files changed, 59 insertions(+), 59 deletions(-) diff --git a/pages/api/hold/request/[id]/index.ts b/pages/api/hold/request/[id]/index.ts index e3ba184a3..25f6ee9e4 100644 --- a/pages/api/hold/request/[id]/index.ts +++ b/pages/api/hold/request/[id]/index.ts @@ -2,7 +2,7 @@ import type { NextApiRequest, NextApiResponse } from "next" import { postHoldRequest, - fetchHoldRequestEligibility, + fetchPatronEligibility, } from "../../../../../src/server/api/hold" import { BASE_URL, PATHS } from "../../../../../src/config/constants" @@ -22,7 +22,7 @@ async function handler(req: NextApiRequest, res: NextApiResponse) { const holdId = req.query.id as string const [, itemId] = holdId.split("-") - const patronEligibilityStatus = await fetchHoldRequestEligibility(patronId) + const patronEligibilityStatus = await fetchPatronEligibility(patronId) if (patronEligibilityStatus && !patronEligibilityStatus?.eligibility) { switch (jsEnabled) { diff --git a/pages/hold/request/[id]/edd.tsx b/pages/hold/request/[id]/edd.tsx index bc7261cfd..5af511d63 100644 --- a/pages/hold/request/[id]/edd.tsx +++ b/pages/hold/request/[id]/edd.tsx @@ -18,7 +18,7 @@ import useLoading from "../../../../src/hooks/useLoading" import { fetchBib } from "../../../../src/server/api/bib" import { fetchDeliveryLocations, - fetchHoldRequestEligibility, + fetchPatronEligibility, } from "../../../../src/server/api/hold" import { initialEDDFormState } from "../../../../src/utils/holdPageUtils" @@ -235,7 +235,7 @@ export async function getServerSideProps({ params, req, res }) { const isEddAvailable = eddRequestable && item.isAvailable - const patronEligibilityStatus = await fetchHoldRequestEligibility(patronId) + const patronEligibilityStatus = await fetchPatronEligibility(patronId) if (!patronEligibilityStatus) { console.error( diff --git a/pages/hold/request/[id]/index.tsx b/pages/hold/request/[id]/index.tsx index 8bfb0ecde..264a30e6c 100644 --- a/pages/hold/request/[id]/index.tsx +++ b/pages/hold/request/[id]/index.tsx @@ -19,7 +19,7 @@ import useLoading from "../../../../src/hooks/useLoading" import { fetchBib } from "../../../../src/server/api/bib" import { fetchDeliveryLocations, - fetchHoldRequestEligibility, + fetchPatronEligibility, } from "../../../../src/server/api/hold" import initializePatronTokenAuth, { @@ -257,7 +257,7 @@ export async function getServerSideProps({ params, req, res }) { ) } - const patronEligibilityStatus = await fetchHoldRequestEligibility(patronId) + const patronEligibilityStatus = await fetchPatronEligibility(patronId) if (!patronEligibilityStatus) { console.error( diff --git a/src/server/api/hold.ts b/src/server/api/hold.ts index 320ee8c4e..bb9edf726 100644 --- a/src/server/api/hold.ts +++ b/src/server/api/hold.ts @@ -123,59 +123,6 @@ export async function postHoldRequest( } } -/** - * Getter function for hold request details. - */ -// TODO: Add return type -export async function fetchHoldDetails( - requestId: string -): Promise { - try { - const client = await nyplApiClient() - const holdDetailsResult = await client.get(`/hold-requests/${requestId}`) - const { id, pickupLocation, patron } = holdDetailsResult.data - - return { - requestId: id, - patronId: patron, - pickupLocation, - status: 200, - } - } catch (error) { - console.error( - `Error fetching hold request details in fetchHoldRequestDetails server function, requestId: ${requestId}`, - error.message - ) - - return { status: 400 } - } -} - -/** - * Getter function for hold request eligibility for patrons. - */ -export async function fetchHoldRequestEligibility( - patronId: string -): Promise { - const eligibilityEndpoint = `/patrons/${patronId}/hold-request-eligibility` - - try { - const client = await nyplApiClient() - const eligibilityResult = await client.get(eligibilityEndpoint, { - cache: false, - }) - - return eligibilityResult as PatronEligibilityStatus - } catch (error) { - console.error( - `Error fetching hold request eligibility in fetchHoldRequestEligibility server function, patronId: ${patronId}`, - error.message - ) - - return { eligibility: false } - } -} - /** * Post EDD requests to discovery API. */ @@ -229,3 +176,56 @@ export async function postEDDRequest( } } } + +/** + * Getter function for hold request details. + */ +// TODO: Add return type +export async function fetchHoldDetails( + requestId: string +): Promise { + try { + const client = await nyplApiClient() + const holdDetailsResult = await client.get(`/hold-requests/${requestId}`) + const { id, pickupLocation, patron } = holdDetailsResult.data + + return { + requestId: id, + patronId: patron, + pickupLocation, + status: 200, + } + } catch (error) { + console.error( + `Error fetching hold request details in fetchHoldRequestDetails server function, requestId: ${requestId}`, + error.message + ) + + return { status: 400 } + } +} + +/** + * Getter function for hold request eligibility for patrons. + */ +export async function fetchPatronEligibility( + patronId: string +): Promise { + const eligibilityEndpoint = `/patrons/${patronId}/hold-request-eligibility` + + try { + const client = await nyplApiClient() + const eligibilityResult = await client.get(eligibilityEndpoint, { + cache: false, + }) + + return eligibilityResult as PatronEligibilityStatus + } catch (error) { + console.error( + `Error fetching hold request eligibility in fetchPatronEligibility server function, patronId: ${patronId}`, + error.message + ) + + return { eligibility: false } + } +} From 5c108a7e12b48d2edd742e40df21464e84f02a23 Mon Sep 17 00:00:00 2001 From: Diego Cohen Date: Mon, 9 Dec 2024 15:57:51 -0500 Subject: [PATCH 36/50] Add tests for fetching patron eligibility status --- pages/hold/request/[id]/index.tsx | 2 -- src/server/api/hold.ts | 8 ++++-- src/server/api/tests/hold.test.ts | 45 +++++++++++++++++++++++++++++-- src/types/holdPageTypes.ts | 3 ++- 4 files changed, 51 insertions(+), 7 deletions(-) diff --git a/pages/hold/request/[id]/index.tsx b/pages/hold/request/[id]/index.tsx index 264a30e6c..be94d2318 100644 --- a/pages/hold/request/[id]/index.tsx +++ b/pages/hold/request/[id]/index.tsx @@ -63,8 +63,6 @@ export default function HoldRequestPage({ patronEligibilityStatus: defaultEligibilityStatus, }: HoldRequestPropsType) { const metadataTitle = `Item Request | ${SITE_NAME}` - console.log("defaultEligibilityStatus", defaultEligibilityStatus) - console.log("defaultErrorStatus", defaultErrorStatus) const bib = new Bib(discoveryBibResult) const item = new Item(discoveryItemResult, bib) diff --git a/src/server/api/hold.ts b/src/server/api/hold.ts index bb9edf726..20e7ed146 100644 --- a/src/server/api/hold.ts +++ b/src/server/api/hold.ts @@ -219,13 +219,17 @@ export async function fetchPatronEligibility( cache: false, }) - return eligibilityResult as PatronEligibilityStatus + if (!eligibilityResult.eligibility) { + throw new Error("Improperly formatted eligibility from Discovery API") + } + + return { status: 200, ...eligibilityResult } as PatronEligibilityStatus } catch (error) { console.error( `Error fetching hold request eligibility in fetchPatronEligibility server function, patronId: ${patronId}`, error.message ) - return { eligibility: false } + return { status: 500 } } } diff --git a/src/server/api/tests/hold.test.ts b/src/server/api/tests/hold.test.ts index 9e831fdd9..4f70bcf08 100644 --- a/src/server/api/tests/hold.test.ts +++ b/src/server/api/tests/hold.test.ts @@ -2,9 +2,14 @@ import { fetchDeliveryLocations, postHoldRequest, postEDDRequest, + fetchPatronEligibility, + fetchHoldDetails, } from "../hold" import type { DeliveryLocationsResult } from "../../../types/locationTypes" -import type { HoldPostResult } from "../../../types/holdPageTypes" +import type { + HoldPostResult, + PatronEligibilityStatus, +} from "../../../types/holdPageTypes" jest.mock("../../nyplApiClient", () => { return jest @@ -79,12 +84,30 @@ jest.mock("../../nyplApiClient", () => { .mockImplementationOnce(async () => { return await new Promise((resolve) => { resolve({ - get: () => { + post: () => { throw new Error("Error posting EDD request") }, }) }) }) + .mockImplementationOnce(async () => { + return await new Promise((resolve) => { + resolve({ + get: jest.fn().mockReturnValueOnce({ + eligibility: true, + }), + }) + }) + }) + .mockImplementationOnce(async () => { + return await new Promise((resolve) => { + resolve({ + get: () => { + throw new Error("Error getting patron eligibility status") + }, + }) + }) + }) }) describe("fetchDeliveryLocations", () => { @@ -173,3 +196,21 @@ describe("postEDDRequest", () => { expect(holdPostResult.status).toEqual(500) }) }) + +describe("fetchPatronEligibility", () => { + it("should return a patron's hold eligibility status from Discovery API", async () => { + const patonEligibility = (await fetchPatronEligibility( + "123" + )) as PatronEligibilityStatus + + expect(patonEligibility.status).toEqual(200) + expect(patonEligibility.eligibility).toEqual(true) + }) + it("should return a 500 status if there was an error", async () => { + const patonEligibility = (await fetchPatronEligibility( + "123" + )) as PatronEligibilityStatus + + expect(patonEligibility.status).toEqual(500) + }) +}) diff --git a/src/types/holdPageTypes.ts b/src/types/holdPageTypes.ts index b33f2bcc9..81078d065 100644 --- a/src/types/holdPageTypes.ts +++ b/src/types/holdPageTypes.ts @@ -38,11 +38,12 @@ export interface HoldDetailsResult { } export interface PatronEligibilityStatus { - eligibility: boolean + eligibility?: boolean expired?: boolean moneyOwed?: boolean ptypeDisallowsHolds?: boolean reachedHoldLimit?: boolean + status: HTTPStatusCode } export interface DiscoveryHoldPostParams { From 74393430f955add400ffabef3db7eece7eaf8672 Mon Sep 17 00:00:00 2001 From: Diego Cohen Date: Mon, 9 Dec 2024 16:23:42 -0500 Subject: [PATCH 37/50] Fix patron eligibility error handling and add tests for fetchHoldDetails --- pages/api/hold/request/[id]/index.ts | 2 +- pages/hold/request/[id]/edd.tsx | 11 ++--- pages/hold/request/[id]/index.tsx | 11 ++--- src/server/api/hold.ts | 9 +++- src/server/api/tests/hold.test.ts | 74 +++++++++++++++++++++++++++- src/types/appTypes.ts | 2 +- src/types/holdPageTypes.ts | 1 - 7 files changed, 87 insertions(+), 23 deletions(-) diff --git a/pages/api/hold/request/[id]/index.ts b/pages/api/hold/request/[id]/index.ts index 25f6ee9e4..3141561f3 100644 --- a/pages/api/hold/request/[id]/index.ts +++ b/pages/api/hold/request/[id]/index.ts @@ -24,7 +24,7 @@ async function handler(req: NextApiRequest, res: NextApiResponse) { const patronEligibilityStatus = await fetchPatronEligibility(patronId) - if (patronEligibilityStatus && !patronEligibilityStatus?.eligibility) { + if (patronEligibilityStatus.status === 401) { switch (jsEnabled) { case true: return res.status(401).json({ diff --git a/pages/hold/request/[id]/edd.tsx b/pages/hold/request/[id]/edd.tsx index 5af511d63..58cf34e5e 100644 --- a/pages/hold/request/[id]/edd.tsx +++ b/pages/hold/request/[id]/edd.tsx @@ -237,14 +237,9 @@ export async function getServerSideProps({ params, req, res }) { const patronEligibilityStatus = await fetchPatronEligibility(patronId) - if (!patronEligibilityStatus) { - console.error( - "EDD Page - Error fetching patronEligibilityStatus in getServerSideProps" - ) - } - const locationOrEligibilityFetchFailed = - locationStatus !== 200 || !patronEligibilityStatus + locationStatus !== 200 || + ![200, 401].includes(patronEligibilityStatus?.status) return { props: { @@ -254,7 +249,7 @@ export async function getServerSideProps({ params, req, res }) { isAuthenticated, errorStatus: locationOrEligibilityFetchFailed ? "failed" - : patronEligibilityStatus.eligibility === false + : patronEligibilityStatus.status === 401 ? "patronIneligible" : !isEddAvailable ? "eddUnavailable" diff --git a/pages/hold/request/[id]/index.tsx b/pages/hold/request/[id]/index.tsx index be94d2318..5c4fd90f7 100644 --- a/pages/hold/request/[id]/index.tsx +++ b/pages/hold/request/[id]/index.tsx @@ -257,14 +257,9 @@ export async function getServerSideProps({ params, req, res }) { const patronEligibilityStatus = await fetchPatronEligibility(patronId) - if (!patronEligibilityStatus) { - console.error( - "HoldRequest Page - Error fetching patronEligibilityStatus in getServerSideProps" - ) - } - const locationOrEligibilityFetchFailed = - locationStatus !== 200 || !patronEligibilityStatus + locationStatus !== 200 || + ![200, 401].includes(patronEligibilityStatus?.status) return { props: { @@ -276,7 +271,7 @@ export async function getServerSideProps({ params, req, res }) { patronEligibilityStatus, errorStatus: locationOrEligibilityFetchFailed ? "failed" - : patronEligibilityStatus.eligibility === false + : patronEligibilityStatus.status === 401 ? "patronIneligible" : null, }, diff --git a/src/server/api/hold.ts b/src/server/api/hold.ts index 20e7ed146..97993bd30 100644 --- a/src/server/api/hold.ts +++ b/src/server/api/hold.ts @@ -201,7 +201,7 @@ export async function fetchHoldDetails( error.message ) - return { status: 400 } + return { status: 500 } } } @@ -219,10 +219,15 @@ export async function fetchPatronEligibility( cache: false, }) - if (!eligibilityResult.eligibility) { + // There should always be en eligibilty boolean attribute returned from Discovery API + if (eligibilityResult.eligibility === undefined) { throw new Error("Improperly formatted eligibility from Discovery API") } + if (eligibilityResult.eligibility === false) { + return { status: 401 } + } + return { status: 200, ...eligibilityResult } as PatronEligibilityStatus } catch (error) { console.error( diff --git a/src/server/api/tests/hold.test.ts b/src/server/api/tests/hold.test.ts index 4f70bcf08..3f33d2b54 100644 --- a/src/server/api/tests/hold.test.ts +++ b/src/server/api/tests/hold.test.ts @@ -9,6 +9,7 @@ import type { DeliveryLocationsResult } from "../../../types/locationTypes" import type { HoldPostResult, PatronEligibilityStatus, + HoldDetailsResult, } from "../../../types/holdPageTypes" jest.mock("../../nyplApiClient", () => { @@ -95,6 +96,23 @@ jest.mock("../../nyplApiClient", () => { resolve({ get: jest.fn().mockReturnValueOnce({ eligibility: true, + expired: false, + moneyOwed: false, + ptypeDisallowsHolds: false, + reachedHoldLimit: false, + }), + }) + }) + }) + .mockImplementationOnce(async () => { + return await new Promise((resolve) => { + resolve({ + get: jest.fn().mockReturnValueOnce({ + eligibility: false, + expired: true, + moneyOwed: true, + ptypeDisallowsHolds: false, + reachedHoldLimit: false, }), }) }) @@ -108,6 +126,28 @@ jest.mock("../../nyplApiClient", () => { }) }) }) + .mockImplementationOnce(async () => { + return await new Promise((resolve) => { + resolve({ + get: jest.fn().mockReturnValueOnce({ + data: { + id: "123", + patron: "456", + pickupLocation: "mal17", + }, + }), + }) + }) + }) + .mockImplementationOnce(async () => { + return await new Promise((resolve) => { + resolve({ + get: () => { + throw new Error("Error getting hold details") + }, + }) + }) + }) }) describe("fetchDeliveryLocations", () => { @@ -203,8 +243,21 @@ describe("fetchPatronEligibility", () => { "123" )) as PatronEligibilityStatus - expect(patonEligibility.status).toEqual(200) - expect(patonEligibility.eligibility).toEqual(true) + expect(patonEligibility).toEqual({ + status: 200, + eligibility: true, + expired: false, + moneyOwed: false, + ptypeDisallowsHolds: false, + reachedHoldLimit: false, + }) + }) + it("should return a 401 status if the patron is ineligibile", async () => { + const patonEligibility = (await fetchPatronEligibility( + "123" + )) as PatronEligibilityStatus + + expect(patonEligibility.status).toEqual(401) }) it("should return a 500 status if there was an error", async () => { const patonEligibility = (await fetchPatronEligibility( @@ -214,3 +267,20 @@ describe("fetchPatronEligibility", () => { expect(patonEligibility.status).toEqual(500) }) }) + +describe("fetchHoldDetails", () => { + it("should return details for a given hold request ID from Discovery API", async () => { + const holdDetails = (await fetchHoldDetails("123")) as HoldDetailsResult + expect(holdDetails).toEqual({ + requestId: "123", + patronId: "456", + pickupLocation: "mal17", + status: 200, + }) + }) + + it("should return return a 500 status if there was an error", async () => { + const holdDetails = (await fetchHoldDetails("123")) as HoldDetailsResult + expect(holdDetails.status).toEqual(500) + }) +}) diff --git a/src/types/appTypes.ts b/src/types/appTypes.ts index 4134d650d..89ac8425c 100644 --- a/src/types/appTypes.ts +++ b/src/types/appTypes.ts @@ -23,7 +23,7 @@ export interface Features { export type Environment = "development" | "qa" | "production" -export type HTTPStatusCode = 200 | 307 | 400 | 404 | 500 +export type HTTPStatusCode = 200 | 307 | 400 | 401 | 404 | 500 export type HTTPResponse = { status: HTTPStatusCode diff --git a/src/types/holdPageTypes.ts b/src/types/holdPageTypes.ts index 81078d065..042de74c6 100644 --- a/src/types/holdPageTypes.ts +++ b/src/types/holdPageTypes.ts @@ -38,7 +38,6 @@ export interface HoldDetailsResult { } export interface PatronEligibilityStatus { - eligibility?: boolean expired?: boolean moneyOwed?: boolean ptypeDisallowsHolds?: boolean From 22d5f4eb69d48fcbdd2a19e31a1d428eaf90bb1a Mon Sep 17 00:00:00 2001 From: Diego Cohen Date: Tue, 10 Dec 2024 11:26:53 -0500 Subject: [PATCH 38/50] Separate confirmation related changes --- __test__/pages/hold/eddRequestPage.test.tsx | 26 +-- __test__/pages/hold/requestPage.test.tsx | 4 +- pages/api/hold/request/[id]/index.ts | 29 +-- pages/hold/request/[id]/edd.tsx | 64 +++---- pages/hold/request/[id]/index.tsx | 103 +++++----- src/components/HoldPages/EDDRequestForm.tsx | 13 +- .../HoldPages/HoldRequestBanner.tsx | 95 ++++++++++ .../HoldPages/HoldRequestErrorBanner.tsx | 177 ------------------ src/server/api/hold.ts | 116 ++++++------ src/server/api/tests/hold.test.ts | 68 ------- src/types/appTypes.ts | 2 +- src/types/holdPageTypes.ts | 17 +- src/utils/holdPageUtils.ts | 17 ++ 13 files changed, 270 insertions(+), 461 deletions(-) create mode 100644 src/components/HoldPages/HoldRequestBanner.tsx delete mode 100644 src/components/HoldPages/HoldRequestErrorBanner.tsx diff --git a/__test__/pages/hold/eddRequestPage.test.tsx b/__test__/pages/hold/eddRequestPage.test.tsx index 56023a55c..d1ec932fa 100644 --- a/__test__/pages/hold/eddRequestPage.test.tsx +++ b/__test__/pages/hold/eddRequestPage.test.tsx @@ -19,9 +19,9 @@ import { BASE_URL, PATHS, EDD_FORM_FIELD_COPY, - HOLD_PAGE_ERROR_HEADINGS, } from "../../../src/config/constants" import { fetchDeliveryLocations } from "../../../src/server/api/hold" +import { EDDPageStatusMessages } from "../../../src/utils/holdPageUtils" jest.mock("../../../src/server/auth") jest.mock("../../../src/server/api/bib") @@ -74,12 +74,14 @@ describe("EDD Request page", () => { params: { id }, req: mockReq, res: mockRes, + query: {}, }) expect(responseWithZeroRedirects.redirect).toBeDefined() const responseWithTwoRedirects = await getServerSideProps({ params: { id: "123-456" }, req: { ...mockReq, cookies: { nyplAccountRedirects: 2 } }, res: mockRes, + query: {}, }) expect(responseWithTwoRedirects.redirect).toBeDefined() }) @@ -96,6 +98,7 @@ describe("EDD Request page", () => { params: { id }, req: mockReq, res: mockRes, + query: {}, }) expect(responseWithoutRedirect.redirect).not.toBeDefined() }) @@ -104,6 +107,7 @@ describe("EDD Request page", () => { params: { id }, req: mockReq, res: mockRes, + query: {}, }) expect(response.redirect).toBeUndefined() }) @@ -119,6 +123,7 @@ describe("EDD Request page", () => { params: { id }, res: mockRes, req: mockReq, + query: {}, }) expect(mockRes.setHeader.mock.calls[0]).toStrictEqual([ "Set-Cookie", @@ -144,6 +149,7 @@ describe("EDD Request page", () => { params: { id }, res: mockRes, req: mockReq, + query: {}, }) expect(responseWithAeonRedirect.redirect).toStrictEqual({ destination: bibWithSingleAeonItem.resource.items[0].aeonUrl[0], @@ -232,9 +238,7 @@ describe("EDD Request page", () => { expect(screen.getByTestId("hold-request-error")).toBeInTheDocument() }) - expect( - screen.getByText("Request failed.", { exact: false }) - ).toBeInTheDocument() + expect(screen.getByText("Request failed")).toBeInTheDocument() expect( screen.queryByText( @@ -303,11 +307,11 @@ describe("EDD Request page", () => { discoveryItemResult={bibWithItems.resource.items[0]} patronId="123" isAuthenticated={true} - errorStatus="eddUnavailable" + pageStatus="unavailable" /> ) expect( - screen.getByText(HOLD_PAGE_ERROR_HEADINGS.eddUnavailable) + screen.getByText(EDDPageStatusMessages.unavailable.heading) ).toBeInTheDocument() }) it("shows a failed error message when the page loads with an failed status", async () => { @@ -317,11 +321,11 @@ describe("EDD Request page", () => { discoveryItemResult={bibWithItems.resource.items[0]} patronId="123" isAuthenticated={true} - errorStatus="failed" + pageStatus="failed" /> ) expect( - screen.getByText(HOLD_PAGE_ERROR_HEADINGS.failed) + screen.getByText(EDDPageStatusMessages.failed.heading) ).toBeInTheDocument() }) it("shows an invalid error message when the page loads with an invalid status", async () => { @@ -331,13 +335,11 @@ describe("EDD Request page", () => { discoveryItemResult={bibWithItems.resource.items[0]} patronId="123" isAuthenticated={true} - errorStatus="invalid" + pageStatus="invalid" /> ) expect( - screen.getByText( - "Some fields contain errors. Please correct and submit again." - ) + screen.getByText(EDDPageStatusMessages.invalid.message) ).toBeInTheDocument() }) }) diff --git a/__test__/pages/hold/requestPage.test.tsx b/__test__/pages/hold/requestPage.test.tsx index f8dee1bfa..554d9600b 100644 --- a/__test__/pages/hold/requestPage.test.tsx +++ b/__test__/pages/hold/requestPage.test.tsx @@ -237,9 +237,7 @@ describe("Hold Request page", () => { expect(screen.getByTestId("hold-request-error")).toBeInTheDocument() }) - expect( - screen.getByText("Request failed.", { exact: false }) - ).toBeInTheDocument() + expect(screen.getByText("Request failed")).toBeInTheDocument() expect( screen.queryByText( diff --git a/pages/api/hold/request/[id]/index.ts b/pages/api/hold/request/[id]/index.ts index 3141561f3..bc5111f48 100644 --- a/pages/api/hold/request/[id]/index.ts +++ b/pages/api/hold/request/[id]/index.ts @@ -1,9 +1,6 @@ import type { NextApiRequest, NextApiResponse } from "next" -import { - postHoldRequest, - fetchPatronEligibility, -} from "../../../../../src/server/api/hold" +import { postHoldRequest } from "../../../../../src/server/api/hold" import { BASE_URL, PATHS } from "../../../../../src/config/constants" /** @@ -17,29 +14,10 @@ async function handler(req: NextApiRequest, res: NextApiResponse) { } try { - const { patronId, source, pickupLocation, jsEnabled } = JSON.parse(req.body) - const holdId = req.query.id as string const [, itemId] = holdId.split("-") - const patronEligibilityStatus = await fetchPatronEligibility(patronId) - - if (patronEligibilityStatus.status === 401) { - switch (jsEnabled) { - case true: - return res.status(401).json({ - patronEligibilityStatus, - }) - - // Server side redirect on ineligibility when Js is disabled - // TODO: Move this to seaprate API route - // TODO: Parse these query params in getServerSideProps - default: - return res.redirect( - `${BASE_URL}${PATHS.HOLD_REQUEST}/${holdId}?patronEligibility=${patronEligibilityStatus}` - ) - } - } + const { patronId, source, pickupLocation, jsEnabled } = JSON.parse(req.body) const holdRequestResponse = await postHoldRequest({ itemId, @@ -56,7 +34,6 @@ async function handler(req: NextApiRequest, res: NextApiResponse) { } // Return a 200 status when the hold request is posted successfully via JS fetch - // TODO: Make this a separate API route instead of a fetch attribute if (jsEnabled) { return res.status(200).json({ pickupLocation: pickupLocationFromResponse, @@ -71,7 +48,7 @@ async function handler(req: NextApiRequest, res: NextApiResponse) { } catch (error) { const { statusText } = error as Response - res.status(500).json({ + return res.status(500).json({ title: "Error posting hold request to Discovery API", status: 500, detail: statusText || error.message, diff --git a/pages/hold/request/[id]/edd.tsx b/pages/hold/request/[id]/edd.tsx index 58cf34e5e..058e3c3b4 100644 --- a/pages/hold/request/[id]/edd.tsx +++ b/pages/hold/request/[id]/edd.tsx @@ -9,18 +9,18 @@ import { import Layout from "../../../../src/components/Layout/Layout" import EDDRequestForm from "../../../../src/components/HoldPages/EDDRequestForm" -import HoldRequestErrorBanner from "../../../../src/components/HoldPages/HoldRequestErrorBanner" +import HoldRequestBanner from "../../../../src/components/HoldPages/HoldRequestBanner" import HoldRequestItemDetails from "../../../../src/components/HoldPages/HoldRequestItemDetails" import { SITE_NAME, BASE_URL, PATHS } from "../../../../src/config/constants" import useLoading from "../../../../src/hooks/useLoading" import { fetchBib } from "../../../../src/server/api/bib" +import { fetchDeliveryLocations } from "../../../../src/server/api/hold" import { - fetchDeliveryLocations, - fetchPatronEligibility, -} from "../../../../src/server/api/hold" -import { initialEDDFormState } from "../../../../src/utils/holdPageUtils" + EDDPageStatusMessages, + initialEDDFormState, +} from "../../../../src/utils/holdPageUtils" import initializePatronTokenAuth, { doRedirectBasedOnNyplAccountRedirects, @@ -35,7 +35,7 @@ import type { DiscoveryItemResult } from "../../../../src/types/itemTypes" import type { EDDRequestParams, - HoldErrorStatus, + EDDPageStatus, } from "../../../../src/types/holdPageTypes" interface EDDRequestPropsType { @@ -43,7 +43,8 @@ interface EDDRequestPropsType { discoveryItemResult: DiscoveryItemResult patronId: string isAuthenticated?: boolean - errorStatus?: HoldErrorStatus + eddRequestable?: boolean + pageStatus?: EDDPageStatus } /** @@ -54,7 +55,7 @@ export default function EDDRequestPage({ discoveryItemResult, patronId, isAuthenticated, - errorStatus: defaultErrorStatus, + pageStatus: defaultPageStatus, }: EDDRequestPropsType) { const metadataTitle = `Electronic Delivery Request | ${SITE_NAME}` @@ -63,7 +64,7 @@ export default function EDDRequestPage({ const holdId = `${item.bibId}-${item.id}` - const [errorStatus, setErrorStatus] = useState(defaultErrorStatus) + const [pageStatus, setPageStatus] = useState(defaultPageStatus) const [eddFormState, setEddFormState] = useState({ ...initialEDDFormState, @@ -78,14 +79,10 @@ export default function EDDRequestPage({ const isLoading = useLoading() useEffect(() => { - if ( - errorStatus && - errorStatus !== "invalid" && - bannerContainerRef.current - ) { + if (pageStatus && pageStatus !== "invalid" && bannerContainerRef.current) { bannerContainerRef.current.focus() } - }, [errorStatus]) + }, [pageStatus]) const postEDDRequest = async (eddParams: EDDRequestParams) => { try { @@ -105,13 +102,13 @@ export default function EDDRequestPage({ "HoldRequestPage: Error in edd request api response", responseJson.error ) - setErrorStatus("failed") + setPageStatus("failed") setFormPosting(false) return } const { requestId } = responseJson - setErrorStatus(null) + setPageStatus(null) setFormPosting(false) // Success state @@ -123,7 +120,7 @@ export default function EDDRequestPage({ "HoldRequestPage: Error in hold request api response", error ) - setErrorStatus("failed") + setPageStatus("failed") setFormPosting(false) } } @@ -144,8 +141,13 @@ export default function EDDRequestPage({ {/* Always render the wrapper element that will display the dynamically rendered notification for focus management */} - {errorStatus && ( - + {pageStatus && ( + )} @@ -154,12 +156,12 @@ export default function EDDRequestPage({ {isLoading || formPosting ? ( - ) : errorStatus !== "eddUnavailable" ? ( + ) : pageStatus !== "unavailable" ? ( ) : null} @@ -168,7 +170,7 @@ export default function EDDRequestPage({ ) } -export async function getServerSideProps({ params, req, res }) { +export async function getServerSideProps({ params, req, res, query }) { const { id } = params // authentication redirect @@ -230,30 +232,18 @@ export async function getServerSideProps({ params, req, res }) { await fetchDeliveryLocations(item.barcode, patronId) if (locationStatus !== 200) { - console.error("EDD Page - Error fetching edd in getServerSideProps") + throw new Error("EDD Page - Error fetching edd in getServerSideProps") } const isEddAvailable = eddRequestable && item.isAvailable - const patronEligibilityStatus = await fetchPatronEligibility(patronId) - - const locationOrEligibilityFetchFailed = - locationStatus !== 200 || - ![200, 401].includes(patronEligibilityStatus?.status) - return { props: { discoveryBibResult, discoveryItemResult, patronId, isAuthenticated, - errorStatus: locationOrEligibilityFetchFailed - ? "failed" - : patronEligibilityStatus.status === 401 - ? "patronIneligible" - : !isEddAvailable - ? "eddUnavailable" - : null, + pageStatus: !isEddAvailable ? "unavailable" : null, }, } } catch (error) { diff --git a/pages/hold/request/[id]/index.tsx b/pages/hold/request/[id]/index.tsx index 5c4fd90f7..37322f67b 100644 --- a/pages/hold/request/[id]/index.tsx +++ b/pages/hold/request/[id]/index.tsx @@ -10,7 +10,7 @@ import { import Layout from "../../../../src/components/Layout/Layout" import HoldRequestForm from "../../../../src/components/HoldPages/HoldRequestForm" -import HoldRequestErrorBanner from "../../../../src/components/HoldPages/HoldRequestErrorBanner" +import HoldRequestBanner from "../../../../src/components/HoldPages/HoldRequestBanner" import HoldRequestItemDetails from "../../../../src/components/HoldPages/HoldRequestItemDetails" import { SITE_NAME, BASE_URL, PATHS } from "../../../../src/config/constants" @@ -19,7 +19,7 @@ import useLoading from "../../../../src/hooks/useLoading" import { fetchBib } from "../../../../src/server/api/bib" import { fetchDeliveryLocations, - fetchPatronEligibility, + fetchHoldRequestEligibility, } from "../../../../src/server/api/hold" import initializePatronTokenAuth, { @@ -33,10 +33,6 @@ import Item from "../../../../src/models/Item" import type { DiscoveryBibResult } from "../../../../src/types/bibTypes" import type { DiscoveryItemResult } from "../../../../src/types/itemTypes" import type { DeliveryLocation } from "../../../../src/types/locationTypes" -import type { - HoldErrorStatus, - PatronEligibilityStatus, -} from "../../../../src/types/holdPageTypes" interface HoldRequestPropsType { discoveryBibResult: DiscoveryBibResult @@ -44,8 +40,6 @@ interface HoldRequestPropsType { deliveryLocations?: DeliveryLocation[] patronId: string isAuthenticated?: boolean - errorStatus?: HoldErrorStatus - patronEligibilityStatus?: PatronEligibilityStatus } /** @@ -59,8 +53,6 @@ export default function HoldRequestPage({ deliveryLocations = [], patronId, isAuthenticated, - errorStatus: defaultErrorStatus, - patronEligibilityStatus: defaultEligibilityStatus, }: HoldRequestPropsType) { const metadataTitle = `Item Request | ${SITE_NAME}` @@ -69,10 +61,9 @@ export default function HoldRequestPage({ const holdId = `${item.bibId}-${item.id}` - const [errorStatus, setErrorStatus] = useState(defaultErrorStatus) - const [patronEligibilityStatus, setPatronEligibilityStatus] = useState( - defaultEligibilityStatus - ) + // Initialize alert to true if item is not available. This will show the error banner. + const [alert, setAlert] = useState(!item.isAvailable) + const [errorDetail, setErrorDetail] = useState("") const [formPosting, setFormPosting] = useState(false) const bannerContainerRef = useRef() @@ -80,10 +71,10 @@ export default function HoldRequestPage({ const isLoading = useLoading() useEffect(() => { - if (errorStatus && bannerContainerRef.current) { + if (alert && bannerContainerRef.current) { bannerContainerRef.current.focus() } - }, [errorStatus]) + }, [alert]) const handleSubmit = async (e) => { e.preventDefault() @@ -102,37 +93,32 @@ export default function HoldRequestPage({ }) const responseJson = await response.json() - switch (response.status) { - // Patron is ineligible to place holds - case 401: - setFormPosting(false) - setErrorStatus("patronIneligible") - setPatronEligibilityStatus(responseJson.patronEligibilityStatus) - bannerContainerRef.current.focus() - break - // Server side error placing the hold request - case 500: - setFormPosting(false) - console.error( - "HoldRequestPage: Error in hold request api response", - responseJson.error - ) - setErrorStatus("failed") - break - default: - setFormPosting(false) - // Success state - await router.push( - `${PATHS.HOLD_CONFIRMATION}/${holdId}?pickupLocation=${responseJson.pickupLocation}&requestId=${responseJson.requestId}` - ) + if (response.status !== 200) { + console.error( + "HoldRequestPage: Error in hold request api response", + responseJson.error + ) + setAlert(true) + setErrorDetail(responseJson?.error?.detail || "") + setFormPosting(false) + return } + const { pickupLocation: pickupLocationFromResponse, requestId } = + responseJson + + setFormPosting(false) + + // Success state + await router.push( + `${PATHS.HOLD_CONFIRMATION}/${holdId}?pickupLocation=${pickupLocationFromResponse}&requestId=${requestId}` + ) } catch (error) { console.error( "HoldRequestPage: Error in hold request api response", error ) - setErrorStatus("failed") setFormPosting(false) + setAlert(true) } } @@ -152,11 +138,18 @@ export default function HoldRequestPage({ {/* Always render the wrapper element that will display the dynamically rendered notification for focus management */} - {errorStatus && ( - )} @@ -207,7 +200,7 @@ export async function getServerSideProps({ params, req, res }) { redirectCount + 1 }; Max-Age=10; path=/; domain=.nypl.org;` ) - const redirect = getLoginRedirect(req, `${PATHS.HOLD_REQUEST}/${id}`) + const redirect = getLoginRedirect(req, `${PATHS.HOLD_REQUEST}[${id}]`) return { redirect: { @@ -220,6 +213,10 @@ export async function getServerSideProps({ params, req, res }) { try { const patronId = patronTokenResponse?.decodedPatron?.sub + // TODO: implement this function + const holdRequestEligibility = await fetchHoldRequestEligibility(patronId) + console.log("holdRequestEligibility", holdRequestEligibility) + // fetch bib and item const [bibId, itemId] = id.split("-") @@ -250,17 +247,11 @@ export async function getServerSideProps({ params, req, res }) { await fetchDeliveryLocations(item.barcode, patronId) if (locationStatus !== 200) { - console.error( - "HoldRequest Page - Error fetching deliveryLocations in getServerSideProps" + throw new Error( + "Hold Page - Error fetching delivery locations in getServerSideProps" ) } - const patronEligibilityStatus = await fetchPatronEligibility(patronId) - - const locationOrEligibilityFetchFailed = - locationStatus !== 200 || - ![200, 401].includes(patronEligibilityStatus?.status) - return { props: { discoveryBibResult, @@ -268,12 +259,6 @@ export async function getServerSideProps({ params, req, res }) { deliveryLocations, patronId, isAuthenticated, - patronEligibilityStatus, - errorStatus: locationOrEligibilityFetchFailed - ? "failed" - : patronEligibilityStatus.status === 401 - ? "patronIneligible" - : null, }, } } catch (error) { diff --git a/src/components/HoldPages/EDDRequestForm.tsx b/src/components/HoldPages/EDDRequestForm.tsx index 0a6097ab2..0a7fd3dc8 100644 --- a/src/components/HoldPages/EDDRequestForm.tsx +++ b/src/components/HoldPages/EDDRequestForm.tsx @@ -21,16 +21,13 @@ import { initialEDDInvalidFields, isInvalidField, } from "../../utils/holdPageUtils" -import type { - EDDRequestParams, - HoldErrorStatus, -} from "../../types/holdPageTypes" +import type { EDDRequestParams, EDDPageStatus } from "../../types/holdPageTypes" interface EDDRequestFormProps { eddFormState: EDDRequestParams setEddFormState: React.Dispatch> handleSubmit: (eddParams: EDDRequestParams) => void - setErrorStatus: (errorStatus: HoldErrorStatus) => void + setPageStatus: (status: EDDPageStatus) => void holdId: string } @@ -41,7 +38,7 @@ const EDDRequestForm = ({ eddFormState, setEddFormState, handleSubmit, - setErrorStatus, + setPageStatus, holdId, }: EDDRequestFormProps) => { // Set the invalid fields as an array in state to keep track of the first invalid field for focus on submit @@ -89,10 +86,10 @@ const EDDRequestForm = ({ // Prevent form submission and focus on first invalid field if there is one if (firstInvalidField) { - setErrorStatus("invalid") + setPageStatus("invalid") validatedInputRefs?.[firstInvalidField.key]?.current.focus() } else { - setErrorStatus(null) + setPageStatus(null) handleSubmit(eddFormState) } } diff --git a/src/components/HoldPages/HoldRequestBanner.tsx b/src/components/HoldPages/HoldRequestBanner.tsx new file mode 100644 index 000000000..afaaac058 --- /dev/null +++ b/src/components/HoldPages/HoldRequestBanner.tsx @@ -0,0 +1,95 @@ +import { useContext } from "react" +import { Box, Banner, Button } from "@nypl/design-system-react-components" + +import { FeedbackContext } from "../../../src/context/FeedbackContext" +import type { ItemMetadata } from "../../../src/types/itemTypes" +import type Item from "../../../src/models/Item" +import RCLink from "../Links/RCLink/RCLink" + +interface HoldRequestBannerProps { + item: Item + heading: string + errorMessage: string + errorDetail?: string + pageStatus?: string +} + +/** + * The HoldRequestBanner renders an error notification on the hold page that includes a button to + * open the feedback form, pre-populated with item metadata. + */ +const HoldRequestBanner = ({ + item, + heading, + errorMessage, + errorDetail, + pageStatus = "failed", +}: HoldRequestBannerProps) => { + const { onOpen, setItemMetadata } = useContext(FeedbackContext) + + const onContact = (metadata: ItemMetadata) => { + setItemMetadata(metadata) + onOpen() + } + + return ( + + + {errorMessage} + {pageStatus !== "invalid" ? ( + <> + {" Please try again, "} + {" "} + for assistance, or{" "} + start a new search. + + ) : null} + + {errorDetail ? {errorDetail} : null} + + } + mb="s" + /> + ) +} + +export default HoldRequestBanner diff --git a/src/components/HoldPages/HoldRequestErrorBanner.tsx b/src/components/HoldPages/HoldRequestErrorBanner.tsx deleted file mode 100644 index ee7288b4e..000000000 --- a/src/components/HoldPages/HoldRequestErrorBanner.tsx +++ /dev/null @@ -1,177 +0,0 @@ -import { useContext } from "react" -import { - Text, - Box, - Banner, - Button, - List, -} from "@nypl/design-system-react-components" - -import type { - HoldErrorStatus, - PatronEligibilityStatus, -} from "../../types/holdPageTypes" -import { FeedbackContext } from "../../context/FeedbackContext" -import type { ItemMetadata } from "../../types/itemTypes" -import type Item from "../../models/Item" -import RCLink from "../Links/RCLink/RCLink" -import ExternalLink from "../Links/ExternalLink/ExternalLink" -import { - PATHS, - HOLD_PAGE_ERROR_HEADINGS, - HOLD_PAGE_CONTACT_PREFIXES, -} from "../../config/constants" -import { appConfig } from "../../config/config" - -interface HoldRequestErrorBannerProps { - item: Item - errorStatus?: HoldErrorStatus - patronEligibilityStatus?: PatronEligibilityStatus -} - -/** - * The HoldRequestErrorBanner renders an error notification on the hold page that includes a button to - * open the feedback form, pre-populated with item metadata. - */ -const HoldRequestErrorBanner = ({ - item, - errorStatus = "failed", - patronEligibilityStatus, -}: HoldRequestErrorBannerProps) => { - const { onOpen, setItemMetadata } = useContext(FeedbackContext) - - const onContact = (metadata: ItemMetadata) => { - setItemMetadata(metadata) - onOpen() - } - - return ( - - - {HOLD_PAGE_CONTACT_PREFIXES?.[errorStatus] ? ( - - <> - {HOLD_PAGE_CONTACT_PREFIXES?.[errorStatus]} - {" Please try again, "} - {" "} - for assistance, or{" "} - start a new search. - - - ) : null} - {(() => { - switch (errorStatus) { - case "invalid": - return "Some fields contain errors. Please correct and submit again." - case "patronIneligible": - return patronEligibilityStatus ? ( - - ) : null - default: - return null - } - })()} - - - } - mb="s" - /> - ) -} - -const PatronErrors = ({ - patronEligibilityStatus, -}: { - patronEligibilityStatus: PatronEligibilityStatus -}) => { - const { expired, moneyOwed, ptypeDisallowsHolds, reachedHoldLimit } = - patronEligibilityStatus - - const hasSpecificReason = - expired || moneyOwed || ptypeDisallowsHolds || reachedHoldLimit - - // Generic patron error displayed in heading, don't show reasons list if there isn't one - if (!hasSpecificReason) return null - - return ( - - This is because: - - Your account has expired -- Please see{" "} - - Library Terms and Conditions -- Renewing or Validating Your - Library Card - {" "} - about renewing your card. - , - ] - : []), - ...(moneyOwed - ? [ - <> - Your fines have exceeded the limit — you can pay your fines in - a branch or online from the links under{" "} - My Account. - , - ] - : []), - ...(ptypeDisallowsHolds - ? ["Your card does not permit placing holds on ReCAP materials."] - : []), - ...(reachedHoldLimit - ? ["You have reached the allowed number of holds."] - : []), - ]} - /> - - ) -} - -export default HoldRequestErrorBanner diff --git a/src/server/api/hold.ts b/src/server/api/hold.ts index 97993bd30..cc3ade36f 100644 --- a/src/server/api/hold.ts +++ b/src/server/api/hold.ts @@ -112,6 +112,7 @@ export async function postHoldRequest( requestId, } } catch (error) { + console.log("error", error) console.error( `Error posting hold request in postHoldRequest server function, itemId: ${itemId}`, error.message @@ -123,6 +124,59 @@ export async function postHoldRequest( } } +/** + * Getter function for hold request details. + */ +// TODO: Add return type +export async function fetchHoldDetails( + requestId: string +): Promise { + try { + const client = await nyplApiClient() + const holdDetailsResult = await client.get(`/hold-requests/${requestId}`) + const { id, pickupLocation, patron } = holdDetailsResult.data + + return { + requestId: id, + patronId: patron, + pickupLocation, + status: 200, + } + } catch (error) { + console.error( + `Error fetching hold request details in fetchHoldRequestDetails server function, requestId: ${requestId}`, + error.message + ) + + return { status: 500 } + } +} + +/** + * Getter function for hold request eligibility for patrons. + */ +export async function fetchHoldRequestEligibility( + patronId: string +): Promise { + const eligibilityEndpoint = `/patrons/${patronId}/hold-request-eligibility` + + try { + const client = await nyplApiClient() + const eligibilityResult = await client.get(eligibilityEndpoint, { + cache: false, + }) + + return eligibilityResult as PatronEligibilityStatus + } catch (error) { + console.error( + `Error fetching hold request eligibility in fetchHoldRequestEligibility server function, patronId: ${patronId}`, + error.message + ) + + return { eligibility: false } + } +} + /** * Post EDD requests to discovery API. */ @@ -176,65 +230,3 @@ export async function postEDDRequest( } } } - -/** - * Getter function for hold request details. - */ -// TODO: Add return type -export async function fetchHoldDetails( - requestId: string -): Promise { - try { - const client = await nyplApiClient() - const holdDetailsResult = await client.get(`/hold-requests/${requestId}`) - const { id, pickupLocation, patron } = holdDetailsResult.data - - return { - requestId: id, - patronId: patron, - pickupLocation, - status: 200, - } - } catch (error) { - console.error( - `Error fetching hold request details in fetchHoldRequestDetails server function, requestId: ${requestId}`, - error.message - ) - - return { status: 500 } - } -} - -/** - * Getter function for hold request eligibility for patrons. - */ -export async function fetchPatronEligibility( - patronId: string -): Promise { - const eligibilityEndpoint = `/patrons/${patronId}/hold-request-eligibility` - - try { - const client = await nyplApiClient() - const eligibilityResult = await client.get(eligibilityEndpoint, { - cache: false, - }) - - // There should always be en eligibilty boolean attribute returned from Discovery API - if (eligibilityResult.eligibility === undefined) { - throw new Error("Improperly formatted eligibility from Discovery API") - } - - if (eligibilityResult.eligibility === false) { - return { status: 401 } - } - - return { status: 200, ...eligibilityResult } as PatronEligibilityStatus - } catch (error) { - console.error( - `Error fetching hold request eligibility in fetchPatronEligibility server function, patronId: ${patronId}`, - error.message - ) - - return { status: 500 } - } -} diff --git a/src/server/api/tests/hold.test.ts b/src/server/api/tests/hold.test.ts index 3f33d2b54..6a310653e 100644 --- a/src/server/api/tests/hold.test.ts +++ b/src/server/api/tests/hold.test.ts @@ -2,13 +2,11 @@ import { fetchDeliveryLocations, postHoldRequest, postEDDRequest, - fetchPatronEligibility, fetchHoldDetails, } from "../hold" import type { DeliveryLocationsResult } from "../../../types/locationTypes" import type { HoldPostResult, - PatronEligibilityStatus, HoldDetailsResult, } from "../../../types/holdPageTypes" @@ -91,41 +89,6 @@ jest.mock("../../nyplApiClient", () => { }) }) }) - .mockImplementationOnce(async () => { - return await new Promise((resolve) => { - resolve({ - get: jest.fn().mockReturnValueOnce({ - eligibility: true, - expired: false, - moneyOwed: false, - ptypeDisallowsHolds: false, - reachedHoldLimit: false, - }), - }) - }) - }) - .mockImplementationOnce(async () => { - return await new Promise((resolve) => { - resolve({ - get: jest.fn().mockReturnValueOnce({ - eligibility: false, - expired: true, - moneyOwed: true, - ptypeDisallowsHolds: false, - reachedHoldLimit: false, - }), - }) - }) - }) - .mockImplementationOnce(async () => { - return await new Promise((resolve) => { - resolve({ - get: () => { - throw new Error("Error getting patron eligibility status") - }, - }) - }) - }) .mockImplementationOnce(async () => { return await new Promise((resolve) => { resolve({ @@ -237,37 +200,6 @@ describe("postEDDRequest", () => { }) }) -describe("fetchPatronEligibility", () => { - it("should return a patron's hold eligibility status from Discovery API", async () => { - const patonEligibility = (await fetchPatronEligibility( - "123" - )) as PatronEligibilityStatus - - expect(patonEligibility).toEqual({ - status: 200, - eligibility: true, - expired: false, - moneyOwed: false, - ptypeDisallowsHolds: false, - reachedHoldLimit: false, - }) - }) - it("should return a 401 status if the patron is ineligibile", async () => { - const patonEligibility = (await fetchPatronEligibility( - "123" - )) as PatronEligibilityStatus - - expect(patonEligibility.status).toEqual(401) - }) - it("should return a 500 status if there was an error", async () => { - const patonEligibility = (await fetchPatronEligibility( - "123" - )) as PatronEligibilityStatus - - expect(patonEligibility.status).toEqual(500) - }) -}) - describe("fetchHoldDetails", () => { it("should return details for a given hold request ID from Discovery API", async () => { const holdDetails = (await fetchHoldDetails("123")) as HoldDetailsResult diff --git a/src/types/appTypes.ts b/src/types/appTypes.ts index 89ac8425c..4134d650d 100644 --- a/src/types/appTypes.ts +++ b/src/types/appTypes.ts @@ -23,7 +23,7 @@ export interface Features { export type Environment = "development" | "qa" | "production" -export type HTTPStatusCode = 200 | 307 | 400 | 401 | 404 | 500 +export type HTTPStatusCode = 200 | 307 | 400 | 404 | 500 export type HTTPResponse = { status: HTTPStatusCode diff --git a/src/types/holdPageTypes.ts b/src/types/holdPageTypes.ts index 042de74c6..60e9fa2d0 100644 --- a/src/types/holdPageTypes.ts +++ b/src/types/holdPageTypes.ts @@ -38,11 +38,13 @@ export interface HoldDetailsResult { } export interface PatronEligibilityStatus { + eligibility: boolean expired?: boolean + blocked?: boolean moneyOwed?: boolean ptypeDisallowsHolds?: boolean reachedHoldLimit?: boolean - status: HTTPStatusCode + hasIssues?: boolean } export interface DiscoveryHoldPostParams { @@ -57,13 +59,12 @@ export interface DiscoveryHoldPostParams { docDeliveryData?: EDDRequestParams } -export type HoldErrorStatus = - | null - | "failed" - | "eddUnavailable" - | "invalid" - | "patronIneligible" - | "serverError" +export type EDDPageStatus = null | "failed" | "unavailable" | "invalid" + +export interface EDDStatusMessage { + heading?: string + message: string +} export interface EDDFormAction { type: EDDFormActionType diff --git a/src/utils/holdPageUtils.ts b/src/utils/holdPageUtils.ts index d5161c033..02209ffa1 100644 --- a/src/utils/holdPageUtils.ts +++ b/src/utils/holdPageUtils.ts @@ -1,6 +1,8 @@ import type { EDDRequestParams, EDDFormValidatedField, + EDDPageStatus, + EDDStatusMessage, } from "../types/holdPageTypes" import { EMAIL_REGEX } from "../config/constants" @@ -21,6 +23,21 @@ export const initialEDDFormState: EDDRequestParams = { pickupLocation: "edd", } +export const EDDPageStatusMessages: Record = { + failed: { + heading: "Request failed", + message: "We were unable to process your request at this time.", + }, + unavailable: { + heading: "Electronic delivery unavailable", + message: + "Electronic delivery options for this item are currently unavailable.", + }, + invalid: { + message: "Some fields contain errors. Please correct and submit again.", + }, +} + // Initial state for invalid fields in the EDD form to keep track of the first invalid field for focus on submit export const initialEDDInvalidFields: EDDFormValidatedField[] = [ { key: "emailAddress", isInvalid: false }, From 06a100b7848c393bff912c34229b5a75b1ed6a5c Mon Sep 17 00:00:00 2001 From: Diego Cohen Date: Tue, 10 Dec 2024 11:35:41 -0500 Subject: [PATCH 39/50] Restore config file --- src/config/config.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/config/config.ts b/src/config/config.ts index a9bbe1892..fe46b743e 100644 --- a/src/config/config.ts +++ b/src/config/config.ts @@ -69,7 +69,6 @@ export const appConfig: AppConfig = { researchMaterialsHelp: "https://www.nypl.org/help/request-research-materials", tokenUrl: "https://isso.nypl.org/", - renewCard: "https://www.nypl.org/help/library-card/terms-conditions#renew", }, // Array of closed nypl location keys (available options for NYPL locations: all, schwarzman, schomburg, lpa) closedLocations: [] as (NYPLocationKey | "all")[], From 8be9f65f092a6d980acc8c4cf3ecde5feba1bbde Mon Sep 17 00:00:00 2001 From: Diego Cohen Date: Tue, 10 Dec 2024 13:16:00 -0500 Subject: [PATCH 40/50] Add confirmation page tests --- __test__/pages/hold/confirmation.test.tsx | 223 +++++++++++++++++- pages/hold/confirmation/[id].tsx | 1 - .../HoldPages/HoldConfirmationFAQ.tsx | 6 +- 3 files changed, 219 insertions(+), 11 deletions(-) diff --git a/__test__/pages/hold/confirmation.test.tsx b/__test__/pages/hold/confirmation.test.tsx index 4634112bd..ec79c804d 100644 --- a/__test__/pages/hold/confirmation.test.tsx +++ b/__test__/pages/hold/confirmation.test.tsx @@ -1,12 +1,140 @@ -import HoldConfirmationPage from "../../../pages/hold/confirmation/[id]" +import HoldConfirmationPage, { + getServerSideProps, +} from "../../../pages/hold/confirmation/[id]" import { render, screen } from "../../../src/utils/testUtils" import { bibWithItems } from "../../fixtures/bibFixtures" +import initializePatronTokenAuth, { + doRedirectBasedOnNyplAccountRedirects, +} from "../../../src/server/auth" + +import { fetchBib } from "../../../src/server/api/bib" +import { + fetchHoldDetails, + fetchDeliveryLocations, +} from "../../../src/server/api/hold" + +jest.mock("../../../src/server/auth") +jest.mock("../../../src/server/api/bib") +jest.mock("../../../src/server/sierraClient") +jest.mock("../../../src/server/api/hold") + +jest.mock("next/router", () => jest.requireActual("next-router-mock")) + +const mockRes = { + setHeader: jest.fn(), +} +const id = "b15080796-i39333697" +const mockReq = { + headers: { + host: "local.nypl.org:8080", + }, + url: `/hold/confirmation/${id}`, + cookies: { + nyplIdentityPatron: '{"access_token":123}', + }, +} + describe("Hold Confirmation page", () => { - describe("Hold Confirmation page UI", () => { + describe("logout redirect handling", () => { + beforeEach(() => { + ;(initializePatronTokenAuth as jest.Mock).mockResolvedValue({ + isTokenValid: true, + errorCode: null, + decodedPatron: { sub: "123" }, + }) + ;(fetchHoldDetails as jest.Mock).mockResolvedValue({ + patronId: "123", + pickupLocation: "mal17", + status: 200, + }) + ;(fetchBib as jest.Mock).mockResolvedValue({ + discoveryBibResult: bibWithItems.resource, + status: 200, + }) + ;(fetchDeliveryLocations as jest.Mock).mockResolvedValue({ + eddRequestable: true, + status: 200, + }) + }) + + it("redirects if cookie count is less than 3", async () => { + ;(doRedirectBasedOnNyplAccountRedirects as jest.Mock).mockReturnValue( + true + ) + ;(initializePatronTokenAuth as jest.Mock).mockResolvedValue({ + isTokenValid: false, + }) + + const responseWithZeroRedirects = await getServerSideProps({ + params: { id }, + req: mockReq, + res: mockRes, + query: {}, + }) + expect(responseWithZeroRedirects.redirect).toBeDefined() + const responseWithTwoRedirects = await getServerSideProps({ + params: { id: "123-456" }, + req: { ...mockReq, cookies: { nyplAccountRedirects: 2 } }, + res: mockRes, + query: {}, + }) + expect(responseWithTwoRedirects.redirect).toBeDefined() + }) + it("does not redirect if doRedirect method returns false", async () => { + ;(doRedirectBasedOnNyplAccountRedirects as jest.Mock).mockReturnValue( + false + ) + ;(initializePatronTokenAuth as jest.Mock).mockResolvedValue({ + decodedPatron: { sub: "123" }, + isTokenValid: false, + }) + + const responseWithoutRedirect = await getServerSideProps({ + params: { id }, + req: mockReq, + res: mockRes, + query: {}, + }) + expect(responseWithoutRedirect.redirect).not.toBeDefined() + }) + it("does not redirect if patron is authenticated", async () => { + const response = await getServerSideProps({ + params: { id }, + req: mockReq, + res: mockRes, + query: {}, + }) + expect(response.redirect).toBeUndefined() + }) + it("updates the nyplAccountRedirectsCookie upon redirecting", async () => { + ;(doRedirectBasedOnNyplAccountRedirects as jest.Mock).mockReturnValue( + true + ) + ;(initializePatronTokenAuth as jest.Mock).mockResolvedValue({ + decodedPatron: { sub: "123" }, + isTokenValid: false, + }) + await getServerSideProps({ + params: { id }, + res: mockRes, + req: mockReq, + query: {}, + }) + expect(mockRes.setHeader.mock.calls[0]).toStrictEqual([ + "Set-Cookie", + "nyplAccountRedirects=1; Max-Age=10; path=/; domain=.nypl.org;", + ]) + }) + }) + + describe("On-site Confirmation page UI", () => { beforeEach(() => { render( - + ) }) @@ -16,14 +144,95 @@ describe("Hold Confirmation page", () => { ) }) - it("renders a success banner", () => { + it("renders a success banner with a link to the requested item's bib", () => { expect(screen.getAllByRole("heading", { level: 2 })[1]).toHaveTextContent( "Request successful" ) + expect( + screen.queryByText( + "You're all set! We have received your request for", + { + exact: false, + } + ) + ).toBeInTheDocument() + + const bibLink = screen.getByText("Urban spaghetti.") + expect(bibLink).toHaveAttribute( + "href", + "/research/research-catalog/bib/b15080796" + ) + }) + it("renders an item details table with a pickup location for on-site holds", () => { + expect(screen.getByTestId("pickup-location")).toHaveTextContent( + "Schwarzman Building" + ) + expect(screen.getByTestId("call-number")).toHaveTextContent( + "JFK 01-374 no. 4 (2001)" + ) + expect(screen.getByTestId("barcode")).toHaveTextContent("33433130221975") + }) + it("renders an on-site specific faq accordion", () => { + expect(screen.getByTestId("on-site-confirmation-faq")).toBeInTheDocument() + }) + it("renders a back to search link", () => { + const searchLink = screen.getByText("Start a new search") + expect(searchLink).toHaveAttribute( + "href", + "/research/research-catalog/search" + ) + }) + }) + describe("Electronic Delivery Confirmation page UI", () => { + beforeEach(() => { + render( + + ) + }) + + it("renders an H2", () => { + expect(screen.getAllByRole("heading", { level: 2 })[0]).toHaveTextContent( + "Request scan" + ) + }) + + it("renders a success banner with a link to the requested item's bib", () => { + expect(screen.getAllByRole("heading", { level: 2 })[1]).toHaveTextContent( + "Request successful" + ) + expect( + screen.queryByText( + "You're all set! We have received your scan request for", + { + exact: false, + } + ) + ).toBeInTheDocument() + + const bibLink = screen.getByText("Urban spaghetti.") + expect(bibLink).toHaveAttribute( + "href", + "/research/research-catalog/bib/b15080796" + ) + }) + it("renders an item details table without a pickup location for EDD holds", () => { + expect(screen.queryByTestId("pickup-location")).not.toBeInTheDocument() + expect(screen.getByTestId("call-number")).toHaveTextContent( + "JFK 01-374 no. 4 (2001)" + ) + expect(screen.getByTestId("barcode")).toHaveTextContent("33433130221975") + }) + it("renders an edd-specific faq accordion", () => { + expect(screen.getByTestId("edd-confirmation-faq")).toBeInTheDocument() }) - it("renders a faq accordion", () => { - expect(screen.getByRole("heading", { level: 3 })).toHaveTextContent( - "Frequently asked questions" + it("renders a back to search link", () => { + const searchLink = screen.getByText("Start a new search") + expect(searchLink).toHaveAttribute( + "href", + "/research/research-catalog/search" ) }) }) diff --git a/pages/hold/confirmation/[id].tsx b/pages/hold/confirmation/[id].tsx index dfdfd1e25..b626eb493 100644 --- a/pages/hold/confirmation/[id].tsx +++ b/pages/hold/confirmation/[id].tsx @@ -10,7 +10,6 @@ import { } from "../../../src/config/constants" import Bib from "../../../src/models/Bib" -import Item from "../../../src/models/Item" import RCLink from "../../../src/components/Links/RCLink/RCLink" import ExternalLink from "../../../src/components/Links/RCLink/RCLink" diff --git a/src/components/HoldPages/HoldConfirmationFAQ.tsx b/src/components/HoldPages/HoldConfirmationFAQ.tsx index 3dd35f3b3..92a6f5a20 100644 --- a/src/components/HoldPages/HoldConfirmationFAQ.tsx +++ b/src/components/HoldPages/HoldConfirmationFAQ.tsx @@ -1,4 +1,4 @@ -import { Heading, Accordion } from "@nypl/design-system-react-components" +import { Heading, Accordion, Box } from "@nypl/design-system-react-components" import { holdConfirmationFAQData, @@ -14,7 +14,7 @@ interface HoldConfirmationFAQProps { **/ const HoldConfirmationFAQ = ({ isEDD = false }: HoldConfirmationFAQProps) => { return ( - <> + Frequently asked questions @@ -22,7 +22,7 @@ const HoldConfirmationFAQ = ({ isEDD = false }: HoldConfirmationFAQProps) => { accordionData={isEDD ? eddConfirmationFAQData : holdConfirmationFAQData} isDefaultOpen /> - + ) } From c2cf8c3e2e79b887831c56c21dca9f9fb4c2b567 Mon Sep 17 00:00:00 2001 From: Diego Cohen Date: Tue, 10 Dec 2024 13:19:46 -0500 Subject: [PATCH 41/50] Update changelog --- CHANGELOG | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/CHANGELOG b/CHANGELOG index a93fc17c7..62d3b8fb0 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -5,7 +5,12 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). -## Unreleased +## Prerelease + +### Added + +- Added and integrated hold confirmation details fetcher function (SCC-3762) +- Added server-side auth redirect to hold confirmation page (SCC-3762) ### Updated From 122cded872a67427123d0666a4dd79ec750ef914 Mon Sep 17 00:00:00 2001 From: Diego Cohen Date: Tue, 10 Dec 2024 15:06:08 -0500 Subject: [PATCH 42/50] Remove ternaries from HoldConfirmationItemDetails --- src/components/HoldPages/HoldConfirmationItemDetails.tsx | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/components/HoldPages/HoldConfirmationItemDetails.tsx b/src/components/HoldPages/HoldConfirmationItemDetails.tsx index d8d7c82e4..eed39a1a7 100644 --- a/src/components/HoldPages/HoldConfirmationItemDetails.tsx +++ b/src/components/HoldPages/HoldConfirmationItemDetails.tsx @@ -26,19 +26,15 @@ const HoldConfirmationItemDetails = ({ mb="l" mt={0} > - {pickupLocationLabel ? ( + {pickupLocationLabel && ( - ) : ( - <> )} - {item.barcode ? ( + {item.barcode && ( - ) : ( - <> )} ) From 82533e84e8771bc9858c495b308fe75e5c6cbe50 Mon Sep 17 00:00:00 2001 From: Diego Cohen Date: Tue, 10 Dec 2024 17:19:25 -0500 Subject: [PATCH 43/50] Comment out eligibility fetch --- pages/hold/request/[id]/index.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pages/hold/request/[id]/index.tsx b/pages/hold/request/[id]/index.tsx index 37322f67b..5bbc6ada8 100644 --- a/pages/hold/request/[id]/index.tsx +++ b/pages/hold/request/[id]/index.tsx @@ -214,8 +214,8 @@ export async function getServerSideProps({ params, req, res }) { const patronId = patronTokenResponse?.decodedPatron?.sub // TODO: implement this function - const holdRequestEligibility = await fetchHoldRequestEligibility(patronId) - console.log("holdRequestEligibility", holdRequestEligibility) + // const holdRequestEligibility = await fetchHoldRequestEligibility(patronId) + // console.log("holdRequestEligibility", holdRequestEligibility) // fetch bib and item const [bibId, itemId] = id.split("-") From 52a1e65bd2321c7a935bd7171aff6a1b1fcf96f7 Mon Sep 17 00:00:00 2001 From: Diego Cohen Date: Tue, 10 Dec 2024 18:30:09 -0500 Subject: [PATCH 44/50] Restore fragment syntax --- src/components/HoldPages/HoldConfirmationItemDetails.tsx | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/components/HoldPages/HoldConfirmationItemDetails.tsx b/src/components/HoldPages/HoldConfirmationItemDetails.tsx index eed39a1a7..d8d7c82e4 100644 --- a/src/components/HoldPages/HoldConfirmationItemDetails.tsx +++ b/src/components/HoldPages/HoldConfirmationItemDetails.tsx @@ -26,15 +26,19 @@ const HoldConfirmationItemDetails = ({ mb="l" mt={0} > - {pickupLocationLabel && ( + {pickupLocationLabel ? ( + ) : ( + <> )} - {item.barcode && ( + {item.barcode ? ( + ) : ( + <> )} ) From 5ba879fcba8240f3b1a3bcfc8f808adaf5f58510 Mon Sep 17 00:00:00 2001 From: Diego Cohen Date: Wed, 11 Dec 2024 11:25:08 -0500 Subject: [PATCH 45/50] Pass list children as props in detail element --- src/components/BibPage/BibDetail.tsx | 12 +++++++++--- .../HoldPages/HoldConfirmationItemDetails.tsx | 8 ++------ src/components/HoldPages/HoldRequestItemDetails.tsx | 4 +--- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/components/BibPage/BibDetail.tsx b/src/components/BibPage/BibDetail.tsx index 73571c1d8..4fbbcd86c 100644 --- a/src/components/BibPage/BibDetail.tsx +++ b/src/components/BibPage/BibDetail.tsx @@ -14,6 +14,7 @@ import type { } from "../../types/bibDetailsTypes" import { rtlOrLtr, isItTheLastElement } from "../../utils/bibUtils" import type { ReactNode } from "react" +import { child } from "winston" interface BibDetailsProps { details: AnyBibDetail[] @@ -60,9 +61,14 @@ const DetailElement = (label: string, listChildren: ReactNode[]) => { <>
{label}
- - {listChildren} - + ( + <>{listElement} + ))} + />
) diff --git a/src/components/HoldPages/HoldConfirmationItemDetails.tsx b/src/components/HoldPages/HoldConfirmationItemDetails.tsx index d8d7c82e4..eed39a1a7 100644 --- a/src/components/HoldPages/HoldConfirmationItemDetails.tsx +++ b/src/components/HoldPages/HoldConfirmationItemDetails.tsx @@ -26,19 +26,15 @@ const HoldConfirmationItemDetails = ({ mb="l" mt={0} > - {pickupLocationLabel ? ( + {pickupLocationLabel && ( - ) : ( - <> )} - {item.barcode ? ( + {item.barcode && ( - ) : ( - <> )} ) diff --git a/src/components/HoldPages/HoldRequestItemDetails.tsx b/src/components/HoldPages/HoldRequestItemDetails.tsx index eb3e9c599..7b3a87052 100644 --- a/src/components/HoldPages/HoldRequestItemDetails.tsx +++ b/src/components/HoldPages/HoldRequestItemDetails.tsx @@ -29,10 +29,8 @@ const HoldRequestItemDetails = ({ item }: HoldRequestItemDetailsProps) => { link="internal" /> - {item.volume ? ( + {item.volume && ( - ) : ( - <> )} ) From 702b3ec82560a5853ce889880276cc09e785a1f5 Mon Sep 17 00:00:00 2001 From: Diego Cohen Date: Wed, 11 Dec 2024 11:34:54 -0500 Subject: [PATCH 46/50] Remove fragment --- pages/hold/confirmation/[id].tsx | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/pages/hold/confirmation/[id].tsx b/pages/hold/confirmation/[id].tsx index b626eb493..560580997 100644 --- a/pages/hold/confirmation/[id].tsx +++ b/pages/hold/confirmation/[id].tsx @@ -72,15 +72,13 @@ export default function HoldConfirmationPage({ mb="l" heading="Request successful" content={ - <> - - You're all set! We have received your{" "} - {isEDD ? "scan " : ""}request for{" "} - - {item.bibTitle} - - - + + You're all set! We have received your {isEDD ? "scan " : ""} + request for{" "} + + {item.bibTitle} + + } /> Date: Wed, 11 Dec 2024 11:46:35 -0500 Subject: [PATCH 47/50] Restore fragments --- src/components/BibPage/BibDetail.tsx | 11 +++-------- .../HoldPages/HoldConfirmationItemDetails.tsx | 8 ++++++-- src/components/HoldPages/HoldRequestItemDetails.tsx | 4 +++- 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/src/components/BibPage/BibDetail.tsx b/src/components/BibPage/BibDetail.tsx index 4fbbcd86c..dc9e92386 100644 --- a/src/components/BibPage/BibDetail.tsx +++ b/src/components/BibPage/BibDetail.tsx @@ -61,14 +61,9 @@ const DetailElement = (label: string, listChildren: ReactNode[]) => { <>
{label}
- ( - <>{listElement} - ))} - /> + + {listChildren} +
) diff --git a/src/components/HoldPages/HoldConfirmationItemDetails.tsx b/src/components/HoldPages/HoldConfirmationItemDetails.tsx index eed39a1a7..d8d7c82e4 100644 --- a/src/components/HoldPages/HoldConfirmationItemDetails.tsx +++ b/src/components/HoldPages/HoldConfirmationItemDetails.tsx @@ -26,15 +26,19 @@ const HoldConfirmationItemDetails = ({ mb="l" mt={0} > - {pickupLocationLabel && ( + {pickupLocationLabel ? ( + ) : ( + <> )} - {item.barcode && ( + {item.barcode ? ( + ) : ( + <> )} ) diff --git a/src/components/HoldPages/HoldRequestItemDetails.tsx b/src/components/HoldPages/HoldRequestItemDetails.tsx index 7b3a87052..eb3e9c599 100644 --- a/src/components/HoldPages/HoldRequestItemDetails.tsx +++ b/src/components/HoldPages/HoldRequestItemDetails.tsx @@ -29,8 +29,10 @@ const HoldRequestItemDetails = ({ item }: HoldRequestItemDetailsProps) => { link="internal" /> - {item.volume && ( + {item.volume ? ( + ) : ( + <> )} ) From dbc84d18297af8f5a0af827fcff16daa68e88f97 Mon Sep 17 00:00:00 2001 From: Diego Cohen Date: Wed, 11 Dec 2024 11:52:15 -0500 Subject: [PATCH 48/50] Remove bib and item initialization from getServerSideProps --- pages/hold/confirmation/[id].tsx | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pages/hold/confirmation/[id].tsx b/pages/hold/confirmation/[id].tsx index 560580997..144402640 100644 --- a/pages/hold/confirmation/[id].tsx +++ b/pages/hold/confirmation/[id].tsx @@ -164,15 +164,15 @@ export async function getServerSideProps({ params, req, res, query }) { } const { discoveryBibResult } = await fetchBib(bibId, {}, itemId) - if (!discoveryBibResult?.items?.length) { + // Get the item barcode's directly from discoveryBibResult to avoid initializing the entire Item model. + const itemBarcode = discoveryBibResult?.items?.[0]?.["idBarcode"]?.[0] + + if (!itemBarcode) { throw new Error("Hold Confirmation Page - Item not found") } - const bib = new Bib(discoveryBibResult) - const item = bib.items[0] - const { deliveryLocations } = await fetchDeliveryLocations( - item.barcode, + itemBarcode, patronId ) const pickupLocationLabel = deliveryLocations?.find( From fdf92d64e4a85718b87fbb56ff8c11a96ab06c2b Mon Sep 17 00:00:00 2001 From: Diego Cohen Date: Wed, 11 Dec 2024 11:52:52 -0500 Subject: [PATCH 49/50] Fix comment --- pages/hold/confirmation/[id].tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pages/hold/confirmation/[id].tsx b/pages/hold/confirmation/[id].tsx index 144402640..4f5eb2aca 100644 --- a/pages/hold/confirmation/[id].tsx +++ b/pages/hold/confirmation/[id].tsx @@ -168,7 +168,9 @@ export async function getServerSideProps({ params, req, res, query }) { const itemBarcode = discoveryBibResult?.items?.[0]?.["idBarcode"]?.[0] if (!itemBarcode) { - throw new Error("Hold Confirmation Page - Item not found") + throw new Error( + "Hold Confirmation Page - Item barcode not found in discoveryBibResult" + ) } const { deliveryLocations } = await fetchDeliveryLocations( From e89a97642be31ac20c39cdd58f94f4d0455aba8f Mon Sep 17 00:00:00 2001 From: Diego Cohen Date: Thu, 12 Dec 2024 14:09:27 -0500 Subject: [PATCH 50/50] Remove winston line --- src/components/BibPage/BibDetail.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/src/components/BibPage/BibDetail.tsx b/src/components/BibPage/BibDetail.tsx index dc9e92386..73571c1d8 100644 --- a/src/components/BibPage/BibDetail.tsx +++ b/src/components/BibPage/BibDetail.tsx @@ -14,7 +14,6 @@ import type { } from "../../types/bibDetailsTypes" import { rtlOrLtr, isItTheLastElement } from "../../utils/bibUtils" import type { ReactNode } from "react" -import { child } from "winston" interface BibDetailsProps { details: AnyBibDetail[]