Skip to content

Commit c099115

Browse files
Add PatientDeleter service class
This class will make it easier to delete `Patient`s, deleting all of the associated objects, before deleting the `Patient`s themselves. It features a `confirm_production_delete` argument, which must be `true` if `Rails.env.production?`. This should help prevent any accidental deletion of patients in production. Jira-Issue: MAV-7067
1 parent 2b75eed commit c099115

2 files changed

Lines changed: 373 additions & 0 deletions

File tree

app/lib/patient_deleter.rb

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
# frozen_string_literal: true
2+
3+
class PatientDeleter
4+
class ProductionDeletionError < StandardError
5+
def message
6+
"PatientDeleter requires `confirm_production_delete: true` in production"
7+
end
8+
end
9+
10+
def initialize(patients:, confirm_production_delete: false)
11+
@patients = patients
12+
@confirm_production_delete = confirm_production_delete
13+
end
14+
15+
def call
16+
if Rails.env.production? && !@confirm_production_delete
17+
raise ProductionDeletionError
18+
end
19+
20+
ActiveRecord::Base.transaction do
21+
delete_related(ArchiveReason)
22+
delete_related(AttendanceRecord)
23+
delete_related(ClinicNotification)
24+
delete_related(ConsentNotification)
25+
delete_related(Consent)
26+
delete_related(GillickAssessment)
27+
delete_related(ImportantNotice)
28+
delete_related(Note)
29+
delete_related(PatientChangeset)
30+
delete_related(PatientLocation)
31+
delete_related(PatientSpecificDirection)
32+
delete_related(PreScreening)
33+
delete_related(SchoolMove)
34+
delete_related(SessionNotification)
35+
delete_related(Triage)
36+
delete_related(VaccinationRecord.with_discarded)
37+
38+
parent_ids =
39+
Parent.joins(parent_relationships: :patient).merge(@patients).ids
40+
delete_related(ParentRelationship)
41+
Parent
42+
.where(id: parent_ids)
43+
.left_joins(:parent_relationships)
44+
.where(parent_relationships: { id: nil })
45+
.destroy_all
46+
47+
@patients.destroy_all
48+
end
49+
end
50+
51+
def self.call(...) = new(...).call
52+
53+
private_class_method :new
54+
55+
private
56+
57+
def delete_related(scope)
58+
scope.joins(:patient).merge(@patients).delete_all
59+
end
60+
end

spec/lib/patient_deleter_spec.rb

Lines changed: 313 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,313 @@
1+
# frozen_string_literal: true
2+
3+
describe PatientDeleter do
4+
subject(:call) { described_class.call(patients:, confirm_production_delete:) }
5+
6+
let(:confirm_production_delete) { false }
7+
let(:programme) { Programme.hpv }
8+
let(:team) { create(:team, programmes: [programme]) }
9+
let(:session) { create(:session, team:, programmes: [programme]) }
10+
let(:patients) { Patient.where(id: patient.id) }
11+
let!(:patient) { create(:patient) }
12+
13+
it "deletes the patient" do
14+
expect { call }.to change(Patient, :count).by(-1)
15+
expect { patient.reload }.to raise_error(ActiveRecord::RecordNotFound)
16+
end
17+
18+
context "with multiple patients" do
19+
let!(:other_patient) { create(:patient) }
20+
let(:patients) { Patient.where(id: [patient.id, other_patient.id]) }
21+
22+
it "deletes all patients" do
23+
expect { call }.to change(Patient, :count).by(-2)
24+
end
25+
end
26+
27+
context "in production" do
28+
before { allow(Rails.env).to receive(:production?).and_return(true) }
29+
30+
context "when confirm_production_delete is false" do
31+
let(:confirm_production_delete) { false }
32+
33+
it "raises an error" do
34+
expect { call }.to raise_error(PatientDeleter::ProductionDeletionError)
35+
end
36+
37+
it "does not delete the patient" do
38+
expect { call }.to raise_error(PatientDeleter::ProductionDeletionError)
39+
expect { patient.reload }.not_to raise_error
40+
end
41+
end
42+
43+
context "when confirm_production_delete is true" do
44+
let(:confirm_production_delete) { true }
45+
46+
it "deletes the patient" do
47+
expect { call }.to change(Patient, :count).by(-1)
48+
end
49+
end
50+
end
51+
52+
context "with associated records" do
53+
let(:archive_reason) do
54+
create(:archive_reason, :moved_out_of_area, patient:, team:)
55+
end
56+
let(:attendance_record) { create(:attendance_record, :present, patient:) }
57+
let(:patient_changeset) do
58+
create(:patient_changeset, :class_import, patient:)
59+
end
60+
let(:clinic_notification) do
61+
create(:clinic_notification, :initial_invitation, patient:, session:)
62+
end
63+
let(:consent_notification) do
64+
create(:consent_notification, :request, patient:, session:)
65+
end
66+
let(:consent) { create(:consent, patient:, programme:) }
67+
let(:gillick_assessment) do
68+
create(:gillick_assessment, :competent, patient:)
69+
end
70+
let(:important_notice) { create(:important_notice, :deceased, patient:) }
71+
let(:note) { create(:note, patient:, session:) }
72+
let(:patient_programme_vaccinations_search) do
73+
create(:patient_programme_vaccinations_search, patient:, programme:)
74+
end
75+
let(:patient_specific_direction) do
76+
create(:patient_specific_direction, patient:, programme:)
77+
end
78+
let(:patient_location) { create(:patient_location, session:, patient:) }
79+
let(:pre_screening) { create(:pre_screening, patient:) }
80+
let(:school_move) { create(:school_move, :to_school, patient:) }
81+
let(:session_notification) do
82+
create(:session_notification, :school_reminder, patient:, session:)
83+
end
84+
let(:triage) { create(:triage, :safe_to_vaccinate, patient:, programme:) }
85+
let(:vaccination_record) do
86+
create(:vaccination_record, patient:, session:, programme:)
87+
end
88+
let(:discarded_vaccination_record) do
89+
create(:vaccination_record, :discarded, patient:, session:, programme:)
90+
end
91+
92+
it "deletes archive reasons" do
93+
archive_reason
94+
expect { call }.to change(ArchiveReason, :count).by(-1)
95+
end
96+
97+
it "deletes attendance records" do
98+
attendance_record
99+
expect { call }.to change(AttendanceRecord, :count).by(-1)
100+
end
101+
102+
it "deletes patient changesets" do
103+
patient_changeset
104+
expect { call }.to change(PatientChangeset, :count).by(-1)
105+
end
106+
107+
it "deletes clinic notifications" do
108+
clinic_notification
109+
expect { call }.to change(ClinicNotification, :count).by(-1)
110+
end
111+
112+
it "deletes consent notifications" do
113+
consent_notification
114+
expect { call }.to change(ConsentNotification, :count).by(-1)
115+
end
116+
117+
it "deletes consents" do
118+
consent
119+
expect { call }.to change(Consent, :count).by(-1)
120+
end
121+
122+
it "deletes gillick assessments" do
123+
gillick_assessment
124+
expect { call }.to change(GillickAssessment, :count).by(-1)
125+
end
126+
127+
it "deletes important notices" do
128+
important_notice
129+
expect { call }.to change(ImportantNotice, :count).by(-1)
130+
end
131+
132+
it "deletes notes" do
133+
note
134+
expect { call }.to change(Note, :count).by(-1)
135+
end
136+
137+
it "deletes patient programme vaccinations searches" do
138+
patient_programme_vaccinations_search
139+
expect { call }.to change(PatientProgrammeVaccinationsSearch, :count).by(
140+
-1
141+
)
142+
end
143+
144+
it "deletes patient specific directions" do
145+
patient_specific_direction
146+
expect { call }.to change(PatientSpecificDirection, :count).by(-1)
147+
end
148+
149+
it "deletes patient locations" do
150+
patient_location
151+
expect { call }.to change(PatientLocation, :count).by(-1)
152+
end
153+
154+
it "deletes pre-screenings" do
155+
pre_screening
156+
expect { call }.to change(PreScreening, :count).by(-1)
157+
end
158+
159+
it "deletes school moves" do
160+
school_move
161+
expect { call }.to change(SchoolMove, :count).by(-1)
162+
end
163+
164+
it "deletes session notifications" do
165+
session_notification
166+
expect { call }.to change(SessionNotification, :count).by(-1)
167+
end
168+
169+
it "deletes triages" do
170+
triage
171+
expect { call }.to change(Triage, :count).by(-1)
172+
end
173+
174+
it "deletes vaccination records" do
175+
vaccination_record
176+
expect { call }.to change(VaccinationRecord.with_discarded, :count).by(-1)
177+
end
178+
179+
it "deletes discarded vaccination records" do
180+
discarded_vaccination_record
181+
expect { call }.to change(VaccinationRecord.with_discarded, :count).by(-1)
182+
end
183+
end
184+
185+
context "with import associations" do
186+
let(:class_import) { create(:class_import) }
187+
let(:cohort_import) { create(:cohort_import) }
188+
let(:immunisation_import) { create(:immunisation_import, team:) }
189+
190+
before do
191+
patient.class_imports << class_import
192+
patient.cohort_imports << cohort_import
193+
patient.immunisation_imports << immunisation_import
194+
end
195+
196+
it "does not delete class imports" do
197+
expect { call }.not_to change(ClassImport, :count)
198+
end
199+
200+
it "does not delete cohort imports" do
201+
expect { call }.not_to change(CohortImport, :count)
202+
end
203+
204+
it "does not delete immunisation imports" do
205+
expect { call }.not_to change(ImmunisationImport, :count)
206+
end
207+
end
208+
209+
context "with parent relationships" do
210+
let(:parent_relationship) { create(:parent_relationship, patient:) }
211+
let(:parent) { parent_relationship.parent }
212+
213+
it "deletes the parent relationship" do
214+
parent_relationship
215+
expect { call }.to change(ParentRelationship, :count).by(-1)
216+
end
217+
218+
it "destroys orphaned parents" do
219+
parent_relationship
220+
expect { call }.to change(Parent, :count).by(-1)
221+
expect { parent.reload }.to raise_error(ActiveRecord::RecordNotFound)
222+
end
223+
224+
context "when the parent has another child" do
225+
let!(:other_patient) { create(:patient) }
226+
227+
before { create(:parent_relationship, parent:, patient: other_patient) }
228+
229+
context "when only one child is deleted" do
230+
it "removes only 1 parent relationship" do
231+
expect { call }.to change(ParentRelationship, :count).by(-1)
232+
end
233+
234+
it "keeps the parent" do
235+
expect { call }.not_to change(Parent, :count)
236+
expect { parent.reload }.not_to raise_error
237+
end
238+
239+
it "does not remove the parent relationship with the other child" do
240+
call
241+
expect(other_patient.parents.reload).to contain_exactly(parent)
242+
end
243+
end
244+
end
245+
246+
context "when all the parent's children are being deleted" do
247+
let!(:other_patient) { create(:patient) }
248+
let(:patients) { Patient.where(id: [patient.id, other_patient.id]) }
249+
250+
before { create(:parent_relationship, parent:, patient: other_patient) }
251+
252+
it "deletes both parent relationships" do
253+
parent_relationship
254+
expect { call }.to change(ParentRelationship, :count).by(-2)
255+
end
256+
257+
it "destroys the parent" do
258+
parent_relationship
259+
expect { call }.to change(Parent, :count).by(-1)
260+
expect { parent.reload }.to raise_error(ActiveRecord::RecordNotFound)
261+
end
262+
end
263+
end
264+
265+
# This test ensures that if a new table is added with a non-cascading FK
266+
# to patients, and prompt the developer to handle it in PatientDeleter.
267+
describe "non-cascading patient FK coverage" do
268+
it "covers all non-cascading FK relationships to the patients table" do
269+
non_cascading_fk_tables =
270+
ActiveRecord::Base.connection.tables.flat_map do |table|
271+
foreign_key =
272+
ActiveRecord::Base
273+
.connection
274+
.foreign_keys(table)
275+
.select do |fk|
276+
fk.to_table == "patients" && fk.options[:on_delete] != :cascade
277+
end
278+
foreign_key.map(&:from_table)
279+
end
280+
non_cascading_fk_tables = non_cascading_fk_tables.to_set
281+
282+
# Tables explicitly handled by PatientDeleter
283+
explicitly_handled = %w[
284+
archive_reasons
285+
attendance_records
286+
clinic_notifications
287+
consent_notifications
288+
consents
289+
gillick_assessments
290+
important_notices
291+
notes
292+
parent_relationships
293+
patient_changesets
294+
patient_locations
295+
patient_programme_vaccinations_searches
296+
patient_specific_directions
297+
pre_screenings
298+
school_moves
299+
session_notifications
300+
triages
301+
vaccination_records
302+
].to_set
303+
304+
unhandled = non_cascading_fk_tables - explicitly_handled
305+
306+
expect(unhandled).to(
307+
be_empty,
308+
"The following tables have non-cascading FKs to patients but are " \
309+
"not handled in PatientDeleter: #{unhandled.to_a.sort.join(", ")}"
310+
)
311+
end
312+
end
313+
end

0 commit comments

Comments
 (0)