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 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
/>
- >
+
)
}
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 3e5148ac3..31eb3a3e5 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",
@@ -193,3 +194,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)
+ })
+})