Queue request emails with data in a hash

Then, once it has been queued, turn the hash back into a Requests::Submission object. This helps with #4609, since sidekiq only allows us to queue emails with data in a very specific list of data types.
pulibrary · Jan 23, 2025 · a999a6e · a999a6e
1 parent 35d57e8
commit a999a6e
Show file tree

Hide file tree

Showing 9 changed files with 85 additions and 44 deletions.
diff --git a/app/controllers/requests/form_controller.rb b/app/controllers/requests/form_controller.rb
@@ -114,7 +114,7 @@ def sanitize(str)
       # This has to be a utility function to prevent ActiveJob from trying to serialize too many objects
       # :reek:UtilityFunction
       def send_error_email(errors, submission)
-        Requests::RequestMailer.send("service_error_email", errors, submission).deliver_later
+        Requests::RequestMailer.send("service_error_email", errors, submission.to_h).deliver_later
       end
   end
 end
diff --git a/app/mailers/requests/request_mailer.rb b/app/mailers/requests/request_mailer.rb
@@ -5,7 +5,7 @@ class RequestMailer < ApplicationMailer
     helper "requests/application"
 
     def digitize_fill_in_confirmation(submission)
-      @submission = submission
+      @submission = Submission.new_from_hash submission
       @delivery_mode = "edd"
       subject = I18n.t('requests.paging.email_subject', pick_up_location: "Digitization")
       destination_email = @submission.email
@@ -15,8 +15,8 @@ def digitize_fill_in_confirmation(submission)
     end
 
     def paging_email(submission)
-      @submission = submission
-      pick_ups = paging_pick_ups(submission:)
+      @submission = Submission.new_from_hash submission
+      pick_ups = paging_pick_ups(submission: @submission)
       subject = I18n.t('requests.paging.email_subject', pick_up_location: pick_ups.join(", "))
       destination_email = "[email protected]"
       mail(to: destination_email,
@@ -25,8 +25,8 @@ def paging_email(submission)
     end
 
     def paging_confirmation(submission)
-      @submission = submission
-      pick_ups = paging_pick_ups(submission:)
+      @submission = Submission.new_from_hash submission
+      pick_ups = paging_pick_ups(submission: @submission)
       subject = I18n.t('requests.paging.email_subject', pick_up_location: pick_ups.join(", "))
       destination_email = @submission.email
       mail(to: destination_email,
@@ -43,7 +43,7 @@ def pres_confirmation(submission)
     end
 
     def annex_email(submission)
-      @submission = submission
+      @submission = Submission.new_from_hash submission
       destination_email = annex_email_destinations(submission: @submission)
       mail(to: destination_email,
            from: I18n.t('requests.default.email_from'),
@@ -55,7 +55,7 @@ def annex_confirmation(submission)
     end
 
     def annex_in_library_email(submission)
-      @submission = submission
+      @submission = Submission.new_from_hash submission
       mail(to: I18n.t('requests.annex.email'),
            from: I18n.t('requests.default.email_from'),
            subject: I18n.t('requests.annex_in_library.email_subject'))
@@ -73,11 +73,11 @@ def annex_edd_confirmation(submission)
 
     # temporary changes issue 438
     def on_shelf_email(submission)
-      location_email = get_location_contact_email(submission.items.first[:location_code])
-      @submission = submission
+      @submission = Submission.new_from_hash submission
+      location_email = get_location_contact_email(@submission.items.first[:location_code])
       # Location and destination are the same forthe moment
       # destination_email = I18n.t('requests.on_shelf.email')
-      subject = "#{I18n.t('requests.on_shelf.email_subject')} (#{submission.items.first[:location_code].upcase}) #{submission.items.first[:call_number]}"
+      subject = "#{I18n.t('requests.on_shelf.email_subject')} (#{@submission.items.first[:location_code].upcase}) #{@submission.items.first[:call_number]}"
       mail(to: location_email,
            # cc: destination_email,
            from: I18n.t('requests.default.email_from'),
@@ -86,7 +86,7 @@ def on_shelf_email(submission)
 
     # temporary changes issue 438
     def on_shelf_confirmation(submission)
-      @submission = submission
+      @submission = Submission.new_from_hash submission
       destination_email = @submission.email
       subject = "#{Requests::BibdataService.delivery_locations[@submission.items.first['pick_up']]['label']} #{I18n.t('requests.on_shelf.email_subject_patron')}"
       mail(to: destination_email,
@@ -117,7 +117,7 @@ def recap_no_items_email(submission)
     end
 
     def recap_no_items_confirmation(submission)
-      @submission = submission
+      @submission = Submission.new_from_hash submission
       destination_email = @submission.email
       subject = I18n.t('requests.recap.email_subject')
       mail(to: destination_email,
@@ -157,7 +157,7 @@ def recap_marquand_in_library_confirmation(submission)
 
     def recap_in_library_email(submission)
       # only send an email to the libraries if this is a marquand request
-      request_email(submission:, subject_key: 'requests.recap_marquand.email_subject', destination_key: 'requests.recap_marquand.email_destination') if submission.marquand?
+      request_email(submission:, subject_key: 'requests.recap_marquand.email_subject', destination_key: 'requests.recap_marquand.email_destination') if Submission.new_from_hash(submission).marquand?
     end
 
     def recap_in_library_confirmation(submission)
@@ -211,7 +211,7 @@ def marquand_in_library_confirmation(submission)
     end
 
     def service_error_email(errors, submission)
-      @submission = submission
+      @submission = Submission.new_from_hash submission
       @errors = errors
       error_types = @errors.flat_map { |_key, value| value }.pluck(:type).uniq
       destination_email = if error_types.include?("digitize")
@@ -236,7 +236,7 @@ def invalid_illiad_patron_email(user_attributes, transaction_attributes)
     private
 
       def confirmation_email(submission:, subject_key:, from_key: 'requests.default.email_from', partial: nil)
-        @submission = submission
+        @submission = Submission.new_from_hash submission
         destination_email = @submission.email
         subject = I18n.t(subject_key)
         mail(to: destination_email,
@@ -246,7 +246,7 @@ def confirmation_email(submission:, subject_key:, from_key: 'requests.default.em
       end
 
       def request_email(submission:, subject_key:, destination_key: 'requests.default.email_destination', from_key: 'requests.default.email_from')
-        @submission = submission
+        @submission = Submission.new_from_hash submission
         destination_email = I18n.t(destination_key)
         mail(to: destination_email,
              from: I18n.t(from_key),

diff --git a/app/models/requests/submission.rb b/app/models/requests/submission.rb
@@ -19,6 +19,13 @@ def initialize(params, patron)
       @success_messages = []
     end
 
+    # :reek:DuplicateMethodCall
+    def self.new_from_hash(original_hash)
+      requestable = original_hash['items']&.map(&:with_indifferent_access) || []
+      patron = Patron.new(user: User.find_by(uid: original_hash['patron']['netid']), patron_hash: original_hash['patron'].with_indifferent_access)
+      new({ requestable:, bib: original_hash['bib'] }, patron)
+    end
+
     attr_reader :patron, :success_messages
 
     def email

diff --git a/app/models/requests/submissions/illiad.rb b/app/models/requests/submissions/illiad.rb
@@ -23,7 +23,7 @@ def handle_item(item)
 
     def send_mail
       return if errors.present?
-      Requests::RequestMailer.send("interlibrary_loan_confirmation", submission).deliver_later
+      Requests::RequestMailer.send("interlibrary_loan_confirmation", submission.to_h).deliver_later
     end
 
     def success_message

diff --git a/app/models/requests/submissions/recap.rb b/app/models/requests/submissions/recap.rb
@@ -18,8 +18,9 @@ def handle
 
     def send_mail
       return if errors.present?
-      Requests::RequestMailer.send("#{service_type}_email", submission).deliver_later unless ['recap_edd', 'recap'].include?(service_type)
-      Requests::RequestMailer.send("#{service_type}_confirmation", submission).deliver_later
+      hashed_submission = submission.to_h # Sidekiq will only accept a hash, not a Requests::Submission object
+      Requests::RequestMailer.send("#{service_type}_email", hashed_submission).deliver_later unless ['recap_edd', 'recap'].include?(service_type)
+      Requests::RequestMailer.send("#{service_type}_confirmation", hashed_submission).deliver_later
     end
 
     private
@@ -60,7 +61,7 @@ def handle_hold_for_item(item)
       # This has to be a utility function to prevent ActiveJob from trying to serialize too many objects
       # :reek:UtilityFunction
       def send_error_email(errors, submission)
-        Requests::RequestMailer.send("service_error_email", errors, submission).deliver_later
+        Requests::RequestMailer.send("service_error_email", errors, submission.to_h).deliver_later
       end
   end
 end
diff --git a/app/models/requests/submissions/service.rb b/app/models/requests/submissions/service.rb
@@ -23,8 +23,9 @@ def submitted
     # :reek:UtilityFunction
     def send_mail
       return if errors.present?
-      Requests::RequestMailer.send("#{service_type}_email", submission).deliver_later
-      Requests::RequestMailer.send("#{service_type}_confirmation", submission).deliver_later
+      hashed_submission = submission.to_h # Sidekiq will only accept a hash, not a Requests::Submission object
+      Requests::RequestMailer.send("#{service_type}_email", hashed_submission).deliver_later
+      Requests::RequestMailer.send("#{service_type}_confirmation", hashed_submission).deliver_later
     end
 
     def error_hash

diff --git a/spec/mailers/requests/request_mailer_spec.rb b/spec/mailers/requests/request_mailer_spec.rb
@@ -69,7 +69,7 @@
     end
 
     let(:mail) do
-      described_class.send("interlibrary_loan_confirmation", submission_for_ill).deliver_later.perform_now
+      described_class.send("interlibrary_loan_confirmation", submission_for_ill.to_h).deliver_later.perform_now
     end
 
     it "renders the headers" do
@@ -122,7 +122,7 @@
     end
 
     let(:mail) do
-      described_class.send("pres_email", submission_for_preservation).deliver_later.perform_now
+      described_class.send("pres_email", submission_for_preservation.to_h).deliver_later.perform_now
     end
 
     it "renders the headers" do
@@ -176,7 +176,7 @@
     end
 
     let(:mail) do
-      described_class.send("pres_confirmation", submission_for_preservation).deliver_later.perform_now
+      described_class.send("pres_confirmation", submission_for_preservation.to_h).deliver_later.perform_now
     end
 
     it "renders the headers" do
@@ -227,11 +227,11 @@
     end
 
     let(:mail) do
-      described_class.send("paging_email", submission_for_no_items).deliver_later.perform_now
+      described_class.send("paging_email", submission_for_no_items.to_h).deliver_later.perform_now
     end
 
     let(:confirmation) do
-      described_class.send("paging_confirmation", submission_for_no_items).deliver_later.perform_now
+      described_class.send("paging_confirmation", submission_for_no_items.to_h).deliver_later.perform_now
     end
 
     it "renders the headers" do
@@ -287,11 +287,11 @@
     end
 
     let(:mail) do
-      described_class.send("annex_email", submission_for_annex).deliver_later.perform_now
+      described_class.send("annex_email", submission_for_annex.to_h).deliver_later.perform_now
     end
 
     let(:confirmation_mail) do
-      described_class.send("annex_confirmation", submission_for_annex).deliver_later.perform_now
+      described_class.send("annex_confirmation", submission_for_annex.to_h).deliver_later.perform_now
     end
 
     it "renders email to library staffs" do
@@ -350,11 +350,11 @@
     end
 
     let(:mail) do
-      described_class.send("annex_email", submission_for_anxadoc).deliver_later.perform_now
+      described_class.send("annex_email", submission_for_anxadoc.to_h).deliver_later.perform_now
     end
 
     let(:confirmation_mail) do
-      described_class.send("annex_confirmation", submission_for_anxadoc).deliver_later.perform_now
+      described_class.send("annex_confirmation", submission_for_anxadoc.to_h).deliver_later.perform_now
     end
 
     it "renders and email to the librarians" do
@@ -412,7 +412,7 @@
     end
 
     let(:mail) do
-      described_class.send("on_order_email", submission_for_on_order).deliver_later.perform_now
+      described_class.send("on_order_email", submission_for_on_order.to_h).deliver_later.perform_now
     end
 
     it "renders the headers" do
@@ -463,7 +463,7 @@
     end
 
     let(:mail) do
-      described_class.send("on_order_confirmation", submission_for_on_order).deliver_later.perform_now
+      described_class.send("on_order_confirmation", submission_for_on_order.to_h).deliver_later.perform_now
     end
 
     it "renders the headers" do
@@ -517,7 +517,7 @@
     end
 
     let(:mail) do
-      described_class.send("in_process_email", submission_for_in_process).deliver_later.perform_now
+      described_class.send("in_process_email", submission_for_in_process.to_h).deliver_later.perform_now
     end
 
     it "renders the headers" do
@@ -570,7 +570,7 @@
     end
 
     let(:mail) do
-      described_class.send("in_process_confirmation", submission_for_in_process).deliver_later.perform_now
+      described_class.send("in_process_confirmation", submission_for_in_process.to_h).deliver_later.perform_now
     end
 
     it "renders the headers" do
@@ -633,7 +633,7 @@
     end
 
     let(:confirmation_mail) do
-      described_class.send("recap_confirmation", submission_for_recap).deliver_later.perform_now
+      described_class.send("recap_confirmation", submission_for_recap.to_h).deliver_later.perform_now
     end
 
     it "renders the confirmation" do
@@ -695,7 +695,7 @@
     end
 
     let(:mail) do
-      described_class.send("recap_edd_confirmation", submission_for_recap).deliver_later.perform_now
+      described_class.send("recap_edd_confirmation", submission_for_recap.to_h).deliver_later.perform_now
     end
 
     it "renders the headers" do
@@ -751,15 +751,15 @@
     end
     # rubocop:disable RSpec/ExampleLength
     it "sends the email and renders the headers and body" do
-      mail = described_class.send("on_shelf_email", submission_for_on_shelf).deliver_later.perform_now
+      mail = described_class.send("on_shelf_email", submission_for_on_shelf.to_h).deliver_later.perform_now
       expect(mail.subject).to eq("#{I18n.t('requests.on_shelf.email_subject')} (FIRESTONE$STACKS) PS3566.I428 A6 2015")
       expect(mail.to).to eq([I18n.t('requests.on_shelf.email')])
       expect(mail.from).to eq([I18n.t('requests.default.email_from')])
       expect(mail.body.encoded).to have_content I18n.t('requests.on_shelf.email_conf_msg')
     end
 
     it "sends the confirmation email and renders the headers and body" do
-      mail = described_class.send("on_shelf_confirmation", submission_for_on_shelf).deliver_later.perform_now
+      mail = described_class.send("on_shelf_confirmation", submission_for_on_shelf.to_h).deliver_later.perform_now
       expect(mail.subject).to eq("Firestone Library #{I18n.t('requests.on_shelf.email_subject_patron')}")
       expect(mail.to).to eq([submission_for_on_shelf.email])
       expect(mail.from).to eq([I18n.t('requests.default.email_from')])
@@ -807,14 +807,14 @@
 
     # rubocop:disable RSpec/ExampleLength
     it "sends the email and renders the headers and body" do
-      mail = described_class.send("on_shelf_email", submission_for_on_shelf).deliver_later.perform_now
+      mail = described_class.send("on_shelf_email", submission_for_on_shelf.to_h).deliver_later.perform_now
       expect(mail.subject).to eq("#{I18n.t('requests.on_shelf.email_subject')} (EASTASIAN$CJK) PL2727.S2 C574 1998")
       expect(mail.to).to eq(["[email protected]"])
       expect(mail.from).to eq([I18n.t('requests.default.email_from')])
     end
 
     it "sends the confirmation email and renders the headers and body" do
-      mail = described_class.send("on_shelf_confirmation", submission_for_on_shelf).deliver_later.perform_now
+      mail = described_class.send("on_shelf_confirmation", submission_for_on_shelf.to_h).deliver_later.perform_now
       expect(mail.subject).to eq("East Asian Library #{I18n.t('requests.on_shelf.email_subject_patron')}")
       expect(mail.to).to eq([submission_for_on_shelf.email])
       expect(mail.from).to eq([I18n.t('requests.default.email_from')])
@@ -863,7 +863,7 @@
 
     # rubocop:disable RSpec/ExampleLength
     it "sends the email and renders the headers and body" do
-      mail = described_class.send("on_shelf_email", submission_for_on_shelf).deliver_later.perform_now
+      mail = described_class.send("on_shelf_email", submission_for_on_shelf.to_h).deliver_later.perform_now
       expect(mail.subject).to eq("#{I18n.t('requests.on_shelf.email_subject')} (PLASMA$STACKS) PL2727.S2 C574 1998")
       expect(mail.to).to eq(["[email protected]"])
       expect(mail.from).to eq([I18n.t('requests.default.email_from')])
@@ -919,7 +919,7 @@
     end
 
     it "sends the error email and renders the headers and body with id and barcode" do
-      mail = described_class.send("service_error_email", errors, submission_for_clancy_error).deliver_later.perform_now
+      mail = described_class.send("service_error_email", errors, submission_for_clancy_error.to_h).deliver_later.perform_now
       expect(mail.subject).to eq(I18n.t('requests.error.service_error_subject'))
       expect(mail.to).to eq([I18n.t('requests.error.service_error_email')])
       expect(mail.from).to eq([I18n.t('requests.default.email_from')])

diff --git a/spec/models/requests/submission_spec.rb b/spec/models/requests/submission_spec.rb
@@ -1145,5 +1145,36 @@
       end
     end
   end
+
+  describe 'new_from_hash' do
+    before do
+      user = FactoryBot.create(:user, uid: 'jj')
+      user.save
+    end
+    it 'creates a submission with a patron' do
+      submission = described_class.new_from_hash({ 'patron' => { 'last_name' => 'Jónsdóttir', 'first_name' => 'Jóna', 'netid' => 'jj', 'active_email' => '[email protected]' } })
+
+      expect(submission.patron.last_name).to eq 'Jónsdóttir'
+      expect(submission.patron.first_name).to eq 'Jóna'
+      expect(submission.patron.cas_provider?).to be true
+      expect(submission.email).to eq '[email protected]'
+    end
+
+    it 'creates a submission with a bib' do
+      submission = described_class.new_from_hash({ 'bib' => { 'title' => 'My favorite book' }, 'patron' => { 'netid' => 'jj' } })
+      expect(submission.bib['title']).to eq 'My favorite book'
+    end
+
+    it 'creates a submission with items that allow string or symbol keys' do
+      submission = described_class.new_from_hash({ 'items' => [
+                                                   {
+                                                     "selected" => "true",
+                                                     "location_code" => "recap$pa"
+                                                   }
+                                                 ], 'patron' => { 'netid' => 'jj' } })
+      expect(submission.items.first['location_code']).to eq 'recap$pa'
+      expect(submission.items.first[:location_code]).to eq 'recap$pa'
+    end
+  end
 end
 # rubocop:enable Metrics/BlockLength