Browse Source

Allow setting log reassignment choice

pull/2596/head
Kat 2 years ago
parent
commit
fea5fd9d9e
  1. 45
      app/controllers/users_controller.rb
  2. 4
      app/views/users/log_reassignment.html.erb
  3. 4
      config/locales/en.yml
  4. 2
      config/routes.rb
  5. 271
      spec/requests/users_controller_spec.rb

45
app/controllers/users_controller.rb

@ -163,6 +163,20 @@ class UsersController < ApplicationController
end end
end end
def update_log_reassignment
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_confirm_organisation_change_path(@user, log_reassignment_params)
else
render :log_reassignment, status: :unprocessable_entity
end
end
private private
def validate_attributes def validate_attributes
@ -232,6 +246,10 @@ private
user_params.except(:organisation_id) user_params.except(:organisation_id)
end 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
@ -267,4 +285,31 @@ private
def updating_organisation? def updating_organisation?
user_params["organisation_id"].present? && @user.organisation_id != user_params["organisation_id"].to_i user_params["organisation_id"].present? && @user.organisation_id != user_params["organisation_id"].to_i
end 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.map(&:managing_organisation) + @user.assigned_to_sales_logs.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.map(&:owning_organisation) + @user.assigned_to_sales_logs.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

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

@ -4,7 +4,7 @@
<%= govuk_back_link(href: aliased_user_edit(@user, current_user)) %> <%= govuk_back_link(href: aliased_user_edit(@user, current_user)) %>
<% end %> <% end %>
<%= form_for(@user, html: { method: :patch }) do |f| %> <%= form_with model: @user, method: :patch, url: user_log_reassignment_path(@user) 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">
<%= f.govuk_error_summary %> <%= f.govuk_error_summary %>
@ -25,6 +25,8 @@
:name, :name,
legend: { text: "Log reassignment", hidden: true } %> legend: { text: "Log reassignment", hidden: true } %>
<%= f.hidden_field :organisation_id, value: @new_organisation.id %>
<div class="govuk-button-group"> <div class="govuk-button-group">
<%= f.govuk_submit "Continue" %> <%= f.govuk_submit "Continue" %>
<%= govuk_button_link_to "Cancel", aliased_user_edit(@user, current_user), secondary: true %> <%= govuk_button_link_to "Cancel", aliased_user_edit(@user, current_user), secondary: true %>

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:

2
config/routes.rb

@ -125,6 +125,8 @@ Rails.application.routes.draw 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" get "log-reassignment", to: "users#log_reassignment"
patch "log-reassignment", to: "users#update_log_reassignment"
patch "confirm-organisation-change", to: "users#confirm-organisation-change"
collection do collection do
get :search get :search

271
spec/requests/users_controller_spec.rb

@ -1956,121 +1956,236 @@ 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 end
context "when user changes email, dpo, key_contact" do let(:personalisation) do
let(:params) { { id: other_user.id, user: { name: new_name, email: new_email, is_dpo: "true", is_key_contact: "true" } } } {
name: params[:user][:name],
email: new_email,
organisation: other_user.organisation.name,
link: include("/account/confirmation?confirmation_token="),
}
end
it "allows changing email, dpo, key_contact" do before do
patch "/users/#{other_user.id}", headers: headers, params: params other_user.legacy_users.destroy_all
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 "does not bypass sign in for the support user" do it "shows flash notice" do
patch "/users/#{other_user.id}", headers: headers, params: params patch("/users/#{other_user.id}", headers:, params:)
follow_redirect!
expect(page).to have_content("#{other_user.reload.name}’s account") expect(flash[:notice]).to eq("An email has been sent to #{new_email} to confirm this change.")
expect(page).to have_content(other_user.reload.email.to_s)
end end
context "when the support user tries to update the user’s password", :aggregate_failures do it "sends new flow emails" do
let(:params) do expect(notify_client).to receive(:send_email).with(
{ email_address: other_user.email,
id: user.id, user: { password: new_name, password_confirmation: new_name, name: "new name", email: new_email } template_id: User::FOR_OLD_EMAIL_CHANGED_BY_OTHER_USER_TEMPLATE_ID,
} personalisation: {
end new_email:,
old_email: other_user.email,
},
).once
let(:personalisation) do expect(notify_client).to receive(:send_email).with(
{ email_address: new_email,
name: params[:user][:name], template_id: User::FOR_NEW_EMAIL_CHANGED_BY_OTHER_USER_TEMPLATE_ID,
email: new_email, personalisation: {
organisation: other_user.organisation.name, new_email:,
old_email: other_user.email,
link: include("/account/confirmation?confirmation_token="), link: include("/account/confirmation?confirmation_token="),
} },
).once
expect(notify_client).not_to receive(:send_email)
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
end
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 "and log reassignment choice is not present" do
let(:params) { { user: { organisation_id: new_organisation.id, log_reassignment: nil } } }
before do before do
other_user.legacy_users.destroy_all 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 "shows flash notice" do it "displays the error message" do
patch("/users/#{other_user.id}", headers:, params:) expect(response).to have_http_status(:unprocessable_entity)
expect(page).to have_content("Select if you want to reassign logs")
end
end
expect(flash[:notice]).to eq("An email has been sent to #{new_email} to confirm this change.") 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 end
it "sends new flow emails" do it "does not update the user's organisation" do
expect(notify_client).to receive(:send_email).with( expect(other_user.reload.organisation).not_to eq(new_organisation)
email_address: other_user.email, end
template_id: User::FOR_OLD_EMAIL_CHANGED_BY_OTHER_USER_TEMPLATE_ID,
personalisation: { it "redirects to confirmation page" do
new_email:, expect(response).to redirect_to("/users/#{other_user.id}/confirm-organisation-change?log_reassignment=reassign_all&organisation_id=#{new_organisation.id}")
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:
end end
end end
context "when updating organisation" do context "and log reassignment choice is to unassign logs" do
let(:new_organisation) { create(:organisation) } let(:params) { { user: { organisation_id: new_organisation.id, log_reassignment: "unassign" } } }
before do before do
patch "/users/#{other_user.id}", headers:, params: 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
context "and organisation id is nil" do it "redirects to confirmation page" do
let(:params) { { id: other_user.id, user: { organisation_id: "" } } } expect(response).to redirect_to("/users/#{other_user.id}/confirm-organisation-change?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" } } }
it "does not update the organisation" do 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 "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(response).to have_http_status(:unprocessable_entity)
expect(page).to have_selector(".govuk-error-summary__title") 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 end
end
context "and organisation id is not nil" do context "and log reassignment choice is to change managing agent" do
let(:params) { { id: other_user.id, user: { organisation_id: new_organisation.id, name: "new_name" } } } let(:params) { { user: { organisation_id: new_organisation.id, log_reassignment: "reassign_managing_agent" } } }
it "does not update the organisation" do context "when users organisation manages the logs" do
expect(user.reload.organisation).not_to eq(new_organisation) 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 end
it "redirects to log reassignment page" do it "required the new org to have managing agent relationship with the current user org" do
expect(response).to redirect_to("/users/#{other_user.id}/log-reassignment?organisation_id=#{new_organisation.id}") 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
end
it "updated other fields" do context "when different organisations manage the logs" do
expect(other_user.reload.name).to eq("new_name") 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

Loading…
Cancel
Save