From 7c2b0703fb4c23b5ec9d2c0655f55cb5058c776a Mon Sep 17 00:00:00 2001 From: Atharva Raykar <24277692+tfidfwastaken@users.noreply.github.com> Date: Tue, 7 Nov 2023 15:06:52 +0530 Subject: [PATCH] Import API: Fix incorrect patient IDs in patient-associated resources (#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. --- .../fhir_condition_importer.rb | 2 +- .../bulk_api_import/fhir_importable.rb | 8 ----- .../fhir_medication_request_importer.rb | 2 +- .../fhir_observation_importer.rb | 4 +-- .../bulk_api_import/fhir_patient_importer.rb | 2 +- ...6123819_fix_incorrect_fhir_translations.rb | 32 +++++++++++++++++++ db/data_schema.rb | 2 +- .../fhir_condition_importer_spec.rb | 4 +-- .../bulk_api_import/fhir_importable_spec.rb | 17 +++------- .../fhir_medication_request_importer_spec.rb | 4 +-- .../fhir_observation_importer_spec.rb | 4 +-- 11 files changed, 46 insertions(+), 35 deletions(-) create mode 100644 db/data/20231106123819_fix_incorrect_fhir_translations.rb diff --git a/app/services/bulk_api_import/fhir_condition_importer.rb b/app/services/bulk_api_import/fhir_condition_importer.rb index 152adab21e..4cee828137 100644 --- a/app/services/bulk_api_import/fhir_condition_importer.rb +++ b/app/services/bulk_api_import/fhir_condition_importer.rb @@ -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", diff --git a/app/services/bulk_api_import/fhir_importable.rb b/app/services/bulk_api_import/fhir_importable.rb index 98b5b17ae2..269e88c8c2 100644 --- a/app/services/bulk_api_import/fhir_importable.rb +++ b/app/services/bulk_api_import/fhir_importable.rb @@ -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 diff --git a/app/services/bulk_api_import/fhir_medication_request_importer.rb b/app/services/bulk_api_import/fhir_medication_request_importer.rb index 2bf73d8ce2..4f5cf5d943 100644 --- a/app/services/bulk_api_import/fhir_medication_request_importer.rb +++ b/app/services/bulk_api_import/fhir_medication_request_importer.rb @@ -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], diff --git a/app/services/bulk_api_import/fhir_observation_importer.rb b/app/services/bulk_api_import/fhir_observation_importer.rb index 497888a5d0..936c74789e 100644 --- a/app/services/bulk_api_import/fhir_observation_importer.rb +++ b/app/services/bulk_api_import/fhir_observation_importer.rb @@ -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], @@ -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], diff --git a/app/services/bulk_api_import/fhir_patient_importer.rb b/app/services/bulk_api_import/fhir_patient_importer.rb index 1feadaa499..730031682b 100644 --- a/app/services/bulk_api_import/fhir_patient_importer.rb +++ b/app/services/bulk_api_import/fhir_patient_importer.rb @@ -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 diff --git a/db/data/20231106123819_fix_incorrect_fhir_translations.rb b/db/data/20231106123819_fix_incorrect_fhir_translations.rb new file mode 100644 index 0000000000..3fb8bc13b0 --- /dev/null +++ b/db/data/20231106123819_fix_incorrect_fhir_translations.rb @@ -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 diff --git a/db/data_schema.rb b/db/data_schema.rb index 120a6f0fd7..84760f0cde 100644 --- a/db/data_schema.rb +++ b/db/data_schema.rb @@ -1 +1 @@ -DataMigrate::Data.define(version: 20231018062055) +DataMigrate::Data.define(version: 20231106123819) diff --git a/spec/services/bulk_api_import/fhir_condition_importer_spec.rb b/spec/services/bulk_api_import/fhir_condition_importer_spec.rb index 5b15de9386..805acf0c21 100644 --- a/spec/services/bulk_api_import/fhir_condition_importer_spec.rb +++ b/spec/services/bulk_api_import/fhir_condition_importer_spec.rb @@ -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, diff --git a/spec/services/bulk_api_import/fhir_importable_spec.rb b/spec/services/bulk_api_import/fhir_importable_spec.rb index b577b6ef6d..b50edf2eda 100644 --- a/spec/services/bulk_api_import/fhir_importable_spec.rb +++ b/spec/services/bulk_api_import/fhir_importable_spec.rb @@ -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, @@ -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 @@ -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 diff --git a/spec/services/bulk_api_import/fhir_medication_request_importer_spec.rb b/spec/services/bulk_api_import/fhir_medication_request_importer_spec.rb index dcc6e1bbd2..396d10bcf8 100644 --- a/spec/services/bulk_api_import/fhir_medication_request_importer_spec.rb +++ b/spec/services/bulk_api_import/fhir_medication_request_importer_spec.rb @@ -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, diff --git a/spec/services/bulk_api_import/fhir_observation_importer_spec.rb b/spec/services/bulk_api_import/fhir_observation_importer_spec.rb index d86c88a4ac..bcc15a0409 100644 --- a/spec/services/bulk_api_import/fhir_observation_importer_spec.rb +++ b/spec/services/bulk_api_import/fhir_observation_importer_spec.rb @@ -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,