Merge pull request #5806 from nhsuk/alistair/national-reporting-hide-import-issues

Remove import issues for national reporting imports

Author: alistair-white-horne-tng

app/components/app_imports_navigation_component.rb

@@ -20,11 +20,13 @@ def call
         selected: active == :imported
       )
 
-      nav.with_item(
-        href: imports_issues_path,
-        text: issues_text,
-        selected: active == :issues
-      )
+      if policy(%i[import issue]).index?
+        nav.with_item(
+          href: imports_issues_path,
+          text: issues_text,
+          selected: active == :issues
+        )
+      end
 
       if policy(ImportantNotice).index?
         nav.with_item(

app/controllers/concerns/navigation_concern.rb

@@ -59,7 +59,7 @@ def set_navigation_items
       @navigation_items << {
         title: t("imports.index.title_short"),
         path: imports_path,
-        count: @cached_counts.import_issues
+        count: (@cached_counts.import_issues if policy(%i[import issue]).index?)
       }
     end
 

app/models/immunisation_import_row.rb

@@ -173,7 +173,11 @@ def to_vaccination_record
     }
 
     vaccination_record =
-      if uuid.present?
+      if bulk?
+        VaccinationRecord.find_or_initialize_by(
+          attributes.merge(attributes_to_stage_if_already_exists)
+        )
+      elsif uuid.present?
         VaccinationRecord
           .find_by!(uuid: uuid.to_s)
           .tap { it.stage_changes(attributes) }

app/policies/import/issue_policy.rb

@@ -1,11 +1,11 @@
 # frozen_string_literal: true
 
 class Import::IssuePolicy < ApplicationPolicy
-  def index? = true
+  def index? = !team.has_upload_only_access?
 
-  def create? = true
+  def create? = !team.has_upload_only_access?
 
-  def show? = true
+  def show? = !team.has_upload_only_access?
 
-  def update? = true
+  def update? = !team.has_upload_only_access?
 end

app/views/imports/_header.html.erb

@@ -1,18 +1,19 @@
 <%= h1 t("imports.index.title"), size: "xl" %>
 
-<% if current_team.has_upload_only_access? %>
-  <p class="nhsuk-u-reading-width">
-    Use this page to upload and import vaccination records.
-  </p>
-<% else %>
-  <p class="nhsuk-u-reading-width">
+<p class="nhsuk-u-reading-width">
+  <% if policy(ClassImport).new? && policy(CohortImport).new? %>
     Use this page to upload and import child, class list and vaccination records.
-  </p>
-<% end %>
+  <% else %>
+    Use this page to upload and import vaccination records.
+  <% end %>
+</p>
 
 <p class="nhsuk-u-reading-width">
   After import, files move to the <strong>Completed imports</strong> tab.
-  Any close matches to resolve will appear in the <strong>Issues</strong> tab.
+
+  <% if policy(%i[import issue]).index? %>
+    Any close matches to resolve will appear in the <strong>Issues</strong> tab.
+  <% end %>
 </p>
 
 <p class="nhsuk-u-reading-width">

spec/features/import_vaccination_records_bulk_spec.rb

@@ -11,6 +11,7 @@
 
     when_i_go_to_the_import_page
     then_i_should_see_the_upload_link
+    and_i_should_not_see_any_reference_to_import_issues
 
     when_i_click_on_the_upload_link
     then_i_should_see_the_upload_page
@@ -102,6 +103,17 @@ def then_i_should_see_the_upload_link
     expect(page).to have_button("Upload records")
   end
 
+  def and_i_should_not_see_any_reference_to_import_issues
+    expect(page).not_to have_content(
+      "Any close matches to resolve will appear in the Issues tab."
+    )
+
+    navigation_items = page.all(".app-secondary-navigation__link")
+    expect(navigation_items.map(&:text)).not_to include(
+      a_string_including("Issues")
+    )
+  end
+
   def when_i_click_on_the_upload_link
     click_on "Upload records"
   end

spec/features/upload_only_team_spec.rb

@@ -13,7 +13,8 @@
   scenario "Navigation shows only import, children and your team" do
     given_i_am_signed_in_as_an_upload_only_team
     when_i_visit_the_dashboard
-    then_i_should_see_only_import_children_and_team_navigation_items
+    then_i_should_see_only_import_and_children_navigation_items
+    and_there_should_be_no_count_next_to_the_import_link
   end
 
   scenario "Children page search shows limited filters and the patient's card" do
@@ -110,13 +111,17 @@ def and_i_should_see_the_children_card
     expect(card).to have_link("Children", href: patients_path)
   end
 
-  def then_i_should_see_only_import_children_and_team_navigation_items
+  def then_i_should_see_only_import_and_children_navigation_items
     navigation_items = page.all(".nhsuk-header__navigation-item")
     expect(navigation_items.count).to eq(2)
     expect(navigation_items[0]).to have_link("Imports", href: imports_path)
     expect(navigation_items[1]).to have_link("Children", href: patients_path)
   end
 
+  def and_there_should_be_no_count_next_to_the_import_link
+    expect(page).not_to have_css(".app-count", text: "(0)")
+  end
+
   def when_i_visit_the_children_page
     visit patients_path
   end

spec/models/immunisation_import_row_spec.rb

@@ -15,7 +15,7 @@
   end
 
   let(:programmes) { [Programme.hpv] }
-  let(:team) { create(:team, ods_code: "abc", programmes:) }
+  let(:team) { create(:team, ods_code: "ABC", programmes:) }
 
   let(:nhs_number) { "9449306168" }
   let(:given_name) { "Harry" }
@@ -25,7 +25,7 @@
   let(:vaccinator) { create(:user, team:) }
   let(:valid_patient_data) do
     {
-      "ORGANISATION_CODE" => "abc",
+      "ORGANISATION_CODE" => "ABC",
       "SCHOOL_NAME" => "Hogwarts",
       "SCHOOL_URN" => "123456",
       "PERSON_FORENAME" => given_name,
@@ -2417,7 +2417,7 @@
 
       let(:import_type) { "bulk" }
 
-      shared_examples "with existing vaccination records" do |programme|
+      shared_examples "date deduplication with existing vaccination records" do |programme|
         let(:patient) { create(:patient, nhs_number:) }
 
         let(:other_vaccination_record) do
@@ -2429,15 +2429,19 @@
 
           before { other_vaccination_record }
 
+          it { should_not be_nil }
           it { should_not eq other_vaccination_record }
+          its(:pending_changes) { should be_empty }
         end
 
         context "with an existing vaccination record on a different date in the same academic year" do
           let(:performed_at) { Time.zone.local(2026, 1, 1, 12, 10, 2) }
 
           before { other_vaccination_record }
 
+          it { should_not be_nil }
           it { should_not eq other_vaccination_record }
+          its(:pending_changes) { should be_empty }
         end
 
         context "with an existing vaccination record on the same date" do
@@ -2466,9 +2470,11 @@
 
         let(:data) { valid_bulk_flu_data }
 
+        let(:programme) { Programme.flu }
+
         it { should be_administered }
 
-        its(:programme) { should eq(Programme.flu) }
+        its(:programme) { should eq programme }
 
         its(:source) { should eq("bulk_upload") }
 
@@ -2552,7 +2558,73 @@
 
         include_examples "with pseudo-postcodes"
 
-        include_examples "with existing vaccination records", Programme.flu
+        include_examples "date deduplication with existing vaccination records",
+                         Programme.flu
+
+        context "when a similar vaccination record exists" do
+          let!(:other_vaccination_record) do
+            vaccine = Vaccine.find_by(nivs_name: "AstraZeneca Fluenz LAIV")
+
+            create(
+              :vaccination_record,
+              :sourced_from_bulk_upload,
+              patient:,
+              programme:,
+              performed_at: Time.zone.local(2026, 1, 5, 0, 0, 0),
+              location:,
+              local_patient_id: "CIN-OXFORD-pat123456",
+              local_patient_id_uri: "https://cinnamon.nhs.uk/0de/system1",
+              performed_by_user: nil,
+              performed_by_given_name: vaccinator.given_name,
+              performed_by_family_name: vaccinator.family_name,
+              vaccine:,
+              batch:
+                create(
+                  :batch,
+                  team: nil,
+                  name: "456",
+                  expiry: Date.new(2026, 1, 6),
+                  vaccine:
+                ) # different
+            )
+          end
+
+          it { should_not be_nil }
+          it { should_not eq other_vaccination_record }
+          its(:pending_changes) { should be_empty }
+        end
+
+        context "when an identical vaccination record exists" do
+          let!(:other_vaccination_record) do
+            vaccine = Vaccine.find_by(nivs_name: "AstraZeneca Fluenz LAIV")
+            create(
+              :vaccination_record,
+              :sourced_from_bulk_upload,
+              patient:,
+              programme:,
+              performed_at: Time.zone.local(2026, 1, 5, 0, 0, 0),
+              location:,
+              local_patient_id: "CIN-OXFORD-pat123456",
+              local_patient_id_uri: "https://cinnamon.nhs.uk/0de/system1",
+              performed_by_user: nil,
+              performed_by_given_name: vaccinator.given_name,
+              performed_by_family_name: vaccinator.family_name,
+              vaccine:,
+              batch:
+                create(
+                  :batch,
+                  team: nil,
+                  name: "123",
+                  expiry: Date.new(2026, 1, 6),
+                  vaccine:
+                ) # identical
+            )
+          end
+
+          it { should_not be_nil }
+          it { should eq other_vaccination_record }
+          its(:pending_changes) { should be_empty }
+        end
       end
 
       context "of type hpv" do
@@ -2570,9 +2642,11 @@
 
         let(:data) { valid_bulk_hpv_data }
 
+        let(:programme) { Programme.hpv }
+
         it { should be_administered }
 
-        its(:programme) { should eq(Programme.hpv) }
+        its(:programme) { should eq programme }
 
         its(:source) { should eq("bulk_upload") }
 
@@ -2649,7 +2723,8 @@
 
         include_examples "with pseudo-postcodes"
 
-        include_examples "with existing vaccination records", Programme.hpv
+        include_examples "date deduplication with existing vaccination records",
+                         Programme.hpv
       end
     end
   end

spec/policies/import/issue_policy_spec.rb

@@ -11,7 +11,7 @@
 
   permissions :index?, :create?, :edit?, :new?, :show?, :update? do
     it { should permit(poc_only_user, vaccination_record) }
-    it { should permit(upload_only_user, vaccination_record) }
+    it { should_not permit(upload_only_user, vaccination_record) }
   end
 
   permissions :destroy? do