Merge pull request #640 from NYPL/main

main -> qa remove contributor from aggs

Author: danamansana

.github/workflows/test-and-deploy.yml

@@ -15,13 +15,33 @@ 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
+    if: github.ref == 'refs/heads/qa'
     steps:
       - name: Checkout repo
         uses: actions/checkout@v3
@@ -31,7 +51,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
@@ -60,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

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' } }
 }
 

package-lock.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/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",

package.json

@@ -0,0 +1,46 @@
+require('dotenv').config('config/qa.env')
+const axios = require('axios')
+const { expectations, ptypes } = require('./delivery-locations-constants')
+
+const checkLocationsForPtype = async (ptype) => {
+  const problems = []
+  const match = []
+  await Promise.all(Object.values(expectations).map(async (expectation) => {
+    const deliveryLocationsFromApi = await getDeliveryLocations(expectation.barcode, ptypes[ptype])
+    let totalMatch = true
+    const registerProblem = (problem) => {
+      problems.push({ barcode: expectation.barcode, deliveryLocationsFromApi, ...problem })
+      totalMatch = false
+    }
+    const checkForValue = (expectedValue, action) => {
+      const includedValueIncluded = deliveryLocationsFromApi.some((label) => label.includes(expectedValue))
+      const match = action === 'include' ? includedValueIncluded : !includedValueIncluded
+      if (!match) {
+        registerProblem({ [`expectedTo${action}`]: expectedValue })
+      }
+    }
+    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`)
+  })
+}
+
+theThing()

test/integration/delivery-locations-by-barcode.test.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
+}

test/integration/delivery-locations-constants.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)
     })
   })