Refactored to make the renewal response from Sirsi uniform. (#84)

* Refactored to make the renewal response from Sirsi uniform.
* Swap "error_message" with "sirsi_response"

Co-authored-by: Banu Hapeloglu Kutlu <bzk60@psu.edu>

app/controllers/renewals_controller.rb

@@ -78,17 +78,10 @@ def bulk_renewal_summary_flash(response, type)
     end
 
     def renewal_attempt_report(renewal_response)
-      if renewal_response.respond_to?(:each)
-        non_renewal_reason = error_prompt renewal_response[:error_message]
-        return renewal_response[:renewal].bib_summary + (non_renewal_reason || '')
+      if renewal_response[:sirsi_response].present?
+        return "#{renewal_response[:renewal].bib_summary} #{tag(:br)} Denied: #{renewal_response[:sirsi_response]}"
       end
 
-      renewal_response.bib_summary
-    end
-
-    def error_prompt(error_message)
-      return if error_message.empty?
-
-      content_tag(:span, "<br>Denied: #{error_message}", nil, false)
+      renewal_response[:renewal].bib_summary
     end
 end

app/services/symphony_client.rb

@@ -76,10 +76,10 @@ def renew_items(user, checkouts)
 
       case response.status
       when 200
-        status[:success] << checkout
+        status[:success] << { renewal: checkout, sirsi_response: nil }
       else
         Rails.logger.error("Renewal (#{checkout.item_key}): #{response.body}")
-        status[:error] << { renewal: checkout, error_message: (error_message(response) || '') }
+        status[:error] << { renewal: checkout, sirsi_response: (error_message(response) || '') }
       end
     end
   end

spec/controllers/renewals_controller_spec.rb

@@ -44,7 +44,8 @@
     end
 
     context 'when all items returns 200' do
-      let(:renew_items_response) { { success: [checkouts[0], checkouts[1]], error: [] } }
+      let(:renew_items_response) { { success: [{ renewal: checkouts[0], sirsi_response: nil },
+                                               { renewal: checkouts[1], sirsi_response: nil }], error: [] } }
 
       it 'renews the items and sets flash messages' do
         post :create, params: { renewal_list: ['123', '456'] }
@@ -60,9 +61,9 @@
     end
 
     context 'when not all items return 200' do
-      let(:non_renewal_reason) { 'Item has holds' }
-      let(:error_response) { { renewal: checkouts[2], error_message: non_renewal_reason } }
-      let(:renew_items_response) { { success: [checkouts[0]], error: [error_response] } }
+      let(:renew_items_response) { { success: [{ renewal: checkouts[0], sirsi_response: nil },
+                                               { renewal: checkouts[1], sirsi_response: nil }],
+                                     error: [{ renewal: checkouts[2], sirsi_response: 'Item has holds' }] } }
 
       it 'renews the eligible items and sets flash messages' do
         post :create, params: { renewal_list: ['123', '789'] }
@@ -96,8 +97,9 @@
     end
 
     context 'when response include errored items with empty error message' do
-      let(:error_response) { { renewal: checkouts[2], error_message: '' } }
-      let(:renew_items_response) { { success: [checkouts[0]], error: [error_response] } }
+      let(:error_response) { { renewal: checkouts[2], sirsi_response: '' } }
+      let(:renew_items_response) { { success: [{ renewal: checkouts[0], sirsi_response: nil },
+                                               { renewal: checkouts[1], sirsi_response: nil }], error: [] } }
 
       it 'error messages include items title only' do
         post :create, params: { renewal_list: ['123', '789'] }
@@ -108,7 +110,7 @@
 
     context 'when user renews more than renewal flash limit items' do
       let(:non_renewal_reason) { 'Item has holds' }
-      let(:error_response) { { renewal: checkouts[2], error_message: non_renewal_reason } }
+      let(:error_response) { { renewal: checkouts[2], sirsi_response: non_renewal_reason } }
       let(:renew_items_response) {
         { success: [checkouts[0], [checkouts[1]]], error: [error_response] }
       }

spec/services/symphony_client_spec.rb

@@ -110,15 +110,16 @@
 
     it 'returns all responses for individual renewal requests in symphony regardless of success or error' do
       renew_response = client.renew_items(user, [checkouts.first, checkouts.second])
-      error_response = { renewal: checkouts.second, error_message: 'Item has holds' }
+      fail_response = { renewal: checkouts.second, sirsi_response: 'Item has holds' }
+      success_response = { renewal: checkouts.first, sirsi_response: nil }
 
-      expect(renew_response).to eq error: [error_response],
-                                   success: [checkouts.first]
+      expect(renew_response).to eq error: [fail_response],
+                                   success: [success_response]
     end
 
     it 'returns customized error messages' do
       renew_response = client.renew_items(user, [checkouts.last])
-      error_response = { renewal: checkouts.last, error_message: 'Item has holds, cannot be renewed.' }
+      error_response = { renewal: checkouts.last, sirsi_response: 'Item has holds, cannot be renewed.' }
 
       expect(renew_response).to include error: [error_response]
     end