From d177b4bd09ee9c3a45df253772d2eee919a0938f Mon Sep 17 00:00:00 2001
From: Dani <60626984+dcoleman21@users.noreply.github.com>
Date: Wed, 27 Nov 2024 13:34:06 -0500
Subject: [PATCH 01/20] adds error class and retry_on call logic

---
 app/jobs/person_and_veteran_event_remediation_job.rb | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/app/jobs/person_and_veteran_event_remediation_job.rb b/app/jobs/person_and_veteran_event_remediation_job.rb
index a5eaee018c0..a6212c5de2f 100644
--- a/app/jobs/person_and_veteran_event_remediation_job.rb
+++ b/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

From be8b6a54fddcaa472c5fd40aa183fbb1f513cb94 Mon Sep 17 00:00:00 2001
From: Dani <60626984+dcoleman21@users.noreply.github.com>
Date: Wed, 27 Nov 2024 13:34:38 -0500
Subject: [PATCH 02/20] add slack notification error logic

---
 .../remediations/duplicate_person_remediation_service.rb        | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/app/services/remediations/duplicate_person_remediation_service.rb b/app/services/remediations/duplicate_person_remediation_service.rb
index 7dad5262699..aeafd7c8b11 100644
--- a/app/services/remediations/duplicate_person_remediation_service.rb
+++ b/app/services/remediations/duplicate_person_remediation_service.rb
@@ -35,7 +35,9 @@ def find_and_update_records
         end
       end
       true
+      SlackService.new.send_notification("Job completed successfully", self.class.name)
     rescue StandardError => error
+      SlackService.new.send_notification("Job failed with error: #{error.message}", "Error in #{self.class.name}")
       Rails.logger.error "an error occured #{error}"
       false
     end

From 14db10017dcd64c5d4675502760bf7e3247a79e2 Mon Sep 17 00:00:00 2001
From: Dani <60626984+dcoleman21@users.noreply.github.com>
Date: Wed, 27 Nov 2024 13:34:59 -0500
Subject: [PATCH 03/20] add slack notification error logic

---
 app/services/remediations/veteran_record_remediation_service.rb | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/app/services/remediations/veteran_record_remediation_service.rb b/app/services/remediations/veteran_record_remediation_service.rb
index eeb95bcccfd..522e7f01f61 100644
--- a/app/services/remediations/veteran_record_remediation_service.rb
+++ b/app/services/remediations/veteran_record_remediation_service.rb
@@ -34,7 +34,9 @@ def dup_fix(file_number)
       duplicate_veterans_collections = @dups.flat_map { |dup| grab_collections(dup.file_number) }
       update_records!(duplicate_veterans_collections, file_number)
       true
+      SlackService.new.send_notification("Job completed successfully", self.class.name)
     rescue StandardError => error
+      SlackService.new.send_notification("Job failed with error: #{error.message}", "Error in #{self.class.name}")
       Rails.logger.error "an error occured #{error}"
       false
       # sentry log / metabase dashboard

From 78e359824c9497de6b1ba5dcc8b7ee48b6dd13d6 Mon Sep 17 00:00:00 2001
From: Dani <60626984+dcoleman21@users.noreply.github.com>
Date: Mon, 2 Dec 2024 10:23:24 -0500
Subject: [PATCH 04/20] remove unecessary true false return values

---
 .../remediations/duplicate_person_remediation_service.rb        | 2 --
 1 file changed, 2 deletions(-)

diff --git a/app/services/remediations/duplicate_person_remediation_service.rb b/app/services/remediations/duplicate_person_remediation_service.rb
index aeafd7c8b11..1e81cb65b7b 100644
--- a/app/services/remediations/duplicate_person_remediation_service.rb
+++ b/app/services/remediations/duplicate_person_remediation_service.rb
@@ -34,12 +34,10 @@ def find_and_update_records
           records.update_all("#{column}": og_person.participant_id)
         end
       end
-      true
       SlackService.new.send_notification("Job completed successfully", self.class.name)
     rescue StandardError => error
       SlackService.new.send_notification("Job failed with error: #{error.message}", "Error in #{self.class.name}")
       Rails.logger.error "an error occured #{error}"
-      false
     end
   end
 end

From ead4528d074863c9b72d35df21974b0f692b9541 Mon Sep 17 00:00:00 2001
From: Dani <60626984+dcoleman21@users.noreply.github.com>
Date: Mon, 2 Dec 2024 10:24:11 -0500
Subject: [PATCH 05/20] remove unneeded comments

---
 .../remediations/veteran_record_remediation_service.rb         | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/app/services/remediations/veteran_record_remediation_service.rb b/app/services/remediations/veteran_record_remediation_service.rb
index 522e7f01f61..276a2265998 100644
--- a/app/services/remediations/veteran_record_remediation_service.rb
+++ b/app/services/remediations/veteran_record_remediation_service.rb
@@ -30,15 +30,12 @@ def remediate!
 
   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)
-      true
       SlackService.new.send_notification("Job completed successfully", self.class.name)
     rescue StandardError => error
       SlackService.new.send_notification("Job failed with error: #{error.message}", "Error in #{self.class.name}")
       Rails.logger.error "an error occured #{error}"
-      false
       # sentry log / metabase dashboard
     end
   end

From 903366ca3705798e291b142582672377e4d28e71 Mon Sep 17 00:00:00 2001
From: Dani <60626984+dcoleman21@users.noreply.github.com>
Date: Mon, 2 Dec 2024 11:25:09 -0500
Subject: [PATCH 06/20] adds begin-rescue block in remediate! method as well

---
 .../duplicate_person_remediation_service.rb   | 24 +++++++++++++++----
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/app/services/remediations/duplicate_person_remediation_service.rb b/app/services/remediations/duplicate_person_remediation_service.rb
index 1e81cb65b7b..a6128273495 100644
--- a/app/services/remediations/duplicate_person_remediation_service.rb
+++ b/app/services/remediations/duplicate_person_remediation_service.rb
@@ -15,8 +15,19 @@ def initialize(updated_person_id:, duplicate_person_ids:)
   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
 
@@ -34,10 +45,13 @@ def find_and_update_records
           records.update_all("#{column}": og_person.participant_id)
         end
       end
-      SlackService.new.send_notification("Job completed successfully", self.class.name)
+      true # Successfully completed, return true
     rescue StandardError => error
-      SlackService.new.send_notification("Job failed with error: #{error.message}", "Error in #{self.class.name}")
-      Rails.logger.error "an error occured #{error}"
+      # 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
 end

From 33fec4330167d905470122251b9c2d479e20f39a Mon Sep 17 00:00:00 2001
From: Dani <60626984+dcoleman21@users.noreply.github.com>
Date: Mon, 2 Dec 2024 11:25:42 -0500
Subject: [PATCH 07/20] updates test to spy on Rails.logger to ensure it is
 capturing correctly

---
 ...plicate_person_remediation_service_spec.rb | 21 +++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/spec/services/remediations/duplicate_person_remediation_service_spec.rb b/spec/services/remediations/duplicate_person_remediation_service_spec.rb
index a5070309b07..3a661d945dd 100644
--- a/spec/services/remediations/duplicate_person_remediation_service_spec.rb
+++ b/spec/services/remediations/duplicate_person_remediation_service_spec.rb
@@ -67,14 +67,27 @@
 
     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"))
+
+        # 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 "logs the error and does not destroy duplicate persons" do
-        result = service.remediate!
+        # Ensure the error was logged
+        expect(Rails.logger).to have_received(:error).with("An error occurred during remediation: Something went wrong")
+
+        # 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")
 
-        expect(result).to be_falsey
+        # Ensure that duplicate persons are not destroyed
         expect(duplicate_person1).not_to have_received(:destroy!)
         expect(duplicate_person2).not_to have_received(:destroy!)
       end

From 1546c11068478ed34960b1b84fe96a418127855d Mon Sep 17 00:00:00 2001
From: Dani <60626984+dcoleman21@users.noreply.github.com>
Date: Mon, 2 Dec 2024 11:54:03 -0500
Subject: [PATCH 08/20] updates remediate! and dup_fix methods with failure
 logic

---
 .../veteran_record_remediation_service.rb     | 36 ++++++++++++-------
 1 file changed, 24 insertions(+), 12 deletions(-)

diff --git a/app/services/remediations/veteran_record_remediation_service.rb b/app/services/remediations/veteran_record_remediation_service.rb
index 276a2265998..e343150dc47 100644
--- a/app/services/remediations/veteran_record_remediation_service.rb
+++ b/app/services/remediations/veteran_record_remediation_service.rb
@@ -11,18 +11,28 @@ def initialize(before_fn, after_fn)
   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
 
@@ -32,10 +42,12 @@ def dup_fix(file_number)
     begin
       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)
+      # SlackService.new.send_notification("Job completed successfully", self.class.name)
+      true
     rescue StandardError => error
-      SlackService.new.send_notification("Job failed with error: #{error.message}", "Error in #{self.class.name}")
       Rails.logger.error "an error occured #{error}"
+      SlackService.new.send_notification("Job failed with error: #{error.message}", "Error in #{self.class.name}")
+      false # Indicate failure
       # sentry log / metabase dashboard
     end
   end

From 01ede4bc28baabae63d27dffc7158ef7117cc64c Mon Sep 17 00:00:00 2001
From: Dani <60626984+dcoleman21@users.noreply.github.com>
Date: Mon, 2 Dec 2024 11:54:32 -0500
Subject: [PATCH 09/20] updates tests to capture rails logger error and slack
 notificaiton

---
 ...veteran_record_remediation_service_spec.rb | 49 +++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/spec/services/remediations/veteran_record_remediation_service_spec.rb b/spec/services/remediations/veteran_record_remediation_service_spec.rb
index cd002c09c0d..7591279f8e2 100644
--- a/spec/services/remediations/veteran_record_remediation_service_spec.rb
+++ b/spec/services/remediations/veteran_record_remediation_service_spec.rb
@@ -102,5 +102,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

From fc0bf83b159e87589f8290526867f8f0842b1178 Mon Sep 17 00:00:00 2001
From: Dani <60626984+dcoleman21@users.noreply.github.com>
Date: Mon, 2 Dec 2024 16:23:10 -0500
Subject: [PATCH 10/20] adds mock to find and update record method to raise
 exception and and get caught by rescue block

---
 .../remediations/duplicate_person_remediation_service_spec.rb | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/spec/services/remediations/duplicate_person_remediation_service_spec.rb b/spec/services/remediations/duplicate_person_remediation_service_spec.rb
index 3a661d945dd..9021abe5ffd 100644
--- a/spec/services/remediations/duplicate_person_remediation_service_spec.rb
+++ b/spec/services/remediations/duplicate_person_remediation_service_spec.rb
@@ -78,8 +78,12 @@
       end
 
       it "logs the error and does not destroy duplicate persons" do
+        # Run the method that should raise an error
+        service.remediate!
+
         # 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)

From da2c424577735c63d8375ac5bb15b0919a4402bc Mon Sep 17 00:00:00 2001
From: Dani <60626984+dcoleman21@users.noreply.github.com>
Date: Mon, 2 Dec 2024 18:38:36 -0500
Subject: [PATCH 11/20] fix weird merge conflict causing test failure

---
 .../duplicate_person_remediation_service_spec.rb  | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/spec/services/remediations/duplicate_person_remediation_service_spec.rb b/spec/services/remediations/duplicate_person_remediation_service_spec.rb
index 5fa9fd4c3b8..9b1b9d03b3d 100644
--- a/spec/services/remediations/duplicate_person_remediation_service_spec.rb
+++ b/spec/services/remediations/duplicate_person_remediation_service_spec.rb
@@ -70,6 +70,8 @@
         # 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)
 
@@ -77,6 +79,19 @@
         allow(Rails.logger).to receive(:error)
       end
 
+      it "does not update any records and skips destroying duplicate persons" do
+        result = service.remediate!
+
+        mock_records.each do |mock_record|
+          expect(mock_record).not_to have_received(:update!)
+        end
+
+        expect(duplicate_person1).not_to have_received(:destroy!)
+        expect(duplicate_person2).not_to have_received(:destroy!)
+
+        expect(result).to be_falsey
+      end
+
       it "logs the error and does not destroy duplicate persons" do
         # Run the method that should raise an error
         service.remediate!

From 86fcefb6e15b40b1eb3f2b7cb2f3accc23535c03 Mon Sep 17 00:00:00 2001
From: Dani <60626984+dcoleman21@users.noreply.github.com>
Date: Mon, 2 Dec 2024 18:45:49 -0500
Subject: [PATCH 12/20] adds missing end

---
 .../remediations/duplicate_person_remediation_service_spec.rb    | 1 +
 1 file changed, 1 insertion(+)

diff --git a/spec/services/remediations/duplicate_person_remediation_service_spec.rb b/spec/services/remediations/duplicate_person_remediation_service_spec.rb
index 9b1b9d03b3d..da0a3e26a8b 100644
--- a/spec/services/remediations/duplicate_person_remediation_service_spec.rb
+++ b/spec/services/remediations/duplicate_person_remediation_service_spec.rb
@@ -64,6 +64,7 @@
         expect(duplicate_person1).to have_received(:destroy!)
         expect(duplicate_person2).to have_received(:destroy!)
       end
+    end
 
     context "when an error occurs during remediation" do
       before do

From 1dc415de98ef0f33c041750f7afb4c28c16264bf Mon Sep 17 00:00:00 2001
From: Dani <60626984+dcoleman21@users.noreply.github.com>
Date: Mon, 2 Dec 2024 18:55:38 -0500
Subject: [PATCH 13/20] adds missing tests to duplicate person remediation
 service

---
 ...plicate_person_remediation_service_spec.rb | 54 +++++++++++++++----
 1 file changed, 43 insertions(+), 11 deletions(-)

diff --git a/spec/services/remediations/duplicate_person_remediation_service_spec.rb b/spec/services/remediations/duplicate_person_remediation_service_spec.rb
index da0a3e26a8b..e90dd3ad1d4 100644
--- a/spec/services/remediations/duplicate_person_remediation_service_spec.rb
+++ b/spec/services/remediations/duplicate_person_remediation_service_spec.rb
@@ -26,18 +26,37 @@
     }
   end
 
+  let(:mock_records) do
+    records = []
+    column_mapping.map do |klass, column_name|
+      record = instance_double(klass.to_s, id: SecureRandom.random_number(1000), "#{column_name}": "987654321")
+      record2 = instance_double(klass.to_s, id: SecureRandom.random_number(1000), "#{column_name}": "324576891")
+      allow(record).to receive(:update!).with("#{column_name}": updated_person.participant_id)
+      allow(record).to receive(:class).and_return(klass)
+      allow(record).to receive(:attributes).and_return("#{column_name}": "987654321")
+      allow(record2).to receive(:update!).with("#{column_name}": updated_person.participant_id)
+      allow(record2).to receive(:class).and_return(klass)
+      allow(record2).to receive(:attributes).and_return("#{column_name}": "324576891")
+      records.push(record, record2)
+    end
+    records
+  end
+
   before do
     # Mock finding the updated person
     allow(Person).to receive(:find_by).with(id: updated_person.id).and_return(updated_person)
     allow(Person).to receive(:where).with(id: duplicate_person_ids).and_return([duplicate_person1, duplicate_person2])
 
     column_mapping.each do |klass, column_name|
-      records_double = double("ActiveRecord::Relation")
+      relevant_records = mock_records.select { |record| record.class == klass }
+
+      allow(klass).to receive(:where)
+        .with("#{column_name}": %w[987654321 324576891])
+        .and_return(relevant_records)
 
-      # Use symbol keys instead of string keys in where
-      allow(klass).to receive(:where).with(column_name => %w[duplicate_participant_id_1 duplicate_participant_id_2])
-        .and_return(records_double)
-      allow(records_double).to receive(:update_all).with(column_name => updated_person.participant_id)
+      relevant_records.each do |record|
+        allow(record).to receive(:update!).with("#{column_name}": updated_person.participant_id).and_return(true)
+      end
     end
 
     # Mock destroy! on duplicate persons
@@ -48,13 +67,14 @@
   describe "#remediate!" do
     context "when remediation is successful" do
       it "updates all records with duplicate participant_ids to the updated person participant_id" do
-        expect(service.remediate!).to be_truthy
-
+        service.remediate!
         column_mapping.each do |klass, column_name|
-          expect(klass).to have_received(:where)
-            .with(column_name => %w[duplicate_participant_id_1 duplicate_participant_id_2])
-          expect(klass.where(column_name => %w[duplicate_participant_id_1 duplicate_participant_id_2]))
-            .to have_received(:update_all).with(column_name => updated_person.participant_id)
+          relevant_records = mock_records.select { |record| record.class == klass }
+          expect(relevant_records.size).to eq(mock_records.count { |record| record.class == klass })
+
+          relevant_records.each do |mock_record|
+            expect(mock_record).to have_received(:update!).with("#{column_name}": updated_person.participant_id)
+          end
         end
       end
 
@@ -64,6 +84,18 @@
         expect(duplicate_person1).to have_received(:destroy!)
         expect(duplicate_person2).to have_received(:destroy!)
       end
+
+      it "does not update unrelated records" do
+        service.remediate!
+
+        unrelated_records = mock_records.reject do |record|
+          %w[987654321 324576891].include?(record.attributes.values.first)
+        end
+
+        unrelated_records.each do |unrelated_record|
+          expect(unrelated_record).not_to have_received(:update!)
+        end
+      end
     end
 
     context "when an error occurs during remediation" do

From f627dd0075c907cccd78d196cf4a9bd03724bcb0 Mon Sep 17 00:00:00 2001
From: Dani <60626984+dcoleman21@users.noreply.github.com>
Date: Tue, 3 Dec 2024 09:59:50 -0500
Subject: [PATCH 14/20] fix lint errors

---
 .../remediations/duplicate_person_remediation_service.rb       | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/app/services/remediations/duplicate_person_remediation_service.rb b/app/services/remediations/duplicate_person_remediation_service.rb
index 51baf9775ad..51ad8cc3df6 100644
--- a/app/services/remediations/duplicate_person_remediation_service.rb
+++ b/app/services/remediations/duplicate_person_remediation_service.rb
@@ -29,7 +29,8 @@ def remediate!
     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}")
+      SlackService.new.send_notification("Job failed during remediation: #{error.message}",
+                                         "Error in #{self.class.name}")
       false # Indicate failure
     end
   end

From 1ffb0843adafc2c608017528a654c819001b1ae3 Mon Sep 17 00:00:00 2001
From: Dani <60626984+dcoleman21@users.noreply.github.com>
Date: Tue, 3 Dec 2024 10:00:15 -0500
Subject: [PATCH 15/20] fix lint errors

---
 .../remediations/veteran_record_remediation_service_spec.rb   | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/spec/services/remediations/veteran_record_remediation_service_spec.rb b/spec/services/remediations/veteran_record_remediation_service_spec.rb
index 5fd464c52c3..489af255da8 100644
--- a/spec/services/remediations/veteran_record_remediation_service_spec.rb
+++ b/spec/services/remediations/veteran_record_remediation_service_spec.rb
@@ -134,8 +134,8 @@
         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")
+        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

From c5f5eed98c57d0034271a2d33ab209baf8eccc96 Mon Sep 17 00:00:00 2001
From: Dani <60626984+dcoleman21@users.noreply.github.com>
Date: Tue, 3 Dec 2024 12:38:03 -0500
Subject: [PATCH 16/20] remove rails logger from the remediate method

---
 .../duplicate_person_remediation_service.rb    | 18 ++----------------
 1 file changed, 2 insertions(+), 16 deletions(-)

diff --git a/app/services/remediations/duplicate_person_remediation_service.rb b/app/services/remediations/duplicate_person_remediation_service.rb
index 51ad8cc3df6..cb1d77f8dd8 100644
--- a/app/services/remediations/duplicate_person_remediation_service.rb
+++ b/app/services/remediations/duplicate_person_remediation_service.rb
@@ -18,20 +18,8 @@ def initialize(updated_person_id:, duplicate_person_ids:, event_record:)
   end
 
   def remediate!
-    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
+    if find_and_update_records
+      @dup_persons.each(&:destroy!)
     end
   end
 
@@ -52,13 +40,11 @@ def find_and_update_records
           end
         end
       end
-      true # Successfully completed, return true
     rescue StandardError => error
       # 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
 

From 2baa6f10f485ebd59a391ff0619d3b1948f7bdb2 Mon Sep 17 00:00:00 2001
From: Dani <60626984+dcoleman21@users.noreply.github.com>
Date: Tue, 3 Dec 2024 12:38:53 -0500
Subject: [PATCH 17/20] refactor tests to pass with removed logic from
 remediate method

---
 ...plicate_person_remediation_service_spec.rb | 26 +++----------------
 1 file changed, 4 insertions(+), 22 deletions(-)

diff --git a/spec/services/remediations/duplicate_person_remediation_service_spec.rb b/spec/services/remediations/duplicate_person_remediation_service_spec.rb
index e90dd3ad1d4..b1c6fd865af 100644
--- a/spec/services/remediations/duplicate_person_remediation_service_spec.rb
+++ b/spec/services/remediations/duplicate_person_remediation_service_spec.rb
@@ -100,16 +100,9 @@
 
     context "when an error occurs during remediation" do
       before do
-        # 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"))
-
+        # Force `find_and_update_records` to return false to simulate a failure
+        allow(service).to receive(:find_and_update_records).and_return(false)
         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
@@ -126,20 +119,9 @@
       end
 
       it "logs the error and does not destroy duplicate persons" do
-        # Run the method that should raise an error
-        service.remediate!
-
-        # 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")
+        result = service.remediate!
 
-        # Ensure that duplicate persons are not destroyed
+        expect(result).to be_falsey
         expect(duplicate_person1).not_to have_received(:destroy!)
         expect(duplicate_person2).not_to have_received(:destroy!)
       end

From 421d028c6c6124763cb2a85f62a6a6e18b741dd4 Mon Sep 17 00:00:00 2001
From: Dani <60626984+dcoleman21@users.noreply.github.com>
Date: Tue, 3 Dec 2024 12:39:45 -0500
Subject: [PATCH 18/20] edit failure logic in remediate method

---
 .../remediations/veteran_record_remediation_service.rb       | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/app/services/remediations/veteran_record_remediation_service.rb b/app/services/remediations/veteran_record_remediation_service.rb
index 9c1f1df9165..1f7ace801e5 100644
--- a/app/services/remediations/veteran_record_remediation_service.rb
+++ b/app/services/remediations/veteran_record_remediation_service.rb
@@ -20,10 +20,6 @@ def remediate!
         # 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
@@ -43,7 +39,6 @@ def dup_fix(file_number)
     begin
       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}"

From bfb634c6ec8d0b10f45b998cfcc0b5b887bd289b Mon Sep 17 00:00:00 2001
From: Dani <60626984+dcoleman21@users.noreply.github.com>
Date: Tue, 3 Dec 2024 12:40:43 -0500
Subject: [PATCH 19/20] refactor tests to pass for removal of logic in
 remediate method

---
 .../veteran_record_remediation_service_spec.rb             | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/spec/services/remediations/veteran_record_remediation_service_spec.rb b/spec/services/remediations/veteran_record_remediation_service_spec.rb
index 489af255da8..12c45f2f802 100644
--- a/spec/services/remediations/veteran_record_remediation_service_spec.rb
+++ b/spec/services/remediations/veteran_record_remediation_service_spec.rb
@@ -121,9 +121,6 @@
 
         # 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
@@ -132,10 +129,6 @@
 
         # 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

From 0885c1bb819a22fd246e396cec0223175c6cb20f Mon Sep 17 00:00:00 2001
From: Dani <60626984+dcoleman21@users.noreply.github.com>
Date: Tue, 3 Dec 2024 13:01:08 -0500
Subject: [PATCH 20/20] fixes lint error

---
 .../remediations/veteran_record_remediation_service.rb         | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/app/services/remediations/veteran_record_remediation_service.rb b/app/services/remediations/veteran_record_remediation_service.rb
index 1f7ace801e5..91996b45aab 100644
--- a/app/services/remediations/veteran_record_remediation_service.rb
+++ b/app/services/remediations/veteran_record_remediation_service.rb
@@ -28,7 +28,8 @@ def remediate!
     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}")
+      SlackService.new.send_notification("Job failed during remediation: #{error.message}",
+                                         "Error in #{self.class.name}")
       false # Indicate failure
     end
   end