From 951cea22ba7f96df2c8f8e91e3213c415140f786 Mon Sep 17 00:00:00 2001 From: danamansana Date: Fri, 23 Jan 2026 13:44:09 -0500 Subject: [PATCH 01/10] Remove contributorLiteral and creatorLiteral from aggs response, fix tests --- lib/elasticsearch/config.js | 2 -- test/resources.test.js | 32 ++++++++++++++++---------------- 2 files changed, 16 insertions(+), 18 deletions(-) diff --git a/lib/elasticsearch/config.js b/lib/elasticsearch/config.js index b63c3545..a7ae053d 100644 --- a/lib/elasticsearch/config.js +++ b/lib/elasticsearch/config.js @@ -124,8 +124,6 @@ const AGGREGATIONS_SPEC = { buildingLocation: { terms: { field: 'buildingLocationIds' } }, subjectLiteral: { terms: { field: 'subjectLiteral.raw' } }, language: { terms: { field: 'language_packed' } }, - contributorLiteral: { terms: { field: 'contributorLiteral.raw' } }, - creatorLiteral: { terms: { field: 'creatorLiteral.raw' } }, collection: { terms: { field: 'collectionIds' } } } diff --git a/test/resources.test.js b/test/resources.test.js index 056a2926..427f90d5 100644 --- a/test/resources.test.js +++ b/test/resources.test.js @@ -250,7 +250,7 @@ describe('Resources query', function () { q: 'toast', filters: { subjectLiteral: ['S1'], - contributorLiteral: ['C1', 'C2'] + collection: ['C1', 'C2'] } }) const queries = resourcesPrivMethods.aggregationQueriesForParams(params) @@ -260,27 +260,27 @@ describe('Resources query', function () { expect(Object.keys(queries[0].aggregations)).to.have.lengthOf(numAggregations - 2) expect(queries[0].query.bool.filter).to.be.a('array') // Expect the subjectLiteral filter: - expect(queries[0].query.bool.filter[0].term['subjectLiteral.raw'] === 'S1') - // .. And the contributorLiteral filters: - expect(queries[0]).to.nested.include({ 'query.bool.filter[1].bool.should[0].bool.should[0].term.contributorLiteral\\.keywordLowercased': 'C1' }) - expect(queries[0]).to.nested.include({ 'query.bool.filter[1].bool.should[1].bool.should[0].term.contributorLiteral\\.keywordLowercased': 'C2' }) + expect(queries[0].query.bool.filter[1].term['subjectLiteral.raw'] === 'S1') + // .. And the collection filters: + expect(queries[0]).to.nested.include({ 'query.bool.filter[0].bool.should[0].term.collectionIds': 'C1' }) + expect(queries[0]).to.nested.include({ 'query.bool.filter[0].bool.should[1].term.collectionIds': 'C2' }) - // Expect second aggregation for subjectLiteral: + // Expect second aggregation for collection: expect(Object.keys(queries[1].aggregations)).to.have.lengthOf(1) - expect(queries[1]).to.nested.include({ 'aggregations.subjectLiteral.terms.field': 'subjectLiteral.raw' }) - // Expect this agg to filter on the other active filter, contributorLiteral: - expect(queries[1]).to.nested.include({ 'query.bool.filter[0].bool.should[0].bool.should[0].term.contributorLiteral\\.keywordLowercased': 'C1' }) - expect(queries[1]).to.nested.include({ 'query.bool.filter[0].bool.should[1].bool.should[0].term.contributorLiteral\\.keywordLowercased': 'C2' }) + expect(queries[1]).to.nested.include({ 'aggregations.collection.terms.field': 'collectionIds' }) + // Expect this agg to filter on the other active filter, subjectLiteral: expect(queries[1].query.bool.filter).to.have.lengthOf(1) - expect(queries[1].query.bool.filter[0].bool.should).to.have.lengthOf(2) - // Expect last aggregation for contributorLiteral: + expect(queries[1].query.bool.filter[0].term['subjectLiteral.raw'] === 'S1') + + // Expect last aggregation for subjectLiteral: expect(Object.keys(queries[2].aggregations)).to.have.lengthOf(1) - expect(queries[2]).to.nested.include({ 'aggregations.contributorLiteral.terms.field': 'contributorLiteral.raw' }) - // Expect this agg to filter on the other active filter, subjectLiteral: + expect(queries[2]).to.nested.include({ 'aggregations.subjectLiteral.terms.field': 'subjectLiteral.raw' }) + // Expect this agg to filter on the other active filter, collection: + expect(queries[2]).to.nested.include({ 'query.bool.filter[0].bool.should[0].term.collectionIds': 'C1' }) + expect(queries[2]).to.nested.include({ 'query.bool.filter[0].bool.should[1].term.collectionIds': 'C2' }) expect(queries[2].query.bool.filter).to.have.lengthOf(1) - - expect(queries[2].query.bool.filter[0].term['subjectLiteral.raw'] === 'S1') + expect(queries[2].query.bool.filter[0].bool.should).to.have.lengthOf(2) }) }) From 28125f2103c2b2a80ef3518aabbb869140ac9f67 Mon Sep 17 00:00:00 2001 From: Vera Kahn Date: Wed, 4 Feb 2026 10:19:38 -0500 Subject: [PATCH 02/10] start test and script --- package.json | 1 + 1 file changed, 1 insertion(+) diff --git a/package.json b/package.json index fb3a01e2..e7d0a6d7 100644 --- a/package.json +++ b/package.json @@ -33,6 +33,7 @@ }, "scripts": { "test": "./node_modules/.bin/standard --env mocha && NODE_ENV=test ./node_modules/.bin/mocha test --exit", + "test-integration": "./node_modules/.bin/standard && ./node_modules/.bin/mocha test/integration", "start": "node server.js", "deploy-development": "git checkout development && git pull origin development && eb deploy discovery-api-dev --profile nypl-sandbox", "deploy-qa": "git checkout qa && git pull origin qa && eb deploy discovery-api-qa --profile nypl-digital-dev", From f76142bb66fbfdd999bd7ba45d22583aeda2a308 Mon Sep 17 00:00:00 2001 From: Vera Kahn Date: Wed, 4 Feb 2026 14:34:33 -0500 Subject: [PATCH 03/10] wip: framework works, expectations not correct --- package-lock.json | 1 + package.json | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/package-lock.json b/package-lock.json index 57dba3b4..e142a8e2 100644 --- a/package-lock.json +++ b/package-lock.json @@ -3712,6 +3712,7 @@ "version": "0.11.0", "resolved": "https://registry.npmjs.org/@pkgjs/parseargs/-/parseargs-0.11.0.tgz", "integrity": "sha512-+1VkjdD0QBLPodGrJUeqarH8VAIvQODIbwh9XpP5Syisf7YoQgsJKPNFoqqLQlu+VQ/tVSshMR6loPMn8U+dPg==", + "dev": true, "optional": true, "engines": { "node": ">=14" diff --git a/package.json b/package.json index e7d0a6d7..3383b465 100644 --- a/package.json +++ b/package.json @@ -33,7 +33,7 @@ }, "scripts": { "test": "./node_modules/.bin/standard --env mocha && NODE_ENV=test ./node_modules/.bin/mocha test --exit", - "test-integration": "./node_modules/.bin/standard && ./node_modules/.bin/mocha test/integration", + "test-integration": "./node_modules/.bin/mocha test/integration", "start": "node server.js", "deploy-development": "git checkout development && git pull origin development && eb deploy discovery-api-dev --profile nypl-sandbox", "deploy-qa": "git checkout qa && git pull origin qa && eb deploy discovery-api-qa --profile nypl-digital-dev", From 9fc93c215a8b236b766ab4ad123109023afafab7 Mon Sep 17 00:00:00 2001 From: Vera Kahn Date: Wed, 4 Feb 2026 16:02:23 -0500 Subject: [PATCH 04/10] test file --- .../delivery-locations-by-barcode.test.js | 73 +++++++++++++++++++ 1 file changed, 73 insertions(+) create mode 100644 test/integration/delivery-locations-by-barcode.test.js diff --git a/test/integration/delivery-locations-by-barcode.test.js b/test/integration/delivery-locations-by-barcode.test.js new file mode 100644 index 00000000..ef5ece3d --- /dev/null +++ b/test/integration/delivery-locations-by-barcode.test.js @@ -0,0 +1,73 @@ +require('dotenv').config('config/qa.env') +const axios = require('axios') + +const lpa = 'performing' +const schomburg = 'schomburg' +const sasb = 'schwarzman' +const scholar = 'scholar' +const expectations = { + // princeton: { + // barcode: '32101067443802', + // scholar: { includes: [lpa, schomburg, sasb, scholar] }, + // general: { includes: [lpa, schomburg, sasb], excludes: [scholar] } + // }, + // harvard: { + // barcode: '32044135801371', + // scholar: { includes: [lpa, schomburg, sasb, scholar] }, + // general: { includes: [lpa, schomburg, sasb], excludes: [scholar] } + // } + columbia: { + barcode: 'MR75708230', + scholar: { includes: [lpa, schomburg, sasb, scholar] }, + general: { includes: [lpa, schomburg, sasb], excludes: [scholar] } + } + // nyplOffsite: { + // barcode: '33433073236758', + // scholar: { includes: [lpa, schomburg, sasb, scholar] }, + // general: { includes: [lpa, schomburg, sasb], excludes: [scholar] } + // }, + // lpa: { + // barcode: '33433085319774', + // scholar: { includes: [lpa], excludes: [scholar, schomburg, sasb] }, + // general: { includes: [lpa], excludes: [scholar, schomburg, sasb] } + // }, + // schomburg: { + // barcode: '33433119354979', + // scholar: { includes: [schomburg], excludes: [scholar, sasb, lpa] }, + // general: { includes: [schomburg], excludes: [scholar, sasb, lpa] } + // }, + // // nyplM1: { + // // barcode: null, + // // scholar: { includes: [sasb], excludes: [scholar, lpa, schomburg] }, + // // general: { includes: [sasb], excludes: [scholar, lpa, schomburg] } + // // }, + // nyplM2: { + // barcode: '33333069027734', + // scholar: { includes: [sasb], excludes: [scholar, lpa, schomburg] }, + // general: { includes: [sasb], excludes: [scholar, lpa, schomburg] } + // } +} +const barcodeQueryParams = Object.values(expectations).map(expectation => `barcodes[]=${expectation.barcode}`).join('&') +const scholarId = '5427701' + +const theThing = async (ptype = 'scholar') => { + const { data: { itemListElement: deliveryLocationsPerRecord } } = await axios.get(`http://localhost:8082/api/v0.1/request/deliveryLocationsByBarcode?${barcodeQueryParams}&patronId=${scholarId}`) + const problems = [] + const match = [] + Object.values(expectations).forEach((expectation, i) => { + // per record + const deliveryLocationIdsFromApi = deliveryLocationsPerRecord[i].deliveryLocation.map(loc => loc.prefLabel.toLowerCase()) + for (const expectedIncludedValue of expectation.scholar.includes) { + const includedValueIncluded = deliveryLocationIdsFromApi.some((label) => label.includes(expectedIncludedValue)) + if (!includedValueIncluded) { + problems.push({ barcode: expectation.barcode, deliveryLocationIdsFromApi, expectedToInclude: expectedIncludedValue }) + } else { + match.push({ barcode: expectation.barcode, deliveryLocationIdsFromApi, expectedToInclude: expectation[ptype].includes }) + } + } + }) + console.log(match.length) + console.log(problems) +} + +theThing() From 740467dea1385ed28bba10feeb5d298cd76fb890 Mon Sep 17 00:00:00 2001 From: Vera Kahn Date: Thu, 5 Feb 2026 10:33:45 -0500 Subject: [PATCH 05/10] tests extend to general and excludes --- .../delivery-locations-by-barcode.test.js | 119 +++++++++++------- 1 file changed, 75 insertions(+), 44 deletions(-) diff --git a/test/integration/delivery-locations-by-barcode.test.js b/test/integration/delivery-locations-by-barcode.test.js index ef5ece3d..516dcba3 100644 --- a/test/integration/delivery-locations-by-barcode.test.js +++ b/test/integration/delivery-locations-by-barcode.test.js @@ -6,68 +6,99 @@ const schomburg = 'schomburg' const sasb = 'schwarzman' const scholar = 'scholar' const expectations = { - // princeton: { - // barcode: '32101067443802', - // scholar: { includes: [lpa, schomburg, sasb, scholar] }, - // general: { includes: [lpa, schomburg, sasb], excludes: [scholar] } - // }, - // harvard: { - // barcode: '32044135801371', - // scholar: { includes: [lpa, schomburg, sasb, scholar] }, - // general: { includes: [lpa, schomburg, sasb], excludes: [scholar] } - // } + princeton: { + barcode: '32101067443802', + scholar: { includes: [lpa, schomburg, sasb, scholar], excludes: [] }, + general: { includes: [lpa, schomburg, sasb], excludes: [scholar] } + }, + harvard: { + barcode: '32044135801371', + scholar: { includes: [lpa, schomburg, sasb, scholar], excludes: [] }, + general: { includes: [lpa, schomburg, sasb], excludes: [scholar] } + }, + // // recap customer code MR only to LPA columbia: { barcode: 'MR75708230', - scholar: { includes: [lpa, schomburg, sasb, scholar] }, + scholar: { includes: [lpa], excludes: [scholar] }, + general: { includes: [lpa], excludes: [scholar] } + }, + nyplOffsite: { + barcode: '33433073236758', + scholar: { includes: [lpa, schomburg, sasb, scholar], excludes: [] }, general: { includes: [lpa, schomburg, sasb], excludes: [scholar] } - } - // nyplOffsite: { - // barcode: '33433073236758', - // scholar: { includes: [lpa, schomburg, sasb, scholar] }, - // general: { includes: [lpa, schomburg, sasb], excludes: [scholar] } - // }, - // lpa: { - // barcode: '33433085319774', - // scholar: { includes: [lpa], excludes: [scholar, schomburg, sasb] }, - // general: { includes: [lpa], excludes: [scholar, schomburg, sasb] } - // }, - // schomburg: { - // barcode: '33433119354979', - // scholar: { includes: [schomburg], excludes: [scholar, sasb, lpa] }, - // general: { includes: [schomburg], excludes: [scholar, sasb, lpa] } - // }, - // // nyplM1: { - // // barcode: null, - // // scholar: { includes: [sasb], excludes: [scholar, lpa, schomburg] }, - // // general: { includes: [sasb], excludes: [scholar, lpa, schomburg] } - // // }, - // nyplM2: { - // barcode: '33333069027734', + }, + lpa: { + barcode: '33433085319774', + scholar: { includes: [lpa], excludes: [scholar, schomburg, sasb] }, + general: { includes: [lpa], excludes: [scholar, schomburg, sasb] } + }, + schomburg: { + barcode: '33433119354979', + scholar: { includes: [schomburg], excludes: [scholar, sasb, lpa] }, + general: { includes: [schomburg], excludes: [scholar, sasb, lpa] } + }, + // nyplM1: { + // barcode: null, // scholar: { includes: [sasb], excludes: [scholar, lpa, schomburg] }, // general: { includes: [sasb], excludes: [scholar, lpa, schomburg] } - // } + // }, + nyplM2: { + barcode: '33333069027734', + scholar: { includes: [sasb], excludes: [scholar, lpa, schomburg] }, + general: { includes: [sasb], excludes: [scholar, lpa, schomburg] } + } } const barcodeQueryParams = Object.values(expectations).map(expectation => `barcodes[]=${expectation.barcode}`).join('&') -const scholarId = '5427701' -const theThing = async (ptype = 'scholar') => { - const { data: { itemListElement: deliveryLocationsPerRecord } } = await axios.get(`http://localhost:8082/api/v0.1/request/deliveryLocationsByBarcode?${barcodeQueryParams}&patronId=${scholarId}`) +const patronIds = { + scholar: '5427701', + general: '67427431' +} + +const checkLocationsForPtype = async (ptype = 'scholar') => { + const { data: { itemListElement: deliveryLocationsPerRecord } } = await axios.get(`http://localhost:8082/api/v0.1/request/deliveryLocationsByBarcode?${barcodeQueryParams}&patronId=${patronIds[ptype]}`) const problems = [] const match = [] Object.values(expectations).forEach((expectation, i) => { // per record - const deliveryLocationIdsFromApi = deliveryLocationsPerRecord[i].deliveryLocation.map(loc => loc.prefLabel.toLowerCase()) - for (const expectedIncludedValue of expectation.scholar.includes) { + const deliveryLocationIdsFromApi = deliveryLocationsPerRecord + // find delivery location data by barcode match - api does not return in consistent order + .find((deliveryData) => { + return deliveryData.identifier.some((id) => { + return id.includes(expectation.barcode) + }) + }) + .deliveryLocation.map(loc => loc.prefLabel.toLowerCase()) + let totalMatch + const matchObject = { barcode: expectation.barcode, deliveryLocationIdsFromApi, expectedToInclude: expectation[ptype].includes, expectedToExclude: expectation[ptype].excludes } + for (const expectedIncludedValue of expectation[ptype].includes) { const includedValueIncluded = deliveryLocationIdsFromApi.some((label) => label.includes(expectedIncludedValue)) if (!includedValueIncluded) { + totalMatch = false problems.push({ barcode: expectation.barcode, deliveryLocationIdsFromApi, expectedToInclude: expectedIncludedValue }) - } else { - match.push({ barcode: expectation.barcode, deliveryLocationIdsFromApi, expectedToInclude: expectation[ptype].includes }) } } + for (const expectedExcludedValue of expectation[ptype].excludes) { + const excludedValueExcluded = !deliveryLocationIdsFromApi.some((label) => label.includes(expectedExcludedValue)) + if (!excludedValueExcluded) { + // throw Error(`${expectedExcludedValue} ${deliveryLocationIdsFromApi}`) + totalMatch = false + problems.push({ barcode: expectation.barcode, deliveryLocationIdsFromApi, expectedToExclude: expectedExcludedValue }) + } + } + if (totalMatch) match.push(matchObject) }) - console.log(match.length) - console.log(problems) + return { match, problems } +} + +const theThing = async () => { + const [generalResults, scholarResults] = await Promise.all(['general', 'scholar'].map((checkLocationsForPtype))) + if (generalResults.problems.length) { + console.error('Error with general ptype delivery results, ', generalResults.problems) + } else console.log('Delivery locations successfully returned for general ptype', generalResults.match) + if (scholarResults.problems.length) { + console.error('Error with general ptype delivery results, ', scholarResults.problems) + } else console.log('Delivery locations successfully returned for scholar ptype ', scholarResults.match) } theThing() From 7f9c5b2ed7bf7204ea578bdcdb113933f62cbe89 Mon Sep 17 00:00:00 2001 From: Vera Kahn Date: Thu, 5 Feb 2026 10:47:49 -0500 Subject: [PATCH 06/10] it does the thing --- .../delivery-locations-by-barcode.test.js | 25 ++++++++++--------- 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/test/integration/delivery-locations-by-barcode.test.js b/test/integration/delivery-locations-by-barcode.test.js index 516dcba3..c036511b 100644 --- a/test/integration/delivery-locations-by-barcode.test.js +++ b/test/integration/delivery-locations-by-barcode.test.js @@ -44,19 +44,20 @@ const expectations = { // }, nyplM2: { barcode: '33333069027734', - scholar: { includes: [sasb], excludes: [scholar, lpa, schomburg] }, + scholar: { includes: [sasb, scholar], excludes: [lpa, schomburg] }, general: { includes: [sasb], excludes: [scholar, lpa, schomburg] } } } -const barcodeQueryParams = Object.values(expectations).map(expectation => `barcodes[]=${expectation.barcode}`).join('&') -const patronIds = { +const ptypes = { scholar: '5427701', general: '67427431' } +const barcodeQueryParams = Object.values(expectations).map(expectation => `barcodes[]=${expectation.barcode}`).join('&') + const checkLocationsForPtype = async (ptype = 'scholar') => { - const { data: { itemListElement: deliveryLocationsPerRecord } } = await axios.get(`http://localhost:8082/api/v0.1/request/deliveryLocationsByBarcode?${barcodeQueryParams}&patronId=${patronIds[ptype]}`) + const { data: { itemListElement: deliveryLocationsPerRecord } } = await axios.get(`http://localhost:8082/api/v0.1/request/deliveryLocationsByBarcode?${barcodeQueryParams}&patronId=${ptypes[ptype]}`) const problems = [] const match = [] Object.values(expectations).forEach((expectation, i) => { @@ -69,7 +70,7 @@ const checkLocationsForPtype = async (ptype = 'scholar') => { }) }) .deliveryLocation.map(loc => loc.prefLabel.toLowerCase()) - let totalMatch + let totalMatch = true const matchObject = { barcode: expectation.barcode, deliveryLocationIdsFromApi, expectedToInclude: expectation[ptype].includes, expectedToExclude: expectation[ptype].excludes } for (const expectedIncludedValue of expectation[ptype].includes) { const includedValueIncluded = deliveryLocationIdsFromApi.some((label) => label.includes(expectedIncludedValue)) @@ -92,13 +93,13 @@ const checkLocationsForPtype = async (ptype = 'scholar') => { } const theThing = async () => { - const [generalResults, scholarResults] = await Promise.all(['general', 'scholar'].map((checkLocationsForPtype))) - if (generalResults.problems.length) { - console.error('Error with general ptype delivery results, ', generalResults.problems) - } else console.log('Delivery locations successfully returned for general ptype', generalResults.match) - if (scholarResults.problems.length) { - console.error('Error with general ptype delivery results, ', scholarResults.problems) - } else console.log('Delivery locations successfully returned for scholar ptype ', scholarResults.match) + const results = await Promise.all(Object.keys(ptypes).map((checkLocationsForPtype))) + Object.keys(ptypes).forEach((ptype, i) => { + const resultsForPtype = results[i] + if (resultsForPtype.problems.length) { + console.error(`Error with ${ptype} ptype delivery results, `, resultsForPtype.problems) + } else console.log(`Delivery locations successfully returned for ${ptype} ptype`, resultsForPtype.match) + }) } theThing() From 9e36b07a6df89552ded492c29833935444df9836 Mon Sep 17 00:00:00 2001 From: Vera Kahn Date: Fri, 6 Feb 2026 10:27:24 -0500 Subject: [PATCH 07/10] move constants out --- .../delivery-locations-by-barcode.test.js | 60 ++----------------- .../delivery-locations-constants.js | 56 +++++++++++++++++ 2 files changed, 60 insertions(+), 56 deletions(-) create mode 100644 test/integration/delivery-locations-constants.js diff --git a/test/integration/delivery-locations-by-barcode.test.js b/test/integration/delivery-locations-by-barcode.test.js index c036511b..d631d209 100644 --- a/test/integration/delivery-locations-by-barcode.test.js +++ b/test/integration/delivery-locations-by-barcode.test.js @@ -1,58 +1,7 @@ require('dotenv').config('config/qa.env') const axios = require('axios') - -const lpa = 'performing' -const schomburg = 'schomburg' -const sasb = 'schwarzman' -const scholar = 'scholar' -const expectations = { - princeton: { - barcode: '32101067443802', - scholar: { includes: [lpa, schomburg, sasb, scholar], excludes: [] }, - general: { includes: [lpa, schomburg, sasb], excludes: [scholar] } - }, - harvard: { - barcode: '32044135801371', - scholar: { includes: [lpa, schomburg, sasb, scholar], excludes: [] }, - general: { includes: [lpa, schomburg, sasb], excludes: [scholar] } - }, - // // recap customer code MR only to LPA - columbia: { - barcode: 'MR75708230', - scholar: { includes: [lpa], excludes: [scholar] }, - general: { includes: [lpa], excludes: [scholar] } - }, - nyplOffsite: { - barcode: '33433073236758', - scholar: { includes: [lpa, schomburg, sasb, scholar], excludes: [] }, - general: { includes: [lpa, schomburg, sasb], excludes: [scholar] } - }, - lpa: { - barcode: '33433085319774', - scholar: { includes: [lpa], excludes: [scholar, schomburg, sasb] }, - general: { includes: [lpa], excludes: [scholar, schomburg, sasb] } - }, - schomburg: { - barcode: '33433119354979', - scholar: { includes: [schomburg], excludes: [scholar, sasb, lpa] }, - general: { includes: [schomburg], excludes: [scholar, sasb, lpa] } - }, - // nyplM1: { - // barcode: null, - // scholar: { includes: [sasb], excludes: [scholar, lpa, schomburg] }, - // general: { includes: [sasb], excludes: [scholar, lpa, schomburg] } - // }, - nyplM2: { - barcode: '33333069027734', - scholar: { includes: [sasb, scholar], excludes: [lpa, schomburg] }, - general: { includes: [sasb], excludes: [scholar, lpa, schomburg] } - } -} - -const ptypes = { - scholar: '5427701', - general: '67427431' -} +const { expectations, ptypes } = require('./delivery-locations-constants') +const assert = require('assert') const barcodeQueryParams = Object.values(expectations).map(expectation => `barcodes[]=${expectation.barcode}`).join('&') @@ -74,7 +23,7 @@ const checkLocationsForPtype = async (ptype = 'scholar') => { const matchObject = { barcode: expectation.barcode, deliveryLocationIdsFromApi, expectedToInclude: expectation[ptype].includes, expectedToExclude: expectation[ptype].excludes } for (const expectedIncludedValue of expectation[ptype].includes) { const includedValueIncluded = deliveryLocationIdsFromApi.some((label) => label.includes(expectedIncludedValue)) - if (!includedValueIncluded) { + if (!includedValueIncluded || i === 2) { totalMatch = false problems.push({ barcode: expectation.barcode, deliveryLocationIdsFromApi, expectedToInclude: expectedIncludedValue }) } @@ -82,7 +31,6 @@ const checkLocationsForPtype = async (ptype = 'scholar') => { for (const expectedExcludedValue of expectation[ptype].excludes) { const excludedValueExcluded = !deliveryLocationIdsFromApi.some((label) => label.includes(expectedExcludedValue)) if (!excludedValueExcluded) { - // throw Error(`${expectedExcludedValue} ${deliveryLocationIdsFromApi}`) totalMatch = false problems.push({ barcode: expectation.barcode, deliveryLocationIdsFromApi, expectedToExclude: expectedExcludedValue }) } @@ -98,7 +46,7 @@ const theThing = async () => { const resultsForPtype = results[i] if (resultsForPtype.problems.length) { console.error(`Error with ${ptype} ptype delivery results, `, resultsForPtype.problems) - } else console.log(`Delivery locations successfully returned for ${ptype} ptype`, resultsForPtype.match) + } }) } diff --git a/test/integration/delivery-locations-constants.js b/test/integration/delivery-locations-constants.js new file mode 100644 index 00000000..b46d812c --- /dev/null +++ b/test/integration/delivery-locations-constants.js @@ -0,0 +1,56 @@ +const lpa = 'performing' +const schomburg = 'schomburg' +const sasb = 'schwarzman' +const scholar = 'scholar' +const ptypes = { + scholar: '5427701', + general: '67427431' +} +const expectations = { + princeton: { + barcode: '32101067443802', + scholar: { includes: [lpa, schomburg, sasb, scholar], excludes: [] }, + general: { includes: [lpa, schomburg, sasb], excludes: [scholar] } + }, + harvard: { + barcode: '32044135801371', + scholar: { includes: [lpa, schomburg, sasb, scholar], excludes: [] }, + general: { includes: [lpa, schomburg, sasb], excludes: [scholar] } + }, + // // recap customer code MR only to LPA + columbia: { + barcode: 'MR75708230', + scholar: { includes: [lpa], excludes: [scholar] }, + general: { includes: [lpa], excludes: [scholar] } + }, + nyplOffsite: { + barcode: '33433073236758', + scholar: { includes: [lpa, schomburg, sasb, scholar], excludes: [] }, + general: { includes: [lpa, schomburg, sasb], excludes: [scholar] } + }, + lpa: { + barcode: '33433085319774', + scholar: { includes: [lpa], excludes: [scholar, schomburg, sasb] }, + general: { includes: [lpa], excludes: [scholar, schomburg, sasb] } + }, + schomburg: { + barcode: '33433119354979', + scholar: { includes: [schomburg], excludes: [scholar, sasb, lpa] }, + general: { includes: [schomburg], excludes: [scholar, sasb, lpa] } + }, + // nyplM1: { + // barcode: null, + // scholar: { includes: [sasb], excludes: [scholar, lpa, schomburg] }, + // general: { includes: [sasb], excludes: [scholar, lpa, schomburg] } + // }, + nyplM2: { + barcode: '33333069027734', + scholar: { includes: [sasb, scholar], excludes: [lpa, schomburg] }, + general: { includes: [sasb], excludes: [scholar, lpa, schomburg] } + } +} + +module.exports = { + expectations, + ptypes +} From 8de831115c78599663783d48676cc44e698db2eb Mon Sep 17 00:00:00 2001 From: Vera Kahn Date: Fri, 6 Feb 2026 15:28:32 -0500 Subject: [PATCH 08/10] little refactor --- .../delivery-locations-by-barcode.test.js | 53 ++++++++----------- 1 file changed, 23 insertions(+), 30 deletions(-) diff --git a/test/integration/delivery-locations-by-barcode.test.js b/test/integration/delivery-locations-by-barcode.test.js index d631d209..2302b12b 100644 --- a/test/integration/delivery-locations-by-barcode.test.js +++ b/test/integration/delivery-locations-by-barcode.test.js @@ -1,52 +1,45 @@ require('dotenv').config('config/qa.env') const axios = require('axios') const { expectations, ptypes } = require('./delivery-locations-constants') -const assert = require('assert') -const barcodeQueryParams = Object.values(expectations).map(expectation => `barcodes[]=${expectation.barcode}`).join('&') - -const checkLocationsForPtype = async (ptype = 'scholar') => { - const { data: { itemListElement: deliveryLocationsPerRecord } } = await axios.get(`http://localhost:8082/api/v0.1/request/deliveryLocationsByBarcode?${barcodeQueryParams}&patronId=${ptypes[ptype]}`) +const checkLocationsForPtype = async (ptype) => { const problems = [] const match = [] - Object.values(expectations).forEach((expectation, i) => { - // per record - const deliveryLocationIdsFromApi = deliveryLocationsPerRecord - // find delivery location data by barcode match - api does not return in consistent order - .find((deliveryData) => { - return deliveryData.identifier.some((id) => { - return id.includes(expectation.barcode) - }) - }) - .deliveryLocation.map(loc => loc.prefLabel.toLowerCase()) + await Promise.all(Object.values(expectations).map(async (expectation) => { + const deliveryLocationsFromApi = await getDeliveryLocations(expectation.barcode, ptypes[ptype]) let totalMatch = true - const matchObject = { barcode: expectation.barcode, deliveryLocationIdsFromApi, expectedToInclude: expectation[ptype].includes, expectedToExclude: expectation[ptype].excludes } - for (const expectedIncludedValue of expectation[ptype].includes) { - const includedValueIncluded = deliveryLocationIdsFromApi.some((label) => label.includes(expectedIncludedValue)) - if (!includedValueIncluded || i === 2) { - totalMatch = false - problems.push({ barcode: expectation.barcode, deliveryLocationIdsFromApi, expectedToInclude: expectedIncludedValue }) - } + const registerProblem = (problem) => { + problems.push({ barcode: expectation.barcode, deliveryLocationsFromApi, ...problem }) + totalMatch = false } - for (const expectedExcludedValue of expectation[ptype].excludes) { - const excludedValueExcluded = !deliveryLocationIdsFromApi.some((label) => label.includes(expectedExcludedValue)) - if (!excludedValueExcluded) { - totalMatch = false - problems.push({ barcode: expectation.barcode, deliveryLocationIdsFromApi, expectedToExclude: expectedExcludedValue }) + const checkForValue = (expectedValue, action) => { + const includedValueIncluded = deliveryLocationsFromApi.some((label) => label.includes(expectedValue)) + const match = action === 'include' ? includedValueIncluded : !includedValueIncluded + if (!match) { + registerProblem({ [`expectedTo${action}`]: expectedValue }) } } - if (totalMatch) match.push(matchObject) - }) + expectation[ptype].includes.forEach((expectedValue) => checkForValue(expectedValue, 'include')) + expectation[ptype].excludes.forEach((expectedValue) => checkForValue(expectedValue, 'exclude')) + if (totalMatch) match.push({ barcode: expectation.barcode, deliveryLocationsFromApi, expectedToInclude: expectation[ptype].includes, expectedToExclude: expectation[ptype].excludes }) + })) return { match, problems } } +const getDeliveryLocations = async (barcode, patronId) => { + const { data: { itemListElement: deliveryLocationsPerRecord } } = await axios.get(`http://localhost:8082/api/v0.1/request/deliveryLocationsByBarcode?barcodes[]=${barcode}&patronId=${patronId}`) + // per record + return deliveryLocationsPerRecord[0] + .deliveryLocation.map(loc => loc.prefLabel.toLowerCase()) +} + const theThing = async () => { const results = await Promise.all(Object.keys(ptypes).map((checkLocationsForPtype))) Object.keys(ptypes).forEach((ptype, i) => { const resultsForPtype = results[i] if (resultsForPtype.problems.length) { console.error(`Error with ${ptype} ptype delivery results, `, resultsForPtype.problems) - } + } else console.log(`All delivery location checks for ${ptype} patron type successful`) }) } From 439adf01044325856bfb55be134f07b9e79cae79 Mon Sep 17 00:00:00 2001 From: Vera Kahn Date: Fri, 6 Feb 2026 15:57:12 -0500 Subject: [PATCH 09/10] update gha script --- .github/workflows/test-and-deploy.yml | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test-and-deploy.yml b/.github/workflows/test-and-deploy.yml index 2e77b1da..4447d27b 100644 --- a/.github/workflows/test-and-deploy.yml +++ b/.github/workflows/test-and-deploy.yml @@ -15,13 +15,34 @@ jobs: run: npm ci - name: Unit Tests run: npm test - deploy-qa: + integration-test-qa: permissions: id-token: write contents: read runs-on: ubuntu-latest needs: tests if: github.ref == 'refs/heads/qa' + steps: + - uses: actions/checkout@v4 + - name: Set Node version + uses: actions/setup-node@v4 + with: + node-version-file: '.nvmrc' + - name: Install dependencies + run: npm ci + - name: Start service + run: ENV=qa npm start & + - name: Run tests + run: npm run test-integration + deploy-qa: + permissions: + id-token: write + contents: read + runs-on: ubuntu-latest + needs: + - tests + - integration-test-qa + if: github.ref == 'refs/heads/qa' steps: - name: Checkout repo uses: actions/checkout@v3 @@ -31,7 +52,6 @@ jobs: with: role-to-assume: arn:aws:iam::946183545209:role/GithubActionsDeployerRole aws-region: us-east-1 - - name: Log in to ECR id: login-ecr uses: aws-actions/amazon-ecr-login@v1 From 63ce4627a9c6ff92e6ee292ea2ead61c2223f200 Mon Sep 17 00:00:00 2001 From: Vera Kahn Date: Fri, 6 Feb 2026 16:39:02 -0500 Subject: [PATCH 10/10] integration on qa2 --- .github/workflows/test-and-deploy.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test-and-deploy.yml b/.github/workflows/test-and-deploy.yml index 4447d27b..6423821a 100644 --- a/.github/workflows/test-and-deploy.yml +++ b/.github/workflows/test-and-deploy.yml @@ -41,7 +41,6 @@ jobs: runs-on: ubuntu-latest needs: - tests - - integration-test-qa if: github.ref == 'refs/heads/qa' steps: - name: Checkout repo @@ -80,7 +79,9 @@ jobs: id-token: write contents: read runs-on: ubuntu-latest - needs: tests + needs: + - tests + - inte if: github.ref == 'refs/heads/qa2' steps: - name: Checkout repo