import user: separate users from multiple orgs (#5346)

The Import User was previously created by finding the first facility in
the database and assigning the user to that facility. This is a problem
in any instance that has more than one organization, since it will
affiliate the import user with whichever organization that first
facility belonged to.

We resolve this by requiring an organization ID when finding or creating
an Import User.

**Story card:** [sc-11563]

## Test instructions

Call the import API from two different organizations, and ensure that
there are two different import users with the correct organization
affiliations.

app/services/bulk_api_import/fhir_appointment_importer.rb

@@ -6,6 +6,7 @@ class BulkApiImport::FhirAppointmentImporter
   def initialize(resource:, organization_id:)
     @resource = resource
     @organization_id = organization_id
+    @import_user = find_or_create_import_user(organization_id)
   end
 
   def import
@@ -16,12 +17,12 @@ def import
     appointment = Appointment.merge(transformed_params)
     appointment.update_patient_status
 
-    AuditLog.merge_log(import_user, appointment) if appointment.present?
+    AuditLog.merge_log(@import_user, appointment) if appointment.present?
     appointment
   end
 
   def request_metadata
-    {user_id: import_user.id}
+    {user_id: @import_user.id}
   end
 
   def build_attributes

app/services/bulk_api_import/fhir_condition_importer.rb

@@ -9,19 +9,20 @@ class BulkApiImport::FhirConditionImporter
   def initialize(resource:, organization_id:)
     @resource = resource
     @organization_id = organization_id
+    @import_user = find_or_create_import_user(organization_id)
   end
 
   def import
     merge_result = build_attributes
       .then { Api::V3::MedicalHistoryTransformer.from_request(_1).merge(metadata) }
       .then { MedicalHistory.merge(_1) }
 
-    AuditLog.merge_log(import_user, merge_result) if merge_result.present?
+    AuditLog.merge_log(@import_user, merge_result) if merge_result.present?
     merge_result
   end
 
   def metadata
-    {user_id: import_user.id}
+    {user_id: @import_user.id}
   end
 
   def build_attributes

app/services/bulk_api_import/fhir_importable.rb

@@ -1,6 +1,6 @@
 module BulkApiImport::FhirImportable
-  def import_user
-    ImportUser.find_or_create
+  def find_or_create_import_user(org_id)
+    ImportUser.find_or_create(org_id: org_id)
   end
 
   def import

app/services/bulk_api_import/fhir_medication_request_importer.rb

@@ -17,19 +17,20 @@ class BulkApiImport::FhirMedicationRequestImporter
   def initialize(resource:, organization_id:)
     @resource = resource
     @organization_id = organization_id
+    @import_user = find_or_create_import_user(organization_id)
   end
 
   def import
     merge_result = build_attributes
       .then { Api::V3::PrescriptionDrugTransformer.from_request(_1).merge(metadata) }
       .then { PrescriptionDrug.merge(_1) }
 
-    AuditLog.merge_log(import_user, merge_result) if merge_result.present?
+    AuditLog.merge_log(@import_user, merge_result) if merge_result.present?
     merge_result
   end
 
   def metadata
-    {user_id: import_user.id}
+    {user_id: @import_user.id}
   end
 
   def build_attributes

app/services/bulk_api_import/fhir_observation_importer.rb

@@ -12,6 +12,7 @@ class BulkApiImport::FhirObservationImporter
   def initialize(resource:, organization_id:)
     @resource = resource
     @organization_id = organization_id
+    @import_user = find_or_create_import_user(organization_id)
   end
 
   def import
@@ -24,7 +25,7 @@ def import
       raise "unknown observation type"
     end
 
-    AuditLog.merge_log(import_user, merge_result) if merge_result.present?
+    AuditLog.merge_log(@import_user, merge_result) if merge_result.present?
     merge_result
   end
 
@@ -45,7 +46,7 @@ def build_blood_pressure_attributes
       id: translate_id(@resource.dig(:identifier, 0, :value), 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,
+      user_id: @import_user.id,
       recorded_at: @resource[:effectiveDateTime],
       **dig_blood_pressure,
       **timestamps
@@ -65,7 +66,7 @@ def build_blood_sugar_attributes
       id: translate_id(@resource.dig(:identifier, 0, :value), 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,
+      user_id: @import_user.id,
       recorded_at: @resource[:effectiveDateTime],
       **dig_blood_sugar,
       **timestamps
@@ -88,6 +89,6 @@ def current_timezone_offset
 
   # For compatibility with SyncEncounterObservation
   def current_user
-    import_user
+    @import_user
   end
 end

app/services/bulk_api_import/fhir_patient_importer.rb

@@ -6,19 +6,20 @@ class BulkApiImport::FhirPatientImporter
   def initialize(resource:, organization_id:)
     @resource = resource
     @organization_id = organization_id
+    @import_user = find_or_create_import_user(organization_id)
   end
 
   def import
     merge_result = build_attributes
       .then { Api::V3::PatientTransformer.from_nested_request(_1) }
       .then { MergePatientService.new(_1, request_metadata: request_metadata).merge }
 
-    AuditLog.merge_log(import_user, merge_result) if merge_result.present?
+    AuditLog.merge_log(@import_user, merge_result) if merge_result.present?
     merge_result
   end
 
   def request_metadata
-    {request_user_id: import_user.id,
+    {request_user_id: @import_user.id,
      request_facility_id: @resource.dig(:managingOrganization, 0, :value)}
   end
 

app/services/import_user.rb

@@ -1,16 +1,20 @@
 class ImportUser
   IMPORT_USER_PHONE_NUMBER = "0000000001"
 
-  def self.find_or_create
-    find || create
+  def self.find_or_create(org_id:)
+    find(org_id) || create(org_id)
   end
 
-  def self.find
-    PhoneNumberAuthentication.find_by(phone_number: IMPORT_USER_PHONE_NUMBER)&.user
+  def self.find(org_id)
+    PhoneNumberAuthentication.joins(:user)
+      .find_by(phone_number: IMPORT_USER_PHONE_NUMBER, user: {organization_id: org_id})&.user
   end
 
-  def self.create
-    facility = Facility.first
+  def self.create(org_id)
+    facility = Organization.find_by(id: org_id).facilities.first
+    unless facility.present?
+      raise ArgumentError, "given organization: #{org_id} does not exist or has no facilities"
+    end
 
     user = User.new(
       full_name: "import-user",

app/services/patient_import/importer.rb

@@ -52,7 +52,7 @@ def import
   def import_patient(params)
     MergePatientService.new(params, request_metadata: {
       request_facility_id: facility.id,
-      request_user_id: ImportUser.find_or_create.id
+      request_user_id: ImportUser.find_or_create(org_id: facility.organization_id).id
     }).merge
   end
 
@@ -79,6 +79,6 @@ def current_timezone_offset
 
   # SyncEncounterObservation compability
   def current_user
-    ImportUser.find_or_create
+    ImportUser.find_or_create(org_id: facility.organization_id)
   end
 end

app/services/patient_import/spreadsheet_transformer.rb

@@ -217,7 +217,7 @@ def registration_facility_id
     end
 
     def import_user
-      @import_user = ImportUser.find_or_create
+      @import_user = ImportUser.find_or_create(org_id: facility.organization_id)
     end
 
     def patient_status(row)

spec/jobs/bulk_api_import_job_spec.rb

@@ -2,11 +2,11 @@
 
 RSpec.describe BulkApiImportJob do
   include ActiveJob::TestHelper
-  before { FactoryBot.create(:facility) } # needed for our bot import user
+  let(:facility) { create(:facility) }
 
   describe "#perform_later" do
     let(:resource) { build_condition_import_resource }
-    let(:job) { described_class.perform_later(resources: [resource], organization_id: "org_id") }
+    let(:job) { described_class.perform_later(resources: [resource], organization_id: facility.organization_id) }
 
     it "queues the job" do
       expect { job }.to have_enqueued_job(described_class).once.on_queue("default")

spec/services/bulk_api_import/fhir_appointment_importer_spec.rb

@@ -1,10 +1,9 @@
 require "rails_helper"
 
 RSpec.describe BulkApiImport::FhirAppointmentImporter do
-  before { create(:facility) }
-  let(:import_user) { ImportUser.find_or_create }
-  let(:org_id) { import_user.organization_id }
-  let(:facility) { import_user.facility }
+  let(:facility) { create(:facility) }
+  let(:org_id) { facility.organization_id }
+  let(:import_user) { ImportUser.find_or_create(org_id: org_id) }
   let(:facility_identifiers) do
     create(:facility_business_identifier, facility: facility, identifier_type: :external_org_facility_id)
   end

spec/services/bulk_api_import/fhir_condition_importer_spec.rb

@@ -1,9 +1,9 @@
 require "rails_helper"
 
 RSpec.describe BulkApiImport::FhirConditionImporter do
-  before { create(:facility) }
-  let(:import_user) { ImportUser.find_or_create }
-  let(:org_id) { import_user.organization_id }
+  let(:facility) { create(:facility) }
+  let(:org_id) { facility.organization_id }
+  let(:import_user) { ImportUser.find_or_create(org_id: org_id) }
   let(:identifier) { SecureRandom.uuid }
   let(:patient) do
     build_stubbed(:patient, id: Digest::UUID.uuid_v5(Digest::UUID::DNS_NAMESPACE + org_id, identifier))

spec/services/bulk_api_import/fhir_importable_spec.rb

@@ -1,16 +1,15 @@
 require "rails_helper"
 
 RSpec.describe BulkApiImport::FhirImportable do
-  before { create(:facility) }
-  let(:import_user) { ImportUser.find_or_create }
-  let(:facility) { import_user.facility }
+  let(:facility) { create(:facility) }
   let(:org_id) { facility.organization_id }
+  let(:import_user) { ImportUser.find_or_create(org_id: org_id) }
   let(:facility_identifiers) do
     create(:facility_business_identifier, facility: facility, identifier_type: :external_org_facility_id)
   end
 
-  describe "#import_user" do
-    specify { expect(Object.new.extend(described_class).import_user).to eq(import_user) }
+  describe "#find_or_create_import_user" do
+    specify { expect(Object.new.extend(described_class).find_or_create_import_user(org_id)).to eq(import_user) }
   end
 
   describe "#translate_facility_id" do

spec/services/bulk_api_import/fhir_medication_request_importer_spec.rb

@@ -1,10 +1,9 @@
 require "rails_helper"
 
 RSpec.describe BulkApiImport::FhirMedicationRequestImporter do
-  before { create(:facility) }
-  let(:import_user) { ImportUser.find_or_create }
-  let(:facility) { import_user.facility }
+  let(:facility) { create(:facility) }
   let(:org_id) { facility.organization_id }
+  let(:import_user) { ImportUser.find_or_create(org_id: org_id) }
   let(:facility_identifier) do
     create(:facility_business_identifier, facility: facility, identifier_type: :external_org_facility_id)
   end
@@ -48,7 +47,7 @@
 
   describe "#contained_medication" do
     it "fetches the contained medication" do
-      expect(described_class.new(resource: {contained: [{code: {}}]}, organization_id: "").contained_medication)
+      expect(described_class.new(resource: {contained: [{code: {}}]}, organization_id: org_id).contained_medication)
         .to eq({code: {}})
     end
   end
@@ -65,7 +64,7 @@
           resource: {
             dosageInstruction: [{timing: {code: input_code}}]
           },
-          organization_id: ""
+          organization_id: org_id
         ).frequency).to eq(expected_value)
       end
     end
@@ -111,7 +110,7 @@
         {status: "inactive", deletion_status: true},
         {status: "inactive", deletion_status: true}
       ].each do |status:, deletion_status:|
-        expect(described_class.new(resource: {contained: [{status: status}]}, organization_id: "").drug_deleted?)
+        expect(described_class.new(resource: {contained: [{status: status}]}, organization_id: org_id).drug_deleted?)
           .to eq(deletion_status)
       end
     end

spec/services/bulk_api_import/fhir_observation_importer_spec.rb

@@ -1,9 +1,9 @@
 require "rails_helper"
 
 RSpec.describe BulkApiImport::FhirObservationImporter do
-  before { create(:facility) }
-  let(:import_user) { ImportUser.find_or_create }
-  let(:facility) { import_user.facility }
+  let(:facility) { create(:facility) }
+  let(:org_id) { facility.organization_id }
+  let(:import_user) { ImportUser.find_or_create(org_id: org_id) }
   let(:org_id) { facility.organization_id }
   let(:facility_identifier) do
     create(:facility_business_identifier, facility: facility, identifier_type: :external_org_facility_id)

spec/services/bulk_api_import/fhir_patient_importer_spec.rb

@@ -1,10 +1,9 @@
 require "rails_helper"
 
 RSpec.describe BulkApiImport::FhirPatientImporter do
-  before { create(:facility) }
-  let(:import_user) { ImportUser.find_or_create }
-  let(:org_id) { import_user.organization_id }
-  let(:facility) { import_user.facility }
+  let(:facility) { create(:facility) }
+  let(:org_id) { facility.organization_id }
+  let(:import_user) { ImportUser.find_or_create(org_id: org_id) }
   let(:facility_identifier) do
     create(:facility_business_identifier, facility: facility, identifier_type: :external_org_facility_id)
   end

spec/services/bulk_api_import/importer_spec.rb

@@ -1,10 +1,8 @@
 require "rails_helper"
 
 RSpec.describe BulkApiImport::Importer do
-  before { FactoryBot.create(:facility) } # needed for our bot import user
-
   describe "#import" do
-    let(:facility) { Facility.first }
+    let(:facility) { create(:facility) }
     let(:organization_id) { facility.organization_id }
     let(:facility_identifier) do
       create(:facility_business_identifier, facility: facility, identifier_type: :external_org_facility_id)
@@ -80,7 +78,8 @@
 
   describe "#resource_importer" do
     it "fetches the correct importer" do
-      importer = described_class.new(resource_list: [], organization_id: "org_id")
+      org_id = create(:facility).organization_id
+      importer = described_class.new(resource_list: [], organization_id: org_id)
 
       [
         {input: {resourceType: "Patient"}, expected_importer: BulkApiImport::FhirPatientImporter},
@@ -89,7 +88,7 @@
         {input: {resourceType: "MedicationRequest"}, expected_importer: BulkApiImport::FhirMedicationRequestImporter},
         {input: {resourceType: "Condition"}, expected_importer: BulkApiImport::FhirConditionImporter}
       ].each do |input:, expected_importer:|
-        expect(importer.resource_importer(input, "org_id"))
+        expect(importer.resource_importer(input, org_id))
           .to be_an_instance_of(expected_importer)
       end
     end