Merge pull request #6573 from NHSDigital/refactor-pds-feature-flags

thomasleese · web-flow · commit d965a8251c4d · 2026-04-16T17:04:42.000+01:00
Refactor PDS feature flags
diff --git a/app/jobs/process_patient_changeset_job.rb b/app/jobs/process_patient_changeset_job.rb
@@ -19,7 +19,7 @@ def perform(patient_changeset_id)
     if patient_changeset.import.changesets.pending.none?
       import = patient_changeset.import
 
-      if Flipper.enabled?(:import_search_pds)
+      if Flipper.enabled?(:pds) && Flipper.enabled?(:pds_search_during_import)
         import.validate_pds_match_rate!
         return if import.low_pds_match_rate?
       end
diff --git a/app/lib/nhs/pds.rb b/app/lib/nhs/pds.rb
@@ -34,7 +34,8 @@ class InvalidSearchData < StandardError
 
   class << self
     def get_patient(nhs_number)
-      return unless Settings.pds.enabled
+      return unless Flipper.enabled?(:pds)
+
       NHS::API.connection.get(
         "personal-demographics/FHIR/R4/Patient/#{nhs_number}"
       )
@@ -55,7 +56,8 @@ def get_patient(nhs_number)
     end
 
     def search_patients(attributes)
-      return unless Settings.pds.enabled
+      return unless Flipper.enabled?(:pds)
+
       if (missing_attrs = (attributes.keys.map(&:to_s) - SEARCH_FIELDS)).any?
         raise "Unrecognised attributes: #{missing_attrs.join(", ")}"
       end
diff --git a/app/lib/update_patients_from_pds.rb b/app/lib/update_patients_from_pds.rb
@@ -27,6 +27,6 @@ def self.call(...) = new(...).call
   attr_reader :patients, :queue
 
   def enqueue?
-    @enqueue ||= Settings.pds.enqueue_bulk_updates
+    Flipper.enabled?(:pds) && Flipper.enabled?(:pds_enqueue_bulk_updates)
   end
 end
diff --git a/app/models/patient_import.rb b/app/models/patient_import.rb
@@ -40,7 +40,7 @@ def process_import!
         PatientChangeset.from_import_row(row:, import: self, row_number:)
       end
 
-    if Flipper.enabled?(:import_search_pds)
+    if Flipper.enabled?(:pds) && Flipper.enabled?(:pds_search_during_import)
       process_no_postcode_changesets(self.changesets.without_postcode)
       if self.changesets.with_postcode.any?
         enqueue_pds_cascading_searches(self.changesets.with_postcode)
@@ -158,7 +158,7 @@ def process_no_postcode_changesets(changesets)
 
   def enqueue_review_jobs(changesets)
     review_changesets =
-      if Flipper.enabled?(:import_search_pds)
+      if Flipper.enabled?(:pds) && Flipper.enabled?(:pds_search_during_import)
         changesets.with_postcode
       else
         changesets
diff --git a/config/feature_flags.yml b/config/feature_flags.yml
@@ -39,10 +39,15 @@ import_choose_academic_year: >-
 import_concurrency_per_server: >-
   Controls whether to limit the concurrency of the import jobs to be global or to be per-server.
 
-import_search_pds: Perform PDS lookups as part of the patient import processing.
-
 ops_tools: Enable the operational support tools; timeline and graph.
 
+pds: Main switch for PDS integration. Turning this off will disable all PDS-related features.
+
+pds_enqueue_bulk_updates: >-
+  Whether to enqueue jobs which updates a large number of patients with the data from PDS in bulk.
+
+pds_search_during_import: Perform PDS lookups as part of the patient import processing.
+
 sync_national_reporting_to_imms_api: >-
   Sync immunisations records uploaded as part of national reporting to the NHS Immunisations API.
 
diff --git a/config/settings.yml b/config/settings.yml
@@ -42,8 +42,6 @@ immunisations_api:
   rate_limit_per_second: 20
 
 pds:
-  enabled: true
-  enqueue_bulk_updates: true
   raise_unknown_gp_practice: true
   rate_limit_per_second: 50
 
diff --git a/config/settings/development.yml b/config/settings/development.yml
@@ -18,8 +18,6 @@ nhs_api:
   disable_authentication: true
 
 pds:
-  enabled: false
-  enqueue_bulk_updates: false
   rate_limit_per_second: 5
 
 splunk:
diff --git a/config/settings/end_to_end.yml b/config/settings/end_to_end.yml
@@ -18,8 +18,6 @@ nhs_api:
   disable_authentication: true
 
 pds:
-  enabled: false
-  enqueue_bulk_updates: false
   rate_limit_per_second: 5
 
 splunk:
diff --git a/spec/features/cli_pds_get_spec.rb b/spec/features/cli_pds_get_spec.rb
@@ -2,7 +2,7 @@
 
 require_relative "../../app/lib/mavis_cli"
 
-describe "mavis pds get" do
+describe "mavis pds get", :pds do
   it "runs successfully" do
     given_the_request_is_stubbed
 
diff --git a/spec/features/cli_pds_search_spec.rb b/spec/features/cli_pds_search_spec.rb
@@ -2,7 +2,7 @@
 
 require_relative "../../app/lib/mavis_cli"
 
-describe "mavis pds search" do
+describe "mavis pds search", :pds do
   it "runs successfully" do
     given_the_request_is_stubbed
 
diff --git a/spec/features/import_child_pds_lookup_extravaganza_spec.rb b/spec/features/import_child_pds_lookup_extravaganza_spec.rb
@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-describe "Import child records" do
+describe "Import child records", :pds do
   let(:today) { Time.zone.local(2025, 9, 1, 12, 0, 0) }
 
   around { |example| travel_to(today) { example.run } }
@@ -278,7 +278,7 @@ def and_an_existing_patient_record_exists
   end
 
   def and_pds_lookups_dont_return_any_matches
-    Flipper.enable(:import_search_pds)
+    Flipper.enable(:pds_search_during_import)
 
     csv =
       CSV.new(
@@ -305,7 +305,7 @@ def and_pds_lookups_dont_return_any_matches
   end
 
   def and_pds_lookup_during_import_is_enabled
-    Flipper.enable(:import_search_pds)
+    Flipper.enable(:pds_search_during_import)
 
     stub_pds_cascading_search(
       family_name: "Tweedle",
diff --git a/spec/features/import_child_records_preparation_spec.rb b/spec/features/import_child_records_preparation_spec.rb
@@ -107,7 +107,7 @@ def given_today_is_the_start_of_the_2024_25_preparation_period
   end
 
   def and_pds_lookup_during_import_is_enabled
-    Flipper.enable(:import_search_pds)
+    Flipper.enable(:pds_search_during_import)
 
     stub_pds_search_to_return_a_patient(
       "9990000026",
diff --git a/spec/features/import_child_records_spec.rb b/spec/features/import_child_records_spec.rb
@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-describe "Import child records" do
+describe "Import child records", :pds do
   around { |example| travel_to(Date.new(2023, 5, 20)) { example.run } }
 
   scenario "User uploads a file" do
@@ -128,7 +128,7 @@ def given_the_app_is_setup
   end
 
   def and_pds_lookup_during_import_is_enabled
-    Flipper.enable(:import_search_pds)
+    Flipper.enable(:pds_search_during_import)
 
     stub_pds_search_to_return_a_patient(
       "9990000026",
diff --git a/spec/features/import_child_records_with_duplicates_spec.rb b/spec/features/import_child_records_with_duplicates_spec.rb
@@ -155,7 +155,7 @@ def given_i_am_signed_in
   def and_pds_lookup_during_import_is_enabled
     return unless ENV["PDS_LOOKUP_DURING_IMPORT"] == "1"
 
-    Flipper.enable(:import_search_pds)
+    Flipper.enable(:pds_search_during_import)
 
     stub_pds_search_to_return_a_patient(
       "9990000026",
diff --git a/spec/features/import_child_records_with_twins_spec.rb b/spec/features/import_child_records_with_twins_spec.rb
@@ -1,10 +1,10 @@
 # frozen_string_literal: true
 
-describe "Child record imports twins" do
+describe "Child record imports twins", :pds do
   around { |example| travel_to(Date.new(2024, 12, 1)) { example.run } }
 
-  before { Flipper.enable(:import_search_pds) }
-  after { Flipper.disable(:import_search_pds) }
+  before { Flipper.enable(:pds_search_during_import) }
+  after { Flipper.disable(:pds_search_during_import) }
 
   scenario "User reviews and selects between duplicate records" do
     and_pds_lookup_during_import_returns_nhs_numbers
diff --git a/spec/features/import_class_lists_move_spec.rb b/spec/features/import_class_lists_move_spec.rb
@@ -172,7 +172,7 @@ def given_an_hpv_programme_is_underway
   end
 
   def and_pds_lookup_during_import_is_enabled
-    Flipper.enable(:import_search_pds)
+    Flipper.enable(:pds_search_during_import)
 
     stub_pds_search_to_return_a_patient(
       "9990000026",
diff --git a/spec/features/import_class_lists_spec.rb b/spec/features/import_class_lists_spec.rb
@@ -87,7 +87,7 @@ def given_an_hpv_programme_is_underway
   end
 
   def and_pds_lookup_during_import_is_enabled
-    Flipper.enable(:import_search_pds)
+    Flipper.enable(:pds_search_during_import)
 
     stub_pds_search_to_return_a_patient(
       "9990000026",
diff --git a/spec/features/import_class_lists_with_duplicates_spec.rb b/spec/features/import_class_lists_with_duplicates_spec.rb
@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-describe "Class list imports duplicates" do
+describe "Class list imports duplicates", :pds do
   around { |example| travel_to(Date.new(2023, 5, 20)) { example.run } }
 
   scenario "User reviews and selects between duplicate records" do
@@ -97,7 +97,7 @@ def given_i_am_signed_in
   end
 
   def and_pds_lookup_during_import_is_enabled
-    Flipper.enable(:import_search_pds)
+    Flipper.enable(:pds_search_during_import)
 
     stub_pds_search_to_return_a_patient(
       "9990000018",
diff --git a/spec/features/important_notices_spec.rb b/spec/features/important_notices_spec.rb
@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-describe "Important notices" do
+describe "Important notices", :pds do
   around { |example| travel_to(Date.new(2023, 5, 20)) { example.run } }
 
   before { given_my_team_exists }
diff --git a/spec/features/parental_consent_create_patient_spec.rb b/spec/features/parental_consent_create_patient_spec.rb
@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-describe "Parental consent create patient" do
+describe "Parental consent create patient", :pds do
   before { given_the_app_is_setup }
 
   around { |example| travel_to(Date.new(2025, 7, 31)) { example.run } }
diff --git a/spec/features/patient_invalidation_spec.rb b/spec/features/patient_invalidation_spec.rb
@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-describe "Patient invalidation deletes vaccination record from API" do
+describe "Patient invalidation deletes vaccination record from API", :pds do
   around { |example| travel_to(Date.new(2025, 8, 7)) { example.run } }
 
   scenario "PDS check invalidates patient and deletes vaccination record from API" do
diff --git a/spec/jobs/enqueue_update_patients_from_pds_job_spec.rb b/spec/jobs/enqueue_update_patients_from_pds_job_spec.rb
@@ -14,7 +14,12 @@
   end
   let!(:never_updated_patient) { create(:patient, updated_from_pds_at: nil) }
 
-  it "only queues jobs for the approriate patients" do
+  before do
+    Flipper.enable(:pds)
+    Flipper.enable(:pds_enqueue_bulk_updates)
+  end
+
+  it "only queues jobs for the appropriate patients" do
     expect { perform_now }.to have_enqueued_job(
       PatientUpdateFromPDSJob
     ).exactly(4).times
diff --git a/spec/jobs/patient_nhs_number_lookup_job_spec.rb b/spec/jobs/patient_nhs_number_lookup_job_spec.rb
@@ -5,7 +5,10 @@
 
   let(:programme) { Programme.sample }
 
-  before { create(:gp_practice, ods_code: "H81109") }
+  before do
+    create(:gp_practice, ods_code: "H81109")
+    Flipper.enable(:pds)
+  end
 
   context "with an NHS number already" do
     let(:patient) { create(:patient, nhs_number: "0123456789") }
diff --git a/spec/jobs/patient_update_from_pds_job_spec.rb b/spec/jobs/patient_update_from_pds_job_spec.rb
diff --git a/spec/jobs/process_consent_form_job_spec.rb b/spec/jobs/process_consent_form_job_spec.rb
diff --git a/spec/jobs/process_patient_changeset_job_spec.rb b/spec/jobs/process_patient_changeset_job_spec.rb
diff --git a/spec/lib/nhs/pds_spec.rb b/spec/lib/nhs/pds_spec.rb
diff --git a/spec/lib/update_patients_from_pds_spec.rb b/spec/lib/update_patients_from_pds_spec.rb
diff --git a/spec/models/class_import_spec.rb b/spec/models/class_import_spec.rb
diff --git a/spec/models/cohort_import_spec.rb b/spec/models/cohort_import_spec.rb
diff --git a/spec/models/immunisation_import_spec.rb b/spec/models/immunisation_import_spec.rb
diff --git a/spec/models/pds/patient_spec.rb b/spec/models/pds/patient_spec.rb
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
