31a2a5a6-71e0-4c97-89ac-edd0d51725ca

Committed 23 Oct 2025 09:51PM UTC coverage: 89.759% (-0.03%) from 89.788%

Build # 31a2a5a6-71e0-4c97-89ac-edd0d51725ca

Build Type

Pull #2969

circleci

Committed by

sandbergja

Commit Message

Remove unnecessary call from bibliographic holding availability endpoint

This affects the endpoint /bibliographic/:bib_id/holdings/:holding_id/availability,
which is used by the Requests form.

Prior to this commit, this endpoint made 2 or more requests to Alma:
* 1 to retrieve the MarcXML record along with requests and general
  availability data
* 1 or more to retrieve the item availability data

The first call was only used to retrieve the bib id -- something that
we already have because the user provided it in the request. Therefore,
we can eliminate this call and save an extra 500-1000ms each time we
load the request form (and be gentler on our API limits).

Co-authored-by: Christina Chortaria <christinach@users.noreply.github.com>
Co-authored-by: Mark Zelesky <mzelesky@users.noreply.github.com>

Pull Request Pull Request #2969: Remove unnecessary call from bibliographic holding availability endpoint

Run Details

6 of 6 new or added lines in 1 file covered. (100.0%)

2 existing lines in 2 files now uncovered.

9019 of 10048 relevant lines covered (89.76%)

344.57 hits per line

Source File
Press 'n' to go to next uncovered line, 'b' for previous

73.5

/app/controllers/bibliographic_controller.rb

class BibliographicController < ApplicationController
  include FormattingConcern

  def adapter
    @adapter ||= AlmaAdapter.new
  end

  def index
    if params[:bib_id]
      if params.fetch(:holdings_only, '0') == '1'
        redirect_to action: :bib_holdings, bib_id: sanitized_bibid, status: :moved_permanently
      elsif params.fetch(:items_only, '0') == '1'
        redirect_to action: :bib_items, bib_id: sanitized_bibid, status: :moved_permanently
      else
        redirect_to action: :bib, bib_id: sanitized_bibid, status: :moved_permanently
      end
    else
      render plain: 'Record please supply a bib id', status: :not_found
    end
  end

  # Returns availability for a single ID
  # Client: This endpoint is used by orangelight to render status on the catalog
  #   show page
  def availability
    id = params[:bib_id]
    availability = adapter.get_availability_one(id:, deep_check: (params[:deep] == 'true'))
    respond_to do |wants|
      wants.json { render json: availability }
    end
  rescue StandardError => e
    handle_alma_exception(exception: e, message: "Failed to retrieve availability for ID: #{id}")
  end

  # Returns availability for multiple IDs
  # Client: This endpoint is used by orangelight to render status on the catalog
  #   search results page
  def availability_many
    ids = (params[:bib_ids] || '').split(',')
    availability = adapter.get_availability_many(ids:, deep_check: ActiveModel::Type::Boolean.new.cast(params[:deep]))
    respond_to do |wants|
      wants.json { render json: availability }
    end
  rescue StandardError => e
    handle_alma_exception(exception: e, message: "Failed to retrieve availability for IDs: #{ids}")
  end

  # Returns availability for a single holding in a bib record
  # Client: This endpoint is used by Requests to populate a request form and
  #   submit requests to the ILS
  def availability_holding
    if params[:bib_id] && params[:holding_id]
      availability = adapter.get_availability_holding(id: params[:bib_id], holding_id: params[:holding_id])
      respond_to do |wants|
        wants.json { render json: availability, status: availability.nil? ? 404 : 200 }
      end
    else
      render plain: 'Please supply a bib id and a holding id', status: :not_found
    end
  rescue StandardError => e
    handle_alma_exception(exception: e, message: "Failed to retrieve holdings for: #{params[:bib_id]}/#{params[:holding_id]}")
  end

  # Client: This endpoint is used by orangelight to present the staff view
  #   and sometimes by individuals to pull records from the ILS
  def bib
    opts = {
      holdings: params.fetch('holdings', 'true') == 'true',
      holdings_in_bib: params.fetch('holdings_in_bib', 'true') == 'true'
    }

    begin
      records = adapter.get_bib_record(sanitized_bibid)
      records.strip_non_numeric! unless opts[:holdings]
    rescue StandardError => e
      return handle_alma_exception(exception: e, message: "Failed to retrieve the record using the bib. ID: #{sanitized_bibid}")
    end

    if records.nil?
      render plain: "Record #{params[:bib_id]} not found or suppressed", status: :not_found
      Rails.logger.error "Record #{params[:bib_id]} not found or suppressed"
    else
      respond_to do |wants|
        wants.json  do
          json = MultiJson.dump(pass_records_through_xml_parser(records))
          render json:
        end
        wants.xml do
          xml = records_to_xml_string(records)
          render xml:
        end
      end
    end
  end

  # Client: Used by firestone_locator to pull bibliographic data
  #   Also used to pull orangelight and pul_solr test fixtures
  def bib_solr
    opts = {
      holdings: params.fetch('holdings', 'true') == 'true',
      holdings_in_bib: params.fetch('holdings_in_bib', 'true') == 'true'
    }
    records = adapter.get_bib_record(sanitized_bibid)
    if records.nil?
      render plain: "Record #{params[:bib_id]} not found or suppressed", status: :not_found
    else
      solr_doc = indexer.map_record(records)
      render json: solr_doc
    end
  rescue StandardError => e
    handle_alma_exception(exception: e, message: "Failed to retrieve the holding records for the bib. ID: #{sanitized_bibid}")
  end

  # Client: No known use cases
  def bib_holdings
    records = adapter.get_holding_records(sanitized_bibid)
    if records.empty?
      render plain: "Record #{params[:bib_id]} not found or suppressed", status: :not_found
    else
      respond_to do |wants|
        wants.json  do
          json = MultiJson.dump(pass_records_through_xml_parser(records))
          render json:
        end
        wants.xml do
          xml = records_to_xml_string(records)
          render xml:
        end
      end
    end
  rescue StandardError => e
    handle_alma_exception(exception: e, message: "Failed to retrieve the holding records for the bib. ID: #{sanitized_bibid}")
  end

  # bibliographic/:bib_id/items
  # Client: Used by figgy to check CDL status. Used by firestone_locator for
  #   call number and location data
  def bib_items
    item_keys = %w[id pid perm_location temp_location]
    holding_summary = adapter.get_items_for_bib(sanitized_bibid).holding_summary(item_key_filter: item_keys)

    respond_to do |wants|
      wants.json  { render json: MultiJson.dump(add_locator_call_no(holding_summary)) }
      wants.xml { render xml: '<todo but="You probably want JSON anyway" />' }
    end
  rescue Alma::BibItemSet::ResponseError
    render_not_found(params[:bib_id])
  rescue StandardError => e
    handle_alma_exception(exception: e, message: "Failed to retrieve items for bib ID: #{sanitized_bibid}")
  end

  private

    def render_not_found(id)
      render plain: "Record #{id} not found or suppressed", status: :not_found
    end

    # Ensure that the client is authenticated and the user is a catalog administrator
    def protect
      if user_signed_in?
        render plain: 'You are unauthorized', status: :forbidden if !current_user.catalog_admin?
      else
        redirect_to user_cas_omniauth_authorize_path
      end
    end

    # Generate the options for retrieving bib. records from Voyager
    # @return [Hash]
    def voyager_opts
      {
        holdings: params.fetch('holdings', 'true') == 'true',
        holdings_in_bib: params.fetch('holdings_in_bib', 'true') == 'true'
      }
    end

    # Access the URL helpers for the application
    # @return [Array<ActionDispatch::Routing::RouteSet::NamedRouteCollection::UrlHelper>]
    def url_helpers
      Rails.application.routes.url_helpers
    end

    # Access the global Traject Object
    # @return [Traject::Indexer::MarcIndexer] the Traject indexer
    def indexer
      TRAJECT_INDEXER
    end

    # Generate the URL for the application root
    # @return [String] the root URL
    def root_url
      url_helpers.root_url(host: request.host_with_port)
    end

    # Generates the URL for the bibliographic record
    # @return [String] the URL
    def bib_id_url
      url_helpers.show_bib_url(params[:bib_id], host: request.host_with_port)
    end

    # Sanitizes the bib_id HTTP parameter
    # @return [String]
    def sanitized_bibid
      CGI.escape(params[:bib_id])
    end

    def add_locator_call_no(records)
      records.each do |location, holdings|
        next unless location == 'firestone$stacks'

        holdings.each do |holding|
          holding['sortable_call_number'] = sortable_call_number(holding['call_number'])
        end
      end
    end

    def sortable_call_number(call_no)
      return call_no unless /^[A-Za-z]/.match?(call_no)

      call_no = make_sortable_call_number(call_no)
      lsort_result = Lcsort.normalize(call_no)
      return lsort_result.gsub('..', '.') unless lsort_result.nil?

      force_number_part_to_have_4_digits(call_no)
    rescue StandardError
      call_no
    end

    def make_sortable_call_number(call_no)
      tokens = call_no.split(' ')
      needs_adjustment = %w[oversize folio].include? tokens.first.downcase
      return call_no unless needs_adjustment

      # Move the first token (e.g. Oversize or Folio) to the end
      (tokens[1..] << tokens[0]).join(' ')
    end

    # This routine adjust something from "A53.blah" to "A0053.blah" for sorting purposes
    #
    def force_number_part_to_have_4_digits(call_no)
      dot_parts = call_no.tr(',', '.').split('.')
      return call_no if dot_parts.count <= 1

      parts = dot_parts[0].scan(/[A-Za-z]+|\d+/)
      parts[1] = parts[1].rjust(4, '0')
      dot_parts[0] = parts.join('.')
      dot_parts.join('.')
    end
end

1	class BibliographicController < ApplicationController	1✔
2	include FormattingConcern	1✔
3
4	def adapter	1✔
5	@adapter \|\|= AlmaAdapter.new	46✔
6	end
7
8	def index	1✔
9	if params[:bib_id]	4✔
10	if params.fetch(:holdings_only, '0') == '1'	3✔
11	redirect_to action: :bib_holdings, bib_id: sanitized_bibid, status: :moved_permanently	1✔
12	elsif params.fetch(:items_only, '0') == '1'	2✔
13	redirect_to action: :bib_items, bib_id: sanitized_bibid, status: :moved_permanently	1✔
14	else
15	redirect_to action: :bib, bib_id: sanitized_bibid, status: :moved_permanently	1✔
16	end
17	else
18	render plain: 'Record please supply a bib id', status: :not_found	1✔
19	end
20	end
21
22	# Returns availability for a single ID
23	# Client: This endpoint is used by orangelight to render status on the catalog
24	# show page
25	def availability	1✔
26	id = params[:bib_id]	2✔
27	availability = adapter.get_availability_one(id:, deep_check: (params[:deep] == 'true'))	2✔
28	respond_to do \|wants\|	1✔
29	wants.json { render json: availability }	2✔
30	end
31	rescue StandardError => e
32	handle_alma_exception(exception: e, message: "Failed to retrieve availability for ID: #{id}")	1✔
33	end
34
35	# Returns availability for multiple IDs
36	# Client: This endpoint is used by orangelight to render status on the catalog
37	# search results page
38	def availability_many	1✔
39	ids = (params[:bib_ids] \|\| '').split(',')	3✔
40	availability = adapter.get_availability_many(ids:, deep_check: ActiveModel::Type::Boolean.new.cast(params[:deep]))	3✔
41	respond_to do \|wants\|	1✔
42	wants.json { render json: availability }	2✔
43	end
44	rescue StandardError => e
45	handle_alma_exception(exception: e, message: "Failed to retrieve availability for IDs: #{ids}")	2✔
46	end
47
48	# Returns availability for a single holding in a bib record
49	# Client: This endpoint is used by Requests to populate a request form and
50	# submit requests to the ILS
51	def availability_holding	1✔
52	if params[:bib_id] && params[:holding_id]	6✔
53	availability = adapter.get_availability_holding(id: params[:bib_id], holding_id: params[:holding_id])	6✔
54	respond_to do \|wants\|	6✔
55	wants.json { render json: availability, status: availability.nil? ? 404 : 200 }	12✔
56	end
57	else
58	render plain: 'Please supply a bib id and a holding id', status: :not_found	×
59	end
60	rescue StandardError => e
UNCOV 61	handle_alma_exception(exception: e, message: "Failed to retrieve holdings for: #{params[:bib_id]}/#{params[:holding_id]}")	×
62	end
63
64	# Client: This endpoint is used by orangelight to present the staff view
65	# and sometimes by individuals to pull records from the ILS
66	def bib	1✔
67	opts = {
68	holdings: params.fetch('holdings', 'true') == 'true',	13✔
69	holdings_in_bib: params.fetch('holdings_in_bib', 'true') == 'true'
70	}
71
72	begin
73	records = adapter.get_bib_record(sanitized_bibid)	13✔
74	records.strip_non_numeric! unless opts[:holdings]	2✔
75	rescue StandardError => e
76	return handle_alma_exception(exception: e, message: "Failed to retrieve the record using the bib. ID: #{sanitized_bibid}")	2✔
77	end
78
79	if records.nil?	2✔
80	render plain: "Record #{params[:bib_id]} not found or suppressed", status: :not_found	×
81	Rails.logger.error "Record #{params[:bib_id]} not found or suppressed"	×
82	else
83	respond_to do \|wants\|	2✔
84	wants.json do	2✔
85	json = MultiJson.dump(pass_records_through_xml_parser(records))	×
86	render json:	×
87	end
88	wants.xml do	2✔
89	xml = records_to_xml_string(records)	2✔
90	render xml:	2✔
91	end
92	end
93	end
94	end
95
96	# Client: Used by firestone_locator to pull bibliographic data
97	# Also used to pull orangelight and pul_solr test fixtures
98	def bib_solr	1✔
99	opts = {
100	holdings: params.fetch('holdings', 'true') == 'true',	3✔
101	holdings_in_bib: params.fetch('holdings_in_bib', 'true') == 'true'
102	}
103	records = adapter.get_bib_record(sanitized_bibid)	3✔
104	if records.nil?	×
105	render plain: "Record #{params[:bib_id]} not found or suppressed", status: :not_found	×
106	else
107	solr_doc = indexer.map_record(records)	×
108	render json: solr_doc	×
109	end
110	rescue StandardError => e
111	handle_alma_exception(exception: e, message: "Failed to retrieve the holding records for the bib. ID: #{sanitized_bibid}")	×
112	end
113
114	# Client: No known use cases
115	def bib_holdings	1✔
116	records = adapter.get_holding_records(sanitized_bibid)	7✔
117	if records.empty?	1✔
118	render plain: "Record #{params[:bib_id]} not found or suppressed", status: :not_found	×
119	else
120	respond_to do \|wants\|	1✔
121	wants.json do	1✔
122	json = MultiJson.dump(pass_records_through_xml_parser(records))	×
123	render json:	×
124	end
125	wants.xml do	1✔
126	xml = records_to_xml_string(records)	1✔
127	render xml:	1✔
128	end
129	end
130	end
131	rescue StandardError => e
132	handle_alma_exception(exception: e, message: "Failed to retrieve the holding records for the bib. ID: #{sanitized_bibid}")	3✔
133	end
134
135	# bibliographic/:bib_id/items
136	# Client: Used by figgy to check CDL status. Used by firestone_locator for
137	# call number and location data
138	def bib_items	1✔
139	item_keys = %w[id pid perm_location temp_location]	12✔
140	holding_summary = adapter.get_items_for_bib(sanitized_bibid).holding_summary(item_key_filter: item_keys)	12✔
141
142	respond_to do \|wants\|	6✔
143	wants.json { render json: MultiJson.dump(add_locator_call_no(holding_summary)) }	12✔
144	wants.xml { render xml: '<todo but="You probably want JSON anyway" />' }	6✔
145	end
146	rescue Alma::BibItemSet::ResponseError
147	render_not_found(params[:bib_id])	2✔
148	rescue StandardError => e
149	handle_alma_exception(exception: e, message: "Failed to retrieve items for bib ID: #{sanitized_bibid}")	×
150	end
151
152	private	1✔
153
154	def render_not_found(id)	1✔
155	render plain: "Record #{id} not found or suppressed", status: :not_found	2✔
156	end
157
158	# Ensure that the client is authenticated and the user is a catalog administrator
159	def protect	1✔
160	if user_signed_in?	×
161	render plain: 'You are unauthorized', status: :forbidden if !current_user.catalog_admin?	×
162	else
163	redirect_to user_cas_omniauth_authorize_path	×
164	end
165	end
166
167	# Generate the options for retrieving bib. records from Voyager
168	# @return [Hash]
169	def voyager_opts	1✔
170	{
171	holdings: params.fetch('holdings', 'true') == 'true',	×
172	holdings_in_bib: params.fetch('holdings_in_bib', 'true') == 'true'
173	}
174	end
175
176	# Access the URL helpers for the application
177	# @return [Array<ActionDispatch::Routing::RouteSet::NamedRouteCollection::UrlHelper>]
178	def url_helpers	1✔
179	Rails.application.routes.url_helpers	×
180	end
181
182	# Access the global Traject Object
183	# @return [Traject::Indexer::MarcIndexer] the Traject indexer
184	def indexer	1✔
185	TRAJECT_INDEXER	×
186	end
187
188	# Generate the URL for the application root
189	# @return [String] the root URL
190	def root_url	1✔
191	url_helpers.root_url(host: request.host_with_port)	×
192	end
193
194	# Generates the URL for the bibliographic record
195	# @return [String] the URL
196	def bib_id_url	1✔
197	url_helpers.show_bib_url(params[:bib_id], host: request.host_with_port)	×
198	end
199
200	# Sanitizes the bib_id HTTP parameter
201	# @return [String]
202	def sanitized_bibid	1✔
203	CGI.escape(params[:bib_id])	43✔
204	end
205
206	def add_locator_call_no(records)	1✔
207	records.each do \|location, holdings\|	6✔
208	next unless location == 'firestone$stacks'	9✔
209
210	holdings.each do \|holding\|	2✔
211	holding['sortable_call_number'] = sortable_call_number(holding['call_number'])	2✔
212	end
213	end
214	end
215
216	def sortable_call_number(call_no)	1✔
217	return call_no unless /^[A-Za-z]/.match?(call_no)	2✔
218
219	call_no = make_sortable_call_number(call_no)	2✔
220	lsort_result = Lcsort.normalize(call_no)	2✔
221	return lsort_result.gsub('..', '.') unless lsort_result.nil?	2✔
222
223	force_number_part_to_have_4_digits(call_no)	×
224	rescue StandardError
225	call_no	×
226	end
227
228	def make_sortable_call_number(call_no)	1✔
229	tokens = call_no.split(' ')	2✔
230	needs_adjustment = %w[oversize folio].include? tokens.first.downcase	2✔
231	return call_no unless needs_adjustment	2✔
232
233	# Move the first token (e.g. Oversize or Folio) to the end
234	(tokens[1..] << tokens[0]).join(' ')	1✔
235	end
236
237	# This routine adjust something from "A53.blah" to "A0053.blah" for sorting purposes
238	#
239	def force_number_part_to_have_4_digits(call_no)	1✔
240	dot_parts = call_no.tr(',', '.').split('.')	×
241	return call_no if dot_parts.count <= 1	×
242
243	parts = dot_parts[0].scan(/[A-Za-z]+\|\d+/)	×
244	parts[1] = parts[1].rjust(4, '0')	×
245	dot_parts[0] = parts.join('.')	×
246	dot_parts.join('.')	×
247	end
248	end

pulibrary / bibdata / 31a2a5a6-71e0-4c97-89ac-edd0d51725ca

Source File Press 'n' to go to next uncovered line, 'b' for previous

Source File
Press 'n' to go to next uncovered line, 'b' for previous