Skip to content

Commit

Permalink
Link imports with created records
Browse files Browse the repository at this point in the history
This updates the cohort and immunisation imports to create links between
the import record (`CohortImport` and `ImmunisationImport`) with the
objects that are getting imported. This doesn't necessarily mean that
the import was the one that first created the object because of
duplicates, but it does tell us whether the record was part of that
import in some way.

This slightly changes the meaning of this relationship, but it
introduces a number of improvements:

- We can simplify the patient and vaccination record duplicate
  controller.
- It allows us to have a more complete picture of the import process,
  users can see if the same record was part of multiple imports, and
  trace how the object might have changed over time.
- We can still determine if an import was responsible for creating an
  object by comparing the `processed_at` and `created_at` times, or the
  `recorded_at` times, albeit without a direct relationship.
  • Loading branch information
thomasleese committed Sep 9, 2024
1 parent 01ecbb4 commit e296520
Show file tree
Hide file tree
Showing 9 changed files with 78 additions and 90 deletions.
12 changes: 1 addition & 11 deletions app/controllers/immunisation_imports/patients_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,6 @@ def set_immunisation_import
end

def set_patient
@patient =
Patient
.joins(patient_sessions: :vaccination_records)
.where(
patient_sessions: {
vaccination_records: {
imported_from: @immunisation_import
}
}
)
.find(params[:id])
@patient = @immunisation_import.patients.find(params[:id])
end
end
4 changes: 2 additions & 2 deletions app/controllers/vaccination_records_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,9 @@ def vaccination_records
policy_scope(VaccinationRecord)
.includes(
:batch,
:programme,
:imported_from,
:immunisation_imports,
:performed_by_user,
:programme,
:vaccine,
patient: :school,
session: :location
Expand Down
9 changes: 6 additions & 3 deletions app/models/cohort_import.rb
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,12 @@ def parse_row(row_data)
def process_row(row)
location = Location.find_by(urn: row.school_urn)

location.patients.create!(
row.to_patient.merge(parent: Parent.new(row.to_parent))
)
patient =
location.patients.create!(
row.to_patient.merge(parent: Parent.new(row.to_parent))
)

patient.cohort_imports << self unless patient.cohort_imports.exists?(id)

:new_record_count
end
Expand Down
54 changes: 32 additions & 22 deletions app/models/immunisation_import.rb
Original file line number Diff line number Diff line change
Expand Up @@ -69,38 +69,48 @@ def count_columns
end

def parse_row(row_data)
ImmunisationImportRow.new(
data: row_data,
programme:,
user: uploaded_by,
imported_from: self
)
ImmunisationImportRow.new(data: row_data, programme:, user: uploaded_by)
end

def process_row(row)
if (vaccination_record = row.to_vaccination_record)
if vaccination_record.new_record?
vaccination_record.save!
:new_record_count
else
:exact_duplicate_record_count
end
count_column_to_increment =
(
if vaccination_record.new_record? || vaccination_record.draft?
:new_record_count
else
:exact_duplicate_record_count
end
)

vaccination_record.save!

link_records(
vaccination_record,
vaccination_record.batch,
vaccination_record.location,
vaccination_record.patient,
vaccination_record.patient_session,
vaccination_record.session
)

count_column_to_increment
else
:not_administered_record_count
end
end

def record_rows
vaccination_records.draft.each do |vaccination_record|
if (patient_session = vaccination_record.patient_session).draft?
patient_session.update!(active: true)
def link_records(*records)
records.each do |record|
unless record.immunisation_imports.exists?(id)
record.immunisation_imports << self
end

if (session = vaccination_record.session).draft?
session.update!(active: true)
end

vaccination_record.update!(recorded_at: Time.zone.now)
end
end

def record_rows
patient_sessions.draft.update_all(active: true)
sessions.draft.update_all(active: true)
vaccination_records.draft.update_all(recorded_at: Time.zone.now)
end
end
41 changes: 17 additions & 24 deletions app/models/immunisation_import_row.rb
Original file line number Diff line number Diff line change
Expand Up @@ -64,33 +64,31 @@ class ImmunisationImportRow
presence: true,
if: :requires_performed_by?

def initialize(data:, programme:, user:, imported_from:)
def initialize(data:, programme:, user:)
@data = data
@programme = programme
@user = user
@imported_from = imported_from
end

def to_vaccination_record
return unless valid?

return unless administered

VaccinationRecord
.recorded
.or(VaccinationRecord.where(imported_from:))
.create_with(imported_from:, notes:, recorded_at: nil)
.find_or_initialize_by(
administered_at:,
batch:,
delivery_method:,
delivery_site:,
dose_sequence:,
patient_session:,
performed_by_family_name:,
performed_by_given_name:,
vaccine:
)
VaccinationRecord.create_with(
notes:,
recorded_at: nil
).find_or_initialize_by(
administered_at:,
batch:,
delivery_method:,
delivery_site:,
dose_sequence:,
patient_session:,
performed_by_family_name:,
performed_by_given_name:,
vaccine:
)
end

def patient
Expand All @@ -111,9 +109,7 @@ def session
@session ||=
@programme
.sessions
.active
.or(Session.where(imported_from:))
.create_with(imported_from:, active: false)
.create_with(active: false)
.find_or_create_by!(
date: session_date,
location:,
Expand Down Expand Up @@ -267,8 +263,6 @@ def performed_by_family_name

private

attr_reader :imported_from

def administered_at
administered ? (session_date.in_time_zone + 12.hours) : nil
end
Expand Down Expand Up @@ -392,15 +386,14 @@ def patient_attributes
first_name: patient_first_name,
gender_code: patient_gender_code,
home_educated:,
imported_from:,
last_name: patient_last_name,
nhs_number: patient_nhs_number,
school:
}
end

def staged_patient_attributes
patient_attributes.except(:imported_from, :school).merge(
patient_attributes.except(:school).merge(
school_id: patient_attributes[:school]&.id
)
end
Expand Down
4 changes: 3 additions & 1 deletion app/views/immunisation_imports/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,6 @@
<% end %>
<% end %>
<%= render AppVaccinationRecordTableComponent.new(@vaccination_records) %>
<%= render AppVaccinationRecordTableComponent.new(
@immunisation_import.recorded? ? @vaccination_records.recorded : @vaccination_records.draft
) %>
2 changes: 1 addition & 1 deletion app/views/vaccination_records/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
{ text: @programme.name, href: programme_vaccination_records_path(@programme) },
{ text: t("vaccination_records.index.title"), href: programme_vaccination_records_path(@programme) },
]) %>
<% elsif (immunisation_import = @vaccination_record.imported_from) %>
<% elsif (immunisation_import = @vaccination_record.immunisation_imports.sort_by(&:created_at).last) %>
<%= render AppBacklinkComponent.new(
href: edit_programme_immunisation_import_path(
@programme,
Expand Down
10 changes: 1 addition & 9 deletions spec/models/immunisation_import_row_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,12 @@

describe ImmunisationImportRow, type: :model do
subject(:immunisation_import_row) do
described_class.new(
data:,
programme:,
user: uploaded_by,
imported_from: immunisation_import
)
described_class.new(data:, programme:, user: uploaded_by)
end

let(:programme) { create(:programme, :flu, academic_year: 2023) }
let(:team) { create(:team, ods_code: "abc") }
let(:uploaded_by) { create(:user, teams: [team]) }
let(:immunisation_import) do
create(:immunisation_import, programme:, uploaded_by:)
end

let(:nhs_number) { "1234567890" }
let(:first_name) { "Harry" }
Expand Down
32 changes: 15 additions & 17 deletions spec/models/immunisation_import_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -125,22 +125,22 @@
expect { process! }
.to change(immunisation_import, :processed_at).from(nil)
.and change(immunisation_import.vaccination_records, :count).by(7)
.and not_change(immunisation_import.locations, :count)
.and change(immunisation_import.locations, :count).by(1)
.and change(immunisation_import.patients, :count).by(7)
.and change(immunisation_import.sessions, :count).by(1)
.and change(PatientSession, :count).by(7)
.and change(Batch, :count).by(4)
.and change(immunisation_import.patient_sessions, :count).by(7)
.and change(immunisation_import.batches, :count).by(4)

# Second import should not duplicate the vaccination records if they're
# identical.

# stree-ignore
expect { immunisation_import.process! }
.to not_change(immunisation_import, :processed_at)
.and not_change(immunisation_import.vaccination_records, :count)
.and not_change(immunisation_import.locations, :count)
.and not_change(immunisation_import.patients, :count)
.and not_change(immunisation_import.sessions, :count)
.and not_change(VaccinationRecord, :count)
.and not_change(Location, :count)
.and not_change(Patient, :count)
.and not_change(Session, :count)
.and not_change(PatientSession, :count)
.and not_change(Batch, :count)
end
Expand Down Expand Up @@ -171,22 +171,22 @@
expect { process! }
.to change(immunisation_import, :processed_at).from(nil)
.and change(immunisation_import.vaccination_records, :count).by(7)
.and not_change(immunisation_import.locations, :count)
.and change(immunisation_import.locations, :count).by(1)
.and change(immunisation_import.patients, :count).by(7)
.and change(immunisation_import.sessions, :count).by(1)
.and change(PatientSession, :count).by(7)
.and change(Batch, :count).by(5)
.and change(immunisation_import.patient_sessions, :count).by(7)
.and change(immunisation_import.batches, :count).by(5)

# Second import should not duplicate the vaccination records if they're
# identical.

# stree-ignore
expect { immunisation_import.process! }
.to not_change(immunisation_import, :processed_at)
.and not_change(immunisation_import.vaccination_records, :count)
.and not_change(immunisation_import.locations, :count)
.and not_change(immunisation_import.patients, :count)
.and not_change(immunisation_import.sessions, :count)
.and not_change(VaccinationRecord, :count)
.and not_change(Location, :count)
.and not_change(Patient, :count)
.and not_change(Session, :count)
.and not_change(PatientSession, :count)
.and not_change(Batch, :count)
end
Expand Down Expand Up @@ -233,9 +233,7 @@
end

it "doesn't create an additional patient" do
expect { process! }.to change(immunisation_import.patients, :count).by(
6
)
expect { process! }.to change(Patient, :count).by(6)
end

it "doesn't update the NHS number on the existing patient" do
Expand Down

0 comments on commit e296520

Please sign in to comment.