diff --git a/config/test.env b/config/test.env index f3f39d0b..9ab73b92 100644 --- a/config/test.env +++ b/config/test.env @@ -9,7 +9,7 @@ NYPL_OAUTH_URL=http://oauth.example.com ENCRYPTED_NYPL_OAUTH_ID=encrypted-nypl-oauth-id ENCRYPTED_NYPL_OAUTH_SECRET=encrypted-nypl-oauth-id -NYPL_CORE_VERSION=v2.21 +NYPL_CORE_VERSION=v2.23 LOG_LEVEL=error FEATURES=on-site-edd diff --git a/lib/delivery-locations-resolver.js b/lib/delivery-locations-resolver.js index 18bbbb57..eafc84e5 100644 --- a/lib/delivery-locations-resolver.js +++ b/lib/delivery-locations-resolver.js @@ -12,7 +12,7 @@ class DeliveryLocationsResolver { } static requestableBasedOnHoldingLocation (item) { - const locationCode = this.extractLocationCode(item) + const locationCode = DeliveryLocationsResolver.extractLocationCode(item) if (!DeliveryLocationsResolver.nyplCoreLocation(locationCode)) { logger.warn(`DeliveryLocationsResolver: Unrecognized holdingLocation for ${item.uri}: ${locationCode}`) @@ -166,7 +166,7 @@ class DeliveryLocationsResolver { return { id: `loc:${location.code}`, label: location.label, - sortPosition: this.sortPosition(location) + sortPosition: DeliveryLocationsResolver.sortPosition(location) } }) // Either way, sort deliveryLocation entries by name: @@ -191,14 +191,14 @@ class DeliveryLocationsResolver { } static attachRecapDeliveryInfo (item) { - const info = this.getRecapDeliveryInfo(item) + const info = DeliveryLocationsResolver.getRecapDeliveryInfo(item) item.eddRequestable = info.eddRequestable item.deliveryLocation = info.deliveryLocation return item } static attachOnsiteDeliveryInfo (item) { - const info = this.getOnsiteDeliveryInfo(item) + const info = DeliveryLocationsResolver.getOnsiteDeliveryInfo(item) item.eddRequestable = info.eddRequestable item.deliveryLocation = info.deliveryLocation return item @@ -212,15 +212,15 @@ class DeliveryLocationsResolver { const hasRecapCustomerCode = item.recapCustomerCode && item.recapCustomerCode[0] const nyplItem = isItemNyplOwned(item) if (!hasRecapCustomerCode) { - const requestableBasedOnHoldingLocation = nyplItem ? this.requestableBasedOnHoldingLocation(item) : true + const requestableBasedOnHoldingLocation = nyplItem ? DeliveryLocationsResolver.requestableBasedOnHoldingLocation(item) : true // the length of the list of delivery locations is checked later to determine physical requestability // In case of an offsite item with no recap customer code, we want this to be based on holding location // so we put a placeholder '' in case it is requestable based on holding location deliveryLocation = requestableBasedOnHoldingLocation ? [''] : [] eddRequestable = requestableBasedOnHoldingLocation - } else if (!nyplItem || this.requestableBasedOnHoldingLocation(item)) { - deliveryLocation = this.deliveryLocationsByRecapCustomerCode(item.recapCustomerCode[0]) - eddRequestable = this.__eddRequestableByCustomerCode(item.recapCustomerCode[0]) + } else if (!nyplItem || DeliveryLocationsResolver.requestableBasedOnHoldingLocation(item)) { + deliveryLocation = DeliveryLocationsResolver.deliveryLocationsByRecapCustomerCode(item.recapCustomerCode[0]) + eddRequestable = DeliveryLocationsResolver.__eddRequestableByCustomerCode(item.recapCustomerCode[0]) } else { deliveryLocation = [] eddRequestable = false @@ -233,7 +233,7 @@ class DeliveryLocationsResolver { eddRequestable: false, deliveryLocation: [] } - const holdingLocationCode = this.extractLocationCode(item) + const holdingLocationCode = DeliveryLocationsResolver.extractLocationCode(item) const sierraData = DeliveryLocationsResolver.nyplCoreLocation(holdingLocationCode) if (!sierraData) { // This case is mainly to satisfy a test which wants eddRequestable = false @@ -243,15 +243,15 @@ class DeliveryLocationsResolver { } // if nypl core says it's unrequestable, it can still be eddRequestable, // but its definitely not phys requestable. - deliveryInfo.eddRequestable = this.eddRequestableByOnSiteCriteria(item) - if (!this.requestableBasedOnHoldingLocation(item)) { + deliveryInfo.eddRequestable = DeliveryLocationsResolver.eddRequestableByOnSiteCriteria(item) + if (!DeliveryLocationsResolver.requestableBasedOnHoldingLocation(item)) { return deliveryInfo } // if nypl-core reports that a holding location's delivery locations // should be found by M2 code, but only if the item has an M2 customer code const deliverableToResolution = sierraData.deliverableToResolution if (deliverableToResolution === 'm2-customer-code' && item.m2CustomerCode && item.m2CustomerCode[0]) { - deliveryInfo.deliveryLocation = this.deliveryLocationsByM2CustomerCode(item.m2CustomerCode[0]) + deliveryInfo.deliveryLocation = DeliveryLocationsResolver.deliveryLocationsByM2CustomerCode(item.m2CustomerCode[0]) } // if no value, default to sierra location lookup if (!deliverableToResolution) { @@ -275,15 +275,15 @@ class DeliveryLocationsResolver { } /** - * Given an array of items (ES hits), returns the same items with `eddRequestable` & `deliveryLocations` + * Given an array of items (ES hits), returns the same items with `eddRequestable` & `deliveryLocations`. We verify all recap customer codes because our indexed data may be stale. * * @return Promise> A Promise that resolves and array of items, modified to include `eddRequestable` & `deliveryLocations` */ static attachDeliveryLocationsAndEddRequestability (items, scholarRoom) { // Extract ReCAP barcodes from items: - const recapBarcodes = this.extractRecapBarcodes(items) + const recapBarcodes = DeliveryLocationsResolver.extractRecapBarcodes(items) // Get a map from barcodes to ReCAP customercodes: - return this.__recapCustomerCodesByBarcodes(recapBarcodes) + return DeliveryLocationsResolver.__recapCustomerCodesByBarcodes(recapBarcodes) .then((barcodeToRecapCustomerCode) => { // Now map over items to affix deliveryLocations: return items.map((item) => { @@ -292,15 +292,15 @@ class DeliveryLocationsResolver { item.recapCustomerCode = [barcodeToRecapCustomerCode[barcode]] // If recap has a customer code for this barcode, map it by recap cust code: if (item.recapCustomerCode[0]) { - item = this.attachRecapDeliveryInfo(item) + item = DeliveryLocationsResolver.attachRecapDeliveryInfo(item) // Otherwise, it's an onsite item } else { - item = this.attachOnsiteDeliveryInfo(item) + item = DeliveryLocationsResolver.attachOnsiteDeliveryInfo(item) } // Establish default for Electronic Document Delivery flag: item.eddRequestable = !!item.eddRequestable - const filteredDeliveryLocationsWithScholarRoom = this.filterLocations(item.deliveryLocation, scholarRoom) - item.deliveryLocation = this.formatLocations(filteredDeliveryLocationsWithScholarRoom) + const filteredDeliveryLocationsWithScholarRoom = DeliveryLocationsResolver.filterLocations(item.deliveryLocation, scholarRoom) + item.deliveryLocation = DeliveryLocationsResolver.formatLocations(filteredDeliveryLocationsWithScholarRoom) return item }) }) diff --git a/lib/requestability_resolver.js b/lib/requestability_resolver.js index 0b192d6c..8dc22a1c 100644 --- a/lib/requestability_resolver.js +++ b/lib/requestability_resolver.js @@ -2,46 +2,57 @@ const DeliveryLocationsResolver = require('./delivery-locations-resolver') const { isItemNyplOwned } = require('./ownership_determination') const { isInRecap } = require('./util') const logger = require('./logger') + 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 && item.recapCustomerCode[0] + 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 && - deliveryInfo.deliveryLocation.length) || 0} delivery locations.` + ? `${(deliveryInfo.deliveryLocation?.length) || 0} delivery locations.` : 'Missing customer code' } else if (!itemIsInRecap) { deliveryInfo = DeliveryLocationsResolver.getOnsiteDeliveryInfo(item) - physRequestableCriteria = `${(deliveryInfo.deliveryLocation && - deliveryInfo.deliveryLocation.length) || 0} delivery locations.` + physRequestableCriteria = `${(deliveryInfo.deliveryLocation?.length) || 0} delivery locations.` } - item.eddRequestable = !!deliveryInfo.eddRequestable - item.physRequestable = !!(deliveryInfo.deliveryLocation && - deliveryInfo.deliveryLocation.length) - item.specRequestable = !!item.aeonUrl + item.specRequestable = this.buildSpecRequestable(item, parentBibHasFindingAid) + item.physRequestable = !!deliveryInfo.deliveryLocation?.length + 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] + console.log({ specRequestable: item.specRequestable, physRequestable: item.physRequestable, eddRequestable: item.eddRequestable, physRequestableCriteria }) return item }) }) return elasticSearchResponse } + + static buildSpecRequestable (item, parentBibHasFindingAid) { + const holdingLocation = DeliveryLocationsResolver.extractLocationCode(item) + const nyplCoreLocation = DeliveryLocationsResolver.nyplCoreLocation(holdingLocation) + const isSpecialCollectionsOnlyAccessType = !!(nyplCoreLocation?.collectionAccessType === 'special') + return !!item.aeonUrl || parentBibHasFindingAid || isSpecialCollectionsOnlyAccessType + } } module.exports = RequestabilityResolver diff --git a/test/fixtures/specRequestable/findingAid-es-response.js b/test/fixtures/specRequestable/findingAid-es-response.js new file mode 100644 index 00000000..7de9c342 --- /dev/null +++ b/test/fixtures/specRequestable/findingAid-es-response.js @@ -0,0 +1,152 @@ +module.exports = () => { + return { + _shards: { + failed: 0, + successful: 1, + total: 1 + }, + took: 1, + hits: { + total: 1, + max_score: 1.3862944, + hits: [ + { + _id: 'b10980129', + _source: { + supplementaryContent: [{ + label: 'Finding aid', + url: 'spaghetti.com' + }], + items: [ + { + holdingLocation: [ + { + label: 'OFFSITE - Request in Advance', + id: 'loc:rc2ma' + } + ], + status_packed: [ + 'status:na||Not Available' + ], + owner: [ + { + id: 'orgs:1000', + label: 'Stephen A. Schwarzman Building' + } + ], + deliveryLocation: [ + { + id: 'loc:mala', + label: 'SASB - Allen Scholar Room' + } + ], + deliveryLocation_packed: [ + 'loc:mala||SASB - Allen Scholar Room' + ], + uri: 'i10283664', + accessMessage_packed: [ + 'accessMessage:2||ADV REQUEST' + ], + accessMessage: [ + { + id: 'accessMessage:2', + label: 'ADV REQUEST' + } + ], + status: [ + { + id: 'status:na', + label: 'Not available' + } + ], + owner_packed: [ + 'orgs:1000||Stephen A. Schwarzman Building' + ], + requestable: [ + false + ], + identifier: [ + 'urn:barcode:1000546836' + ], + holdingLocation_packed: [ + 'loc:rc2ma||OFFSITE - Request in Advance' + ], + shelfMark: [ + '*OFC 90-2649' + ], + suppressed: [ + false + ] + }, + { + holdingLocation: [ + { + label: 'OFFSITE - Request in Advance', + id: 'loc:rc2ma' + } + ], + status_packed: [ + 'status:a||Available' + ], + owner: [ + { + id: 'orgs:1000', + label: 'Stephen A. Schwarzman Building' + } + ], + deliveryLocation: [ + { + id: 'loc:mala', + label: 'SASB - Allen Scholar Room' + } + ], + deliveryLocation_packed: [ + 'loc:mala||SASB - Allen Scholar Room' + ], + uri: 'i102836649', + accessMessage_packed: [ + 'accessMessage:2||ADV REQUEST' + ], + accessMessage: [ + { + id: 'accessMessage:2', + label: 'ADV REQUEST' + } + ], + status: [ + { + id: 'status:a', + label: 'Available' + } + ], + owner_packed: [ + 'orgs:1000||Stephen A. Schwarzman Building' + ], + requestable: [ + false + ], + identifier: [ + 'urn:barcode:10005468369' + ], + holdingLocation_packed: [ + 'loc:rc2ma||OFFSITE - Request in Advance' + ], + shelfMark: [ + '*OFC 90-2649 2' + ], + suppressed: [ + false + ] + } + ] + + }, + _type: 'resource', + _index: 'resources-2017-06-13', + _score: 154.93451 + } + ] + }, + timed_out: false + } +} diff --git a/test/fixtures/specRequestable/phys-requestable-override.js b/test/fixtures/specRequestable/phys-requestable-override.js new file mode 100644 index 00000000..6c022618 --- /dev/null +++ b/test/fixtures/specRequestable/phys-requestable-override.js @@ -0,0 +1,297 @@ +module.exports = { + _shards: { + failed: 0, + successful: 1, + total: 1 + }, + took: 1, + hits: { + total: 1, + max_score: 1.3862944, + hits: [ + { + _id: 'b10980129', + _source: { + numItems: [ + 4 + ], + createdString: [ + '1989' + ], + issuance: [ + { + label: 'monograph/item', + id: 'urn:biblevel:m' + } + ], + supplementaryContent: [{ label: 'Finding aid', url: 'spaghetti.com' }], + creatorLiteral: [ + 'Maḥfūẓ, Najīb, 1911-2006.' + ], + creator_sort: [ + 'maḥfūẓ, najīb, 1911-2006.' + ], + level: 'debug', + items: [ + { + holdingLocation_packed: [ + 'loc:scff2||Schomburg Center - Research & Reference' + ], + suppressed: [ + false + ], + shelfMark: [ + 'Sc D 90-863' + ], + accessMessage_packed: [ + 'accessMessage:1||USE IN LIBRARY' + ], + uri: 'i10283665', + accessMessage: [ + { + label: 'USE IN LIBRARY', + id: 'accessMessage:1' + } + ], + catalogItemType: [ + { + id: 'catalogItemType:2', + label: 'book non-circ' + } + ], + deliveryLocation_packed: [ + 'loc:sc||Schomburg Center' + ], + owner: [ + { + label: 'Schomburg Center for Research in Black Culture, Jean Blackwell Hutson Research and Reference Division', + id: 'orgs:1114' + } + ], + deliveryLocation: [ + { + label: 'Schomburg Center', + id: 'loc:sc' + } + ], + identifier: [ + 'urn:barcode:32101071572406' + ], + requestable: [ + true + ], + owner_packed: [ + 'orgs:1114||Schomburg Center for Research in Black Culture, Jean Blackwell Hutson Research and Reference Division' + ], + status: [ + { + label: 'Available', + id: 'status:a' + } + ], + holdingLocation: [ + { + label: 'Schomburg Center - Research & Reference', + id: 'loc:scff2' + } + ], + status_packed: [ + 'status:a||Available' + ] + }, + { + holdingLocation_packed: [ + 'loc:scff2||Schomburg Center - Research & Reference' + ], + suppressed: [ + false + ], + shelfMark: [ + 'Sc D 90-863' + ], + accessMessage_packed: [ + 'accessMessage:1||USE IN LIBRARY' + ], + uri: 'i10283665777', + accessMessage: [ + { + label: 'USE IN LIBRARY', + id: 'accessMessage:1' + } + ], + catalogItemType: [ + { + id: 'catalogItemType:2', + label: 'book non-circ' + } + ], + deliveryLocation_packed: [ + 'loc:sc||Schomburg Center' + ], + owner: [ + { + label: 'Schomburg Center for Research in Black Culture, Jean Blackwell Hutson Research and Reference Division', + id: 'orgs:1114' + } + ], + deliveryLocation: [ + { + label: 'Schomburg Center', + id: 'loc:sc' + } + ], + identifier: [ + 'urn:barcode:32101071572406777' + ], + requestable: [ + true + ], + owner_packed: [ + 'orgs:1114||Schomburg Center for Research in Black Culture, Jean Blackwell Hutson Research and Reference Division' + ], + status: [ + { + label: 'Not Available', + id: 'status:na' + } + ], + holdingLocation: [ + { + label: 'Schomburg Center - Research & Reference', + id: 'loc:scff2' + } + ], + status_packed: [ + 'status:na||Not Available' + ] + } + ], + message: 'ResourceSerializer#serialize', + materialType_packed: [ + 'resourcetypes:txt||Text' + ], + suppressed: [ + 'false' + ], + placeOfPublication: [ + 'New York :' + ], + dateEndString: [ + '1984' + ], + title_sort: [ + 'the thief and the dogs' + ], + uris: [ + 'b11293188', + 'b11293188-i22566485', + 'b11293188-i22566489', + 'b11293188-i10283665', + 'b11293188-i10283664' + ], + language: [ + { + id: 'lang:eng', + label: 'English' + } + ], + dateString: [ + '1989' + ], + identifier: [ + 'urn:bnum:11293188', + 'urn:oclc:12248278', + 'urn:lcc:PJ7846.A46', + 'urn:lccCoarse:PJ7695.8-7976' + ], + publisher: [ + 'Doubleday,' + ], + type: [ + 'nypl:Item' + ], + createdYear: [ + 1989 + ], + contributor_sort: [ + 'badawī, muḥammad muṣṭafá.' + ], + materialType: [ + { + id: 'resourcetypes:txt', + label: 'Text' + } + ], + numAvailable: [ + 2 + ], + dimensions: [ + '22 cm.' + ], + carrierType_packed: [ + 'carriertypes:nc||volume' + ], + note: [ + 'Translation of: al-Liṣṣ wa-al-kilāb.' + ], + dateStartYear: [ + 1989 + ], + shelfMark: [ + '*OFC 90-2649' + ], + idOwi: [ + 'urn:owi:58201773' + ], + mediaType: [ + { + label: 'unmediated', + id: 'mediatypes:n' + } + ], + title: [ + 'The thief and the dogs', + 'The thief and the dogs /' + ], + titleAlt: [ + 'Liṣṣ wa-al-kilāb.' + ], + language_packed: [ + 'lang:eng||English' + ], + mediaType_packed: [ + 'mediatypes:n||unmediated' + ], + titleDisplay: [ + 'The thief and the dogs / Naguib Mahfouz ; translated by Trevor Le Gassick, M.M. Badawi ; revised by John Rodenbeck.' + ], + uri: 'b11293188', + extent: [ + '158 p. ;' + ], + carrierType: [ + { + id: 'carriertypes:nc', + label: 'volume' + } + ], + issuance_packed: [ + 'urn:biblevel:m||monograph/item' + ], + contributorLiteral: [ + 'Badawī, Muḥammad Muṣṭafá.', + 'Le Gassick, Trevor.', + 'Rodenbeck, John.' + ], + dateEndYear: [ + 1984 + ] + }, + _type: 'resource', + _index: 'resources-2017-06-13', + _score: 154.93451 + } + ] + }, + timed_out: false +} diff --git a/test/fixtures/specRequestable-es-response.js b/test/fixtures/specRequestable/specRequestable-es-response.js similarity index 99% rename from test/fixtures/specRequestable-es-response.js rename to test/fixtures/specRequestable/specRequestable-es-response.js index e2133d07..f86ef6dc 100644 --- a/test/fixtures/specRequestable-es-response.js +++ b/test/fixtures/specRequestable/specRequestable-es-response.js @@ -174,7 +174,7 @@ module.exports = () => { holdingLocation: [ { label: 'Schomburg Center - Research & Reference', - id: 'loc:scff2' + id: 'loc:mao82' } ], status_packed: [ diff --git a/test/requestability_resolver.test.js b/test/requestability_resolver.test.js index eb25d997..da8de713 100644 --- a/test/requestability_resolver.test.js +++ b/test/requestability_resolver.test.js @@ -1,18 +1,35 @@ const RequestabilityResolver = require('../lib/requestability_resolver') const elasticSearchResponse = require('./fixtures/elastic_search_response.js') -const specRequestableElasticSearchResponse = require('./fixtures/specRequestable-es-response') +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 () { - const NyplResponse = elasticSearchResponse.fakeElasticSearchResponseNyplItem() + let NyplResponse + before(() => { + NyplResponse = elasticSearchResponse.fakeElasticSearchResponseNyplItem() + }) it('sets physRequestable false for items with no barcodes', () => { const noBarcode = noBarcodeResponse() 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' @@ -172,10 +189,25 @@ describe('RequestabilityResolver', () => { expect(specRequestableItem.specRequestable).to.equal(true) }) - it('marks items as not specRequestable when there is no aeonURL present', function () { + it('marks items as specRequestable when there is a special collectionAccessType designation', function () { const response = RequestabilityResolver.fixItemRequestability(specRequestableElasticSearchResponse()) const items = response.hits.hits[0]._source.items + const specRequestableItem = items.find((item) => item.uri === 'i10283665777') + 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 + const specRequestableItem = items.find((item) => item.uri === 'i10283665') expect(specRequestableItem.specRequestable).to.equal(false) })