Skip to content

Refactor PDSCascadingSearchJob to use strings #6738

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
thomasleese wants to merge 3 commits into
base: next
Choose a base branch
from
Open

Refactor PDSCascadingSearchJob to use strings #6738

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1efae8f
Make PDS cascading steps a constant
thomasleese Apr 30, 2026
f8ee9da
Use strings in `PDSCascadingSearchJob`
thomasleese Apr 30, 2026
77d2ba7
Handle `PDSCascadingSearchJob`s with symbols
thomasleese Apr 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
138 changes: 71 additions & 67 deletions app/jobs/pds_cascading_search_job.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,14 @@ 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"

# 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}",
Expand All @@ -19,15 +25,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
Expand All @@ -37,18 +43,18 @@ 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 ||
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)
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
Expand All @@ -59,6 +65,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:,
Expand All @@ -69,9 +124,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

Expand All @@ -84,8 +139,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

Expand All @@ -107,57 +162,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!

Expand Down
2 changes: 1 addition & 1 deletion app/models/patient_import.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading
Loading