Skip to content

Commit

Permalink
Import API: Fix incorrect patient IDs in patient-associated resources (
Browse files Browse the repository at this point in the history
…#5320)

The patient identifiers were created incorrectly in the dependent
resources. We accidentally added a namespace prefix that's used to
identify the patient business identifier, rather than the patient itself
(which does not use any kind of prefix).

While refactoring the `translate_patient_id` method, it became clear
that this utility had no usage across multiple FHIR resource importers,
so we removed it and made the mixin interface smaller.

Our specs also need rectification, they used the wrong patient creation
logic.

Since the business identifiers were created correctly, but the resources
associated with those patients were not, we add a data migration to
rectify the situation.

It turns out that we have been creating these dependent resources with
the `patient_id` set to the `id` of the business identifier, instead of
the `patient_id` of the business identifier. This migration swaps out
the `patient_id` of the dependent resource.
  • Loading branch information
tfidfwastaken authored Nov 7, 2023
1 parent 5a02985 commit 7c2b070
Show file tree
Hide file tree
Showing 11 changed files with 46 additions and 35 deletions.
2 changes: 1 addition & 1 deletion app/services/bulk_api_import/fhir_condition_importer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ def metadata
def build_attributes
{
id: translate_id(@resource.dig(:identifier, 0, :value), org_id: @organization_id),
patient_id: translate_patient_id(@resource[:subject][:identifier], org_id: @organization_id),
patient_id: translate_id(@resource[:subject][:identifier], org_id: @organization_id),
prior_heart_attack: "unknown",
prior_stroke: "unknown",
chronic_kidney_disease: "unknown",
Expand Down
8 changes: 0 additions & 8 deletions app/services/bulk_api_import/fhir_importable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,4 @@ def translate_facility_id(id, org_id:)
facility_groups: {organization_id: org_id})
.facility.id
end

def translate_patient_id(id, org_id:)
translate_id(
id,
org_id: org_id,
ns_prefix: "patient_business_identifier"
)
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ def metadata
def build_attributes
{
id: translate_id(@resource.dig(:identifier, 0, :value), org_id: @organization_id),
patient_id: translate_patient_id(@resource[:subject][:identifier], org_id: @organization_id),
patient_id: translate_id(@resource[:subject][:identifier], org_id: @organization_id),
facility_id: translate_facility_id(@resource[:performer][:identifier], org_id: @organization_id),
is_protocol_drug: false,
name: contained_medication[:code][:coding][0][:display],
Expand Down
4 changes: 2 additions & 2 deletions app/services/bulk_api_import/fhir_observation_importer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ def import_blood_sugar
def build_blood_pressure_attributes
{
id: translate_id(@resource.dig(:identifier, 0, :value), org_id: @organization_id),
patient_id: translate_patient_id(@resource[:subject][:identifier], org_id: @organization_id),
patient_id: translate_id(@resource[:subject][:identifier], org_id: @organization_id),
facility_id: translate_facility_id(@resource[:performer][0][:identifier], org_id: @organization_id),
user_id: import_user.id,
recorded_at: @resource[:effectiveDateTime],
Expand All @@ -63,7 +63,7 @@ def dig_blood_pressure
def build_blood_sugar_attributes
{
id: translate_id(@resource.dig(:identifier, 0, :value), org_id: @organization_id),
patient_id: translate_patient_id(@resource[:subject][:identifier], org_id: @organization_id),
patient_id: translate_id(@resource[:subject][:identifier], org_id: @organization_id),
facility_id: translate_facility_id(@resource[:performer][0][:identifier], org_id: @organization_id),
user_id: import_user.id,
recorded_at: @resource[:effectiveDateTime],
Expand Down
2 changes: 1 addition & 1 deletion app/services/bulk_api_import/fhir_patient_importer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ def address
def business_identifiers
[
{
id: translate_patient_id(identifier, org_id: @organization_id),
id: translate_id(identifier, org_id: @organization_id, ns_prefix: "patient_business_identifier"),
identifier: identifier,
identifier_type: :external_import_id,
**timestamps
Expand Down
32 changes: 32 additions & 0 deletions db/data/20231106123819_fix_incorrect_fhir_translations.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
# frozen_string_literal: true

class FixIncorrectFhirTranslations < ActiveRecord::Migration[6.1]
FACILITY_ID = "f472c5db-188f-4563-9bc7-9f86a6ed6403"

def up
unless CountryConfig.current_country?("Bangladesh") && ENV["SIMPLE_SERVER_ENV"] == "production"
return print "FixIncorrectFhirTranslations is only for production Bangladesh"
end

patients_from_facility = Patient.where(assigned_facility_id: FACILITY_ID)
patient_business_identifiers = patients_from_facility.flat_map(&:business_identifiers)
patient_business_identifiers.each do |identifier|
resources_to_update = [
*MedicalHistory.where(patient_id: identifier.id),
*BloodPressure.where(patient_id: identifier.id),
*BloodSugar.where(patient_id: identifier.id),
*PrescriptionDrug.where(patient_id: identifier.id)
]

resources_to_update.each do |resource|
puts "updating patient ID of #{resource.class.name}:#{resource.id} from #{resource.patient_id} to #{identifier.patient_id}"
resource.patient_id = identifier.patient_id
resource.save!
end
end
end

def down
puts "FixIncorrectFhirTranslations cannot be reversed."
end
end
2 changes: 1 addition & 1 deletion db/data_schema.rb
Original file line number Diff line number Diff line change
@@ -1 +1 @@
DataMigrate::Data.define(version: 20231018062055)
DataMigrate::Data.define(version: 20231106123819)
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,7 @@
let(:org_id) { import_user.organization_id }
let(:identifier) { SecureRandom.uuid }
let(:patient) do
build_stubbed(:patient, id: Digest::UUID.uuid_v5(
Digest::UUID::DNS_NAMESPACE + org_id + "patient_business_identifier", identifier
))
build_stubbed(:patient, id: Digest::UUID.uuid_v5(Digest::UUID::DNS_NAMESPACE + org_id, identifier))
end
let(:patient_identifier) do
build_stubbed(:patient_business_identifier, patient: patient,
Expand Down
17 changes: 5 additions & 12 deletions spec/services/bulk_api_import/fhir_importable_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,7 @@
describe "#translate_patient_id" do
let(:patient_identifier) { SecureRandom.uuid }
let(:patient) do
build_stubbed(:patient, id: Digest::UUID.uuid_v5(
Digest::UUID::DNS_NAMESPACE + org_id + "patient_business_identifier", patient_identifier
))
build_stubbed(:patient, id: Digest::UUID.uuid_v5(Digest::UUID::DNS_NAMESPACE + org_id, patient_identifier))
end
let(:patient_business_identifier) do
build_stubbed(:patient_business_identifier, patient: patient,
Expand All @@ -53,8 +51,7 @@
end

it "translates the patient ID correctly" do
expect(Object.new.extend(described_class)
.translate_patient_id(patient_business_identifier.identifier, org_id: org_id))
expect(Object.new.extend(described_class).translate_id(patient_business_identifier.identifier, org_id: org_id))
.to eq(patient.id)
end

Expand All @@ -67,19 +64,15 @@
org2 = create(:organization, id: SecureRandom.uuid, name: "Another Org")
org2_facility = create(:facility, facility_group: create(:facility_group, organization: org2))
org2_patient = create(:patient,
id: Digest::UUID.uuid_v5(
Digest::UUID::DNS_NAMESPACE + org2.id + "patient_business_identifier", clashing_identifier
),
id: Digest::UUID.uuid_v5(Digest::UUID::DNS_NAMESPACE + org2.id, clashing_identifier),
assigned_facility: org2_facility)
create(:patient_business_identifier, identifier: clashing_identifier,
patient: org2_patient, identifier_type: :external_import_id)

expect(Object.new.extend(described_class)
.translate_patient_id(clashing_identifier, org_id: org1_id))
expect(Object.new.extend(described_class).translate_id(clashing_identifier, org_id: org1_id))
.to eq(org1_patient.id)

expect(Object.new.extend(described_class)
.translate_patient_id(clashing_identifier, org_id: org2.id))
expect(Object.new.extend(described_class).translate_id(clashing_identifier, org_id: org2.id))
.to eq(org2_patient.id)
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,7 @@
end
let(:identifier) { SecureRandom.uuid }
let(:patient) do
build_stubbed(:patient, id: Digest::UUID.uuid_v5(
Digest::UUID::DNS_NAMESPACE + org_id + "patient_business_identifier", identifier
))
build_stubbed(:patient, id: Digest::UUID.uuid_v5(Digest::UUID::DNS_NAMESPACE + org_id, identifier))
end
let(:patient_identifier) do
build_stubbed(:patient_business_identifier, patient: patient,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,7 @@
end
let(:identifier) { SecureRandom.uuid }
let(:patient) do
build_stubbed(:patient, id: Digest::UUID.uuid_v5(
Digest::UUID::DNS_NAMESPACE + org_id + "patient_business_identifier", identifier
))
build_stubbed(:patient, id: Digest::UUID.uuid_v5(Digest::UUID::DNS_NAMESPACE + org_id, identifier))
end
let(:patient_identifier) do
build_stubbed(:patient_business_identifier, patient: patient,
Expand Down

0 comments on commit 7c2b070

Please sign in to comment.