Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use the holdingType value (or inferred type from the location) to determine if something is an electronic holding #999

Merged
merged 1 commit into from
Jul 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 4 additions & 22 deletions lib/folio/eresource_holdings_builder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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?
Expand Down Expand Up @@ -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
6 changes: 4 additions & 2 deletions spec/integration/folio_config_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,8 @@
let(:items_and_holdings) do
{ 'items' => [],
'holdings' =>
[{ 'location' =>
[{ 'holdingType' => { 'name' => 'Electronic' },
'location' =>
{ 'permanentLocation' =>
{ 'code' => 'SUL-ELECTRONIC' },
'effectiveLocation' =>
Expand Down Expand Up @@ -146,7 +147,8 @@
let(:items_and_holdings) do
{ 'items' => [],
'holdings' =>
[{ 'location' =>
[{ 'holdingType' => { 'name' => 'Electronic' },
'location' =>
{ 'permanentLocation' =>
{ 'code' => 'LAW-ELECTRONIC' },
'effectiveLocation' =>
Expand Down
36 changes: 24 additions & 12 deletions spec/lib/folio/eresource_holdings_builder_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@
let(:items_and_holdings) do
{ 'items' => [],
'holdings' =>
[{ 'location' =>
[{ 'holdingType' => { 'name' => 'Electronic' },
'location' =>
{ 'permanentLocation' =>
{ 'code' => 'SUL-ELECTRONIC' },
'effectiveLocation' =>
Expand Down Expand Up @@ -63,7 +64,8 @@
let(:items_and_holdings) do
{ 'items' => [],
'holdings' =>
[{ 'location' =>
[{ 'holdingType' => { 'name' => 'Electronic' },
'location' =>
{ 'permanentLocation' =>
{ 'code' => 'LAW-ELECTRONIC' },
'effectiveLocation' =>
Expand All @@ -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 }
Expand All @@ -97,7 +92,7 @@
let(:items_and_holdings) do
{ 'items' => [],
'holdings' =>
[{ 'location' =>
[{ 'holdingType' => { 'name' => 'Electronic' }, 'location' =>
{ 'permanentLocation' =>
{ 'code' => 'SUL-SDR' },
'effectiveLocation' =>
Expand All @@ -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