Handle non-unqiue email imports

app/services/imports/user_import_service.rb

@@ -14,11 +14,16 @@ module Imports
       organisation = Organisation.find_by(old_org_id: user_field_value(xml_document, "institution"))
       old_user_id = user_field_value(xml_document, "id")
       name = user_field_value(xml_document, "full-name")
+      email = user_field_value(xml_document, "email")
       if User.find_by(old_user_id:, organisation:)
         @logger.warn("User #{name} with old user id #{old_user_id} is already present, skipping.")
+      elsif (user = User.find_by(email:, organisation:))
+        is_dpo = user.is_data_protection_officer? || is_dpo?(user_field_value(xml_document, "user-type"))
+        role = highest_role(user.role, role(user_field_value(xml_document, "user-type")))
+        user.update!(role:, is_dpo:)
       else
         User.create!(
-          email: user_field_value(xml_document, "user-name"),
+          email:,
           name:,
           password: Devise.friendly_token,
           phone: user_field_value(xml_document, "telephone-no"),
@@ -45,6 +50,14 @@ module Imports
       }[field_value.downcase.strip]
     end
 
+    def highest_role(role_a, role_b)
+      return unless role_a || role_b
+      return role_a unless role_b
+      return role_b unless role_a
+
+      [role_a, role_b].map(&:to_sym).sort! { |a, b| User::ROLES[b] <=> User::ROLES[a] }.first
+    end
+
     def is_dpo?(field_value)
       return false if field_value.blank?
 

spec/fixtures/softwire_imports/users/10c887710550844e2551b3e0fb88dc9b4a8a642b.xml

@@ -0,0 +1,13 @@
+<user:user xmlns:user="dclg:user">
+  <user:id>10c887710550844e2551b3e0fb88dc9b4a8a642b</user:id>
+  <user:password>xxx</user:password>
+  <user:full-name>John Doe</user:full-name>
+  <user:user-name>john.doe</user:user-name>
+  <user:institution>7c5bd5fb549c09a2c55d7cb90d7ba84927e64618</user:institution>
+  <user:email>john.doe@gov.uk</user:email>
+  <user:user-type>Data Protection Officer</user:user-type>
+  <user:active>true</user:active>
+  <user:deleted>false</user:deleted>
+  <user:contact-priority-id>None</user:contact-priority-id>
+  <user:telephone-no>02012345678</user:telephone-no>
+</user:user>

spec/fixtures/softwire_imports/users/b7829b1a5dfb68bb1e01c08445830c0add40907c.xml

@@ -2,7 +2,7 @@
   <user:id>b7829b1a5dfb68bb1e01c08445830c0add40907c</user:id>
   <user:password>xxx</user:password>
   <user:full-name>John Doe</user:full-name>
-  <user:user-name>john.doe@gov.uk</user:user-name>
+  <user:user-name>john.doe</user:user-name>
   <user:institution>7c5bd5fb549c09a2c55d7cb90d7ba84927e64618</user:institution>
   <user:email>john.doe@gov.uk</user:email>
   <user:user-type>Private Data Downloader</user:user-type>

spec/fixtures/softwire_imports/users/d4729b1a5dfb68bb1e01c08445830c0add40907c.xml

@@ -2,7 +2,7 @@
   <user:id>d4729b1a5dfb68bb1e01c08445830c0add40907c</user:id>
   <user:password>xxx</user:password>
   <user:full-name>John Doe</user:full-name>
-  <user:user-name>john.doe@gov.uk</user:user-name>
+  <user:user-name>john.doe</user:user-name>
   <user:institution>7c5bd5fb549c09a2c55d7cb90d7ba84927e64618</user:institution>
   <user:email>john.doe@gov.uk</user:email>
   <user:user-type>Co-ordinator</user:user-type>

spec/fixtures/softwire_imports/users/d6717836154cd9a58f9e2f1d3077e3ab81e07613.xml

@@ -2,7 +2,7 @@
   <user:id>d6717836154cd9a58f9e2f1d3077e3ab81e07613</user:id>
   <user:password>xxx</user:password>
   <user:full-name>John Doe</user:full-name>
-  <user:user-name>john.doe@gov.uk</user:user-name>
+  <user:user-name>john.doe</user:user-name>
   <user:institution>7c5bd5fb549c09a2c55d7cb90d7ba84927e64618</user:institution>
   <user:email>john.doe@gov.uk</user:email>
   <user:user-type>Co-ordinator</user:user-type>

spec/fixtures/softwire_imports/users/fc7625a02b24ae16162aa63ae7cb33feeec0c373.xml

@@ -2,7 +2,7 @@
   <user:id>fc7625a02b24ae16162aa63ae7cb33feeec0c373</user:id>
   <user:password>xxx</user:password>
   <user:full-name>John Doe</user:full-name>
-  <user:user-name>john.doe@gov.uk</user:user-name>
+  <user:user-name>john.doe</user:user-name>
   <user:institution>7c5bd5fb549c09a2c55d7cb90d7ba84927e64618</user:institution>
   <user:email>john.doe@gov.uk</user:email>
   <user:user-type>Data Provider</user:user-type>

spec/services/imports/user_import_service_spec.rb

@@ -57,6 +57,18 @@ RSpec.describe Imports::UserImportService do
       end
     end
 
+    context "when the user is a data protection officer" do
+      let(:old_user_id) { "10c887710550844e2551b3e0fb88dc9b4a8a642b" }
+
+      it "marks them as a data protection officer" do
+        FactoryBot.create(:organisation, old_org_id:)
+        import_service.create_users("user_directory")
+
+        user = User.find_by(old_user_id:)
+        expect(user.is_data_protection_officer?).to be true
+      end
+    end
+
     context "when the user was a 'Key Performance Contact' in the old system" do
       let(:old_user_id) { "d4729b1a5dfb68bb1e01c08445830c0add40907c" }
 
@@ -92,5 +104,54 @@ RSpec.describe Imports::UserImportService do
         import_service.create_users("user_directory")
       end
     end
+
+    context "when a user has already been imported with that email" do
+      let!(:org) { FactoryBot.create(:organisation, old_org_id:) }
+      let!(:user) { FactoryBot.create(:user, :data_provider, organisation: org, email: "john.doe@gov.uk") }
+
+      context "when the duplicate role is higher than the original role" do
+        let(:old_user_id) { "d4729b1a5dfb68bb1e01c08445830c0add40907c" }
+
+        it "upgrades their role" do
+          import_service.create_users("user_directory")
+          expect(user.reload).to be_data_coordinator
+        end
+
+        it "does not create a new record" do
+          expect { import_service.create_users("user_directory") }
+            .not_to change(User, :count)
+        end
+      end
+
+      context "when the duplicate role is lower than the original role" do
+        let!(:user) { FactoryBot.create(:user, :data_coordinator, organisation: org, email: "john.doe@gov.uk") }
+        let(:old_user_id) { "fc7625a02b24ae16162aa63ae7cb33feeec0c373" }
+
+        it "does not change their role" do
+          expect { import_service.create_users("user_directory") }
+            .not_to(change { user.reload.role })
+        end
+
+        it "does not create a new record" do
+          expect { import_service.create_users("user_directory") }
+            .not_to change(User, :count)
+        end
+      end
+
+      context "when the duplicate record is a data protection officer role" do
+        let!(:user) { FactoryBot.create(:user, :data_coordinator, organisation: org, email: "john.doe@gov.uk") }
+        let(:old_user_id) { "10c887710550844e2551b3e0fb88dc9b4a8a642b" }
+
+        it "marks them as a data protection officer" do
+          import_service.create_users("user_directory")
+          expect(user.reload.is_data_protection_officer?).to be true
+        end
+
+        it "does not create a new record" do
+          expect { import_service.create_users("user_directory") }
+            .not_to change(User, :count)
+        end
+      end
+    end
   end
 end