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 diff --git a/__test__/pages/hold/confirmation.test.tsx b/__test__/pages/hold/confirmation.test.tsx index 546473a9a..ec79c804d 100644 --- a/__test__/pages/hold/confirmation.test.tsx +++ b/__test__/pages/hold/confirmation.test.tsx @@ -1,10 +1,141 @@ -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() + render( + + ) }) it("renders an H2", () => { @@ -13,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 5304df2cc..4f5eb2aca 100644 --- a/pages/hold/confirmation/[id].tsx +++ b/pages/hold/confirmation/[id].tsx @@ -9,18 +9,31 @@ import { PATHS, } from "../../../src/config/constants" +import Bib from "../../../src/models/Bib" + +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 HoldConfirmationItemDetails from "../../../src/components/HoldPages/HoldConfirmationItemDetails" -import { fetchHoldDetails } from "../../../src/server/api/hold" +import { + fetchHoldDetails, + fetchDeliveryLocations, +} from "../../../src/server/api/hold" +import { fetchBib } from "../../../src/server/api/bib" import initializePatronTokenAuth, { doRedirectBasedOnNyplAccountRedirects, getLoginRedirect, } from "../../../src/server/auth" +import type { DiscoveryBibResult } from "../../../src/types/bibTypes" + interface HoldConfirmationPageProps { isEDD?: boolean + pickupLocationLabel?: string + discoveryBibResult: DiscoveryBibResult } /** @@ -29,8 +42,14 @@ interface HoldConfirmationPageProps { */ export default function HoldConfirmationPage({ isEDD = false, + pickupLocationLabel, + discoveryBibResult, }: HoldConfirmationPageProps) { const metadataTitle = `Request Confirmation | ${SITE_NAME}` + + const bib = new Bib(discoveryBibResult) + const item = bib?.items[0] + return ( <> @@ -47,26 +66,48 @@ 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 {isEDD ? "scan " : ""} + request for{" "} + + {item.bibTitle} + } /> + + + Start a new search + ) } -export async function getServerSideProps({ req, res, query }) { - const { pickupLocation, requestId } = query +export async function getServerSideProps({ params, req, res, query }) { + const { id } = params + const { requestId, pickupLocation } = query + // authentication redirect const patronTokenResponse = await initializePatronTokenAuth(req.cookies) const isAuthenticated = patronTokenResponse.isTokenValid @@ -85,7 +126,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,7 +140,8 @@ export async function getServerSideProps({ req, res, query }) { const patronId = patronTokenResponse?.decodedPatron?.sub try { - const { patronId: patronIdFromResponse } = await fetchHoldDetails(requestId) + const { patronId: patronIdFromResponse, status: responseStatus } = + await fetchHoldDetails(requestId) if (patronId !== patronIdFromResponse) { throw new Error( @@ -107,9 +149,43 @@ 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) + + // 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 barcode not found in discoveryBibResult" + ) + } + + const { deliveryLocations } = await fetchDeliveryLocations( + itemBarcode, + patronId + ) + const pickupLocationLabel = deliveryLocations?.find( + (location) => location.value === pickupLocation + )?.label + return { props: { isEDD: pickupLocation === "edd", + pickupLocationLabel: pickupLocationLabel || null, + discoveryBibResult, }, } } catch (error) { diff --git a/pages/hold/request/[id]/edd.tsx b/pages/hold/request/[id]/edd.tsx index 5991e992a..058e3c3b4 100644 --- a/pages/hold/request/[id]/edd.tsx +++ b/pages/hold/request/[id]/edd.tsx @@ -10,7 +10,7 @@ import { import Layout from "../../../../src/components/Layout/Layout" import EDDRequestForm from "../../../../src/components/HoldPages/EDDRequestForm" import HoldRequestBanner from "../../../../src/components/HoldPages/HoldRequestBanner" -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" @@ -153,7 +153,7 @@ export default function EDDRequestPage({ Request scan - + {isLoading || formPosting ? ( ) : pageStatus !== "unavailable" ? ( diff --git a/pages/hold/request/[id]/index.tsx b/pages/hold/request/[id]/index.tsx index 2feb9554b..5bbc6ada8 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 HoldRequestBanner from "../../../../src/components/HoldPages/HoldRequestBanner" -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" @@ -156,7 +156,7 @@ export default function HoldRequestPage({ Request for on-site use - + {isLoading || formPosting ? ( { noStyling type="dl" showRowDividers={false} - className={styles.bibDetails + styles.inBibPage} + className={`${styles.bibDetails} ${styles.inBibPage}`} > {details.map( (detail: BibDetail | LinkedBibDetail | SubjectHeadingDetail) => { diff --git a/src/components/HoldPages/HoldConfirmationFAQ.tsx b/src/components/HoldPages/HoldConfirmationFAQ.tsx index c57adc7f2..5d53583f8 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 /> - + ) } diff --git a/src/components/HoldPages/HoldConfirmationItemDetails.tsx b/src/components/HoldPages/HoldConfirmationItemDetails.tsx new file mode 100644 index 000000000..d8d7c82e4 --- /dev/null +++ b/src/components/HoldPages/HoldConfirmationItemDetails.tsx @@ -0,0 +1,47 @@ +import { List } from "@nypl/design-system-react-components" + +import type Item from "../../../src/models/Item" +import { PlainTextElement } from "../BibPage/BibDetail" + +import bibDetailStyles from "../../../styles/components/BibDetails.module.scss" + +interface HoldConfirmationItemDetailsProps { + item: Item + pickupLocationLabel?: string +} + +/** + * The HoldConfirmationItemDetails renders item details on the hold confirmation page. + */ +const HoldConfirmationItemDetails = ({ + item, + pickupLocationLabel, +}: HoldConfirmationItemDetailsProps) => { + return ( + + {pickupLocationLabel ? ( + + ) : ( + <> + )} + + {item.barcode ? ( + + ) : ( + <> + )} + + ) +} + +export default HoldConfirmationItemDetails diff --git a/src/components/HoldPages/HoldItemDetails.tsx b/src/components/HoldPages/HoldRequestItemDetails.tsx similarity index 71% rename from src/components/HoldPages/HoldItemDetails.tsx rename to src/components/HoldPages/HoldRequestItemDetails.tsx index 265df35ec..eb3e9c599 100644 --- a/src/components/HoldPages/HoldItemDetails.tsx +++ b/src/components/HoldPages/HoldRequestItemDetails.tsx @@ -1,19 +1,19 @@ 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 { +interface HoldRequestItemDetailsProps { item: Item } /** - * The HoldItemDetails renders item details on the hold page. + * The HoldRequestItemDetails renders item details on the hold page. */ -const HoldItemDetails = ({ item }: HoldItemDetailsProps) => { +const HoldRequestItemDetails = ({ item }: HoldRequestItemDetailsProps) => { return ( { ) } -export default HoldItemDetails +export default HoldRequestItemDetails diff --git a/src/config/constants.ts b/src/config/constants.ts index 14a2affb5..bee332fbc 100644 --- a/src/config/constants.ts +++ b/src/config/constants.ts @@ -20,6 +20,7 @@ export const PATHS = { HOME: "/", SEARCH: "/search", ADVANCED_SEARCH: "/search/advanced", + MY_ACCOUNT: "/account", HOLD_REQUEST: "/hold/request", HOLD_CONFIRMATION: "/hold/confirmation", BIB: "/bib", @@ -195,3 +196,16 @@ export const EDD_FORM_FIELD_COPY = { helperText: "Provide additional instructions here.", }, } + +export const HOLD_PAGE_ERROR_HEADINGS = { + failed: "Request failed.", + eddUnavailable: + "Electronic delivery options for this item are currently unavailable.", + patronIneligible: "There is a problem with your library account.", +} + +export const HOLD_PAGE_CONTACT_PREFIXES = { + failed: "We were unable to process your request at this time.", + eddUnavailable: + "Electronic delivery options for this item are currently unavailable.", +} diff --git a/src/server/api/hold.ts b/src/server/api/hold.ts index 4a10d2f38..cc3ade36f 100644 --- a/src/server/api/hold.ts +++ b/src/server/api/hold.ts @@ -148,7 +148,7 @@ export async function fetchHoldDetails( error.message ) - return { status: 400 } + return { status: 500 } } } diff --git a/src/server/api/tests/hold.test.ts b/src/server/api/tests/hold.test.ts index 9e831fdd9..6a310653e 100644 --- a/src/server/api/tests/hold.test.ts +++ b/src/server/api/tests/hold.test.ts @@ -2,9 +2,13 @@ import { fetchDeliveryLocations, postHoldRequest, postEDDRequest, + fetchHoldDetails, } from "../hold" import type { DeliveryLocationsResult } from "../../../types/locationTypes" -import type { HoldPostResult } from "../../../types/holdPageTypes" +import type { + HoldPostResult, + HoldDetailsResult, +} from "../../../types/holdPageTypes" jest.mock("../../nyplApiClient", () => { return jest @@ -79,12 +83,34 @@ 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({ + 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", () => { @@ -173,3 +199,20 @@ describe("postEDDRequest", () => { expect(holdPostResult.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) + }) +})