Merge pull request #6043 from nhsuk/fix-changesets-keep-both-records

Scope changesets to processed when keeping both records in import duplicates

Author: murugapl

app/forms/import_duplicate_form.rb

@@ -66,10 +66,29 @@ def discard_pending_changes!
   end
 
   def keep_both_changes!
-    if can_keep_both? && can_apply?
-      changeset = object.changesets.includes(:import).order(:created_at).last
-      object.apply_pending_changes_to_new_record!(changeset:)
-    end
+    return unless can_keep_both?
+    return unless can_apply?
+
+    object.apply_pending_changes_to_new_record!(
+      changeset: changeset_for_keep_both
+    )
+  end
+
+  def changeset_for_keep_both
+    scope = object.changesets.includes(:import).order(:created_at)
+
+    return scope.last unless Flipper.enabled?(:import_review_screen)
+
+    completed_import_statuses = %w[
+      processed
+      partially_processed
+      removing_parent_relationships
+    ]
+
+    scope
+      .processed
+      .select { completed_import_statuses.include?(it.import&.status) }
+      .last
   end
 
   def reset_count!

spec/forms/import_duplicate_form_spec.rb

@@ -55,13 +55,21 @@
             :class_import,
             :import_issue,
             import: class_import,
-            patient: existing_patient
+            patient: existing_patient,
+            status: :processed
           )
         end
 
         before do
           existing_patient.update!(pending_changes: { "given_name" => "Twin" })
           class_import.parent_relationships << import_relationship
+          class_import.update!(
+            status: :processed,
+            processed_at: Time.current,
+            new_record_count: 0,
+            changed_record_count: 0,
+            exact_duplicate_record_count: 0
+          )
         end
 
         it "moves imported parent relationships to the new patient when keeping both" do
@@ -203,4 +211,68 @@
       end
     end
   end
+
+  describe "#changeset_for_keep_both" do
+    subject(:selected_changeset) { form.send(:changeset_for_keep_both) }
+
+    let(:team) { create(:team, programmes: [programme]) }
+    let(:session) { create(:session, team:, programmes: [programme]) }
+    let(:existing_patient) { create(:patient) }
+
+    let(:form) do
+      described_class.new(
+        apply_changes: "keep_both",
+        object: existing_patient,
+        current_team: team
+      )
+    end
+
+    let(:completed_import) { create(:class_import, :processed, session:) }
+
+    let(:incomplete_import) do
+      create(:class_import, session:, status: :in_review)
+    end
+
+    let!(:eligible_old) do
+      create(
+        :patient_changeset,
+        :class_import,
+        :import_issue,
+        import: completed_import,
+        patient: existing_patient,
+        status: :processed,
+        matched_on_nhs_number: false,
+        created_at: 3.minutes.ago
+      )
+    end
+
+    let!(:newest_processed_but_incomplete_import) do
+      create(
+        :patient_changeset,
+        :class_import,
+        :import_issue,
+        import: incomplete_import,
+        patient: existing_patient,
+        status: :processed,
+        matched_on_nhs_number: false,
+        created_at: 1.minute.ago
+      )
+    end
+
+    context "when import_review_screen is enabled" do
+      before { Flipper.enable(:import_review_screen) }
+
+      it "returns the latest processed changeset from a completed import" do
+        expect(selected_changeset.id).to eq(eligible_old.id)
+      end
+    end
+
+    context "when import_review_screen is disabled" do
+      it "returns the latest changeset regardless of statuses" do
+        expect(selected_changeset.id).to eq(
+          newest_processed_but_incomplete_import.id
+        )
+      end
+    end
+  end
 end