Merge pull request #999 from sul-dlss/994-lane-elect

Use the holdingType value (or inferred type from the location) to determine if something is an electronic holding
sul-dlss · Jul 24, 2023 · 7b29ad5 · 7b29ad5
2 parents 1d76657 + 2f6daff
commit 7b29ad5
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 36 deletions.
diff --git a/lib/folio/eresource_holdings_builder.rb b/lib/folio/eresource_holdings_builder.rb
@@ -7,22 +7,6 @@ module Folio
   class EresourceHoldingsBuilder
     CALL_NUMBER = 'INTERNET RESOURCE'
     TYPE = 'ONLINE'
-    ONLINE_LOCATION_CODES = %w[BUS-ELECTRONIC
-                               BUS-SDR
-                               HILA-ELECTRONIC
-                               HILA-SDR
-                               LANE-ECOLL
-                               LANE-ECOMP
-                               LANE-EDATA
-                               LANE-EDOC
-                               LANE-EPER
-                               LANE-IMAGE
-                               LANE-ISIIF
-                               LANE-MOBI
-                               LAW-ELECTRONIC
-                               LAW-SDR
-                               SUL-ELECTRONIC
-                               SUL-SDR].freeze
 
     def self.build(hrid, holdings, marc_record)
       new(hrid, holdings, marc_record).build
@@ -40,7 +24,7 @@ def build
       # If there isn't a holding with an electronic holding code
       # we assume the fulltext link supplements a physical item,
       # like a PURL or HathiTrust link and we don't need to do anything.
-      return [] unless electronic_holding_location_code
+      return [] unless electronic_holding_location
 
       fields = fulltext_links
       fields = electronic_location_fields.first(1) if fields.empty?
@@ -93,16 +77,14 @@ def library_code
     end
 
     def mapped_location_codes
-      @mapped_location_codes ||= LocationsMap.for(electronic_holding_location_code)
+      @mapped_location_codes ||= LocationsMap.for(electronic_holding_location.dig('location', 'effectiveLocation', 'code'))
     end
 
     # This finds the first holding matching an online location code.
     # This approach works fine unless there are records with multiple
     # e-resource holdings associated with different locations.
-    def electronic_holding_location_code
-      @electronic_holding_location_code ||=
-        (holdings || []).map { |h| h.dig('location', 'permanentLocation', 'code') }
-                        .find { |c| ONLINE_LOCATION_CODES.include?(c) }
+    def electronic_holding_location
+      @electronic_holding_location ||= holdings&.find { |h| (h.dig('holdingType', 'name') || h.dig('location', 'effectiveLocation', 'details', 'holdingTypeName')) == 'Electronic' }
     end
   end
 end
diff --git a/spec/integration/folio_config_spec.rb b/spec/integration/folio_config_spec.rb
@@ -113,7 +113,8 @@
     let(:items_and_holdings) do
       { 'items' => [],
         'holdings' =>
-         [{ 'location' =>
+         [{ 'holdingType' => { 'name' => 'Electronic' },
+            'location' =>
           { 'permanentLocation' =>
             { 'code' => 'SUL-ELECTRONIC' },
             'effectiveLocation' =>
@@ -146,7 +147,8 @@
       let(:items_and_holdings) do
         { 'items' => [],
           'holdings' =>
-           [{ 'location' =>
+           [{ 'holdingType' => { 'name' => 'Electronic' },
+              'location' =>
             { 'permanentLocation' =>
               { 'code' => 'LAW-ELECTRONIC' },
               'effectiveLocation' =>

diff --git a/spec/lib/folio/eresource_holdings_builder_spec.rb b/spec/lib/folio/eresource_holdings_builder_spec.rb
@@ -24,7 +24,8 @@
   let(:items_and_holdings) do
     { 'items' => [],
       'holdings' =>
-       [{ 'location' =>
+       [{ 'holdingType' => { 'name' => 'Electronic' },
+          'location' =>
           { 'permanentLocation' =>
             { 'code' => 'SUL-ELECTRONIC' },
             'effectiveLocation' =>
@@ -63,7 +64,8 @@
     let(:items_and_holdings) do
       { 'items' => [],
         'holdings' =>
-         [{ 'location' =>
+         [{ 'holdingType' => { 'name' => 'Electronic' },
+            'location' =>
           { 'permanentLocation' =>
             { 'code' => 'LAW-ELECTRONIC' },
             'effectiveLocation' =>
@@ -76,18 +78,11 @@
     it { expect(holdings.first.library).to eq 'LAW' }
   end
 
-  context 'the holding does not include an electronic location' do
+  context 'the holding does not include an electronic type' do
     let(:items_and_holdings) do
       { 'items' => [],
         'holdings' =>
-         [{ 'location' =>
-          { 'permanentLocation' =>
-            { 'code' => 'LAW-BASEMENT' },
-            'effectiveLocation' =>
-            { 'code' => 'LAW-BASEMENT' } },
-            'suppressFromDiscovery' => false,
-            'id' => '81a56270-e8dd-5759-8083-5cc96cdf0045',
-            'holdingsStatements' => [] }] }
+         [{ 'holdingType' => { 'name' => 'Whatever' } }] }
     end
 
     it { expect(holdings).to be_empty }
@@ -97,7 +92,7 @@
     let(:items_and_holdings) do
       { 'items' => [],
         'holdings' =>
-         [{ 'location' =>
+         [{ 'holdingType' => { 'name' => 'Electronic' }, 'location' =>
           { 'permanentLocation' =>
             { 'code' => 'SUL-SDR' },
             'effectiveLocation' =>
@@ -108,4 +103,21 @@
     end
     it { expect(holdings.first.home_location).to eq 'INTERNET' }
   end
+
+  context 'the holding library is Lane (without a explicit holdingType)' do
+    let(:items_and_holdings) do
+      { 'items' => [],
+        'holdings' =>
+         [{ 'location' =>
+          { 'permanentLocation' =>
+            { 'code' => 'LANE-EDATA' },
+            'effectiveLocation' =>
+            { 'code' => 'LANE-EDATA',  'details' => { 'holdingTypeName' => 'Electronic' } } },
+            'suppressFromDiscovery' => false,
+            'id' => '81a56270-e8dd-5759-8083-5cc96cdf0045',
+            'holdingsStatements' => [] }] }
+    end
+
+    it { expect(holdings.first.library).to eq 'LANE-MED' }
+  end
 end