Skip to content

Commit b13aae3

Browse files
authored
Fix bug where searching with no results would raise an error (#3389)
This fixes a bug where searching for patients on the session tabs which results in no patients being returned, the user would see an error page instead of no results. This was happening as the `includes` statement on the patient was incorrectly generating a SQL query that relied on a parameter being present but wasn't passing the parameter in. I haven't got to the bottom of exactly why this was wrong, but it seems to me to potentially be a bug in ActiveRecord. Fixing this involves switching to using `preload` instead of `includes`, and putting an explicit `joins` where we need to query on the patient directly. The original bug was present in version 2.1.2, but was only present on the session outcome tabs, this was made worse in 75b6f47 which applied the bug to all the tabs on the session page. - Sentry issue from 2.1.2: https://good-machine.sentry.io/issues/6528353258/ - Further regression: https://good-machine.sentry.io/issues/6536786279/
2 parents c086156 + 05fdcfb commit b13aae3

File tree

3 files changed

+67
-8
lines changed

3 files changed

+67
-8
lines changed

app/jobs/send_clinic_initial_invitations_job.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,9 @@ def patient_sessions(session, school:, programmes:, session_date:)
2727

2828
session
2929
.patient_sessions
30+
.joins(:patient)
3031
.includes_programmes
31-
.preload(
32+
.includes(
3233
:session_notifications,
3334
patient: %i[consents parents vaccination_records]
3435
)

app/models/patient_session.rb

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -71,10 +71,13 @@ class PatientSession < ApplicationRecord
7171
joins(:patient).merge(Patient.in_programmes(programmes))
7272
end
7373

74-
scope :search_by_name, ->(name) { merge(Patient.search_by_name(name)) }
74+
scope :search_by_name,
75+
->(name) { joins(:patient).merge(Patient.search_by_name(name)) }
7576

7677
scope :search_by_year_groups,
77-
->(year_groups) { merge(Patient.search_by_year_groups(year_groups)) }
78+
->(year_groups) do
79+
joins(:patient).merge(Patient.search_by_year_groups(year_groups))
80+
end
7881

7982
scope :search_by_date_of_birth_year,
8083
->(year) do
@@ -100,8 +103,7 @@ class PatientSession < ApplicationRecord
100103
)
101104
end
102105

103-
scope :includes_programmes,
104-
-> { eager_load(:patient).preload(session: :programmes) }
106+
scope :includes_programmes, -> { preload(:patient, session: :programmes) }
105107

106108
scope :has_consent_status,
107109
->(status, programme:) do

spec/features/patient_search_spec.rb

Lines changed: 59 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,32 +34,59 @@
3434
then_i_see_patients_by_date_of_birth
3535
end
3636

37+
scenario "Search result returns no patients" do
38+
given_that_i_am_signed_in
39+
40+
when_i_visit_the_session_consent_tab
41+
and_i_search_for_a_name_that_doesnt_exist
42+
then_i_see_no_results
43+
44+
when_i_visit_the_session_triage_tab
45+
and_i_search_for_a_name_that_doesnt_exist
46+
then_i_see_no_results
47+
48+
when_i_visit_the_session_register_tab
49+
and_i_search_for_a_name_that_doesnt_exist
50+
then_i_see_no_results
51+
52+
when_i_visit_the_session_record_tab
53+
and_i_search_for_a_name_that_doesnt_exist
54+
then_i_see_no_results
55+
56+
when_i_visit_the_session_outcome_tab
57+
and_i_search_for_a_name_that_doesnt_exist
58+
then_i_see_no_results
59+
end
60+
3761
def given_that_i_am_signed_in
3862
organisation = create(:organisation, :with_one_nurse)
3963

64+
location = create(:school, name: "Waterloo Road")
65+
@session = create(:session, location:, organisation:)
66+
4067
[
4168
%w[Aaron Smith],
4269
%w[Aardvark Jones],
4370
%w[Casey Brown],
4471
%w[Cassidy Wilson],
4572
%w[Bob Taylor]
4673
].each do |(given_name, family_name)|
47-
create(:patient, given_name:, family_name:, organisation:)
74+
create(:patient, given_name:, family_name:, session: @session)
4875
end
4976

5077
create(
5178
:patient,
5279
given_name: "Salvor",
5380
family_name: "Hardin",
54-
organisation:,
81+
session: @session,
5582
nhs_number: nil
5683
)
5784

5885
create(
5986
:patient,
6087
given_name: "Hari",
6188
family_name: "Seldon",
62-
organisation:,
89+
session: @session,
6390
date_of_birth: Date.new(2013, 1, 1)
6491
)
6592

@@ -172,4 +199,33 @@ def then_i_see_patients_by_date_of_birth
172199
expect(page).not_to have_content("HARDIN, Salvor")
173200
expect(page).to have_content("SELDON, Hari")
174201
end
202+
203+
def when_i_visit_the_session_consent_tab
204+
visit session_consent_path(@session)
205+
end
206+
207+
def when_i_visit_the_session_triage_tab
208+
visit session_triage_path(@session)
209+
end
210+
211+
def when_i_visit_the_session_register_tab
212+
visit session_register_path(@session)
213+
end
214+
215+
def when_i_visit_the_session_record_tab
216+
visit session_record_path(@session)
217+
end
218+
219+
def when_i_visit_the_session_outcome_tab
220+
visit session_outcome_path(@session)
221+
end
222+
223+
def and_i_search_for_a_name_that_doesnt_exist
224+
fill_in "Search", with: "Name doesn't exist"
225+
click_on "Search"
226+
end
227+
228+
def then_i_see_no_results
229+
expect(page).to have_content("No children matching search criteria found")
230+
end
175231
end

0 commit comments

Comments
 (0)