Refactor to extract class methods into instance methods (#2735)

* Refactor to extract class methods into instance methods

This will make it easier to find the next refactor steps.
Co-authored-by: Anna Headley <anna.headley@gmail.com>
Co-authored-by: Eliot Jordan <eliot.jordan@gmail.com>
Co-authored-by: Trey Pendraon <tpendragon@princeton.edu>

* Update test to use instance methods

app/services/physical_holdings_markup_builder.rb

@@ -5,12 +5,16 @@ class PhysicalHoldingsMarkupBuilder < HoldingRequestsBuilder
 
   # Generate <span> markup used in links for browsing by call numbers
   # @return [String] the markup
-  def self.call_number_span
+  def call_number_span
     %(<span class="link-text">#{I18n.t('blacklight.holdings.browse')}</span>\
       <span class="icon-bookslibrary"></span>)
   end
 
-  def self.call_number_link(holding, cn_value)
+  ##
+  # Add call number link
+  # @param [Hash] holding
+  # @param [String] cn_value - a call number
+  def call_number_link(holding, cn_value)
     cn = ''
     unless cn_value.nil?
       children = call_number_span
@@ -25,7 +29,7 @@ def self.call_number_link(holding, cn_value)
     content_tag(:td, cn.html_safe, class: 'holding-call-number')
   end
 
-  def self.holding_location_repository
+  def holding_location_repository
     children = content_tag(:span,
                            'On-site access',
                            class: 'availability-icon badge badge-success',
@@ -34,15 +38,15 @@ def self.holding_location_repository
     content_tag(:td, children.html_safe)
   end
 
-  def self.holding_location_scsb_span
+  def holding_location_scsb_span
     markup = content_tag(:span, '',
                          title: '',
                          class: 'availability-icon badge',
                          data: { toggle: 'tooltip' })
     markup
   end
 
-  def self.holding_location_scsb(holding, bib_id, holding_id)
+  def holding_location_scsb(holding, bib_id, holding_id)
     content_tag(:td, holding_location_scsb_span.html_safe,
                 class: 'holding-status',
                 data: {
@@ -54,7 +58,7 @@ def self.holding_location_scsb(holding, bib_id, holding_id)
                 })
   end
 
-  def self.holding_location_default(bib_id, holding_id, location_rules)
+  def holding_location_default(bib_id, holding_id, location_rules)
     children = content_tag(:span, '', class: 'availability-icon')
     content_tag(:td,
                 children.html_safe,
@@ -63,12 +67,12 @@ def self.holding_location_default(bib_id, holding_id, location_rules)
                   'availability_record' => true,
                   'record_id' => bib_id,
                   'holding_id' => holding_id,
-                  aeon: aeon_location?(location_rules)
+                  aeon: self.class.aeon_location?(location_rules)
                 })
   end
 
   # For when a holding record has a value for the "dspace" key, but it is set to false
-  def self.holding_location_unavailable
+  def holding_location_unavailable
     children = content_tag(:span,
                            'Unavailable',
                            class: 'availability-icon badge badge-danger',
@@ -169,6 +173,10 @@ def self.aeon_location?(location)
     location.nil? ? false : location[:aeon_location]
   end
 
+  def aeon_location?(location)
+    self.class.aeon_location?(location)
+  end
+
   def self.scsb_location?(location)
     location.nil? ? false : /^scsb.+/ =~ location['code']
   end
@@ -218,7 +226,7 @@ def self.location_services_block(adapter, holding_id, location_rules, link, hold
   # Generate a request label based upon the holding location
   # @param location_rules [Hash] the location for the holding
   # @return [String] the label
-  def self.request_label(location_rules)
+  def request_label(location_rules)
     if aeon_location?(location_rules)
       'Reading Room Request'
     else
@@ -256,6 +264,10 @@ def self.scsb_supervised_items?(holding)
     end
   end
 
+  def scsb_supervised_items?(holding)
+    self.class.scsb_supervised_items?(holding)
+  end
+
   ##
   def self.listify_array(arr)
     arr = arr.map do |e|
@@ -265,7 +277,8 @@ def self.listify_array(arr)
   end
 
   # Generate the links for a given holding
-  def self.request_placeholder(adapter, holding_id, location_rules, holding)
+  # TODO: Come back and remove class method calls
+  def request_placeholder(adapter, holding_id, location_rules, holding)
     doc_id = adapter.doc_id
     link = if !location_rules.nil? && /^scsb.+/ =~ location_rules['code']
              if scsb_supervised_items?(holding)
@@ -277,7 +290,7 @@ def self.request_placeholder(adapter, holding_id, location_rules, holding)
              else
                link_to(request_label(location_rules),
                        "/requests/#{doc_id}",
-                       title: request_tooltip(location_rules),
+                       title: self.class.request_tooltip(location_rules),
                        class: 'request btn btn-xs btn-primary',
                        data: { toggle: 'tooltip' })
              end
@@ -290,11 +303,11 @@ def self.request_placeholder(adapter, holding_id, location_rules, holding)
            else
              link_to(request_label(location_rules),
                      "/requests/#{doc_id}?mfhd=#{holding_id}",
-                     title: request_tooltip(location_rules),
+                     title: self.class.request_tooltip(location_rules),
                      class: 'request btn btn-xs btn-primary',
                      data: { toggle: 'tooltip' })
            end
-    markup = location_services_block(adapter, holding_id, location_rules, link, holding)
+    markup = self.class.location_services_block(adapter, holding_id, location_rules, link, holding)
     markup
   end
 
@@ -403,22 +416,21 @@ def process_physical_holding(holding, holding_id)
           cn_value
         )
       end
-      markup << self.class.call_number_link(holding, cn_value)
+      markup << call_number_link(holding, cn_value)
       markup << if @adapter.repository_holding?(holding)
-                  self.class.holding_location_repository
+                  holding_location_repository
                 elsif @adapter.scsb_holding?(holding) && !@adapter.empty_holding?(holding)
-                  self.class.holding_location_scsb(holding, bib_id, holding_id)
+                  holding_location_scsb(holding, bib_id, holding_id)
                 # dspace: false
                 elsif @adapter.unavailable_holding?(holding)
-                  self.class.holding_location_unavailable
+                  holding_location_unavailable
                 else
-                  self.class.holding_location_default(bib_id,
-                                                      holding_id,
-                                                      location_rules)
+                  holding_location_default(bib_id,
+                                           holding_id,
+                                           location_rules)
                 end
 
-      request_placeholder_markup = \
-        self.class.request_placeholder(@adapter, holding_id, location_rules, holding)
+      request_placeholder_markup = request_placeholder(@adapter, holding_id, location_rules, holding)
       markup << request_placeholder_markup.html_safe
 
       holding_notes = ''

spec/services/physical_holdings_markup_builder_spec.rb

@@ -59,7 +59,7 @@
   end
 
   describe '.request_label' do
-    let(:request_label) { described_class.request_label(location_rules) }
+    let(:request_label) { builder.request_label(location_rules) }
 
     context 'for holdings within aeon locations' do
       let(:location_rules) do
@@ -210,7 +210,7 @@
   end
 
   describe '.request_placeholder' do
-    let(:request_placeholder_markup) { described_class.request_placeholder(adapter, holding_id, location_rules, holding) }
+    let(:request_placeholder_markup) { builder.request_placeholder(adapter, holding_id, location_rules, holding) }
 
     it 'generates the markup for request links' do
       expect(request_placeholder_markup).to include '<td class="location-services service-conditional"'