Some finessing from QA - item filters and undefined sorting

lib/resources.js

@@ -245,10 +245,12 @@ module.exports = function (app, _private = null) {
             }
           }
         }
-        if (params.all_items) {
+        const paramsIncludesItemLevelFiltering = Object.keys(params)
+          .filter((param) => param.startsWith('item_')).length > 0
+        const returnAllItems = params.all_items && !paramsIncludesItemLevelFiltering
+        if (returnAllItems) {
           body._source.excludes = EXCLUDE_FIELDS.filter((field) => field !== '*_sort')
-        }
-        if (!params.all_items) {
+        } else {
           // No specific item requested, so add pagination and matching params:
           const itemsOptions = {
             size: params.items_size,
@@ -276,6 +278,7 @@ module.exports = function (app, _private = null) {
         return app.esClient.search(body)
           .then((resp) => {
             resp = resp.body
+
             // Mindfully throw errors for known issues:
             if (!resp || !resp.hits) {
               throw new Error('Error connecting to index')
@@ -1218,9 +1221,9 @@ const buildElasticQueryForFilters = function (params) {
 const buildElasticQuery = function (params) {
   // Build ES query:
   const query = {}
-
-    // clean up params
-    ;['q'].forEach(function (param) {
+  const paramsArray = ['q']
+  // clean up params
+  paramsArray.forEach(function (param) {
     if (params[param]) {
       params[param] = params[param].replace(/date:/g, 'dateStartYear:')
       params[param] = params[param].replace(/location:/g, 'locations:')

lib/response_massager.js

@@ -1,7 +1,7 @@
 const LocationLabelUpdater = require('./location_label_updater')
 const AvailabilityResolver = require('./availability_resolver.js')
 const parallelFieldsExtractor = require('./parallel-fields-extractor')
-const { isAeonUrl } = require('../lib/util')
+const { isAeonUrl, sortOnPropWithUndefinedLast } = require('../lib/util')
 const FulfillmentResolver = require('./fulfillment_resolver')
 const RequestabilityResolver = require('./requestability_resolver')
 
@@ -22,10 +22,10 @@ class ResponseMassager {
   processInnerHitsProperties (response, sortOnEnumerationChronology) {
     response.hits.hits.forEach((hit) => {
       if (sortOnEnumerationChronology) {
-        hit._source.items.sort((a, b) => a.enumerationChronology_sort[0] > b.enumerationChronology_sort[0] ? -1 : 1)
+        hit._source.items.sort(sortOnPropWithUndefinedLast('enumerationChronology_sort'))
       }
       // Process "items" inner_hits
-      if (hit.inner_hits && hit.inner_hits.items) {
+      if (hit?.inner_hits?.items) {
         // Reassign items inner_hits to .items
         hit._source.items = hit.inner_hits.items.hits.hits.map((itemHit) => itemHit._source)
 
@@ -40,7 +40,7 @@ class ResponseMassager {
         hit._source.numItemsTotal = [unfilteredItems.hits.total]
       }
       // Process "electronicResources" inner_hits
-      if (hit.inner_hits && hit.inner_hits.electronicResources) {
+      if (hit?.inner_hits?.electronicResources) {
         // If record doesn't have indexed electronicResources...
         if (!hit._source.electronicResources || hit._source.electronicResources.length === 0) {
           // Gather up all records in the electronicResources inner_hit,
@@ -61,10 +61,14 @@ class ResponseMassager {
 
   massagedResponse (request, options = {}) {
     let response = this.elasticSearchResponse
-    const allItemsBibQuery = request?.query?.all_items
+    const queryKeys = Object.keys(request?.query || {})
+    const itemFilterQuery = queryKeys
+      .filter((param) => param.startsWith('item_')).length > 0
+    const returnAllItems = queryKeys?.includes('all_items') &&
+      !itemFilterQuery
     // Inspect response inner_hits queries and move properties around to ease
     // serialization:
-    response = this.processInnerHitsProperties(response, allItemsBibQuery)
+    response = this.processInnerHitsProperties(response, returnAllItems)
 
     // Rename parallel fields:
     response = parallelFieldsExtractor(response)

lib/util.js

@@ -1,6 +1,23 @@
 const logger = require('./logger')
 const isItemNyplOwned = require('./ownership_determination').isItemNyplOwned
 
+exports.sortOnPropWithUndefinedLast = (property) => {
+  return function (a, b) {
+    // equal items sort equally
+    if (a[property]?.[0] === b[property]?.[0]) {
+      return 0
+    }
+    // nulls sort after anything else
+    if (!a[property]?.[0]) {
+      return 1
+    }
+    if (!b[property]?.[0]) {
+      return -1
+    }
+    return a[property]?.[0] > b[property]?.[0] ? -1 : 1
+  }
+}
+
 exports.buildJsonLdContext = function (prefixes) {
   const context = JSON.parse(JSON.stringify(prefixes))
   delete context.urn