Daniandrew/appeals 59845

app/jobs/person_and_veteran_event_remediation_job.rb

@@ -3,6 +3,12 @@
 class PersonAndVeteranEventRemediationJob < CaseflowJob
   queue_with_priority :low_priority
 
+  class PersonAndVeteranRemediationJobError < StandardError; end
+
+  retry_on(PersonAndVeteranRemediationJobError, attempts: 3, wait: :exponentially_longer) do |job, exception|
+    Rails.logger.error("#{job.class.name} (#{job.job_id}) failed with error: #{exception}")
+  end
+
   def setup_job
     RequestStore.store[:current_user] = User.system_user
   end

app/services/remediations/duplicate_person_remediation_service.rb

@@ -18,8 +18,19 @@ def initialize(updated_person_id:, duplicate_person_ids:, event_record:)
   end
 
   def remediate!
-    if find_and_update_records
-      @dup_persons.each(&:destroy!)
+    begin
+      if find_and_update_records
+        @dup_persons.each(&:destroy!)
+      else
+        Rails.logger.error "find_and_update_records failed"
+        SlackService.new.send_notification("Job failed during record update", "Error in #{self.class.name}")
+        false
+      end
+    rescue StandardError => error
+      # This will catch any errors that happen during the execution of find_and_update_records or subsequent operations
+      Rails.logger.error "An error occurred during remediation: #{error.message}"
+      SlackService.new.send_notification("Job failed during remediation: #{error.message}", "Error in #{self.class.name}")
+      false # Indicate failure
     end
   end
 
@@ -40,10 +51,13 @@ def find_and_update_records
           end
         end
       end
-      true
+      true # Successfully completed, return true
     rescue StandardError => error
-      Rails.logger.error "an error occured #{error}"
-      false
+      # Log the error specific to find_and_update_records and return false
+      Rails.logger.error "Error in find_and_update_records: #{error.message}"
+      SlackService.new.send_notification("Error in find_and_update_records: #{error.message}",
+                                         "Error in #{self.class.name}")
+      false # Indicate failure
     end
   end
 

app/services/remediations/veteran_record_remediation_service.rb

@@ -12,32 +12,43 @@ def initialize(before_fn, after_fn, event_record)
   end
 
   def remediate!
-    real_v = Veteran.find_by_file_number(@after_fn)
-    @dups = Veteran.where(ssn: real_v.ssn).reject { |v| v.id == real_v.id }
+    begin
+      real_v = Veteran.find_by_file_number(@after_fn)
+      @dups = Veteran.where(ssn: real_v.ssn).reject { |v| v.id == real_v.id }
 
-    if @dups.any?
-      # If there are duplicates, run dup_fix on @after_fn
-      if dup_fix(@after_fn)
-        @dups.each(&:destroy!)
+      if @dups.any?
+        # If there are duplicates, run dup_fix on @after_fn
+        if dup_fix(@after_fn)
+          @dups.each(&:destroy!)
+        else
+          Rails.logger.error "dup_fix failed"
+          SlackService.new.send_notification("Job failed during record update", "Error in #{self.class.name}")
+          false
+        end
+      else
+        # Otherwise, fix veteran records normally
+        fix_vet_records
       end
-
-    else
-      # Otherwise, fix veteran records normally
-      fix_vet_records
+    rescue StandardError => error
+      # This will catch any errors that happen during the execution of find_and_update_records or subsequent operations
+      Rails.logger.error "An error occurred during remediation: #{error.message}"
+      SlackService.new.send_notification("Job failed during remediation: #{error.message}", "Error in #{self.class.name}")
+      false # Indicate failure
     end
   end
 
   private
 
   def dup_fix(file_number)
     begin
-      # should we run the base transaction nested like this or is that bad practice?
       duplicate_veterans_collections = @dups.flat_map { |dup| grab_collections(dup.file_number) }
       update_records!(duplicate_veterans_collections, file_number)
+      # SlackService.new.send_notification("Job completed successfully", self.class.name)
       true
     rescue StandardError => error
       Rails.logger.error "an error occured #{error}"
-      false
+      SlackService.new.send_notification("Job failed with error: #{error.message}", "Error in #{self.class.name}")
+      false # Indicate failure
       # sentry log / metabase dashboard
     end
   end

spec/services/remediations/duplicate_person_remediation_service_spec.rb

@@ -100,9 +100,16 @@
 
     context "when an error occurs during remediation" do
       before do
-        # Force `find_and_update_records` to return false to simulate a failure
-        allow(service).to receive(:find_and_update_records).and_return(false)
+        # Force `find_and_update_records` to raise an exception to simulate an error
+        allow(service).to receive(:find_and_update_records).and_raise(StandardError.new("Something went wrong"))
+
         allow(mock_records.first).to receive(:update!).and_raise(StandardError, "Test error")
+
+        # Mock SlackService notification
+        allow(SlackService).to receive_message_chain(:new, :send_notification)
+
+        # Spy on Rails.logger to check for error logging
+        allow(Rails.logger).to receive(:error)
       end
 
       it "does not update any records and skips destroying duplicate persons" do
@@ -119,9 +126,20 @@
       end
 
       it "logs the error and does not destroy duplicate persons" do
-        result = service.remediate!
+        # Run the method that should raise an error
+        service.remediate!
 
-        expect(result).to be_falsey
+        # Ensure the error was logged
+        expect(Rails.logger).to have_received(:error).with("An error occurred during remediation: Something went wrong")
+          .once
+
+        # Ensure Slack notification was sent
+        expect(SlackService).to have_received(:new)
+        expect(SlackService.new).to have_received(:send_notification)
+          .with("Job failed during remediation: Something went wrong",
+                "Error in Remediations::DuplicatePersonRemediationService")
+
+        # Ensure that duplicate persons are not destroyed
         expect(duplicate_person1).not_to have_received(:destroy!)
         expect(duplicate_person2).not_to have_received(:destroy!)
       end

spec/services/remediations/veteran_record_remediation_service_spec.rb

@@ -100,5 +100,54 @@
         end
       end
     end
+
+    context "when an error occurs in dup_fix" do
+      let(:error_message) { "Something went wrong" }
+
+      before do
+        # Force `dup_fix` to raise an error
+        allow(vet_record_service).to receive(:dup_fix).and_return(false)
+
+        # Spy on Rails.logger to check for logging behavior
+        allow(Rails.logger).to receive(:error)
+
+        # Mock SlackService to expect failure notification
+        allow(SlackService).to receive_message_chain(:new, :send_notification)
+      end
+
+      it "logs the error" do
+        # Call remediate! and expect it to return false
+        result = vet_record_service.remediate!
+
+        # Expect the method to return false (indicating failure)
+        expect(result).to be_falsey
+
+        # Ensure that the logger received the expected error message
+        expect(Rails.logger).to have_received(:error).with("dup_fix failed")
+      end
+
+      it "sends a failure notification via Slack" do
+        # Call remediate! and expect it to return false
+        result = vet_record_service.remediate!
+
+        # Expect the method to return false (indicating failure)
+        expect(result).to be_falsey
+
+        # Verify SlackService sends the failure notification
+        expect(SlackService.new).to have_received(:send_notification).
+          with("Job failed during record update", "Error in Remediations::VeteranRecordRemediationService")
+      end
+
+      it "does not destroy the duplicate veterans" do
+        # Call remediate! and expect it to return false
+        result = vet_record_service.remediate!
+
+        # Expect the method to return false (indicating failure)
+        expect(result).to be_falsey
+
+        # Ensure that destroy! is not called on the duplicate veterans due to the failure
+        expect(duplicate_veteran).not_to have_received(:destroy!)
+      end
+    end
   end
 end