From a9148b36f31206149fd80a62955906d3fa5fb414 Mon Sep 17 00:00:00 2001 From: Alistair White-Horne Date: Wed, 23 Apr 2025 11:40:19 +0100 Subject: [PATCH] Automatically accept incoming changes which only differ in case --- .../concerns/pending_changes_concern.rb | 20 ++++- spec/models/class_import_row_spec.rb | 69 ++++++++++++++++ spec/models/class_import_spec.rb | 15 ++++ spec/models/cohort_import_row_spec.rb | 82 ++++++++++++++++++- spec/models/cohort_import_spec.rb | 33 +++++++- spec/models/immunisation_import_row_spec.rb | 27 ++++++ 6 files changed, 239 insertions(+), 7 deletions(-) diff --git a/app/models/concerns/pending_changes_concern.rb b/app/models/concerns/pending_changes_concern.rb index 618ae6e366..307f201754 100644 --- a/app/models/concerns/pending_changes_concern.rb +++ b/app/models/concerns/pending_changes_concern.rb @@ -6,17 +6,33 @@ module PendingChangesConcern included { attribute :pending_changes, :jsonb, default: {} } def stage_changes(attributes) + need_save = false + new_pending_changes = attributes.each_with_object({}) do |(attr, new_value), staged_changes| current_value = public_send(attr) - staged_changes[attr.to_s] = new_value if new_value != current_value + + if normalise_for_comparison(new_value) == + normalise_for_comparison(current_value) + if new_value != current_value + public_send("#{attr}=", new_value) + pending_changes.delete(attr.to_s) + need_save = true + end + else + staged_changes[attr.to_s] = new_value + end end - if new_pending_changes.any? + if new_pending_changes.any? || need_save update!(pending_changes: pending_changes.merge(new_pending_changes)) end end + def normalise_for_comparison(value) + value.respond_to?(:downcase) ? value.downcase : value + end + def with_pending_changes return self if pending_changes.blank? diff --git a/spec/models/class_import_row_spec.rb b/spec/models/class_import_row_spec.rb index 6bc4fe77ff..3cc6ccea09 100644 --- a/spec/models/class_import_row_spec.rb +++ b/spec/models/class_import_row_spec.rb @@ -177,6 +177,22 @@ expect(parents.first.full_name).to eq("John Smith") end end + + context "when uploading different caps name" do + let!(:existing_parent) do + create(:parent, full_name: "JENNY SMITH", email: "jenny@example.com") + end + + let(:capitalised_parent_2_data) do + { + "PARENT_2_EMAIL" => "jenny@example.com", + "PARENT_2_NAME" => "Jenny Smith" + } + end + let(:data) { valid_data.merge(capitalised_parent_2_data) } + + it { should include(existing_parent) } + end end describe "#to_patient" do @@ -511,6 +527,59 @@ expect(patient.pending_changes).to be_empty end end + + context "with an existing patient with different capitalisation" do + let(:data) do + { + "CHILD_ADDRESS_LINE_1" => "10 Downing Street", + "CHILD_PREFERRED_FIRST_NAME" => "Jim", + "CHILD_DATE_OF_BIRTH" => "2010-01-01", + "CHILD_FIRST_NAME" => "Jimmy", + "CHILD_GENDER" => "Male", + "CHILD_LAST_NAME" => "Smith", + "CHILD_PREFERRED_LAST_NAME" => "Smithy", + "CHILD_NHS_NUMBER" => "9990000018", + "CHILD_POSTCODE" => "sw1a 1aa", + "CHILD_TOWN" => "London" + } + end + + let!(:existing_patient) do + create( + :patient, + address_postcode: "SW1A 1AA", + family_name: "SMITH", + gender_code: "male", + given_name: "JIMMY", + nhs_number: "9990000018", + address_line_1: "10 DOWNING STREET", + preferred_given_name: "JIM", + preferred_family_name: "SMITHY", + date_of_birth: Date.new(2010, 1, 1), + address_town: "LONDON" + ) + end + + it { should eq(existing_patient) } + + it "saves the incoming values" do + expect(patient).to have_attributes( + address_postcode: "SW1A 1AA", + family_name: "Smith", + gender_code: "male", + given_name: "Jimmy", + nhs_number: "9990000018", + address_line_1: "10 Downing Street", + preferred_given_name: "Jim", + preferred_family_name: "Smithy", + address_town: "London" + ) + end + + it "doesn't stage the capitalisation differences" do + expect(patient.pending_changes).to be_empty + end + end end describe "#to_parent_relationships" do diff --git a/spec/models/class_import_spec.rb b/spec/models/class_import_spec.rb index edac41ccca..a8f92baf84 100644 --- a/spec/models/class_import_spec.rb +++ b/spec/models/class_import_spec.rb @@ -332,6 +332,21 @@ end end + context "with an existing parent matching the name but a different case" do + let!(:existing_parent) do + create(:parent, full_name: "JOHN smith", email: "john@example.com") + end + + it "doesn't create an additional parent" do + expect { process! }.to change(Parent, :count).by(4) + end + + it "changes the parent's name to the incoming version" do + process! + expect(existing_parent.reload.full_name).to eq("John Smith") + end + end + context "with an existing patient in a different school" do let(:patient) do create(:patient, nhs_number: "9990000018", school: create(:school)) diff --git a/spec/models/cohort_import_row_spec.rb b/spec/models/cohort_import_row_spec.rb index 863054e689..c2d4f8a5b2 100644 --- a/spec/models/cohort_import_row_spec.rb +++ b/spec/models/cohort_import_row_spec.rb @@ -178,6 +178,22 @@ ) end end + + context "when uploading different caps name" do + let(:capitalised_parent_2_data) do + { + "PARENT_2_EMAIL" => "jenny@example.com", + "PARENT_2_NAME" => "Jenny Smith" + } + end + let(:data) { valid_data.merge(capitalised_parent_2_data) } + + let!(:existing_parent) do + create(:parent, full_name: "JENNY SMITH", email: "jenny@example.com") + end + + it { should eq([existing_parent]) } + end end describe "#to_patient" do @@ -242,6 +258,7 @@ family_name: "Smith", gender_code: "not_known", given_name: "Jimmy", + preferred_given_name: "Jim", nhs_number: "9990000018", address_line_1: "10 Downing Street", address_line_2: "", @@ -267,14 +284,15 @@ let!(:existing_patient) do create( :patient, + address_line_1: "10 Downing Street", + address_line_2: "", + address_town: "London", address_postcode: "SW1A 1AA", + given_name: "Jimmy", family_name: "Smith", + preferred_given_name: "Jim", gender_code: "female", - given_name: "Jimmy", nhs_number: "9990000018", - address_line_1: "10 Downing Street", - address_line_2: "", - address_town: "London", birth_academic_year: 2009, date_of_birth: Date.new(2010, 1, 1), registration: "8AB" @@ -366,6 +384,7 @@ :patient, family_name: "Smith", given_name: "Jimmy", + preferred_given_name: "Jim", gender_code: "male", nhs_number: "9990000018", birth_academic_year: 2009, @@ -400,6 +419,7 @@ :patient, family_name: "Smith", given_name: "Jimmy", + preferred_given_name: "Jim", gender_code: "male", nhs_number: "9990000018", birth_academic_year: 2009, @@ -465,6 +485,60 @@ ) end end + + context "with an existing patient with different capitalisation" do + let(:data) do + { + "CHILD_ADDRESS_LINE_1" => "10 Downing Street", + "CHILD_PREFERRED_FIRST_NAME" => "Jim", + "CHILD_DATE_OF_BIRTH" => "2010-01-01", + "CHILD_FIRST_NAME" => "Jimmy", + "CHILD_GENDER" => "Male", + "CHILD_LAST_NAME" => "Smith", + "CHILD_PREFERRED_LAST_NAME" => "Smithy", + "CHILD_NHS_NUMBER" => "9990000018", + "CHILD_POSTCODE" => "sw1a 1aa", + "CHILD_SCHOOL_URN" => school_urn, + "CHILD_TOWN" => "London" + } + end + + let!(:existing_patient) do + create( + :patient, + address_postcode: "SW1A 1AA", + family_name: "SMITH", + gender_code: "male", + given_name: "JIMMY", + nhs_number: "9990000018", + address_line_1: "10 DOWNING STREET", + preferred_given_name: "JIM", + preferred_family_name: "SMITHY", + date_of_birth: Date.new(2010, 1, 1), + address_town: "LONDON" + ) + end + + it { should eq(existing_patient) } + + it "saves the incoming values" do + expect(patient).to have_attributes( + address_postcode: "SW1A 1AA", + family_name: "Smith", + gender_code: "male", + given_name: "Jimmy", + nhs_number: "9990000018", + address_line_1: "10 Downing Street", + preferred_given_name: "Jim", + preferred_family_name: "Smithy", + address_town: "London" + ) + end + + it "doesn't stage the capitalisation differences" do + expect(patient.pending_changes).to be_empty + end + end end describe "#to_school_move" do diff --git a/spec/models/cohort_import_spec.rb b/spec/models/cohort_import_spec.rb index d9d7be2c27..897d64b41b 100644 --- a/spec/models/cohort_import_spec.rb +++ b/spec/models/cohort_import_spec.rb @@ -298,7 +298,7 @@ end context "with an existing patient matching the name but a different case" do - before do + let!(:existing_patient) do create( :patient, given_name: "JIMMY", @@ -312,6 +312,37 @@ it "doesn't create an additional patient" do expect { process! }.to change(Patient, :count).by(2) end + + it "doesn't stage the changes to the names" do + process! + expect(existing_patient.reload.pending_changes).not_to have_key( + "given_name" + ) + expect(existing_patient.reload.pending_changes).not_to have_key( + "family_name" + ) + end + + it "automatically accepts the incoming case differences" do + process! + expect(existing_patient.reload.given_name).to eq("Jimmy") + expect(existing_patient.reload.family_name).to eq("Smith") + end + end + + context "with an existing parent matching the name but a different case" do + let!(:existing_parent) do + create(:parent, full_name: "JOHN smith", email: "john@example.com") + end + + it "doesn't create an additional parent" do + expect { process! }.to change(Parent, :count).by(2) + end + + it "changes the parent's name to the incoming version" do + process! + expect(existing_parent.reload.full_name).to eq("John Smith") + end end context "with an existing patient that was previously removed from cohort" do diff --git a/spec/models/immunisation_import_row_spec.rb b/spec/models/immunisation_import_row_spec.rb index 19a1ff94b4..2aff502410 100644 --- a/spec/models/immunisation_import_row_spec.rb +++ b/spec/models/immunisation_import_row_spec.rb @@ -1487,6 +1487,33 @@ end end + context "with an existing matching patient but mismatching capitalisation, without NHS number" do + let(:data) do + valid_data.except("NHS_NUMBER").merge( + { + "PERSON_FORENAME" => "RON", + "PERSON_SURNAME" => "WEASLEY", + "PERSON_POSTCODE" => "sw1a 1aa" + } + ) + end + + let!(:existing_patient) do + create( + :patient, + given_name: "Ron", + family_name: "Weasley", + date_of_birth: Date.parse(date_of_birth), + address_postcode:, + nhs_number: "9990000018" + ) + end + + it "still matches to a patient" do + expect(patient).to eq(existing_patient) + end + end + describe "#address_postcode" do subject { patient.address_postcode }