diff --git a/app/models/form/lettings/questions/created_by_id.rb b/app/models/form/lettings/questions/created_by_id.rb index ea6375152..e4d6dfbbd 100644 --- a/app/models/form/lettings/questions/created_by_id.rb +++ b/app/models/form/lettings/questions/created_by_id.rb @@ -18,17 +18,16 @@ class Form::Lettings::Questions::CreatedById < ::Form::Question users = [] users += if current_user.support? [ - ( - if log.owning_organisation - log.owning_organisation.absorbing_organisation.present? ? log.owning_organisation&.absorbing_organisation&.users&.visible : log.owning_organisation&.users&.visible - end), - ( - if log.managing_organisation - log.managing_organisation.absorbing_organisation.present? ? log.managing_organisation&.absorbing_organisation&.users&.visible : log.managing_organisation.users&.visible - end), + if log.owning_organisation + log.owning_organisation.absorbing_organisation.present? ? log.owning_organisation&.absorbing_organisation&.users : log.owning_organisation&.users + end&.visible&.activated, + if log.managing_organisation + log.managing_organisation.absorbing_organisation.present? ? log.managing_organisation&.absorbing_organisation&.users : log.managing_organisation.users + end&.visible&.activated, ].flatten else - current_user.organisation.users.visible + # ensure data coordinators can't assign a log to an inactive user + current_user.organisation.users.visible.activated end.uniq.compact users.each_with_object(ANSWER_OPTS.dup) do |user, hsh| diff --git a/app/models/form/sales/questions/created_by_id.rb b/app/models/form/sales/questions/created_by_id.rb index 516afd2bc..500873138 100644 --- a/app/models/form/sales/questions/created_by_id.rb +++ b/app/models/form/sales/questions/created_by_id.rb @@ -19,13 +19,13 @@ class Form::Sales::Questions::CreatedById < ::Form::Question users = [] users += if current_user.support? [ - ( - if log.managing_organisation - log.managing_organisation.absorbing_organisation.present? ? log.managing_organisation&.absorbing_organisation&.users&.visible : log.managing_organisation.users.visible - end), + if log.managing_organisation + log.managing_organisation.absorbing_organisation.present? ? log.managing_organisation&.absorbing_organisation&.users : log.managing_organisation.users + end&.visible&.activated, ].flatten else - log.managing_organisation.users.visible + # ensure data coordinators can't assign a log to an inactive user + log.managing_organisation.users.visible.activated end.uniq.compact users.each_with_object(ANSWER_OPTS.dup) do |user, hsh| hsh[user.id] = present_user(user) diff --git a/app/models/user.rb b/app/models/user.rb index 1733aeb7d..f10c9456a 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -83,6 +83,8 @@ class User < ApplicationRecord } scope :not_signed_in, -> { where(last_sign_in_at: nil, active: true) } scope :deactivated, -> { where(active: false) } + scope :activated, -> { where(active: true) } + # in some cases we only count the user as active if they completed the onboarding flow and signed in, rather than just being added scope :active_status, -> { where(active: true).where.not(last_sign_in_at: nil) } scope :visible, lambda { |user = nil| if user && !user.support? diff --git a/spec/models/form/lettings/questions/created_by_id_spec.rb b/spec/models/form/lettings/questions/created_by_id_spec.rb index 1cae12396..75736ab44 100644 --- a/spec/models/form/lettings/questions/created_by_id_spec.rb +++ b/spec/models/form/lettings/questions/created_by_id_spec.rb @@ -59,6 +59,17 @@ RSpec.describe Form::Lettings::Questions::CreatedById, type: :model do expect(question.displayed_answer_options(lettings_log, support_user)).to eq(expected_option_for_users(managing_org_user.organisation.users.visible + owning_org_user.organisation.users.visible)) end end + + context "when organisation has inactive users" do + before do + create(:user, name: "Inactive user", active: false, organisation: owning_org_user.organisation) + create(:user, name: "Inactive managing user", active: false, organisation: managing_org_user.organisation) + end + + it "does not display inactive users" do + expect(question.displayed_answer_options(lettings_log, support_user)).to eq(expected_option_for_users(managing_org_user.organisation.users.visible.activated + owning_org_user.organisation.users.visible.activated)) + end + end end end @@ -86,6 +97,16 @@ RSpec.describe Form::Lettings::Questions::CreatedById, type: :model do expect(question.displayed_answer_options(lettings_log, data_coordinator)).to eq(expected_option_for_users(data_coordinator.organisation.users.visible)) end end + + context "when organisation has inactive users" do + before do + create(:user, name: "Inactive user", active: false, organisation: data_coordinator.organisation) + end + + it "does not display inactive users" do + expect(question.displayed_answer_options(lettings_log, data_coordinator)).to eq(expected_option_for_users(data_coordinator.organisation.users.visible.activated)) + end + end end end end diff --git a/spec/models/form/sales/questions/created_by_id_spec.rb b/spec/models/form/sales/questions/created_by_id_spec.rb index 0c21a21c8..0ae984da4 100644 --- a/spec/models/form/sales/questions/created_by_id_spec.rb +++ b/spec/models/form/sales/questions/created_by_id_spec.rb @@ -55,6 +55,16 @@ RSpec.describe Form::Sales::Questions::CreatedById, type: :model do expect(question.displayed_answer_options(sales_log, support_user)).to eq(expected_option_for_users(owning_org_user.organisation.users.visible)) end end + + context "when organisation has inactive users" do + before do + create(:user, name: "Inactive user", active: false, organisation: owning_org_user.organisation) + end + + it "does not display inactive users" do + expect(question.displayed_answer_options(sales_log, support_user)).to eq(expected_option_for_users(owning_org_user.organisation.users.visible.activated)) + end + end end end @@ -86,6 +96,16 @@ RSpec.describe Form::Sales::Questions::CreatedById, type: :model do expect(question.displayed_answer_options(sales_log, data_coordinator)).to eq(expected_option_for_users(owning_org_user.organisation.users.visible)) end end + + context "when organisation has inactive users" do + before do + create(:user, name: "Inactive user", active: false, organisation: data_coordinator.organisation) + end + + it "does not display deleted users" do + expect(question.displayed_answer_options(sales_log, data_coordinator)).to eq(expected_option_for_users(owning_org_user.organisation.users.visible.activated)) + end + end end end end