diff --git a/app/services/imports/user_import_service.rb b/app/services/imports/user_import_service.rb index e8195fcd9..32250b1b9 100644 --- a/app/services/imports/user_import_service.rb +++ b/app/services/imports/user_import_service.rb @@ -12,17 +12,21 @@ module Imports def create_user(xml_document) organisation = Organisation.find_by(old_org_id: user_field_value(xml_document, "institution")) - User.create!( - email: user_field_value(xml_document, "user-name"), - name: user_field_value(xml_document, "full-name"), - password: Devise.friendly_token, - phone: user_field_value(xml_document, "telephone-no"), - old_user_id: user_field_value(xml_document, "id"), - organisation:, - role: PROVIDER_TYPE[user_field_value(xml_document, "user-type")], - ) - rescue ActiveRecord::RecordNotUnique - @logger.warn("User #{name} with old user id #{old_user_id} is already present, skipping.") + old_user_id = user_field_value(xml_document, "id") + name = user_field_value(xml_document, "full-name") + if User.find_by(old_user_id: old_user_id, organisation: organisation) + @logger.warn("User #{name} with old user id #{old_user_id} is already present, skipping.") + else + User.create!( + email: user_field_value(xml_document, "user-name"), + name: name, + password: Devise.friendly_token, + phone: user_field_value(xml_document, "telephone-no"), + old_user_id: old_user_id, + organisation:, + role: PROVIDER_TYPE[user_field_value(xml_document, "user-type")], + ) + end end def user_field_value(xml_document, field) diff --git a/spec/services/imports/user_import_service_spec.rb b/spec/services/imports/user_import_service_spec.rb index 0e0c4d8b0..4aefc4e2b 100644 --- a/spec/services/imports/user_import_service_spec.rb +++ b/spec/services/imports/user_import_service_spec.rb @@ -2,36 +2,51 @@ require "rails_helper" RSpec.describe Imports::UserImportService do let(:fixture_directory) { "spec/fixtures/softwire_imports/users" } - let(:user_file) { File.open("#{fixture_directory}/fc7625a02b24ae16162aa63ae7cb33feeec0c373.xml") } + let(:old_user_id) { "fc7625a02b24ae16162aa63ae7cb33feeec0c373" } + let(:old_org_id) { "7c5bd5fb549c09a2c55d7cb90d7ba84927e64618" } + let(:user_file) { File.open("#{fixture_directory}/#{old_user_id}.xml") } let(:storage_service) { instance_double(StorageService) } context "when importing users" do subject(:import_service) { described_class.new(storage_service) } - before do allow(storage_service).to receive(:list_files) - .and_return(["user_directory/fc7625a02b24ae16162aa63ae7cb33feeec0c373.xml"]) + .and_return(["user_directory/#{old_user_id}.xml"]) allow(storage_service).to receive(:get_file_io) - .with("user_directory/fc7625a02b24ae16162aa63ae7cb33feeec0c373.xml") + .with("user_directory/#{old_user_id}.xml") .and_return(user_file) end it "successfully create a user with the expected data" do - FactoryBot.create(:organisation, old_org_id: "7c5bd5fb549c09a2c55d7cb90d7ba84927e64618") + FactoryBot.create(:organisation, old_org_id: old_org_id) import_service.create_users("user_directory") - user = User.find_by(old_user_id: "fc7625a02b24ae16162aa63ae7cb33feeec0c373") + user = User.find_by(old_user_id: old_user_id) expect(user.name).to eq("John Doe") expect(user.email).to eq("john.doe@gov.uk") expect(user.encrypted_password).not_to be_nil expect(user.phone).to eq("02012345678") expect(user).to be_data_provider - expect(user.organisation.old_org_id).to eq("7c5bd5fb549c09a2c55d7cb90d7ba84927e64618") + expect(user.organisation.old_org_id).to eq(old_org_id) end it "refuses to create a user belonging to a non existing organisation" do expect { import_service.create_users("user_directory") } .to raise_error(ActiveRecord::RecordInvalid, /Organisation must exist/) end + + context "when the user has already been imported previously" do + let!(:org) { FactoryBot.create(:organisation, old_org_id: old_org_id) } + let!(:user) do + FactoryBot.create( + :user, old_user_id: old_user_id, organisation: org + ) + end + + it "logs that the user already exists" do + expect(Rails.logger).to receive(:warn) + import_service.create_users("user_directory") + end + end end end