Merge pull request #417 from NYPL/NOREF-item-status-aggregation-bug

Fix bug incorporating offsites in item status agg
NYPL · Nov 8, 2024 · ef2d69f · ef2d69f
2 parents e184e5d + 8aa46cc
commit ef2d69f
Show file tree

Hide file tree

Showing 4 changed files with 69 additions and 37 deletions.
diff --git a/lib/elasticsearch/client.js b/lib/elasticsearch/client.js
@@ -115,7 +115,7 @@ clientWrapper.nonRecapItemStatusAggregation = (bnum) => {
 
   return clientWrapper.search(esQuery)
     .then((resp) => {
-      return deepValue(resp, 'body.aggregations.statuses.nonrecap_statuses.nonrecap_status_buckets.buckets')
+      return deepValue(resp, 'aggregations.statuses.nonrecap_statuses.nonrecap_status_buckets.buckets')
     })
 }
 

diff --git a/test/availability_resolver.test.js b/test/availability_resolver.test.js
@@ -323,21 +323,27 @@ describe('Response with updated availability', function () {
       esClient.search.restore()
     })
 
-    it('preserves Available statuses for items that are also available in ReCAP', () => {
+    it('preserves Available statuses for items that are also available in ReCAP', async () => {
+      // Start with an availability resolver wrapping a ES response for a bib
+      // with 4 items, where the item-agg shows 2 are RC and all 4 are indexed
+      // as Available..
       const availabilityResolver = new AvailabilityResolver(
         require('./fixtures/es-response-b1234-recap-statuses.json')
       )
 
+      // Emulate SCSB reporting that there are two Available items for this bib:
       const recapBarcodesByStatus = {
         Available: ['barcode1', 'barcode2']
       }
-      return availabilityResolver.responseWithUpdatedAvailability({ recapBarcodesByStatus })
-        .then((modifiedResponse) => {
-          const buckets = modifiedResponse.aggregations.item_status._nested.buckets
-          expect(buckets).to.deep.equal([
-            { key: 'status:a||Available', doc_count: 4 }
-          ])
-        })
+      await availabilityResolver._fixItemStatusAggregation({ recapBarcodesByStatus })
+      const modifiedResponse = availabilityResolver.elasticSearchResponse
+      const buckets = modifiedResponse.aggregations.item_status._nested.buckets
+      // Because there are 2 offsite items, and our mocked scsb status response
+      // shows two Available items, we expect the final status agg to show 4
+      // Available
+      expect(buckets).to.deep.equal([
+        { key: 'status:a||Available', doc_count: 4 }
+      ])
     })
 
     it('incorporates Not Available statuses for items that are not available in ReCAP', () => {

diff --git a/test/elasticsearch-client.test.js b/test/elasticsearch-client.test.js
@@ -0,0 +1,28 @@
+const { expect } = require('chai')
+const sinon = require('sinon')
+
+const esClient = require('../lib/elasticsearch/client')
+
+describe('Elasticsearch Client', () => {
+  describe('nonRecapItemStatusAggregation', () => {
+    beforeEach(() => {
+      // We expect these tests to trigger an ES query to retrieve aggregated
+      // non-reap-statuses for the bib:
+      sinon.stub(esClient, 'search')
+        .callsFake(() => {
+          return Promise.resolve(require('./fixtures/es-response-b1234-just-non-recap-statuses.json'))
+        })
+    })
+
+    afterEach(() => esClient.search.restore())
+
+    it('retrieves item status aggregation', async () => {
+      const resp = await esClient.nonRecapItemStatusAggregation('b1234')
+
+      expect(resp).to.be.a('array')
+      expect(resp).to.deep.equal([
+        { key: 'status:a||Available', doc_count: 2 }
+      ])
+    })
+  })
+})
diff --git a/test/fixtures/es-response-b1234-just-non-recap-statuses.json b/test/fixtures/es-response-b1234-just-non-recap-statuses.json
@@ -1,32 +1,30 @@
 {
-  "body": {
-    "took": 11,
-    "timed_out": false,
-    "_shards": {
-      "total": 3,
-      "successful": 3,
-      "failed": 0
-    },
-    "hits": {
-      "total": 1,
-      "max_score": 0.0,
-      "hits": []
-    },
-    "aggregations": {
-      "statuses": {
-        "doc_count": 4,
-        "nonrecap_statuses": {
-          "doc_count": 2,
-          "nonrecap_status_buckets": {
-            "doc_count_error_upper_bound": 0,
-            "sum_other_doc_count": 0,
-            "buckets": [
-              {
-                "key": "status:a||Available",
-                "doc_count": 2
-              }
-            ]
-          }
+  "took": 11,
+  "timed_out": false,
+  "_shards": {
+    "total": 3,
+    "successful": 3,
+    "failed": 0
+  },
+  "hits": {
+    "total": 1,
+    "max_score": 0.0,
+    "hits": []
+  },
+  "aggregations": {
+    "statuses": {
+      "doc_count": 4,
+      "nonrecap_statuses": {
+        "doc_count": 2,
+        "nonrecap_status_buckets": {
+          "doc_count_error_upper_bound": 0,
+          "sum_other_doc_count": 0,
+          "buckets": [
+            {
+              "key": "status:a||Available",
+              "doc_count": 2
+            }
+          ]
         }
       }
     }