Scc 4142/spec coll no findin #430

charmingduchess · 2025-01-02T16:27:01Z

Ticket:

JIRA ticket 4142

This PR does the following:

Add special collections messaging to the item tables
create new item Availability model to handle logic

How has this been tested?

unit tests and visual verification

Accessibility concerns or updates

Checklist:

I updated the CHANGELOG with the appropriate information and JIRA ticket number (if applicable).
I have added relevant accessibility documentation for this pull request.
All new and existing tests passed.

vercel · 2025-01-02T16:27:05Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
research-catalog	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jan 3, 2025 8:48pm

dgcohen · 2025-01-02T19:20:35Z

.github/pull_request_template.md

@@ -1,6 +1,7 @@
 ## Ticket:



i'm down with this change, but did you mean to put this outside the parens?

I didn't push up the final update to this! yes it should be

dgcohen · 2025-01-02T19:28:28Z

src/components/ItemTable/ItemAvailability.tsx

+    case RECAP_AEON:
+    case RECAP_AEON_FINDING_AID:
+      message = (
+        <>


small nit: fragment isn't needed here

dgcohen · 2025-01-02T19:29:12Z

src/components/ItemTable/ItemAvailability.tsx

+    case EDGE_CASE:
+      message = <ContactALibrarian item={itemMetadata} />
+      break
+    case RECAP_AEON:


is this case meant to be blank?

it's not blank, it is combined with the one after it

dgcohen · 2025-01-02T19:29:57Z

src/components/ItemTable/ItemAvailability/AvailableByAppointment.tsx

+
+const AvailableByAppointment = ({ displayPeriod = false }) => {
+  return (
+    <>


small nit: fragment isn't needed here

dgcohen · 2025-01-02T19:39:43Z

src/config/constants.ts

@@ -134,3 +134,22 @@ export const NYPL_LOCATIONS = {
    address: "476 Fifth Avenue (42nd St and Fifth Ave)",
  },
 }
+
+export const availabilityKeys = {


very small nit: I know we haven't settled on naming conventions yet but can you rename to AVAILABILITY_KEYS to match the other constants?

dgcohen · 2025-01-02T19:40:27Z

will re-review once the last test is fixed, but looks good! just added some small nitpicky comments.

EdwinGuzman

Looks good, just a few questions/comments.

EdwinGuzman · 2025-01-03T15:33:49Z

src/components/ItemTable/ItemAvailability.test.tsx

@@ -13,10 +15,153 @@ import {
  itemUnavailable,
 } from "../../../__test__/fixtures/itemFixtures"
 import { searchResultPhysicalItems } from "../../../__test__/fixtures/searchResultPhysicalItems"
+import { notDeepEqual } from "assert"


Not used, remove it.

EdwinGuzman · 2025-01-03T15:36:51Z

src/components/ItemTable/ItemAvailability.test.tsx

+        screen.queryByText("Schwarzman Building - Main Reading Room 315")
+      ).not.toBeInTheDocument()
+    })
+    it("recap aeon", () => {


There's a "NO finding aid" in the next test. This test also doesn't have a finding aid, right? If that's right add the same text here.

EdwinGuzman · 2025-01-03T15:42:09Z

src/components/ItemTable/ItemAvailability.test.tsx

+      })
+      render(<ItemAvailability item={item} />)
+      expect(screen.getByText("Available by appointment")).toBeInTheDocument()
+      expect(screen.getByRole("link")).toHaveTextContent(


This should be the example for this case: https://research-catalog-git-scc-4142-spec-coll-messaging-nypl.vercel.app/research/research-catalog/bib/b17058276 (row 8 of spreadsheet). There should be no link but this test does have a link and it passes. Or am I missing something?

EdwinGuzman · 2025-01-03T15:47:05Z

src/components/ItemTable/ItemAvailability/AvailableByAppointment.tsx

+import { appConfig } from "../../../config/config"
+import ExternalLink from "../../Links/ExternalLink/ExternalLink"
+
+const AvailableByAppointment = ({ displayPeriod = false }) => {


Nice way of solving sentence structure through react components.

EdwinGuzman · 2025-01-03T15:47:57Z

src/components/ItemTable/ItemAvailability/AvailableByAppointment.tsx

+
+const AvailableAt = ({ location }) => {
+  if (!location?.endpoint) return null
+  else return <> {` at ${location.prefLabel}. `}</>


nit: you don't need a return since it will always go to this case if the statement above fails.

EdwinGuzman · 2025-01-03T16:03:21Z

src/components/ItemTable/ItemAvailability/ContactALibrarian.tsx

+const ContactALibrarian = ({
+  item,
+}: {
+  item: {


Not necessary but just showing another way to type this. This is derived from the Item class when it's used as a class. You should be able to do Pick<Item, "id" | "barcode" | "callNumber" | "bibId">.

And also reuse it in NotAvailable if you give this its own name.

thats a good pattern to keep in mind! i kept going back and forth between passing the whole item and just the needed properties. Pick would have been useful.

EdwinGuzman · 2025-01-03T16:05:12Z

src/config/constants.ts

+
+export const availabilityKeys = {
+  // anything not covered by the cases below is EDGE_CASE
+  EDGE_CASE: "edgeCase",


Can you add a test for this?

I don't actually know that this case is possible, so I contrived it in the test as item.availability.key = "edgeCase"

EdwinGuzman · 2025-01-03T16:06:25Z

src/components/ItemTable/ItemAvailability.test.tsx

+  describe("special collections", () => {
+    it("onsite aeon finding aid", () => {
+      const item = new Item(itemPhysicallyRequestable, parentBib)
+      item.availability = new ItemAvailabilityModel({


This gets called automatically in Item when it's initialized. Are you doing it here just for the tests?

yea I put in in here so it would be explicit per test exactly what case was being tested. i find it messy and hard to maintain specific actual fixtures for all these different cases.

I suppose I could have just updated item.availabilty.key explicitly for each one.

EdwinGuzman · 2025-01-03T17:05:38Z

src/components/ItemTable/ItemAvailability.test.tsx

+        screen.queryByText("Schwarzman Building - Main Reading Room 315")
+      ).not.toBeInTheDocument()
+    })
+    it("recap YES aeon NO finding ait", () => {


nit: typo "ait"

charmingduchess added 30 commits December 17, 2024 09:49

add build avail key and use switch statment

fcc1c3a

extract contact a librarian

c9e8075

extract all availability messaging

dba9aa8

move availability componetns to model

879ef5b

use constants for avail keys

9d67c2e

use consttants for keys

287ba38

add finding aid to bib model;

d2415ef

add finding aid to item and availability models

c59596f

start to split aeon statuses

383a6f9

actual refactor

03cb92f

clean up;

d4d0cf6

move message render to component

ca84fb8

start availability model tests

39f8c10

more tests for item availability

bc233b0

create all cases in model

b71027e

start speccoll tests

790e7bb

rm non avail details from itemavailability

13a0c72

more tests

2c591bd

build out rendering cases and tests

3ac3d6f

add provisional special collections designation

4b5730b

fix bib/item instantiation bug

caf8731

add test for bibs and finding aid

17628f4

update to specRequestable and variable casing of finding aid

a923226

fix recap aeon only case

a4cf18d

update request buttons to remove ternary

115ec56

formatting periods

57beae1

fix periods in tests

74b8e76

return recap general collections message from switch

a04fe17

fix types

5fb0ea1

return component instead of string

a4be195

charmingduchess added 2 commits December 23, 2024 11:20

move dash

7d13a04

add url to pr template

070ae86

charmingduchess changed the base branch from main to special-collections January 2, 2025 16:27

vercel bot deployed to Preview January 2, 2025 16:28 View deployment

charmingduchess added 2 commits January 2, 2025 11:28

add url to template

776d431

update changelog

f18ee1a

charmingduchess requested review from dgcohen and EdwinGuzman January 2, 2025 16:31

fix link test

9a94953

dgcohen reviewed Jan 2, 2025

View reviewed changes

EdwinGuzman reviewed Jan 3, 2025

View reviewed changes

vercel bot deployed to Preview January 3, 2025 16:20 View deployment

charmingduchess added 2 commits January 3, 2025 11:26

rm unneccesssary fragments

adb16e9

add edge case test

ed9d4f3

vercel bot deployed to Preview January 3, 2025 16:52 View deployment

update test names

d9c834f

vercel bot deployed to Preview January 3, 2025 16:58 View deployment

charmingduchess requested review from dgcohen and EdwinGuzman January 3, 2025 16:58

dgcohen approved these changes Jan 3, 2025

View reviewed changes

EdwinGuzman approved these changes Jan 3, 2025

View reviewed changes

fix typo in test name

20c16f5

vercel bot deployed to Preview January 3, 2025 20:48 View deployment

charmingduchess merged commit 311be0b into special-collections Jan 3, 2025
3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Scc 4142/spec coll no findin #430

Scc 4142/spec coll no findin #430

charmingduchess commented Jan 2, 2025 •

edited

Loading

vercel bot commented Jan 2, 2025 •

edited

Loading

dgcohen Jan 2, 2025

charmingduchess Jan 3, 2025

dgcohen Jan 2, 2025

dgcohen Jan 2, 2025

charmingduchess Jan 3, 2025

dgcohen Jan 2, 2025

dgcohen Jan 2, 2025

dgcohen commented Jan 2, 2025

EdwinGuzman left a comment

EdwinGuzman Jan 3, 2025

EdwinGuzman Jan 3, 2025

EdwinGuzman Jan 3, 2025

EdwinGuzman Jan 3, 2025

EdwinGuzman Jan 3, 2025

EdwinGuzman Jan 3, 2025

EdwinGuzman Jan 3, 2025

charmingduchess Jan 3, 2025

EdwinGuzman Jan 3, 2025

charmingduchess Jan 3, 2025 •

edited

Loading

EdwinGuzman Jan 3, 2025

charmingduchess Jan 3, 2025

charmingduchess Jan 3, 2025

EdwinGuzman Jan 3, 2025

Scc 4142/spec coll no findin #430

Scc 4142/spec coll no findin #430

Conversation

charmingduchess commented Jan 2, 2025 • edited Loading

Ticket:

This PR does the following:

How has this been tested?

Accessibility concerns or updates

Checklist:

vercel bot commented Jan 2, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dgcohen commented Jan 2, 2025

EdwinGuzman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

charmingduchess Jan 3, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

charmingduchess commented Jan 2, 2025 •

edited

Loading

vercel bot commented Jan 2, 2025 •

edited

Loading

charmingduchess Jan 3, 2025 •

edited

Loading