diff --git a/app/models/form/lettings/questions/created_by_id.rb b/app/models/form/lettings/questions/created_by_id.rb index 5459c1956..e4d6dfbbd 100644 --- a/app/models/form/lettings/questions/created_by_id.rb +++ b/app/models/form/lettings/questions/created_by_id.rb @@ -18,18 +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 # ensure data coordinators can't assign a log to an inactive user - current_user.organisation.users.visible.active_status + 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 2171d2f4b..500873138 100644 --- a/app/models/form/sales/questions/created_by_id.rb +++ b/app/models/form/sales/questions/created_by_id.rb @@ -19,14 +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 # ensure data coordinators can't assign a log to an inactive user - log.managing_organisation.users.visible.active_status + 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 8d27f4b48..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 @@ -74,7 +85,7 @@ RSpec.describe Form::Lettings::Questions::CreatedById, type: :model do let(:user_in_same_org) { create(:user, organisation: data_coordinator.organisation) } it "only displays users that belong user's org" do - expect(question.displayed_answer_options(lettings_log, data_coordinator)).to eq(expected_option_for_users(data_coordinator.organisation.users.active_status)) + expect(question.displayed_answer_options(lettings_log, data_coordinator)).to eq(expected_option_for_users(data_coordinator.organisation.users)) end context "when organisation has deleted users" do @@ -83,7 +94,7 @@ RSpec.describe Form::Lettings::Questions::CreatedById, type: :model do end it "does not display deleted users" do - expect(question.displayed_answer_options(lettings_log, data_coordinator)).to eq(expected_option_for_users(data_coordinator.organisation.users.visible.active_status)) + expect(question.displayed_answer_options(lettings_log, data_coordinator)).to eq(expected_option_for_users(data_coordinator.organisation.users.visible)) end end @@ -92,8 +103,8 @@ RSpec.describe Form::Lettings::Questions::CreatedById, type: :model 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(lettings_log, data_coordinator)).to eq(expected_option_for_users(data_coordinator.organisation.users.visible.active_status)) + 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 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 5fb0e6dda..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 @@ -74,7 +84,7 @@ RSpec.describe Form::Sales::Questions::CreatedById, type: :model do end it "only displays users that belong to managing organisation" do - expect(question.displayed_answer_options(sales_log, data_coordinator)).to eq(expected_option_for_users(owning_org_user.organisation.users.active_status)) + expect(question.displayed_answer_options(sales_log, data_coordinator)).to eq(expected_option_for_users(owning_org_user.organisation.users)) end context "when organisation has deleted users" do @@ -83,7 +93,7 @@ RSpec.describe Form::Sales::Questions::CreatedById, type: :model do 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.active_status)) + expect(question.displayed_answer_options(sales_log, data_coordinator)).to eq(expected_option_for_users(owning_org_user.organisation.users.visible)) end end @@ -92,8 +102,8 @@ RSpec.describe Form::Sales::Questions::CreatedById, type: :model 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(sales_log, data_coordinator)).to eq(expected_option_for_users(data_coordinator.organisation.users.visible.active_status)) + 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