Separate confirmation related changes

NYPL · Dec 10, 2024 · 22d5f4e · 22d5f4e
1 parent 7439343
commit 22d5f4e
Show file tree

Hide file tree

Showing 13 changed files with 270 additions and 461 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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,15 +35,16 @@ import type { DiscoveryItemResult } from "../../../../src/types/itemTypes"
 
 import type {
   EDDRequestParams,
-  HoldErrorStatus,
+  EDDPageStatus,
 } from "../../../../src/types/holdPageTypes"
 
 interface EDDRequestPropsType {
   discoveryBibResult: DiscoveryBibResult
   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 */}
         <Box tabIndex={-1} ref={bannerContainerRef}>
-          {errorStatus && (
-            <HoldRequestErrorBanner item={item} errorStatus={errorStatus} />
+          {pageStatus && (
+            <HoldRequestBanner
+              item={item}
+              heading={EDDPageStatusMessages[pageStatus].heading}
+              errorMessage={EDDPageStatusMessages[pageStatus].message}
+              pageStatus={pageStatus}
+            />
           )}
         </Box>
         <Heading level="h2" mb="l" size="heading3">
@@ -154,12 +156,12 @@ export default function EDDRequestPage({
         <HoldRequestItemDetails item={item} />
         {isLoading || formPosting ? (
           <SkeletonLoader showImage={false} data-testid="edd-request-loading" />
-        ) : errorStatus !== "eddUnavailable" ? (
+        ) : pageStatus !== "unavailable" ? (
           <EDDRequestForm
             eddFormState={eddFormState}
             setEddFormState={setEddFormState}
             handleSubmit={postEDDRequest}
-            setErrorStatus={setErrorStatus}
+            setPageStatus={setPageStatus}
             holdId={holdId}
           />
         ) : 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) {