From 258ef6364fa51477b224d5081073fdaffc812ca9 Mon Sep 17 00:00:00 2001 From: Jack S Date: Mon, 26 Jun 2023 12:25:05 +0100 Subject: [PATCH] Refactor script to create users with unique email --- ..._protection_confirmation_import_service.rb | 19 ++++++++++--- ...ection_confirmation_import_service_spec.rb | 27 +++---------------- 2 files changed, 19 insertions(+), 27 deletions(-) diff --git a/app/services/imports/data_protection_confirmation_import_service.rb b/app/services/imports/data_protection_confirmation_import_service.rb index 30d28b647..e2694ecba 100644 --- a/app/services/imports/data_protection_confirmation_import_service.rb +++ b/app/services/imports/data_protection_confirmation_import_service.rb @@ -8,6 +8,10 @@ module Imports def create_data_protection_confirmation(xml_document) org = Organisation.find_by(old_org_id: record_field_value(xml_document, "institution")) + + return log_org_must_exist if org.blank? + return log_dpc_already_present(org) if org.data_protection_confirmed? + dp_officer = User.find_by( name: record_field_value(xml_document, "dp-user"), organisation: org, @@ -20,6 +24,9 @@ module Imports organisation: org, is_dpo: true, encrypted_password: SecureRandom.hex(10), + email: SecureRandom.uuid, + confirmed_at: Time.zone.now, + active: false, ) dp_officer.save!(validate: false) end @@ -32,14 +39,18 @@ module Imports old_org_id: record_field_value(xml_document, "institution"), created_at: record_field_value(xml_document, "change-date").to_time(:utc), ) - rescue ActiveRecord::RecordNotUnique - id = record_field_value(xml_document, "id") - dp_officer_name = record_field_value(xml_document, "dp-user") - @logger.warn("Data protection confirmation #{id} created by #{dp_officer_name} for #{org.name} is already present, skipping.") end def record_field_value(xml_document, field) field_value(xml_document, "dataprotect", field) end + + def log_dpc_already_present(org) + @logger.warn("Data protection confirmation for #{org.name} with id #{org.id} is already present, skipping.") + end + + def log_org_must_exist + @logger.error("Organisation must exist") + end end end diff --git a/spec/services/imports/data_protection_confirmation_import_service_spec.rb b/spec/services/imports/data_protection_confirmation_import_service_spec.rb index 7c3d8f8b2..eb386c117 100644 --- a/spec/services/imports/data_protection_confirmation_import_service_spec.rb +++ b/spec/services/imports/data_protection_confirmation_import_service_spec.rb @@ -23,7 +23,7 @@ RSpec.describe Imports::DataProtectionConfirmationImportService do context "when the organisation in the import file doesn't exist in the system" do it "does not create a data protection confirmation" do - expect(logger).to receive(:error).with(/Organisation must exist/) + expect(logger).to receive(:error).with("Organisation must exist") import_service.create_data_protection_confirmations("data_protection_directory") end end @@ -32,11 +32,12 @@ RSpec.describe Imports::DataProtectionConfirmationImportService do let!(:organisation) { create(:organisation, :without_dpc, old_org_id:) } context "when a data protection officer with matching name does not exists for the organisation" do - it "creates a data protection officer without sign in credentials" do + it "creates an inactive data protection officer" do expect { import_service.create_data_protection_confirmations("data_protection_directory") } .to change(User, :count).by(1) data_protection_officer = User.find_by(organisation:, is_dpo: true) - expect(data_protection_officer.email).to eq("") + expect(data_protection_officer.confirmed_at).not_to be_nil + expect(data_protection_officer.active).to be false end it "successfully create a data protection confirmation record with the expected data" do @@ -46,26 +47,6 @@ RSpec.describe Imports::DataProtectionConfirmationImportService do expect(confirmation.confirmed).to be_truthy expect(Time.zone.local_to_utc(confirmation.created_at)).to eq(Time.utc(2018, 0o6, 0o5, 10, 36, 49)) end - - context "when a user with empty email already exists" do - before do - User.new( - name: "Test McTest", - organisation: create(:organisation), - encrypted_password: SecureRandom.hex(10), - ).save!(validate: false) - allow(logger).to receive(:warn) - end - - it "does not create a data sharing confirmation because it thinks it exists already and logs a misleading message" do - expect(logger).to receive(:warn).with("Data protection confirmation 7c5bd5fb549c09a2c55d7cb90d7ba84927e64618 created by John Doe for DLUHC is already present, skipping.") - import_service.create_data_protection_confirmations("data_protection_directory") - end - - it "does add a data protection confirmation and the org still does not have it" do - expect { import_service.create_data_protection_confirmations("data_protection_directory") }.to not_change { organisation.reload.data_protection_confirmation }.from(nil) - end - end end context "when a data protection officer with matching name already exists for the organisation" do