refactors loading of checkout history report

Gemfile

@@ -24,7 +24,7 @@ gem "yabeda-prometheus"
 
 gem "alma_rest_client",
   git: "https://github.com/mlibrary/alma_rest_client",
-  tag: "1.3.1"
+  tag: "v2.0.0"
 
 group :development, :test do
   # Call 'byebug' anywhere in the code to stop execution and get a debugger console

Gemfile.lock

@@ -1,11 +1,13 @@
 GIT
   remote: https://github.com/mlibrary/alma_rest_client
-  revision: 2014809367ece3f21935bd24b45a12285f61e253
-  tag: 1.3.1
+  revision: 9606225d82480b6d1568902813ae9018dd8c1acc
+  tag: v2.0.0
   specs:
-    alma_rest_client (1.3.1)
+    alma_rest_client (2.0.0)
       activesupport (~> 7.0, >= 4.2)
-      httparty
+      faraday
+      faraday-retry
+      httpx
       rexml
 
 GEM
@@ -101,7 +103,6 @@ GEM
       bigdecimal
       rexml
     crass (1.0.6)
-    csv (3.3.2)
     database_cleaner (2.1.0)
       database_cleaner-active_record (>= 2, < 3)
     database_cleaner-active_record (2.2.0)
@@ -121,14 +122,21 @@ GEM
       railties (>= 5.0.0)
     faker (3.5.1)
       i18n (>= 1.8.11, < 2)
+    faraday (2.12.2)
+      faraday-net_http (>= 2.0, < 3.5)
+      json
+      logger
+    faraday-net_http (3.4.0)
+      net-http (>= 0.5.0)
+    faraday-retry (2.2.1)
+      faraday (~> 2.0)
     ffi (1.17.0-x86_64-linux-gnu)
     globalid (1.2.1)
       activesupport (>= 6.1)
     hashdiff (1.1.2)
-    httparty (0.22.0)
-      csv
-      mini_mime (>= 1.0.0)
-      multi_xml (>= 0.5.2)
+    http-2 (1.0.2)
+    httpx (1.4.0)
+      http-2 (>= 1.0.0)
     i18n (1.14.6)
       concurrent-ruby (~> 1.0)
     io-console (0.8.0)
@@ -157,9 +165,9 @@ GEM
     mini_mime (1.1.5)
     minitest (5.25.4)
     msgpack (1.7.5)
-    multi_xml (0.7.1)
-      bigdecimal (~> 3.1)
     mysql2 (0.5.6)
+    net-http (0.6.0)
+      uri
     net-imap (0.5.4)
       date
       net-protocol
@@ -301,6 +309,7 @@ GEM
     unicode-display_width (3.1.2)
       unicode-emoji (~> 4.0, >= 4.0.4)
     unicode-emoji (4.0.4)
+    uri (1.0.2)
     useragent (0.16.11)
     webmock (3.24.0)
       addressable (>= 2.8.0)

app/utils/checkout_history_loader.rb

@@ -0,0 +1,5 @@
+module CheckoutHistoryLoader
+  class LoanLoadError < StandardError; end
+
+  class FetchReportError < StandardError; end
+end

app/utils/checkout_history_loader/loan_loader.rb

@@ -0,0 +1,48 @@
+module CheckoutHistoryLoader
+  class LoanLoader
+    def initialize(report_item:, user:, loan:)
+      @report_item = report_item
+      @user = user
+      @loan = loan
+    end
+
+    def has_identical_checkout_date?
+      @loan.checkout_date&.to_date&.to_fs(:db) == @report_item.checkout_date
+    end
+
+    def load
+      return if @loan.return_date.present?
+      return if @report_item.return_date.blank?
+
+      @loan.tap do |l|
+        l.user = @user
+        l.id = @report_item.id
+        l.title = @report_item.title
+        l.author = @report_item.author
+        l.mms_id = @report_item.mms_id
+        l.return_date = @report_item.return_date
+        l.checkout_date = @report_item.checkout_date
+        l.barcode = @report_item.barcode
+        l.call_number = @report_item.call_number
+        l.description = @report_item.description
+      end
+
+      if @loan.save
+        Rails.logger.info("item_loan '#{@loan.id}' saved")
+      else
+        Rails.logger.error("item_loan '#{@loan.id}' not saved: #{@loan.errors.full_messages}")
+        raise LoanLoadError, @loan.errors.full_messages
+      end
+    end
+
+    def self.load(report_item)
+      user = User.find_or_create_by_uniqname(report_item.uniqname)
+      unless user.retain_history
+        Rails.logger.warn("item_loan '#{report_item.id}' not saved: patron opt out")
+        return
+      end
+      loan = Loan.find_or_create_by(id: report_item.id)
+      new(report_item: report_item, user: user, loan: loan).load
+    end
+  end
+end

app/utils/checkout_history_loader/report.rb

@@ -0,0 +1,28 @@
+module CheckoutHistoryLoader
+  class Report
+    include Enumerable
+
+    def initialize(items)
+      @items = items
+    end
+
+    def each(&block)
+      @items.each(&block)
+    end
+
+    def self.for_rows(rows)
+      new(rows.map { |x| ReportItem.new(x) })
+    end
+
+    def self.fetch
+      rows = []
+      response = AlmaRestClient.client.get_report(path: ENV.fetch("CIRC_REPORT_PATH")) do |row|
+        rows.push ReportItem.new(row)
+      end
+      if response.status != 200
+        raise FetchReportError, response.body
+      end
+      new(rows)
+    end
+  end
+end

app/utils/checkout_history_loader/report_item.rb

@@ -0,0 +1,47 @@
+module CheckoutHistoryLoader
+  class ReportItem
+    def initialize(row)
+      @row = row
+    end
+
+    def uniqname
+      @row["User Primary Identifier"]
+    end
+
+    def id
+      @row["Item Loan Id"]
+    end
+
+    def title
+      @row["Title"]&.slice(0, 255)
+    end
+
+    def author
+      @row["Author"]&.slice(0, 255)
+    end
+
+    def mms_id
+      @row["MMS Id"]
+    end
+
+    def checkout_date
+      @row["Loan Date"]
+    end
+
+    def return_date
+      @row["Return Date"]
+    end
+
+    def barcode
+      @row["Barcode"]
+    end
+
+    def call_number
+      @row["Call Number"]
+    end
+
+    def description
+      @row["Description"]
+    end
+  end
+end

lib/tasks/alma_circ_history.rake

@@ -6,41 +6,27 @@ namespace :alma_circ_history do
     Rails.logger.tagged("Circ Load") do
       Rails.logger.info("Started")
       Yabeda.checkout_history_load_last_success.set({}, Time.now.to_i)
-      client = AlmaRestClient.client
-      response = client.get_report(path: ENV.fetch("CIRC_REPORT_PATH"))
-      if response.code != 200
-        Rails.logger.error("Alma Report Failed to Load")
-        next
+      # client = AlmaRestClient.client
+      # response = client.get_report(path: ENV.fetch("CIRC_REPORT_PATH"))
+      # if response.code != 200
+      # Rails.logger.error("Alma Report Failed to Load")
+      # next
+      # end
+      # report_items = CheckoutHistoryLoader::Report.for_rows(response.parsed_response)
+      begin
+        report_items = CheckoutHistoryLoader::Report.fetch
+      rescue CheckoutHistoryLoader::FetchReportError => e
+        Rails.logger.error("Alma Report Failed to Load: #{e}")
+        exit
       end
-      summary[:active_loans] = response.parsed_response.count
-      response.parsed_response.each do |row|
-        u = User.find_or_create_by_uniqname(row["User Primary Identifier"])
-        unless u.retain_history
-          Rails.logger.warn("item_loan '#{row["Item Loan Id"]}' not saved: patron opt out")
+      report_items.each do |report_item|
+        summary[:active_loans] += 1
+        begin
+          CheckoutHistoryLoader::LoanLoader.load(report_item)
+        rescue CheckoutHistoryLoader::LoanLoadError
           next
         end
-        loan = Loan.find_or_create_by(id: row["Item Loan Id"])
-        next if loan.return_date.present?
-        next if loan.checkout_date&.to_date&.to_fs(:db) == row["Loan Date"] && row["Return Date"].blank?
-        # mrio: using `tap` so I can use block syntax
-        loan.tap do |l|
-          l.user = u
-          l.id = row["Item Loan Id"]
-          l.title = row["Title"]&.slice(0, 255)
-          l.author = row["Author"]&.slice(0, 255)
-          l.mms_id = row["MMS Id"]
-          l.return_date = row["Return Date"]
-          l.checkout_date = row["Loan Date"]
-          l.barcode = row["Barcode"]
-          l.call_number = row["Call Number"]
-          l.description = row["Description"]
-        end
-        if loan.save
-          Rails.logger.info("item_loan '#{loan.id}' saved")
-          summary[:loans_loaded] = summary[:loans_loaded] + 1
-        else
-          Rails.logger.warn("item_loan '#{loan.id}' not saved: #{loan.errors.full_messages}")
-        end
+        summary[:loans_loaded] += 1
       end
       Rails.logger.info("Finished")
       Rails.logger.info("Summary: #{summary}")
@@ -57,11 +43,11 @@ namespace :alma_circ_history do
       Rails.logger.info("Started")
       client = AlmaRestClient.client
       response = client.get_report(path: ENV.fetch("PATRON_REPORT_PATH"))
-      if response.code != 200
+      if response.status != 200
         Rails.logger.error("Alma Report Failed to Load")
         next
       end
-      non_expired_users = response.parsed_response.map { |row| row["User Primary Identifier"].downcase }
+      non_expired_users = response.body.map { |row| row["User Primary Identifier"].downcase }
       User.all.each do |user|
         uniqname = user.uniqname
         if non_expired_users.include?(uniqname)

spec/spec_helper.rb

@@ -13,7 +13,10 @@
 # it.
 #
 # See http://rubydoc.info/gems/rspec-core/RSpec/Core/Configuration
+require "webmock"
+require "alma_rest_client"
 RSpec.configure do |config|
+  include AlmaRestClient::Test::Helpers
   # rspec-expectations config goes here. You can use an alternate
   # assertion/expectation library such as wrong or the stdlib/minitest
   # assertions if you prefer.
@@ -92,14 +95,3 @@
   #   # as the one that triggered the failure.
   #   Kernel.srand config.seed
 end
-def stub_alma_get_request(url:, body: "{}", status: 200, query: {})
-  stub_request(:get, "#{ENV["ALMA_API_HOST"]}/almaws/v1/#{url}").with(
-    headers: {
-      :accept => "application/json",
-      :Authorization => "apikey #{ENV["ALMA_API_KEY"]}",
-      "Accept-Encoding" => "gzip;q=1.0,deflate;q=0.6,identity;q=0.3",
-      "User-Agent" => "Ruby"
-    },
-    query: query
-  ).to_return(body: body, status: status, headers: {content_type: "application/json"})
-end

spec/tasks/alma_circ_history_task_spec.rb

@@ -5,7 +5,7 @@
     @pushgateway_stub = stub_request(:post, "#{ENV["PROMETHEUS_PUSH_GATEWAY"]}/metrics/job/checkout_history")
     @stub = stub_alma_get_request(url: "analytics/reports",
       query: {path: ENV.fetch("CIRC_REPORT_PATH"), col_names: true, limit: 1000},
-      body: File.read("./spec/fixtures/circ_history.json"))
+      output: File.read("./spec/fixtures/circ_history.json"))
   end
   after(:each) do
     Rake::Task["alma_circ_history:load"].reenable
@@ -22,8 +22,8 @@
   it "logs an error if it can't load the report" do
     @stub.to_return(body: File.read("./spec/fixtures/alma_error.json"), status: 500, headers: {content_type: "application/json"})
     @stub.response # clear out original response
-    expect(Rails.logger).to receive(:error).with("Alma Report Failed to Load")
-    load_circ_history
+    expect(Rails.logger).to receive(:error).with(/Alma Report Failed to Load/)
+    expect { load_circ_history }.to raise_error SystemExit
   end
   it "loads new circ history into the db and downcases uniqnames" do
     user_ajones
@@ -144,7 +144,7 @@
   before(:each) do
     @stub = stub_alma_get_request(url: "analytics/reports",
       query: {path: ENV.fetch("PATRON_REPORT_PATH"), col_names: true, limit: 1000},
-      body: File.read("./spec/fixtures/non_expired_patrons.json"))
+      output: File.read("./spec/fixtures/non_expired_patrons.json"))
   end
   after(:each) do
     Rake::Task["alma_circ_history:purge"].reenable