refactor physRequestable finished

NYPL · Mar 5, 2025 · cfa9742 · cfa9742
1 parent 6b7b752
commit cfa9742
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 40 deletions.
diff --git a/lib/requestability_resolver.js b/lib/requestability_resolver.js
@@ -10,45 +10,40 @@ class RequestabilityResolver {
         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, physRequestableCriteria } = this.buildPhysRequestable(item)
+          const itemIsInRecap = isInRecap(item)
+          const deliveryInfo = itemIsInRecap
+            ? DeliveryLocationsResolver.getRecapDeliveryInfo(item)
+            : DeliveryLocationsResolver.getOnsiteDeliveryInfo(item)
+
           item.specRequestable = this.buildSpecRequestable(item, parentBibHasFindingAid)
-          item.physRequestable = !!deliveryInfo.deliveryLocation?.length
+          item.physRequestable = this.buildPhysRequestable(item, deliveryInfo)
           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 buildPhysRequestable (item) {
-    let deliveryInfo
+  static buildPhysRequestable (item, deliveryInfo) {
     let physRequestableCriteria
-    const itemIsInRecap = isInRecap(item)
+    let physRequestable
     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.`
+    const itemIsInRecapMissingRecapCustomerCode = isInRecap(item) && !hasRecapCustomerCode
+    // recap items missing codes should default to true for phys and edd
+    // requestable, unless it has a non-requestable holding location
+    if (itemIsInRecapMissingRecapCustomerCode) physRequestableCriteria = 'Missing customer code'
+    if (deliveryInfo.deliveryLocation?.length > 0) physRequestableCriteria = `${(deliveryInfo.deliveryLocation?.length) || 0} delivery locations.`
+    physRequestable = itemIsInRecapMissingRecapCustomerCode || !!deliveryInfo.deliveryLocation?.length
+    // 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'
+      physRequestable = false
     }
-    return { deliveryInfo, physRequestableCriteria }
+    logger.debug(`item ${item.uri}: `, { physRequestable: item.physRequestable, physRequestableCriteria })
+
+    return physRequestable
   }
 
   static buildSpecRequestable (item, parentBibHasFindingAid) {

diff --git a/test/requestability_resolver.test.js b/test/requestability_resolver.test.js
@@ -5,8 +5,6 @@ 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 +17,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'