diff --git a/app/controllers/barcode_controller.rb b/app/controllers/barcode_controller.rb
index 6f7ef7f..e8f54a9 100644
--- a/app/controllers/barcode_controller.rb
+++ b/app/controllers/barcode_controller.rb
@@ -2,6 +2,7 @@
class BarcodeController < ApplicationController
before_action :authenticate, only: [:update]
+ skip_before_action :verify_authenticity_token, only: [:update] # No need for CSRF token for API token auth endpoint
rescue_from Faraday::Error, with: :handle_faraday_error
rescue_from Bibdata::Exceptions::UnresolvableHoldingsPermanentLocationError,
diff --git a/app/jobs/location_cleanup_job.rb b/app/jobs/location_cleanup_job.rb
new file mode 100644
index 0000000..c15bf9f
--- /dev/null
+++ b/app/jobs/location_cleanup_job.rb
@@ -0,0 +1,32 @@
+# frozen_string_literal: true
+
+# This job is generally queued by a POST request to the /barcodes/#{barcode}/update endpoint, which is made by the
+# ReCAP SCSB system during an item accession. During that request, we synchronously flip the parent holdings location,
+# and then we queue this job to asynchronously perform post-accession actions (like clearing the holdings temporary
+# location, item permanent location, and item temporary location).
+class LocationCleanupJob < ApplicationJob
+ queue_as :location_cleanup
+
+ def perform(barcode) # rubocop:disable Metrics/MethodLength
+ # Clear item record permanent location
+ Bibdata::FolioApiClient.instance.update_item_record_location(
+ item_barcode: barcode,
+ location_type: :permanent,
+ new_location_code: nil
+ )
+
+ # Clear item record temporary location
+ Bibdata::FolioApiClient.instance.update_item_record_location(
+ item_barcode: barcode,
+ location_type: :temporary,
+ new_location_code: nil
+ )
+
+ # Clear parent holdings record temporary location
+ Bibdata::FolioApiClient.instance.update_item_parent_holdings_record_location(
+ item_barcode: barcode,
+ location_type: :temporary,
+ new_location_code: nil
+ )
+ end
+end
diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb
index b96da5c..c0edfea 100644
--- a/app/views/layouts/application.html.erb
+++ b/app/views/layouts/application.html.erb
@@ -1,7 +1,7 @@
- Triclops
+ Bibdata
<%= csrf_meta_tags %>
<%= csp_meta_tag %>
diff --git a/config/application.rb b/config/application.rb
index bc9a4cf..12f1cda 100644
--- a/config/application.rb
+++ b/config/application.rb
@@ -43,11 +43,11 @@ class Application < Rails::Application
# config.eager_load_paths << Rails.root.join("extras")
# Load custom configs
- config.bibdata = Rails.application.config_for(:bibdata)
+ config.bibdata = config_for(:bibdata)
config.folio = config_for(:folio)
# Use Resque for ActiveJob
- config.active_job.queue_adapter = :resque
+ config.active_job.queue_adapter = Rails.application.config_for(:resque)[:run_jobs_inline] ? :inline : :resque
config.active_job.queue_name_prefix = "bibdata.#{Rails.env}"
config.active_job.queue_name_delimiter = '.'
end
diff --git a/config/templates/redis.template.yml b/config/templates/redis.template.yml
index 002ecb6..551bb1f 100644
--- a/config/templates/redis.template.yml
+++ b/config/templates/redis.template.yml
@@ -9,3 +9,4 @@ development:
test:
<<: *default
+ port: 7379
diff --git a/config/templates/resque.template.yml b/config/templates/resque.template.yml
index d640450..489beff 100644
--- a/config/templates/resque.template.yml
+++ b/config/templates/resque.template.yml
@@ -1,4 +1,5 @@
default: &default
+ run_jobs_inline: false
workers:
'*' : 1
polling_interval: 5
@@ -9,6 +10,7 @@ development:
test:
<<: *default
+ run_jobs_inline: true
production:
<<: *default
diff --git a/docker-compose.yml b/docker-compose.yml
index cb087c8..2d7f642 100644
--- a/docker-compose.yml
+++ b/docker-compose.yml
@@ -7,7 +7,7 @@ services:
redis:
image: redis:7.0.2
ports:
- - 6379:6379
+ - 6380:6379
volumes:
- redis-data:/data
diff --git a/lib/bibdata/folio_api_client.rb b/lib/bibdata/folio_api_client.rb
index 388eefe..9bcfbf7 100644
--- a/lib/bibdata/folio_api_client.rb
+++ b/lib/bibdata/folio_api_client.rb
@@ -59,7 +59,7 @@ def update_item_record_location(item_barcode:, location_type:, new_location_code
rescue Faraday::Error => e
raise Bibdata::Exceptions::LocationNotFoundError, 'Could not update item record permanent location to '\
"\"#{new_location_code}\". "\
- "FOLIO error message: #{e.response[:body]}"
+ "FOLIO error message: #{e.response&.fetch(:body) || e.message}"
end
def update_item_parent_holdings_record_location(item_barcode:, location_type:, new_location_code:)
@@ -100,7 +100,7 @@ def update_item_parent_holdings_record_location(item_barcode:, location_type:, n
rescue Faraday::Error => e
raise Bibdata::Exceptions::LocationNotFoundError, 'Could not update holdings record permanent location to '\
"\"#{new_location_code}\". "\
- "FOLIO error message: #{e.response[:body]}"
+ "FOLIO error message: #{e.response&.fetch(:body) || e.message}"
end
def clear_item_record_temporary_location(item_barcode:, location_type:, new_location_code:)
diff --git a/lib/bibdata/scsb.rb b/lib/bibdata/scsb.rb
index 944a728..90f27fa 100644
--- a/lib/bibdata/scsb.rb
+++ b/lib/bibdata/scsb.rb
@@ -53,8 +53,7 @@ def self.merged_marc_record_for_barcode(barcode, flip_location:)
if flip_location
perform_location_flip!(
- barcode, item_record, holdings_record,
- current_holdings_permanent_location_code, material_type_record&.fetch('name')
+ barcode, current_holdings_permanent_location_code, material_type_record&.fetch('name')
)
# After performing a flip, invoke this method again with `flip_location: false` to retrieve the latest updated
@@ -79,23 +78,15 @@ def self.enhance_base_marc_record!(
end
def self.perform_location_flip!(
- barcode, item_record, holdings_record, current_holdings_permanent_location_code, material_type_name
+ barcode, current_holdings_permanent_location_code, material_type_name
)
- # If this item record has a permanent location, we want to clear it.
- # Do this before updating the holdings location, since a holdings update
- # will result in an item version update.
- clear_item_permanent_location_if_present!(barcode, item_record)
-
# If the current holdings permanent location does not equal the desired flipped location,
# update the holdings record permanent location to the flipped location.
update_holdings_permanent_location_if_required!(
barcode, current_holdings_permanent_location_code, material_type_name
)
- # If this record has a holdings temporary location or item temporary location,
- # send a notification email because assignment of temporary locations is
- # undesirable for us and that needs to be manually corrected.
- send_notification_email_if_temporary_locations_found(barcode, item_record, holdings_record)
+ LocationCleanupJob.perform_later(barcode)
end
# rubocop:disable Metrics/AbcSize
@@ -152,43 +143,27 @@ def self.update_holdings_permanent_location_if_required!(
end
# rubocop:enable Metrics/AbcSize
- def self.clear_item_permanent_location_if_present!(barcode, item_record)
- return if item_record['permanentLocationId'].blank?
-
- self.location_change_logger.unknown(
- "#{barcode}: Trying to clear item permanent location "\
- "(was originally FOLIO location #{item_record['permanentLocationId']})"
- )
- Bibdata::FolioApiClient.instance.update_item_record_location(
- item_barcode: barcode, location_type: :permanent, new_location_code: nil
- )
- self.location_change_logger.unknown(
- "#{barcode}: Cleared item permanent location "\
- "(was originally FOLIO location #{item_record['permanentLocationId']})"
- )
- end
-
- def self.send_notification_email_if_temporary_locations_found(
- barcode, item_record, holdings_record
- )
- temporary_location_notification_messages = []
- if item_record['temporaryLocationId'].present?
- error_message = 'Found unwanted item temporary location.'
- self.location_change_logger.unknown("#{barcode}: #{error_message}")
- temporary_location_notification_messages << error_message
- end
- if holdings_record['temporaryLocationId'].present?
- error_message = 'Found unwanted parent holdings temporary location.'
- self.location_change_logger.unknown("#{barcode}: #{error_message}")
- temporary_location_notification_messages << error_message
- end
-
- return if temporary_location_notification_messages.empty?
-
- BarcodeUpdateErrorMailer.with(
- barcode: barcode, errors: temporary_location_notification_messages
- ).generate_email.deliver
- end
+ # def self.send_notification_email_if_temporary_locations_found(
+ # barcode, item_record, holdings_record
+ # )
+ # temporary_location_notification_messages = []
+ # if item_record['temporaryLocationId'].present?
+ # error_message = 'Found unwanted item temporary location.'
+ # self.location_change_logger.unknown("#{barcode}: #{error_message}")
+ # temporary_location_notification_messages << error_message
+ # end
+ # if holdings_record['temporaryLocationId'].present?
+ # error_message = 'Found unwanted parent holdings temporary location.'
+ # self.location_change_logger.unknown("#{barcode}: #{error_message}")
+ # temporary_location_notification_messages << error_message
+ # end
+
+ # return if temporary_location_notification_messages.empty?
+
+ # BarcodeUpdateErrorMailer.with(
+ # barcode: barcode, errors: temporary_location_notification_messages
+ # ).generate_email.deliver
+ # end
# Returns the value if present, otherwise returns nil.
def self.get_original_876_x_value(marc_record)
diff --git a/spec/bibdata/scsb_spec.rb b/spec/bibdata/scsb_spec.rb
index 16716f7..bdc4449 100644
--- a/spec/bibdata/scsb_spec.rb
+++ b/spec/bibdata/scsb_spec.rb
@@ -156,15 +156,10 @@
expect(Bibdata::Scsb).to receive(:update_holdings_permanent_location_if_required!).with(
barcode, current_holdings_permanent_location_code, material_type_name
)
- expect(Bibdata::Scsb).to receive(:clear_item_permanent_location_if_present!).with(
- barcode, item_record
- )
- expect(Bibdata::Scsb).to receive(:send_notification_email_if_temporary_locations_found).with(
- barcode, item_record, holdings_record
- )
+ expect(LocationCleanupJob).to receive(:perform_later).with(barcode)
Bibdata::Scsb.perform_location_flip!(
- barcode, item_record, holdings_record, current_holdings_permanent_location_code, material_type_name
+ barcode, current_holdings_permanent_location_code, material_type_name
)
end
end
@@ -241,73 +236,48 @@
end
end
- describe '.clear_item_permanent_location_if_present!' do
- it 'clears the item permanent location if the item currently has a permanent location' do
-
- expect(Bibdata::Scsb.location_change_logger).to receive(:unknown).with(/Trying to clear item permanent location/)
- expect(Bibdata::FolioApiClient.instance).to receive(
- :update_item_record_location
- ).with(
- item_barcode: barcode, location_type: :permanent, new_location_code: nil
- )
- expect(Bibdata::Scsb.location_change_logger).to receive(:unknown).with(/Cleared item permanent location/)
-
- Bibdata::Scsb.clear_item_permanent_location_if_present!(
- barcode, item_record
- )
- end
-
- it 'does not try to clear the item permanent location if the item does not currently have a permanent location' do
- item_record['permanentLocationId'] = nil
- expect(Bibdata::FolioApiClient.instance).not_to receive(:update_item_record_location)
- Bibdata::Scsb.clear_item_permanent_location_if_present!(
- barcode, item_record
- )
- end
- end
-
- describe '.send_notification_email_if_temporary_locations_found' do
- it 'sends an email with one error if the item has a temporary location set' do
- item_record['temporaryLocationId'] = 'some-location-id'
- holdings_record.delete('temporaryLocationId')
- expect(Bibdata::Scsb.location_change_logger).to receive(:unknown).with(/Found unwanted item temporary location/)
- expect(BarcodeUpdateErrorMailer).to receive(:with).with(barcode: barcode, errors: [an_instance_of(String)])
- Bibdata::Scsb.send_notification_email_if_temporary_locations_found(
- barcode, item_record, holdings_record
- )
- end
-
- it "sends an email with one error if the item's parent holdings record has a temporary location set" do
- item_record.delete('temporaryLocationId')
- holdings_record['temporaryLocationId'] = 'some-location-id'
- expect(Bibdata::Scsb.location_change_logger).to receive(:unknown).with(/Found unwanted parent holdings temporary location/)
- expect(BarcodeUpdateErrorMailer).to receive(:with).with(barcode: barcode, errors: [an_instance_of(String)])
- Bibdata::Scsb.send_notification_email_if_temporary_locations_found(
- barcode, item_record, holdings_record
- )
- end
-
- it "sends an email with two errors if the item and its parent holdings record both have temporary locations set" do
- item_record['temporaryLocationId'] = 'some-location-id'
- holdings_record['temporaryLocationId'] = 'some-location-id'
-
- expect(Bibdata::Scsb.location_change_logger).to receive(:unknown).with(/Found unwanted item temporary location/)
- expect(Bibdata::Scsb.location_change_logger).to receive(:unknown).with(/Found unwanted parent holdings temporary location/)
- expect(BarcodeUpdateErrorMailer).to receive(:with).with(
- barcode: barcode, errors: [an_instance_of(String), an_instance_of(String)]
- )
- Bibdata::Scsb.send_notification_email_if_temporary_locations_found(
- barcode, item_record, holdings_record
- )
- end
-
- it 'does not try to send an email if the item and its parent holdings record do not have temporary locations set' do
- expect(BarcodeUpdateErrorMailer).not_to receive(:with)
- Bibdata::Scsb.send_notification_email_if_temporary_locations_found(
- barcode, item_record, holdings_record
- )
- end
- end
+ # describe '.send_notification_email_if_temporary_locations_found' do
+ # it 'sends an email with one error if the item has a temporary location set' do
+ # item_record['temporaryLocationId'] = 'some-location-id'
+ # holdings_record.delete('temporaryLocationId')
+ # expect(Bibdata::Scsb.location_change_logger).to receive(:unknown).with(/Found unwanted item temporary location/)
+ # expect(BarcodeUpdateErrorMailer).to receive(:with).with(barcode: barcode, errors: [an_instance_of(String)])
+ # Bibdata::Scsb.send_notification_email_if_temporary_locations_found(
+ # barcode, item_record, holdings_record
+ # )
+ # end
+
+ # it "sends an email with one error if the item's parent holdings record has a temporary location set" do
+ # item_record.delete('temporaryLocationId')
+ # holdings_record['temporaryLocationId'] = 'some-location-id'
+ # expect(Bibdata::Scsb.location_change_logger).to receive(:unknown).with(/Found unwanted parent holdings temporary location/)
+ # expect(BarcodeUpdateErrorMailer).to receive(:with).with(barcode: barcode, errors: [an_instance_of(String)])
+ # Bibdata::Scsb.send_notification_email_if_temporary_locations_found(
+ # barcode, item_record, holdings_record
+ # )
+ # end
+
+ # it "sends an email with two errors if the item and its parent holdings record both have temporary locations set" do
+ # item_record['temporaryLocationId'] = 'some-location-id'
+ # holdings_record['temporaryLocationId'] = 'some-location-id'
+
+ # expect(Bibdata::Scsb.location_change_logger).to receive(:unknown).with(/Found unwanted item temporary location/)
+ # expect(Bibdata::Scsb.location_change_logger).to receive(:unknown).with(/Found unwanted parent holdings temporary location/)
+ # expect(BarcodeUpdateErrorMailer).to receive(:with).with(
+ # barcode: barcode, errors: [an_instance_of(String), an_instance_of(String)]
+ # )
+ # Bibdata::Scsb.send_notification_email_if_temporary_locations_found(
+ # barcode, item_record, holdings_record
+ # )
+ # end
+
+ # it 'does not try to send an email if the item and its parent holdings record do not have temporary locations set' do
+ # expect(BarcodeUpdateErrorMailer).not_to receive(:with)
+ # Bibdata::Scsb.send_notification_email_if_temporary_locations_found(
+ # barcode, item_record, holdings_record
+ # )
+ # end
+ # end
end
describe '.location_change_logger' do
diff --git a/spec/jobs/location_cleanup_job_spec.rb b/spec/jobs/location_cleanup_job_spec.rb
new file mode 100644
index 0000000..5c0506e
--- /dev/null
+++ b/spec/jobs/location_cleanup_job_spec.rb
@@ -0,0 +1,27 @@
+require 'rails_helper'
+
+RSpec.describe LocationCleanupJob do
+ let(:instance) { described_class.new }
+ let(:barcode) { 'CU23392169' }
+
+ describe '#perform' do
+ it 'performs the expected operations' do
+ expect(Bibdata::FolioApiClient.instance).to receive(:update_item_record_location).with(
+ item_barcode: barcode,
+ location_type: :permanent,
+ new_location_code: nil
+ )
+ expect(Bibdata::FolioApiClient.instance).to receive(:update_item_record_location).with(
+ item_barcode: barcode,
+ location_type: :temporary,
+ new_location_code: nil
+ )
+ expect(Bibdata::FolioApiClient.instance).to receive(:update_item_parent_holdings_record_location).with(
+ item_barcode: barcode,
+ location_type: :temporary,
+ new_location_code: nil
+ )
+ instance.perform(barcode)
+ end
+ end
+end