baarkerlounger
/
submit-social-housing-lettings-and-sales-data
mirror of https://github.com/communitiesuk/submit-social-housing-lettings-and-sales-data.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity
Browse Source

Address comments 

- only select required users
- remove not needed CYA checks
pull/1533/head
Jack S 3 years ago
parent
d7102b6a7b
commit
c315f6f17c
4 changed files with 37 additions and 127 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Split View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 50
      app/models/form/lettings/questions/created_by_id.rb
  2. 27
      app/models/form/sales/questions/created_by_id.rb
  3. 46
      spec/models/form/lettings/questions/created_by_id_spec.rb
  4. 41
      spec/models/form/sales/questions/created_by_id_spec.rb

50
app/models/form/lettings/questions/created_by_id.rb
Unescape View File

@ -10,42 +10,32 @@ class Form::Lettings::Questions::CreatedById < ::Form::Question
end
def answer_options
return ANSWER_OPTS unless ActiveRecord::Base.connected?
ANSWER_OPTS
end
User.select(:id, :name, :email).each_with_object(ANSWER_OPTS.dup) do |user, hsh|
hsh[user.id] = "#{user.name} (#{user.email})"
def displayed_answer_options(log, current_user = nil)
return ANSWER_OPTS unless current_user
users = []
users += if current_user.support?
[
(log.owning_organisation&.users if log.owning_organisation),
(log.managing_organisation&.users if log.managing_organisation),
].flatten
else
current_user.organisation.users
end.uniq.compact
users.each_with_object(ANSWER_OPTS.dup) do |user, hsh|
hsh[user.id] = present_user(user)
hsh
end
end
def displayed_answer_options(log, user = nil)
return ANSWER_OPTS unless user
return ANSWER_OPTS unless user.support? || user.data_coordinator?
user_ids = [""]
user_ids += if user.support?
[
(log.owning_organisation&.users&.pluck(:id) if log.owning_organisation),
(log.managing_organisation&.users&.pluck(:id) if log.managing_organisation),
]
else
user.organisation.users.pluck(:id)
end.flatten.uniq
answer_options.select { |k, _v| user_ids.include?(k) }
end
def label_from_value(value, _log = nil, _user = nil)
return unless value
answer_options[value]
end
def hidden_in_check_answers?(_log, current_user)
return false if current_user.support?
return false if current_user.data_coordinator?
true
present_user(User.find(value))
end
def derived?
@ -54,6 +44,10 @@ class Form::Lettings::Questions::CreatedById < ::Form::Question
private
def present_user(user)
"#{user.name} (#{user.email})"
end
def selected_answer_option_is_derived?(_log)
true
end

27
app/models/form/sales/questions/created_by_id.rb
Unescape View File

@ -10,30 +10,25 @@ class Form::Sales::Questions::CreatedById < ::Form::Question
end
def answer_options
return ANSWER_OPTS unless ActiveRecord::Base.connected?
User.select(:id, :name, :email).each_with_object(ANSWER_OPTS.dup) do |user, hsh|
hsh[user.id] = "#{user.name} (#{user.email})"
hsh
end
ANSWER_OPTS
end
def displayed_answer_options(log, user = nil)
def displayed_answer_options(log, current_user = nil)
return ANSWER_OPTS unless log.owning_organisation
return ANSWER_OPTS unless user
return ANSWER_OPTS unless user.support? || user.data_coordinator?
users = user.support? ? log.owning_organisation.users : user.organisation.users
return ANSWER_OPTS unless current_user
user_ids = users.pluck(:id) + [""]
users = current_user.support? ? log.owning_organisation.users : current_user.organisation.users
answer_options.select { |k, _v| user_ids.include?(k) }
users.each_with_object(ANSWER_OPTS.dup) do |user, hsh|
hsh[user.id] = present_user(user)
hsh
end
end
def label_from_value(value, _log = nil, _user = nil)
return unless value
answer_options[value]
present_user(User.find(value))
end
def hidden_in_check_answers?(_log, current_user)
@ -49,6 +44,10 @@ class Form::Sales::Questions::CreatedById < ::Form::Question
private
def present_user(user)
"#{user.name} (#{user.email})"
end
def selected_answer_option_is_derived?(_log)
false
end

46
spec/models/form/lettings/questions/created_by_id_spec.rb
Unescape View File

@ -8,16 +8,6 @@ RSpec.describe Form::Lettings::Questions::CreatedById, type: :model do
let(:page) { instance_double(Form::Page) }
let(:subsection) { instance_double(Form::Subsection) }
let(:form) { instance_double(Form) }
let(:user_1) { create(:user, name: "first user") }
let(:user_2) { create(:user, name: "second user") }
let(:user_3) { create(:user, name: "third user") }
let!(:expected_answer_options) do
{
"" => "Select an option",
user_1.id => "#{user_1.name} (#{user_1.email})",
user_2.id => "#{user_2.name} (#{user_2.email})",
}
end
it "has correct page" do
expect(question.page).to eq(page)
@ -44,7 +34,7 @@ RSpec.describe Form::Lettings::Questions::CreatedById, type: :model do
end
it "has the correct answer options" do
expect(question.answer_options).to eq(expected_answer_options)
expect(question.answer_options).to eq({ "" => "Select an option" })
end
it "is marked as derived" do
@ -56,10 +46,6 @@ RSpec.describe Form::Lettings::Questions::CreatedById, type: :model do
let(:managing_org_user) { create(:user) }
let(:support_user) { create(:user, :support, organisation: owning_org_user.organisation) }
it "is shown in check answers" do
expect(question.hidden_in_check_answers?(nil, support_user)).to be false
end
describe "#displayed_answer_options" do
let(:lettings_log) do
create(:lettings_log, created_by: support_user, owning_organisation: owning_org_user.organisation, managing_organisation: managing_org_user.organisation)
@ -84,10 +70,6 @@ RSpec.describe Form::Lettings::Questions::CreatedById, type: :model do
let(:managing_org_user) { create(:user) }
let(:data_coordinator) { create(:user, :data_coordinator) }
it "is shown in check answers" do
expect(question.hidden_in_check_answers?(nil, data_coordinator)).to be false
end
describe "#displayed_answer_options" do
let(:lettings_log) do
create(:lettings_log, created_by: data_coordinator, owning_organisation: data_coordinator.organisation, managing_organisation: managing_org_user.organisation)
@ -107,30 +89,4 @@ RSpec.describe Form::Lettings::Questions::CreatedById, type: :model do
end
end
end
context "when the current user is data_provider" do
let(:data_provider) { create(:user, :data_provider) }
it "is not shown in check answers" do
expect(question.hidden_in_check_answers?(nil, data_provider)).to be true
end
describe "#displayed_answer_options" do
let(:owning_org_user) { create(:user) }
let(:lettings_log) { create(:lettings_log, owning_organisation: owning_org_user.organisation) }
let(:user_in_same_org) { create(:user, organisation: data_provider.organisation) }
let(:expected_answer_options) do
{
"" => "Select an option",
user_in_same_org.id => "#{user_in_same_org.name} (#{user_in_same_org.email})",
data_provider.id => "#{data_provider.name} (#{data_provider.email})",
}
end
it "only displays users that belong user's org" do
expect(question.displayed_answer_options(lettings_log, data_provider)).to eq(Form::Lettings::Questions::CreatedById::ANSWER_OPTS)
end
end
end
end

41
spec/models/form/sales/questions/created_by_id_spec.rb
Unescape View File

@ -8,15 +8,6 @@ RSpec.describe Form::Sales::Questions::CreatedById, type: :model do
let(:page) { instance_double(Form::Page) }
let(:subsection) { instance_double(Form::Subsection) }
let(:form) { instance_double(Form) }
let(:user_1) { create(:user, name: "first user") }
let(:user_2) { create(:user, name: "second user") }
let!(:expected_answer_options) do
{
"" => "Select an option",
user_1.id => "#{user_1.name} (#{user_1.email})",
user_2.id => "#{user_2.name} (#{user_2.email})",
}
end
it "has correct page" do
expect(question.page).to eq(page)
@ -42,10 +33,6 @@ RSpec.describe Form::Sales::Questions::CreatedById, type: :model do
expect(question.hint_text).to be_nil
end
it "has the correct answer options" do
expect(question.answer_options).to eq(expected_answer_options)
end
it "is marked as derived" do
expect(question.derived?).to be true
end
@ -83,7 +70,7 @@ RSpec.describe Form::Sales::Questions::CreatedById, type: :model do
describe "#displayed_answer_options" do
let(:owning_org_user) { create(:user) }
let(:sales_log) { create(:sales_log, owning_organisation: owning_org_user.organisation) }
let(:user_in_same_org) { create(:user, organisation: data_coordinator.organisation) }
let!(:user_in_same_org) { create(:user, organisation: data_coordinator.organisation) }
let(:expected_answer_options) do
{
@ -98,30 +85,4 @@ RSpec.describe Form::Sales::Questions::CreatedById, type: :model do
end
end
end
context "when the current user is data_provider" do
let(:data_provider) { create(:user, :data_provider) }
it "is not shown in check answers" do
expect(question.hidden_in_check_answers?(nil, data_provider)).to be true
end
describe "#displayed_answer_options" do
let(:owning_org_user) { create(:user) }
let(:sales_log) { create(:sales_log, owning_organisation: owning_org_user.organisation) }
let(:user_in_same_org) { create(:user, organisation: data_provider.organisation) }
let(:expected_answer_options) do
{
"" => "Select an option",
user_in_same_org.id => "#{user_in_same_org.name} (#{user_in_same_org.email})",
data_provider.id => "#{data_provider.name} (#{data_provider.email})",
}
end
it "only displays users that belong user's org" do
expect(question.displayed_answer_options(sales_log, data_provider)).to eq(Form::Sales::Questions::CreatedById::ANSWER_OPTS)
end
end
end
end

Write Preview
Loading…
Cancel
Save