Browse Source

CLDC-3404 Allow support to update user organisation (#2596)

* Allow support to change user organisation

* Redirect to log-reassignment page when updating organisation

* Add log reassignment page

* Allow setting log reassignment choice

* Add organisation change confirmation page

* Update logs based on log reassignment selection

* Skip log reassignment question if user has no logs

* Send organisation update email

* Update routes in accessibility tests

* Allo moving deactivated users

* Move org field above role

* Only display banner if email has changed

* rescue StandardError

* Only reassign visible logs

* Clean user error messages

* Update pluralization in email

* Mark related bulk uploads as cancelled by user

* Add moved user banner

* Update routing for fix-choice page

* Update pluralization again
pull/2613/head^2 v0.4.72
kosiakkatrina 4 months ago committed by GitHub
parent
commit
15e4f79b83
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
  1. 120
      app/controllers/users_controller.rb
  2. 42
      app/helpers/user_helper.rb
  3. 4
      app/models/bulk_upload.rb
  4. 4
      app/models/forms/bulk_upload_lettings_resume/fix_choice.rb
  5. 4
      app/models/forms/bulk_upload_sales_resume/fix_choice.rb
  6. 118
      app/models/user.rb
  7. 24
      app/policies/user_policy.rb
  8. 2
      app/views/bulk_upload_lettings_results/show.html.erb
  9. 2
      app/views/bulk_upload_lettings_results/summary.html.erb
  10. 2
      app/views/bulk_upload_sales_results/show.html.erb
  11. 2
      app/views/bulk_upload_sales_results/summary.html.erb
  12. 12
      app/views/bulk_upload_shared/_moved_user_banner.html.erb
  13. 19
      app/views/users/edit.html.erb
  14. 36
      app/views/users/log_reassignment.html.erb
  15. 24
      app/views/users/organisation_change_confirmation.html.erb
  16. 10
      app/views/users/show.html.erb
  17. 4
      config/locales/en.yml
  18. 4
      config/routes.rb
  19. 5
      db/migrate/20240911152702_add_moved_user_to_bu.rb
  20. 3
      db/schema.rb
  21. 6
      spec/features/accessibility_spec.rb
  22. 86
      spec/helpers/user_helper_spec.rb
  23. 300
      spec/models/user_spec.rb
  24. 14
      spec/policies/user_policy_spec.rb
  25. 45
      spec/requests/bulk_upload_lettings_results_controller_spec.rb
  26. 39
      spec/requests/bulk_upload_lettings_resume_controller_spec.rb
  27. 59
      spec/requests/bulk_upload_sales_results_controller_spec.rb
  28. 39
      spec/requests/bulk_upload_sales_resume_controller_spec.rb
  29. 506
      spec/requests/users_controller_spec.rb

120
app/controllers/users_controller.rb

@ -56,20 +56,24 @@ class UsersController < ApplicationController
def key_contact; end def key_contact; end
def edit def edit
redirect_to user_path(@user) unless @user.active? redirect_to user_path(@user) unless @user.active? || current_user.support?
end end
def update def update
validate_attributes validate_attributes
if @user.errors.empty? && @user.update(user_params) if @user.errors.empty? && @user.update(user_params_without_org)
if @user == current_user if @user == current_user
bypass_sign_in @user bypass_sign_in @user
flash[:notice] = I18n.t("devise.passwords.updated") if user_params.key?("password") flash[:notice] = I18n.t("devise.passwords.updated") if user_params.key?("password")
if user_params.key?("email") if user_params.key?("email") && user_params[:email] != @user.email
flash[:notice] = I18n.t("devise.email.updated", email: @user.unconfirmed_email) flash[:notice] = I18n.t("devise.email.updated", email: @user.unconfirmed_email)
end end
redirect_to account_path if updating_organisation?
redirect_to user_log_reassignment_path(@user, organisation_id: user_params[:organisation_id])
else
redirect_to account_path
end
else else
user_name = @user.name&.possessive || @user.email.possessive user_name = @user.name&.possessive || @user.email.possessive
if user_params[:active] == "false" if user_params[:active] == "false"
@ -79,10 +83,15 @@ class UsersController < ApplicationController
@user.reactivate! @user.reactivate!
@user.send_confirmation_instructions @user.send_confirmation_instructions
flash[:notice] = I18n.t("devise.activation.reactivated", user_name:) flash[:notice] = I18n.t("devise.activation.reactivated", user_name:)
elsif user_params.key?("email") elsif user_params.key?("email") && user_params[:email] != @user.email
flash[:notice] = I18n.t("devise.email.updated", email: @user.unconfirmed_email) flash[:notice] = I18n.t("devise.email.updated", email: @user.unconfirmed_email)
end end
redirect_to user_path(@user)
if updating_organisation?
redirect_to user_log_reassignment_path(@user, organisation_id: user_params[:organisation_id])
else
redirect_to user_path(@user)
end
end end
elsif user_params.key?("password") elsif user_params.key?("password")
format_error_messages format_error_messages
@ -144,6 +153,58 @@ class UsersController < ApplicationController
redirect_to users_organisation_path(@user.organisation), notice: I18n.t("notification.user_deleted", name: @user.name) redirect_to users_organisation_path(@user.organisation), notice: I18n.t("notification.user_deleted", name: @user.name)
end end
def log_reassignment
authorize @user
assigned_to_logs_count = @user.assigned_to_lettings_logs.visible.count + @user.assigned_to_sales_logs.visible.count
return redirect_to user_organisation_change_confirmation_path(@user, organisation_id: params[:organisation_id]) if assigned_to_logs_count.zero?
if params[:organisation_id].present? && Organisation.where(id: params[:organisation_id]).exists?
@new_organisation = Organisation.find(params[:organisation_id])
else
redirect_to user_path(@user)
end
end
def update_log_reassignment
authorize @user
return redirect_to user_path(@user) unless log_reassignment_params[:organisation_id].present? && Organisation.where(id: log_reassignment_params[:organisation_id]).exists?
@new_organisation = Organisation.find(log_reassignment_params[:organisation_id])
validate_log_reassignment
if @user.errors.empty?
redirect_to user_organisation_change_confirmation_path(@user, log_reassignment_params)
else
render :log_reassignment, status: :unprocessable_entity
end
end
def organisation_change_confirmation
authorize @user
assigned_to_logs_count = @user.assigned_to_lettings_logs.visible.count + @user.assigned_to_sales_logs.visible.count
return redirect_to user_path(@user) if params[:organisation_id].blank? || !Organisation.where(id: params[:organisation_id]).exists?
return redirect_to user_path(@user) if params[:log_reassignment].blank? && assigned_to_logs_count.positive?
@new_organisation = Organisation.find(params[:organisation_id])
@log_reassignment = params[:log_reassignment]
end
def confirm_organisation_change
authorize @user
assigned_to_logs_count = @user.assigned_to_lettings_logs.visible.count + @user.assigned_to_sales_logs.visible.count
return redirect_to user_path(@user) if log_reassignment_params[:organisation_id].blank? || !Organisation.where(id: log_reassignment_params[:organisation_id]).exists?
return redirect_to user_path(@user) if log_reassignment_params[:log_reassignment].blank? && assigned_to_logs_count.positive?
@new_organisation = Organisation.find(log_reassignment_params[:organisation_id])
@log_reassignment = log_reassignment_params[:log_reassignment]
@user.reassign_logs_and_update_organisation(@new_organisation, @log_reassignment)
redirect_to user_path(@user)
end
private private
def validate_attributes def validate_attributes
@ -157,6 +218,10 @@ private
elsif !user_params[:phone].nil? && !valid_phone_number?(user_params[:phone]) elsif !user_params[:phone].nil? && !valid_phone_number?(user_params[:phone])
@user.errors.add :phone @user.errors.add :phone
end end
if user_params.key?(:organisation_id) && user_params[:organisation_id].blank?
@user.errors.add :organisation_id, :blank
end
end end
def valid_phone_number?(number) def valid_phone_number?(number)
@ -191,8 +256,10 @@ private
def user_params def user_params
if @user == current_user if @user == current_user
if current_user.data_coordinator? || current_user.support? if current_user.data_coordinator?
params.require(:user).permit(:email, :phone, :phone_extension, :name, :password, :password_confirmation, :role, :is_dpo, :is_key_contact, :initial_confirmation_sent) params.require(:user).permit(:email, :phone, :phone_extension, :name, :password, :password_confirmation, :role, :is_dpo, :is_key_contact, :initial_confirmation_sent)
elsif current_user.support?
params.require(:user).permit(:email, :phone, :phone_extension, :name, :password, :password_confirmation, :role, :is_dpo, :is_key_contact, :initial_confirmation_sent, :organisation_id)
else else
params.require(:user).permit(:email, :phone, :phone_extension, :name, :password, :password_confirmation, :initial_confirmation_sent) params.require(:user).permit(:email, :phone, :phone_extension, :name, :password, :password_confirmation, :initial_confirmation_sent)
end end
@ -203,6 +270,14 @@ private
end end
end end
def user_params_without_org
user_params.except(:organisation_id)
end
def log_reassignment_params
params.require(:user).permit(:log_reassignment, :organisation_id)
end
def created_user_redirect_path def created_user_redirect_path
if current_user.support? if current_user.support?
users_path users_path
@ -234,4 +309,35 @@ private
def session_filters def session_filters
filter_manager.session_filters filter_manager.session_filters
end end
def updating_organisation?
user_params["organisation_id"].present? && @user.organisation_id != user_params["organisation_id"].to_i
end
def validate_log_reassignment
return @user.errors.add :log_reassignment, :blank if log_reassignment_params[:log_reassignment].blank?
case log_reassignment_params[:log_reassignment]
when "reassign_stock_owner"
required_managing_agents = (@user.assigned_to_lettings_logs.visible.map(&:managing_organisation) + @user.assigned_to_sales_logs.visible.map(&:managing_organisation)).uniq
current_managing_agents = @new_organisation.managing_agents
missing_managing_agents = required_managing_agents - current_managing_agents
if missing_managing_agents.any?
new_organisation = @new_organisation.name
missing_managing_agents = missing_managing_agents.map(&:name).sort.to_sentence
@user.errors.add :log_reassignment, I18n.t("activerecord.errors.models.user.attributes.log_reassignment.missing_managing_agents", new_organisation:, missing_managing_agents:)
end
when "reassign_managing_agent"
required_stock_owners = (@user.assigned_to_lettings_logs.visible.map(&:owning_organisation) + @user.assigned_to_sales_logs.visible.map(&:owning_organisation)).uniq
current_stock_owners = @new_organisation.stock_owners
missing_stock_owners = required_stock_owners - current_stock_owners
if missing_stock_owners.any?
new_organisation = @new_organisation.name
missing_stock_owners = missing_stock_owners.map(&:name).sort.to_sentence
@user.errors.add :log_reassignment, I18n.t("activerecord.errors.models.user.attributes.log_reassignment.missing_stock_owners", new_organisation:, missing_stock_owners:)
end
end
end
end end

42
app/helpers/user_helper.rb

@ -14,4 +14,46 @@ module UserHelper
def delete_user_link(user) def delete_user_link(user)
govuk_button_link_to "Delete this user", delete_confirmation_user_path(user), warning: true govuk_button_link_to "Delete this user", delete_confirmation_user_path(user), warning: true
end end
def organisation_change_warning(user, new_organisation)
logs_count = user.assigned_to_lettings_logs.count + user.assigned_to_sales_logs.count
logs_count_text = logs_count == 1 ? "is #{logs_count} log" : "are #{logs_count} logs"
"You’re moving #{user.name} from #{user.organisation.name} to #{new_organisation.name}. There #{logs_count_text} assigned to them."
end
def organisation_change_confirmation_warning(user, new_organisation, log_reassignment)
log_reassignment_text = "There are no logs assigned to them."
logs_count = user.assigned_to_lettings_logs.count + user.assigned_to_sales_logs.count
if logs_count.positive?
case log_reassignment
when "reassign_all"
log_reassignment_text = "The stock owner and managing agent on their logs will change to #{new_organisation.name}."
when "reassign_stock_owner"
log_reassignment_text = "The stock owner on their logs will change to #{new_organisation.name}."
when "reassign_managing_agent"
log_reassignment_text = "The managing agent on their logs will change to #{new_organisation.name}."
when "unassign"
log_reassignment_text = "Their logs will be unassigned."
end
end
"You’re moving #{user.name} from #{user.organisation.name} to #{new_organisation.name}. #{log_reassignment_text}"
end
def remove_attributes_from_error_messages(user)
modified_errors = []
user.errors.each do |error|
cleaned_message = error.type.gsub(error.attribute.to_s.humanize, "").strip
modified_errors << [error.attribute, cleaned_message]
end
user.errors.clear
modified_errors.each do |attribute, message|
user.errors.add(attribute, message)
end
end
end end

4
app/models/bulk_upload.rb

@ -101,6 +101,10 @@ class BulkUpload < ApplicationRecord
logs.filter_by_status("in_progress").map(&:missing_answers_count).sum(0) logs.filter_by_status("in_progress").map(&:missing_answers_count).sum(0)
end end
def moved_user_name
User.find_by(id: moved_user_id)&.name
end
private private
def generate_identifier def generate_identifier

4
app/models/forms/bulk_upload_lettings_resume/fix_choice.rb

@ -56,7 +56,7 @@ module Forms
end end
def preflight_valid? def preflight_valid?
bulk_upload.choice != "create-fix-inline" && bulk_upload.choice != "bulk-confirm-soft-validations" bulk_upload.choice.blank?
end end
def preflight_redirect def preflight_redirect
@ -65,6 +65,8 @@ module Forms
page_bulk_upload_lettings_resume_path(bulk_upload, :chosen) page_bulk_upload_lettings_resume_path(bulk_upload, :chosen)
when "bulk-confirm-soft-validations" when "bulk-confirm-soft-validations"
page_bulk_upload_lettings_soft_validations_check_path(bulk_upload, :chosen) page_bulk_upload_lettings_soft_validations_check_path(bulk_upload, :chosen)
else
bulk_upload_lettings_result_path(bulk_upload)
end end
end end
end end

4
app/models/forms/bulk_upload_sales_resume/fix_choice.rb

@ -56,7 +56,7 @@ module Forms
end end
def preflight_valid? def preflight_valid?
bulk_upload.choice != "create-fix-inline" && bulk_upload.choice != "bulk-confirm-soft-validations" bulk_upload.choice.blank?
end end
def preflight_redirect def preflight_redirect
@ -65,6 +65,8 @@ module Forms
page_bulk_upload_sales_resume_path(bulk_upload, :chosen) page_bulk_upload_sales_resume_path(bulk_upload, :chosen)
when "bulk-confirm-soft-validations" when "bulk-confirm-soft-validations"
page_bulk_upload_sales_soft_validations_check_path(bulk_upload, :chosen) page_bulk_upload_sales_soft_validations_check_path(bulk_upload, :chosen)
else
bulk_upload_sales_result_path(bulk_upload)
end end
end end
end end

118
app/models/user.rb

@ -49,6 +49,13 @@ class User < ApplicationRecord
support: 99, support: 99,
}.freeze }.freeze
LOG_REASSIGNMENT = {
reassign_all: "Yes, change the stock owner and the managing agent",
reassign_stock_owner: "Yes, change the stock owner but keep the managing agent the same",
reassign_managing_agent: "Yes, change the managing agent but keep the stock owner the same",
unassign: "No, unassign the logs",
}.freeze
enum role: ROLES enum role: ROLES
scope :search_by_name, ->(name) { where("users.name ILIKE ?", "%#{name}%") } scope :search_by_name, ->(name) { where("users.name ILIKE ?", "%#{name}%") }
@ -80,6 +87,8 @@ class User < ApplicationRecord
scope :visible, -> { where(discarded_at: nil) } scope :visible, -> { where(discarded_at: nil) }
scope :own_and_managing_org_users, ->(organisation) { where(organisation: organisation.child_organisations + [organisation]) } scope :own_and_managing_org_users, ->(organisation) { where(organisation: organisation.child_organisations + [organisation]) }
attr_accessor :log_reassignment
def lettings_logs def lettings_logs
if support? if support?
LettingsLog.all LettingsLog.all
@ -161,6 +170,7 @@ class User < ApplicationRecord
USER_REACTIVATED_TEMPLATE_ID = "ac45a899-490e-4f59-ae8d-1256fc0001f9".freeze USER_REACTIVATED_TEMPLATE_ID = "ac45a899-490e-4f59-ae8d-1256fc0001f9".freeze
FOR_OLD_EMAIL_CHANGED_BY_OTHER_USER_TEMPLATE_ID = "3eb80517-1051-4dfc-b4cc-cb18228a3829".freeze FOR_OLD_EMAIL_CHANGED_BY_OTHER_USER_TEMPLATE_ID = "3eb80517-1051-4dfc-b4cc-cb18228a3829".freeze
FOR_NEW_EMAIL_CHANGED_BY_OTHER_USER_TEMPLATE_ID = "0cdd0be1-7fa5-4808-8225-ae4c5a002352".freeze FOR_NEW_EMAIL_CHANGED_BY_OTHER_USER_TEMPLATE_ID = "0cdd0be1-7fa5-4808-8225-ae4c5a002352".freeze
ORGANISATION_UPDATE_TEMPLATE_ID = "4b7716c0-cc5c-41dd-92e4-a0dff03bdf5e".freeze
def reset_password_notify_template def reset_password_notify_template
RESET_PASSWORD_TEMPLATE_ID RESET_PASSWORD_TEMPLATE_ID
@ -270,6 +280,48 @@ class User < ApplicationRecord
"#{phone}, Ext. #{phone_extension}" "#{phone}, Ext. #{phone_extension}"
end end
def assigned_to_lettings_logs
lettings_logs.where(assigned_to: self)
end
def assigned_to_sales_logs
sales_logs.where(assigned_to: self)
end
def reassign_logs_and_update_organisation(new_organisation, log_reassignment)
return unless new_organisation
ActiveRecord::Base.transaction do
lettings_logs_to_reassign = assigned_to_lettings_logs.visible
sales_logs_to_reassign = assigned_to_sales_logs.visible
current_organisation = organisation
logs_count = lettings_logs_to_reassign.count + sales_logs_to_reassign.count
return if logs_count.positive? && (log_reassignment.blank? || !LOG_REASSIGNMENT.key?(log_reassignment.to_sym))
update!(organisation: new_organisation)
case log_reassignment
when "reassign_all"
reassign_all_orgs(new_organisation, lettings_logs_to_reassign, sales_logs_to_reassign)
when "reassign_stock_owner"
reassign_stock_owners(new_organisation, lettings_logs_to_reassign, sales_logs_to_reassign)
when "reassign_managing_agent"
reassign_managing_agents(new_organisation, lettings_logs_to_reassign, sales_logs_to_reassign)
when "unassign"
unassign_organisations(lettings_logs_to_reassign, sales_logs_to_reassign, current_organisation)
end
cancel_related_bulk_uploads
send_organisation_change_email(current_organisation, new_organisation, log_reassignment, logs_count)
rescue StandardError => e
Rails.logger.error("User update failed with: #{e.message}")
Sentry.capture_exception(e)
raise ActiveRecord::Rollback
end
end
protected protected
# Checks whether a password is needed or not. For validations only. # Checks whether a password is needed or not. For validations only.
@ -294,4 +346,70 @@ private
DataProtectionConfirmationMailer.send_confirmation_email(self).deliver_later DataProtectionConfirmationMailer.send_confirmation_email(self).deliver_later
end end
def reassign_all_orgs(new_organisation, lettings_logs_to_reassign, sales_logs_to_reassign)
lettings_logs_to_reassign.update_all(owning_organisation_id: new_organisation.id, managing_organisation_id: new_organisation.id, values_updated_at: Time.zone.now)
sales_logs_to_reassign.update_all(owning_organisation_id: new_organisation.id, managing_organisation_id: new_organisation.id, values_updated_at: Time.zone.now)
end
def reassign_stock_owners(new_organisation, lettings_logs_to_reassign, sales_logs_to_reassign)
lettings_logs_to_reassign.update_all(owning_organisation_id: new_organisation.id, values_updated_at: Time.zone.now)
sales_logs_to_reassign.update_all(owning_organisation_id: new_organisation.id, values_updated_at: Time.zone.now)
end
def reassign_managing_agents(new_organisation, lettings_logs_to_reassign, sales_logs_to_reassign)
lettings_logs_to_reassign.update_all(managing_organisation_id: new_organisation.id, values_updated_at: Time.zone.now)
sales_logs_to_reassign.update_all(managing_organisation_id: new_organisation.id, values_updated_at: Time.zone.now)
end
def unassign_organisations(lettings_logs_to_reassign, sales_logs_to_reassign, current_organisation)
if User.find_by(name: "Unassigned", organisation: current_organisation)
unassigned_user = User.find_by(name: "Unassigned", organisation: current_organisation)
else
unassigned_user = User.new(
name: "Unassigned",
organisation_id:,
is_dpo: false,
encrypted_password: SecureRandom.hex(10),
email: SecureRandom.uuid,
confirmed_at: Time.zone.now,
active: false,
)
unassigned_user.save!(validate: false)
end
lettings_logs_to_reassign.update_all(assigned_to_id: unassigned_user.id, values_updated_at: Time.zone.now)
sales_logs_to_reassign.update_all(assigned_to_id: unassigned_user.id, values_updated_at: Time.zone.now)
end
def send_organisation_change_email(current_organisation, new_organisation, log_reassignment, logs_count)
reassigned_logs_text = ""
assigned_logs_count = logs_count == 1 ? "is 1 log" : "are #{logs_count} logs"
case log_reassignment
when "reassign_all"
reassigned_logs_text = "There #{assigned_logs_count} assigned to you. The stock owner and managing agent on #{logs_count == 1 ? 'this log' : 'these logs'} has been changed from #{current_organisation.name} to #{new_organisation.name}."
when "reassign_stock_owner"
reassigned_logs_text = "There #{assigned_logs_count} assigned to you. The stock owner on #{logs_count == 1 ? 'this log' : 'these logs'} has been changed from #{current_organisation.name} to #{new_organisation.name}."
when "reassign_managing_agent"
reassigned_logs_text = "There #{assigned_logs_count} assigned to you. The managing agent on #{logs_count == 1 ? 'this log' : 'these logs'} has been changed from #{current_organisation.name} to #{new_organisation.name}."
when "unassign"
reassigned_logs_text = "There #{assigned_logs_count} assigned to you. #{logs_count == 1 ? 'This' : 'These'} have now been unassigned."
end
template_id = ORGANISATION_UPDATE_TEMPLATE_ID
personalisation = {
from_organisation: "#{current_organisation.name} (Organisation ID: #{current_organisation.id})",
to_organisation: "#{new_organisation.name} (Organisation ID: #{new_organisation.id})",
reassigned_logs_text:,
}
DeviseNotifyMailer.new.send_email(email, template_id, personalisation)
end
def cancel_related_bulk_uploads
lettings_bu_ids = LettingsLog.where(assigned_to: self, status: "pending").map(&:bulk_upload_id).compact.uniq
BulkUpload.where(id: lettings_bu_ids).update!(choice: "cancelled-by-moved-user", moved_user_id: id)
sales_bu_ids = SalesLog.where(assigned_to: self, status: "pending").map(&:bulk_upload_id).compact.uniq
BulkUpload.where(id: sales_bu_ids).update!(choice: "cancelled-by-moved-user", moved_user_id: id)
end
end end

24
app/policies/user_policy.rb

@ -10,17 +10,15 @@ class UserPolicy
@current_user == @user @current_user == @user
end end
def edit_roles?
(@current_user.data_coordinator? || @current_user.support?) && @user.active?
end
%w[ %w[
edit_roles? edit_roles?
edit_dpo? edit_dpo?
edit_key_contact? edit_key_contact?
].each do |method_name| ].each do |method_name|
define_method method_name do define_method method_name do
(@current_user.data_coordinator? || @current_user.support?) && @user.active? return true if @current_user.support?
@current_user.data_coordinator? && @user.active?
end end
end end
@ -30,7 +28,9 @@ class UserPolicy
edit_names? edit_names?
].each do |method_name| ].each do |method_name|
define_method method_name do define_method method_name do
(@current_user == @user || @current_user.data_coordinator? || @current_user.support?) && @user.active? return true if @current_user.support?
(@current_user == @user || @current_user.data_coordinator?) && @user.active?
end end
end end
@ -45,6 +45,18 @@ class UserPolicy
!has_any_logs_in_editable_collection_period && !has_signed_data_protection_agreement? !has_any_logs_in_editable_collection_period && !has_signed_data_protection_agreement?
end end
%w[
edit_organisation?
log_reassignment?
update_log_reassignment?
organisation_change_confirmation?
confirm_organisation_change?
].each do |method_name|
define_method method_name do
@current_user.support?
end
end
private private
def has_any_logs_in_editable_collection_period def has_any_logs_in_editable_collection_period

2
app/views/bulk_upload_lettings_results/show.html.erb

@ -2,6 +2,8 @@
<%= govuk_back_link(href: :back) %> <%= govuk_back_link(href: :back) %>
<% end %> <% end %>
<%= render partial: "bulk_upload_shared/moved_user_banner" %>
<div class="govuk-grid-row"> <div class="govuk-grid-row">
<div class="govuk-grid-column-two-thirds"> <div class="govuk-grid-column-two-thirds">
<span class="govuk-caption-l">Bulk upload for lettings (<%= @bulk_upload.year_combo %>)</span> <span class="govuk-caption-l">Bulk upload for lettings (<%= @bulk_upload.year_combo %>)</span>

2
app/views/bulk_upload_lettings_results/summary.html.erb

@ -1,3 +1,5 @@
<%= render partial: "bulk_upload_shared/moved_user_banner" %>
<div class="govuk-grid-row"> <div class="govuk-grid-row">
<div class="govuk-grid-column-two-thirds"> <div class="govuk-grid-column-two-thirds">
<span class="govuk-caption-l">Bulk upload for lettings (<%= @bulk_upload.year_combo %>)</span> <span class="govuk-caption-l">Bulk upload for lettings (<%= @bulk_upload.year_combo %>)</span>

2
app/views/bulk_upload_sales_results/show.html.erb

@ -2,6 +2,8 @@
<%= govuk_back_link(href: :back) %> <%= govuk_back_link(href: :back) %>
<% end %> <% end %>
<%= render partial: "bulk_upload_shared/moved_user_banner" %>
<div class="govuk-grid-row"> <div class="govuk-grid-row">
<div class="govuk-grid-column-two-thirds"> <div class="govuk-grid-column-two-thirds">
<span class="govuk-caption-l">Bulk Upload for sales (<%= @bulk_upload.year_combo %>)</span> <span class="govuk-caption-l">Bulk Upload for sales (<%= @bulk_upload.year_combo %>)</span>

2
app/views/bulk_upload_sales_results/summary.html.erb

@ -1,3 +1,5 @@
<%= render partial: "bulk_upload_shared/moved_user_banner" %>
<div class="govuk-grid-row"> <div class="govuk-grid-row">
<div class="govuk-grid-column-two-thirds"> <div class="govuk-grid-column-two-thirds">
<span class="govuk-caption-l">Bulk upload for sales (<%= @bulk_upload.year_combo %>)</span> <span class="govuk-caption-l">Bulk upload for sales (<%= @bulk_upload.year_combo %>)</span>

12
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 %>
<p class="govuk-notification-banner__heading govuk-!-width-full" style="max-width: fit-content">
This error report is out of date.
<p>
<% 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 %>

19
app/views/users/edit.html.erb

@ -7,6 +7,7 @@
<%= form_for(@user, as: :user, html: { method: :patch }) do |f| %> <%= form_for(@user, as: :user, html: { method: :patch }) do |f| %>
<div class="govuk-grid-row"> <div class="govuk-grid-row">
<div class="govuk-grid-column-two-thirds"> <div class="govuk-grid-column-two-thirds">
<% remove_attributes_from_error_messages(@user) %>
<%= f.govuk_error_summary %> <%= f.govuk_error_summary %>
<h1 class="govuk-heading-l"> <h1 class="govuk-heading-l">
@ -32,6 +33,24 @@
autocomplete: "tel-extension", autocomplete: "tel-extension",
spellcheck: "false" %> spellcheck: "false" %>
<% if UserPolicy.new(current_user, @user).edit_organisation? %>
<% null_option = [OpenStruct.new(id: "", name: "Select an option")] %>
<% organisations = Organisation.filter_by_active.map { |org| OpenStruct.new(id: org.id, name: org.name) } %>
<% answer_options = null_option + organisations %>
<%= f.govuk_select(:organisation_id,
label: { text: "Organisation", size: "m" },
"data-controller": "accessible-autocomplete") do %>
<% answer_options.each do |answer| %>
<option value="<%= answer.id %>"
data-synonyms="<%= answer_option_synonyms(answer.resource) %>"
data-append="<%= answer_option_append(answer.resource) %>"
data-hint="<%= answer_option_hint(answer.resource) %>"
<%= @user.organisation_id == answer.id ? "selected" : "" %>><%= answer.name || answer.resource %></option>
<% end %>
<% end %>
<% end %>
<% if current_user.data_coordinator? || current_user.support? %> <% if current_user.data_coordinator? || current_user.support? %>
<% roles = current_user.assignable_roles.map { |key, _| OpenStruct.new(id: key, name: key.to_s.humanize) } %> <% roles = current_user.assignable_roles.map { |key, _| OpenStruct.new(id: key, name: key.to_s.humanize) } %>

36
app/views/users/log_reassignment.html.erb

@ -0,0 +1,36 @@
<% content_for :title, "Should this user’s logs move to their new organisation?" %>
<% content_for :before_content do %>
<%= govuk_back_link(href: aliased_user_edit(@user, current_user)) %>
<% end %>
<%= form_with model: @user, method: :patch, url: user_log_reassignment_path(@user) do |f| %>
<div class="govuk-grid-row">
<div class="govuk-grid-column-two-thirds">
<%= f.govuk_error_summary %>
<h1 class="govuk-heading-l">
<%= content_for(:title) %>
</h1>
<%= govuk_warning_text do %>
<%= organisation_change_warning(@user, @new_organisation) %>
<% end %>
<% log_reassignment = User::LOG_REASSIGNMENT.map { |key, value| OpenStruct.new(id: key, name: value) } %>
<%= f.govuk_collection_radio_buttons :log_reassignment,
log_reassignment,
:id,
:name,
legend: { text: "Log reassignment", hidden: true } %>
<%= f.hidden_field :organisation_id, value: @new_organisation.id %>
<div class="govuk-button-group">
<%= f.govuk_submit "Continue" %>
<%= govuk_button_link_to "Cancel", aliased_user_edit(@user, current_user), secondary: true %>
</div>
</div>
</div>
<% end %>

24
app/views/users/organisation_change_confirmation.html.erb

@ -0,0 +1,24 @@
<% content_for :before_content do %>
<% content_for :title, "Are you sure you want to move this user?" %>
<%= govuk_back_link(href: :back) %>
<% end %>
<div class="govuk-grid-row">
<div class="govuk-grid-column-two-thirds-from-desktop">
<h1 class="govuk-heading-xl">
<%= content_for(:title) %>
</h1>
<%= govuk_warning_text(text: organisation_change_confirmation_warning(@user, @new_organisation, @log_reassignment)) %>
<%= form_with model: @user, url: user_organisation_change_confirmation_path(@user), method: :patch do |f| %>
<%= f.hidden_field :organisation_id, value: @new_organisation.id %>
<%= f.hidden_field :log_reassignment, value: @log_reassignment %>
<div class="govuk-button-group">
<%= f.govuk_submit "Move this user" %>
<%= govuk_button_link_to "Cancel", user_log_reassignment_path(@user, organisation_id: @new_organisation.id, log_reassignment: @log_reassignment), secondary: true %>
</div>
<% end %>
</div>
</div>

10
app/views/users/show.html.erb

@ -68,7 +68,15 @@
<%= summary_list.with_row do |row| <%= summary_list.with_row do |row|
row.with_key { "Organisation" } 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_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 %> end %>
<%= summary_list.with_row do |row| <%= summary_list.with_row do |row|

4
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. too_short: The password you entered is too short. Enter a password that is %{count} characters or longer.
reset_password_token: reset_password_token:
invalid: "That link is invalid. Check you are using the correct link." 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: merge_request:
attributes: attributes:
absorbing_organisation_id: absorbing_organisation_id:

4
config/routes.rb

@ -124,6 +124,10 @@ Rails.application.routes.draw do
resources :users do resources :users do
get "edit-dpo", to: "users#dpo" get "edit-dpo", to: "users#dpo"
get "edit-key-contact", to: "users#key_contact" 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 collection do
get :search get :search

5
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

3
db/schema.rb

@ -10,7 +10,7 @@
# #
# It's strongly recommended that you check this file into your version control system. # It's strongly recommended that you check this file into your version control system.
ActiveRecord::Schema[7.0].define(version: 2024_08_22_080228) do ActiveRecord::Schema[7.0].define(version: 2024_09_11_152702) do
# These are extensions that must be enabled in order to support this database # These are extensions that must be enabled in order to support this database
enable_extension "plpgsql" enable_extension "plpgsql"
@ -42,6 +42,7 @@ ActiveRecord::Schema[7.0].define(version: 2024_08_22_080228) do
t.text "choice" t.text "choice"
t.integer "total_logs_count" t.integer "total_logs_count"
t.string "rent_type_fix_status", default: "not_applied" t.string "rent_type_fix_status", default: "not_applied"
t.integer "moved_user_id"
t.index ["identifier"], name: "index_bulk_uploads_on_identifier", unique: true t.index ["identifier"], name: "index_bulk_uploads_on_identifier", unique: true
t.index ["user_id"], name: "index_bulk_uploads_on_user_id" t.index ["user_id"], name: "index_bulk_uploads_on_user_id"
end end

6
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| 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 = route.path.spec.to_s
route_path.gsub(":id", other_user.id.to_s).gsub(":user_id", other_user.id.to_s).gsub("(.:format)", "") 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 }.uniq
end end
before do
create(:lettings_log, assigned_to: other_user)
end
it "is has accessible pages" do it "is has accessible pages" do
user_paths.each do |path| user_paths.each do |path|
visit(path) visit(path)

86
spec/helpers/user_helper_spec.rb

@ -61,4 +61,90 @@ RSpec.describe UserHelper do
end end
end 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 end

300
spec/models/user_spec.rb

@ -528,4 +528,304 @@ RSpec.describe User, type: :model do
end end
end 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 end

14
spec/policies/user_policy_spec.rb

@ -100,6 +100,20 @@ RSpec.describe UserPolicy do
end end
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 permissions :delete? do
context "with active user" do context "with active user" do
let(:user) { create(:user, last_sign_in_at: Time.zone.yesterday) } let(:user) { create(:user, last_sign_in_at: Time.zone.yesterday) }

45
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).to be_successful
expect(response.body).to include(bulk_upload.filename) expect(response.body).to include(bulk_upload.filename)
end 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
end end
@ -107,5 +129,28 @@ RSpec.describe BulkUploadLettingsResultsController, type: :request do
expect(response).to be_not_found expect(response).to be_not_found
end end
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
end end

39
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.") 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
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 end
describe "GET /lettings-logs/bulk-upload-resume/:ID/fix-choice" do 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") expect(response).to redirect_to("/lettings-logs/bulk-upload-soft-validations-check/#{bulk_upload.id}/chosen")
end end
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 end
describe "GET /lettings-logs/bulk-upload-resume/:ID/fix-choice?soft_errors_only=true" do describe "GET /lettings-logs/bulk-upload-resume/:ID/fix-choice?soft_errors_only=true" do

59
spec/requests/bulk_upload_sales_results_controller_spec.rb

@ -11,6 +11,42 @@ RSpec.describe BulkUploadSalesResultsController, type: :request do
sign_in viewing_user sign_in viewing_user
end 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 describe "GET /sales-logs/bulk-upload-results/:ID" do
it "renders correct year" do it "renders correct year" do
get "/sales-logs/bulk-upload-results/#{bulk_upload.id}" 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 expect(response).to be_not_found
end end
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
end end

39
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.") 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
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 end
describe "GET /sales-logs/bulk-upload-resume/:ID/fix-choice" do 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") expect(response).to redirect_to("/sales-logs/bulk-upload-soft-validations-check/#{bulk_upload.id}/chosen")
end end
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 end
describe "GET /sales-logs/bulk-upload-resume/:ID/fix-choice?soft_errors_only=true" do describe "GET /sales-logs/bulk-upload-resume/:ID/fix-choice?soft_errors_only=true" do

506
spec/requests/users_controller_spec.rb

@ -124,6 +124,13 @@ RSpec.describe UsersController, type: :request do
expect(response).to redirect_to("/account/sign-in") expect(response).to redirect_to("/account/sign-in")
end end
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 end
context "when user is signed in as a data provider" do 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: "role")
expect(page).not_to have_link("Change", text: "if data protection officer") 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: "if a key contact")
expect(page).not_to have_link("Change", text: "organisation")
end end
it "does not allow deactivating the user" do 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: "role")
expect(page).not_to have_link("Change", text: "if data protection officer") 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: "if a key contact")
expect(page).not_to have_link("Change", text: "organisation")
end end
it "does not allow deactivating the user" do 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[role]")
expect(page).not_to have_field("user[is_dpo]") 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[is_key_contact]")
expect(page).not_to have_field("user[organisation_id]")
end end
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]) expect(result.keys).to match_array([org_user.id.to_s, managing_user.id.to_s])
end end
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 end
context "when user is signed in as a data coordinator" do 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: "role")
expect(page).to have_link("Change", text: "if data protection officer") 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: "if a key contact")
expect(page).not_to have_link("Change", text: "organisation")
end end
it "does not allow deactivating the user" do 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: "role")
expect(page).to have_link("Change", text: "if data protection officer") 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: "if a key contact")
expect(page).not_to have_link("Change", text: "organisation")
end end
it "allows deactivating the user" do 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[name]")
expect(page).to have_field("user[email]") expect(page).to have_field("user[email]")
expect(page).to have_field("user[role]") expect(page).to have_field("user[role]")
expect(page).not_to have_field("user[organisation_id]")
end end
it "does not allow setting the role to `support`" do 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[name]")
expect(page).to have_field("user[email]") expect(page).to have_field("user[email]")
expect(page).to have_field("user[role]") expect(page).to have_field("user[role]")
expect(page).not_to have_field("user[organisation_id]")
end end
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]) expect(result.keys).to match_array([org_user.id.to_s, managing_user.id.to_s])
end end
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 end
context "when user is signed in as a support user" do 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: "role")
expect(page).to have_link("Change", text: "if data protection officer") 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: "if a key contact")
expect(page).to have_link("Change", text: "organisation")
end end
it "does not allow deactivating the user" do 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: "role")
expect(page).to have_link("Change", text: "if data protection officer") 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: "if a key contact")
expect(page).to have_link("Change", text: "organisation")
end end
it "links to user organisation" do 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[role]")
expect(page).to have_field("user[phone]") expect(page).to have_field("user[phone]")
expect(page).to have_field("user[phone_extension]") expect(page).to have_field("user[phone_extension]")
expect(page).to have_field("user[organisation_id]")
end end
it "allows setting the role to `support`" do 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[role]")
expect(page).to have_field("user[phone]") expect(page).to have_field("user[phone]")
expect(page).to have_field("user[phone_extension]") expect(page).to have_field("user[phone_extension]")
expect(page).to have_field("user[organisation_id]")
end end
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[role]")
expect(page).to have_field("user[phone]") expect(page).to have_field("user[phone]")
expect(page).to have_field("user[phone_extension]") expect(page).to have_field("user[phone_extension]")
expect(page).to have_field("user[organisation_id]")
end end
end end
@ -1682,10 +1715,11 @@ RSpec.describe UsersController, type: :request do
get "/users/#{other_user.id}/edit", headers:, params: {} get "/users/#{other_user.id}/edit", headers:, params: {}
end end
it "redirects to user details page" do it "allows editing the user" do
expect(response).to redirect_to("/users/#{other_user.id}") expect(page).to have_field("user[name]")
follow_redirect! expect(page).to have_field("user[email]")
expect(page).not_to have_link("Change") expect(page).to have_field("user[role]")
expect(page).to have_field("user[organisation_id]")
end end
end end
end end
@ -1820,6 +1854,20 @@ RSpec.describe UsersController, type: :request do
patch "/users/#{other_user.id}", headers:, params: patch "/users/#{other_user.id}", headers:, params:
end end
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 end
context "when we update the user password" do 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") expect(page).to have_selector(".govuk-error-summary__title")
end end
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 end
context "when the current user does not match the user ID" do context "when the current user does not match the user ID" do
@ -1890,89 +1971,237 @@ RSpec.describe UsersController, type: :request do
end end
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
context "when the user is not part of the same organisation as the current user" do let(:other_user) { create(:user) }
let(:other_user) { create(:user) } let(:params) { { id: other_user.id, user: { name: new_name } } }
let(:params) { { id: other_user.id, user: { name: new_name } } }
it "updates the user" do it "updates the user" do
expect { patch "/users/#{other_user.id}", headers:, params: } expect { patch "/users/#{other_user.id}", headers:, params: }
.to change { other_user.reload.name }.from(other_user.name).to(new_name) .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
end
it "tracks who updated the record" do it "does not bypass sign in for the support user" do
expect { patch "/users/#{other_user.id}", headers:, params: } patch "/users/#{other_user.id}", headers: headers, params: params
.to change { other_user.reload.versions.last.actor&.id }.from(nil).to(user.id) 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 end
context "when user changes email, dpo, key_contact" do it "shows flash notice" do
let(:params) { { id: other_user.id, user: { name: new_name, email: new_email, is_dpo: "true", is_key_contact: "true" } } } 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:, params:
patch "/users/#{other_user.id}", headers: headers, params: params end
other_user.reload end
expect(other_user.unconfirmed_email).to eq(new_email)
expect(other_user.is_data_protection_officer?).to be true context "when updating organisation" do
expect(other_user.is_key_contact?).to be true 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
end end
it "does not bypass sign in for the support user" do context "and organisation id is not nil" do
patch "/users/#{other_user.id}", headers: headers, params: params let(:params) { { id: other_user.id, user: { organisation_id: new_organisation.id, name: "new_name" } } }
follow_redirect!
expect(page).to have_content("#{other_user.reload.name}’s account") it "does not update the organisation" do
expect(page).to have_content(other_user.reload.email.to_s) 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
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 context "and log reassignment choice is not present" do
let(:params) do let(:params) { { user: { organisation_id: new_organisation.id, log_reassignment: nil } } }
{
id: user.id, user: { password: new_name, password_confirmation: new_name, name: "new name", email: new_email } before do
} patch "/users/#{other_user.id}/log-reassignment", headers:, params:
end end
let(:personalisation) do it "does not update the user's organisation" do
{ expect(other_user.reload.organisation).not_to eq(new_organisation)
name: params[:user][:name], end
email: new_email,
organisation: other_user.organisation.name, it "displays the error message" do
link: include("/account/confirmation?confirmation_token="), 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 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 before do
other_user.legacy_users.destroy_all patch "/users/#{other_user.id}/log-reassignment", headers:, params:
end end
it "shows flash notice" do it "does not update the user's organisation" do
patch("/users/#{other_user.id}", headers:, params:) 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
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 it "required the new org to have stock owner relationship with the current user org" do
expect(notify_client).to receive(:send_email).with( expect(response).to have_http_status(:unprocessable_entity)
email_address: other_user.email, expect(page).to have_content("New org must be a stock owner of #{other_user.organisation_name} to make this change.")
template_id: User::FOR_OLD_EMAIL_CHANGED_BY_OTHER_USER_TEMPLATE_ID, end
personalisation: { end
new_email:,
old_email: other_user.email, context "when different organisations manage the logs" do
}, before do
).once create(:lettings_log, managing_organisation: other_user.organisation, assigned_to: other_user)
create(:lettings_log, managing_organisation: new_organisation_2, assigned_to: other_user)
expect(notify_client).to receive(:send_email).with( create(:sales_log, managing_organisation: new_organisation_3, assigned_to: other_user)
email_address: new_email, patch "/users/#{other_user.id}/log-reassignment", headers:, params:
template_id: User::FOR_NEW_EMAIL_CHANGED_BY_OTHER_USER_TEMPLATE_ID, end
personalisation: {
new_email:, it "required the new org to have stock owner relationship with the managing organisations" do
old_email: other_user.email, expect(response).to have_http_status(:unprocessable_entity)
link: include("/account/confirmation?confirmation_token="), 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
).once end
end
expect(notify_client).not_to receive(:send_email)
context "and log reassignment choice is to change managing agent" do
patch "/users/#{other_user.id}", headers:, params: 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 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]) 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
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 end
describe "title link" do describe "title link" do

Loading…
Cancel
Save