From 6ce9ccb78b55cc8d9d4d8d550db68c0bbe4999bf Mon Sep 17 00:00:00 2001 From: Kat Date: Tue, 20 Aug 2024 14:29:10 +0100 Subject: [PATCH] Skip log reassignment question if user has no logs --- app/controllers/users_controller.rb | 5 ++++ app/models/user.rb | 3 +++ app/policies/user_policy.rb | 14 +++++++---- spec/models/user_spec.rb | 31 +++++++++++++++++++++++ spec/requests/users_controller_spec.rb | 35 +++++++++++++++++++------- 5 files changed, 74 insertions(+), 14 deletions(-) diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index d1d131e2b..bc3eebca4 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -155,6 +155,8 @@ class UsersController < ApplicationController def log_reassignment authorize @user + assigned_to_logs_count = @user.assigned_to_lettings_logs.count + @user.assigned_to_sales_logs.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]) @@ -164,6 +166,7 @@ class UsersController < ApplicationController 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]) @@ -178,6 +181,7 @@ class UsersController < ApplicationController end def organisation_change_confirmation + authorize @user if params[:organisation_id].blank? || params[:log_reassignment].blank? || !Organisation.where(id: params[:organisation_id]).exists? return redirect_to user_path(@user) end @@ -187,6 +191,7 @@ class UsersController < ApplicationController end def confirm_organisation_change + authorize @user if log_reassignment_params[:organisation_id].blank? || log_reassignment_params[:log_reassignment].blank? || !Organisation.where(id: log_reassignment_params[:organisation_id]).exists? return redirect_to user_path(@user) end diff --git a/app/models/user.rb b/app/models/user.rb index fac343879..bad4145e3 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -295,6 +295,9 @@ class User < ApplicationRecord sales_logs_to_reassign = assigned_to_sales_logs 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 diff --git a/app/policies/user_policy.rb b/app/policies/user_policy.rb index 084ccbf20..3d89e2380 100644 --- a/app/policies/user_policy.rb +++ b/app/policies/user_policy.rb @@ -45,12 +45,16 @@ class UserPolicy !has_any_logs_in_editable_collection_period && !has_signed_data_protection_agreement? end - def edit_organisation? - @current_user.support? && @user.active? - end - - def log_reassignment? + %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? && @user.active? + end end private diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 3bf618e56..eb53ae989 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -707,5 +707,36 @@ RSpec.describe User, type: :model do 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 + end + + context "and user has logs" do + it "does not move the user to the new organisation" do + user.reassign_logs_and_update_organisation(new_organisation, nil) + + expect(user.organisation).not_to eq(new_organisation) + end + end + end end end diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index 46003aa06..11b85fb43 100644 --- a/spec/requests/users_controller_spec.rb +++ b/spec/requests/users_controller_spec.rb @@ -2411,6 +2411,10 @@ RSpec.describe UsersController, type: :request do 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}") @@ -2418,6 +2422,10 @@ RSpec.describe UsersController, type: :request do 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}") @@ -2427,17 +2435,26 @@ RSpec.describe UsersController, type: :request do context "with valid organisation id" do let(:new_organisation) { create(:organisation, name: "new org") } - before do - create(:lettings_log, assigned_to: other_user) + 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 - 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") + 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