From bfa40c19756e01cb3e05e2998dc67caa253ec9bf Mon Sep 17 00:00:00 2001 From: Jack S Date: Wed, 16 Nov 2022 11:39:07 +0000 Subject: [PATCH] Address comments --- app/helpers/question_attribute_helper.rb | 2 +- .../form/lettings/pages/housing_provider.rb | 2 -- .../lettings/pages/managing_organisation.rb | 2 -- .../lettings/questions/housing_provider.rb | 20 ++++++++----------- .../questions/managing_organisation.rb | 20 ++++++++----------- config/initializers/feature_toggle.rb | 4 +--- db/seeds.rb | 2 +- 7 files changed, 19 insertions(+), 33 deletions(-) diff --git a/app/helpers/question_attribute_helper.rb b/app/helpers/question_attribute_helper.rb index 020ea1909..601368c22 100644 --- a/app/helpers/question_attribute_helper.rb +++ b/app/helpers/question_attribute_helper.rb @@ -11,7 +11,7 @@ module QuestionAttributeHelper { "data-controller": "conditional-question", "data-action": "click->conditional-question#displayConditional", - "data-info": { conditional_questions: conditional_for, type: type }.to_json, + "data-info": { conditional_questions: conditional_for, type: }.to_json, } end diff --git a/app/models/form/lettings/pages/housing_provider.rb b/app/models/form/lettings/pages/housing_provider.rb index 5f017bc00..a1c3a6687 100644 --- a/app/models/form/lettings/pages/housing_provider.rb +++ b/app/models/form/lettings/pages/housing_provider.rb @@ -13,8 +13,6 @@ class Form::Lettings::Pages::HousingProvider < ::Form::Page ] end - # For an organisation owns and manages ONLY their own housing stock AND NOT uses agents to manage the properties AND NOT manages housing stock of other organisations? - # In "set up this lettings log" no extra question should be shown def routed_to?(log, current_user) return false unless current_user return true if current_user.support? diff --git a/app/models/form/lettings/pages/managing_organisation.rb b/app/models/form/lettings/pages/managing_organisation.rb index 340b6375c..67e8ae586 100644 --- a/app/models/form/lettings/pages/managing_organisation.rb +++ b/app/models/form/lettings/pages/managing_organisation.rb @@ -13,8 +13,6 @@ class Form::Lettings::Pages::ManagingOrganisation < ::Form::Page ] end - # For an organisation owns and manages ONLY their own housing stock AND NOT uses agents to manage the properties AND NOT manages housing stock of other organisations? - # In "set up this lettings log" no extra question should be shown def routed_to?(log, current_user) return false unless current_user return true if current_user.support? diff --git a/app/models/form/lettings/questions/housing_provider.rb b/app/models/form/lettings/questions/housing_provider.rb index 69859a58f..f0b69f9f7 100644 --- a/app/models/form/lettings/questions/housing_provider.rb +++ b/app/models/form/lettings/questions/housing_provider.rb @@ -19,10 +19,7 @@ class Form::Lettings::Questions::HousingProvider < ::Form::Question answer_opts[current_user.organisation.id] = "#{current_user.organisation.name} (Your organisation)" end - housing_providers.select(:id, :name).each_with_object(answer_opts) do |organisation, hsh| - hsh[organisation.id] = organisation.name - hsh - end + answer_opts.merge(housing_providers_answer_options) end def displayed_answer_options(log, user = nil) @@ -48,8 +45,7 @@ class Form::Lettings::Questions::HousingProvider < ::Form::Question return false unless @current_user return false if @current_user.support? - # Hide when less than 2 housing providers - housing_providers.count < 2 + housing_providers_answer_options.count < 2 end def enabled @@ -62,11 +58,11 @@ private true end - def housing_providers - @housing_providers ||= if current_user.support? - Organisation.all - else - current_user.organisation.housing_providers - end + def housing_providers_answer_options + @housing_providers_answer_options ||= if current_user.support? + Organisation + else + current_user.organisation.housing_providers + end.pluck(:id, :name).to_h end end diff --git a/app/models/form/lettings/questions/managing_organisation.rb b/app/models/form/lettings/questions/managing_organisation.rb index a0f6a8742..ff6da010a 100644 --- a/app/models/form/lettings/questions/managing_organisation.rb +++ b/app/models/form/lettings/questions/managing_organisation.rb @@ -25,10 +25,7 @@ class Form::Lettings::Questions::ManagingOrganisation < ::Form::Question opts[current_user.organisation.id] = "#{current_user.organisation.name} (Your organisation)" end - managing_organisations.select(:id, :name).each_with_object(opts) do |organisation, hsh| - hsh[organisation.id] = organisation.name - hsh - end + opts.merge(managing_organisations_answer_options) end def displayed_answer_options(log, user) @@ -54,8 +51,7 @@ class Form::Lettings::Questions::ManagingOrganisation < ::Form::Question return false unless @current_user return false if @current_user.support? - # Hide when less than 2 managing_agents - managing_organisations.count < 2 + managing_organisations_answer_options.count < 2 end def enabled @@ -68,11 +64,11 @@ private true end - def managing_organisations - @managing_organisations ||= if current_user.support? - log.owning_organisation - else - current_user.organisation - end.managing_agents + def managing_organisations_answer_options + @managing_organisations_answer_options ||= if current_user.support? + log.owning_organisation + else + current_user.organisation + end.managing_agents.pluck(:id, :name).to_h end end diff --git a/config/initializers/feature_toggle.rb b/config/initializers/feature_toggle.rb index e54141368..1a4b8a862 100644 --- a/config/initializers/feature_toggle.rb +++ b/config/initializers/feature_toggle.rb @@ -22,8 +22,6 @@ class FeatureToggle end def self.managing_for_other_user_enabled? - return true unless Rails.env.production? - - false + !Rails.env.production? end end diff --git a/db/seeds.rb b/db/seeds.rb index 9aaaa8c51..9d78fd8cb 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -39,7 +39,7 @@ unless Rails.env.test? provider_type: "LA", ) managing_agent2 = Organisation.find_or_create_by!( - name: "Managing Agent 1", + name: "Managing Agent 2", address_line1: "2 Marsham Street", address_line2: "London", postcode: "SW1P 4DF",