Merge pull request #4299 from nhsuk/fix-patient-changeset-bugs

Fix PatientChangeset bugs in importing parents and parent_relationships. Only perform PDS search if postcode present in upload.

Author: murugapl

app/jobs/process_patient_changesets_job.rb

@@ -29,6 +29,7 @@ def perform(patient_changeset)
   private
 
   def search_for_patient(attrs)
+    return nil if attrs["address_postcode"].blank?
     PDS::Patient.search(
       family_name: attrs["family_name"],
       given_name: attrs["given_name"],

app/models/patient_changeset.rb

@@ -102,14 +102,14 @@ def parents
         parent =
           Parent.match_existing(
             patient: existing_patients.first,
-            email: attrs[:email],
-            phone: attrs[:phone],
-            full_name: attrs[:full_name]
+            email: attrs["email"],
+            phone: attrs["phone"],
+            full_name: attrs["full_name"]
           ) || Parent.new
 
-        parent.email = attrs[:email] if attrs[:email]
-        parent.full_name = attrs[:full_name] if attrs[:full_name]
-        parent.phone = attrs[:phone] if attrs[:phone]
+        parent.email = attrs["email"] if attrs["email"]
+        parent.full_name = attrs["full_name"] if attrs["full_name"]
+        parent.phone = attrs["phone"] if attrs["phone"]
         parent.phone_receive_updates = false if parent.phone.blank?
 
         parent
@@ -121,8 +121,7 @@ def parent_relationships
       [
         parent_1_attributes["relationship"],
         parent_2_attributes["relationship"]
-      ].compact
-        .map { parent_relationship_attributes(it) }
+      ].map { parent_relationship_attributes(it) }
         .zip(parents)
         .map do |relationship, parent|
           ParentRelationship

spec/features/import_child_pds_lookup_extravaganza_spec.rb

@@ -14,28 +14,50 @@
     when_i_visit_the_import_page
     and_i_upload_import_file("pds_extravaganza.csv")
     then_i_should_see_the_import_page
-    and_i_should_see_one_new_patient_created
+    and_i_should_see_correct_patient_counts
 
     # Case 1: Patient with existing NHS number (Albert) - nothing should happen
     and_i_see_the_patient_uploaded_with_nhs_number
+    and_parents_are_created_for_albert
 
     # Case 2: Existing patient without NHS number (Betty) - should not show duplicate review
     and_i_do_not_see_an_import_review_for_the_first_patient_uploaded_without_nhs_number
     when_i_click_on_the_patient_without_review
     then_i_see_the_new_patient_has_an_nhs_number
+    and_betty_has_correct_parent_relationships
 
     # Case 3: Existing patient with NHS number (Catherine) - should show duplicate review
     when_i_go_back_to_the_import_page
     then_i_see_an_import_review_for_the_second_patient_uploaded_without_nhs_number
     when_i_click_review
     then_i_see_both_records_have_an_nhs_number
+    and_i_see_address_differences_for_review
     when_i_use_duplicate_record_during_merge
     then_the_existing_patient_has_an_nhs_number_in_mavis
+    and_catherine_parents_are_handled_correctly
 
     # Case 4: New patient without NHS number (Charlie) - should be created with NHS number from PDS
     when_i_go_back_to_the_import_page
     when_i_click_on_new_patient_uploaded_without_an_nhs_number
     then_i_see_the_new_patient_now_has_an_nhs_number
+    and_charlie_has_no_parents_as_expected
+
+    # Case 5: Home educated patient (Emma) - test school move handling
+    when_i_go_back_to_the_import_page
+    when_i_click_on_home_educated_patient
+    then_i_see_home_educated_patient_details
+    and_emma_has_correct_parent_data
+
+    # Case 6: Patient with parent but no relationship specified (Oliver)
+    when_i_go_back_to_the_import_page
+    when_i_click_on_patient_with_unknown_relationship
+    then_i_see_patient_with_unknown_relationship_details
+    and_oliver_has_unknown_relationship_parent
+
+    then_school_moves_are_created_appropriately
+
+    and_all_parent_relationships_are_established
+    and_import_counts_are_correct
   end
 
   def given_i_am_signed_in
@@ -52,9 +74,12 @@ def given_i_am_signed_in
 
   def and_an_hpv_programme_is_underway
     @school = create(:school, urn: "123456", team: @team)
+    @clinic = create(:generic_clinic, team: @team)
 
     @session =
       create(:session, team: @team, location: @school, programmes: [@programme])
+    @clinic_session =
+      create(:session, team: @team, location: @clinic, programmes: [@programme])
   end
 
   def and_an_existing_patient_record_exists
@@ -91,7 +116,16 @@ def and_an_existing_patient_record_exists
         session: @session
       )
 
-    # Catherine - will have different address, causing review
+    @different_school = create(:school, urn: "456789", team: @team)
+    @different_school_session =
+      create(
+        :session,
+        team: @team,
+        location: @different_school,
+        programmes: [@programme]
+      )
+
+    # Catherine - will have different address and school, causing review, and school move
     @existing_patient_duplicate_review =
       create(
         :patient,
@@ -104,11 +138,26 @@ def and_an_existing_patient_record_exists
         address_line_2: "",
         address_town: "Birmingham", # Different from CSV
         address_postcode: "B1 1AA", # Different from CSV
-        school: nil,
-        session: @session
+        school: @different_school,
+        session: @different_school_session
       )
 
+    @existing_parent =
+      create(
+        :parent,
+        full_name: "John Tweedle",
+        email: "john.tweedle@email.com"
+      )
+    create(
+      :parent_relationship,
+      parent: @existing_parent,
+      patient: @existing_patient_uploaded_with_nhs_number,
+      type: "unknown"
+    )
+
     expect(Patient.count).to eq(3)
+    expect(ParentRelationship.count).to eq(1)
+    expect(Parent.count).to eq(1)
   end
 
   def and_pds_lookup_during_import_is_enabled
@@ -137,6 +186,14 @@ def and_pds_lookup_during_import_is_enabled
       "birthdate" => "eq2011-03-15",
       "address-postalcode" => "SW2 2BB"
     )
+
+    stub_pds_search_to_return_a_patient(
+      "9435783309",
+      "family" => "Homeschool",
+      "given" => "Emma",
+      "birthdate" => "eq2010-06-01",
+      "address-postalcode" => "SW3 3AA"
+    )
   end
 
   def when_i_visit_the_import_page
@@ -215,11 +272,187 @@ def then_i_see_the_new_patient_now_has_an_nhs_number
   end
 
   def then_the_existing_patient_has_an_nhs_number_in_mavis
-    expect(Patient.count).to eq(4)
+    expect(Patient.count).to eq(6)
     patient = Patient.where(given_name: "Catherine").first
     expect(patient.nhs_number).to eq("9876543210")
     expect(patient.address_line_1).to eq("456 New Street")
     expect(patient.address_town).to eq("London")
     expect(patient.address_postcode).to eq("SW2 2BB")
   end
+
+  def and_i_should_see_correct_patient_counts
+    perform_enqueued_jobs
+    expect(Patient.count).to eq(6)
+  end
+
+  def and_parents_are_created_for_albert
+    albert = Patient.find_by(given_name: "Albert", family_name: "Tweedle")
+    expect(albert.parents.count).to eq(2)
+    expect(albert.parents.map(&:full_name)).to contain_exactly(
+      "John Tweedle",
+      "Mary Tweedle"
+    )
+    expect(albert.parents.find_by(full_name: "John Tweedle").email).to eq(
+      "john.tweedle@email.com"
+    )
+  end
+
+  def and_betty_has_correct_parent_relationships
+    betty = Patient.find_by(given_name: "Betty", family_name: "Samson")
+    expect(betty.parent_relationships.count).to eq(2)
+
+    dad_relationship =
+      betty
+        .parent_relationships
+        .joins(:parent)
+        .find_by(parents: { full_name: "Robert Samson" })
+    expect(dad_relationship.type).to eq("father")
+
+    mum_relationship =
+      betty
+        .parent_relationships
+        .joins(:parent)
+        .find_by(parents: { full_name: "Linda Samson" })
+    expect(mum_relationship.type).to eq("mother")
+
+    linda = Parent.find_by(full_name: "Linda Samson")
+    expect(linda.phone).to be_blank
+    expect(linda.phone_receive_updates).to be false
+  end
+
+  def and_i_see_address_differences_for_review
+    expect(page).to have_content("999 Old Street") # Original address
+    expect(page).to have_content("456 New Street") # New address from CSV
+    expect(page).to have_content("Birmingham") # Original town
+    expect(page).to have_content("London") # New town from CSV
+  end
+
+  def and_catherine_parents_are_handled_correctly
+    catherine =
+      Patient.find_by(given_name: "Catherine", family_name: "Williams")
+    expect(catherine.parents.count).to eq(2)
+
+    guardian = catherine.parents.find_by(full_name: "David Williams")
+    guardian_relationship =
+      catherine.parent_relationships.find_by(parent: guardian)
+    expect(guardian_relationship.type).to eq("guardian")
+
+    mother = catherine.parents.find_by(full_name: "Sarah Williams")
+    expect(mother.phone).to be_present
+  end
+
+  def and_charlie_has_no_parents_as_expected
+    charlie = Patient.find_by(given_name: "Charlie", family_name: "Brown")
+    expect(charlie.parents.count).to eq(0)
+    expect(charlie.parent_relationships.count).to eq(0)
+  end
+
+  def when_i_click_on_home_educated_patient
+    click_link "HOMESCHOOL, Emma"
+  end
+
+  def then_i_see_home_educated_patient_details
+    expect(page).to have_content("HOMESCHOOL, Emma")
+    expect(page).to have_content("1 June 2010")
+  end
+
+  def and_emma_has_correct_parent_data
+    emma = Patient.find_by(given_name: "Emma", family_name: "Homeschool")
+    expect(emma.parents.count).to eq(1)
+
+    father = emma.parents.first
+    expect(father.full_name).to eq("Mike HomeDad")
+    expect(father.email).to eq("mike@home.com")
+
+    relationship = emma.parent_relationships.first
+    expect(relationship.type).to eq("father")
+  end
+
+  def then_school_moves_are_created_appropriately
+    perform_enqueued_jobs
+
+    charlie = Patient.find_by(given_name: "Charlie")
+    charlie_move = SchoolMoveLogEntry.find_by(patient: charlie)
+    expect(charlie_move).to be_present
+    expect(charlie_move.school).to eq(@school)
+
+    emma = Patient.find_by(given_name: "Emma")
+    emma_move = SchoolMoveLogEntry.find_by(patient: emma)
+    expect(emma_move).to be_present
+    expect(emma_move.home_educated).to be true
+    expect(emma_move.school).to be_nil
+
+    catherine =
+      Patient.find_by(given_name: "Catherine", family_name: "Williams")
+    catherine_move = SchoolMove.find_by(patient: catherine, school: @school)
+    expect(catherine_move).to be_present
+    catherine_log_entry = SchoolMoveLogEntry.find_by(patient: catherine)
+    expect(catherine_log_entry).to be_nil
+    expect(catherine.school).to eq(@different_school)
+  end
+
+  def and_all_parent_relationships_are_established
+    expect(Parent.count).to eq(8)
+    expect(ParentRelationship.count).to eq(8)
+
+    father_relationships = ParentRelationship.where(type: "father")
+    expect(father_relationships.count).to eq(3) # John Tweedle, Mike HomeDad, Robert Samson
+
+    mother_relationships = ParentRelationship.where(type: "mother")
+    expect(mother_relationships.count).to eq(3) # Mary Tweedle, Linda Samson, Sarah Williams
+
+    guardian_relationships = ParentRelationship.where(type: "guardian")
+    expect(guardian_relationships.count).to eq(1) # David Williams
+
+    unknown_relationships = ParentRelationship.where(type: "unknown")
+    expect(unknown_relationships.count).to eq(1) # Jane Doe
+  end
+
+  def and_import_counts_are_correct
+    import = CohortImport.last
+    expect(import.patients.count).to eq(6)
+  end
+
+  def when_i_click_on_patient_with_unknown_relationship
+    click_link "GREEN, Oliver"
+  end
+
+  def then_i_see_patient_with_unknown_relationship_details
+    expect(page).to have_content("GREEN, Oliver")
+    expect(page).to have_content("15 August 2010")
+  end
+
+  def and_oliver_has_unknown_relationship_parent
+    oliver = Patient.find_by(given_name: "Oliver", family_name: "Green")
+    expect(oliver.parents.count).to eq(1)
+
+    parent = oliver.parents.first
+    expect(parent.full_name).to eq("Jane Doe")
+    expect(parent.email).to be_blank
+
+    relationship = oliver.parent_relationships.first
+    expect(relationship.type).to eq("unknown")
+    expect(relationship.label).to eq("Unknown")
+  end
+  def when_i_click_on_patient_with_unknown_relationship
+    click_link "GREEN, Oliver"
+  end
+
+  def then_i_see_patient_with_unknown_relationship_details
+    expect(page).to have_content("GREEN, Oliver")
+    expect(page).to have_content("15 August 2010")
+  end
+
+  def and_oliver_has_unknown_relationship_parent
+    oliver = Patient.find_by(given_name: "Oliver", family_name: "Green")
+    expect(oliver.parents.count).to eq(1)
+
+    parent = oliver.parents.first
+    expect(parent.full_name).to eq("Jane Doe")
+    expect(parent.email).to be_blank
+
+    relationship = oliver.parent_relationships.first
+    expect(relationship.type).to eq("unknown")
+    expect(relationship.label).to eq("Unknown")
+  end
 end

spec/fixtures/cohort_import/pds_extravaganza.csv

@@ -1,5 +1,7 @@
 CHILD_SCHOOL_URN,PARENT_1_NAME,PARENT_1_RELATIONSHIP,PARENT_1_EMAIL,PARENT_1_PHONE,PARENT_2_NAME,PARENT_2_RELATIONSHIP,PARENT_2_EMAIL,PARENT_2_PHONE,CHILD_FIRST_NAME,CHILD_LAST_NAME,CHILD_PREFERRED_GIVEN_NAME,CHILD_DATE_OF_BIRTH,CHILD_YEAR_GROUP,CHILD_ADDRESS_LINE_1,CHILD_ADDRESS_LINE_2,CHILD_TOWN,CHILD_POSTCODE,CHILD_NHS_NUMBER
-123456,,,,,,,,,Albert,Tweedle,,2009-12-29,8,38A Battersea Rise,,London,SW11 1EH,9999075320
-123456,,,,,,,,,Betty,Samson,Beth,2010-01-01,8,123 High Street,,London,SW1A 1AA,
-123456,,,,,,,,,Catherine,Williams,,2009-05-15,8,456 New Street,,London,SW2 2BB,
-123456,,,,,,,,,Charlie,Brown,,2011-03-15,9,42 Wallaby Way,,London,SW2 2BB,
+123456,John Tweedle,Father,john.tweedle@email.com,01234567890,Mary Tweedle,Mother,mary.tweedle@email.com,01234567891,Albert,Tweedle,,2009-12-29,8,38A Battersea Rise,,London,SW11 1EH,9999075320
+123456,Robert Samson,Dad,bob.samson@email.com,01234567892,Linda Samson,Mum,linda.samson@email.com,,Betty,Samson,Beth,2010-01-01,8,123 High Street,,London,SW1A 1AA,
+123456,David Williams,Guardian,david.w@email.com,,Sarah Williams,Mother,sarah.w@email.com,01234567894,Catherine,Williams,,2009-05-15,8,456 New Street,,London,SW2 2BB,
+123456,,,,,,,,,Charlie,Brown,,2011-03-15,9,42 Wallaby Way,,London,SW2 2BB,
+999999,Mike HomeDad,Father,mike@home.com,01234567895,,,,,Emma,Homeschool,,2010-06-01,8,123 Home Street,,London,SW3 3AA,
+123456,Jane Doe,,,01234567896,,,,,Oliver,Green,,2010-08-15,8,789 Silent Street,,London,SW1W 8JL,9435753868