Merge changes for v2.2.2 (#98)

* Merge changes for v2.1.6 (#92) * DISCOVERYACCESS-7627 - for rbenv users * DISCOVERYACCESS-7627 - rails 6 required for cornell-blacklight * DISCOVERYACCESS-7627 - bump version number * DISCOVERYACCESS-7627 - rails 6 for cornell-blacklight * DISCOVERYACCESS-7627 - try rails 7 * DISCOVERYACCESS-7627 - back to rails 6.1 * DISCOVERYACCESS-7627 - bump version number * DISCOVERYACCESS-7627 - forgot save all * Update release notes Co-authored-by: James Reidy <[email protected]> * DISCOVERYACCESS-7838: update my account login page text * Merge changes for v2.2.0 (ReShare) to dev (#94) * WIP * Eliminate ilsapi cgi script reference (talk directly to illiad6.cgi) * Display pickup location for BD * Differentiate pending and available ReShare requests * Bump to v2.2.0 * Ensure ReShare results are only for the user's netid * Remove unused functions * Linting * Skip ILL items with status 'Request Sent to BD' * Avoid duplicate request entries * DISCOVERYACCESS-7964: update links to main site * DISCOVERYACCESS-7964: update link to fine rates * Merge changes for v2.2.2 (#97) * Merge changes for v2.2.1 to master (#96) * Merge changes for v2.1.6 (#92) * DISCOVERYACCESS-7627 - for rbenv users * DISCOVERYACCESS-7627 - rails 6 required for cornell-blacklight * DISCOVERYACCESS-7627 - bump version number * DISCOVERYACCESS-7627 - rails 6 for cornell-blacklight * DISCOVERYACCESS-7627 - try rails 7 * DISCOVERYACCESS-7627 - back to rails 6.1 * DISCOVERYACCESS-7627 - bump version number * DISCOVERYACCESS-7627 - forgot save all * Update release notes Co-authored-by: James Reidy <[email protected]> * DISCOVERYACCESS-7838: update my account login page text * Merge changes for v2.2.0 (ReShare) to dev (#94) * WIP * Eliminate ilsapi cgi script reference (talk directly to illiad6.cgi) * Display pickup location for BD * Differentiate pending and available ReShare requests * Bump to v2.2.0 * Ensure ReShare results are only for the user's netid * Remove unused functions * Linting * Skip ILL items with status 'Request Sent to BD' * Avoid duplicate request entries Co-authored-by: James Reidy <[email protected]> Co-authored-by: Melissa Wallace <[email protected]> * Handle 'checked out in FOLIO' * Don't create catalog links for ReShare items * Code cleanup * Don't show ReShare items with status REQ_CHECKED_IN * Show date shipped for ReShare items in pending requests * Remove nonfunctional checkbox selection from available requests. * Restore source badges for ILL items in checkouts list * Bump to v2.2.2 * Remove debug code --------- Co-authored-by: James Reidy <[email protected]> Co-authored-by: Melissa Wallace <[email protected]> --------- Co-authored-by: James Reidy <[email protected]> Co-authored-by: Melissa Wallace <[email protected]>
cul-it · Mar 6, 2023 · 160d532 · 160d532
1 parent e5746ea
commit 160d532
Show file tree

Hide file tree

Showing 8 changed files with 91 additions and 53 deletions.
diff --git a/app/assets/javascripts/my_account/account.js.coffee b/app/assets/javascripts/my_account/account.js.coffee
@@ -217,6 +217,8 @@ account =
   addSourceBadge: (id, source) ->
     if source == 'bd'
       $("##{id} .source-badge").html('<div class="badge badge-primary">Borrow Direct</div>')
+    if source == 'ill'
+      $("##{id} .source-badge").html('<div class="badge badge-primary">Interlibrary Loan</div>')
 
   # Populate fines/fees in the UI
   showFines: (accountData) ->
@@ -247,7 +249,6 @@ account =
     # Sort out the FOLIO request data into the format and category expected by the views
     folioTitles = []
     folioData.forEach (entry) ->
-      console.log entry
       folioTitles.push entry.item.title
       # Look up the service point based on its ID. getServicePoint() will try to dynamically update the status
       # in the table when the result is ready
@@ -282,8 +283,10 @@ account =
         requestObj = {
           iid: entry.iid, # N.B. The ID used here for FOLIO requests is the REQUEST ID, not the item ID!
           tl: entry.tl,
+          au: entry.au,
           system: 'bd',
-          lo: location
+          lo: location,
+          shipped: entry.shipped
         }
 
         # This is a bit of a hack. An ON_LOAN item is really an available request, but at that point in

diff --git a/app/controllers/my_account/account_controller.rb b/app/controllers/my_account/account_controller.rb
@@ -19,7 +19,7 @@ def heading
     def authenticate_user
       # Master disable -- this kicks the user out of My Account before anything gets going
       if ENV['DISABLE_MY_ACCOUNT']
-        msg = 'My Account is currently unavailable. We apologize for the inconvenience. For more information, check the <a href="https://library.cornell.edu">CUL home page</a> for updates or <a href="https://library.cornell.edu/ask">ask a librarian</a>.'
+        msg = 'My Account is currently unavailable. We apologize for the inconvenience. For more information, check the <a href="https://library.cornell.edu">CUL home page</a> for updates or <a href="https://library.cornell.edu/get-help/ask">ask a librarian</a>.'
         redirect_to '/catalog#index', notice: msg.html_safe
         return
       end
@@ -54,7 +54,7 @@ def index
       end
       # Master disable -- this kicks the user out of My Account before anything gets going
       if ENV['DISABLE_MY_ACCOUNT']
-        msg = 'My Account is currently unavailable. We apologize for the inconvenience. For more information, check the <a href="https://library.cornell.edu">CUL home page</a> for updates or <a href="https://library.cornell.edu/ask">ask a librarian</a>.'
+        msg = 'My Account is currently unavailable. We apologize for the inconvenience. For more information, check the <a href="https://library.cornell.edu">CUL home page</a> for updates or <a href="https://library.cornell.edu/get-help/ask">ask a librarian</a>.'
         redirect_to '/catalog#index', notice: msg.html_safe
       end
 
@@ -193,6 +193,15 @@ 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'
+        # section from FOLIO, so if we skip them here they should show up in the right place.
+        next if i['TransactionStatus'] == 'Checked out in FOLIO'
+
         # This is a hold, recall, or ILL request. Rather than tracking the item ID, we need the request
         # id for potential cancellations.
         # HACK: substitute request id for item id
@@ -201,7 +210,7 @@ def get_illiad_data
         # add a special "item id" for ILLiad items
         if i['system'] == 'illiad'
           i['iid'] = "illiad-#{i['TransactionNumber']}"
-          i['requestDate'] = DateTime.parse(i['TransactionDate']).strftime('%-m/%-d/%y')
+          i['requestDate'] = formatted_date(i['TransactionDate'])
         end
 
         case i['status']
@@ -215,6 +224,7 @@ def get_illiad_data
           i['status'] = 'Charged'
           # checkouts << i
         else
+          i['shipped'] = ship_date(i)
           pending_requests << i
         end
       end
@@ -273,9 +283,15 @@ def ajax_catalog_link_and_source
       source = nil
       if response[:code] < 300
         source = response[:instance]['source']
+        # Try to identify ILL items and set source manually -- it's 'FOLIO' in the actual record.
+        source = 'ill' if ill_item?(response[:instance])
+
         # Ignore Borrow Direct records for the link -- they have an HRID that looks like a legit bibid, but
-        # it's something else BD-related. We can't link to those.
-        link = "https://newcatalog.library.cornell.edu/catalog/#{response[:instance]['hrid']}" if source != 'bd'
+        # it's something else BD-related. We can't link to those. But now, most sources are either MARC or
+        # FOLIO. A FOLIO source indicates that this was a locally-created record -- e.g., for a temporary record
+        # for a BD/ReShare item. Most of the others appear to be MARC-source. This is probably not entirely accurate,
+        # but we can filter out the FOLIO records and probably get things right most of the time.
+        link = "https://newcatalog.library.cornell.edu/catalog/#{response[:instance]['hrid']}" if source == 'MARC'
       end
       render json: { link: link, source: source }
     end
@@ -307,15 +323,6 @@ def get_bd_requests
       # return session[netid + '_bd_items'] if session[netid + '_bd_items']
       Rails.logger.debug "mjc12test: Can't use session value for BD items - doing full lookup"
 
-      # barcode = patron_barcode(netid)
-
-      # Set parameters for the Borrow Direct API
-      # BorrowDirect::Defaults.library_symbol = 'CORNELL'
-      # BorrowDirect::Defaults.find_item_patron_barcode = barcode
-      # BorrowDirect::Defaults.timeout = ENV['BORROW_DIRECT_TIMEOUT'].to_i || 30 # (seconds)
-      # BorrowDirect::Defaults.api_base = BorrowDirect::Defaults::PRODUCTION_API_BASE
-      # BorrowDirect::Defaults.api_key = ENV['BORROW_DIRECT_PROD_API_KEY']
-
       begin
         token = nil
         response = CUL::FOLIO::Edge.authenticate(ENV['RESHARE_STATUS_URL'], ENV['RESHARE_TENANT'], ENV['RESHARE_USER'], ENV['RESHARE_PW'])
@@ -335,42 +342,36 @@ def get_bd_requests
         response = RestClient.get(url, headers)
         # TODO: check that response is in the proper form and there are no returned errors
         items = JSON.parse(response)
+
         # The ReShare match query is too broad and will match any portion of the netid --
         # e.g., the results for 'mjc12' will also include any that are found for 'mjc124'.
         # So we need to ensure that the patronIdentifier of each result matches our netid.
         items.select! { |i| i['patronIdentifier'] == netid }
       rescue RestClient::Exception => e
-      # items = BorrowDirect::RequestQuery.new(barcode).requests('open')
-      # rescue BorrowDirect::Error => e
-      #   # The Borrow Direct gem doesn't differentiate among all of the BD API error types.
-      #   # In this case, PUBQR004 is an exception raised when there are no results for the query
-      #   # (why should that cause an exception??). We don't want to crash and burn just because
-      #   # the user doesn't have any BD requests in the system. But if it's anything else,
-      #   # raise it again -- that indicates a real problem.
-      #   if e.message.include? 'PUBAN003'
-      #     Rails.logger.error "MyAccount error: user could not be authenticated in Borrow Direct"
-      #   else
-      #     # TODO: Add better error handling. For now, BD is causing too many problems with flaky connections;
-      #     # we have to do something other than raise the errors here.
-      #     # raise unless e.message.include? 'PUBQR004'
-      #   end
         items = []
         Rails.logger.error "MyAccount error: Couldn\'t retrieve patron requests from ReShare (#{e})."
       end
-      # Returns an array of BorrowDirect::RequestQuery::Item
+
       cleaned_items = []
       items.each do |item|
-        Rails.logger.debug "mjc12a: item data: HRID: #{item['hrid']} for patronIdentifier #{item['patronIdentifier']}"
-        Rails.logger.debug "mjc12a: state: #{item['state']['code']}, stage: #{item['state']['stage']}"
+        # ReShare items that are in the state REQ_CHECKED_IN have been checked in *by staff at CUL*,
+        # meaning that the book has had a temporary record created in FOLIO and is awaiting pickup
+        # by the patron. Assume that such items will be handled by the FOLIO API data once they have this
+        # status (either in requests/holds before patron pickup, or in loans after pickup.)
+        next if item['state']['code'] == 'REQ_CHECKED_IN'
 
-        # HACK: This is a terrible way to obtain the item title. Unfortunately, this information isn't surfaced
+        # HACK: This is a terrible way to obtain the item title and author. Unfortunately, this information isn't surfaced
         # in the API response, but only provided as part of a marcxml description of the entire item record.
         marc = XmlSimple.xml_in(item['bibRecord'])
         f245 = marc['GetRecord'][0]['record'][0]['metadata'][0]['record'][0]['datafield'].find { |t| t['tag'] == '245' }
         f245a = f245['subfield'].find { |sf| sf['code'] == 'a' }
         f245b = f245['subfield'].find { |sf| sf['code'] == 'b' }
         title = f245b ? "#{f245a['content']} #{f245b['content']}" : f245a['content']
 
+        f100 = marc['GetRecord'][0]['record'][0]['metadata'][0]['record'][0]['datafield'].find { |t| t['tag'] == '100' }
+        f100a = f100['subfield'].find { |sf| sf['code'] == 'a' }
+        author = f100a ? "#{f100a['content']}" : ''
+
         # For the final item, we add a fake item ID number (iid) for compatibility with other items in the system
         # ReShare status *stages* are defined here: 
         # https://github.com/openlibraryenvironment/mod-rs/blob/master/service/src/main/groovy/org/olf/rs/statemodel/StatusStage.groovy
@@ -379,18 +380,49 @@ def get_bd_requests
         # I think we only need to worry about stage to distinguish between pending and available.
         cleaned_items << {
           'tl' => title,
-          'au' => '',
+          'au' => author,
           'system' => 'bd',
           'status' => item['state']['code'],
           'iid' => item['hrid'],
-          'lo' => item['pickupLocation']
+          'lo' => item['pickupLocation'],
+          'shipped' => reshare_shipped_status(item)
         }
       end
       session[netid + '_bd_items'] = cleaned_items
-      Rails.logger.debug "mjc12a: cleaned BD items: #{cleaned_items}"
       render json: cleaned_items
     end
 
+    # Use the 'audit' section of a ReShare item's metadata to determine whether the item has
+    # been shipped or not. If it has, return a string to that effect that can be displayed in the UI.
+    def reshare_shipped_status(item)
+      if item && item['audit']
+        steps = item['audit'].sort_by! { |i| i['auditNo'] }
+        steps.each do |step|
+          step_date = formatted_date(step['dateCreated'])
+          return "Shipped #{step_date}" if step['toStatus']['code'] == 'REQ_SHIPPED'
+        end
+      end
+      return ''
+    end
+
+    # Given a date of the form 2023-01-29T23:14:24Z, return a string formatted to 1/29/23
+    def formatted_date(date)
+      DateTime.parse(date).strftime('%-m/%-d/%y')
+    rescue
+      ''
+    end
+
+    # Given a FOLIO instance record, determine whether it's an ILL item or not. Unfortunately,
+    # we have to rely on implicit metadata and assumption. I.e., an instance is *probably*
+    # sourced from ILL if it (a) has a source of 'FOLIO' (locally created record, probably temporary),
+    # (b) is suppressed from discovery (shouldn't be searchable or requestable), but (c) is *not*
+    # suppressed from staff view.
+    def ill_item?(instance)
+      instance['source'] == 'FOLIO' &&
+      instance['discoverySuppress'] == true &&
+      instance['staffSuppress'] == false
+    end
+
     def user
       netid = nil
       if ENV['DEBUG_USER'] && Rails.env.development?

diff --git a/app/views/my_account/account/_available_requests.html.haml b/app/views/my_account/account/_available_requests.html.haml
@@ -6,19 +6,12 @@
     %table#available-requests-table.table.table-striped.table-responsive
       %thead
         %tr
-          %th{:scope => 'col', :class => 'myacct-col-5'}
-            = check_box_tag 'toggle-all-requests', 1, nil, { :id => '', :class => 'select-all', 'aria-label' => 'Select all' }
-            %span.sr-only
-              Select all
           %th{:scope => 'col', :class => 'myacct-col-50'} Title
           %th{:scope => 'col', :class => 'myacct-col-20'} Author
           %th{:scope => 'col', :class => 'myacct-col-20'} Pick up at
       %tbody
         - @available_requests.each do |c|
-          %tr.item{:id => "#{c['iid']}"}
-            %td
-              = label_tag "select-#{c['iid']}", 'Select', { :class => 'sr-only' }
-              = check_box_tag "select-#{c['iid']}", 1, false, { 'aria-label' => 'Select', :title => 'Select' }  
+          %tr.item{:id => "#{c['iid']}"} 
             %td
               = c['ou_title'] || c['tl']
             %td

diff --git a/app/views/my_account/account/_checkouts.html.haml b/app/views/my_account/account/_checkouts.html.haml
@@ -37,11 +37,6 @@
               -#     This item cannot be renewed.
               -# - if c['is_bd']
               %div.source-badge
-                -# %div.badge.badge-primary
-                  -# Borrow Direct
-              - if c['is_ill']
-                %div.badge.badge-success
-                  Interlibrary Loan
             %td
               =c['item']['author']
             %td.status
@@ -50,7 +45,7 @@
     - if ENV['MY_ACCOUNT_READONLY']
       %div.mt-4
         %p
-          My Account is currently in read-only mode. Items cannot be renewed or cancelled. Please <a href="https://library.cornell.edu/ask">ask a librarian</a> for assistance if needed.
+          My Account is currently in read-only mode. Items cannot be renewed or cancelled. Please <a href="https://library.cornell.edu/get-help/ask">ask a librarian</a> for assistance if needed.
     - else
       = button_tag 'Renew', :type => 'submit', :id => 'renew', :value => 'renew', :class => 'btn btn-danger'
       %span{:id => 'request-loading-spinner'}

diff --git a/app/views/my_account/account/_fines.html.haml b/app/views/my_account/account/_fines.html.haml
@@ -21,4 +21,4 @@
           %td
             = number_to_currency(f['chargeAmount']['amount'])
 %p
-= link_to "View fine rates", "https://www.library.cornell.edu/services/borrow/fines"
+= link_to "View fine rates", "https://newcatalog.library.cornell.edu/myaccount/login"
diff --git a/app/views/my_account/account/_pending_requests.html.haml b/app/views/my_account/account/_pending_requests.html.haml
@@ -19,6 +19,7 @@
         %th{:scope => 'col', :class => 'myacct-col-50'} Title
         %th{:scope => 'col', :class => 'myacct-col-20'} Author
         %th{:scope => 'col', :class => 'myacct-col-20'} Type
+        %th{:scope => 'col', :class => 'myacct-col-20'} Status
     %tbody
       - @pending_requests.each do |c|
         - has_ill_pending = true if c['system'] == "illiad"
@@ -38,6 +39,8 @@
             = c['ou_aulast'] || c['au']
           %td
             = request_types[c['ttype'] || c['system']]
+          %td
+            = c['shipped']
 
   - if ENV['MY_ACCOUNT_READONLY']
     %div.mt-4

diff --git a/lib/my_account/version.rb b/lib/my_account/version.rb
@@ -1,3 +1,3 @@
 module MyAccount
-  VERSION = "2.2.1"
+  VERSION = "2.2.2"
 end
diff --git a/release_notes.md b/release_notes.md
@@ -1,5 +1,17 @@
 # Release Notes - my-account
 
+## v2.2.2
+
+### Improvements
+- Show date shipped for ReShare items in pending requests.
+- Show author for ReShare items in requests.
+- Remove nonfunctional checkbox selection from available requests.
+
+### Bug fixes
+- Don't show catalog links for ReShare items.
+- Don't show ReShare items with status REQ_CHECKED_IN (let FOLIO handle them) (avoid duplicates).
+- Restore source badges for ILL items in checkout list.
+
 ## v2.2.1
 - Filter out ReShare items that don't match user's NetID.
 - Skip ILL requests with status 'Request Sent to BD' (avoid duplicates).