Make ImportantNoticeGeneratorJob idempotent

This ensures that the job can be run multiple times for the same patient
and not raise an error about duplicate indexes.

Jira-Issue: MAV-2309

Author: thomasleese

app/jobs/important_notice_generator_job.rb

@@ -33,9 +33,7 @@ def process_batch(patients)
     patient_ids = patients.map(&:id)
 
     existing_notices =
-      ImportantNotice
-        .where(patient_id: patient_ids)
-        .index_by { |n| notice_key(n) }
+      ImportantNotice.where(patient_id: patient_ids).index_by { notice_key(it) }
 
     patient_team_ids =
       patients.each_with_object({}) do |patient, hash|
@@ -53,7 +51,7 @@ def process_batch(patients)
     end
 
     if notices_to_create.any?
-      ImportantNotice.import(notices_to_create, validate: false)
+      ImportantNotice.import!(notices_to_create, on_duplicate_key_ignore: true)
     end
 
     if notice_ids_to_dismiss.any?

spec/jobs/important_notice_generator_job_spec.rb

@@ -11,39 +11,45 @@
   let(:patient) { create(:patient) }
 
   before do
-    # Associate patient with teams through locations
     create(:patient_location, patient:, session: session_a)
     create(:patient_location, patient:, session: session_b)
   end
 
   describe "#perform" do
+    subject(:perform) { described_class.new.perform(patient.id) }
+
     context "when patient exists in multiple teams" do
       context "deceased" do
-        it "creates deceased notices for all associated teams" do
+        before do
           patient.update_columns(
             # if we use update! the jobs gets called automatically
             date_of_death: Time.current,
             date_of_death_recorded_at: Time.current
           )
+        end
 
-          expect { described_class.new.perform(patient.id) }.to change {
-            team_a.important_notices.count
-          }.by(1).and change { team_b.important_notices.count }.by(1)
+        it "creates deceased notices for all associated teams" do
+          expect { perform }.to change { team_a.important_notices.count }.by(
+            1
+          ).and change { team_b.important_notices.count }.by(1)
 
           expect(team_a.important_notices.first.type).to eq("deceased")
           expect(team_a.important_notices.first.message).to eq(
             "Record updated with child’s date of death"
           )
+
+          expect { described_class.new.perform(patient.id) }.to not_change(
+            team_a.important_notices,
+            :count
+          ).and not_change(team_b.important_notices, :count)
         end
       end
 
       context "restricted" do
+        before { patient.update_column(:restricted_at, Time.current) }
+
         it "creates restricted notices for all associated teams" do
-          patient.update_column(:restricted_at, Time.current)
-          expect { described_class.new.perform(patient.id) }.to change(
-            ImportantNotice,
-            :count
-          ).by(2)
+          expect { perform }.to change(ImportantNotice, :count).by(2)
 
           notices = patient.important_notices.where(type: :restricted)
           expect(notices.pluck(:team_id)).to contain_exactly(
@@ -55,11 +61,13 @@
         end
 
         context "patient is no longer restricted" do
-          it "dismisses existing restricted notices" do
+          before do
             patient.update!(restricted_at: Time.current)
             patient.update_column(:restricted_at, nil)
+          end
 
-            expect { described_class.new.perform(patient.id) }.to change {
+          it "dismisses existing restricted notices" do
+            expect { perform }.to change {
               ImportantNotice
                 .active(team: team_a)
                 .where(patient:, type: :restricted)
@@ -70,13 +78,10 @@
       end
 
       context "invalidated" do
-        it "creates invalidated notices for all associated teams" do
-          patient.update_column(:invalidated_at, Time.current)
+        before { patient.update_column(:invalidated_at, Time.current) }
 
-          expect { described_class.new.perform(patient.id) }.to change(
-            ImportantNotice,
-            :count
-          ).by(2)
+        it "creates invalidated notices for all associated teams" do
+          expect { perform }.to change(ImportantNotice, :count).by(2)
 
           notices = patient.important_notices.where(type: :invalidated)
           expect(notices.pluck(:team_id)).to contain_exactly(
@@ -88,11 +93,13 @@
         end
 
         context "patient is no longer invalidated" do
-          it "dismisses existing invalidated notices" do
+          before do
             patient.update!(invalidated_at: Time.current)
             patient.update_column(:invalidated_at, nil)
+          end
 
-            expect { described_class.new.perform(patient.id) }.to change {
+          it "dismisses existing invalidated notices" do
+            expect { perform }.to change {
               ImportantNotice
                 .active(team: team_a)
                 .where(patient:, type: :invalidated)