From b894ef7494d0828921dc4f371b9c82ade5812c14 Mon Sep 17 00:00:00 2001 From: Manny Dinssa Date: Fri, 5 Jul 2024 12:30:55 +0100 Subject: [PATCH 01/10] List in alphabetical order using second line of address (.i.e. hint/name) --- app/models/form/lettings/questions/location_id.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/form/lettings/questions/location_id.rb b/app/models/form/lettings/questions/location_id.rb index d79bcab76..371f66a08 100644 --- a/app/models/form/lettings/questions/location_id.rb +++ b/app/models/form/lettings/questions/location_id.rb @@ -30,7 +30,7 @@ class Form::Lettings::Questions::LocationId < ::Form::Question return {} unless lettings_log.scheme scheme_location_ids = lettings_log.scheme.locations.visible.confirmed.pluck(:id) - answer_options.select { |k, _v| scheme_location_ids.include?(k.to_i) } + answer_options.select { |k, _v| scheme_location_ids.include?(k.to_i) }.sort_by { |_k, v| v["hint"] } end def hidden_in_check_answers?(lettings_log, _current_user = nil) From cd1aad495d0f424dc3fb5f6f3ef44fe2800b98f2 Mon Sep 17 00:00:00 2001 From: Manny Dinssa Date: Fri, 5 Jul 2024 12:33:13 +0100 Subject: [PATCH 02/10] Create alternative select view for schemes with 20+ locations --- app/helpers/question_view_helper.rb | 9 ++- .../lettings_log_variables.rb | 7 -- app/models/form/lettings/pages/location.rb | 1 + .../form/lettings/pages/location_search.rb | 20 ++++++ .../lettings/questions/location_id_search.rb | 64 +++++++++++++++++++ app/models/form/lettings/subsections/setup.rb | 1 + app/models/form/question.rb | 2 + app/models/lettings_log.rb | 15 +++++ spec/helpers/question_view_helper_spec.rb | 54 +++++++++++++++- .../lettings/pages/location_search_spec.rb | 19 ++++++ .../form/lettings/pages/location_spec.rb | 1 + 11 files changed, 183 insertions(+), 10 deletions(-) create mode 100644 app/models/form/lettings/pages/location_search.rb create mode 100644 app/models/form/lettings/questions/location_id_search.rb create mode 100644 spec/models/form/lettings/pages/location_search_spec.rb diff --git a/app/helpers/question_view_helper.rb b/app/helpers/question_view_helper.rb index 0c67a9f89..66600a25a 100644 --- a/app/helpers/question_view_helper.rb +++ b/app/helpers/question_view_helper.rb @@ -32,14 +32,19 @@ module QuestionViewHelper end def answer_option_hint(resource) - return unless resource.instance_of?(Scheme) + return unless resource.instance_of?(Scheme) || resource.instance_of?(Location) - [resource.primary_client_group, resource.secondary_client_group].compact.join(", ") + if resource.instance_of?(Scheme) + [resource.primary_client_group, resource.secondary_client_group].compact.join(", ") + elsif resource.instance_of?(Location) + resource.name + end end def select_option_name(value) return value.service_name if value.respond_to?(:service_name) return value["name"] if value.is_a?(Hash) && value["name"].present? + return value["postcode"] if value.is_a?(Location) end private diff --git a/app/models/derived_variables/lettings_log_variables.rb b/app/models/derived_variables/lettings_log_variables.rb index e316d6efd..e50161188 100644 --- a/app/models/derived_variables/lettings_log_variables.rb +++ b/app/models/derived_variables/lettings_log_variables.rb @@ -28,13 +28,6 @@ module DerivedVariables::LettingsLogVariables 5 => 6, }.freeze - def scheme_has_multiple_locations? - return false unless scheme - - @scheme_locations_count ||= scheme.locations.active_in_2_weeks.size - @scheme_locations_count > 1 - end - def set_derived_fields! clear_inapplicable_derived_values! set_encoded_derived_values!(DEPENDENCIES) diff --git a/app/models/form/lettings/pages/location.rb b/app/models/form/lettings/pages/location.rb index 2c040661a..ba357dc1b 100644 --- a/app/models/form/lettings/pages/location.rb +++ b/app/models/form/lettings/pages/location.rb @@ -5,6 +5,7 @@ class Form::Lettings::Pages::Location < ::Form::Page { "needstype" => 2, "scheme_has_multiple_locations?" => true, + "scheme_has_large_number_of_locations?" => false, }, ] @header = "Location" diff --git a/app/models/form/lettings/pages/location_search.rb b/app/models/form/lettings/pages/location_search.rb new file mode 100644 index 000000000..f27fae281 --- /dev/null +++ b/app/models/form/lettings/pages/location_search.rb @@ -0,0 +1,20 @@ +class Form::Lettings::Pages::LocationSearch < ::Form::Page + def initialize(_id, hsh, subsection) + super("location_search", hsh, subsection) + @depends_on = [ + { + "needstype" => 2, + "scheme_has_multiple_locations?" => true, + "scheme_has_large_number_of_locations?" => true, + }, + ] + @header = "Location" + @next_unresolved_page_id = :check_answers + end + + def questions + @questions ||= [ + Form::Lettings::Questions::LocationIdSearch.new(nil, nil, self), + ] + end +end diff --git a/app/models/form/lettings/questions/location_id_search.rb b/app/models/form/lettings/questions/location_id_search.rb new file mode 100644 index 000000000..6098f2d7e --- /dev/null +++ b/app/models/form/lettings/questions/location_id_search.rb @@ -0,0 +1,64 @@ +class Form::Lettings::Questions::LocationIdSearch < ::Form::Question + def initialize(id, hsh, page) + super + @id = "location_id" + @check_answer_label = "Location" + @header = header_text + @hint_text = '
This scheme has 20 or more locations.
Enter postcode or address.' + @type = "select" + @answer_options = answer_options + @inferred_answers = { + "location.name": { + "needstype": 2, + }, + } + @question_number = QUESTION_NUMBER_FROM_YEAR[form.start_date.year] || QUESTION_NUMBER_FROM_YEAR[QUESTION_NUMBER_FROM_YEAR.keys.max] if form.start_date.present? + @disable_clearing_if_not_routed_or_dynamic_answer_options = true + @top_guidance_partial = "finding_location" + end + + def answer_options + answer_opts = {} + return answer_opts unless ActiveRecord::Base.connected? + + Location.visible.started_in_2_weeks.select(:id, :postcode, :name).each_with_object(answer_opts) do |location, hsh| + hsh[location.id.to_s] = location + hsh + end + end + + def displayed_answer_options(lettings_log, _user = nil) + return {} unless lettings_log.scheme + + scheme_location_ids = lettings_log.scheme.locations.visible.confirmed.pluck(:id) + answer_options.select { |k, _v| scheme_location_ids.include?(k.to_i) } + end + + def hidden_in_check_answers?(lettings_log, _current_user = nil) + !supported_housing_selected?(lettings_log) + end + + def get_extra_check_answer_value(lettings_log) + lettings_log.form.get_question("la", nil).label_from_value(lettings_log.la) + end + +private + + def supported_housing_selected?(lettings_log) + lettings_log.needstype == 2 + end + + def selected_answer_option_is_derived?(_lettings_log) + false + end + + def header_text + if form.start_date && form.start_date.year >= 2023 + "Which location is this letting for?" + else + "Which location is this log for?" + end + end + + QUESTION_NUMBER_FROM_YEAR = { 2023 => 10, 2024 => 5 }.freeze +end diff --git a/app/models/form/lettings/subsections/setup.rb b/app/models/form/lettings/subsections/setup.rb index 85af7d2c4..1970149ed 100644 --- a/app/models/form/lettings/subsections/setup.rb +++ b/app/models/form/lettings/subsections/setup.rb @@ -14,6 +14,7 @@ class Form::Lettings::Subsections::Setup < ::Form::Subsection Form::Lettings::Pages::NeedsType.new(nil, nil, self), Form::Lettings::Pages::Scheme.new(nil, nil, self), Form::Lettings::Pages::Location.new(nil, nil, self), + Form::Lettings::Pages::LocationSearch.new(nil, nil, self), Form::Lettings::Pages::Renewal.new(nil, nil, self), Form::Lettings::Pages::TenancyStartDate.new(nil, nil, self), Form::Lettings::Pages::RentType.new(nil, nil, self), diff --git a/app/models/form/question.rb b/app/models/form/question.rb index aa019c1c6..a692574dd 100644 --- a/app/models/form/question.rb +++ b/app/models/form/question.rb @@ -160,6 +160,8 @@ class Form::Question when "select" if answer_options[value.to_s].respond_to?(:service_name) answer_options[value.to_s].service_name + elsif answer_options[value.to_s].is_a?(Location) + answer_options[value.to_s].postcode else answer_options[value.to_s] end diff --git a/app/models/lettings_log.rb b/app/models/lettings_log.rb index 60ac5e166..19c62576d 100644 --- a/app/models/lettings_log.rb +++ b/app/models/lettings_log.rb @@ -706,6 +706,20 @@ class LettingsLog < Log self.la = inferred_la if inferred_la.present? end + def scheme_has_multiple_locations? + return false unless scheme + + @scheme_locations_count ||= scheme.locations.active_in_2_weeks.size + @scheme_locations_count > 1 + end + + def scheme_has_large_number_of_locations? + return false unless scheme + + @scheme_locations_count ||= scheme.locations.active_in_2_weeks.size + @scheme_locations_count > 19 + end + private def reset_invalid_unresolved_log_fields! @@ -871,4 +885,5 @@ private uprn_selection_changed? || startdate_changed? end end + end diff --git a/spec/helpers/question_view_helper_spec.rb b/spec/helpers/question_view_helper_spec.rb index 2f7e82285..3ccaa57df 100644 --- a/spec/helpers/question_view_helper_spec.rb +++ b/spec/helpers/question_view_helper_spec.rb @@ -32,7 +32,7 @@ RSpec.describe QuestionViewHelper do end end - context "when viewig a question without a caption" do + context "when viewing a question without a caption" do let(:caption_text) { nil } it "returns nil" do @@ -114,4 +114,56 @@ RSpec.describe QuestionViewHelper do end end end + + describe "select_option_name" do + context "when value is a location" do + let(:value) { build(:location)} + + it "returns the location's postcode" do + expect(select_option_name(value)).to eq(value.postcode) + end + end + + context "when value is a hash with a name key" do + let(:value) { { "name" => "example name" } } + + it "returns the value of the name key" do + expect(select_option_name(value)).to eq(value["name"]) + end + end + + context "when value responds to service_name" do + let(:value) { build(:scheme)} + + it "returns the value of the service_name method" do + expect(select_option_name(value)).to eq(value.service_name) + end + end + end + + describe "answer_option_hint" do + context "when not a scheme or location" do + let(:resource) { { "value" => "not a scheme or location" }} + + it "returns nil" do + expect(answer_option_hint(resource)).to be_nil + end + end + + context "when resource is a scheme" do + let(:resource) { build(:scheme, primary_client_group: "O", secondary_client_group: "E") } + + it "returns the primary and secondary client groups" do + expect(answer_option_hint(resource)).to eq("Homeless families with support needs, People with mental health problems") + end + end + + context "when resource is a location" do + let(:resource) { build(:location) } + + it "returns the location's name" do + expect(answer_option_hint(resource)).to eq(resource.name) + end + end + end end diff --git a/spec/models/form/lettings/pages/location_search_spec.rb b/spec/models/form/lettings/pages/location_search_spec.rb new file mode 100644 index 000000000..c4362e782 --- /dev/null +++ b/spec/models/form/lettings/pages/location_search_spec.rb @@ -0,0 +1,19 @@ +require "rails_helper" + +RSpec.describe Form::Lettings::Pages::LocationSearch, type: :model do + subject(:page) { described_class.new(page_id, page_definition, subsection) } + + let(:page_id) { nil } + let(:page_definition) { nil } + let(:subsection) { instance_double(Form::Subsection) } + + it "has the correct depends_on" do + expect(page.depends_on).to eq([ + { + "needstype" => 2, + "scheme_has_multiple_locations?" => true, + "scheme_has_large_number_of_locations?" => true, + }, + ]) + end +end diff --git a/spec/models/form/lettings/pages/location_spec.rb b/spec/models/form/lettings/pages/location_spec.rb index 10e8fdf09..6de0b59dd 100644 --- a/spec/models/form/lettings/pages/location_spec.rb +++ b/spec/models/form/lettings/pages/location_spec.rb @@ -38,6 +38,7 @@ RSpec.describe Form::Lettings::Pages::Location, type: :model do { "needstype" => 2, "scheme_has_multiple_locations?" => true, + "scheme_has_large_number_of_locations?" => false, }, ]) end From c0d00207f6756811774b0e391e07474049684ffc Mon Sep 17 00:00:00 2001 From: Manny Dinssa Date: Fri, 5 Jul 2024 12:33:25 +0100 Subject: [PATCH 03/10] Add to git ignore vs code --- .gitignore | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index b2b589114..ddda1de2c 100644 --- a/.gitignore +++ b/.gitignore @@ -53,8 +53,9 @@ yarn-debug.log* .DS_Store .generators .rakeTasks +.vscode /app/assets/builds/* !/app/assets/builds/.keep -spec/examples.txt +spec/examples.txt \ No newline at end of file From bd135d6a5d7c536b059ca61b583a772802f679b1 Mon Sep 17 00:00:00 2001 From: Manny Dinssa Date: Mon, 8 Jul 2024 10:20:25 +0100 Subject: [PATCH 04/10] Sort by alphabetical section first, then by numerical --- app/models/form/lettings/questions/location_id.rb | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/app/models/form/lettings/questions/location_id.rb b/app/models/form/lettings/questions/location_id.rb index 371f66a08..4ada2f27a 100644 --- a/app/models/form/lettings/questions/location_id.rb +++ b/app/models/form/lettings/questions/location_id.rb @@ -30,7 +30,12 @@ class Form::Lettings::Questions::LocationId < ::Form::Question return {} unless lettings_log.scheme scheme_location_ids = lettings_log.scheme.locations.visible.confirmed.pluck(:id) - answer_options.select { |k, _v| scheme_location_ids.include?(k.to_i) }.sort_by { |_k, v| v["hint"] } + answer_options.select { |k, _v| scheme_location_ids.include?(k.to_i) } + .sort_by do |_, v| + name = v["hint"].match(/[a-zA-Z].*/).to_s + number = v["hint"].match(/\d+/).to_s.to_i + [name, number] + end end def hidden_in_check_answers?(lettings_log, _current_user = nil) From 2d6c41e78cc216223b1d71b54d516bcf3c8e104b Mon Sep 17 00:00:00 2001 From: Manny Dinssa Date: Mon, 8 Jul 2024 10:35:26 +0100 Subject: [PATCH 05/10] Correct syntax offences --- app/models/lettings_log.rb | 1 - spec/helpers/question_view_helper_spec.rb | 6 +++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/app/models/lettings_log.rb b/app/models/lettings_log.rb index 19c62576d..82f04512d 100644 --- a/app/models/lettings_log.rb +++ b/app/models/lettings_log.rb @@ -885,5 +885,4 @@ private uprn_selection_changed? || startdate_changed? end end - end diff --git a/spec/helpers/question_view_helper_spec.rb b/spec/helpers/question_view_helper_spec.rb index 3ccaa57df..cf45a94be 100644 --- a/spec/helpers/question_view_helper_spec.rb +++ b/spec/helpers/question_view_helper_spec.rb @@ -117,7 +117,7 @@ RSpec.describe QuestionViewHelper do describe "select_option_name" do context "when value is a location" do - let(:value) { build(:location)} + let(:value) { build(:location) } it "returns the location's postcode" do expect(select_option_name(value)).to eq(value.postcode) @@ -133,7 +133,7 @@ RSpec.describe QuestionViewHelper do end context "when value responds to service_name" do - let(:value) { build(:scheme)} + let(:value) { build(:scheme) } it "returns the value of the service_name method" do expect(select_option_name(value)).to eq(value.service_name) @@ -143,7 +143,7 @@ RSpec.describe QuestionViewHelper do describe "answer_option_hint" do context "when not a scheme or location" do - let(:resource) { { "value" => "not a scheme or location" }} + let(:resource) { { "value" => "not a scheme or location" } } it "returns nil" do expect(answer_option_hint(resource)).to be_nil From c74c842dfa936766bbd4fa36327bbebfba0b51d4 Mon Sep 17 00:00:00 2001 From: Manny Dinssa Date: Mon, 8 Jul 2024 10:54:17 +0100 Subject: [PATCH 06/10] Fix failing tests expecting an empty hash --- app/models/form/lettings/questions/location_id.rb | 2 +- app/models/form/lettings/questions/location_id_search.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/form/lettings/questions/location_id.rb b/app/models/form/lettings/questions/location_id.rb index 4ada2f27a..d462c83f7 100644 --- a/app/models/form/lettings/questions/location_id.rb +++ b/app/models/form/lettings/questions/location_id.rb @@ -35,7 +35,7 @@ class Form::Lettings::Questions::LocationId < ::Form::Question name = v["hint"].match(/[a-zA-Z].*/).to_s number = v["hint"].match(/\d+/).to_s.to_i [name, number] - end + end.to_h end def hidden_in_check_answers?(lettings_log, _current_user = nil) diff --git a/app/models/form/lettings/questions/location_id_search.rb b/app/models/form/lettings/questions/location_id_search.rb index 6098f2d7e..d085572e4 100644 --- a/app/models/form/lettings/questions/location_id_search.rb +++ b/app/models/form/lettings/questions/location_id_search.rb @@ -31,7 +31,7 @@ class Form::Lettings::Questions::LocationIdSearch < ::Form::Question return {} unless lettings_log.scheme scheme_location_ids = lettings_log.scheme.locations.visible.confirmed.pluck(:id) - answer_options.select { |k, _v| scheme_location_ids.include?(k.to_i) } + answer_options.select { |k, _v| scheme_location_ids.include?(k.to_i) }.to_h end def hidden_in_check_answers?(lettings_log, _current_user = nil) From c1bad2bc7ea9fecf1f2d8307f20b69d6596255e1 Mon Sep 17 00:00:00 2001 From: Manny Dinssa Date: Mon, 8 Jul 2024 10:58:53 +0100 Subject: [PATCH 07/10] Remove do...end block, Lint offence --- app/models/form/lettings/questions/location_id.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/form/lettings/questions/location_id.rb b/app/models/form/lettings/questions/location_id.rb index d462c83f7..59101592f 100644 --- a/app/models/form/lettings/questions/location_id.rb +++ b/app/models/form/lettings/questions/location_id.rb @@ -31,11 +31,11 @@ class Form::Lettings::Questions::LocationId < ::Form::Question scheme_location_ids = lettings_log.scheme.locations.visible.confirmed.pluck(:id) answer_options.select { |k, _v| scheme_location_ids.include?(k.to_i) } - .sort_by do |_, v| + .sort_by { |_, v| name = v["hint"].match(/[a-zA-Z].*/).to_s number = v["hint"].match(/\d+/).to_s.to_i [name, number] - end.to_h + }.to_h end def hidden_in_check_answers?(lettings_log, _current_user = nil) From d486e2f4e2c23dfb85cb66babf4f74e361c07e88 Mon Sep 17 00:00:00 2001 From: Manny Dinssa Date: Mon, 8 Jul 2024 12:45:15 +0100 Subject: [PATCH 08/10] Create test --- .../lettings/questions/location_id_spec.rb | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/spec/models/form/lettings/questions/location_id_spec.rb b/spec/models/form/lettings/questions/location_id_spec.rb index 103ca3404..55452f05c 100644 --- a/spec/models/form/lettings/questions/location_id_spec.rb +++ b/spec/models/form/lettings/questions/location_id_spec.rb @@ -147,6 +147,34 @@ RSpec.describe Form::Lettings::Questions::LocationId, type: :model do expect(question.displayed_answer_options(lettings_log).count).to eq(1) end end + + context "and some locations start with numbers" do + before do + FactoryBot.create(:location, scheme:, startdate: Time.utc(2022, 5, 5), name: "2 Abe Road") + FactoryBot.create(:location, scheme:, startdate: Time.utc(2022, 5, 6), name: "1 Abe Road") + FactoryBot.create(:location, scheme:, startdate: Time.utc(2022, 5, 7), name: "1 Lake Lane") + FactoryBot.create(:location, scheme:, startdate: Time.utc(2022, 5, 8), name: "3 Abe Road") + FactoryBot.create(:location, scheme:, startdate: Time.utc(2022, 5, 9), name: "2 Lake Lane") + FactoryBot.create(:location, scheme:, startdate: Time.utc(2022, 5, 10), name: "Smith Avenue") + FactoryBot.create(:location, scheme:, startdate: Time.utc(2022, 5, 11), name: "Abacus Road") + FactoryBot.create(:location, scheme:, startdate: Time.utc(2022, 5, 12), name: "Hawthorne Road") + lettings_log.update!(scheme:) + end + + it "orders the locations by name then numerically" do + displayed_locations = question.displayed_answer_options(lettings_log) + expect(displayed_locations.values.map { |v| v["hint"] }).to eq([ + "Abacus Road", + "1 Abe Road", + "2 Abe Road", + "3 Abe Road", + "Hawthorne Road", + "1 Lake Lane", + "2 Lake Lane", + "Smith Avenue", + ]) + end + end end end From ad24928738be432fad3dab94e05f0e441fa755a1 Mon Sep 17 00:00:00 2001 From: Manny Dinssa Date: Mon, 8 Jul 2024 12:50:32 +0100 Subject: [PATCH 09/10] Condensed test --- spec/models/form/lettings/questions/location_id_spec.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/spec/models/form/lettings/questions/location_id_spec.rb b/spec/models/form/lettings/questions/location_id_spec.rb index 55452f05c..ac3cfb8b5 100644 --- a/spec/models/form/lettings/questions/location_id_spec.rb +++ b/spec/models/form/lettings/questions/location_id_spec.rb @@ -162,8 +162,7 @@ RSpec.describe Form::Lettings::Questions::LocationId, type: :model do end it "orders the locations by name then numerically" do - displayed_locations = question.displayed_answer_options(lettings_log) - expect(displayed_locations.values.map { |v| v["hint"] }).to eq([ + expect(question.displayed_answer_options(lettings_log).values.map { |v| v["hint"] }).to eq([ "Abacus Road", "1 Abe Road", "2 Abe Road", From a65f3fb2b27c79427b2d5385ecd658c5c7455480 Mon Sep 17 00:00:00 2001 From: Manny Dinssa Date: Mon, 8 Jul 2024 16:53:58 +0100 Subject: [PATCH 10/10] Add additional page to setup test --- spec/models/form/lettings/subsections/setup_spec.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/spec/models/form/lettings/subsections/setup_spec.rb b/spec/models/form/lettings/subsections/setup_spec.rb index 824f4bf80..074f7ae5e 100644 --- a/spec/models/form/lettings/subsections/setup_spec.rb +++ b/spec/models/form/lettings/subsections/setup_spec.rb @@ -30,6 +30,7 @@ RSpec.describe Form::Lettings::Subsections::Setup, type: :model do needs_type scheme location + location_search renewal tenancy_start_date rent_type @@ -54,6 +55,7 @@ RSpec.describe Form::Lettings::Subsections::Setup, type: :model do needs_type scheme location + location_search renewal tenancy_start_date rent_type