From 501fcd91e0c4b0495c380fb786e2f13afddd40e0 Mon Sep 17 00:00:00 2001 From: baarkerlounger <5101747+baarkerlounger@users.noreply.github.com> Date: Tue, 10 May 2022 15:14:04 +0100 Subject: [PATCH] Inactive users (#564) * Allow users to be marked as inactive * Import inactive users --- app/services/imports/user_import_service.rb | 6 +++--- db/migrate/20220510134721_add_inactive_users.rb | 5 +++++ db/schema.rb | 3 ++- .../9ed81a262215a1634f0809effa683e38924d8bcb.xml | 13 +++++++++++++ spec/models/user_spec.rb | 4 ++++ spec/services/imports/user_import_service_spec.rb | 10 ++++++++++ 6 files changed, 37 insertions(+), 4 deletions(-) create mode 100644 db/migrate/20220510134721_add_inactive_users.rb create mode 100644 spec/fixtures/softwire_imports/users/9ed81a262215a1634f0809effa683e38924d8bcb.xml diff --git a/app/services/imports/user_import_service.rb b/app/services/imports/user_import_service.rb index 492cc715c..aec227f54 100644 --- a/app/services/imports/user_import_service.rb +++ b/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 diff --git a/db/migrate/20220510134721_add_inactive_users.rb b/db/migrate/20220510134721_add_inactive_users.rb new file mode 100644 index 000000000..453549a2c --- /dev/null +++ b/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 diff --git a/db/schema.rb b/db/schema.rb index ba9001e74..669a9b325 100644 --- a/db/schema.rb +++ b/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" diff --git a/spec/fixtures/softwire_imports/users/9ed81a262215a1634f0809effa683e38924d8bcb.xml b/spec/fixtures/softwire_imports/users/9ed81a262215a1634f0809effa683e38924d8bcb.xml new file mode 100644 index 000000000..791486bab --- /dev/null +++ b/spec/fixtures/softwire_imports/users/9ed81a262215a1634f0809effa683e38924d8bcb.xml @@ -0,0 +1,13 @@ + + 9ed81a262215a1634f0809effa683e38924d8bcb + xxx + John Doe + john.doe + 7c5bd5fb549c09a2c55d7cb90d7ba84927e64618 + john.doe2@gov.uk + Data Provider + false + false + None + 02012345678 + diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index da518eb34..97b1fdaf1 100644 --- a/spec/models/user_spec.rb +++ b/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 diff --git a/spec/services/imports/user_import_service_spec.rb b/spec/services/imports/user_import_service_spec.rb index 541ce899e..b3be1cd12 100644 --- a/spec/services/imports/user_import_service_spec.rb +++ b/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