From 1efae8f965a6f7f7c1081659e67935a7ef6819d7 Mon Sep 17 00:00:00 2001 From: Thomas Leese Date: Thu, 30 Apr 2026 11:12:31 +0100 Subject: [PATCH 1/3] Make PDS cascading steps a constant Since it doesn't need to be dynamic. Jira-Issue: MAV-7288 --- app/jobs/pds_cascading_search_job.rb | 108 +++++++++++++-------------- 1 file changed, 53 insertions(+), 55 deletions(-) diff --git a/app/jobs/pds_cascading_search_job.rb b/app/jobs/pds_cascading_search_job.rb index 48de7c86e1..10941b43e8 100644 --- a/app/jobs/pds_cascading_search_job.rb +++ b/app/jobs/pds_cascading_search_job.rb @@ -37,7 +37,7 @@ def perform(searchable, step_name: nil, search_results: [], queue: :pds) searchable.save! - next_step = steps[step_name][result] + next_step = STEPS[step_name][result] if result == :error || next_step.nil? || next_step == :give_up || multiple_nhs_numbers_found?(search_results) || @@ -48,7 +48,7 @@ def perform(searchable, step_name: nil, search_results: [], queue: :pds) else PatientUpdateFromPDSJob.perform_later(searchable, search_results) end - elsif next_step.in?(steps.keys) + elsif next_step.in?(STEPS.keys) raise "Recursive step detected: #{next_step}" if next_step == step_name enqueue_next_search(searchable, next_step, search_results, queue) else @@ -59,6 +59,55 @@ def perform(searchable, step_name: nil, search_results: [], queue: :pds) private + STEPS = { + no_fuzzy_with_history: { + no_matches: :no_fuzzy_with_wildcard_postcode, + one_match: :save_nhs_number_if_unique, + too_many_matches: :no_fuzzy_without_history + }, + no_fuzzy_without_history: { + no_matches: :give_up, + one_match: :save_nhs_number_if_unique, + too_many_matches: :give_up, + format_query: ->(query) { query.merge(history: false) } + }, + no_fuzzy_with_wildcard_postcode: { + no_matches: :no_fuzzy_with_wildcard_given_name, + one_match: :no_fuzzy_with_wildcard_given_name, + too_many_matches: :no_fuzzy_with_wildcard_given_name, + format_query: + lambda do |query| + query[:address_postcode] = query[:address_postcode].dup + query[:address_postcode][2..] = "*" + query + end + }, + no_fuzzy_with_wildcard_given_name: { + no_matches: :no_fuzzy_with_wildcard_family_name, + one_match: :no_fuzzy_with_wildcard_family_name, + too_many_matches: :no_fuzzy_with_wildcard_family_name, + skip_step: :no_fuzzy_with_wildcard_family_name, + format_query: + lambda do |query| + query[:given_name] = query[:given_name].dup + query[:given_name][3..] = "*" + query + end + }, + no_fuzzy_with_wildcard_family_name: { + no_matches: :save_nhs_number_if_unique, + one_match: :save_nhs_number_if_unique, + too_many_matches: :save_nhs_number_if_unique, + skip_step: :save_nhs_number_if_unique, + format_query: + lambda do |query| + query[:family_name] = query[:family_name].dup + query[:family_name][3..] = "*" + query + end + } + }.freeze + def search_for_patient( family_name:, given_name:, @@ -84,8 +133,8 @@ def search_for_patient( fuzzy: false } - if steps[step_name][:format_query].respond_to?(:call) - result = steps[step_name][:format_query].call(query) + if STEPS[step_name][:format_query].respond_to?(:call) + result = STEPS[step_name][:format_query].call(query) query = result if result.is_a?(Hash) end @@ -107,57 +156,6 @@ def search_for_patient( [:error, nil] end - def steps - { - no_fuzzy_with_history: { - no_matches: :no_fuzzy_with_wildcard_postcode, - one_match: :save_nhs_number_if_unique, - too_many_matches: :no_fuzzy_without_history - }, - no_fuzzy_without_history: { - no_matches: :give_up, - one_match: :save_nhs_number_if_unique, - too_many_matches: :give_up, - format_query: ->(query) { query.merge(history: false) } - }, - no_fuzzy_with_wildcard_postcode: { - no_matches: :no_fuzzy_with_wildcard_given_name, - one_match: :no_fuzzy_with_wildcard_given_name, - too_many_matches: :no_fuzzy_with_wildcard_given_name, - format_query: - lambda do |query| - query[:address_postcode] = query[:address_postcode].dup - query[:address_postcode][2..] = "*" - query - end - }, - no_fuzzy_with_wildcard_given_name: { - no_matches: :no_fuzzy_with_wildcard_family_name, - one_match: :no_fuzzy_with_wildcard_family_name, - too_many_matches: :no_fuzzy_with_wildcard_family_name, - skip_step: :no_fuzzy_with_wildcard_family_name, - format_query: - lambda do |query| - query[:given_name] = query[:given_name].dup - query[:given_name][3..] = "*" - query - end - }, - no_fuzzy_with_wildcard_family_name: { - no_matches: :save_nhs_number_if_unique, - one_match: :save_nhs_number_if_unique, - too_many_matches: :save_nhs_number_if_unique, - skip_step: :save_nhs_number_if_unique, - format_query: - lambda do |query| - query[:family_name] = query[:family_name].dup - query[:family_name][3..] = "*" - query - end - } - } - end - def enqueue_next_search(searchable, step_name, search_results, queue) searchable.save! From f8ee9dae9f3024cd2e7b16b96582803f5c4f01d3 Mon Sep 17 00:00:00 2001 From: Thomas Leese Date: Thu, 30 Apr 2026 11:20:36 +0100 Subject: [PATCH 2/3] Use strings in `PDSCascadingSearchJob` This is in preparation for converting this job to Sidekiq where all arguments need to be JSON serialisable, of which symbols are not. We also store these values in a JSON blob on the `PatientChangeset` model so I think it makes sense to use strings here since they will be serialised in to strings anyway. I've removed a `describe` block from the test to simplify the file, which has resulted in a whitespace change on each line as the code is de-indented. Jira-Issue: MAV-7288 --- app/jobs/pds_cascading_search_job.rb | 70 +-- app/models/patient_import.rb | 2 +- spec/jobs/pds_cascading_search_job_spec.rb | 552 ++++++++++----------- 3 files changed, 309 insertions(+), 315 deletions(-) diff --git a/app/jobs/pds_cascading_search_job.rb b/app/jobs/pds_cascading_search_job.rb index 10941b43e8..3956df83dd 100644 --- a/app/jobs/pds_cascading_search_job.rb +++ b/app/jobs/pds_cascading_search_job.rb @@ -6,8 +6,10 @@ class PDSCascadingSearchJob < ApplicationJobActiveJob queue_as :pds retry_on Faraday::ServerError, wait: :polynomially_longer - def perform(searchable, step_name: nil, search_results: [], queue: :pds) - step_name ||= :no_fuzzy_with_history + def perform(searchable, step_name: nil, search_results: nil, queue: nil) + step_name ||= "no_fuzzy_with_history" + search_results ||= [] + queue ||= "pds" SemanticLogger.tagged( searchable: "#{searchable.class.name}##{searchable.id}", @@ -19,15 +21,15 @@ def perform(searchable, step_name: nil, search_results: [], queue: :pds) given_name: searchable.given_name, date_of_birth: searchable.date_of_birth, address_postcode: searchable.address_postcode, - step_name: step_name + step_name: ) search_result = { - step: step_name, - result: result, - nhs_number: pds_patient&.nhs_number, - created_at: Time.current - }.with_indifferent_access + "step" => step_name, + "result" => result.to_s, + "nhs_number" => pds_patient&.nhs_number, + "created_at" => Time.current.iso8601 + } if searchable.is_a?(PatientChangeset) searchable.search_results << search_result @@ -39,9 +41,9 @@ def perform(searchable, step_name: nil, search_results: [], queue: :pds) next_step = STEPS[step_name][result] - if result == :error || next_step.nil? || next_step == :give_up || + if result == :error || next_step.nil? || next_step == "give_up" || multiple_nhs_numbers_found?(search_results) || - next_step == :save_nhs_number_if_unique + next_step == "save_nhs_number_if_unique" searchable.save! if searchable.is_a?(PatientChangeset) ProcessPatientChangesetJob.perform_later(searchable.id) @@ -60,21 +62,21 @@ def perform(searchable, step_name: nil, search_results: [], queue: :pds) private STEPS = { - no_fuzzy_with_history: { - no_matches: :no_fuzzy_with_wildcard_postcode, - one_match: :save_nhs_number_if_unique, - too_many_matches: :no_fuzzy_without_history + "no_fuzzy_with_history" => { + no_matches: "no_fuzzy_with_wildcard_postcode", + one_match: "save_nhs_number_if_unique", + too_many_matches: "no_fuzzy_without_history" }, - no_fuzzy_without_history: { - no_matches: :give_up, - one_match: :save_nhs_number_if_unique, - too_many_matches: :give_up, + "no_fuzzy_without_history" => { + no_matches: "give_up", + one_match: "save_nhs_number_if_unique", + too_many_matches: "give_up", format_query: ->(query) { query.merge(history: false) } }, - no_fuzzy_with_wildcard_postcode: { - no_matches: :no_fuzzy_with_wildcard_given_name, - one_match: :no_fuzzy_with_wildcard_given_name, - too_many_matches: :no_fuzzy_with_wildcard_given_name, + "no_fuzzy_with_wildcard_postcode" => { + no_matches: "no_fuzzy_with_wildcard_given_name", + one_match: "no_fuzzy_with_wildcard_given_name", + too_many_matches: "no_fuzzy_with_wildcard_given_name", format_query: lambda do |query| query[:address_postcode] = query[:address_postcode].dup @@ -82,11 +84,11 @@ def perform(searchable, step_name: nil, search_results: [], queue: :pds) query end }, - no_fuzzy_with_wildcard_given_name: { - no_matches: :no_fuzzy_with_wildcard_family_name, - one_match: :no_fuzzy_with_wildcard_family_name, - too_many_matches: :no_fuzzy_with_wildcard_family_name, - skip_step: :no_fuzzy_with_wildcard_family_name, + "no_fuzzy_with_wildcard_given_name" => { + no_matches: "no_fuzzy_with_wildcard_family_name", + one_match: "no_fuzzy_with_wildcard_family_name", + too_many_matches: "no_fuzzy_with_wildcard_family_name", + skip_step: "no_fuzzy_with_wildcard_family_name", format_query: lambda do |query| query[:given_name] = query[:given_name].dup @@ -94,11 +96,11 @@ def perform(searchable, step_name: nil, search_results: [], queue: :pds) query end }, - no_fuzzy_with_wildcard_family_name: { - no_matches: :save_nhs_number_if_unique, - one_match: :save_nhs_number_if_unique, - too_many_matches: :save_nhs_number_if_unique, - skip_step: :save_nhs_number_if_unique, + "no_fuzzy_with_wildcard_family_name" => { + no_matches: "save_nhs_number_if_unique", + one_match: "save_nhs_number_if_unique", + too_many_matches: "save_nhs_number_if_unique", + skip_step: "save_nhs_number_if_unique", format_query: lambda do |query| query[:family_name] = query[:family_name].dup @@ -118,9 +120,9 @@ def search_for_patient( return :no_postcode, nil if address_postcode.blank? case step_name - when :no_fuzzy_with_wildcard_given_name + when "no_fuzzy_with_wildcard_given_name" return :skip_step, nil if given_name.length <= 3 - when :no_fuzzy_with_wildcard_family_name + when "no_fuzzy_with_wildcard_family_name" return :skip_step, nil if family_name.length <= 3 end diff --git a/app/models/patient_import.rb b/app/models/patient_import.rb index c4c7315c50..603a723f4a 100644 --- a/app/models/patient_import.rb +++ b/app/models/patient_import.rb @@ -178,7 +178,7 @@ def enqueue_pds_cascading_searches(changesets) changesets.find_each do |cs| PDSCascadingSearchJob.set(queue: :imports).perform_later( cs, - queue: :imports + queue: "imports" ) end end diff --git a/spec/jobs/pds_cascading_search_job_spec.rb b/spec/jobs/pds_cascading_search_job_spec.rb index fd28676d13..1c2ce7d52e 100644 --- a/spec/jobs/pds_cascading_search_job_spec.rb +++ b/spec/jobs/pds_cascading_search_job_spec.rb @@ -34,345 +34,337 @@ around { |example| travel_to(today) { example.run } } - describe "#perform" do - context "when finding a patient on first attempt" do - before do - allow(PDS::Patient).to receive(:search).and_return(mock_patient) - end - - it "saves the search result and enqueues ProcessPatientChangesetJob" do - expect { - described_class.perform_now(patient_changeset) - }.to have_enqueued_job(ProcessPatientChangesetJob).with( - patient_changeset.id - ) - - patient_changeset.reload + context "when finding a patient on first attempt" do + before { allow(PDS::Patient).to receive(:search).and_return(mock_patient) } - expect(patient_changeset.search_results.count).to eq(1) - expect(patient_changeset.search_results.first).to include( - "step" => "no_fuzzy_with_history", - "result" => "one_match", - "nhs_number" => "9449306168" - ) - end + it "saves the search result and enqueues ProcessPatientChangesetJob" do + expect { + described_class.perform_now(patient_changeset) + }.to have_enqueued_job(ProcessPatientChangesetJob).with( + patient_changeset.id + ) + + patient_changeset.reload + + expect(patient_changeset.search_results.count).to eq(1) + expect(patient_changeset.search_results.first).to include( + "step" => "no_fuzzy_with_history", + "result" => "one_match", + "nhs_number" => "9449306168" + ) end + end - context "when no match found, cascading through steps" do - before { allow(PDS::Patient).to receive(:search).and_return(nil) } + context "when no match found, cascading through steps" do + before { allow(PDS::Patient).to receive(:search).and_return(nil) } - it "enqueues next search step on no match" do - expect { - described_class.perform_now( - patient_changeset, - step_name: :no_fuzzy_with_history - ) - }.to have_enqueued_job(described_class).with( + it "enqueues next search step on no match" do + expect { + described_class.perform_now( patient_changeset, - step_name: :no_fuzzy_with_wildcard_postcode, - search_results: [ - { - "step" => :no_fuzzy_with_history, - "result" => :no_matches, - "nhs_number" => nil, - "created_at" => Time.current - } - ], - queue: :pds + step_name: "no_fuzzy_with_history" ) - end + }.to have_enqueued_job(described_class).with( + patient_changeset, + step_name: "no_fuzzy_with_wildcard_postcode", + search_results: [ + { + "step" => "no_fuzzy_with_history", + "result" => "no_matches", + "nhs_number" => nil, + "created_at" => Time.current.iso8601 + } + ], + queue: "pds" + ) end + end - context "when no postcode provided" do - let(:patient_changeset) do - create( - :patient_changeset, - import: import, - data: { - upload: { - child: { - given_name: "Charlie", - family_name: "Brown", - date_of_birth: "2010-01-01", - address_postcode: nil - } - }, - search_results: [] - } - ) - end + context "when no postcode provided" do + let(:patient_changeset) do + create( + :patient_changeset, + import: import, + data: { + upload: { + child: { + given_name: "Charlie", + family_name: "Brown", + date_of_birth: "2010-01-01", + address_postcode: nil + } + }, + search_results: [] + } + ) + end - it "records no_postcode result and enqueues ProcessPatientChangesetJob" do - expect { - described_class.perform_now(patient_changeset) - }.to have_enqueued_job(ProcessPatientChangesetJob).with( - patient_changeset.id - ) + it "records no_postcode result and enqueues ProcessPatientChangesetJob" do + expect { + described_class.perform_now(patient_changeset) + }.to have_enqueued_job(ProcessPatientChangesetJob).with( + patient_changeset.id + ) - patient_changeset.reload + patient_changeset.reload - expect(patient_changeset.search_results.first).to include( - "step" => "no_fuzzy_with_history", - "result" => "no_postcode" - ) - end + expect(patient_changeset.search_results.first).to include( + "step" => "no_fuzzy_with_history", + "result" => "no_postcode" + ) end + end - context "when name too short for wildcard search" do - let(:patient_changeset) do - create( - :patient_changeset, - import: import, - data: { - upload: { - child: { - given_name: "Ed", - family_name: "Li", - date_of_birth: "2010-01-01", - address_postcode: "SW1A 1AA" - } - }, - search_results: [] - } - ) - end + context "when name too short for wildcard search" do + let(:patient_changeset) do + create( + :patient_changeset, + import: import, + data: { + upload: { + child: { + given_name: "Ed", + family_name: "Li", + date_of_birth: "2010-01-01", + address_postcode: "SW1A 1AA" + } + }, + search_results: [] + } + ) + end - before { allow(PDS::Patient).to receive(:search).and_return(nil) } + before { allow(PDS::Patient).to receive(:search).and_return(nil) } - it "skips wildcard name steps and completes search" do - described_class.perform_now(patient_changeset) + it "skips wildcard name steps and completes search" do + described_class.perform_now(patient_changeset) - perform_enqueued_jobs_while_exists(only: described_class) - perform_enqueued_jobs(only: ProcessPatientChangesetJob) + perform_enqueued_jobs_while_exists(only: described_class) + perform_enqueued_jobs(only: ProcessPatientChangesetJob) - patient_changeset.reload + patient_changeset.reload - skip_results = - patient_changeset.search_results.select do |r| - r["result"] == "skip_step" - end - expect(skip_results.count).to eq(2) - expect(skip_results.map { |r| r["step"] }).to contain_exactly( - "no_fuzzy_with_wildcard_given_name", - "no_fuzzy_with_wildcard_family_name" - ) - end + skip_results = + patient_changeset.search_results.select { it["result"] == "skip_step" } + expect(skip_results.count).to eq(2) + expect(skip_results.map { |r| r["step"] }).to contain_exactly( + "no_fuzzy_with_wildcard_given_name", + "no_fuzzy_with_wildcard_family_name" + ) end + end - context "when multiple NHS numbers found across steps" do - it "stops cascading and enqueues ProcessPatientChangesetJob" do - allow(PDS::Patient).to receive(:search).and_return(mock_patient) + context "when multiple NHS numbers found across steps" do + it "stops cascading and enqueues ProcessPatientChangesetJob" do + allow(PDS::Patient).to receive(:search).and_return(mock_patient) + + described_class.perform_now( + patient_changeset, + step_name: "no_fuzzy_with_history" + ) + + patient_changeset.reload + patient_changeset.search_results << { + "step" => "no_fuzzy_with_wildcard_postcode", + "result" => "one_match", + "nhs_number" => "9876543210", + "created_at" => Time.current.iso8601 + } + patient_changeset.save! + expect { described_class.perform_now( patient_changeset, - step_name: :no_fuzzy_with_history + step_name: "no_fuzzy_with_wildcard_given_name", + search_results: patient_changeset.search_results ) + }.to have_enqueued_job(ProcessPatientChangesetJob).with( + patient_changeset.id + ) + end + end - patient_changeset.reload - patient_changeset.search_results << { - step: "no_fuzzy_with_wildcard_postcode", - result: "one_match", - nhs_number: "9876543210", - created_at: Time.current - }.with_indifferent_access - patient_changeset.save! - - expect { - described_class.perform_now( - patient_changeset, - step_name: :no_fuzzy_with_wildcard_given_name, - search_results: patient_changeset.search_results - ) - }.to have_enqueued_job(ProcessPatientChangesetJob).with( - patient_changeset.id - ) - end + context "when PDS API returns error" do + before do + allow(PDS::Patient).to receive(:search).and_raise( + Faraday::ServerError.new("error", status: 500) + ) end - context "when PDS API returns error" do - before do - allow(PDS::Patient).to receive(:search).and_raise( - Faraday::ServerError.new("error", status: 500) - ) - end + it "records error result and enqueues ProcessPatientChangesetJob" do + expect(Sentry).to receive(:capture_exception) - it "records error result and enqueues ProcessPatientChangesetJob" do - expect(Sentry).to receive(:capture_exception) + expect { + described_class.perform_now(patient_changeset) + }.to have_enqueued_job(ProcessPatientChangesetJob).with( + patient_changeset.id + ) - expect { - described_class.perform_now(patient_changeset) - }.to have_enqueued_job(ProcessPatientChangesetJob).with( - patient_changeset.id - ) + patient_changeset.reload - patient_changeset.reload + expect(patient_changeset.search_results.first).to include( + "step" => "no_fuzzy_with_history", + "result" => "error" + ) + end + end - expect(patient_changeset.search_results.first).to include( - "step" => "no_fuzzy_with_history", - "result" => "error" - ) - end + context "when PDS API returns an invalid search data error" do + before do + allow(PDS::Patient).to receive(:search).and_raise( + NHS::PDS::InvalidSearchData.new + ) end - context "when PDS API returns an invalid search data error" do - before do - allow(PDS::Patient).to receive(:search).and_raise( - NHS::PDS::InvalidSearchData.new - ) - end + it "records error result and enqueues ProcessPatientChangesetJob" do + expect(Sentry).to receive(:capture_exception) - it "records error result and enqueues ProcessPatientChangesetJob" do - expect(Sentry).to receive(:capture_exception) + expect { + described_class.perform_now(patient_changeset) + }.to have_enqueued_job(ProcessPatientChangesetJob).with( + patient_changeset.id + ) - expect { - described_class.perform_now(patient_changeset) - }.to have_enqueued_job(ProcessPatientChangesetJob).with( - patient_changeset.id - ) + patient_changeset.reload - patient_changeset.reload + expect(patient_changeset.search_results.first).to include( + "step" => "no_fuzzy_with_history", + "result" => "error" + ) + end + end - expect(patient_changeset.search_results.first).to include( - "step" => "no_fuzzy_with_history", - "result" => "error" - ) - end + context "when too many matches found on first step" do + before do + allow(PDS::Patient).to receive(:search).and_raise( + NHS::PDS::TooManyMatches + ) end - context "when too many matches found on first step" do - before do - allow(PDS::Patient).to receive(:search).and_raise( - NHS::PDS::TooManyMatches - ) - end + it "enqueues next step (no_fuzzy_without_history)" do + expect { + described_class.perform_now(patient_changeset) + }.to have_enqueued_job(described_class).with( + patient_changeset, + step_name: "no_fuzzy_without_history", + search_results: [ + { + "step" => "no_fuzzy_with_history", + "result" => "too_many_matches", + "nhs_number" => nil, + "created_at" => Time.current.iso8601 + }.with_indifferent_access + ], + queue: "pds" + ) + end + end - it "enqueues next step (no_fuzzy_without_history)" do - expect { - described_class.perform_now(patient_changeset) - }.to have_enqueued_job(described_class).with( - patient_changeset, - step_name: :no_fuzzy_without_history, - search_results: [ - { - "step" => :no_fuzzy_with_history, - "result" => :too_many_matches, - "nhs_number" => nil, - "created_at" => Time.current - }.with_indifferent_access - ], - queue: :pds - ) - end + context "when reaching give_up step" do + before do + allow(PDS::Patient).to receive(:search).and_raise( + NHS::PDS::TooManyMatches + ) end - context "when reaching give_up step" do - before do - allow(PDS::Patient).to receive(:search).and_raise( - NHS::PDS::TooManyMatches - ) - end + it "stops cascading and enqueues ProcessPatientChangesetJob" do + described_class.perform_now( + patient_changeset, + step_name: "no_fuzzy_with_history" + ) - it "stops cascading and enqueues ProcessPatientChangesetJob" do + expect { described_class.perform_now( patient_changeset, - step_name: :no_fuzzy_with_history - ) - - expect { - described_class.perform_now( - patient_changeset, - step_name: :no_fuzzy_without_history - ) - }.to have_enqueued_job(ProcessPatientChangesetJob).with( - patient_changeset.id + step_name: "no_fuzzy_without_history" ) - end + }.to have_enqueued_job(ProcessPatientChangesetJob).with( + patient_changeset.id + ) end + end - context "when running for a Patient object" do - let(:patient) { create(:patient, nhs_number: nil) } + context "when running for a Patient object" do + let(:patient) { create(:patient, nhs_number: nil) } - let(:search_results) { [] } + let(:search_results) { [] } - before do - allow(PDS::Patient).to receive(:search).and_return(mock_patient) - end + before { allow(PDS::Patient).to receive(:search).and_return(mock_patient) } - it "saves the search result into the provided array and enqueues PatientUpdateFromPDSJob" do - expect { - described_class.perform_now(patient, search_results:) - }.to have_enqueued_job(PatientUpdateFromPDSJob).with( - patient, - search_results - ) + it "saves the search result into the provided array and enqueues PatientUpdateFromPDSJob" do + expect { + described_class.perform_now(patient, search_results:) + }.to have_enqueued_job(PatientUpdateFromPDSJob).with( + patient, + search_results + ) - expect(search_results.count).to eq(1) - expect(search_results.first).to include( - step: :no_fuzzy_with_history, - result: :one_match, - nhs_number: "9449306168" - ) - end - - it "enqueues next step when no matches found" do - allow(PDS::Patient).to receive(:search).and_return(nil) - - expect { - described_class.perform_now( - patient, - step_name: :no_fuzzy_with_history, - search_results: search_results - ) - }.to have_enqueued_job(described_class).with( - patient, - step_name: :no_fuzzy_with_wildcard_postcode, - search_results: search_results, - queue: :pds - ) - end - - it "stops cascading when multiple NHS numbers found" do - search_results.concat( - [ - { - step: "no_fuzzy_with_history", - result: "one_match", - nhs_number: "9435780156", - created_at: Time.current - }.with_indifferent_access, - { - step: "no_fuzzy_with_history", - result: "one_match", - nhs_number: "9435792103", - created_at: Time.current - }.with_indifferent_access - ] - ) + expect(search_results.count).to eq(1) + expect(search_results.first).to include( + "step" => "no_fuzzy_with_history", + "result" => "one_match", + "nhs_number" => "9449306168" + ) + end - expect { - described_class.perform_now(patient, search_results:) - }.to have_enqueued_job(PatientUpdateFromPDSJob).with( + it "enqueues next step when no matches found" do + allow(PDS::Patient).to receive(:search).and_return(nil) + + expect { + described_class.perform_now( patient, - search_results + step_name: "no_fuzzy_with_history", + search_results: search_results ) - end + }.to have_enqueued_job(described_class).with( + patient, + step_name: "no_fuzzy_with_wildcard_postcode", + search_results: search_results, + queue: "pds" + ) + end - it "records error result and enqueues PatientUpdateFromPDSJob on PDS error" do - allow(PDS::Patient).to receive(:search).and_raise( - Faraday::ServerError.new("boom", status: 500) - ) + it "stops cascading when multiple NHS numbers found" do + search_results.concat( + [ + { + "step" => "no_fuzzy_with_history", + "result" => "one_match", + "nhs_number" => "9435780156", + "created_at" => Time.current.iso8601 + }.with_indifferent_access, + { + "step" => "no_fuzzy_with_history", + "result" => "one_match", + "nhs_number" => "9435792103", + "created_at" => Time.current.iso8601 + }.with_indifferent_access + ] + ) + + expect { + described_class.perform_now(patient, search_results:) + }.to have_enqueued_job(PatientUpdateFromPDSJob).with( + patient, + search_results + ) + end - expect(Sentry).to receive(:capture_exception) + it "records error result and enqueues PatientUpdateFromPDSJob on PDS error" do + allow(PDS::Patient).to receive(:search).and_raise( + Faraday::ServerError.new("boom", status: 500) + ) - expect { - described_class.perform_now(patient, search_results:) - }.to have_enqueued_job(PatientUpdateFromPDSJob).with( - patient, - search_results - ) + expect(Sentry).to receive(:capture_exception) + + expect { + described_class.perform_now(patient, search_results:) + }.to have_enqueued_job(PatientUpdateFromPDSJob).with( + patient, + search_results + ) - expect(search_results.first).to include(result: :error) - end + expect(search_results.first).to include("result" => "error") end end end From 77d2ba79d76a2307c0cf05963400cb8ed3c92f09 Mon Sep 17 00:00:00 2001 From: Thomas Leese Date: Thu, 30 Apr 2026 11:20:46 +0100 Subject: [PATCH 3/3] Handle `PDSCascadingSearchJob`s with symbols In the previous commit this job was converted to use string arguments. This adds a fallback to ensure that if any existing jobs with symbols are executed correctly. Jira-Issue: MAV-7288 --- app/jobs/pds_cascading_search_job.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/jobs/pds_cascading_search_job.rb b/app/jobs/pds_cascading_search_job.rb index 3956df83dd..68c63e91f4 100644 --- a/app/jobs/pds_cascading_search_job.rb +++ b/app/jobs/pds_cascading_search_job.rb @@ -11,6 +11,10 @@ def perform(searchable, step_name: nil, search_results: nil, queue: nil) search_results ||= [] queue ||= "pds" + # FIXME: Remove these once we're not queuing jobs using symbols. + step_name = step_name.to_s + queue = queue.to_s + SemanticLogger.tagged( searchable: "#{searchable.class.name}##{searchable.id}", step: step_name