Merge pull request #230 from sul-dlss/port_error_log

justinlittman · web-flow · commit e3e6064dfe25 · 2026-07-22T16:09:28.000-04:00
Add indicators of success / error to bulk actions logs.
diff --git a/app/components/search/item_result_component.html.erb b/app/components/search/item_result_component.html.erb
@@ -5,7 +5,8 @@
         <% component.with_caption do %>
           <%= render Search::ResultHeadingComponent.new(index:) do |component| %>
             <%= component.with_link do %>
-              <%= helpers.link_to(title, object_path(druid:), data: { turbo_frame: '_top' }) %>
+              <%# Pre-fetch here breaks tests because can be triggered unintentionally. %>
+              <%= helpers.link_to(title, object_path(druid:), data: { turbo_frame: '_top', turbo_prefetch: !Rails.env.test? }) %>
             <% end %>
           <% end %>
         <% end %>
diff --git a/app/components/search/item_result_component.rb b/app/components/search/item_result_component.rb
@@ -24,8 +24,9 @@ def content_type_values
     end
 
     def admin_policy_values
+      # Pre-fetch here breaks tests because can be triggered unintentionally.
       [helpers.link_to(result.apo_title, object_path(druid: result.apo_druid),
-                       data: { turbo_frame: '_top' })]
+                       data: { turbo_frame: '_top', turbo_prefetch: !Rails.env.test? })]
     end
 
     def collections_values
@@ -65,7 +66,7 @@ def access_rights_values
     def collection_links
       result.collection_druids.map.with_index do |collection_druid, index|
         helpers.link_to(result.collection_titles[index], object_path(druid: collection_druid),
-                        data: { turbo_frame: '_top' })
+                        data: { turbo_frame: '_top', turbo_prefetch: false })
       end
     end
 
diff --git a/app/jobs/bulk_actions/base_csv_job.rb b/app/jobs/bulk_actions/base_csv_job.rb
@@ -19,7 +19,7 @@ def perform_bulk_action
         druid = row[DRUID_COLUMN]
         perform_item_class.new(druid:, index:, job: self, row:).perform
       rescue StandardError => e
-        failure!(druid:, message: "Failed #{e.class} #{e.message}", index:)
+        failure!(druid:, message: "Error: #{e.class} #{e.message}", index:)
       end
     end
 
diff --git a/app/jobs/bulk_actions/base_csv_job_item.rb b/app/jobs/bulk_actions/base_csv_job_item.rb
@@ -13,11 +13,11 @@ def initialize(row:, **args)
     attr_reader :row
 
     def success!(message:)
-      job.success!(druid:, message:, index:)
+      job.success!(druid:, message: "Success: #{message}", index:)
     end
 
     def failure!(message:)
-      job.failure!(druid:, message:, index:)
+      job.failure!(druid:, message: "Error: #{message}", index:)
     end
   end
 end
diff --git a/app/jobs/bulk_actions/base_job_item.rb b/app/jobs/bulk_actions/base_job_item.rb
@@ -26,12 +26,12 @@ def perform
 
     # Indicate that the action was successful.
     def success!(message: nil)
-      job.success!(druid:, message:)
+      job.success!(druid:, message: "Success: #{message}")
     end
 
     # Indicate that the action failed.
     def failure!(message:)
-      job.failure!(druid:, message:)
+      job.failure!(druid:, message: "Error: #{message}")
     end
 
     def cocina_object
diff --git a/app/jobs/bulk_actions/druids_job.rb b/app/jobs/bulk_actions/druids_job.rb
@@ -23,7 +23,7 @@ def perform_bulk_action
       druids.each_with_index do |druid, index|
         perform_item_class.new(druid:, index:, job: self).perform
       rescue StandardError => e
-        failure!(druid:, message: "Failed #{e.class} #{e.message}")
+        failure!(druid:, message: "Error: #{e.class} #{e.message}")
         Rails.logger.error(e.full_message)
         Honeybadger.notify(e)
       end
diff --git a/app/jobs/bulk_actions/import_descriptive_metadata_job.rb b/app/jobs/bulk_actions/import_descriptive_metadata_job.rb
@@ -21,7 +21,7 @@ def perform # rubocop:disable Metrics/AbcSize
         Dor::Services::Client.objects.indexable(druid: cocina_object.externalIdentifier,
                                                 cocina: cocina_object.new(description:))
 
-        return failure!(message: 'Description unchanged') if cocina_object.description == description
+        return success!(message: 'Description unchanged') if cocina_object.description == description
 
         open_new_version_if_needed!(description: description_msg)
 
diff --git a/spec/jobs/bulk_actions/base_csv_job_item_spec.rb b/spec/jobs/bulk_actions/base_csv_job_item_spec.rb
@@ -24,7 +24,7 @@
 
     it 'calls job.success!' do
       bulk_action_item.success!(message: 'Testing successful')
-      expect(job).to have_received(:success!).with(druid:, message: 'Testing successful', index: 2)
+      expect(job).to have_received(:success!).with(druid:, message: 'Success: Testing successful', index: 2)
     end
   end
 
@@ -35,7 +35,7 @@
 
     it 'calls job.failure!' do
       bulk_action_item.failure!(message: 'Testing failed')
-      expect(job).to have_received(:failure!).with(druid:, message: 'Testing failed', index: 2)
+      expect(job).to have_received(:failure!).with(druid:, message: 'Error: Testing failed', index: 2)
     end
   end
 end
diff --git a/spec/jobs/bulk_actions/base_csv_job_spec.rb b/spec/jobs/bulk_actions/base_csv_job_spec.rb
@@ -55,8 +55,8 @@ def perform
 
       expect(log).to have_received(:puts).with(/Starting TestBulkActionCsvJob for BulkAction #{bulk_action.id}/)
       expect(log).to have_received(:puts).with(/Finished TestBulkActionCsvJob for BulkAction #{bulk_action.id}/)
-      expect(log).to have_received(:puts).with(/line 2 - Testing successful for #{druids.first}/o)
-      expect(log).to have_received(:puts).with(/line 3 - Testing failed for #{druids.second}/o)
+      expect(log).to have_received(:puts).with(/line 2 - Success: Testing successful for #{druids.first}/o)
+      expect(log).to have_received(:puts).with(/line 3 - Error: Testing failed for #{druids.second}/o)
 
       expect(bulk_action.reload.druid_count_total).to eq(2)
       expect(bulk_action.druid_count_success).to eq(1)
diff --git a/spec/jobs/bulk_actions/base_job_item_spec.rb b/spec/jobs/bulk_actions/base_job_item_spec.rb
@@ -19,7 +19,7 @@
 
     it 'calls job.success!' do
       bulk_action_item.success!(message: 'Testing successful')
-      expect(job).to have_received(:success!).with(druid:, message: 'Testing successful')
+      expect(job).to have_received(:success!).with(druid:, message: 'Success: Testing successful')
     end
   end
 
@@ -30,7 +30,7 @@
 
     it 'calls job.failure!' do
       bulk_action_item.failure!(message: 'Testing failed')
-      expect(job).to have_received(:failure!).with(druid:, message: 'Testing failed')
+      expect(job).to have_received(:failure!).with(druid:, message: 'Error: Testing failed')
     end
   end
 
@@ -224,7 +224,7 @@
 
       it 'returns false' do
         expect(bulk_action_item.check_update_ability?).to be false
-        expect(job).to have_received(:failure!).with(druid:, message: 'Not authorized to update')
+        expect(job).to have_received(:failure!).with(druid:, message: 'Error: Not authorized to update')
       end
     end
   end
@@ -256,7 +256,7 @@
 
       it 'returns false' do
         expect(bulk_action_item.check_read_ability?).to be false
-        expect(job).to have_received(:failure!).with(druid:, message: 'Not authorized to read')
+        expect(job).to have_received(:failure!).with(druid:, message: 'Error: Not authorized to read')
       end
     end
   end
diff --git a/spec/jobs/bulk_actions/druids_job_spec.rb b/spec/jobs/bulk_actions/druids_job_spec.rb
@@ -46,8 +46,8 @@ def perform
 
       expect(log).to have_received(:puts).with(/Starting TestBulkActionJob for BulkAction #{bulk_action.id}/)
       expect(log).to have_received(:puts).with(/Finished TestBulkActionJob for BulkAction #{bulk_action.id}/)
-      expect(log).to have_received(:puts).with(/Testing successful for #{druids.first}/o)
-      expect(log).to have_received(:puts).with(/Testing failed for #{druids.second}/o)
+      expect(log).to have_received(:puts).with(/Success: Testing successful for #{druids.first}/o)
+      expect(log).to have_received(:puts).with(/Error: Testing failed for #{druids.second}/o)
       expect(log).to have_received(:close)
       expect(export_file).to have_received(:close)
 
@@ -82,8 +82,8 @@ def perform
     it 'performs the job' do
       TestBulkActionJob.perform_now(bulk_action:, druids:)
 
-      expect(log).to have_received(:puts).with(/Testing successful for #{druids.first}/o)
-      expect(log).to have_received(:puts).with(/Failed StandardError Something bad happened for #{druids.second}/o)
+      expect(log).to have_received(:puts).with(/Success: Testing successful for #{druids.first}/o)
+      expect(log).to have_received(:puts).with(/Error: StandardError Something bad happened for #{druids.second}/o)
 
       expect(bulk_action.reload.druid_count_total).to eq(2)
       expect(bulk_action.druid_count_success).to eq(1)
diff --git a/spec/jobs/bulk_actions/import_descriptive_metadata_job_spec.rb b/spec/jobs/bulk_actions/import_descriptive_metadata_job_spec.rb
@@ -135,8 +135,8 @@
       expect(log.string).to include 'Description unchanged'
 
       expect(bulk_action.reload.druid_count_total).to eq 1
-      expect(bulk_action.druid_count_fail).to eq 1
-      expect(bulk_action.druid_count_success).to eq 0
+      expect(bulk_action.druid_count_fail).to eq 0
+      expect(bulk_action.druid_count_success).to eq 1
     end
   end
 end
diff --git a/spec/jobs/bulk_actions/reindex_job_spec.rb b/spec/jobs/bulk_actions/reindex_job_spec.rb
@@ -51,8 +51,8 @@
         expect(bulk_action.druid_count_fail).to eq 2
 
         expect(log.string).to include "Reindex successful for #{druids[0]}"
-        expect(log.string).to include "Failed StandardError StandardError for #{druids[1]}"
-        expect(log.string).to include 'Failed Dor::Services::Client::ConnectionFailed ' \
+        expect(log.string).to include "Error: StandardError StandardError for #{druids[1]}"
+        expect(log.string).to include 'Error: Dor::Services::Client::ConnectionFailed ' \
                                       "Dor::Services::Client::ConnectionFailed for #{druids[2]}"
       end
     end
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
@@ -20,15 +20,15 @@
 SKIPPABLE_TEST_COVERAGE_CONCERNS = %w[assets javascript views].freeze
 
 SimpleCov.start(:rails) do
-  add_filter '/lib/tasks/'
+  skip '/lib/tasks/'
 
   # Use SimpleCov groups to break down test coverage per application concern, e.g.,
   # controllers, models, & jobs. See https://github.com/simplecov-ruby/simplecov#groups
   Dir.glob('app/*').each do |concern_path|
     concern = File.basename(concern_path)
     next if SKIPPABLE_TEST_COVERAGE_CONCERNS.include?(concern)
 
-    add_group concern.capitalize, concern_path
+    group concern.capitalize, concern_path
   end
 
   if ENV['CI']
