Skip to content

Commit

Permalink
Merge pull request #462 from NYPL/qa
Browse files Browse the repository at this point in the history
Deploy delivery locations hotfixes to production
  • Loading branch information
dgcohen authored Jan 24, 2025
2 parents 1645cad + ba85650 commit ec42a31
Show file tree
Hide file tree
Showing 6 changed files with 162 additions and 11 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ 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).

## [1.4.1] Hotfix 2025-1-23

- Fix (derived from DFE) for failing delivery location fetch for off-site items with legacy barcode identifier.

## [1.4.0] 2025-01-22

## Release Notes
Expand Down
11 changes: 6 additions & 5 deletions pages/hold/confirmation/[id].tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ 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"
Expand Down Expand Up @@ -163,14 +164,14 @@ 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]

// Get the item barcode's directly from discoveryBibResult to avoid initializing the entire Item model.
const itemBarcode = discoveryBibResult?.items?.[0]?.["idBarcode"]?.[0]
const bib = new Bib(discoveryBibResult)
const item = new Item(discoveryItemResult, bib)
const itemBarcode = item?.barcode

if (!itemBarcode) {
throw new Error(
"Hold Confirmation Page - Item barcode not found in discoveryBibResult"
)
throw new Error("Hold Confirmation Page - Item barcode not found")
}

const { deliveryLocations } = await fetchDeliveryLocations(
Expand Down
19 changes: 18 additions & 1 deletion src/models/Item.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import {
defaultNYPLLocation,
partnerDefaultLocation,
locationEndpointsMap,
getIdentifiers,
identifiersArray,
} from "../utils/itemUtils"
import { appConfig } from "../config/config"
import ItemAvailability from "./ItemAvailability"
Expand Down Expand Up @@ -53,7 +55,7 @@ export default class Item {
this.format = item.formatLiteral?.length
? item.formatLiteral[0]
: bib.materialType
this.barcode = item.idBarcode?.length ? item.idBarcode[0] : null
this.barcode = this.getBarcodeFromItem(item)
this.location = this.getLocationFromItem(item)
this.aeonUrl = item.aeonUrl?.length ? item.aeonUrl[0] : null
this.dueDate = item.dueDate?.length ? item.dueDate[0] : null
Expand Down Expand Up @@ -116,6 +118,21 @@ export default class Item {
return location
}

// This logic was adapted from DFE as part of a hotfix (delivery locations not loading for some off-site partner items)
// TODO: Refactor this as a follow-up if necessary
getBarcodeFromItem(item: DiscoveryItemResult): string {
if (item.idBarcode?.length) {
return item.idBarcode[0]
}

const identifiers = getIdentifiers(item?.identifier, identifiersArray)

if (identifiers?.barcode) {
return identifiers.barcode
}
return null
}

// Determine if item is Non-NYPL ReCAP by existence of "Recap" string in item source attribute
isPartnerReCAP(): boolean {
return this.source?.indexOf("Recap") !== -1
Expand Down
7 changes: 4 additions & 3 deletions src/server/api/hold.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,11 @@ export async function fetchDeliveryLocations(
}

const deliveryLocations =
discoveryLocationsItem?.deliveryLocation?.map(
(locationElement: DiscoveryLocationElement) =>
discoveryLocationsItem?.deliveryLocation
?.map((locationElement: DiscoveryLocationElement) =>
mapLocationElementToDeliveryLocation(locationElement)
) || []
)
.filter((deliveryLocation) => deliveryLocation) || []
const eddRequestable = discoveryLocationsItem?.eddRequestable || false

// Filter out closed locations
Expand Down
28 changes: 26 additions & 2 deletions src/server/api/tests/hold.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,17 @@ jest.mock("../../nyplApiClient", () => {
get: jest.fn().mockReturnValueOnce({
itemListElement: [
{
deliveryLocation: [{}, {}],
deliveryLocation: [
{
"@id": "loc:mal17",
prefLabel: "Schwarzman Building - Scholar Room 217",
},
{
"@id": "loc:mab",
prefLabel:
"Schwarzman Building - Art & Architecture Room 300",
},
],
},
],
}),
Expand All @@ -34,7 +44,17 @@ jest.mock("../../nyplApiClient", () => {
get: jest.fn().mockReturnValueOnce({
itemListElement: [
{
deliveryLocation: [{}, {}],
deliveryLocation: [
{
"@id": "loc:mal17",
prefLabel: "Schwarzman Building - Scholar Room 217",
},
{
"@id": "loc:mab",
prefLabel:
"Schwarzman Building - Art & Architecture Room 300",
},
],
eddRequestable: true,
},
],
Expand Down Expand Up @@ -183,6 +203,10 @@ describe("fetchDeliveryLocations", () => {
// expect(deliveryLocationResults.deliveryLocations.length).toEqual(0)
expect(deliveryLocationResults.status).toEqual(500)
})

it.todo(
"Add tests for filtering out locations that are not listed in NYPL_LOCATIONS constant (staff-only)"
)
})

describe("postHoldRequest", () => {
Expand Down
104 changes: 104 additions & 0 deletions src/utils/itemUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,107 @@ export const locationEndpointsMap: Record<ItemLocationKey, string> = {
export function locationLabelToKey(label: string): ItemLocationKey {
return label.replace(/SASB/, "Schwarzman").split(" ")[0] as ItemLocationKey
}

// Copied from DFE as part of a hotfix (delivery locations not loading for some off-site partner items)
// TODO: Refactor this as a follow-up if necessary
const itemIdentifierTypeMappings = {
barcode: {
type: "bf:Barcode",
legacyUrnPrefix: "urn:barcode:",
},
bnum: {
type: "nypl:Bnumber",
legacyUrnPrefix: "urn:bnum:",
},
callnumber: {
type: "bf:ShelfMark",
legacyUrnPrefix: "urn:callnumber:",
},
isbn: {
type: "bf:Isbn",
legacyUrnPrefix: "urn:isbn:",
},
issn: {
type: "bf:Issn",
legacyUrnPrefix: "urn:issn:",
},
lccn: {
type: "bf:Lccn",
legacyUrnPrefix: "urn:lccn:",
},
oclc: {
type: "nypl:Oclc",
legacyUrnPrefix: "urn:oclc:",
},
}

// Copied from DFE as part of a hotfix (delivery locations not loading for some off-site partner items)
// TODO: Refactor this as a follow-up if necessary
export const identifiersArray = [{ name: "barcode", value: "bf:Barcode" }]

/**
* Given an identifier value, returns same identifier transformed to entity representation.
*
* Copied from DFE as part of a hotfix (delivery locations not loading for some off-site partner items)
* TODO: Refactor this as a follow-up if necessary
*/
export const ensureLegacyIdentifierIsEntity = (identifier) => {
// If API has given us urn: prefixed identifiers..
if (typeof identifier === "string") {
// Convert them to entitys:
return Object.keys(itemIdentifierTypeMappings)
.filter(
(name) =>
identifier.indexOf(
itemIdentifierTypeMappings[name].legacyUrnPrefix
) === 0
)
.map((name) => ({
"@type": itemIdentifierTypeMappings[name].type,
"@value": identifier.replace(
itemIdentifierTypeMappings[name].legacyUrnPrefix,
""
),
}))
.pop()
}

// Otherwise we assume it's already an entity:
return identifier
}

/**
* Given an array of identifier entries (either serialized as entities or
* string literals), returns the identifers (as entities) that match the
* given rdf:type
*
* Copied from DFE as part of a hotfix (delivery locations not loading for some off-site partner items)
* TODO: Refactor this as a follow-up if necessary
*/
const getIdentifierEntitiesByType = (identifiersArray, type) => {
return identifiersArray
.map(ensureLegacyIdentifierIsEntity)
.filter((identifier) => identifier && identifier["@type"] === type)
}

/**
* Gets into the array of the identifiers of an item. And then targets the identifiers we need
* by the prefixes in neededTagsArray. At last, extracts the identifiers and returns them.
*
* Copied from DFE as part of a hotfix (delivery locations not loading for some off-site partner items)
* TODO: Refactor this as a follow-up if necessary
*/
export const getIdentifiers = (identifiersArray, neededTagsArray) => {
if (!Array.isArray(identifiersArray)) return {}
return neededTagsArray.reduce((identifierMap, neededTag) => {
const matches = getIdentifierEntitiesByType(
identifiersArray,
neededTag.value
)
if (matches && matches.length > 0)
return Object.assign(identifierMap, {
[neededTag.name]: matches[0]["@value"],
})
return identifierMap
}, {})
}

0 comments on commit ec42a31

Please sign in to comment.