Skip to content

Commit 7e8b862

Browse files
committed
Scope changesets to processed when keeping both records in import duplicates
When resolving import duplicate issues by keeping both records (creating a new patient from pending changes), only reassigned the latests processed changeset to the new record. This prevents changesets that are still being reviewed from being incorrectly reassigned. Previously, the code selected the most recent changeset without checking its status. If another CSV import was uploaded for the same patient while the duplicate resolution was in progress, the unprocessed changeset from the new import would be incorrectly reassigned to the newly created patient (instead of staying with the original patient record).
1 parent 1ba7424 commit 7e8b862

2 files changed

Lines changed: 96 additions & 5 deletions

File tree

app/forms/import_duplicate_form.rb

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -66,10 +66,29 @@ def discard_pending_changes!
6666
end
6767

6868
def keep_both_changes!
69-
if can_keep_both? && can_apply?
70-
changeset = object.changesets.includes(:import).order(:created_at).last
71-
object.apply_pending_changes_to_new_record!(changeset:)
72-
end
69+
return unless can_keep_both?
70+
return unless can_apply?
71+
72+
object.apply_pending_changes_to_new_record!(
73+
changeset: changeset_for_keep_both
74+
)
75+
end
76+
77+
def changeset_for_keep_both
78+
scope = object.changesets.includes(:import).order(:created_at)
79+
80+
return scope.last unless Flipper.enabled?(:import_review_screen)
81+
82+
completed_import_statuses = %w[
83+
processed
84+
partially_processed
85+
removing_parent_relationships
86+
]
87+
88+
scope
89+
.processed
90+
.select { completed_import_statuses.include?(it.import&.status) }
91+
.last
7392
end
7493

7594
def reset_count!

spec/forms/import_duplicate_form_spec.rb

Lines changed: 73 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,13 +55,21 @@
5555
:class_import,
5656
:import_issue,
5757
import: class_import,
58-
patient: existing_patient
58+
patient: existing_patient,
59+
status: :processed
5960
)
6061
end
6162

6263
before do
6364
existing_patient.update!(pending_changes: { "given_name" => "Twin" })
6465
class_import.parent_relationships << import_relationship
66+
class_import.update!(
67+
status: :processed,
68+
processed_at: Time.current,
69+
new_record_count: 0,
70+
changed_record_count: 0,
71+
exact_duplicate_record_count: 0
72+
)
6573
end
6674

6775
it "moves imported parent relationships to the new patient when keeping both" do
@@ -203,4 +211,68 @@
203211
end
204212
end
205213
end
214+
215+
describe "#changeset_for_keep_both" do
216+
subject(:selected_changeset) { form.send(:changeset_for_keep_both) }
217+
218+
let(:team) { create(:team, programmes: [programme]) }
219+
let(:session) { create(:session, team:, programmes: [programme]) }
220+
let(:existing_patient) { create(:patient) }
221+
222+
let(:form) do
223+
described_class.new(
224+
apply_changes: "keep_both",
225+
object: existing_patient,
226+
current_team: team
227+
)
228+
end
229+
230+
let(:completed_import) { create(:class_import, :processed, session:) }
231+
232+
let(:incomplete_import) do
233+
create(:class_import, session:, status: :in_review)
234+
end
235+
236+
let!(:eligible_old) do
237+
create(
238+
:patient_changeset,
239+
:class_import,
240+
:import_issue,
241+
import: completed_import,
242+
patient: existing_patient,
243+
status: :processed,
244+
matched_on_nhs_number: false,
245+
created_at: 3.minutes.ago
246+
)
247+
end
248+
249+
let!(:newest_processed_but_incomplete_import) do
250+
create(
251+
:patient_changeset,
252+
:class_import,
253+
:import_issue,
254+
import: incomplete_import,
255+
patient: existing_patient,
256+
status: :processed,
257+
matched_on_nhs_number: false,
258+
created_at: 1.minute.ago
259+
)
260+
end
261+
262+
context "when import_review_screen is enabled" do
263+
before { Flipper.enable(:import_review_screen) }
264+
265+
it "returns the latest processed changeset from a completed import" do
266+
expect(selected_changeset.id).to eq(eligible_old.id)
267+
end
268+
end
269+
270+
context "when import_review_screen is disabled" do
271+
it "returns the latest changeset regardless of statuses" do
272+
expect(selected_changeset.id).to eq(
273+
newest_processed_but_incomplete_import.id
274+
)
275+
end
276+
end
277+
end
206278
end

0 commit comments

Comments
 (0)