From fb89b47b6f283b996db8aff622e1ea6ecfadd9ad Mon Sep 17 00:00:00 2001 From: Manny Dinssa <44172848+Dinssa@users.noreply.github.com> Date: Mon, 22 Jul 2024 11:07:58 +0100 Subject: [PATCH] CLDC-3520 order scheme locations (#2499) * List in alphabetical order using second line of address (.i.e. hint/name) --- .gitignore | 3 +- 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 ++++++ .../form/lettings/questions/location_id.rb | 5 ++ .../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 | 14 ++++ spec/helpers/question_view_helper_spec.rb | 54 +++++++++++++++- .../lettings/pages/location_search_spec.rb | 19 ++++++ .../form/lettings/pages/location_spec.rb | 1 + .../lettings/questions/location_id_spec.rb | 27 ++++++++ .../form/lettings/subsections/setup_spec.rb | 2 + 15 files changed, 218 insertions(+), 11 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/.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 diff --git a/app/helpers/question_view_helper.rb b/app/helpers/question_view_helper.rb index 01745b66f..acb4fa959 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) - "(S#{resource.id})" + [resource.primary_client_group, resource.secondary_client_group].compact.join(", ") + if resource.instance_of?(Scheme) + "(S#{resource.id}) " + [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 91e645c47..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.rb b/app/models/form/lettings/questions/location_id.rb index d79bcab76..59101592f 100644 --- a/app/models/form/lettings/questions/location_id.rb +++ b/app/models/form/lettings/questions/location_id.rb @@ -31,6 +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 { |_, v| + name = v["hint"].match(/[a-zA-Z].*/).to_s + number = v["hint"].match(/\d+/).to_s.to_i + [name, number] + }.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 new file mode 100644 index 000000000..d085572e4 --- /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) }.to_h + 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..eab05c997 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! diff --git a/spec/helpers/question_view_helper_spec.rb b/spec/helpers/question_view_helper_spec.rb index 2f7e82285..af7282cb3 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", id: 2_736_276) } + + it "returns the primary and secondary client groups" do + expect(answer_option_hint(resource)).to eq("(S2736276) 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 diff --git a/spec/models/form/lettings/questions/location_id_spec.rb b/spec/models/form/lettings/questions/location_id_spec.rb index 103ca3404..ac3cfb8b5 100644 --- a/spec/models/form/lettings/questions/location_id_spec.rb +++ b/spec/models/form/lettings/questions/location_id_spec.rb @@ -147,6 +147,33 @@ 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 + expect(question.displayed_answer_options(lettings_log).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 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