Non-modal API for fetching vocabs from ReShare

TECH DEBT: Change modal API for fetching controlled vocabularies to one that returns the fetched data. Fixes PR-2097.
openlibraryenvironment · Jan 13, 2025 · 2a06e04 · 2a06e04
1 parent 7de148f
commit 2a06e04
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 27 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -35,6 +35,7 @@
 * Add Service Level field to blank request form. Fixes PR-2056.
 * Add maximum cost field-pair (`maximumCostsMonetaryValue` and `maximumCostsCurrencyCode`) to blank request form. Fixes PR-2058.
 * TECH DEBT: The `OkapiSession`'s config object is now in its `cfg` member, not `logger`. Fixes PR-2095.
+* TECH DEBT: Change modal API for fetching controlled vocabularies to one that returns the fetched data. Fixes PR-2097.
 
 ## [1.6.0](https://github.com/openlibraryenvironment/listener-openurl/tree/v1.6.0) (2024-03-04)
 

diff --git a/src/OkapiSession.js b/src/OkapiSession.js
@@ -44,7 +44,8 @@ class OkapiSession {
     return json;
   }
 
-  async getPickupLocations() {
+  async listPickupLocations() {
+    // XXX could no-op if this.pickupLocations is already defined
     const path = '/directory/entry?filters=tags.value%3Di%3Dpickup&filters=status.value%3Di%3Dmanaged&perPage=100&stats=true';
     const json = await this._getDataFromReShare(path, 'pickup locations');
     this.pickupLocations = json.results
@@ -57,6 +58,8 @@ class OkapiSession {
         if (typeof b.name !== 'string') return -1;
         return a.name.localeCompare(b.name);
       });
+
+    return this.pickupLocations;
   }
 
   async _getRefDataValues(desc, caption) {
@@ -65,25 +68,30 @@ class OkapiSession {
     return json[0]?.values.map(r => ({ id: r.id, code: r.value, name: r.label }));
   }
 
-  async getCopyrightTypes() {
+  async listCopyrightTypes() {
     // XXX could no-op if this.copyrightTypes is already defined
     this.copyrightTypes = await this._getRefDataValues('copyrightType', 'copyright types');
+    return this.copyrightTypes;
   }
 
-  async getServiceLevels() {
+  async listServiceLevels() {
     // XXX could no-op if this.serviceLevels is already defined
     this.serviceLevels = await this._getRefDataValues('ServiceLevels', 'service levels');
+    return this.serviceLevels;
   }
 
-  async getCurrencies() {
+  async listCurrencies() {
     // XXX could no-op if this.currencies is already defined
     this.currencies = await this._getRefDataValues('CurrencyCodes', 'currencies');
+    return this.currencies;
   }
 
-  async getDefaultCopyrightType() {
+  async listDefaultCopyrightType() {
+    // XXX could no-op if this.defaultCopyrightType is already defined
     const path = '/rs/settings/appSettings?filters=section%3D%3Dother&filters=key%3D%3Ddefault_copyright_type&perPage=1';
     const json = await this._getDataFromReShare(path, 'default copyright type');
     this.defaultCopyrightType = json[0]?.value;
+    return this.defaultCopyrightType;
   }
 
   post(path, payload) {

diff --git a/src/OpenURLServer.js b/src/OpenURLServer.js
@@ -103,11 +103,27 @@ function unArray(val) {
 }
 
 
-function makeFormData(ctx, query, service, valuesNotShownInForm, firstTry, npl) {
+async function makeFormData(ctx, query, service, valuesNotShownInForm, firstTry, npl) {
   const isCopy = query.svc_id === 'copy';
-  const currentCopyrightType = query['rft.copyrightType'] || service.defaultCopyrightType;
   query.svc_id ||= 'loan';
 
+  const promises = [
+    service.listServiceLevels(),
+    service.listCurrencies()
+  ];
+  if (!npl) {
+    promises.push(
+      service.listCopyrightTypes(),
+      service.listPickupLocations(),
+      service.listDefaultCopyrightType(),
+    );
+  }
+
+  const res = await Promise.all(promises);
+  const [serviceLevels, currencies, copyrightTypes, pickupLocations, defaultCopyrightType] = res;
+  const currentCopyrightType = query['rft.copyrightType'] || defaultCopyrightType;
+  console.error('defaultCopyrightType =', defaultCopyrightType, '-- currentCopyrightType =', currentCopyrightType);
+
   const data = Object.assign({}, query, {
     valuesNotShownInForm,
     digitalOnly: ctx.state?.svcCfg?.digitalOnly,
@@ -120,8 +136,8 @@ function makeFormData(ctx, query, service, valuesNotShownInForm, firstTry, npl)
     hasDate: !!query['rft.date'],
     hasISBN: !!query['rft.isbn'],
 
-    onePickupLocation: (service?.pickupLocations?.length === 1),
-    pickupLocations: (service.pickupLocations || []).map(x => ({
+    onePickupLocation: (pickupLocations?.length === 1),
+    pickupLocations: (pickupLocations || []).map(x => ({
       id: x.id,
       code: x.code,
       name: x.name,
@@ -132,7 +148,7 @@ function makeFormData(ctx, query, service, valuesNotShownInForm, firstTry, npl)
       name: x === '' ? '(None selected)' : x === 'bookitem' ? 'Book chapter' : x.charAt(0).toUpperCase() + x.slice(1),
       selected: x === query['rft.genre'] ? 'selected' : '',
     })),
-    copyrightTypes: (ctx.cfg.getValues()?.copyrightTypes || service.copyrightTypes || []).map(x => ({
+    copyrightTypes: (ctx.cfg.getValues()?.copyrightTypes || copyrightTypes || []).map(x => ({
       ...x,
       selected: x.code === currentCopyrightType ? 'selected' : '',
     })),
@@ -141,11 +157,11 @@ function makeFormData(ctx, query, service, valuesNotShownInForm, firstTry, npl)
       name: x.charAt(0).toUpperCase() + x.slice(1),
       checked: x === query.svc_id || (!query.svc_id && i === 0) ? 'checked' : '',
     })),
-    serviceLevels: (service.serviceLevels || []).map(x => ({
+    serviceLevels: (serviceLevels || []).map(x => ({
       ...x,
       selected: x.code === query['svc.level'] ? 'selected' : '',
     })),
-    currencies: (service.currencies || []).map(x => ({
+    currencies: (currencies || []).map(x => ({
       ...x,
       selected: x.code === query['svc.costCurrency'] ? 'selected' : '',
     })),
@@ -192,20 +208,6 @@ async function maybeRenderForm(ctx, next) {
   }
 
   ctx.cfg.log('flow', 'Rendering form', formName);
-  if (!npl) {
-    const pickupLocationPromise = service.getPickupLocations();
-    await service.getCopyrightTypes(); // Needed before getDefaultCopyrightType can be called
-    await Promise.all([
-      pickupLocationPromise,
-      service.getDefaultCopyrightType(),
-    ]);
-  }
-
-  await Promise.all([
-    service.getServiceLevels(),
-    service.getCurrencies()
-  ]);
-
   const originalQuery = co.getQuery();
   const query = {};
   Object.keys(originalQuery).forEach(key => {
@@ -232,7 +234,7 @@ async function maybeRenderForm(ctx, next) {
     .map(key => `<input type="hidden" name="${key}" value="${query[key]?.replaceAll('"', '&quot;')}">`)
     .join('\n');
 
-  const data = makeFormData(ctx, query, service, valuesNotShownInForm, parseInt(ntries) === 0, npl);
+  const data = await makeFormData(ctx, query, service, valuesNotShownInForm, parseInt(ntries) === 0, npl);
   ctx.body = ctx.cfg.runTemplate(formName, data);
 }