Merge pull request #466 from NYPL/SCC-4549/phys-override-spec

charmingduchess · web-flow · commit 7ee942a19205 · 2025-03-13T11:34:46.000-04:00
Scc 4549/phys override spec
diff --git a/lib/requestability_resolver.js b/lib/requestability_resolver.js
@@ -7,50 +7,73 @@ class RequestabilityResolver {
   static fixItemRequestability (elasticSearchResponse) {
     elasticSearchResponse.hits.hits
       .forEach((hit) => {
-        const parentBibHasFindingAid = !!hit._source.supplementaryContent?.find((el) => el.label.toLowerCase() === 'finding aid')
         hit._source.items = hit._source.items.map((item) => {
           if (item.electronicLocator) return item
-          let deliveryInfo
           const itemIsInRecap = isInRecap(item)
-          let physRequestableCriteria
-          const hasRecapCustomerCode = item.recapCustomerCode?.[0]
-          if (itemIsInRecap) {
-            // recap items missing codes should default to true for phys and edd
-            // requestable, unless it has a non-requestable holding location
-            deliveryInfo = DeliveryLocationsResolver.getRecapDeliveryInfo(item)
-            physRequestableCriteria = hasRecapCustomerCode
-              ? `${(deliveryInfo.deliveryLocation?.length) || 0} delivery locations.`
-              : 'Missing customer code'
-          } else if (!itemIsInRecap) {
-            deliveryInfo = DeliveryLocationsResolver.getOnsiteDeliveryInfo(item)
-            physRequestableCriteria = `${(deliveryInfo.deliveryLocation?.length) || 0} delivery locations.`
-          }
-          item.specRequestable = this.buildSpecRequestable(item, parentBibHasFindingAid)
-          item.physRequestable = !!deliveryInfo.deliveryLocation?.length
+          const deliveryInfo = itemIsInRecap
+            ? DeliveryLocationsResolver.getRecapDeliveryInfo(item)
+            : DeliveryLocationsResolver.getOnsiteDeliveryInfo(item)
+          const numDeliveryLocations = deliveryInfo.deliveryLocation?.length
+          item.specRequestable = this.buildSpecRequestable(item)
+          item.physRequestable = this.buildPhysRequestable(item, numDeliveryLocations)
           item.eddRequestable = !!deliveryInfo.eddRequestable && !item.specRequestable
-          // items without barcodes should not be requestable
-          const hasBarcode = (item.identifier || []).some((identifier) => /^(urn|bf):[bB]arcode:\w+/.test(identifier))
-          if (isItemNyplOwned(item) && !hasBarcode) {
-            physRequestableCriteria = 'NYPL item missing barcode'
-            item.physRequestable = false
-          }
-          if (item.specRequestable && item.physRequestable) {
-            item.physRequestable = false
-            physRequestableCriteria = 'specRequestable overrides physRequestable location'
-          }
-          logger.debug(`item ${item.uri}: `, { physRequestable: item.physRequestable, physRequestableCriteria })
           item.requestable = [item.eddRequestable || item.physRequestable || item.specRequestable]
           return item
         })
       })
     return elasticSearchResponse
   }
 
-  static buildSpecRequestable (item, parentBibHasFindingAid) {
+  static buildPhysRequestable (item, numDeliveryLocations) {
+    let physRequestableCriteria
+    let physRequestable
+    const hasRecapCustomerCode = item.recapCustomerCode?.[0]
+    const itemIsInRecapMissingRecapCustomerCode = isInRecap(item) && !hasRecapCustomerCode
+    // items without barcodes should not be requestable
+    const hasBarcode = (item.identifier || []).some((identifier) => /^(urn|bf):[bB]arcode:\w+/.test(identifier))
+    if (isItemNyplOwned(item) && !hasBarcode) {
+      physRequestable = false
+      physRequestableCriteria = 'NYPL item missing barcode'
+      this.logPhysRequestableInfo(physRequestable, physRequestableCriteria, item.uri)
+      return physRequestable
+    }
+    if (isItemNyplOwned(item) && !DeliveryLocationsResolver.requestableBasedOnHoldingLocation(item)) {
+      physRequestableCriteria = 'Unrequestable holding location'
+      physRequestable = false
+      this.logPhysRequestableInfo(physRequestable, physRequestableCriteria, item.uri)
+      return physRequestable
+    }
+    if (itemIsInRecapMissingRecapCustomerCode) {
+      // recap items missing codes should default to true for phys and edd
+      // requestable, if it has a requestable holding location.
+      physRequestable = true
+      physRequestableCriteria = 'Missing customer code'
+      this.logPhysRequestableInfo(physRequestable, physRequestableCriteria, item.uri)
+      return physRequestable
+    }
+    if (numDeliveryLocations === 0) {
+      physRequestableCriteria = 'No delivery locations.'
+      physRequestable = false
+      this.logPhysRequestableInfo(physRequestable, physRequestableCriteria, item.uri)
+      return physRequestable
+    }
+    if (numDeliveryLocations > 0) {
+      physRequestableCriteria = `${numDeliveryLocations} delivery locations.`
+      physRequestable = true
+      this.logPhysRequestableInfo(physRequestable, physRequestableCriteria, item.uri)
+      return physRequestable
+    }
+  }
+
+  static logPhysRequestableInfo (physRequestable, criteria, uri) {
+    logger.debug(`item ${uri}: ${{ physRequestable, criteria }}`)
+  }
+
+  static buildSpecRequestable (item) {
     const holdingLocation = DeliveryLocationsResolver.extractLocationCode(item)
     const nyplCoreLocation = DeliveryLocationsResolver.nyplCoreLocation(holdingLocation)
     const isSpecialCollectionsOnlyAccessType = !!(nyplCoreLocation?.collectionAccessType === 'special')
-    return !!item.aeonUrl || parentBibHasFindingAid || isSpecialCollectionsOnlyAccessType
+    return !!item.aeonUrl || isSpecialCollectionsOnlyAccessType
   }
 }
 
diff --git a/test/fixtures/no_recap_response.js b/test/fixtures/no_recap_response.js
@@ -305,7 +305,7 @@ exports.fakeElasticSearchResponseNyplItem = () => {
                 holdingLocation: [
                   {
                     label: 'OFFSITE - Request in Advance',
-                    id: 'loc:dya0f'
+                    id: 'loc:rcpt8'
                   }
                 ],
                 status_packed: [
diff --git a/test/requestability_resolver.test.js b/test/requestability_resolver.test.js
@@ -2,11 +2,8 @@ const RequestabilityResolver = require('../lib/requestability_resolver')
 const elasticSearchResponse = require('./fixtures/elastic_search_response.js')
 const specRequestableElasticSearchResponse = require('./fixtures/specRequestable/specRequestable-es-response.js')
 const eddElasticSearchResponse = require('./fixtures/edd_elastic_search_response')
-const findingAidElasticSearchResponse = require('./fixtures/specRequestable/findingAid-es-response.js')
 const noBarcodeResponse = require('./fixtures/no_barcode_es_response')
 const noRecapResponse = require('./fixtures/no_recap_response')
-const physRequestableOverride = require('./fixtures/specRequestable/phys-requestable-override.js')
-const DeliveryLocationsResolver = require('../lib/delivery-locations-resolver.js')
 
 describe('RequestabilityResolver', () => {
   describe('fixItemRequestability', function () {
@@ -19,17 +16,6 @@ describe('RequestabilityResolver', () => {
       const resp = RequestabilityResolver.fixItemRequestability(noBarcode)
       expect(resp.hits.hits[0]._source.items.every((item) => item.physRequestable === false)).to.equal(true)
     })
-    it('specRequestable overrides physRequestable, when items have phys requestable holding location', () => {
-      const esResponseItems = physRequestableOverride.hits.hits[0]._source.items
-      const isPhysRequestable = (item) => !!item.deliveryLocation.length
-      const resp = RequestabilityResolver.fixItemRequestability(physRequestableOverride)
-      // verify that items are phys requestable based on location...
-      expect(esResponseItems
-        .map(DeliveryLocationsResolver.getOnsiteDeliveryInfo)
-        .every(isPhysRequestable)).to.equal(true)
-      // ...but overridden by specRequestability
-      expect(resp.hits.hits[0]._source.items.every((item) => !item.physRequestable && item.specRequestable)).to.equal(true)
-    })
 
     it('will set requestable to false for an item not found in ReCAP', function () {
       const indexedButNotAvailableInSCSBURI = 'i22566485'
@@ -197,13 +183,6 @@ describe('RequestabilityResolver', () => {
       expect(specRequestableItem.specRequestable).to.equal(true)
     })
 
-    it('marks items as specRequestable when there is a finding aid on the parent bib', function () {
-      const response = RequestabilityResolver.fixItemRequestability(findingAidElasticSearchResponse())
-
-      const items = response.hits.hits[0]._source.items
-      expect(items.every((item) => item.specRequestable)).to.equal(true)
-    })
-
     it('leaves item as specRequestable false when there is no finding aid, aeon url, or special holding location', () => {
       const response = RequestabilityResolver.fixItemRequestability(elasticSearchResponse.fakeElasticSearchResponseNyplItem())
       const items = response.hits.hits[0]._source.items
@@ -248,16 +227,16 @@ describe('RequestabilityResolver', () => {
 
     it('marks edd and physical requestability correctly', function () {
       const items = resolved.hits.hits[0]._source.items
-      const firstItem = items.find((item) => {
+      const requestableLocationNoRecapCode = items.find((item) => {
         return item.uri === 'i102836649'
       })
-      const secondItem = items.find((item) => {
+      const nonRequestableLocationNoRecapCode = items.find((item) => {
         return item.uri === 'i102836659'
       })
-      expect(firstItem.physRequestable).to.equal(true)
-      expect(firstItem.eddRequestable).to.equal(true)
-      expect(secondItem.physRequestable).to.equal(false)
-      expect(secondItem.eddRequestable).to.equal(false)
+      expect(requestableLocationNoRecapCode.physRequestable).to.equal(true)
+      expect(requestableLocationNoRecapCode.eddRequestable).to.equal(true)
+      expect(nonRequestableLocationNoRecapCode.physRequestable).to.equal(false)
+      expect(nonRequestableLocationNoRecapCode.eddRequestable).to.equal(false)
     })
   })
 })

Original file line number	Diff line number	Diff line change
`@@ -305,7 +305,7 @@ exports.fakeElasticSearchResponseNyplItem = () => {`
`305`	`305`	`holdingLocation: [`
`306`	`306`	`{`
`307`	`307`	`label: 'OFFSITE - Request in Advance',`
`308`		`- id: 'loc:dya0f'`
	`308`	`+ id: 'loc:rcpt8'`
`309`	`309`	`}`
`310`	`310`	`],`
`311`	`311`	`status_packed: [`