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

Extract logic about querying service points to a new class #1714

Closed
wants to merge 1 commit into from
Closed
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
4 changes: 2 additions & 2 deletions app/jobs/submit_folio_request_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -94,13 +94,13 @@ def place_title_hold
def pickup_location_id
@pickup_location_id ||= begin
code = service_point_code(request.destination)
Folio::Types.instance.service_point_id(code)
Folio::Types.service_points.find_by(code:)&.id
end
end

def service_point_code(destination)
# Check if comparable service point code exists, otherwise return default
return destination if Folio::Types.instance.valid_service_point_code?(destination)
return destination if Folio::Types.service_points.find_by(code: destination)

# During cutover and migration, we may still need to depend on the service point defined in settings
Settings.libraries[destination]&.folio_pickup_service_point_code || Settings.folio.default_service_point
Expand Down
6 changes: 4 additions & 2 deletions app/models/folio/pickup_destination.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,16 @@ def initialize(destination_code)
@code = destination_code
end

attr_reader :code

def display_label
Folio::Types.instance.service_point_name(@code)
Folio::Types.service_points.find_by(code:).name
end

# For paging scheduling, we must map from service point to library if folio
# Otherwise, the library code is what we will be receiving
def library_code
Folio::Types.instance.map_to_library_code(@code)
Folio::Types.map_to_library_code(code)
end
end
end
17 changes: 9 additions & 8 deletions app/models/folio/request_abilities.rb
Original file line number Diff line number Diff line change
Expand Up @@ -67,22 +67,23 @@ def pickup_destinations
location_restricted_service_point_codes
end

# rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity
# Find service point which is default for this particular campus
def default_pickup_destination
# Find service point which is default for this particular campus
pickup = Folio::Types.instance.service_points.select do |_k, v|
v.is_default_for_campus.present? && v.is_default_for_campus == request.holdings.first&.effective_location&.campus&.code
end.values.map(&:code)
pickup.present? ? pickup[0] : Settings.folio.default_service_point
campus_code = request.holdings.first&.effective_location&.campus&.code
service_points = if campus_code
Folio::Types.service_points.where(is_default_for_campus: campus_code).map(&:code)
else
[]
end
service_points.first || Settings.folio.default_service_point
end

private

# Returns default service point codes
def default_pickup_service_points
Folio::Types.instance.service_points.select { |_k, v| v.is_default_pickup }.values.map(&:code)
Folio::Types.service_points.where(is_default_pickup: true).map(&:code)
end
# rubocop:enable Metrics/AbcSize, Metrics/CyclomaticComplexity

def additional_pickup_service_points
# Map library to a service point
Expand Down
4 changes: 4 additions & 0 deletions app/models/folio/service_point.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,9 @@ def self.from_dynamic(json)
is_default_pickup: json.dig('details', 'isDefaultPickup'),
is_default_for_campus: json.dig('details', 'isDefaultForCampus'))
end

def pickup_location?
pickup_location == true
end
end
end
32 changes: 32 additions & 0 deletions app/services/folio/service_point_store.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
# frozen_string_literal: true

module Folio
# A cache of service point data
class ServicePointStore
def initialize(data_from_cache)
@data = data_from_cache.map { |p| Folio::ServicePoint.from_dynamic(p) }
end

attr_reader :data

def find_by(args)
if args.key?(:code)
data.find { |candidate| candidate.code == args[:code] }
elsif args.key?(:id)
data.find { |candidate| candidate.id == args[:id] }
else
raise "unknown argument #{args.inspect}"
end
end

def where(args)
if args.key?(:is_default_for_campus)
data.select { |candidate| candidate.is_default_for_campus == args[:is_default_for_campus] }
elsif args.key?(:is_default_pickup)
data.select { |candidate| candidate.is_default_pickup == args[:is_default_pickup] }
else
raise "unknown argument #{args.inspect}"
end
end
end
end
40 changes: 10 additions & 30 deletions app/services/folio/types.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,8 @@ module Folio
# accessing the types.
class Types
class << self
delegate :policies, :circulation_rules, :criteria, :get_type, :locations, :libraries,
:map_to_library_code, :map_to_service_point_code, :service_point_name, :service_point_code,
:service_point_id, :valid_service_point_code?, to: :instance
delegate :policies, :circulation_rules, :criteria, :get_type, :locations, :libraries, :service_points,
:map_to_library_code, :map_to_service_point_code, to: :instance
end

def self.instance
Expand Down Expand Up @@ -46,7 +45,7 @@ def circulation_rules
end

def service_points
get_type('service_points').map { |p| Folio::ServicePoint.from_dynamic(p) }.index_by(&:id)
@service_points ||= ServicePointStore.new(get_type('service_points'))
end

def policies
Expand Down Expand Up @@ -94,40 +93,31 @@ def get_type(type)
def map_to_library_code(service_point_code)
return service_point_code if service_point_code == 'SCAN'

return nil unless valid_service_point_code?(service_point_code)
service_point = service_points.find_by(code: service_point_code)
return nil unless service_point

service_point_id = service_point_id(service_point_code)
library_id = library_for_service_point_id(service_point_id)
library_id = library_for_service_point_id(service_point.id)

# Find the library code associated with this library id
libraries.key?(library_id) ? libraries[library_id]['code'] : nil
end

# Find the service point ID based on this service point code
def service_point_id(service_point_code)
service_points.values.find { |v| v.code == service_point_code }&.id
end

# Find the library id for the location with which this service point is associated
def library_for_service_point_id(service_point_id)
loc = locations.values.find { |location| location['primaryServicePoint'] == service_point_id }
loc && loc['libraryId']
end

# Check if valid service point
def valid_service_point_code?(service_point_code)
service_points.values.any? { |v| v.code == service_point_code }
end

# Given a library code, retrieve the primary service point, ensuring pickup location is true
def map_to_service_point_code(library_code)
# Find library id for the library with this code
library_id = library_id(library_code)
# Get the associated location and related service point
service_point_id = service_point_for_library(library_id)
# Find the service point ID based on this service point code
service_point = service_point_by_id(service_point_id)
service_point.present? && service_point.pickup_location == true ? service_point.code : nil

service_point = service_points.find_by(id: service_point_id)

service_point.code if service_point&.pickup_location?
end

def library_id(library_code)
Expand All @@ -140,16 +130,6 @@ def service_point_for_library(library_id)
loc && loc['primaryServicePoint']
end

def service_point_by_id(service_point_id)
service_points.key?(service_point_id) && service_points[service_point_id]
end

# Get the name for the service point given the code
def service_point_name(code)
# Find the service point with the same code, and return the name
service_points.values.find { |v| v.code == code }&.name
end

private

# rubocop:disable Metrics/MethodLength
Expand Down
2 changes: 1 addition & 1 deletion spec/features/pickup_libraries_dropdown_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

RSpec.describe 'Pickup Libraries Dropdown' do
let(:is_folio) { (Settings.ils.bib_model == 'Folio::Instance') }
let(:folio_pickup_lib_total) { Folio::Types.instance.service_points.select { |_k, v| v.is_default_pickup }.values.length }
let(:folio_pickup_lib_total) { Folio::Types.service_points.where(is_default_pickup: true).length }
let(:standard_pickup_lib_total) { is_folio ? folio_pickup_lib_total : Settings.default_pickup_libraries.count }
let(:media_library) { is_folio ? 'MEDIA-CENTER' : 'MEDIA-MTXT' }
let(:media_label) { is_folio ? 'Media Center' : 'Media Microtext' }
Expand Down
25 changes: 5 additions & 20 deletions spec/helpers/pickup_libraries_helper_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,23 +8,6 @@
%w[ABC XYZ]
end

let(:service_points_json) do
<<~JSON
[
{
"id": "ABC",
"code": "ABC",
"discoveryDisplayName": "Library 2"
},
{
"id": "XYZ",
"code": "XYZ",
"discoveryDisplayName": "Library 1"
}
]
JSON
end

before do
allow(Settings).to receive(:libraries).and_return(
{
Expand All @@ -33,9 +16,11 @@
}
)

allow(Folio::Types.instance).to receive(:service_points).and_return(
JSON.parse(service_points_json).map { |p| Folio::ServicePoint.from_dynamic(p) }.index_by(&:id)
)
allow(Folio::Types.service_points).to receive(:find_by).with(code: 'ABC').and_return(instance_double(Folio::ServicePoint,
name: 'Library 2'))

allow(Folio::Types.service_points).to receive(:find_by).with(code: 'XYZ').and_return(instance_double(Folio::ServicePoint,
name: 'Library 1'))
end

it 'sorts the libraries by the name of the library (and not the code)' do
Expand Down
70 changes: 6 additions & 64 deletions spec/models/folio/pickup_destination_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,75 +3,17 @@
require 'rails_helper'

RSpec.describe Folio::PickupDestination do
subject(:pickup_destination) do
described_class.new(
'GREEN-LOAN'
)
end

let(:service_points_json) do
<<~JSON
[
{
"code": "GREEN-LOAN",
"id": "a5dbb3dc-84f8-4eb3-8bfe-c61f74a9e92d",
"discoveryDisplayName": "Green Library"
}
]
JSON
end

before do
allow(Folio::Types.instance).to receive(:service_points).and_return(
JSON.parse(service_points_json).map { |p| Folio::ServicePoint.from_dynamic(p) }.index_by(&:id)
)
end
let(:pickup_destination) { described_class.new('GREEN-LOAN') }

describe '#display_label' do
it 'returns service point name' do
expect(subject.display_label).to eq 'Green Library'
end
subject { pickup_destination.display_label }

it { is_expected.to eq 'Green Library' }
end

describe '#library_code' do
let(:locations_json) do
<<~JSON
[
{
"id": "4573e824-9273-4f13-972f-cff7bf504217",
"code": "GRE-STACKS",
"discoveryDisplayName": "Stacks",
"libraryId": "f6b5519e-88d9-413e-924d-9ed96255f72e",
"primaryServicePoint": "a5dbb3dc-84f8-4eb3-8bfe-c61f74a9e92d"
}
]
JSON
end

let(:libraries_json) do
<<~JSON
[
{
"id": "f6b5519e-88d9-413e-924d-9ed96255f72e",
"name": "Green Library",
"code": "GREEN",
"campusId": "c365047a-51f2-45ce-8601-e421ca3615c5"
}
]
JSON
end

before do
allow(Folio::Types.instance).to receive(:libraries).and_return(
JSON.parse(libraries_json).index_by { |p| p['id'] }
)
allow(Folio::Types.instance).to receive(:locations).and_return(
JSON.parse(locations_json).index_by { |p| p['id'] }
)
end
subject { pickup_destination.library_code }

it 'returns library code for service point' do
expect(subject.library_code).to eq 'GREEN'
end
it { is_expected.to eq 'GREEN' }
end
end