Inactive users (#564)

* Allow users to be marked as inactive

* Import inactive users

app/services/imports/user_import_service.rb

@@ -16,12 +16,11 @@ module Imports
       name = user_field_value(xml_document, "full-name")
       email = user_field_value(xml_document, "email").downcase.strip
       deleted = user_field_value(xml_document, "deleted")
-      date_deactivated = user_field_value(xml_document, "date-deactivated")
 
       if User.find_by(old_user_id:, organisation:)
         @logger.warn("User #{name} with old user id #{old_user_id} is already present, skipping.")
-      elsif deleted == "true" || date_deactivated.present?
-        @logger.warn("User #{name} with old user id #{old_user_id} is deleted or deactivated, skipping.")
+      elsif deleted == "true"
+        @logger.warn("User #{name} with old user id #{old_user_id} is deleted, 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")))
@@ -38,6 +37,7 @@ module Imports
           role: role(user_field_value(xml_document, "user-type")),
           is_dpo: is_dpo?(user_field_value(xml_document, "user-type")),
           is_key_contact: is_key_contact?(user_field_value(xml_document, "contact-priority-id")),
+          active: user_field_value(xml_document, "active"),
         )
       end
     end

db/migrate/20220510134721_add_inactive_users.rb

@@ -0,0 +1,5 @@
+class AddInactiveUsers < ActiveRecord::Migration[7.0]
+  def change
+    add_column :users, :active, :boolean, default: true
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[7.0].define(version: 2022_05_10_091620) do
+ActiveRecord::Schema[7.0].define(version: 2022_05_10_134721) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
 
@@ -339,6 +339,7 @@ ActiveRecord::Schema[7.0].define(version: 2022_05_10_091620) do
     t.string "direct_otp"
     t.datetime "direct_otp_sent_at", precision: nil
     t.datetime "totp_timestamp", precision: nil
+    t.boolean "active", default: true
     t.index ["email"], name: "index_users_on_email", unique: true
     t.index ["encrypted_otp_secret_key"], name: "index_users_on_encrypted_otp_secret_key", unique: true
     t.index ["organisation_id"], name: "index_users_on_organisation_id"

spec/fixtures/softwire_imports/users/9ed81a262215a1634f0809effa683e38924d8bcb.xml

@@ -0,0 +1,13 @@
+<user:user xmlns:user="dclg:user">
+  <user:id>9ed81a262215a1634f0809effa683e38924d8bcb</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.doe2@gov.uk</user:email>
+  <user:user-type>Data Provider</user:user-type>
+  <user:active>false</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/models/user_spec.rb

@@ -65,6 +65,10 @@ RSpec.describe User, type: :model do
         .to change { user.reload.is_data_protection_officer? }.from(false).to(true)
     end
 
+    it "is active by default" do
+      expect(user.active).to be true
+    end
+
     it "does not require 2FA" do
       expect(user.need_two_factor_authentication?(nil)).to be false
     end

spec/services/imports/user_import_service_spec.rb

@@ -32,6 +32,7 @@ RSpec.describe Imports::UserImportService do
       expect(user).to be_data_provider
       expect(user.organisation.old_org_id).to eq(old_org_id)
       expect(user.is_key_contact?).to be false
+      expect(user.active).to be true
     end
 
     it "refuses to create a user belonging to a non existing organisation" do
@@ -154,6 +155,15 @@ RSpec.describe Imports::UserImportService do
             .not_to change(User, :count)
         end
       end
+
+      context "when the user was deactivated in the old system" do
+        let(:old_user_id) { "9ed81a262215a1634f0809effa683e38924d8bcb" }
+
+        it "marks them as not active" do
+          import_service.create_users("user_directory")
+          expect(User.find_by(old_user_id:).active).to be false
+        end
+      end
     end
   end
 end