Merge pull request #428 from NYPL/main

subject literal trailing period bugfix
NYPL · Dec 13, 2024 · 1b3e2bd · 1b3e2bd
2 parents 3ac35e0 + e82c7cc
commit 1b3e2bd
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 45 deletions.
diff --git a/lib/elasticsearch/config.js b/lib/elasticsearch/config.js
@@ -1,3 +1,5 @@
+const util = require('../util')
+
 // Configure search scopes:
 const SEARCH_SCOPES = {
   all: {
@@ -76,7 +78,19 @@ const SEARCH_SCOPES = {
 const FILTER_CONFIG = {
   recordType: { operator: 'match', field: ['recordTypeId'], repeatable: true },
   owner: { operator: 'match', field: ['items.owner.id', 'items.owner.label'], repeatable: true, path: 'items' },
-  subjectLiteral: { operator: 'match', field: ['subjectLiteral_exploded'], repeatable: true },
+  subjectLiteral: {
+    transform: (val, logger) => {
+      if (typeof val === 'string') {
+        return util.removeTrailingPeriod(val, logger)
+      }
+      if (Array.isArray(val)) {
+        return val.map((val) => util.removeTrailingPeriod(val, logger))
+      }
+    },
+    operator: 'match',
+    field: ['subjectLiteral_exploded'],
+    repeatable: true
+  },
   holdingLocation: { operator: 'match', field: ['items.holdingLocation.id', 'items.holdingLocation.label'], repeatable: true, path: 'items' },
   buildingLocation: { operator: 'match', field: ['buildingLocationIds'], repeatable: true },
   language: { operator: 'match', field: ['language.id', 'language.label'], repeatable: true },

diff --git a/lib/util.js b/lib/util.js
@@ -1,6 +1,13 @@
 const logger = require('./logger')
 const { isItemNyplOwned } = require('./ownership_determination')
 
+exports.removeTrailingPeriod = (x, logger) => {
+  if (x.slice(-1) === '.') {
+    logger.debug('Removing terminal period', JSON.stringify(x, null, 4))
+    return x.slice(0, -1)
+  } else return x
+}
+
 exports.sortOnPropWithUndefinedLast = (property) => {
   return function (a, b) {
     // equal items sort equally
@@ -120,22 +127,8 @@ exports.parseParams = function (params, spec) {
 //   `fields`: Hash - When `type` is 'hash', this property provides field spec to validate internal fields against
 //   `repeatable`: Boolean - If true, array of values may be returned. Otherwise will select last. Default false
 exports.parseParam = function (val, spec) {
-  if (spec.fields &&
-    spec.fields.subjectLiteral &&
-    spec.fields.subjectLiteral.field === 'subjectLiteral_exploded' &&
-    val.subjectLiteral
-  ) {
-    if (typeof val.subjectLiteral === 'string' && val.subjectLiteral.slice(-1) === '.') {
-      val.subjectLiteral = val.subjectLiteral.slice(0, -1)
-      logger.debug('Removing terminal period', JSON.stringify(val, null, 4))
-    } else if (Array.isArray(val.subjectLiteral)) {
-      val.subjectLiteral.forEach((sub, i) => {
-        if (sub.slice(-1) === '.') {
-          val.subjectLiteral[i] = sub.slice(0, -1)
-          logger.debug('Removing terminal period', JSON.stringify(sub, null, 4))
-        }
-      })
-    }
+  if (spec.transform) {
+    val = spec.transform(val, logger)
   }
 
   // Unless it's marked repeatable, convert arrays of values to just last value:

diff --git a/test/util.test.js b/test/util.test.js
@@ -2,6 +2,7 @@ const { expect } = require('chai')
 const mangledEnumerationChronologyItems = require('./fixtures/mangled_enumerationChronology_items.json')
 
 const util = require('../lib/util')
+const { FILTER_CONFIG } = require('../lib/elasticsearch/config')
 
 describe('Util', function () {
   describe('sortOnPropWithUndefinedLast', () => {
@@ -64,19 +65,14 @@ describe('Util', function () {
         filters: {
           subjectLiteral: 'cats',
           contributorLiteral: ['Contrib 1', 'Contrib 2'],
-          date: '2012',
+          dateAfter: '2012',
           badNumeric: 'blah'
         }
       }
       const spec = {
         filters: {
           type: 'hash',
-          fields: {
-            subjectLiteral: { type: 'string' },
-            contributorLiteral: { type: 'string' },
-            date: { type: 'int' },
-            badNumeric: { type: 'int' }
-          }
+          fields: { ...FILTER_CONFIG, badNumeric: { type: 'int' } }
         }
       }
       const outgoing = util.parseParams(incoming, spec)
@@ -87,8 +83,8 @@ describe('Util', function () {
       expect(outgoing.filters.subjectLiteral).to.be.a('string')
       expect(outgoing.filters.subjectLiteral).to.equal('cats')
 
-      expect(outgoing.filters.date).to.be.a('number')
-      expect(outgoing.filters.date).to.equal(2012)
+      expect(outgoing.filters.dateAfter).to.be.a('number')
+      expect(outgoing.filters.dateAfter).to.equal(2012)
 
       expect(outgoing.filters.badNumeric).to.be.a('undefined')
     })
@@ -131,15 +127,7 @@ describe('Util', function () {
     }
 
     const spec = {
-      filters: {
-        type: 'hash',
-        fields: {
-          subjectLiteral: {
-            type: 'string',
-            field: 'subjectLiteral_exploded'
-          }
-        }
-      }
+      filters: { type: 'hash', fields: FILTER_CONFIG }
     }
 
     const outgoing = util.parseParams(incoming, spec)
@@ -154,16 +142,7 @@ describe('Util', function () {
     }
 
     const spec = {
-      filters: {
-        type: 'hash',
-        fields: {
-          subjectLiteral: {
-            type: 'string',
-            field: 'subjectLiteral_exploded',
-            repeatable: true
-          }
-        }
-      }
+      filters: { type: 'hash', fields: FILTER_CONFIG }
     }
 
     const outgoing = util.parseParams(incoming, spec)