From b7441ef7596124ad92e37cff85700d5708655a9c Mon Sep 17 00:00:00 2001
From: Martin van IJcken <martin.vanijcken@tngtech.com>
Date: Fri, 27 Feb 2026 11:53:27 +0000
Subject: [PATCH 1/9] Add school_move_to_unknown_from_another_team? to prevent
 invalid school moves

Prevents cohort imports from moving children to unknown schools when they're already registered at a known school in another team's area. These children should remain at their current school rather than being moved to an unknown school.
---
 app/models/patient_changeset.rb       |  19 +++++
 spec/models/patient_changeset_spec.rb | 102 ++++++++++++++++++++++++++
 2 files changed, 121 insertions(+)

diff --git a/app/models/patient_changeset.rb b/app/models/patient_changeset.rb
index 99f8c037fb..6d26a9add6 100644
--- a/app/models/patient_changeset.rb
+++ b/app/models/patient_changeset.rb
@@ -272,6 +272,10 @@ def school_move
     @school_move ||=
       begin
         return if patient.deceased?
+        if import_type == "CohortImport" &&
+             school_move_to_unknown_from_another_team?
+          return
+        end
         if patient.new_record? || patient.school != school ||
              patient.home_educated != home_educated ||
              patient.not_in_team?(team:, academic_year:) ||
@@ -287,6 +291,21 @@ def school_move
       end
   end
 
+  def school_move_to_unknown_from_another_team?
+    return false if school.present? || home_educated == true
+
+    return false unless patient.persisted?
+    return false if patient.school.nil? && !patient.home_educated?
+
+    current_teams = patient.teams_via_patient_locations
+    return false if current_teams.empty?
+
+    import_team = import.team
+    return false if import_team && current_teams.include?(import_team)
+
+    true
+  end
+
   def existing_patients
     deserialize_attribute(child_attributes, "date_of_birth", :date)
     deserialize_attribute(child_attributes, "invalidated_at", :datetime)
diff --git a/spec/models/patient_changeset_spec.rb b/spec/models/patient_changeset_spec.rb
index 6a2f02f9b6..387f349b6b 100644
--- a/spec/models/patient_changeset_spec.rb
+++ b/spec/models/patient_changeset_spec.rb
@@ -138,4 +138,106 @@
       end
     end
   end
+
+  describe "#school_move_to_unknown_from_another_team?" do
+    let(:team_a) { create(:team, name: "Team A") }
+    let(:team_b) { create(:team, name: "Team B") }
+    let(:school_in_other_team) do
+      create(:school, name: "School in Team B", team: team_b)
+    end
+
+    context "when new location is a known school" do
+      it "returns false" do
+        expect(changeset.school_move_to_unknown_from_another_team?).to be(false)
+      end
+    end
+
+    context "when new location is home educated" do
+      before do
+        changeset.update!(school: nil)
+        changeset.data["upload"]["home_educated"] = true
+        changeset.save!
+      end
+
+      it "returns false" do
+        expect(changeset.school_move_to_unknown_from_another_team?).to be(false)
+      end
+    end
+
+    context "when new location is unknown school and patient is in a school in another team" do
+      let(:generic_clinic) { create(:generic_clinic, team: team_b) }
+      let!(:existing_patient) do
+        create(:patient, nhs_number: "9990000026", school: school_in_other_team)
+      end
+
+      before do
+        changeset.update!(school: nil, patient: existing_patient)
+        changeset.data["upload"]["home_educated"] = false
+        create(
+          :patient_location,
+          patient: existing_patient,
+          location: generic_clinic,
+          academic_year: AcademicYear.current
+        )
+      end
+
+      it "returns true" do
+        expect(changeset.school_move_to_unknown_from_another_team?).to be(true)
+      end
+    end
+
+    context "when new location is unknown school and patient is in unknown school in another team" do
+      let(:generic_clinic) { create(:generic_clinic, team: team_b) }
+      let!(:existing_patient) do
+        create(
+          :patient,
+          nhs_number: "9990000026",
+          school: nil,
+          home_educated: false
+        )
+      end
+
+      before do
+        changeset.update!(school: nil, patient: existing_patient)
+        changeset.data["upload"]["home_educated"] = false
+        create(
+          :patient_location,
+          patient: existing_patient,
+          location: generic_clinic,
+          academic_year: AcademicYear.current
+        )
+      end
+
+      it "returns false" do
+        expect(changeset.school_move_to_unknown_from_another_team?).to be(false)
+      end
+    end
+
+    context "when new location is unknown school and patient is home educated in another team" do
+      let(:generic_clinic) { create(:generic_clinic, team: team_b) }
+      let!(:existing_patient) do
+        create(
+          :patient,
+          nhs_number: "9990000026",
+          school: nil,
+          home_educated: true
+        )
+      end
+
+      before do
+        create(
+          :patient_location,
+          patient: existing_patient,
+          location: generic_clinic,
+          academic_year: AcademicYear.current
+        )
+        changeset.update!(school: nil, patient: existing_patient)
+        changeset.data["upload"]["home_educated"] = false
+      end
+
+      it "returns true" do
+        expect(changeset.school_move_to_unknown_from_another_team?).to be(true)
+      end
+    end
+  end
 end

From 2b333faed1677f26e663267884cc4bfd0179542a Mon Sep 17 00:00:00 2001
From: Martin van IJcken <martin.vanijcken@tngtech.com>
Date: Fri, 27 Feb 2026 12:09:38 +0000
Subject: [PATCH 2/9] Add skipped school moves to import review component

Displays a dedicated section for children who are skipped during import because they're already registered at a known school in another team's area. These children will not be moved to an unknown school.
---
 .../app_import_review_component.html.erb      | 13 +++++
 app/components/app_import_review_component.rb | 11 ++++-
 .../app_import_review_component_spec.rb       | 49 ++++++++++++++++++-
 3 files changed, 71 insertions(+), 2 deletions(-)

diff --git a/app/components/app_import_review_component.html.erb b/app/components/app_import_review_component.html.erb
index b87384bd83..d6923f792d 100644
--- a/app/components/app_import_review_component.html.erb
+++ b/app/components/app_import_review_component.html.erb
@@ -74,6 +74,19 @@
     <% end %>
 <% end %>
 
+<% if @skipped_school_moves.any? %>
+    <h2 class="nhsuk-heading-m">Records not imported - children at schools in other team areas</h2>
+    <p class="nhsuk-u-reading-width">
+        <%= skipped_school_moves_message %>
+    </p>
+    <%= render AppDetailsComponent.new(expander: true, sticky: true) do |c| %>
+        <% c.with_summary do %>
+            <%= pluralize(@skipped_school_moves.count, "record") %> not imported
+        <% end %>
+        <%= render AppImportReviewRecordsSummaryComponent.new(changesets: @skipped_school_moves) %>
+    <% end %>
+<% end %>
+
 <hr class="nhsuk-section-break nhsuk-section-break--visible nhsuk-section-break--l">
 
 <div class="nhsuk-button-group">
diff --git a/app/components/app_import_review_component.rb b/app/components/app_import_review_component.rb
index af8c9eb18c..4f8d45d979 100644
--- a/app/components/app_import_review_component.rb
+++ b/app/components/app_import_review_component.rb
@@ -7,7 +7,8 @@ def initialize(
     new_records:,
     auto_matched_records:,
     import_issues:,
-    school_moves:
+    school_moves:,
+    skipped_school_moves: []
   )
     @import = import
     @inter_team = inter_team.sort_by(&:row_number)
@@ -18,6 +19,7 @@ def initialize(
     @import_issues = import_issues.sort_by(&:row_number)
     @school_moves = school_moves
     @school_moves_from_file = @school_moves.reject { it.row_number.nil? }
+    @skipped_school_moves = skipped_school_moves.sort_by(&:row_number)
   end
 
   private
@@ -69,6 +71,13 @@ def school_moves_message
     end
   end
 
+  def skipped_school_moves_message
+    count = @skipped_school_moves.count
+    "#{count} #{count > 1 ? "children" : "child"} #{count > 1 ? "are" : "is"} already " \
+      "registered at a school in another team's area. #{count > 1 ? "These children" : "This child"} " \
+      "#{count > 1 ? "remain" : "remains"} at their current school."
+  end
+
   def show_cancel_button?
     @new_records.any? || @auto_matched_records.any? || @import_issues.any? ||
       @school_moves_from_file.any?
diff --git a/spec/components/app_import_review_component_spec.rb b/spec/components/app_import_review_component_spec.rb
index 15cc995fe1..00442ea090 100644
--- a/spec/components/app_import_review_component_spec.rb
+++ b/spec/components/app_import_review_component_spec.rb
@@ -10,7 +10,8 @@
       new_records:,
       auto_matched_records:,
       import_issues:,
-      school_moves:
+      school_moves:,
+      skipped_school_moves:
     )
   end
 
@@ -37,6 +38,7 @@
   let(:import_issues) { [] }
   let(:inter_team) { [] }
   let(:school_moves) { [] }
+  let(:skipped_school_moves) { [] }
 
   shared_examples "section with details" do |title:, summary:, count:|
     it "renders section '#{title}'" do
@@ -255,6 +257,51 @@
     end
   end
 
+  describe "with skipped school moves" do
+    let(:other_team_school) do
+      create(:school, team: other_team, name: "School in Other Team")
+    end
+    let(:skipped_patient) do
+      create(:patient, school: other_team_school, team: other_team)
+    end
+    let(:skipped_school_moves) do
+      [
+        create(
+          :patient_changeset,
+          import:,
+          patient: skipped_patient,
+          school: nil
+        )
+      ]
+    end
+
+    before do
+      skipped_school_moves.first.data["upload"]["home_educated"] = false
+      skipped_school_moves.first.save!
+
+      create(
+        :patient_location,
+        patient: skipped_patient,
+        location: other_team_school
+      )
+    end
+
+    include_examples "section with details",
+                     title:
+                       "Records not imported - children at schools in other team areas",
+                     summary: "1 record not imported",
+                     count: 1
+
+    it "shows the section description" do
+      expect(rendered).to have_content(
+        "1 child is already registered at a school in another team's area"
+      )
+      expect(rendered).to have_content(
+        "This child remains at their current school."
+      )
+    end
+  end
+
   describe "buttons" do
     context "with records to import" do
       let(:new_records) do

From e8f5d352b05d64f50c1d6787f6484dc4b3973144 Mon Sep 17 00:00:00 2001
From: Martin van IJcken <martin.vanijcken@tngtech.com>
Date: Fri, 27 Feb 2026 12:33:18 +0000
Subject: [PATCH 3/9] Display skipped school moves on import screens

This displays the skipped school moves on the import review screen and the
completed import screen.
This will not work if the import review screen flag is off, but given we
want to remove the flag soon, and always have it on, that shouldn't be a problem.
---
 app/controllers/cohort_imports_controller.rb  |  12 ++
 app/models/patient_changeset.rb               |  10 +-
 app/views/imports/show.html.erb               |  22 ++++
 .../import_child_records_review_spec.rb       | 120 ++++++++++++++++++
 4 files changed, 163 insertions(+), 1 deletion(-)
 create mode 100644 spec/features/import_child_records_review_spec.rb

diff --git a/app/controllers/cohort_imports_controller.rb b/app/controllers/cohort_imports_controller.rb
index d3bda7371c..31f698f7e9 100644
--- a/app/controllers/cohort_imports_controller.rb
+++ b/app/controllers/cohort_imports_controller.rb
@@ -71,6 +71,12 @@ def show
         @cohort_import.changesets.includes(:patient).nhs_number_discrepancies
 
       @cancelled = @cohort_import.changesets.from_file.cancelled
+      @skipped_school_moves =
+        @cohort_import
+          .changesets
+          .includes(:patient, patient: :school)
+          .from_file
+          .skipped_school_move
     end
 
     render template: "imports/show",
@@ -186,5 +192,11 @@ def set_review_records
         .includes(:school, patient: :school)
         .ready_for_review
         .with_school_moves - @inter_team
+    @skipped_school_moves =
+      @cohort_import
+        .changesets
+        .includes(:patient, patient: :school)
+        .ready_for_review
+        .skipped_school_move
   end
 end
diff --git a/app/models/patient_changeset.rb b/app/models/patient_changeset.rb
index 6d26a9add6..ac205ac95c 100644
--- a/app/models/patient_changeset.rb
+++ b/app/models/patient_changeset.rb
@@ -76,7 +76,13 @@ class PatientChangeset < ApplicationRecord
        validate: true
 
   enum :record_type,
-       { auto_match: 0, new_patient: 1, import_issue: 2, not_in_file: 3 },
+       {
+         auto_match: 0,
+         new_patient: 1,
+         import_issue: 2,
+         not_in_file: 3,
+         skipped_school_move: 4
+       },
        validate: true
 
   scope :nhs_number_discrepancies,
@@ -462,6 +468,8 @@ def stage_and_handle_pending_changes(existing_patient)
   def changeset_type
     if patient.id.nil?
       :new_patient
+    elsif school_move_to_unknown_from_another_team?
+      :skipped_school_move
     elsif patient.pending_changes.any?
       :import_issue
     else
diff --git a/app/views/imports/show.html.erb b/app/views/imports/show.html.erb
index 6c7b77aa38..5372168d35 100644
--- a/app/views/imports/show.html.erb
+++ b/app/views/imports/show.html.erb
@@ -65,6 +65,7 @@
         auto_matched_records: @auto_matched_records,
         import_issues: @import_issues,
         school_moves: @school_moves,
+        skipped_school_moves: @skipped_school_moves || [],
       ) %>
 <% end %>
 
@@ -86,6 +87,27 @@
     </details>
   <% end %>
 
+  <% if @skipped_school_moves.present? %>
+    <h3 class="nhsuk-u-reading-width">
+      Records not imported - children at schools in other team areas
+    </h3>
+    <p class="nhsuk-hint">
+      This upload included <%= @skipped_school_moves.count > 1 ? "children" : "a child" %> who <%= @skipped_school_moves.count == 1 ? "was" : "were" %> already
+      registered at a school in another team's area. <%= @skipped_school_moves.count == 1 ? "This child remains" : "These children remain" %>
+      at their current school.
+    </p>
+    <details class="nhsuk-details nhsuk-expander">
+      <summary class="nhsuk-details__summary" data-module="app-sticky">
+        <span class="nhsuk-details__summary-text">
+          <%= pluralize(@skipped_school_moves.count, "record") %> not imported
+        </span>
+      </summary>
+      <div class="nhsuk-details__text">
+        <%= render AppImportReviewRecordsSummaryComponent.new(changesets: @skipped_school_moves) %>
+      </div>
+    </details>
+  <% end %>
+
   <% if @nhs_discrepancies.present? %>
     <h3 class="nhsuk-u-reading-width">
       NHS numbers updated from PDS
diff --git a/spec/features/import_child_records_review_spec.rb b/spec/features/import_child_records_review_spec.rb
new file mode 100644
index 0000000000..f965972062
--- /dev/null
+++ b/spec/features/import_child_records_review_spec.rb
@@ -0,0 +1,120 @@
+# frozen_string_literal: true
+
+describe "Import child records review" do
+  around { |example| travel_to(Date.new(2023, 5, 20)) { example.run } }
+
+  scenario "Skips moving child from another team to unknown school" do
+    given_i_am_signed_in
+    and_two_teams_exist
+    and_import_review_is_enabled
+    and_a_patient_exists_in_another_teams_school
+
+    when_i_visit_the_import_page
+    and_i_choose_to_import_child_records
+    and_i_upload_a_file_moving_child_to_unknown_school
+    then_i_should_see_the_import_review_page
+    and_i_should_see_the_skipped_school_moves_summary
+
+    when_i_approve_the_import
+    then_the_import_should_be_complete
+    and_i_should_see_the_skipped_school_moves_summary
+    and_the_patient_should_remain_at_their_current_school
+    and_the_patient_should_not_be_linked_to_my_team
+  end
+
+  def given_i_am_signed_in
+    programmes = [Programme.hpv, Programme.menacwy, Programme.td_ipv]
+    @team = create(:team, :with_generic_clinic, :with_one_nurse, programmes:)
+    @user = @team.users.first
+
+    # Create a school in this team (children can be uploaded here)
+    create(:school, urn: "123456", team: @team)
+
+    sign_in @user
+  end
+
+  def and_two_teams_exist
+    programmes = [Programme.hpv, Programme.menacwy, Programme.td_ipv]
+    @other_team =
+      create(:team, :with_one_nurse, programmes:, name: "Other Team")
+    @other_school =
+      create(
+        :school,
+        urn: "111222",
+        name: "Other Team School",
+        team: @other_team
+      )
+  end
+
+  def and_import_review_is_enabled
+    Flipper.enable(:import_review_screen)
+  end
+
+  def and_a_patient_exists_in_another_teams_school
+    @patient =
+      create(
+        :patient,
+        given_name: "Mark",
+        family_name: "Doe",
+        nhs_number: nil,
+        date_of_birth: Date.new(2010, 1, 3),
+        address_postcode: "SW1A 1AA",
+        school: @other_school
+      )
+
+    create(
+      :patient_location,
+      patient: @patient,
+      location: @other_school,
+      academic_year: AcademicYear.current
+    )
+  end
+
+  def when_i_visit_the_import_page
+    visit "/dashboard"
+    click_on "Import", match: :first
+  end
+
+  def and_i_choose_to_import_child_records
+    click_on "Upload records"
+    choose "Child records"
+    click_on "Continue"
+  end
+
+  def and_i_upload_a_file_moving_child_to_unknown_school
+    attach_file_fixture "cohort_import[csv]",
+                        "cohort_import/valid_unknown_school.csv"
+    click_on "Continue"
+    wait_for_import_to_complete_until_review(CohortImport)
+  end
+
+  def then_i_should_see_the_import_review_page
+    page.refresh
+    click_on_most_recent_import(CohortImport)
+    expect(page).to have_content("Review and approve")
+  end
+
+  def and_i_should_see_the_skipped_school_moves_summary
+    expect(page).to have_content("1 record not imported")
+    expect(page).to have_content("at their current school")
+  end
+
+  def when_i_approve_the_import
+    click_on "Approve and import records"
+    wait_for_import_to_commit(CohortImport)
+  end
+
+  def then_the_import_should_be_complete
+    expect(page).to have_content("Completed")
+  end
+
+  def and_the_patient_should_remain_at_their_current_school
+    @patient.reload
+    expect(@patient.school).to eq(@other_school)
+  end
+
+  def and_the_patient_should_not_be_linked_to_my_team
+    expect(@patient.teams).not_to include(@team)
+    expect(@patient.teams).to contain_exactly(@other_team)
+  end
+end

From 8781748b9cb22eb921dba04debe3969c234c4981 Mon Sep 17 00:00:00 2001
From: Martin van IJcken <martin.vanijcken@tngtech.com>
Date: Fri, 27 Feb 2026 13:41:20 +0000
Subject: [PATCH 4/9] Do not process patients that have skipped school move

These records should be skipped entirely from the import so no changes to the
base patient are made from data that is very likely to be outdated (why we're skipping
the school moves in the first place)
---
 app/jobs/commit_import_job.rb                   | 17 ++++++++++++-----
 app/jobs/commit_patient_changesets_job.rb       |  4 ++++
 .../import_child_records_review_spec.rb         | 10 ++++++++++
 3 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/app/jobs/commit_import_job.rb b/app/jobs/commit_import_job.rb
index 4cb83c786a..a20cf32f18 100644
--- a/app/jobs/commit_import_job.rb
+++ b/app/jobs/commit_import_job.rb
@@ -39,12 +39,19 @@ def perform(import_global_id)
       changesets
         .from_file
         .includes(:school)
-        .find_in_batches(batch_size: 100) do |changesets|
-          increment_column_counts!(import, counts, changesets)
+        .find_in_batches(batch_size: 100) do |batch|
+          to_skip = batch.select(&:school_move_to_unknown_from_another_team?)
+          to_process = batch - to_skip
 
-          import_patients_and_parents(changesets, import)
-          import_school_moves(changesets, import)
-          import_pds_search_results(changesets, import)
+          if to_process.any?
+            increment_column_counts!(import, counts, to_process)
+
+            import_patients_and_parents(to_process, import)
+            import_school_moves(to_process, import)
+            import_pds_search_results(to_process, import)
+          end
+
+          to_skip.each(&:processed!)
         end
 
       PatientTeamUpdater.call(patient_scope: import.patients)
diff --git a/app/jobs/commit_patient_changesets_job.rb b/app/jobs/commit_patient_changesets_job.rb
index e8d5e01360..48986650ae 100644
--- a/app/jobs/commit_patient_changesets_job.rb
+++ b/app/jobs/commit_patient_changesets_job.rb
@@ -31,6 +31,8 @@ def perform(patient_changeset_ids)
       changesets.update_all(patient_id: nil)
 
       to_process = changesets.select { review_consistent?(it) }
+      to_skip = changesets.select(&:skipped_school_move?)
+      to_process -= to_skip
 
       if to_process.any?
         increment_column_counts!(import, counts, to_process)
@@ -40,6 +42,8 @@ def perform(patient_changeset_ids)
         to_process.each(&:processed!)
       end
 
+      to_skip.each(&:processed!)
+
       PatientTeamUpdater.call(patient_scope: import.patients)
     end
 
diff --git a/spec/features/import_child_records_review_spec.rb b/spec/features/import_child_records_review_spec.rb
index f965972062..45532c5287 100644
--- a/spec/features/import_child_records_review_spec.rb
+++ b/spec/features/import_child_records_review_spec.rb
@@ -20,6 +20,8 @@
     and_i_should_see_the_skipped_school_moves_summary
     and_the_patient_should_remain_at_their_current_school
     and_the_patient_should_not_be_linked_to_my_team
+    and_the_patient_should_not_be_linked_to_the_import
+    and_the_import_changes_should_not_be_processed
   end
 
   def given_i_am_signed_in
@@ -117,4 +119,12 @@ def and_the_patient_should_not_be_linked_to_my_team
     expect(@patient.teams).not_to include(@team)
     expect(@patient.teams).to contain_exactly(@other_team)
   end
+
+  def and_the_patient_should_not_be_linked_to_the_import
+    expect(@patient.cohort_imports).to be_empty
+  end
+
+  def and_the_import_changes_should_not_be_processed
+    expect(@patient.parents).to be_empty
+  end
 end

From c79b5f1913b79f968fac0f79bef65433f0363564 Mon Sep 17 00:00:00 2001
From: Martin van IJcken <martin.vanijcken@tngtech.com>
Date: Wed, 4 Mar 2026 09:50:11 +0000
Subject: [PATCH 5/9] clarify that school and home educated conditions are
 equal and opposite

---
 app/models/patient_changeset.rb | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/app/models/patient_changeset.rb b/app/models/patient_changeset.rb
index ac205ac95c..0af36d267f 100644
--- a/app/models/patient_changeset.rb
+++ b/app/models/patient_changeset.rb
@@ -298,10 +298,10 @@ def school_move
   end
 
   def school_move_to_unknown_from_another_team?
-    return false if school.present? || home_educated == true
-
     return false unless patient.persisted?
-    return false if patient.school.nil? && !patient.home_educated?
+
+    return false if school.present? || home_educated
+    return false unless patient.school.present? || !patient.home_educated?
 
     current_teams = patient.teams_via_patient_locations
     return false if current_teams.empty?

From 7a2edc0b051a32dcc131bec9abaed5b084f92b62 Mon Sep 17 00:00:00 2001
From: Martin van IJcken <martin.vanijcken@tngtech.com>
Date: Wed, 4 Mar 2026 14:31:32 +0000
Subject: [PATCH 6/9] Simplify current_team evaluation

The team is delegated so we do not need to navigate to it ourselves through
import. It is also not null so we do not need to check for presence.

We need not explicitly check for empty current teams as [].include?(x) is
always false.

Jira-Issue: MAV-3920
---
 app/models/patient_changeset.rb | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/app/models/patient_changeset.rb b/app/models/patient_changeset.rb
index 0af36d267f..9b3914c642 100644
--- a/app/models/patient_changeset.rb
+++ b/app/models/patient_changeset.rb
@@ -304,10 +304,7 @@ def school_move_to_unknown_from_another_team?
     return false unless patient.school.present? || !patient.home_educated?
 
     current_teams = patient.teams_via_patient_locations
-    return false if current_teams.empty?
-
-    import_team = import.team
-    return false if import_team && current_teams.include?(import_team)
+    return false if current_teams.include?(team)
 
     true
   end

From 3e1369d2b7595fa263426826404607580d60447c Mon Sep 17 00:00:00 2001
From: Martin van IJcken <martin.vanijcken@tngtech.com>
Date: Wed, 4 Mar 2026 14:36:13 +0000
Subject: [PATCH 7/9] Clarify intent of the
 `school_move_to_unknown_school_from_another_team?` through variable names

Jira-Issue: MAV-3920
---
 app/jobs/commit_import_job.rb         |  3 ++-
 app/models/patient_changeset.rb       | 16 ++++++++--------
 spec/models/patient_changeset_spec.rb | 20 +++++++++++++++-----
 3 files changed, 25 insertions(+), 14 deletions(-)

diff --git a/app/jobs/commit_import_job.rb b/app/jobs/commit_import_job.rb
index a20cf32f18..28e88f7da4 100644
--- a/app/jobs/commit_import_job.rb
+++ b/app/jobs/commit_import_job.rb
@@ -40,7 +40,8 @@ def perform(import_global_id)
         .from_file
         .includes(:school)
         .find_in_batches(batch_size: 100) do |batch|
-          to_skip = batch.select(&:school_move_to_unknown_from_another_team?)
+          to_skip =
+            batch.select(&:school_move_to_unknown_school_from_another_team?)
           to_process = batch - to_skip
 
           if to_process.any?
diff --git a/app/models/patient_changeset.rb b/app/models/patient_changeset.rb
index 9b3914c642..93f45b628b 100644
--- a/app/models/patient_changeset.rb
+++ b/app/models/patient_changeset.rb
@@ -279,7 +279,7 @@ def school_move
       begin
         return if patient.deceased?
         if import_type == "CohortImport" &&
-             school_move_to_unknown_from_another_team?
+             school_move_to_unknown_school_from_another_team?
           return
         end
         if patient.new_record? || patient.school != school ||
@@ -297,16 +297,16 @@ def school_move
       end
   end
 
-  def school_move_to_unknown_from_another_team?
+  def school_move_to_unknown_school_from_another_team?
     return false unless patient.persisted?
 
-    return false if school.present? || home_educated
-    return false unless patient.school.present? || !patient.home_educated?
+    moving_to_unknown_school =
+      (patient.school.present? || patient.home_educated?) &&
+        !(school.present? || home_educated)
 
-    current_teams = patient.teams_via_patient_locations
-    return false if current_teams.include?(team)
+    from_another_team = !patient.teams_via_patient_locations.include?(team)
 
-    true
+    moving_to_unknown_school && from_another_team
   end
 
   def existing_patients
@@ -465,7 +465,7 @@ def stage_and_handle_pending_changes(existing_patient)
   def changeset_type
     if patient.id.nil?
       :new_patient
-    elsif school_move_to_unknown_from_another_team?
+    elsif school_move_to_unknown_school_from_another_team?
       :skipped_school_move
     elsif patient.pending_changes.any?
       :import_issue
diff --git a/spec/models/patient_changeset_spec.rb b/spec/models/patient_changeset_spec.rb
index 387f349b6b..52b7118ac2 100644
--- a/spec/models/patient_changeset_spec.rb
+++ b/spec/models/patient_changeset_spec.rb
@@ -148,7 +148,9 @@
 
     context "when new location is a known school" do
       it "returns false" do
-        expect(changeset.school_move_to_unknown_from_another_team?).to be(false)
+        expect(
+          changeset.school_move_to_unknown_school_from_another_team?
+        ).to be(false)
       end
     end
 
@@ -160,7 +162,9 @@
       end
 
       it "returns false" do
-        expect(changeset.school_move_to_unknown_from_another_team?).to be(false)
+        expect(
+          changeset.school_move_to_unknown_school_from_another_team?
+        ).to be(false)
       end
     end
 
@@ -182,7 +186,9 @@
       end
 
       it "returns true" do
-        expect(changeset.school_move_to_unknown_from_another_team?).to be(true)
+        expect(
+          changeset.school_move_to_unknown_school_from_another_team?
+        ).to be(true)
       end
     end
 
@@ -209,7 +215,9 @@
       end
 
       it "returns false" do
-        expect(changeset.school_move_to_unknown_from_another_team?).to be(false)
+        expect(
+          changeset.school_move_to_unknown_school_from_another_team?
+        ).to be(false)
       end
     end
 
@@ -236,7 +244,9 @@
       end
 
       it "returns true" do
-        expect(changeset.school_move_to_unknown_from_another_team?).to be(true)
+        expect(
+          changeset.school_move_to_unknown_school_from_another_team?
+        ).to be(true)
       end
     end
   end

From a565b9777163af6f3610059ac2d5e742ac53a7ff Mon Sep 17 00:00:00 2001
From: Martin van IJcken <martin.vanijcken@tngtech.com>
Date: Wed, 4 Mar 2026 14:49:20 +0000
Subject: [PATCH 8/9] refactor: patient changeset tests

Jira-Issue:
---
 spec/factories/patients.rb            |  11 ++
 spec/models/patient_changeset_spec.rb | 219 ++++++++++++--------------
 2 files changed, 109 insertions(+), 121 deletions(-)

diff --git a/spec/factories/patients.rb b/spec/factories/patients.rb
index 3c6b7ec729..dc48e5ae95 100644
--- a/spec/factories/patients.rb
+++ b/spec/factories/patients.rb
@@ -167,6 +167,17 @@
     trait :home_educated do
       school { nil }
       home_educated { true }
+
+      after(:create) do |patient, evaluator|
+        clinic = create(:generic_clinic, team: evaluator.team)
+
+        create(
+          :patient_location,
+          patient:,
+          location: clinic,
+          academic_year: evaluator.academic_year
+        )
+      end
     end
 
     trait :deceased do
diff --git a/spec/models/patient_changeset_spec.rb b/spec/models/patient_changeset_spec.rb
index 52b7118ac2..6d975f1a77 100644
--- a/spec/models/patient_changeset_spec.rb
+++ b/spec/models/patient_changeset_spec.rb
@@ -35,11 +35,15 @@
   subject(:changeset) do
     described_class.from_import_row(
       row: import_row,
-      import: create(:class_import),
+      import: create(:class_import, team:),
       row_number: 1
     )
   end
 
+  let(:team) { create(:team) }
+  let(:school) { create(:school, urn: "123456", team:) }
+  let(:home_educated) { false }
+
   let(:valid_data) do
     {
       child_school_urn: "123456",
@@ -57,23 +61,17 @@
   let(:import_row) do
     instance_double(
       CohortImportRow,
-      school: create(:school, urn: "123456"),
-      import_attributes: patient_import_attributes,
-      parent_1_import_attributes: {
-        full_name: "John Smith",
-        relationship: "Father",
-        email: "john@example.com",
-        phone: "07412345678"
-      },
-      parent_2_import_attributes: {
-      },
+      school:,
+      import_attributes:,
+      parent_1_import_attributes:,
+      parent_2_import_attributes:,
       academic_year: AcademicYear.current,
       school_move_source: "import",
-      home_educated: false
+      home_educated:
     )
   end
 
-  let(:patient_import_attributes) do
+  let(:import_attributes) do
     {
       address_line_1: valid_data[:child_address_line_1],
       address_postcode: valid_data[:child_postcode],
@@ -84,9 +82,21 @@
     }.compact
   end
 
+  let(:parent_1_import_attributes) do
+    {
+      full_name: "John Smith",
+      relationship: "Father",
+      email: "john@example.com",
+      phone: "07412345678"
+    }
+  end
+
+  let(:parent_2_import_attributes) { {} }
+
   describe "#patient" do
-    it "builds patient with normalized attributes" do
-      patient = changeset.patient
+    subject(:patient) { changeset.patient }
+
+    it do
       expect(patient).to have_attributes(
         given_name: "Jimmy",
         family_name: "Smith",
@@ -102,151 +112,118 @@
         create(:patient, nhs_number: "9990000026", given_name: "James")
       end
 
-      it "updates existing patient" do
-        patient = changeset.patient
-        expect(patient).to eq(existing_patient)
-        expect(patient.given_name).to eq("James")
-      end
+      it { should eq(existing_patient) }
+      it { should have_attributes(given_name: "James") }
     end
   end
 
   describe "#parents" do
-    it "builds parent with relationship" do
-      parents = changeset.parents
-      relationships = changeset.parent_relationships
+    subject(:parents) { changeset.parents }
 
-      expect(parents.size).to eq(1)
-      expect(parents.first).to have_attributes(
+    it "builds parent" do
+      expect(parents.sole).to have_attributes(
         full_name: "John Smith",
         email: "john@example.com"
       )
+    end
+  end
+
+  describe "#parent_relationships" do
+    subject(:parent_relationships) { changeset.parent_relationships }
 
-      expect(relationships.first).to have_attributes(type: "father")
+    it "creates a relationship for the parent" do
+      expect(parent_relationships.sole).to have_attributes(type: "father")
+      expect(parent_relationships.sole.parent).to eq changeset.parents.sole
+      expect(parent_relationships.sole.patient).to eq changeset.patient
     end
   end
 
   describe "#school_move" do
+    subject(:school_move) { changeset.school_move }
+
     context "with school change" do
       before do
-        @existing_patient =
-          create(:patient, nhs_number: "9990000026", school: create(:school))
+        create(:patient, nhs_number: "9990000026", school: create(:school))
       end
 
       it "creates school move record" do
-        move = changeset.school_move
-        expect(move.school.urn).to eq("123456")
+        expect(school_move.school.urn).to eq("123456")
       end
     end
   end
 
-  describe "#school_move_to_unknown_from_another_team?" do
-    let(:team_a) { create(:team, name: "Team A") }
-    let(:team_b) { create(:team, name: "Team B") }
+  describe "#school_move_to_unknown_school_from_another_team?" do
+    subject(:is_school_move_to_unknown_school_from_another_team?) do
+      changeset.school_move_to_unknown_school_from_another_team?
+    end
+
+    let(:another_team) { create(:team, name: "Another Team") }
     let(:school_in_other_team) do
-      create(:school, name: "School in Team B", team: team_b)
+      create(:school, name: "School in another team", team: another_team)
     end
 
     context "when new location is a known school" do
-      it "returns false" do
-        expect(
-          changeset.school_move_to_unknown_school_from_another_team?
-        ).to be(false)
-      end
-    end
+      let(:school) { create(:school) }
+      let(:home_educated) { false }
 
-    context "when new location is home educated" do
       before do
-        changeset.update!(school: nil)
-        changeset.data["upload"]["home_educated"] = true
-        changeset.save!
-      end
-
-      it "returns false" do
-        expect(
-          changeset.school_move_to_unknown_school_from_another_team?
-        ).to be(false)
-      end
-    end
-
-    context "when new location is unknown school and patient is in a school in another team" do
-      let(:generic_clinic) { create(:generic_clinic, team: team_b) }
-      let!(:existing_patient) do
         create(:patient, nhs_number: "9990000026", school: school_in_other_team)
       end
 
-      before do
-        changeset.update!(school: nil, patient: existing_patient)
-        changeset.data["upload"]["home_educated"] = false
-        create(
-          :patient_location,
-          patient: existing_patient,
-          location: generic_clinic,
-          academic_year: AcademicYear.current
-        )
-      end
-
-      it "returns true" do
-        expect(
-          changeset.school_move_to_unknown_school_from_another_team?
-        ).to be(true)
-      end
+      it { should be false }
     end
 
-    context "when new location is unknown school and patient is in unknown school in another team" do
-      let(:generic_clinic) { create(:generic_clinic, team: team_b) }
-      let!(:existing_patient) do
-        create(
-          :patient,
-          nhs_number: "9990000026",
-          school: nil,
-          home_educated: false
-        )
-      end
+    context "when new location is home educated" do
+      let(:school) { nil }
+      let(:home_educated) { true }
 
       before do
-        changeset.update!(school: nil, patient: existing_patient)
-        changeset.data["upload"]["home_educated"] = false
-        create(
-          :patient_location,
-          patient: existing_patient,
-          location: generic_clinic,
-          academic_year: AcademicYear.current
-        )
+        create(:patient, nhs_number: "9990000026", school: school_in_other_team)
       end
 
-      it "returns false" do
-        expect(
-          changeset.school_move_to_unknown_school_from_another_team?
-        ).to be(false)
-      end
+      it { should be false }
     end
 
-    context "when new location is unknown school and patient is home educated in another team" do
-      let(:generic_clinic) { create(:generic_clinic, team: team_b) }
-      let!(:existing_patient) do
-        create(
-          :patient,
-          nhs_number: "9990000026",
-          school: nil,
-          home_educated: true
-        )
-      end
-
-      before do
-        create(
-          :patient_location,
-          patient: existing_patient,
-          location: generic_clinic,
-          academic_year: AcademicYear.current
-        )
-        changeset.update!(school: nil, patient: existing_patient)
-        changeset.data["upload"]["home_educated"] = false
-      end
-
-      it "returns true" do
-        expect(
-          changeset.school_move_to_unknown_school_from_another_team?
-        ).to be(true)
+    context "when new location is unknown school" do
+      let(:school) { nil }
+      let(:home_educated) { false }
+
+      context "patient is in a school in another team" do
+        before do
+          create(
+            :patient,
+            nhs_number: "9990000026",
+            school: school_in_other_team
+          )
+        end
+
+        it { should be true }
+      end
+
+      context "patient is in unknown school in another team" do
+        before do
+          create(
+            :patient,
+            nhs_number: "9990000026",
+            school: nil,
+            home_educated: false
+          )
+        end
+
+        it { should be false }
+      end
+
+      context "patient is home educated in another team" do
+        before do
+          create(
+            :patient,
+            nhs_number: "9990000026",
+            school: nil,
+            home_educated: true
+          )
+        end
+
+        it { should be true }
       end
     end
   end

From f8a681c1fe60b66df9e9355f1d5fea4d80fb5ceb Mon Sep 17 00:00:00 2001
From: Martin van IJcken <martin.vanijcken@tngtech.com>
Date: Wed, 4 Mar 2026 15:52:35 +0000
Subject: [PATCH 9/9] Do not skip school moves that need re-review

When these changes were approved, they might have been presented as changes
that would be processed. Now they may have moved from changes that will be
skipped due to a change performed by the other team. In that case they should
not be processed so the user is aware that their status has changed.

Jira-Issue: MAV-3920
---
 app/jobs/commit_import_job.rb             | 5 ++---
 app/jobs/commit_patient_changesets_job.rb | 7 ++++---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/app/jobs/commit_import_job.rb b/app/jobs/commit_import_job.rb
index 28e88f7da4..ef965a5843 100644
--- a/app/jobs/commit_import_job.rb
+++ b/app/jobs/commit_import_job.rb
@@ -40,9 +40,8 @@ def perform(import_global_id)
         .from_file
         .includes(:school)
         .find_in_batches(batch_size: 100) do |batch|
-          to_skip =
-            batch.select(&:school_move_to_unknown_school_from_another_team?)
-          to_process = batch - to_skip
+          to_skip, to_process =
+            batch.partition(&:school_move_to_unknown_school_from_another_team?)
 
           if to_process.any?
             increment_column_counts!(import, counts, to_process)
diff --git a/app/jobs/commit_patient_changesets_job.rb b/app/jobs/commit_patient_changesets_job.rb
index 48986650ae..e746f4dce1 100644
--- a/app/jobs/commit_patient_changesets_job.rb
+++ b/app/jobs/commit_patient_changesets_job.rb
@@ -30,9 +30,10 @@ def perform(patient_changeset_ids)
       # Reset patient_ids to avoid stale associations
       changesets.update_all(patient_id: nil)
 
-      to_process = changesets.select { review_consistent?(it) }
-      to_skip = changesets.select(&:skipped_school_move?)
-      to_process -= to_skip
+      to_skip, to_process =
+        changesets
+          .select { review_consistent?(it) }
+          .partition(&:skipped_school_move?)
 
       if to_process.any?
         increment_column_counts!(import, counts, to_process)