From fff91022adc95e6dcc7309cb8e35865ce768e358 Mon Sep 17 00:00:00 2001 From: Atharva Raykar <24277692+tfidfwastaken@users.noreply.github.com> Date: Tue, 30 Jan 2024 17:38:54 +0530 Subject: [PATCH] import api: ensure no duplicates for Conditions (#5378) **Story card:** [sc-11839] ## Because Conditions were wrongly assumed to be longitudinal, resulting in 1:n mapping of patients to medical histories. Ideally every patient must have only one medical history to ensure that the reporting pipeline is accurate. ## Changes * Change in the import API to ensure that all Conditions will always map to a previous medical history (it will only create a new one if it didn't exist at all for a patient) * A data migration that merges all the previous medical histories for ICDDRB (no one else has sent multiple medical_histories yet). --- .../fhir_condition_importer.rb | 17 +++++-- ...7114102_dedupe_icddrb_medical_histories.rb | 44 +++++++++++++++++ .../dedupe_icddrb_medical_histories_spec.rb | 41 ++++++++++++++++ .../fhir_condition_importer_spec.rb | 49 +++++++++++++++++-- .../services/bulk_api_import/importer_spec.rb | 2 +- 5 files changed, 145 insertions(+), 8 deletions(-) create mode 100644 db/data/20240117114102_dedupe_icddrb_medical_histories.rb create mode 100644 spec/data/dedupe_icddrb_medical_histories_spec.rb diff --git a/app/services/bulk_api_import/fhir_condition_importer.rb b/app/services/bulk_api_import/fhir_condition_importer.rb index eaa37cf194..c9e2dba54b 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), + id: condition_id, patient_id: translate_id(@resource[:subject][:identifier], org_id: @organization_id), prior_heart_attack: "unknown", prior_stroke: "unknown", @@ -41,10 +41,19 @@ def build_attributes }.compact.with_indifferent_access end + def condition_id + @condition_id ||= translate_id(@resource[:subject][:identifier], org_id: @organization_id, ns_prefix: "condition") + end + def diagnoses - @resource[:code][:coding].each_with_object({hypertension: "no", diabetes: "no"}) do |coding, diagnoses| - condition = CONDITION_TRANSLATION[coding[:code]] - diagnoses[condition] = "yes" + @diagnoses ||= begin + prev_htn, prev_dm = MedicalHistory.where(id: condition_id).pluck(:hypertension, :diabetes).first + prior_diagnosis = {hypertension: prev_htn || "no", diabetes: prev_dm || "no"} + + @resource[:code][:coding].each_with_object(prior_diagnosis) do |coding, diagnoses| + condition = CONDITION_TRANSLATION[coding[:code]] + diagnoses[condition] = "yes" + end end end end diff --git a/db/data/20240117114102_dedupe_icddrb_medical_histories.rb b/db/data/20240117114102_dedupe_icddrb_medical_histories.rb new file mode 100644 index 0000000000..3480b65622 --- /dev/null +++ b/db/data/20240117114102_dedupe_icddrb_medical_histories.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +class DedupeIcddrbMedicalHistories < ActiveRecord::Migration[6.1] + FACILITY_ID = "f472c5db-188f-4563-9bc7-9f86a6ed6403" + + def up + unless CountryConfig.current_country?("Bangladesh") && SimpleServer.env.production? + return print "DedupeIcddrbMedicalHistories is only for production Bangladesh" + end + + merge_resolution_rules = { + Set["no"] => "no", + Set["yes"] => "yes", + Set["unknown"] => "unknown", + Set["no", "yes"] => "yes", + Set["no", "unknown"] => "no", + Set["yes", "unknown"] => "yes", + Set["yes", "no", "unknown"] => "yes" + } + + fields_to_resolve = MedicalHistory.defined_enums.keys + + transaction do + medical_histories_by_patient = MedicalHistory.joins(:patient) + .where(patient: {assigned_facility_id: FACILITY_ID}) + .group_by(&:patient_id) + + medical_histories_by_patient.each_value do |merge_candidates| + next if merge_candidates.size < 2 + merged_med_history_attrs = merge_candidates.first&.attributes&.except("id") + fields_to_resolve.each do |field_name| + conflicting_values = merge_candidates.map(&field_name.to_sym).to_set + merged_med_history_attrs[field_name] = merge_resolution_rules.fetch(conflicting_values) + end + MedicalHistory.create!(merged_med_history_attrs) + merge_candidates.map(&:discard!) + end + end + end + + def down + puts "This migration cannot be reversed." + end +end diff --git a/spec/data/dedupe_icddrb_medical_histories_spec.rb b/spec/data/dedupe_icddrb_medical_histories_spec.rb new file mode 100644 index 0000000000..ab77a3e297 --- /dev/null +++ b/spec/data/dedupe_icddrb_medical_histories_spec.rb @@ -0,0 +1,41 @@ +require "rails_helper" +require Rails.root.join("db", "data", "20240117114102_dedupe_icddrb_medical_histories.rb") + +describe DedupeIcddrbMedicalHistories do + before do + allow(CountryConfig).to receive(:current_country?).with("Bangladesh").and_return true + stub_const("SIMPLE_SERVER_ENV", "production") + end + + describe "when the data migration is run" do + it "deduplicates medical histories and deletes older medical history records" do + facility = create(:facility, id: "f472c5db-188f-4563-9bc7-9f86a6ed6403") + patients = create_list(:patient, 3, :without_medical_history, assigned_facility_id: facility.id) + + medical_history1 = create(:medical_history, patient: patients[0], hypertension: "yes", diabetes: "no") + + medical_history2 = create(:medical_history, patient: patients[1], hypertension: "yes", diabetes: "no") + medical_history3 = create(:medical_history, patient: patients[1], hypertension: "no", diabetes: "no") + medical_history4 = create(:medical_history, patient: patients[1], hypertension: "no", diabetes: "yes") + + medical_history5 = create(:medical_history, patient: patients[2], hypertension: "no", diabetes: "no") + medical_history6 = create(:medical_history, patient: patients[2], hypertension: "no", diabetes: "no") + + described_class.new.up + + deduped_patient2 = MedicalHistory.find_by(patient_id: patients[1].id) + deduped_patient3 = MedicalHistory.find_by(patient_id: patients[2].id) + + expect(medical_history1.reload.deleted_at).to be_nil + + expect(medical_history2.reload.deleted_at).to be_a(Time) + expect(medical_history3.reload.deleted_at).to be_a(Time) + expect(medical_history4.reload.deleted_at).to be_a(Time) + expect(deduped_patient2).to have_attributes(hypertension: "yes", diabetes: "yes") + + expect(medical_history5.reload.deleted_at).to be_a(Time) + expect(medical_history6.reload.deleted_at).to be_a(Time) + expect(deduped_patient3).to have_attributes(hypertension: "no", diabetes: "no") + end + end +end 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 f510ff2a73..9176145ecb 100644 --- a/spec/services/bulk_api_import/fhir_condition_importer_spec.rb +++ b/spec/services/bulk_api_import/fhir_condition_importer_spec.rb @@ -15,7 +15,7 @@ end describe "#import" do - it "imports a medication request" do + it "imports a condition" do identifier = patient_identifier.identifier expect { described_class.new( @@ -24,6 +24,45 @@ ).import }.to change(MedicalHistory, :count).by(1) end + + it "it ensures no conditions are duplicated" do + identifier = patient_identifier.identifier + expect { + 2.times do + described_class.new( + resource: build_condition_import_resource.merge(subject: {identifier: identifier}), + organization_id: org_id + ).import + end + }.to change(MedicalHistory, :count).by(1) + end + + it "accumulates diagnoses" do + identifier = patient_identifier.identifier + first_update_time = Time.current + second_update_time = (first_update_time + 1.hour) + + first_medical_history_state = described_class.new( + resource: build_condition_import_resource.merge( + meta: {lastUpdated: first_update_time.iso8601, createdAt: first_update_time.iso8601}, + subject: {identifier: identifier}, + code: {coding: [{code: "38341003"}]} + ), + organization_id: org_id + ).import.slice(:hypertension, :diabetes) + + second_medical_history_state = described_class.new( + resource: build_condition_import_resource.merge( + meta: {lastUpdated: second_update_time.iso8601, createdAt: second_update_time.iso8601}, + subject: {identifier: identifier}, + code: {coding: [{code: "73211009"}]} + ), + organization_id: org_id + ).import.slice(:hypertension, :diabetes) + + expect(first_medical_history_state).to include(diabetes: "no", hypertension: "yes") + expect(second_medical_history_state).to include(diabetes: "yes", hypertension: "yes") + end end describe "#build_attributes" do @@ -41,14 +80,18 @@ describe "#diagnoses" do it "extracts diagnoses for diabetes and hypertension" do + identifier = patient_identifier.identifier [ {coding: [], expected: {hypertension: "no", diabetes: "no"}}, {coding: [{code: "38341003"}], expected: {hypertension: "yes", diabetes: "no"}}, {coding: [{code: "73211009"}], expected: {hypertension: "no", diabetes: "yes"}}, {coding: [{code: "38341003"}, {code: "73211009"}], expected: {hypertension: "yes", diabetes: "yes"}} ].each do |coding:, expected:| - expect(described_class.new(resource: {code: {coding: coding}}, organization_id: org_id).diagnoses) - .to eq(expected) + expect( + described_class.new( + resource: {subject: {identifier: identifier}, code: {coding: coding}}, organization_id: org_id + ).diagnoses + ).to eq(expected) end end end diff --git a/spec/services/bulk_api_import/importer_spec.rb b/spec/services/bulk_api_import/importer_spec.rb index efb9652b14..963efaf84a 100644 --- a/spec/services/bulk_api_import/importer_spec.rb +++ b/spec/services/bulk_api_import/importer_spec.rb @@ -72,7 +72,7 @@ end expect { described_class.new(resource_list: resources, organization_id: organization_id).import } - .to change(MedicalHistory, :count).by(2) + .to change(MedicalHistory, :count).by(1) end end