Merge changes for v2.3.4 (#120)

* DACCESS-381: fix punctuation on login page

* DACCESS-257: Fix bug preventing ILL requests from appearing (#118)

* DACCESS-459: Fix bug breaking ReShare lookups (#117)

* Update call to cul-folio-edge authenticate method to specify old auth method (DACCESS-459)

* Update release notes

* Require new version of cul-folio-edge

* Use ILLiad API to retrieve tranactions (DACCESS-257, DACCESS-460)

* Do things the Ruby way

* Comment out things not being used

* Update release notes

* Formatting and docs

---------

Co-authored-by: Melissa Wallace <[email protected]>

* Add filter for ILL transactions (#119)

* DACCESS-459: Fix bug breaking ReShare lookups (#117)

* Update call to cul-folio-edge authenticate method to specify old auth method (DACCESS-459)

* Update release notes

* Require new version of cul-folio-edge

* Use ILLiad API to retrieve tranactions (DACCESS-257, DACCESS-460)

* Do things the Ruby way

* Comment out things not being used

* Update release notes

* Formatting and docs

* Add ILL transaction filter; update release notes

* Fix merge conflict

---------

Co-authored-by: Melissa Wallace <[email protected]>

---------

Co-authored-by: Melissa Wallace <[email protected]>

Author: Baroquem

app/controllers/my_account/account_controller.rb

@@ -8,6 +8,7 @@
 module MyAccount
   class AccountController < ApplicationController
     #include Reshare
+    include ILL
 
     before_action :heading
     before_action :authenticate_user, except: [:intro]
@@ -149,17 +150,12 @@ def ajax_cancel
     # parsing below greatly. No need to try to figure out whether something is a charged item or a request
     def get_illiad_data
       record = nil
-
       netid = params['netid']
-      # folio_account_data = get_folio_accountinfo netid
-      # Rails.logger.debug "mjc12test: Start parsing"
 
       begin
-        # NOTE: the key MY_ACCOUNT_ISLAPI_URL is a misomer now, because we've eliminated the ilsapi CGI
-        # script and are using the illiad6.cgi script, which ilsapi called, without the middleman. Probably
-        # the key should be renamed at some point.
-        response = RestClient.get "#{ENV['MY_ACCOUNT_ILSAPI_URL']}?netid=#{netid}&fmt=json&wrapper=n"
-        record = JSON.parse response.body
+        # Now using the ILLiad API to retrieve ILL transactions
+        transactions = ill_transactions(netid)
+        items = JSON.parse(transactions)['items']
       rescue => error
         Rails.logger.error "MyAccount error: Could not find a patron entry for #{netid}"
         msg = "We're sorry, but we could not access your account. For help, please email <a href='mailto:[email protected]'>[email protected]</a>"
@@ -171,7 +167,8 @@ def get_illiad_data
       available_requests = []
 
       # Parse the results of the ilsapiE/ILLiad lookup. Loans (from FOLIO) are handled separately
-      record['items'].each do |i|
+      #record['items'].each do |i|
+      items.each do |i|
         # TODO: items returned with a status of 'finef' appear to be duplicates, only
         # indicating that a fine or fee is applied to that item. So we don't need them
         # in this list? But maybe check the fine-related functions below to see if we
@@ -183,9 +180,6 @@ def get_illiad_data
         # show up twice in the user's requests (BD/ReShare and ILL)
         next if i['TransactionStatus'] == 'Request Sent to BD'
 
-        # Skip if this is a ReShare submission that has been rerouted to BD -- otherwise it will
-        # show up twice in the user's requests (BD/ReShare and ILL)
-        next if i['TransactionStatus'] == 'Request Sent to BD'
         # Slight hack: items with the status Checked out in FOLIO are actually waiting to be
         # picked up. This is confusing, because 'checked out' implies that the user
         # has it, but that isn't the case! These should be covered by the patron account 'holds'

app/views/my_account/account/intro.html.haml

@@ -9,10 +9,8 @@
   Email your 
   =link_to('library', 'https://library.cornell.edu/libraries')
   or 
-  =link_to('[email protected]', 'mailto:[email protected]')
-  \.
+  =link_to('[email protected]', 'mailto:[email protected]') + '.'
 
 %p
   Visit the library website for 
-  =link_to('fine information', 'https://www.library.cornell.edu/collections/borrow-renew-return/policies/#fines')
-  \.
+  = link_to('fine information', 'https://www.library.cornell.edu/collections/borrow-renew-return/policies/#fines') + '.'

lib/ill.rb

@@ -0,0 +1,130 @@
+# This module is an interface for ILLiad. Its primary function is to retrieve an array
+# of ILL transactions for a given user, making use of the ILLiad API. The results are
+# filtered and transformed into a JSON object that can be used by the My Account system.
+module ILL
+  require 'rest-client'
+  
+  def ill_transactions(user_id)
+    transactions = fetch_ill_transactions(user_id)
+    # NOTE: The following line should be redundant, but it's here for now, just in case. (See notes in fetch_ill_transactions)
+    transactions = filter_by_status(transactions)
+    transform_fields(transactions)
+  end
+
+  private
+
+  # Fetches ILL (Interlibrary Loan) transactions for a given user via the ILLiad API.
+  #
+  # @param user_id [String] the ID of the user whose transactions are being fetched (netid or guest id)
+  # @return [Array<Hash>] an array of objects representing the user's ILL transactions.
+  # @raise [RestClient::ExceptionWithResponse] if the request to the ILLiad API fails.
+  #
+  # The method sends a GET request to the ILLiad API to retrieve the user's ILL transactions.
+  # It expects the API key and URL to be set in the environment variables 'MY_ACCOUNT_ILLIAD_API_KEY' and 'MY_ACCOUNT_ILLIAD_API_URL'.
+  # A filter is used in the query to try to only retrieve active transactions.
+  def fetch_ill_transactions(user_id)
+    headers = {
+      'Accept' => 'application/json',
+      'APIKey' => ENV['MY_ACCOUNT_ILLIAD_API_KEY']
+    }
+    # The filter is used here because I'm concerned about some users having massive numbers of old, completed transactions. I don't know if this
+    # will speed up or slow down the query. In case this doesn't work properly, the filter_by_status method is called again after the query. The three
+    # values used to catch completed transactions are the ones used in the old illid6.cgi script.
+    filter = "not (TransactionStatus eq 'Request Finished' or TransactionStatus eq 'Cancelled by ILL Staff' or TransactionStatus eq 'Cancelled by Customer')"
+    response = RestClient.get "#{ENV['MY_ACCOUNT_ILLIAD_API_URL']}/Transaction/UserRequests/#{user_id}?$filter=#{filter}", headers
+    transactions = JSON.parse(response.body)
+    transactions
+  end
+
+  # Remove transactions that are completed or cancelled
+  def filter_by_status(transactions)
+    transactions.select do |transaction|
+      !['Request Finished', 'Cancelled by ILL Staff', 'Cancelled by Customer'].include?(transaction['TransactionStatus'])
+    end
+  end
+
+  # Transform the transaction object. The replacement fields are primarily used for display in the My Account requests views.
+  # This method is a near-direct port of the illiad6.cgi Perl script that was previously used as a data source for ILLiad.
+  # illiad6.cgi directly queried the ILLiad database, then added these fields to the results before passing
+  # the whole thing back as a response. Why do we have ii, ou_genre, etc. as fields? Do we still need them
+  # to be this obscure? Can we make them more readable? These are questions to ponder in the future.
+  def transform_fields(transactions)
+    items_array = transactions.map do |transaction|
+      genre = transaction['LoanTitle'] ? 'book' : 'article'
+      title = transaction['LoanTitle'] || transaction['PhotoArticleTitle']
+      title.gsub!(/"/, ' ')
+      spot = title.index('/')
+      title = title[0...spot - 1] if spot
+
+      if genre == 'article'
+        author_last_name = transaction['PhotoArticleAuthor']
+        volume = transaction['PhotoJournalVolume']
+        issn = transaction['ISSN']
+        issue = transaction['PhotoJournalIssue']
+        year = transaction['PhotoJournalYear']
+        pages = transaction['PhotoJournalInclusivePages']
+        full_title = "#{transaction['PhotoJournalTitle']} #{transaction['PhotoArticleTitle']} / #{transaction['PhotoArticleAuthor']} v.#{transaction['PhotoJournalVolume']} # #{transaction['PhotoJournalIssue']} pp. #{transaction['PhotoJournalInclusivePages']} #{transaction['PhotoJournalYear']}"
+      else
+        author_last_name = transaction['LoanAuthor']
+        year = transaction['LoanDate']
+        isbn = transaction['ISSN']
+        spot = transaction['LoanTitle'].index('/')
+        transaction['LoanTitle'] = transaction['LoanTitle'][0...spot - 1] if spot
+        full_title = "#{transaction['LoanTitle']} / #{transaction['LoanAuthor']}"
+      end
+
+      #full_title.gsub!(/[\012\015]/, ' ').gsub!(/"/, ' ').gsub!(/"/, "'")
+      location = transaction['NVTGC']
+      due_date = transaction['DueDate']
+      original_due_date = transaction['DueDate']
+      # renewals_allowed = transaction['RenewalsAllowed']
+      # date = due_date
+
+      status = transaction['TransactionStatus']
+      case status
+      when 'Checked Out to Customer', 'Renewal Requested by Customer'
+        status = "chrged"
+        url = "https://cornell.hosts.atlas-sys.com/illiad/illiad.dll?Action=10&Form=66&Value=#{transaction['TransactionNumber']}"
+      when 'Awaiting Verisign Payment'
+        status = "waiting"
+        url = "https://cornell.hosts.atlas-sys.com/illiad/illiad.dll?Action=10&Form=62"
+      when 'Delivered to Web'
+        status = "waiting"
+        url = "https://cornell.hosts.atlas-sys.com/illiad/illiad.dll?Action=10&Form=75&Value=#{transaction['TransactionNumber']}"
+      when 'Customer Notified via E-Mail'
+        status = "waiting"
+      else
+        status = "pahr"
+        url = "https://cornell.hosts.atlas-sys.com/illiad/illiad.dll?Action=10&Form=63&Value=#{transaction['TransactionNumber']}"
+      end
+
+      # TODO: Review these fields ... some of them are probably unnecessary.
+      {
+        system: "illiad",
+        status: status,
+        ii: transaction['TransactionNumber'],
+        it: transaction['DocumentType'],
+        tl: full_title,
+        od: original_due_date,
+        rd: due_date,
+        lo: location,
+        ou_genre: genre,
+        ou_title: title,
+        ou_aulast: author_last_name,
+        # The following fields don't seem to be used anywhere.
+        # ou_pages: pages,
+        # ou_year: year,
+        # ou_issue: issue,
+        # ou_volume: volume,
+        # ou_issn: issn,
+        url: url,
+        requestDate: transaction['CreationDate'],
+        TransactionDate: transaction['TransactionDate'],
+        TransactionNumber: transaction['TransactionNumber']
+      }
+    end
+
+    { items: items_array }.to_json
+  end
+
+end

lib/my_account/engine.rb

@@ -3,7 +3,8 @@ class Engine < ::Rails::Engine
     isolate_namespace MyAccount
 
     config.autoload_paths += Dir["#{config.root}/spec/support"]
-    config.eager_load_paths += Dir["#{config.root}/lib"]
+    config.autoload_paths += Dir["#{config.root}/lib/**/"]
+    config.eager_load_paths += Dir["#{config.root}/lib/**/"]
     config.assets.paths << config.root.join('/engines/my_account/app/assets/javascripts')
     config.assets.precompile << "my_account/application.js"
     config.assets.precompile << "my_account/account.js.coffee"

lib/my_account/version.rb

@@ -1,3 +1,3 @@
 module MyAccount
-  VERSION = '2.3.3'
+  VERSION = '2.3.4'
 end

release_notes.md

@@ -1,8 +1,13 @@
 # Release Notes - my-account
 
-## IN PROGRESS
+## [2.3.4] - 2025-01-17
+### Changed
+- Remove dependency on external CGI script for ILLiad lookups by adding new ILL module using ILL API (DACCESS-257)
+
 ### Fixed
+- Fix punctuation on login page (DACCESS-381)
 - Specify old authentication method to fix broken ReShare account lookup (DACCESS-459)
+- Fix bug preventing ILLiad requests from appearing (DACCESS-460)
 
 ## [2.3.3] - 2024-09-17
 - Update links to fine information