Bulk upload for sales (<%= @bulk_upload.year_combo %>)
diff --git a/app/views/bulk_upload_shared/_moved_user_banner.html.erb b/app/views/bulk_upload_shared/_moved_user_banner.html.erb
new file mode 100644
index 000000000..9ab97022e
--- /dev/null
+++ b/app/views/bulk_upload_shared/_moved_user_banner.html.erb
@@ -0,0 +1,12 @@
+<% if @bulk_upload.choice == "cancelled-by-moved-user" %>
+ <%= govuk_notification_banner(title_text: "Important") do %>
+
+ This error report is out of date.
+
+ <% if current_user.id == @bulk_upload.moved_user_id %>
+ You moved to a different organisation since this file was uploaded. Reupload the file to get an accurate error report.
+ <% else %>
+ Some logs in this upload are assigned to <%= @bulk_upload.moved_user_name %>, who has moved to a different organisation since this file was uploaded. Reupload the file to get an accurate error report.
+ <% end %>
+ <% end %>
+<% end %>
diff --git a/app/views/bulk_upload_shared/guidance.html.erb b/app/views/bulk_upload_shared/guidance.html.erb
index c32295b13..e530aa5b5 100644
--- a/app/views/bulk_upload_shared/guidance.html.erb
+++ b/app/views/bulk_upload_shared/guidance.html.erb
@@ -81,8 +81,8 @@
<%= accordion.with_section(heading_text: "Next steps") do %>
Once you've saved your CSV file, you can upload it via a button at the top of the lettings and sales logs pages.
When your file is done processing, you will receive an email explaining your next steps. If all your data is valid, your logs will be created. If some data is invalid, you’ll receive an email with instructions about how to resolve the errors.
-
If your file has errors on fields 1 through 17, you must fix these in the CSV. This is because we need to know these answers to validate the rest of the data. Any errors in these fields will be featured in the error report’s summary tab.
-
If none of your errors are in fields 1 through 17, you can choose how to fix the errors. You can either fix them in the CSV and reupload, or create partially complete logs and answer the remaining questions on the CORE site. Any errors that affect a significant number of logs will be featured in the error report’s summary tab to help you decide.
+
If your file has errors on fields 1 through 15 for lettings, or 1 through 18 for sales, you must fix these in the CSV. This is because we need to know these answers to validate the rest of the data. Any errors in these fields will be featured in the error report’s summary tab.
+
If none of your errors are in fields 1 through 15 for lettings, or 1 through 18 for sales, you can choose how to fix the errors. You can either fix them in the CSV and reupload, or create partially complete logs and answer the remaining questions on the CORE site. Any errors that affect a significant number of logs will be featured in the error report’s summary tab to help you decide.
<% end %>
diff --git a/app/views/users/edit.html.erb b/app/views/users/edit.html.erb
index 6fdfdb5aa..4c610c22a 100644
--- a/app/views/users/edit.html.erb
+++ b/app/views/users/edit.html.erb
@@ -7,6 +7,7 @@
<%= form_for(@user, as: :user, html: { method: :patch }) do |f| %>
diff --git a/app/views/users/show.html.erb b/app/views/users/show.html.erb
index df8c0e915..7f42107f8 100644
--- a/app/views/users/show.html.erb
+++ b/app/views/users/show.html.erb
@@ -68,7 +68,15 @@
<%= summary_list.with_row do |row|
row.with_key { "Organisation" }
row.with_value { current_user.support? ? govuk_link_to(@user.organisation.name, lettings_logs_organisation_path(@user.organisation)) : @user.organisation.name }
- row.with_action
+ if UserPolicy.new(current_user, @user).edit_organisation?
+ row.with_action(
+ visually_hidden_text: "organisation",
+ href: aliased_user_edit(@user, current_user),
+ html_attributes: { "data-qa": "change-organisation" },
+ )
+ else
+ row.with_action
+ end
end %>
<%= summary_list.with_row do |row|
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 7d52a92e0..42ef43be1 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -172,6 +172,10 @@ en:
too_short: The password you entered is too short. Enter a password that is %{count} characters or longer.
reset_password_token:
invalid: "That link is invalid. Check you are using the correct link."
+ log_reassignment:
+ blank: "Select if you want to reassign logs"
+ missing_managing_agents: "%{new_organisation} must be a stock owner of %{missing_managing_agents} to make this change."
+ missing_stock_owners: "%{new_organisation} must be a managing agent of %{missing_stock_owners} to make this change."
merge_request:
attributes:
absorbing_organisation_id:
diff --git a/config/routes.rb b/config/routes.rb
index 4817bebb9..f2c86e37c 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -124,6 +124,10 @@ Rails.application.routes.draw do
resources :users do
get "edit-dpo", to: "users#dpo"
get "edit-key-contact", to: "users#key_contact"
+ get "log-reassignment", to: "users#log_reassignment"
+ patch "log-reassignment", to: "users#update_log_reassignment"
+ get "organisation-change-confirmation", to: "users#organisation_change_confirmation"
+ patch "organisation-change-confirmation", to: "users#confirm_organisation_change"
collection do
get :search
diff --git a/db/migrate/20240911152702_add_moved_user_to_bu.rb b/db/migrate/20240911152702_add_moved_user_to_bu.rb
new file mode 100644
index 000000000..9a6079858
--- /dev/null
+++ b/db/migrate/20240911152702_add_moved_user_to_bu.rb
@@ -0,0 +1,5 @@
+class AddMovedUserToBu < ActiveRecord::Migration[7.0]
+ def change
+ add_column :bulk_uploads, :moved_user_id, :integer
+ end
+end
diff --git a/db/schema.rb b/db/schema.rb
index da8df8217..a4c36b8a7 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -10,7 +10,11 @@
#
# It's strongly recommended that you check this file into your version control system.
+<<<<<<< CLDC-3623-Support-user-bulk-uploading
ActiveRecord::Schema[7.0].define(version: 2024_09_05_092332) do
+=======
+ActiveRecord::Schema[7.0].define(version: 2024_09_11_152702) do
+>>>>>>> main
# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
@@ -42,7 +46,11 @@ ActiveRecord::Schema[7.0].define(version: 2024_09_05_092332) do
t.text "choice"
t.integer "total_logs_count"
t.string "rent_type_fix_status", default: "not_applied"
+<<<<<<< CLDC-3623-Support-user-bulk-uploading
t.integer "organisation_id"
+=======
+ t.integer "moved_user_id"
+>>>>>>> main
t.index ["identifier"], name: "index_bulk_uploads_on_identifier", unique: true
t.index ["user_id"], name: "index_bulk_uploads_on_user_id"
end
diff --git a/spec/features/accessibility_spec.rb b/spec/features/accessibility_spec.rb
index d0c218445..97f632d92 100644
--- a/spec/features/accessibility_spec.rb
+++ b/spec/features/accessibility_spec.rb
@@ -29,9 +29,15 @@ RSpec.describe "Accessibility", js: true do
Rails.application.routes.routes.select { |route| route.verb == "GET" && route.path.spec.to_s.start_with?("/user") }.map { |route|
route_path = route.path.spec.to_s
route_path.gsub(":id", other_user.id.to_s).gsub(":user_id", other_user.id.to_s).gsub("(.:format)", "")
+ .gsub("log-reassignment", "log-reassignment?&organisation_id=#{other_user.organisation_id}")
+ .gsub("organisation-change-confirmation", "organisation-change-confirmation?&organisation_id=#{other_user.organisation_id}&log_reassignment=reassign_all")
}.uniq
end
+ before do
+ create(:lettings_log, assigned_to: other_user)
+ end
+
it "is has accessible pages" do
user_paths.each do |path|
visit(path)
diff --git a/spec/helpers/user_helper_spec.rb b/spec/helpers/user_helper_spec.rb
index 829195f6c..c88790aa0 100644
--- a/spec/helpers/user_helper_spec.rb
+++ b/spec/helpers/user_helper_spec.rb
@@ -61,4 +61,90 @@ RSpec.describe UserHelper do
end
end
end
+
+ describe "organisation_change_warning" do
+ context "when user doesn't own any logs" do
+ it "returns a message with the number of logs" do
+ expected_text = "You’re moving #{user.name} from #{user.organisation.name} to #{current_user.organisation.name}. There are 0 logs assigned to them."
+ expect(organisation_change_warning(user, current_user.organisation)).to eq(expected_text)
+ end
+ end
+
+ context "when user owns 1 lettings log" do
+ before do
+ create(:lettings_log, assigned_to: user)
+ end
+
+ it "returns a message with the number of logs" do
+ expected_text = "You’re moving #{user.name} from #{user.organisation.name} to #{current_user.organisation.name}. There is 1 log assigned to them."
+ expect(organisation_change_warning(user, current_user.organisation)).to eq(expected_text)
+ end
+ end
+
+ context "when user owns 1 sales log" do
+ before do
+ create(:sales_log, assigned_to: user)
+ end
+
+ it "returns a message with the number of logs" do
+ expected_text = "You’re moving #{user.name} from #{user.organisation.name} to #{current_user.organisation.name}. There is 1 log assigned to them."
+ expect(organisation_change_warning(user, current_user.organisation)).to eq(expected_text)
+ end
+ end
+
+ context "when user owns multiple logs" do
+ before do
+ create(:lettings_log, assigned_to: user)
+ create(:sales_log, assigned_to: user)
+ end
+
+ it "returns a message with the number of logs" do
+ expected_text = "You’re moving #{user.name} from #{user.organisation.name} to #{current_user.organisation.name}. There are 2 logs assigned to them."
+ expect(organisation_change_warning(user, current_user.organisation)).to eq(expected_text)
+ end
+ end
+ end
+
+ describe "organisation_change_confirmation_warning" do
+ context "when user owns logs" do
+ before do
+ create(:lettings_log, assigned_to: user)
+ end
+
+ context "with reassign all choice" do
+ it "returns the correct message" do
+ expected_text = "You’re moving #{user.name} from #{user.organisation.name} to #{current_user.organisation.name}. The stock owner and managing agent on their logs will change to #{current_user.organisation.name}."
+ expect(organisation_change_confirmation_warning(user, current_user.organisation, "reassign_all")).to eq(expected_text)
+ end
+ end
+
+ context "with reassign stock owners choice" do
+ it "returns the correct message" do
+ expected_text = "You’re moving #{user.name} from #{user.organisation.name} to #{current_user.organisation.name}. The stock owner on their logs will change to #{current_user.organisation.name}."
+ expect(organisation_change_confirmation_warning(user, current_user.organisation, "reassign_stock_owner")).to eq(expected_text)
+ end
+ end
+
+ context "with reassign managing agent choice" do
+ it "returns the correct message" do
+ expected_text = "You’re moving #{user.name} from #{user.organisation.name} to #{current_user.organisation.name}. The managing agent on their logs will change to #{current_user.organisation.name}."
+ expect(organisation_change_confirmation_warning(user, current_user.organisation, "reassign_managing_agent")).to eq(expected_text)
+ end
+ end
+
+ context "with unassign choice" do
+ it "returns the correct message" do
+ expected_text = "You’re moving #{user.name} from #{user.organisation.name} to #{current_user.organisation.name}. Their logs will be unassigned."
+ expect(organisation_change_confirmation_warning(user, current_user.organisation, "unassign")).to eq(expected_text)
+ end
+ end
+ end
+
+ context "when user doesn't own logs" do
+ it "returns the correct message" do
+ expected_text = "You’re moving #{user.name} from #{user.organisation.name} to #{current_user.organisation.name}. There are no logs assigned to them."
+ expect(organisation_change_confirmation_warning(user, current_user.organisation, "reassign_all")).to eq(expected_text)
+ end
+ end
+ end
end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 6a04e9a0b..5cb6cb580 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -528,4 +528,304 @@ RSpec.describe User, type: :model do
end
end
end
+
+ describe "#reassign_logs_and_update_organisation" do
+ let(:user) { create(:user) }
+ let(:new_organisation) { create(:organisation) }
+ let!(:lettings_log) { create(:lettings_log, assigned_to: user) }
+ let!(:sales_log) { create(:sales_log, assigned_to: user) }
+ let(:notify_client) { instance_double(Notifications::Client) }
+ let(:devise_notify_mailer) { DeviseNotifyMailer.new }
+
+ before do
+ allow(DeviseNotifyMailer).to receive(:new).and_return(devise_notify_mailer)
+ allow(devise_notify_mailer).to receive(:notify_client).and_return(notify_client)
+ allow(notify_client).to receive(:send_email).and_return(true)
+ end
+
+ context "when reassigning all orgs for logs" do
+ it "reassigns all logs to the new organisation" do
+ user.reassign_logs_and_update_organisation(new_organisation, "reassign_all")
+ expect(lettings_log.reload.owning_organisation).to eq(new_organisation)
+ expect(lettings_log.managing_organisation).to eq(new_organisation)
+ expect(lettings_log.values_updated_at).not_to be_nil
+ expect(sales_log.reload.owning_organisation).to eq(new_organisation)
+ expect(sales_log.managing_organisation).to eq(new_organisation)
+ expect(sales_log.values_updated_at).not_to be_nil
+ end
+
+ it "moves the user to the new organisation" do
+ user.reassign_logs_and_update_organisation(new_organisation, "reassign_all")
+
+ expect(user.organisation).to eq(new_organisation)
+ end
+
+ it "sends organisation update emails" do
+ expected_personalisation = {
+ from_organisation: "#{user.organisation.name} (Organisation ID: #{user.organisation_id})",
+ to_organisation: "#{new_organisation.name} (Organisation ID: #{new_organisation.id})",
+ reassigned_logs_text: "There are 2 logs assigned to you. The stock owner and managing agent on these logs has been changed from #{user.organisation.name} to #{new_organisation.name}.",
+ }
+ user.reassign_logs_and_update_organisation(new_organisation, "reassign_all")
+
+ expect(notify_client).to have_received(:send_email).with(email_address: user.email, template_id: User::ORGANISATION_UPDATE_TEMPLATE_ID, personalisation: expected_personalisation).once
+ end
+
+ context "and there is an error" do
+ before do
+ allow(user).to receive(:update!).and_raise(ActiveRecord::RecordInvalid)
+ end
+
+ it "rolls back the changes" do
+ user.reassign_logs_and_update_organisation(new_organisation, "reassign_all")
+ expect(lettings_log.reload.owning_organisation).not_to eq(new_organisation)
+ expect(lettings_log.managing_organisation).not_to eq(new_organisation)
+ expect(lettings_log.values_updated_at).to be_nil
+ expect(sales_log.reload.owning_organisation).not_to eq(new_organisation)
+ expect(sales_log.managing_organisation).not_to eq(new_organisation)
+ expect(sales_log.values_updated_at).to be_nil
+ expect(user.organisation).not_to eq(new_organisation)
+ end
+ end
+
+ context "and the user has pending logs assigned to them" do
+ let(:lettings_bu) { create(:bulk_upload, :lettings) }
+ let(:sales_bu) { create(:bulk_upload, :sales) }
+ let!(:pending_lettings_log) { build(:lettings_log, status: "pending", assigned_to: user, bulk_upload: lettings_bu) }
+ let!(:pending_sales_log) { build(:sales_log, status: "pending", assigned_to: user, bulk_upload: sales_bu) }
+
+ before do
+ pending_lettings_log.skip_update_status = true
+ pending_lettings_log.save!
+ pending_sales_log.skip_update_status = true
+ pending_sales_log.save!
+ end
+
+ it "sets choice for fixing the logs to cancelled-by-moved-user" do
+ user.reassign_logs_and_update_organisation(new_organisation, "reassign_all")
+
+ expect(lettings_bu.reload.choice).to eq("cancelled-by-moved-user")
+ expect(sales_bu.reload.choice).to eq("cancelled-by-moved-user")
+ expect(lettings_bu.moved_user_id).to eq(user.id)
+ expect(sales_bu.moved_user_id).to eq(user.id)
+
+ expect(pending_lettings_log.reload.status).to eq("pending")
+ expect(pending_sales_log.reload.status).to eq("pending")
+ end
+ end
+ end
+
+ context "when reassigning stock owners for logs" do
+ it "reassigns stock owners for logs to the new organisation" do
+ user.reassign_logs_and_update_organisation(new_organisation, "reassign_stock_owner")
+ expect(lettings_log.reload.owning_organisation).to eq(new_organisation)
+ expect(lettings_log.managing_organisation).not_to eq(new_organisation)
+ expect(lettings_log.values_updated_at).not_to be_nil
+ expect(sales_log.reload.owning_organisation).to eq(new_organisation)
+ expect(sales_log.managing_organisation).not_to eq(new_organisation)
+ expect(sales_log.values_updated_at).not_to be_nil
+ end
+
+ it "moves the user to the new organisation" do
+ user.reassign_logs_and_update_organisation(new_organisation, "reassign_stock_owner")
+
+ expect(user.organisation).to eq(new_organisation)
+ end
+
+ it "sends organisation update emails" do
+ expected_personalisation = {
+ from_organisation: "#{user.organisation.name} (Organisation ID: #{user.organisation_id})",
+ to_organisation: "#{new_organisation.name} (Organisation ID: #{new_organisation.id})",
+ reassigned_logs_text: "There are 2 logs assigned to you. The stock owner on these logs has been changed from #{user.organisation.name} to #{new_organisation.name}.",
+ }
+ user.reassign_logs_and_update_organisation(new_organisation, "reassign_stock_owner")
+
+ expect(notify_client).to have_received(:send_email).with(email_address: user.email, template_id: User::ORGANISATION_UPDATE_TEMPLATE_ID, personalisation: expected_personalisation).once
+ end
+
+ context "and there is an error" do
+ before do
+ allow(user).to receive(:update!).and_raise(ActiveRecord::RecordInvalid)
+ end
+
+ it "rolls back the changes" do
+ user.reassign_logs_and_update_organisation(new_organisation, "reassign_all")
+ expect(lettings_log.reload.owning_organisation).not_to eq(new_organisation)
+ expect(lettings_log.managing_organisation).not_to eq(new_organisation)
+ expect(lettings_log.values_updated_at).to be_nil
+ expect(sales_log.reload.owning_organisation).not_to eq(new_organisation)
+ expect(sales_log.managing_organisation).not_to eq(new_organisation)
+ expect(sales_log.values_updated_at).to be_nil
+ expect(user.organisation).not_to eq(new_organisation)
+ end
+ end
+ end
+
+ context "when reassigning managing agents for logs" do
+ it "reassigns managing agents for logs to the new organisation" do
+ user.reassign_logs_and_update_organisation(new_organisation, "reassign_managing_agent")
+ expect(lettings_log.reload.owning_organisation).not_to eq(new_organisation)
+ expect(lettings_log.managing_organisation).to eq(new_organisation)
+ expect(lettings_log.values_updated_at).not_to be_nil
+ expect(sales_log.reload.owning_organisation).not_to eq(new_organisation)
+ expect(sales_log.managing_organisation).to eq(new_organisation)
+ expect(sales_log.values_updated_at).not_to be_nil
+ end
+
+ it "moves the user to the new organisation" do
+ user.reassign_logs_and_update_organisation(new_organisation, "reassign_managing_agent")
+
+ expect(user.organisation).to eq(new_organisation)
+ end
+
+ it "sends organisation update emails" do
+ expected_personalisation = {
+ from_organisation: "#{user.organisation.name} (Organisation ID: #{user.organisation_id})",
+ to_organisation: "#{new_organisation.name} (Organisation ID: #{new_organisation.id})",
+ reassigned_logs_text: "There are 2 logs assigned to you. The managing agent on these logs has been changed from #{user.organisation.name} to #{new_organisation.name}.",
+ }
+ user.reassign_logs_and_update_organisation(new_organisation, "reassign_managing_agent")
+
+ expect(notify_client).to have_received(:send_email).with(email_address: user.email, template_id: User::ORGANISATION_UPDATE_TEMPLATE_ID, personalisation: expected_personalisation).once
+ end
+
+ context "and there is an error" do
+ before do
+ allow(user).to receive(:update!).and_raise(ActiveRecord::RecordInvalid)
+ end
+
+ it "rolls back the changes" do
+ user.reassign_logs_and_update_organisation(new_organisation, "reassign_all")
+ expect(lettings_log.reload.owning_organisation).not_to eq(new_organisation)
+ expect(lettings_log.managing_organisation).not_to eq(new_organisation)
+ expect(lettings_log.values_updated_at).to be_nil
+ expect(sales_log.reload.owning_organisation).not_to eq(new_organisation)
+ expect(sales_log.managing_organisation).not_to eq(new_organisation)
+ expect(sales_log.values_updated_at).to be_nil
+ expect(user.organisation).not_to eq(new_organisation)
+ end
+ end
+ end
+
+ context "when unassigning the logs" do
+ context "and unassigned user exists" do
+ let!(:unassigned_user) { create(:user, name: "Unassigned", organisation: user.organisation) }
+
+ it "reassigns all the logs to the unassigned user" do
+ user.reassign_logs_and_update_organisation(new_organisation, "unassign")
+
+ expect(lettings_log.reload.assigned_to).to eq(unassigned_user)
+ expect(lettings_log.values_updated_at).not_to be_nil
+ expect(sales_log.reload.assigned_to).to eq(unassigned_user)
+ expect(sales_log.values_updated_at).not_to be_nil
+ end
+
+ it "moves the user to the new organisation" do
+ user.reassign_logs_and_update_organisation(new_organisation, "unassign")
+
+ expect(user.organisation).to eq(new_organisation)
+ end
+
+ it "sends organisation update emails" do
+ expected_personalisation = {
+ from_organisation: "#{user.organisation.name} (Organisation ID: #{user.organisation_id})",
+ to_organisation: "#{new_organisation.name} (Organisation ID: #{new_organisation.id})",
+ reassigned_logs_text: "There are 2 logs assigned to you. These have now been unassigned.",
+ }
+ user.reassign_logs_and_update_organisation(new_organisation, "unassign")
+
+ expect(notify_client).to have_received(:send_email).with(email_address: user.email, template_id: User::ORGANISATION_UPDATE_TEMPLATE_ID, personalisation: expected_personalisation).once
+ end
+
+ context "and there is an error" do
+ before do
+ allow(user).to receive(:update!).and_raise(ActiveRecord::RecordInvalid)
+ end
+
+ it "rolls back the changes" do
+ user.reassign_logs_and_update_organisation(new_organisation, "unassign")
+ expect(lettings_log.reload.assigned_to).to eq(user)
+ expect(lettings_log.values_updated_at).to be_nil
+ expect(sales_log.reload.assigned_to).to eq(user)
+ expect(sales_log.values_updated_at).to be_nil
+ expect(user.organisation).not_to eq(new_organisation)
+ end
+ end
+ end
+
+ context "and unassigned user doesn't exist" do
+ it "reassigns all the logs to the unassigned user" do
+ user.reassign_logs_and_update_organisation(new_organisation, "unassign")
+
+ expect(lettings_log.reload.assigned_to.name).to eq("Unassigned")
+ expect(lettings_log.values_updated_at).not_to be_nil
+ expect(sales_log.reload.assigned_to.name).to eq("Unassigned")
+ expect(sales_log.values_updated_at).not_to be_nil
+ end
+
+ it "moves the user to the new organisation" do
+ user.reassign_logs_and_update_organisation(new_organisation, "unassign")
+
+ expect(user.organisation).to eq(new_organisation)
+ end
+
+ context "and there is an error" do
+ before do
+ allow(user).to receive(:update!).and_raise(ActiveRecord::RecordInvalid)
+ end
+
+ it "rolls back the changes" do
+ user.reassign_logs_and_update_organisation(new_organisation, "unassign")
+ expect(lettings_log.reload.assigned_to).to eq(user)
+ expect(lettings_log.values_updated_at).to be_nil
+ expect(sales_log.reload.assigned_to).to eq(user)
+ expect(sales_log.values_updated_at).to be_nil
+ expect(user.organisation).not_to eq(new_organisation)
+ end
+ end
+ end
+ end
+
+ context "when log_reassignent is not given" do
+ context "and user has no logs" do
+ let(:user_without_logs) { create(:user) }
+
+ it "moves the user to the new organisation" do
+ user_without_logs.reassign_logs_and_update_organisation(new_organisation, nil)
+
+ expect(user_without_logs.organisation).to eq(new_organisation)
+ end
+
+ context "and there is an error" do
+ before do
+ allow(user_without_logs).to receive(:update!).and_raise(ActiveRecord::RecordInvalid)
+ end
+
+ it "rolls back the changes" do
+ user_without_logs.reassign_logs_and_update_organisation(new_organisation, nil)
+ expect(user_without_logs.organisation).not_to eq(new_organisation)
+ end
+ end
+
+ it "sends organisation update emails" do
+ expected_personalisation = {
+ from_organisation: "#{user_without_logs.organisation.name} (Organisation ID: #{user_without_logs.organisation_id})",
+ to_organisation: "#{new_organisation.name} (Organisation ID: #{new_organisation.id})",
+ reassigned_logs_text: "",
+ }
+ user_without_logs.reassign_logs_and_update_organisation(new_organisation, nil)
+
+ expect(notify_client).to have_received(:send_email).with(email_address: user_without_logs.email, template_id: User::ORGANISATION_UPDATE_TEMPLATE_ID, personalisation: expected_personalisation).once
+ end
+ end
+
+ context "and user has logs" do
+ it "does not move the user to the new organisation" do
+ user.reassign_logs_and_update_organisation(new_organisation, nil)
+
+ expect(user.organisation).not_to eq(new_organisation)
+ end
+ end
+ end
+ end
end
diff --git a/spec/policies/user_policy_spec.rb b/spec/policies/user_policy_spec.rb
index e2266cb48..63f3317d8 100644
--- a/spec/policies/user_policy_spec.rb
+++ b/spec/policies/user_policy_spec.rb
@@ -100,6 +100,20 @@ RSpec.describe UserPolicy do
end
end
+ permissions :edit_organisation? do
+ it "as a provider it does not allow changing organisation" do
+ expect(policy).not_to permit(data_provider, data_provider)
+ end
+
+ it "as a coordinator it does not allow changing organisatio" do
+ expect(policy).not_to permit(data_coordinator, data_provider)
+ end
+
+ it "as a support user allows changing other user's organisation" do
+ expect(policy).to permit(support, data_provider)
+ end
+ end
+
permissions :delete? do
context "with active user" do
let(:user) { create(:user, last_sign_in_at: Time.zone.yesterday) }
diff --git a/spec/requests/bulk_upload_lettings_results_controller_spec.rb b/spec/requests/bulk_upload_lettings_results_controller_spec.rb
index 50f5ecd31..dc78c9d78 100644
--- a/spec/requests/bulk_upload_lettings_results_controller_spec.rb
+++ b/spec/requests/bulk_upload_lettings_results_controller_spec.rb
@@ -60,6 +60,28 @@ RSpec.describe BulkUploadLettingsResultsController, type: :request do
expect(response).to be_successful
expect(response.body).to include(bulk_upload.filename)
end
+
+ context "and bulk upload has been cancelled by not the current moved user" do
+ let(:bulk_upload) { create(:bulk_upload, :lettings, user:, bulk_upload_errors:, choice: "cancelled-by-moved-user", moved_user_id: user.id) }
+
+ it "is displays a correct banner" do
+ get "/lettings-logs/bulk-upload-results/#{bulk_upload.id}/summary"
+
+ expect(response.body).to include("This error report is out of date.")
+ expect(response.body).to include("Some logs in this upload are assigned to #{user.name}, who has moved to a different organisation since this file was uploaded. Reupload the file to get an accurate error report.")
+ end
+ end
+
+ context "and bulk upload has been cancelled by the current moved user" do
+ let(:bulk_upload) { create(:bulk_upload, :lettings, user:, bulk_upload_errors:, choice: "cancelled-by-moved-user", moved_user_id: other_user.id) }
+
+ it "is displays a correct banner" do
+ get "/lettings-logs/bulk-upload-results/#{bulk_upload.id}/summary"
+
+ expect(response.body).to include("This error report is out of date.")
+ expect(response.body).to include("You moved to a different organisation since this file was uploaded. Reupload the file to get an accurate error report.")
+ end
+ end
end
end
@@ -107,5 +129,28 @@ RSpec.describe BulkUploadLettingsResultsController, type: :request do
expect(response).to be_not_found
end
end
+
+ context "and bulk upload has been cancelled by not the current moved user" do
+ let(:other_user) { create(:user, organisation: user.organisation) }
+ let(:bulk_upload) { create(:bulk_upload, :lettings, user:, bulk_upload_errors:, choice: "cancelled-by-moved-user", moved_user_id: other_user.id) }
+
+ it "is displays a correct banner" do
+ get "/lettings-logs/bulk-upload-results/#{bulk_upload.id}/summary"
+
+ expect(response.body).to include("This error report is out of date.")
+ expect(response.body).to include("Some logs in this upload are assigned to #{other_user.name}, who has moved to a different organisation since this file was uploaded. Reupload the file to get an accurate error report.")
+ end
+ end
+
+ context "and bulk upload has been cancelled by the current moved user" do
+ let(:bulk_upload) { create(:bulk_upload, :lettings, user:, bulk_upload_errors:, choice: "cancelled-by-moved-user", moved_user_id: user.id) }
+
+ it "is displays a correct banner" do
+ get "/lettings-logs/bulk-upload-results/#{bulk_upload.id}/summary"
+
+ expect(response.body).to include("This error report is out of date.")
+ expect(response.body).to include("You moved to a different organisation since this file was uploaded. Reupload the file to get an accurate error report.")
+ end
+ end
end
end
diff --git a/spec/requests/bulk_upload_lettings_resume_controller_spec.rb b/spec/requests/bulk_upload_lettings_resume_controller_spec.rb
index d110768f0..f6ac9bb4a 100644
--- a/spec/requests/bulk_upload_lettings_resume_controller_spec.rb
+++ b/spec/requests/bulk_upload_lettings_resume_controller_spec.rb
@@ -32,6 +32,26 @@ RSpec.describe BulkUploadLettingsResumeController, type: :request do
expect(response.body).to include("You have created logs from your bulk upload, and the logs are complete. Return to lettings logs to view them.")
end
end
+
+ context "when a choice to reupload has been made" do
+ it "redirects to the error report" do
+ bulk_upload.update!(choice: "upload-again")
+
+ get "/lettings-logs/bulk-upload-resume/#{bulk_upload.id}/start"
+ follow_redirect!
+ expect(response).to redirect_to("/lettings-logs/bulk-upload-results/#{bulk_upload.id}")
+ end
+ end
+
+ context "when bulk upload was cancelled by moved user" do
+ it "redirects to the error report" do
+ bulk_upload.update!(choice: "cancelled-by-moved-user")
+
+ get "/lettings-logs/bulk-upload-resume/#{bulk_upload.id}/start"
+ follow_redirect!
+ expect(response).to redirect_to("/lettings-logs/bulk-upload-results/#{bulk_upload.id}")
+ end
+ end
end
describe "GET /lettings-logs/bulk-upload-resume/:ID/fix-choice" do
@@ -73,6 +93,25 @@ RSpec.describe BulkUploadLettingsResumeController, type: :request do
expect(response).to redirect_to("/lettings-logs/bulk-upload-soft-validations-check/#{bulk_upload.id}/chosen")
end
end
+
+ context "when a choice to reupload has been made" do
+ let(:bulk_upload) { create(:bulk_upload, :lettings, user:, bulk_upload_errors:, choice: "upload-again") }
+
+ it "redirects to the error report" do
+ get "/lettings-logs/bulk-upload-resume/#{bulk_upload.id}/fix-choice"
+ expect(response).to redirect_to("/lettings-logs/bulk-upload-results/#{bulk_upload.id}")
+ end
+ end
+
+ context "when bulk upload was cancelled by moved user" do
+ let(:bulk_upload) { create(:bulk_upload, :lettings, user:, bulk_upload_errors:, choice: "cancelled-by-moved-user") }
+
+ it "redirects to the error report" do
+ get "/lettings-logs/bulk-upload-resume/#{bulk_upload.id}/start"
+ follow_redirect!
+ expect(response).to redirect_to("/lettings-logs/bulk-upload-results/#{bulk_upload.id}")
+ end
+ end
end
describe "GET /lettings-logs/bulk-upload-resume/:ID/fix-choice?soft_errors_only=true" do
diff --git a/spec/requests/bulk_upload_sales_results_controller_spec.rb b/spec/requests/bulk_upload_sales_results_controller_spec.rb
index ac759529a..1bd171dec 100644
--- a/spec/requests/bulk_upload_sales_results_controller_spec.rb
+++ b/spec/requests/bulk_upload_sales_results_controller_spec.rb
@@ -11,6 +11,42 @@ RSpec.describe BulkUploadSalesResultsController, type: :request do
sign_in viewing_user
end
+ describe "GET /sales-logs/bulk-upload-results/:ID/summary" do
+ context "when viewed by another user in the same org" do
+ let(:other_user) { create(:user, organisation: user.organisation) }
+ let(:viewing_user) { other_user }
+
+ it "is accessible" do
+ get "/sales-logs/bulk-upload-results/#{bulk_upload.id}/summary"
+
+ expect(response).to be_successful
+ expect(response.body).to include(bulk_upload.filename)
+ end
+
+ context "and bulk upload has been cancelled by not the current moved user" do
+ let(:bulk_upload) { create(:bulk_upload, :sales, user:, bulk_upload_errors:, choice: "cancelled-by-moved-user", moved_user_id: user.id) }
+
+ it "is displays a correct banner" do
+ get "/sales-logs/bulk-upload-results/#{bulk_upload.id}/summary"
+
+ expect(response.body).to include("This error report is out of date.")
+ expect(response.body).to include("Some logs in this upload are assigned to #{user.name}, who has moved to a different organisation since this file was uploaded. Reupload the file to get an accurate error report.")
+ end
+ end
+
+ context "and bulk upload has been cancelled by the current moved user" do
+ let(:bulk_upload) { create(:bulk_upload, :sales, user:, bulk_upload_errors:, choice: "cancelled-by-moved-user", moved_user_id: other_user.id) }
+
+ it "is displays a correct banner" do
+ get "/sales-logs/bulk-upload-results/#{bulk_upload.id}/summary"
+
+ expect(response.body).to include("This error report is out of date.")
+ expect(response.body).to include("You moved to a different organisation since this file was uploaded. Reupload the file to get an accurate error report.")
+ end
+ end
+ end
+ end
+
describe "GET /sales-logs/bulk-upload-results/:ID" do
it "renders correct year" do
get "/sales-logs/bulk-upload-results/#{bulk_upload.id}"
@@ -68,5 +104,28 @@ RSpec.describe BulkUploadSalesResultsController, type: :request do
expect(response).to be_not_found
end
end
+
+ context "and bulk upload has been cancelled by not the current moved user" do
+ let(:other_user) { create(:user, organisation: user.organisation) }
+ let(:bulk_upload) { create(:bulk_upload, :sales, user:, bulk_upload_errors:, choice: "cancelled-by-moved-user", moved_user_id: other_user.id) }
+
+ it "is displays a correct banner" do
+ get "/sales-logs/bulk-upload-results/#{bulk_upload.id}/summary"
+
+ expect(response.body).to include("This error report is out of date.")
+ expect(response.body).to include("Some logs in this upload are assigned to #{other_user.name}, who has moved to a different organisation since this file was uploaded. Reupload the file to get an accurate error report.")
+ end
+ end
+
+ context "and bulk upload has been cancelled by the current moved user" do
+ let(:bulk_upload) { create(:bulk_upload, :sales, user:, bulk_upload_errors:, choice: "cancelled-by-moved-user", moved_user_id: user.id) }
+
+ it "is displays a correct banner" do
+ get "/sales-logs/bulk-upload-results/#{bulk_upload.id}/summary"
+
+ expect(response.body).to include("This error report is out of date.")
+ expect(response.body).to include("You moved to a different organisation since this file was uploaded. Reupload the file to get an accurate error report.")
+ end
+ end
end
end
diff --git a/spec/requests/bulk_upload_sales_resume_controller_spec.rb b/spec/requests/bulk_upload_sales_resume_controller_spec.rb
index 36db459d3..9c0efded8 100644
--- a/spec/requests/bulk_upload_sales_resume_controller_spec.rb
+++ b/spec/requests/bulk_upload_sales_resume_controller_spec.rb
@@ -32,6 +32,26 @@ RSpec.describe BulkUploadSalesResumeController, type: :request do
expect(response.body).to include("You have created logs from your bulk upload, and the logs are complete. Return to sales logs to view them.")
end
end
+
+ context "when a choice to reupload has been made" do
+ it "redirects to the error report" do
+ bulk_upload.update!(choice: "upload-again")
+
+ get "/sales-logs/bulk-upload-resume/#{bulk_upload.id}/start"
+ follow_redirect!
+ expect(response).to redirect_to("/sales-logs/bulk-upload-results/#{bulk_upload.id}")
+ end
+ end
+
+ context "when bulk upload was cancelled by moved user" do
+ it "redirects to the error report" do
+ bulk_upload.update!(choice: "cancelled-by-moved-user")
+
+ get "/sales-logs/bulk-upload-resume/#{bulk_upload.id}/start"
+ follow_redirect!
+ expect(response).to redirect_to("/sales-logs/bulk-upload-results/#{bulk_upload.id}")
+ end
+ end
end
describe "GET /sales-logs/bulk-upload-resume/:ID/fix-choice" do
@@ -73,6 +93,25 @@ RSpec.describe BulkUploadSalesResumeController, type: :request do
expect(response).to redirect_to("/sales-logs/bulk-upload-soft-validations-check/#{bulk_upload.id}/chosen")
end
end
+
+ context "when a choice to reupload has been made" do
+ let(:bulk_upload) { create(:bulk_upload, :sales, user:, bulk_upload_errors:, choice: "upload-again") }
+
+ it "redirects to the error report" do
+ get "/sales-logs/bulk-upload-resume/#{bulk_upload.id}/fix-choice"
+ expect(response).to redirect_to("/sales-logs/bulk-upload-results/#{bulk_upload.id}")
+ end
+ end
+
+ context "when bulk upload was cancelled by moved user" do
+ let(:bulk_upload) { create(:bulk_upload, :sales, user:, bulk_upload_errors:, choice: "cancelled-by-moved-user") }
+
+ it "redirects to the error report" do
+ get "/sales-logs/bulk-upload-resume/#{bulk_upload.id}/start"
+ follow_redirect!
+ expect(response).to redirect_to("/sales-logs/bulk-upload-results/#{bulk_upload.id}")
+ end
+ end
end
describe "GET /sales-logs/bulk-upload-resume/:ID/fix-choice?soft_errors_only=true" do
diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb
index dac2806a5..1ab5aa9d4 100644
--- a/spec/requests/users_controller_spec.rb
+++ b/spec/requests/users_controller_spec.rb
@@ -124,6 +124,13 @@ RSpec.describe UsersController, type: :request do
expect(response).to redirect_to("/account/sign-in")
end
end
+
+ describe "#log_reassignment" do
+ it "redirects to the sign in page" do
+ get "/users/#{user.id}/log-reassignment"
+ expect(response).to redirect_to("/account/sign-in")
+ end
+ end
end
context "when user is signed in as a data provider" do
@@ -149,6 +156,7 @@ RSpec.describe UsersController, type: :request do
expect(page).not_to have_link("Change", text: "role")
expect(page).not_to have_link("Change", text: "if data protection officer")
expect(page).not_to have_link("Change", text: "if a key contact")
+ expect(page).not_to have_link("Change", text: "organisation")
end
it "does not allow deactivating the user" do
@@ -208,6 +216,7 @@ RSpec.describe UsersController, type: :request do
expect(page).not_to have_link("Change", text: "role")
expect(page).not_to have_link("Change", text: "if data protection officer")
expect(page).not_to have_link("Change", text: "if a key contact")
+ expect(page).not_to have_link("Change", text: "organisation")
end
it "does not allow deactivating the user" do
@@ -258,6 +267,7 @@ RSpec.describe UsersController, type: :request do
expect(page).not_to have_field("user[role]")
expect(page).not_to have_field("user[is_dpo]")
expect(page).not_to have_field("user[is_key_contact]")
+ expect(page).not_to have_field("user[organisation_id]")
end
end
@@ -430,6 +440,13 @@ RSpec.describe UsersController, type: :request do
expect(result.keys).to match_array([org_user.id.to_s, managing_user.id.to_s])
end
end
+
+ describe "#log_reassignment" do
+ it "returns unauthorized status" do
+ get "/users/#{user.id}/log-reassignment"
+ expect(response).to have_http_status(:unauthorized)
+ end
+ end
end
context "when user is signed in as a data coordinator" do
@@ -607,6 +624,7 @@ RSpec.describe UsersController, type: :request do
expect(page).to have_link("Change", text: "role")
expect(page).to have_link("Change", text: "if data protection officer")
expect(page).to have_link("Change", text: "if a key contact")
+ expect(page).not_to have_link("Change", text: "organisation")
end
it "does not allow deactivating the user" do
@@ -655,6 +673,7 @@ RSpec.describe UsersController, type: :request do
expect(page).to have_link("Change", text: "role")
expect(page).to have_link("Change", text: "if data protection officer")
expect(page).to have_link("Change", text: "if a key contact")
+ expect(page).not_to have_link("Change", text: "organisation")
end
it "allows deactivating the user" do
@@ -713,6 +732,7 @@ RSpec.describe UsersController, type: :request do
expect(page).to have_field("user[name]")
expect(page).to have_field("user[email]")
expect(page).to have_field("user[role]")
+ expect(page).not_to have_field("user[organisation_id]")
end
it "does not allow setting the role to `support`" do
@@ -738,6 +758,7 @@ RSpec.describe UsersController, type: :request do
expect(page).to have_field("user[name]")
expect(page).to have_field("user[email]")
expect(page).to have_field("user[role]")
+ expect(page).not_to have_field("user[organisation_id]")
end
end
@@ -1227,6 +1248,13 @@ RSpec.describe UsersController, type: :request do
expect(result.keys).to match_array([org_user.id.to_s, managing_user.id.to_s])
end
end
+
+ describe "#log_reassignment" do
+ it "returns unauthorised status" do
+ get "/users/#{user.id}/log-reassignment"
+ expect(response).to have_http_status(:unauthorized)
+ end
+ end
end
context "when user is signed in as a support user" do
@@ -1459,6 +1487,7 @@ RSpec.describe UsersController, type: :request do
expect(page).to have_link("Change", text: "role")
expect(page).to have_link("Change", text: "if data protection officer")
expect(page).to have_link("Change", text: "if a key contact")
+ expect(page).to have_link("Change", text: "organisation")
end
it "does not allow deactivating the user" do
@@ -1488,6 +1517,7 @@ RSpec.describe UsersController, type: :request do
expect(page).to have_link("Change", text: "role")
expect(page).to have_link("Change", text: "if data protection officer")
expect(page).to have_link("Change", text: "if a key contact")
+ expect(page).to have_link("Change", text: "organisation")
end
it "links to user organisation" do
@@ -1626,6 +1656,7 @@ RSpec.describe UsersController, type: :request do
expect(page).to have_field("user[role]")
expect(page).to have_field("user[phone]")
expect(page).to have_field("user[phone_extension]")
+ expect(page).to have_field("user[organisation_id]")
end
it "allows setting the role to `support`" do
@@ -1653,6 +1684,7 @@ RSpec.describe UsersController, type: :request do
expect(page).to have_field("user[role]")
expect(page).to have_field("user[phone]")
expect(page).to have_field("user[phone_extension]")
+ expect(page).to have_field("user[organisation_id]")
end
end
@@ -1673,6 +1705,7 @@ RSpec.describe UsersController, type: :request do
expect(page).to have_field("user[role]")
expect(page).to have_field("user[phone]")
expect(page).to have_field("user[phone_extension]")
+ expect(page).to have_field("user[organisation_id]")
end
end
@@ -1682,10 +1715,11 @@ RSpec.describe UsersController, type: :request do
get "/users/#{other_user.id}/edit", headers:, params: {}
end
- it "redirects to user details page" do
- expect(response).to redirect_to("/users/#{other_user.id}")
- follow_redirect!
- expect(page).not_to have_link("Change")
+ it "allows editing the user" do
+ expect(page).to have_field("user[name]")
+ expect(page).to have_field("user[email]")
+ expect(page).to have_field("user[role]")
+ expect(page).to have_field("user[organisation_id]")
end
end
end
@@ -1820,6 +1854,20 @@ RSpec.describe UsersController, type: :request do
patch "/users/#{other_user.id}", headers:, params:
end
end
+
+ context "and email address hasn't changed" do
+ let(:params) { { id: user.id, user: { name: new_name, email: other_user.email, is_dpo: "true", is_key_contact: "true" } } }
+
+ before do
+ user.legacy_users.destroy_all
+ end
+
+ it "shows flash notice" do
+ patch("/users/#{other_user.id}", headers:, params:)
+
+ expect(flash[:notice]).to be_nil
+ end
+ end
end
context "when we update the user password" do
@@ -1838,6 +1886,39 @@ RSpec.describe UsersController, type: :request do
expect(page).to have_selector(".govuk-error-summary__title")
end
end
+
+ context "when updating organisation" do
+ let(:new_organisation) { create(:organisation) }
+
+ before do
+ patch "/users/#{user.id}", headers:, params:
+ end
+
+ context "and organisation id is nil" do
+ let(:params) { { id: user.id, user: { organisation_id: "" } } }
+
+ it "does not update the organisation" do
+ expect(response).to have_http_status(:unprocessable_entity)
+ expect(page).to have_selector(".govuk-error-summary__title")
+ end
+ end
+
+ context "and organisation id is not nil" do
+ let(:params) { { id: user.id, user: { organisation_id: new_organisation.id, name: "new_name" } } }
+
+ it "does not update the organisation" do
+ expect(user.reload.organisation).not_to eq(new_organisation)
+ end
+
+ it "redirects to log reassignment page" do
+ expect(response).to redirect_to("/users/#{user.id}/log-reassignment?organisation_id=#{new_organisation.id}")
+ end
+
+ it "updated other fields" do
+ expect(user.reload.name).to eq("new_name")
+ end
+ end
+ end
end
context "when the current user does not match the user ID" do
@@ -1890,89 +1971,237 @@ RSpec.describe UsersController, type: :request do
end
end
- context "when the current user does not match the user ID" do
- context "when the user is not part of the same organisation as the current user" do
- let(:other_user) { create(:user) }
- let(:params) { { id: other_user.id, user: { name: new_name } } }
+ context "when the user is not part of the same organisation as the current user" do
+ let(:other_user) { create(:user) }
+ let(:params) { { id: other_user.id, user: { name: new_name } } }
- it "updates the user" do
- expect { patch "/users/#{other_user.id}", headers:, params: }
- .to change { other_user.reload.name }.from(other_user.name).to(new_name)
+ it "updates the user" do
+ expect { patch "/users/#{other_user.id}", headers:, params: }
+ .to change { other_user.reload.name }.from(other_user.name).to(new_name)
+ end
+
+ it "tracks who updated the record" do
+ expect { patch "/users/#{other_user.id}", headers:, params: }
+ .to change { other_user.reload.versions.last.actor&.id }.from(nil).to(user.id)
+ end
+
+ context "when user changes email, dpo, key_contact" do
+ let(:params) { { id: other_user.id, user: { name: new_name, email: new_email, is_dpo: "true", is_key_contact: "true" } } }
+
+ it "allows changing email, dpo, key_contact" do
+ patch "/users/#{other_user.id}", headers: headers, params: params
+ other_user.reload
+ expect(other_user.unconfirmed_email).to eq(new_email)
+ expect(other_user.is_data_protection_officer?).to be true
+ expect(other_user.is_key_contact?).to be true
end
+ end
- it "tracks who updated the record" do
- expect { patch "/users/#{other_user.id}", headers:, params: }
- .to change { other_user.reload.versions.last.actor&.id }.from(nil).to(user.id)
+ it "does not bypass sign in for the support user" do
+ patch "/users/#{other_user.id}", headers: headers, params: params
+ follow_redirect!
+ expect(page).to have_content("#{other_user.reload.name}’s account")
+ expect(page).to have_content(other_user.reload.email.to_s)
+ end
+
+ context "when the support user tries to update the user’s password", :aggregate_failures do
+ let(:params) do
+ {
+ id: user.id, user: { password: new_name, password_confirmation: new_name, name: "new name", email: new_email }
+ }
+ end
+
+ let(:personalisation) do
+ {
+ name: params[:user][:name],
+ email: new_email,
+ organisation: other_user.organisation.name,
+ link: include("/account/confirmation?confirmation_token="),
+ }
+ end
+
+ before do
+ other_user.legacy_users.destroy_all
end
- context "when user changes email, dpo, key_contact" do
- let(:params) { { id: other_user.id, user: { name: new_name, email: new_email, is_dpo: "true", is_key_contact: "true" } } }
+ it "shows flash notice" do
+ patch("/users/#{other_user.id}", headers:, params:)
+
+ expect(flash[:notice]).to eq("An email has been sent to #{new_email} to confirm this change.")
+ end
+
+ it "sends new flow emails" do
+ expect(notify_client).to receive(:send_email).with(
+ email_address: other_user.email,
+ template_id: User::FOR_OLD_EMAIL_CHANGED_BY_OTHER_USER_TEMPLATE_ID,
+ personalisation: {
+ new_email:,
+ old_email: other_user.email,
+ },
+ ).once
+
+ expect(notify_client).to receive(:send_email).with(
+ email_address: new_email,
+ template_id: User::FOR_NEW_EMAIL_CHANGED_BY_OTHER_USER_TEMPLATE_ID,
+ personalisation: {
+ new_email:,
+ old_email: other_user.email,
+ link: include("/account/confirmation?confirmation_token="),
+ },
+ ).once
+
+ expect(notify_client).not_to receive(:send_email)
- it "allows changing email, dpo, key_contact" do
- patch "/users/#{other_user.id}", headers: headers, params: params
- other_user.reload
- expect(other_user.unconfirmed_email).to eq(new_email)
- expect(other_user.is_data_protection_officer?).to be true
- expect(other_user.is_key_contact?).to be true
+ patch "/users/#{other_user.id}", headers:, params:
+ end
+ end
+
+ context "when updating organisation" do
+ let(:new_organisation) { create(:organisation) }
+
+ before do
+ patch "/users/#{other_user.id}", headers:, params:
+ end
+
+ context "and organisation id is nil" do
+ let(:params) { { id: other_user.id, user: { organisation_id: "" } } }
+
+ it "does not update the organisation" do
+ expect(response).to have_http_status(:unprocessable_entity)
+ expect(page).to have_selector(".govuk-error-summary__title")
end
end
- it "does not bypass sign in for the support user" do
- patch "/users/#{other_user.id}", headers: headers, params: params
- follow_redirect!
- expect(page).to have_content("#{other_user.reload.name}’s account")
- expect(page).to have_content(other_user.reload.email.to_s)
+ context "and organisation id is not nil" do
+ let(:params) { { id: other_user.id, user: { organisation_id: new_organisation.id, name: "new_name" } } }
+
+ it "does not update the organisation" do
+ expect(user.reload.organisation).not_to eq(new_organisation)
+ end
+
+ it "redirects to log reassignment page" do
+ expect(response).to redirect_to("/users/#{other_user.id}/log-reassignment?organisation_id=#{new_organisation.id}")
+ end
+
+ it "updated other fields" do
+ expect(other_user.reload.name).to eq("new_name")
+ end
end
+ end
+
+ context "when updating log reassignment" do
+ let(:new_organisation) { create(:organisation, name: "New org") }
+ let(:new_organisation_2) { create(:organisation, name: "New org 2") }
+ let(:new_organisation_3) { create(:organisation, name: "New org 3") }
- context "when the support user tries to update the user’s password", :aggregate_failures do
- let(:params) do
- {
- id: user.id, user: { password: new_name, password_confirmation: new_name, name: "new name", email: new_email }
- }
+ context "and log reassignment choice is not present" do
+ let(:params) { { user: { organisation_id: new_organisation.id, log_reassignment: nil } } }
+
+ before do
+ patch "/users/#{other_user.id}/log-reassignment", headers:, params:
end
- let(:personalisation) do
- {
- name: params[:user][:name],
- email: new_email,
- organisation: other_user.organisation.name,
- link: include("/account/confirmation?confirmation_token="),
- }
+ it "does not update the user's organisation" do
+ expect(other_user.reload.organisation).not_to eq(new_organisation)
+ end
+
+ it "displays the error message" do
+ expect(response).to have_http_status(:unprocessable_entity)
+ expect(page).to have_content("Select if you want to reassign logs")
+ end
+ end
+
+ context "and log reassignment choice is to change the stock owner and managing agent" do
+ let(:params) { { user: { organisation_id: new_organisation.id, log_reassignment: "reassign_all" } } }
+
+ before do
+ patch "/users/#{other_user.id}/log-reassignment", headers:, params:
+ end
+
+ it "does not update the user's organisation" do
+ expect(other_user.reload.organisation).not_to eq(new_organisation)
end
+ it "redirects to confirmation page" do
+ expect(response).to redirect_to("/users/#{other_user.id}/organisation-change-confirmation?log_reassignment=reassign_all&organisation_id=#{new_organisation.id}")
+ end
+ end
+
+ context "and log reassignment choice is to unassign logs" do
+ let(:params) { { user: { organisation_id: new_organisation.id, log_reassignment: "unassign" } } }
+
before do
- other_user.legacy_users.destroy_all
+ patch "/users/#{other_user.id}/log-reassignment", headers:, params:
end
- it "shows flash notice" do
- patch("/users/#{other_user.id}", headers:, params:)
+ it "does not update the user's organisation" do
+ expect(other_user.reload.organisation).not_to eq(new_organisation)
+ end
- expect(flash[:notice]).to eq("An email has been sent to #{new_email} to confirm this change.")
+ it "redirects to confirmation page" do
+ expect(response).to redirect_to("/users/#{other_user.id}/organisation-change-confirmation?log_reassignment=unassign&organisation_id=#{new_organisation.id}")
end
+ end
+
+ context "and log reassignment choice is to change stock owner" do
+ let(:params) { { user: { organisation_id: new_organisation.id, log_reassignment: "reassign_stock_owner" } } }
+
+ context "when users organisation manages the logs" do
+ before do
+ create(:lettings_log, managing_organisation: other_user.organisation, assigned_to: other_user)
+ create(:sales_log, managing_organisation: other_user.organisation, assigned_to: other_user)
+ patch "/users/#{other_user.id}/log-reassignment", headers:, params:
+ end
- it "sends new flow emails" do
- expect(notify_client).to receive(:send_email).with(
- email_address: other_user.email,
- template_id: User::FOR_OLD_EMAIL_CHANGED_BY_OTHER_USER_TEMPLATE_ID,
- personalisation: {
- new_email:,
- old_email: other_user.email,
- },
- ).once
-
- expect(notify_client).to receive(:send_email).with(
- email_address: new_email,
- template_id: User::FOR_NEW_EMAIL_CHANGED_BY_OTHER_USER_TEMPLATE_ID,
- personalisation: {
- new_email:,
- old_email: other_user.email,
- link: include("/account/confirmation?confirmation_token="),
- },
- ).once
-
- expect(notify_client).not_to receive(:send_email)
-
- patch "/users/#{other_user.id}", headers:, params:
+ it "required the new org to have stock owner relationship with the current user org" do
+ expect(response).to have_http_status(:unprocessable_entity)
+ expect(page).to have_content("New org must be a stock owner of #{other_user.organisation_name} to make this change.")
+ end
+ end
+
+ context "when different organisations manage the logs" do
+ before do
+ create(:lettings_log, managing_organisation: other_user.organisation, assigned_to: other_user)
+ create(:lettings_log, managing_organisation: new_organisation_2, assigned_to: other_user)
+ create(:sales_log, managing_organisation: new_organisation_3, assigned_to: other_user)
+ patch "/users/#{other_user.id}/log-reassignment", headers:, params:
+ end
+
+ it "required the new org to have stock owner relationship with the managing organisations" do
+ expect(response).to have_http_status(:unprocessable_entity)
+ expect(page).to have_content("New org must be a stock owner of #{other_user.organisation_name}, #{new_organisation_2.name}, and #{new_organisation_3.name} to make this change.")
+ end
+ end
+ end
+
+ context "and log reassignment choice is to change managing agent" do
+ let(:params) { { user: { organisation_id: new_organisation.id, log_reassignment: "reassign_managing_agent" } } }
+
+ context "when users organisation manages the logs" do
+ before do
+ create(:lettings_log, owning_organisation: other_user.organisation, assigned_to: other_user)
+ create(:sales_log, owning_organisation: other_user.organisation, assigned_to: other_user)
+ patch "/users/#{other_user.id}/log-reassignment", headers:, params:
+ end
+
+ it "required the new org to have managing agent relationship with the current user org" do
+ expect(response).to have_http_status(:unprocessable_entity)
+ expect(page).to have_content("New org must be a managing agent of #{other_user.organisation_name} to make this change.")
+ end
+ end
+
+ context "when different organisations manage the logs" do
+ before do
+ create(:lettings_log, owning_organisation: other_user.organisation, assigned_to: other_user)
+ create(:lettings_log, owning_organisation: new_organisation_2, assigned_to: other_user)
+ create(:sales_log, owning_organisation: new_organisation_3, managing_organisation: other_user.organisation, assigned_to: other_user)
+ patch "/users/#{other_user.id}/log-reassignment", headers:, params:
+ end
+
+ it "required the new org to have managing agent relationship with owning organisations" do
+ expect(response).to have_http_status(:unprocessable_entity)
+ expect(page).to have_content("New org must be a managing agent of #{other_user.organisation_name}, #{new_organisation_2.name}, and #{new_organisation_3.name} to make this change.")
+ end
end
end
end
@@ -2194,6 +2423,151 @@ RSpec.describe UsersController, type: :request do
expect(result.keys).to match_array([org_user.id.to_s, managing_user.id.to_s, owner_user.id.to_s, other_user.id.to_s])
end
end
+
+ describe "#log_reassignment" do
+ context "when organisation id is not given" do
+ before do
+ create(:lettings_log, assigned_to: other_user)
+ end
+
+ it "redirects to the user page" do
+ get "/users/#{other_user.id}/log-reassignment"
+ expect(response).to redirect_to("/users/#{other_user.id}")
+ end
+ end
+
+ context "when organisation id does not exist" do
+ before do
+ create(:lettings_log, assigned_to: other_user)
+ end
+
+ it "redirects to the user page" do
+ get "/users/#{other_user.id}/log-reassignment?organisation_id=123123"
+ expect(response).to redirect_to("/users/#{other_user.id}")
+ end
+ end
+
+ context "with valid organisation id" do
+ let(:new_organisation) { create(:organisation, name: "new org") }
+
+ context "and user has assigned logs" do
+ before do
+ create(:lettings_log, assigned_to: other_user)
+ end
+
+ it "allows reassigning logs" do
+ get "/users/#{other_user.id}/log-reassignment?organisation_id=#{new_organisation.id}"
+ expect(page).to have_content("Should this user’s logs move to their new organisation?")
+ expect(page).to have_content("You’re moving #{other_user.name} from #{other_user.organisation_name} to new org. There is 1 log assigned to them.")
+ expect(page).to have_button("Continue")
+ expect(page).to have_link("Back", href: "/users/#{other_user.id}/edit")
+ expect(page).to have_link("Cancel", href: "/users/#{other_user.id}/edit")
+ end
+ end
+
+ context "and user has no assigned logs" do
+ it "redirects to confirm organisation change page" do
+ get "/users/#{other_user.id}/log-reassignment?organisation_id=#{new_organisation.id}"
+ expect(response).to redirect_to("/users/#{other_user.id}/organisation-change-confirmation?organisation_id=#{new_organisation.id}")
+ end
+ end
+ end
+ end
+
+ describe "#confirm_organisation_change" do
+ context "when organisation id is not given" do
+ it "redirects to the user page" do
+ get "/users/#{other_user.id}/organisation-change-confirmation?log_reassignment=reassign_all"
+ expect(response).to redirect_to("/users/#{other_user.id}")
+ end
+ end
+
+ context "when reassignment option is not given" do
+ let(:new_organisation) { create(:organisation, name: "new org") }
+
+ before do
+ create(:lettings_log, assigned_to: other_user)
+ end
+
+ it "redirects to the user page" do
+ get "/users/#{other_user.id}/organisation-change-confirmation?organisation_id=#{new_organisation.id}"
+ expect(response).to redirect_to("/users/#{other_user.id}")
+ end
+ end
+
+ context "when organisation id does not exist" do
+ it "redirects to the user page" do
+ get "/users/#{other_user.id}/organisation-change-confirmation?organisation_id=123123&log_reassignment=reassign_all"
+ expect(response).to redirect_to("/users/#{other_user.id}")
+ end
+ end
+
+ context "with valid organisation id" do
+ let(:new_organisation) { create(:organisation, name: "new org") }
+
+ before do
+ create(:lettings_log, assigned_to: other_user)
+ end
+
+ it "displays confirm organisation change page" do
+ get "/users/#{other_user.id}/organisation-change-confirmation?organisation_id=#{new_organisation.id}&log_reassignment=reassign_all"
+ expect(page).to have_content("Are you sure you want to move this user?")
+ expect(page).to have_content("You’re moving #{other_user.name} from #{other_user.organisation_name} to #{new_organisation.name}. The stock owner and managing agent on their logs will change to #{new_organisation.name}.")
+ end
+ end
+ end
+
+ describe "#confirm_organisation_change patch" do
+ context "when organisation id is not given" do
+ let(:params) { { user: { organisation_id: nil, log_reassignment: "reassign_all" } } }
+
+ it "redirects to the user page" do
+ patch "/users/#{other_user.id}/organisation-change-confirmation", headers:, params: params
+ expect(response).to redirect_to("/users/#{other_user.id}")
+ end
+ end
+
+ context "when reassignment option is not given" do
+ let(:new_organisation) { create(:organisation, name: "new org") }
+ let(:params) { { user: { organisation_id: new_organisation.id, log_reassignment: "reassign_all" } } }
+
+ it "redirects to the user page" do
+ patch "/users/#{other_user.id}/organisation-change-confirmation", headers:, params: params
+ expect(response).to redirect_to("/users/#{other_user.id}")
+ end
+ end
+
+ context "when organisation id does not exist" do
+ let(:params) { { user: { organisation_id: 123_123, log_reassignment: "reassign_all" } } }
+
+ it "redirects to the user page" do
+ patch "/users/#{other_user.id}/organisation-change-confirmation", headers:, params: params
+ expect(response).to redirect_to("/users/#{other_user.id}")
+ end
+ end
+
+ context "with valid organisation id" do
+ let(:new_organisation) { create(:organisation, name: "new org") }
+ let(:params) { { user: { organisation_id: new_organisation.id, log_reassignment: "reassign_all" } } }
+ let!(:lettings_log) { create(:lettings_log, assigned_to: other_user) }
+ let!(:sales_log) { create(:sales_log, assigned_to: other_user) }
+
+ context "and reassign all option" do
+ it "updates logs and moves the user" do
+ patch "/users/#{other_user.id}/organisation-change-confirmation", headers:, params: params
+ expect(response).to redirect_to("/users/#{other_user.id}")
+
+ expect(other_user.reload.organisation).to eq(new_organisation)
+ expect(other_user.lettings_logs.count).to eq(1)
+ expect(other_user.sales_logs.count).to eq(1)
+ expect(lettings_log.reload.managing_organisation).to eq(new_organisation)
+ expect(lettings_log.owning_organisation).to eq(new_organisation)
+ expect(sales_log.reload.managing_organisation).to eq(new_organisation)
+ expect(sales_log.owning_organisation).to eq(new_organisation)
+ end
+ end
+ end
+ end
end
describe "title link" do
diff --git a/spec/services/bulk_upload/lettings/year2024/csv_parser_spec.rb b/spec/services/bulk_upload/lettings/year2024/csv_parser_spec.rb
index 6db8e1806..d0e5b3692 100644
--- a/spec/services/bulk_upload/lettings/year2024/csv_parser_spec.rb
+++ b/spec/services/bulk_upload/lettings/year2024/csv_parser_spec.rb
@@ -41,6 +41,7 @@ RSpec.describe BulkUpload::Lettings::Year2024::CsvParser do
file.write("Duplicate check field?\n")
file.write(BulkUpload::LettingsLogToCsv.new(log:).default_2024_field_numbers_row)
file.write(BulkUpload::LettingsLogToCsv.new(log:).to_2024_csv_row)
+ file.write("\n")
file.rewind
end
@@ -52,6 +53,10 @@ RSpec.describe BulkUpload::Lettings::Year2024::CsvParser do
it "parses csv correctly" do
expect(service.row_parsers[0].field_13).to eql(log.tenancycode)
end
+
+ it "does not parse the last empty row" do
+ expect(service.row_parsers.count).to eq(1)
+ end
end
context "when parsing csv with headers in arbitrary order" do
diff --git a/spec/services/bulk_upload/lettings/year2024/row_parser_spec.rb b/spec/services/bulk_upload/lettings/year2024/row_parser_spec.rb
index 6f23905d1..68de5d22b 100644
--- a/spec/services/bulk_upload/lettings/year2024/row_parser_spec.rb
+++ b/spec/services/bulk_upload/lettings/year2024/row_parser_spec.rb
@@ -1897,6 +1897,18 @@ RSpec.describe BulkUpload::Lettings::Year2024::RowParser do
end
end
end
+
+ describe "log_already_exists?" do
+ let(:attributes) { { bulk_upload: } }
+
+ before do
+ build(:lettings_log, owning_organisation: nil, startdate: nil, tenancycode: nil, location: nil, age1: nil, sex1: nil, ecstat1: nil, brent: nil, scharge: nil, pscharge: nil, supcharg: nil).save(validate: false)
+ end
+
+ it "does not add duplicate logs validation to the blank row" do
+ expect(parser.log_already_exists?).to eq(false)
+ end
+ end
end
describe "#log" do
diff --git a/spec/services/bulk_upload/sales/year2024/csv_parser_spec.rb b/spec/services/bulk_upload/sales/year2024/csv_parser_spec.rb
index ef90bd834..9440b7e8c 100644
--- a/spec/services/bulk_upload/sales/year2024/csv_parser_spec.rb
+++ b/spec/services/bulk_upload/sales/year2024/csv_parser_spec.rb
@@ -17,6 +17,7 @@ RSpec.describe BulkUpload::Sales::Year2024::CsvParser do
file.write("Duplicate check field?\n")
file.write(BulkUpload::SalesLogToCsv.new(log:).default_2024_field_numbers_row)
file.write(BulkUpload::SalesLogToCsv.new(log:).to_2024_csv_row)
+ file.write("\n")
file.rewind
end
@@ -32,6 +33,10 @@ RSpec.describe BulkUpload::Sales::Year2024::CsvParser do
it "counts the number of valid field numbers correctly" do
expect(service).to be_correct_field_count
end
+
+ it "does not parse the last empty row" do
+ expect(service.row_parsers.count).to eq(1)
+ end
end
context "when parsing csv with headers in arbitrary order" do
diff --git a/spec/services/bulk_upload/sales/year2024/row_parser_spec.rb b/spec/services/bulk_upload/sales/year2024/row_parser_spec.rb
index 5dfcd2429..ca8f29e92 100644
--- a/spec/services/bulk_upload/sales/year2024/row_parser_spec.rb
+++ b/spec/services/bulk_upload/sales/year2024/row_parser_spec.rb
@@ -1471,6 +1471,18 @@ RSpec.describe BulkUpload::Sales::Year2024::RowParser do
end
end
end
+
+ describe "log_already_exists?" do
+ let(:attributes) { { bulk_upload: } }
+
+ before do
+ build(:sales_log, owning_organisation: nil, saledate: nil, purchid: nil, age1: nil, sex1: nil, ecstat1: nil).save(validate: false)
+ end
+
+ it "does not add duplicate logs validation to the blank row" do
+ expect(parser.log_already_exists?).to eq(false)
+ end
+ end
end
describe "#log" do