From bbe7054f1eb2e48195fc1ae98f5e578868a7eb9a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Meny?= Date: Fri, 1 Jul 2022 10:51:14 +0100 Subject: [PATCH] Fix tests and reference issues --- app/models/derived_variables/case_log_variables.rb | 6 ++++++ app/models/form/setup/pages/location.rb | 8 ++------ .../questions/{location.rb => location_id.rb} | 14 ++++++-------- app/models/form/setup/questions/scheme_id.rb | 5 +---- spec/models/form/setup/pages/location_spec.rb | 7 ++----- .../{location_spec.rb => location_id_spec.rb} | 4 ++-- spec/models/form/setup/questions/scheme_id_spec.rb | 2 +- spec/models/form_spec.rb | 4 ++-- 8 files changed, 22 insertions(+), 28 deletions(-) rename app/models/form/setup/questions/{location.rb => location_id.rb} (63%) rename spec/models/form/setup/questions/{location_spec.rb => location_id_spec.rb} (87%) diff --git a/app/models/derived_variables/case_log_variables.rb b/app/models/derived_variables/case_log_variables.rb index 16fff3fd0..831d3c7ce 100644 --- a/app/models/derived_variables/case_log_variables.rb +++ b/app/models/derived_variables/case_log_variables.rb @@ -5,6 +5,12 @@ module DerivedVariables::CaseLogVariables FeatureToggle.supported_housing_schemes_enabled? end + def scheme_has_multiple_locations? + return false unless scheme + + scheme.locations.size > 1 + end + def set_derived_fields! # TODO: Remove once we support parent/child relationships self.managing_organisation_id ||= owning_organisation_id diff --git a/app/models/form/setup/pages/location.rb b/app/models/form/setup/pages/location.rb index f61d86736..76d90a292 100644 --- a/app/models/form/setup/pages/location.rb +++ b/app/models/form/setup/pages/location.rb @@ -4,21 +4,17 @@ class Form::Setup::Pages::Location < ::Form::Page @header = "" @description = "" @questions = questions - # Only display if there is more than one location @depends_on = [{ "supported_housing_schemes_enabled?" => true, "needstype" => 2, - "scheme.locations.size" => { - "operator" => ">", - "operand" => 1, - }, + "scheme_has_multiple_locations?" => true, }] @derived = true end def questions [ - Form::Setup::Questions::Location.new(nil, nil, self), + Form::Setup::Questions::LocationId.new(nil, nil, self), ] end end diff --git a/app/models/form/setup/questions/location.rb b/app/models/form/setup/questions/location_id.rb similarity index 63% rename from app/models/form/setup/questions/location.rb rename to app/models/form/setup/questions/location_id.rb index 5e669254a..dac8f489b 100644 --- a/app/models/form/setup/questions/location.rb +++ b/app/models/form/setup/questions/location_id.rb @@ -1,13 +1,11 @@ -class Form::Setup::Questions::Location < ::Form::Question - def initialize(id, hsh, page) - super - @id = "location" +class Form::Setup::Questions::LocationId < ::Form::Question + def initialize(_id, hsh, page) + super("location_id", hsh, page) @check_answer_label = "Location" @header = "Which location is this log for?" @hint_text = "" @type = "radio" @derived = true unless FeatureToggle.supported_housing_schemes_enabled? - @page = page @answer_options = answer_options end @@ -15,8 +13,8 @@ class Form::Setup::Questions::Location < ::Form::Question answer_opts = {} return answer_opts unless ActiveRecord::Base.connected? - Location.select(:id, :postcode).each_with_object(answer_opts) do |location, hsh| - hsh[location.id] = location.postcode + Location.select(:id, :postcode, :address_line1).each_with_object(answer_opts) do |location, hsh| + hsh[location.id.to_s] = { "value" => location.postcode, "hint" => location.address_line1 } hsh end end @@ -24,7 +22,7 @@ class Form::Setup::Questions::Location < ::Form::Question def displayed_answer_options(case_log) return {} unless case_log.scheme - scheme_location_ids = Location.where("scheme_id = #{case_log.scheme.id}").map(&:id) + scheme_location_ids = Location.where(scheme_id: case_log.scheme.id).map(&:id).map(&:to_s) answer_options.select { |k, _v| scheme_location_ids.include?(k) } end diff --git a/app/models/form/setup/questions/scheme_id.rb b/app/models/form/setup/questions/scheme_id.rb index 65c259663..973d2931c 100644 --- a/app/models/form/setup/questions/scheme_id.rb +++ b/app/models/form/setup/questions/scheme_id.rb @@ -6,6 +6,7 @@ class Form::Setup::Questions::SchemeId < ::Form::Question @hint_text = "Enter scheme name or postcode" @type = "select" @answer_options = answer_options + @derived = true unless FeatureToggle.supported_housing_schemes_enabled? end def answer_options @@ -31,10 +32,6 @@ class Form::Setup::Questions::SchemeId < ::Form::Question private - def selected_answer_option_is_derived?(_case_log) - false - end - def supported_housing_selected?(case_log) case_log.needstype == 2 end diff --git a/spec/models/form/setup/pages/location_spec.rb b/spec/models/form/setup/pages/location_spec.rb index c72effb75..b3c1d06a9 100644 --- a/spec/models/form/setup/pages/location_spec.rb +++ b/spec/models/form/setup/pages/location_spec.rb @@ -12,7 +12,7 @@ RSpec.describe Form::Setup::Pages::Location, type: :model do end it "has correct questions" do - expect(page.questions.map(&:id)).to eq(%w[location]) + expect(page.questions.map(&:id)).to eq(%w[location_id]) end it "has the correct id" do @@ -31,10 +31,7 @@ RSpec.describe Form::Setup::Pages::Location, type: :model do expect(page.depends_on).to eq([{ "supported_housing_schemes_enabled?" => true, "needstype" => 2, - "scheme.locations.size" => { - "operator" => ">", - "operand" => 1, - }, + "scheme_has_multiple_locations?" => true, }]) end end diff --git a/spec/models/form/setup/questions/location_spec.rb b/spec/models/form/setup/questions/location_id_spec.rb similarity index 87% rename from spec/models/form/setup/questions/location_spec.rb rename to spec/models/form/setup/questions/location_id_spec.rb index 988a1b53d..dd1bd9735 100644 --- a/spec/models/form/setup/questions/location_spec.rb +++ b/spec/models/form/setup/questions/location_id_spec.rb @@ -1,6 +1,6 @@ require "rails_helper" -RSpec.describe Form::Setup::Questions::Location, type: :model do +RSpec.describe Form::Setup::Questions::LocationId, type: :model do subject(:question) { described_class.new(question_id, question_definition, page) } let(:question_id) { nil } @@ -12,7 +12,7 @@ RSpec.describe Form::Setup::Questions::Location, type: :model do end it "has the correct id" do - expect(question.id).to eq("location") + expect(question.id).to eq("location_id") end it "has the correct header" do diff --git a/spec/models/form/setup/questions/scheme_id_spec.rb b/spec/models/form/setup/questions/scheme_id_spec.rb index d71d8216b..2a71fb2a5 100644 --- a/spec/models/form/setup/questions/scheme_id_spec.rb +++ b/spec/models/form/setup/questions/scheme_id_spec.rb @@ -51,7 +51,7 @@ RSpec.describe Form::Setup::Questions::SchemeId, type: :model do end it "has the correct answer_options based on the schemes the user's organisation owns or manages" do - expected_answer = { scheme.id => scheme.service_name } + expected_answer = { scheme.id.to_s => scheme.service_name } expect(question.displayed_answer_options(case_log)).to eq(expected_answer) end end diff --git a/spec/models/form_spec.rb b/spec/models/form_spec.rb index 8d363ed12..b1870d709 100644 --- a/spec/models/form_spec.rb +++ b/spec/models/form_spec.rb @@ -180,7 +180,7 @@ RSpec.describe Form, type: :model do let(:case_log) { FactoryBot.create(:case_log, :in_progress, needstype: 1) } context "when dependencies are not met" do - let(:expected_invalid) { %w[scheme_id location condition_effects cbl conditional_question_no_second_question net_income_value_check dependent_question offered layear declaration] } + let(:expected_invalid) { %w[scheme_id location_id condition_effects cbl conditional_question_no_second_question net_income_value_check dependent_question offered layear declaration] } it "returns an array of question keys whose pages conditions are not met" do expect(form.invalidated_page_questions(case_log).map(&:id).uniq).to eq(expected_invalid) @@ -188,7 +188,7 @@ RSpec.describe Form, type: :model do end context "with two pages having the same question and only one has dependencies met" do - let(:expected_invalid) { %w[scheme_id location condition_effects cbl conditional_question_no_second_question net_income_value_check dependent_question offered layear declaration] } + let(:expected_invalid) { %w[scheme_id location_id condition_effects cbl conditional_question_no_second_question net_income_value_check dependent_question offered layear declaration] } it "returns an array of question keys whose pages conditions are not met" do case_log["preg_occ"] = "No"