Merge pull request #4706 from pulibrary/add-sidekiq-compatible-submission-hash

Add Requests::Submission#to_h method that is sidekiq-compatible

Author: christinach

app/models/requests/submission.rb

@@ -84,7 +84,7 @@ def service_errors
     end
 
     def pick_up_location
-      Requests::BibdataService.delivery_locations[items.first["pick_up"]]["library"]
+      Requests::BibdataService.delivery_locations.dig(items.first&.dig('pick_up'), "library") || {}
     end
 
     def marquand?
@@ -96,6 +96,25 @@ def edd?(item)
       delivery_mode.present? && delivery_mode == "edd"
     end
 
+    # Convert the submission to a hash that is compatible that
+    # Sidekiq can safely serialize to JSON for redis.  This means:
+    #   * No application-specific objects (unless they are activerecord db objects)
+    #   * No ActiveSupport::HashWithIndifferentAccess objects, we can only have Hash objects
+    #   * The keys of the hash must all be strings, not symbols
+    # See https://github.com/sidekiq/sidekiq/wiki/Best-Practices#1-make-your-job-parameters-small-and-simple
+    def to_h
+      {
+        bib: bib.to_hash.stringify_keys,
+        email: email,
+        errors: errors.messages.stringify_keys,
+        items: items.map { |hash| hash.to_hash.stringify_keys },
+        patron: patron.to_h.to_hash.stringify_keys,
+        pick_up_location: pick_up_location.to_hash.stringify_keys,
+        user_barcode:,
+        user_name:
+      }.stringify_keys
+    end
+
     private
 
       def service_by_type(type)

spec/models/requests/submission_spec.rb

@@ -37,11 +37,11 @@
           "edd_author" => "",
           "edd_art_title" => "",
           "edd_note" => "",
-          "pick_up" => "Firestone Library"
-        },
+          "pick_up" => "PF"
+        }.with_indifferent_access,
         {
           "selected" => "false"
-        }
+        }.with_indifferent_access
       ]
     end
     let(:bib) do
@@ -50,7 +50,7 @@
         "title" => "County and city data book.",
         "author" => "United States",
         "date" => "1949"
-      }
+      }.with_indifferent_access
     end
     let(:params) do
       {
@@ -105,6 +105,39 @@
         expect(submission.partner_item?(submission.items.first)).to be false
       end
     end
+
+    describe '#to_h' do
+      before { stub_delivery_locations }
+      it 'is compatible with sidekiq' do
+        expect({ 'args' => submission.to_h }).to be_compatible_with_sidekiq
+      end
+      it 'contains a bib' do
+        expect(submission.to_h['bib']['id']).to eq '994916543506421'
+        expect(submission.to_h['bib']['title']).to eq 'County and city data book.'
+      end
+      it 'contains items' do
+        expect(submission.to_h['items'].first['call_number']).to eq 'HA202 .U581'
+      end
+      it 'contains an email' do
+        expect(submission.to_h['email']).to eq 'foo@princeton.edu'
+      end
+      it 'contains errors' do
+        expect(submission.to_h['errors']).to be_empty
+      end
+      it 'contains a patron' do
+        expect(submission.to_h['patron']['first_name']).to eq 'Foo'
+        expect(submission.to_h['patron']['last_name']).to eq 'Request'
+      end
+      it 'contains a pick_up_location' do
+        expect(submission.to_h['pick_up_location']['label']).to eq 'Firestone Library'
+      end
+      it 'contains a user_barcode' do
+        expect(submission.to_h['user_barcode']).to eq '22101007797777'
+      end
+      it 'contains a user_name' do
+        expect(submission.to_h['user_name']).to eq 'foo'
+      end
+    end
   end
 
   context 'An invalid Submission' do
@@ -132,6 +165,12 @@
         expect(invalid_submission.errors.full_messages.size).to be > 0
       end
     end
+    describe '#to_h' do
+      before { stub_delivery_locations }
+      it 'is compatible with sidekiq' do
+        expect({ 'args' => invalid_submission.to_h }).to be_compatible_with_sidekiq
+      end
+    end
   end
 
   context 'Recap' do
@@ -247,6 +286,16 @@
       it 'contains an error message' do
         expect(submission.errors.messages).to be_truthy
       end
+
+      describe '#to_h' do
+        before { stub_delivery_locations }
+        it 'is compatible with sidekiq' do
+          expect(submission.to_h).to be_compatible_with_sidekiq
+        end
+        it 'contains errors' do
+          expect(submission.to_h['errors']).to eq({ "items" => [{ "empty_set" => { "text" => "Please Select an Item to Request!", "type" => "options" } }] })
+        end
+      end
     end
 
     describe 'A submission without a pick-up location' do

spec/support/sidekiq_data_check.rb

@@ -0,0 +1,39 @@
+# frozen_string_literal: true
+# This class is responsible for checking whether a provided data structure
+# is compatible with sidekiq.
+#
+# See https://github.com/sidekiq/sidekiq/wiki/Best-Practices#1-make-your-job-parameters-small-and-simple
+class SidekiqDataCheck
+  include Sidekiq::JobUtil
+  def initialize(data_to_check)
+    @data_to_check = { "args" => data_to_check }
+  end
+
+  def valid?
+    check
+    true
+  rescue ArgumentError
+    false
+  end
+
+  def error_message
+    check
+  rescue ArgumentError => e
+    e.message
+  end
+
+  private
+
+    attr_reader :data_to_check
+
+    # Returns nil if valid, raises ArgumentError if not
+    def check
+      Sidekiq::Config::DEFAULTS[:on_complex_arguments] = :raise
+      verify_json data_to_check
+    end
+end
+
+RSpec::Matchers.define :be_compatible_with_sidekiq do
+  match { |actual| SidekiqDataCheck.new(actual).valid? }
+  failure_message { |actual| SidekiqDataCheck.new(actual).error_message }
+end