From 9881fb990aa374fd9d80ff750b2fb17f6308acb1 Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire <94526761+natdeanlewissoftwire@users.noreply.github.com> Date: Thu, 21 Mar 2024 15:18:02 +0000 Subject: [PATCH 1/7] CLDC-3339 Improve select correct address bulk upload error (#2331) * feat: add unable to find address errors when some options are returned if they are low confidence * feat: update copy * feat: add tests --- .../bulk_upload/lettings/year2024/row_parser.rb | 2 +- .../bulk_upload/sales/year2024/row_parser.rb | 2 +- config/locales/en.yml | 2 +- .../lettings/year2024/row_parser_spec.rb | 17 +++++++++++++++++ .../sales/year2024/row_parser_spec.rb | 17 +++++++++++++++++ 5 files changed, 37 insertions(+), 3 deletions(-) diff --git a/app/services/bulk_upload/lettings/year2024/row_parser.rb b/app/services/bulk_upload/lettings/year2024/row_parser.rb index b0802df12..0ad096c2a 100644 --- a/app/services/bulk_upload/lettings/year2024/row_parser.rb +++ b/app/services/bulk_upload/lettings/year2024/row_parser.rb @@ -574,7 +574,7 @@ private end def validate_address_option_found - if !log.address_options_present? && field_16.blank? && (field_17.present? || field_19.present?) + if log.uprn_selection.nil? && field_16.blank? && (field_17.present? || field_19.present?) %i[field_17 field_18 field_19 field_20 field_21 field_22].each do |field| errors.add(field, I18n.t("validations.no_address_found")) end diff --git a/app/services/bulk_upload/sales/year2024/row_parser.rb b/app/services/bulk_upload/sales/year2024/row_parser.rb index be6a9ca63..e5f4f1d61 100644 --- a/app/services/bulk_upload/sales/year2024/row_parser.rb +++ b/app/services/bulk_upload/sales/year2024/row_parser.rb @@ -602,7 +602,7 @@ private end def validate_address_option_found - if !log.address_options_present? && field_22.blank? && (field_23.present? || field_25.present?) + if log.uprn_selection.nil? && field_22.blank? && (field_23.present? || field_25.present?) %i[field_23 field_24 field_25 field_26 field_27 field_28].each do |field| errors.add(field, I18n.t("validations.no_address_found")) end diff --git a/config/locales/en.yml b/config/locales/en.yml index 2f3e63020..e2934273f 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -213,7 +213,7 @@ en: not_answered: "You must answer %{question}" invalid_option: "Enter a valid value for %{question}" invalid_number: "Enter a number for %{question}" - no_address_found: "We could not find this address. Edit the address data or fix this error on the CORE site." + no_address_found: "We could not find this address. Check the address data in your CSV file is correct and complete, or select the correct address using the CORE site." other_field_missing: "If %{main_field_label} is other then %{other_field_label} must be provided" other_field_not_required: "%{other_field_label} must not be provided if %{main_field_label} was not other" diff --git a/spec/services/bulk_upload/lettings/year2024/row_parser_spec.rb b/spec/services/bulk_upload/lettings/year2024/row_parser_spec.rb index 939d90297..c19dbcbc0 100644 --- a/spec/services/bulk_upload/lettings/year2024/row_parser_spec.rb +++ b/spec/services/bulk_upload/lettings/year2024/row_parser_spec.rb @@ -1521,6 +1521,23 @@ RSpec.describe BulkUpload::Lettings::Year2024::RowParser do end end + describe "address fields" do + context "when no address can be found" do + before do + stub_request(:get, /api\.os\.uk\/search\/places\/v1\/find/) + .to_return(status: 200, body: nil, headers: {}) + end + + let(:attributes) { setup_section_params.merge({ field_16: nil, field_17: "address line 1", field_19: "town or city" }) } + + it "adds an appropriate error" do + %i[field_17 field_18 field_19 field_20 field_21 field_22].each do |field| + expect(parser.errors[field]).to eql(["We could not find this address. Check the address data in your CSV file is correct and complete, or select the correct address using the CORE site."]) + end + end + end + end + describe "#field_25" do # unitletas context "when no longer a valid option from previous year" do let(:attributes) { setup_section_params.merge({ field_25: "4" }) } diff --git a/spec/services/bulk_upload/sales/year2024/row_parser_spec.rb b/spec/services/bulk_upload/sales/year2024/row_parser_spec.rb index e68e95f06..d80aca533 100644 --- a/spec/services/bulk_upload/sales/year2024/row_parser_spec.rb +++ b/spec/services/bulk_upload/sales/year2024/row_parser_spec.rb @@ -886,6 +886,23 @@ RSpec.describe BulkUpload::Sales::Year2024::RowParser do end end + describe "address fields" do + context "when no address can be found" do + before do + stub_request(:get, /api\.os\.uk\/search\/places\/v1\/find/) + .to_return(status: 200, body: nil, headers: {}) + end + + let(:attributes) { setup_section_params.merge({ field_22: nil, field_23: "address line 1", field_25: "town or city" }) } + + it "adds an appropriate error" do + %i[field_23 field_24 field_25 field_26 field_27 field_28].each do |field| + expect(parser.errors[field]).to eql(["We could not find this address. Check the address data in your CSV file is correct and complete, or select the correct address using the CORE site."]) + end + end + end + end + describe "#field_18" do # data protection let(:attributes) { setup_section_params.merge({ field_18: nil }) } From 9881bc7c0ddd4b012aabf3fe0db0049e8b77b926 Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire <94526761+natdeanlewissoftwire@users.noreply.github.com> Date: Fri, 22 Mar 2024 09:58:10 +0000 Subject: [PATCH 2/7] feat: add fields to xml for 24/25 on (#2329) --- .../exports/lettings_log_export_constants.rb | 12 ++++++++++++ spec/fixtures/exports/general_needs_log_24_25.xml | 6 ++++++ 2 files changed, 18 insertions(+) diff --git a/app/services/exports/lettings_log_export_constants.rb b/app/services/exports/lettings_log_export_constants.rb index 0e6edd139..3a04c7fab 100644 --- a/app/services/exports/lettings_log_export_constants.rb +++ b/app/services/exports/lettings_log_export_constants.rb @@ -148,6 +148,12 @@ module Exports::LettingsLogExportConstants "county_as_entered", "postcode_full_as_entered", "la_as_entered", + "net_income_value_check", + "rent_value_check", + "scharge_value_check", + "pscharge_value_check", + "supcharg_value_check", + "carehome_charges_value_check", ] (1..8).each do |index| @@ -175,6 +181,12 @@ module Exports::LettingsLogExportConstants "county_as_entered", "postcode_full_as_entered", "la_as_entered", + "net_income_value_check", + "rent_value_check", + "scharge_value_check", + "pscharge_value_check", + "supcharg_value_check", + "carehome_charges_value_check", ] PRE_2024_EXPORT_FIELDS = Set[ diff --git a/spec/fixtures/exports/general_needs_log_24_25.xml b/spec/fixtures/exports/general_needs_log_24_25.xml index 3b27d50af..bcdb8941c 100644 --- a/spec/fixtures/exports/general_needs_log_24_25.xml +++ b/spec/fixtures/exports/general_needs_log_24_25.xml @@ -78,6 +78,7 @@ 0 0 + 4 123 @@ -127,6 +128,7 @@ + 2 3 @@ -142,8 +144,12 @@ London + 2 + + + 2 From cdfca1c023ddf06d75557ae5ed1be444d6810206 Mon Sep 17 00:00:00 2001 From: Robert Sullivan Date: Fri, 22 Mar 2024 12:27:05 +0000 Subject: [PATCH 3/7] CLDC-3336: Add hint text to Q77 for renewals (#2340) --- app/models/form/lettings/questions/previous_tenure.rb | 2 +- app/models/form/lettings/questions/previous_tenure_renewal.rb | 3 ++- .../form/lettings/questions/previous_tenure_renewal_spec.rb | 3 ++- spec/models/form/lettings/questions/previous_tenure_spec.rb | 2 +- 4 files changed, 6 insertions(+), 4 deletions(-) diff --git a/app/models/form/lettings/questions/previous_tenure.rb b/app/models/form/lettings/questions/previous_tenure.rb index d262f4fda..a9ed5202a 100644 --- a/app/models/form/lettings/questions/previous_tenure.rb +++ b/app/models/form/lettings/questions/previous_tenure.rb @@ -6,7 +6,7 @@ class Form::Lettings::Questions::PreviousTenure < ::Form::Question @header = "Where was the household immediately before this letting?" @type = "radio" @check_answers_card_number = 0 - @hint_text = "This is where the household was the night before they moved." + @hint_text = "This is where the household was the night before they moved into this new let." @answer_options = ANSWER_OPTIONS @question_number = QUESTION_NUMBER_FROM_YEAR[form.start_date.year] || QUESTION_NUMBER_FROM_YEAR[QUESTION_NUMBER_FROM_YEAR.keys.max] end diff --git a/app/models/form/lettings/questions/previous_tenure_renewal.rb b/app/models/form/lettings/questions/previous_tenure_renewal.rb index 572de3fd4..d4849d3c2 100644 --- a/app/models/form/lettings/questions/previous_tenure_renewal.rb +++ b/app/models/form/lettings/questions/previous_tenure_renewal.rb @@ -6,7 +6,8 @@ class Form::Lettings::Questions::PreviousTenureRenewal < ::Form::Question @header = "Where was the household immediately before this letting?" @type = "radio" @check_answers_card_number = 0 - @hint_text = "" + @hint_text = "You told us this letting is a renewal. We have removed some options because of this.

+ This is where the household was the night before they moved into this new let." @answer_options = ANSWER_OPTIONS @question_number = QUESTION_NUMBER_FROM_YEAR[form.start_date.year] || QUESTION_NUMBER_FROM_YEAR[QUESTION_NUMBER_FROM_YEAR.keys.max] end diff --git a/spec/models/form/lettings/questions/previous_tenure_renewal_spec.rb b/spec/models/form/lettings/questions/previous_tenure_renewal_spec.rb index 624229dda..8f7325334 100644 --- a/spec/models/form/lettings/questions/previous_tenure_renewal_spec.rb +++ b/spec/models/form/lettings/questions/previous_tenure_renewal_spec.rb @@ -30,7 +30,8 @@ RSpec.describe Form::Lettings::Questions::PreviousTenureRenewal, type: :model do end it "has the correct hint" do - expect(question.hint_text).to eq("") + expect(question.hint_text).to eq("You told us this letting is a renewal. We have removed some options because of this.

+ This is where the household was the night before they moved into this new let.") end it "has the correct answer_options" do diff --git a/spec/models/form/lettings/questions/previous_tenure_spec.rb b/spec/models/form/lettings/questions/previous_tenure_spec.rb index c3a598889..2b46b4499 100644 --- a/spec/models/form/lettings/questions/previous_tenure_spec.rb +++ b/spec/models/form/lettings/questions/previous_tenure_spec.rb @@ -30,7 +30,7 @@ RSpec.describe Form::Lettings::Questions::PreviousTenure, type: :model do end it "has the correct hint" do - expect(question.hint_text).to eq("This is where the household was the night before they moved.") + expect(question.hint_text).to eq("This is where the household was the night before they moved into this new let.") end it "has the correct answer_options" do From 845065d2d3b01e154b28bb42c17574bd91c3491c Mon Sep 17 00:00:00 2001 From: Robert Sullivan Date: Fri, 22 Mar 2024 16:00:56 +0000 Subject: [PATCH 4/7] CLDC-3297: Add assigned to field to xml exports (#2345) * feat: add fields to xml for 24/25 on * CLDC-3297: Add empty assigned to column to XML exports --------- Co-authored-by: natdeanlewissoftwire --- app/services/exports/lettings_log_export_constants.rb | 2 ++ app/services/exports/lettings_log_export_service.rb | 3 +++ spec/fixtures/exports/general_needs_log_24_25.xml | 1 + 3 files changed, 6 insertions(+) diff --git a/app/services/exports/lettings_log_export_constants.rb b/app/services/exports/lettings_log_export_constants.rb index 3a04c7fab..440a44955 100644 --- a/app/services/exports/lettings_log_export_constants.rb +++ b/app/services/exports/lettings_log_export_constants.rb @@ -154,6 +154,7 @@ module Exports::LettingsLogExportConstants "pscharge_value_check", "supcharg_value_check", "carehome_charges_value_check", + "assigned_to", ] (1..8).each do |index| @@ -187,6 +188,7 @@ module Exports::LettingsLogExportConstants "pscharge_value_check", "supcharg_value_check", "carehome_charges_value_check", + "assigned_to", ] PRE_2024_EXPORT_FIELDS = Set[ diff --git a/app/services/exports/lettings_log_export_service.rb b/app/services/exports/lettings_log_export_service.rb index c12e188a2..49cf601c9 100644 --- a/app/services/exports/lettings_log_export_service.rb +++ b/app/services/exports/lettings_log_export_service.rb @@ -230,6 +230,9 @@ module Exports attribute_hash["renttype_detail"] = LettingsLog::RENTTYPE_DETAIL_MAPPING[lettings_log.rent_type] if lettings_log.rent_type.present? + # field to be added in future + attribute_hash["assigned_to"] = nil + attribute_hash end diff --git a/spec/fixtures/exports/general_needs_log_24_25.xml b/spec/fixtures/exports/general_needs_log_24_25.xml index bcdb8941c..45b33b8cd 100644 --- a/spec/fixtures/exports/general_needs_log_24_25.xml +++ b/spec/fixtures/exports/general_needs_log_24_25.xml @@ -172,6 +172,7 @@ test1@example.com 2 + 1 From a38d6a265fd61772830712c8e3589ac5acf42d5d Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Mon, 25 Mar 2024 08:13:05 +0000 Subject: [PATCH 5/7] CLDC-3334 Improve address search flow (#2339) * Refactor page buttons * Immediately show address search results --- app/controllers/form_controller.rb | 2 +- app/helpers/form_page_helper.rb | 28 +++++++ .../form/lettings/pages/address_matcher.rb | 4 + app/models/form/page.rb | 3 +- .../form/sales/pages/address_matcher.rb | 4 + app/views/form/page.html.erb | 15 +--- spec/features/form/form_navigation_spec.rb | 83 ++++++++++++++++--- 7 files changed, 113 insertions(+), 26 deletions(-) diff --git a/app/controllers/form_controller.rb b/app/controllers/form_controller.rb index ae23e564b..117a4ac19 100644 --- a/app/controllers/form_controller.rb +++ b/app/controllers/form_controller.rb @@ -249,7 +249,7 @@ private redirect_to lettings_log_path(@log) unless @log.collection_period_open_for_editing? end - CONFIRMATION_PAGE_IDS = %w[uprn_confirmation].freeze + CONFIRMATION_PAGE_IDS = %w[uprn_confirmation uprn_selection].freeze def correcting_duplicate_logs_redirect_path class_name = @log.class.name.underscore diff --git a/app/helpers/form_page_helper.rb b/app/helpers/form_page_helper.rb index aefbed1c1..dc471da1c 100644 --- a/app/helpers/form_page_helper.rb +++ b/app/helpers/form_page_helper.rb @@ -18,4 +18,32 @@ module FormPageHelper def relevant_check_answers_path(log, subsection) send("#{log.class.name.underscore}_#{subsection.id}_check_answers_path", log) end + + def submit_button_text(page, referrer) + return page.submit_text if page.submit_text.present? + + if accessed_from_duplicate_logs?(referrer) || returning_to_question_page?(page, referrer) + "Save changes" + else + "Save and continue" + end + end + + def cancel_button_text(page, referrer) + if accessed_from_duplicate_logs?(referrer) || returning_to_question_page?(page, referrer) + "Cancel" + else + page.skip_text || "Skip for now" + end + end + + def cancel_button_link(page, referrer, original_log_id, log) + if accessed_from_duplicate_logs?(referrer) + duplicate_log_set_path(log, original_log_id) + elsif returning_to_question_page?(page, referrer) + send(log.form.cancel_path(page, log), log) + else + page.skip_href(log) || send(log.form.next_page_redirect_path(page, log, current_user), log) + end + end end diff --git a/app/models/form/lettings/pages/address_matcher.rb b/app/models/form/lettings/pages/address_matcher.rb index 2f19519b8..a48eb21c3 100644 --- a/app/models/form/lettings/pages/address_matcher.rb +++ b/app/models/form/lettings/pages/address_matcher.rb @@ -16,4 +16,8 @@ class Form::Lettings::Pages::AddressMatcher < ::Form::Page Form::Lettings::Questions::PostcodeForAddressMatcher.new(nil, nil, self), ] end + + def submit_text + "Search" + end end diff --git a/app/models/form/page.rb b/app/models/form/page.rb index 498c69d7b..6485ceb07 100644 --- a/app/models/form/page.rb +++ b/app/models/form/page.rb @@ -1,7 +1,7 @@ class Form::Page attr_accessor :id, :header, :header_partial, :description, :questions, :depends_on, :title_text, :informative_text, :subsection, :hide_subsection_label, :next_unresolved_page_id, - :skip_text, :interruption_screen_question_ids + :skip_text, :interruption_screen_question_ids, :submit_text def initialize(id, hsh, subsection) @id = id @@ -17,6 +17,7 @@ class Form::Page @hide_subsection_label = hsh["hide_subsection_label"] @next_unresolved_page_id = hsh["next_unresolved_page_id"] @skip_text = hsh["skip_text"] + @submit_text = hsh["submit_text"] @interruption_screen_question_ids = hsh["interruption_screen_question_ids"] || [] end end diff --git a/app/models/form/sales/pages/address_matcher.rb b/app/models/form/sales/pages/address_matcher.rb index f4a02972c..033b398c8 100644 --- a/app/models/form/sales/pages/address_matcher.rb +++ b/app/models/form/sales/pages/address_matcher.rb @@ -16,4 +16,8 @@ class Form::Sales::Pages::AddressMatcher < ::Form::Page Form::Sales::Questions::PostcodeForAddressMatcher.new(nil, nil, self), ] end + + def submit_text + "Search" + end end diff --git a/app/views/form/page.html.erb b/app/views/form/page.html.erb index 32ccdaed5..fffc22411 100644 --- a/app/views/form/page.html.erb +++ b/app/views/form/page.html.erb @@ -72,19 +72,8 @@
<% if !@page.interruption_screen? %> - <% if accessed_from_duplicate_logs?(request.query_parameters["referrer"]) %> - <%= f.govuk_submit "Save changes" %> - <%= govuk_link_to "Cancel", duplicate_log_set_path(@log, request.query_parameters["original_log_id"]) %> - <% elsif returning_to_question_page?(@page, request.query_parameters["referrer"]) %> - <%= f.govuk_submit "Save changes" %> - <%= govuk_link_to "Cancel", send(@log.form.cancel_path(@page, @log), @log) %> - <% else %> - <%= f.govuk_submit "Save and continue" %> - <%= govuk_link_to( - (@page.skip_text || "Skip for now"), - (@page.skip_href(@log) || send(@log.form.next_page_redirect_path(@page, @log, current_user), @log)), - ) %> - <% end %> + <%= f.govuk_submit submit_button_text(@page, request.query_parameters["referrer"]) %> + <%= govuk_link_to cancel_button_text(@page, request.query_parameters["referrer"]), cancel_button_link(@page, request.query_parameters["referrer"], request.query_parameters["original_log_id"], @log) %> <% end %>
diff --git a/spec/features/form/form_navigation_spec.rb b/spec/features/form/form_navigation_spec.rb index bb326c76c..9e43dd682 100644 --- a/spec/features/form/form_navigation_spec.rb +++ b/spec/features/form/form_navigation_spec.rb @@ -2,16 +2,7 @@ require "rails_helper" require_relative "helpers" RSpec.describe "Form Navigation" do - around do |example| - Timecop.travel(Time.zone.local(2022, 1, 1)) do - Singleton.__init__(FormHandler) - example.run - end - Timecop.return - Singleton.__init__(FormHandler) - end - - include Helpers + let(:now) { Time.zone.local(2022, 1, 1) } let(:user) { FactoryBot.create(:user) } let(:lettings_log) do FactoryBot.create( @@ -30,7 +21,6 @@ RSpec.describe "Form Navigation" do created_by: user, ) end - let(:id) { lettings_log.id } let(:question_answers) do { @@ -43,6 +33,17 @@ RSpec.describe "Form Navigation" do end let(:fake_2021_2022_form) { Form.new("spec/fixtures/forms/2021_2022.json") } + around do |example| + Timecop.travel(now) do + Singleton.__init__(FormHandler) + example.run + end + Timecop.return + Singleton.__init__(FormHandler) + end + + include Helpers + before do allow(lettings_log.form).to receive(:new_logs_end_date).and_return(Time.zone.today + 1.day) allow(fake_2021_2022_form).to receive(:new_logs_end_date).and_return(Time.zone.today + 1.day) @@ -185,4 +186,64 @@ RSpec.describe "Form Navigation" do expect(page).to have_current_path("/lettings-logs/#{id}/duplicate-logs?original_log_id=#{id}") end end + + describe "searching for an address" do + let(:now) { Time.zone.local(2024, 5, 1) } + + context "with a lettings log" do + let(:lettings_log) { create(:lettings_log, :completed, startdate: Time.zone.local(2024, 5, 5), created_by: user) } + + before do + stub_request(:get, /api\.os\.uk/) + .to_return(status: 200, body: { results: [{ DPA: { MATCH: 0.9, BUILDING_NAME: "result address line 1", POST_TOWN: "result town or city", POSTCODE: "AA1 1AA", UPRN: "12345" } }] }.to_json, headers: {}) + end + + it "allows searching for an address" do + visit("lettings-logs/#{id}/address-matcher") + fill_in("lettings-log-address-line1-input-field", with: "address") + fill_in("lettings-log-postcode-full-input-field", with: "A1 1AA") + click_button(text: "Search") + expect(page).to have_current_path("/lettings-logs/#{id}/uprn-selection") + end + + it "allows searching for an address from check your answers page" do + visit("lettings-logs/#{id}/address-matcher?referrer=check_answers") + fill_in("lettings-log-address-line1-input-field", with: "address") + fill_in("lettings-log-postcode-full-input-field", with: "A1 1AA") + click_button(text: "Search") + expect(page).to have_current_path("/lettings-logs/#{id}/uprn-selection?referrer=check_answers") + choose("lettings-log-uprn-selection-12345-field", allow_label_click: true) + click_button(text: "Save changes") + expect(page).to have_current_path("/lettings-logs/#{id}/property-information/check-answers") + end + end + + context "with a sales log" do + let(:sales_log) { create(:sales_log, :completed, saledate: Time.zone.local(2024, 5, 5), created_by: user) } + + before do + stub_request(:get, /api\.os\.uk/) + .to_return(status: 200, body: { results: [{ DPA: { MATCH: 0.9, BUILDING_NAME: "result address line 1", POST_TOWN: "result town or city", POSTCODE: "AA1 1AA", UPRN: "12345" } }] }.to_json, headers: {}) + end + + it "allows searching for an address" do + visit("sales-logs/#{sales_log.id}/address-matcher") + fill_in("sales-log-address-line1-input-field", with: "address") + fill_in("sales-log-postcode-full-input-field", with: "A1 1AA") + click_button(text: "Search") + expect(page).to have_current_path("/sales-logs/#{sales_log.id}/uprn-selection") + end + + it "allows searching for an address from check your answers page" do + visit("sales-logs/#{sales_log.id}/address-matcher?referrer=check_answers") + fill_in("sales-log-address-line1-input-field", with: "address") + fill_in("sales-log-postcode-full-input-field", with: "A1 1AA") + click_button(text: "Search") + expect(page).to have_current_path("/sales-logs/#{sales_log.id}/uprn-selection?referrer=check_answers") + choose("sales-log-uprn-selection-12345-field", allow_label_click: true) + click_button(text: "Save changes") + expect(page).to have_current_path("/sales-logs/#{sales_log.id}/property-information/check-answers") + end + end + end end From 6dc697fa5ba743d1d8f7b1fe0d5cccb4b6e3c4ba Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Mon, 25 Mar 2024 08:52:29 +0000 Subject: [PATCH 6/7] Specify start date for 2023 specific logs in tests (#2346) --- spec/models/log_spec.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/spec/models/log_spec.rb b/spec/models/log_spec.rb index b35737c67..73533f881 100644 --- a/spec/models/log_spec.rb +++ b/spec/models/log_spec.rb @@ -8,22 +8,22 @@ RSpec.describe Log, type: :model do describe "#calculate_status" do it "returns the correct status for a completed sales log" do - complete_sales_log = create(:sales_log, :completed, status: nil) + complete_sales_log = create(:sales_log, :completed, saledate: Time.zone.local(2023, 12, 12), status: nil) expect(complete_sales_log.calculate_status).to eq "completed" end it "returns the correct status for an in progress sales log" do - in_progress_sales_log = create(:sales_log, :in_progress, status: nil) + in_progress_sales_log = create(:sales_log, :in_progress, saledate: Time.zone.local(2023, 12, 12), status: nil) expect(in_progress_sales_log.calculate_status).to eq "in_progress" end it "returns the correct status for a completed lettings log" do - complete_lettings_log = create(:lettings_log, :completed, status: nil) + complete_lettings_log = create(:lettings_log, :completed, startdate: Time.zone.local(2023, 12, 12), voiddate: Time.zone.local(2023, 12, 11), mrcdate: Time.zone.local(2023, 12, 11), status: nil) expect(complete_lettings_log.calculate_status).to eq "completed" end it "returns the correct status for an in progress lettings log" do - in_progress_lettings_log = create(:lettings_log, :in_progress, status: nil) + in_progress_lettings_log = create(:lettings_log, :in_progress, startdate: Time.zone.local(2023, 12, 12), status: nil) expect(in_progress_lettings_log.calculate_status).to eq "in_progress" end end @@ -48,7 +48,7 @@ RSpec.describe Log, type: :model do end context "when a non setup field is invalid for a lettings log" do - subject(:model) { build(:lettings_log, :completed, offered: 234) } + subject(:model) { build(:lettings_log, :completed, startdate: Time.zone.local(2023, 12, 12), offered: 234) } it "blanks it" do model.valid? From 695f542fdcd1f6c0b593cf399e1bff34f603b3ec Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire <94526761+natdeanlewissoftwire@users.noreply.github.com> Date: Mon, 25 Mar 2024 09:11:28 +0000 Subject: [PATCH 7/7] CLDC-3327 Deduplicate null errors in bulk upload (#2338) * feat: validate nulls last in bu * feat: add tests * feat: update org errors and tests * feat: update test copy * feat: update tests * CLDC-3328 Make bulk upload errors consistent between lettings and sales (#2341) * feat: add validate_address_fields to lettings for 2024 * CLDC-3338: Add tolerance to discounted sale calculations (#2333) * Rename method * Update staircase/non staircase validations * Add errors to type * Remove validate_shared_ownership_deposit * Don't add setup BU errors, deduplicate different sale type errors * Add tolerance * Reuse method * Rename methods * Skip type error completely in BU * Update validation messages * Update over tolerance method * C:DC-3338: Add tolerance to grant calculations --------- Co-authored-by: Kat * feat: add validate_address_fields to lettings for 2024 * feat: update tests * Revert "CLDC-3338: Add tolerance to discounted sale calculations (#2333)" This reverts commit cdecdfebfefa1e3d0f61810e408bafc1df2a7401. --------- Co-authored-by: Robert Sullivan Co-authored-by: Kat --------- Co-authored-by: Robert Sullivan Co-authored-by: Kat --- .../lettings/year2024/row_parser.rb | 23 +++++++++++++++---- .../bulk_upload/sales/year2024/row_parser.rb | 7 +++--- .../lettings/year2023/row_parser_spec.rb | 2 +- .../lettings/year2024/row_parser_spec.rb | 19 +++++++++++---- .../sales/year2024/row_parser_spec.rb | 22 +++++++++++++++++- 5 files changed, 58 insertions(+), 15 deletions(-) diff --git a/app/services/bulk_upload/lettings/year2024/row_parser.rb b/app/services/bulk_upload/lettings/year2024/row_parser.rb index 0ad096c2a..2994408c8 100644 --- a/app/services/bulk_upload/lettings/year2024/row_parser.rb +++ b/app/services/bulk_upload/lettings/year2024/row_parser.rb @@ -411,13 +411,14 @@ class BulkUpload::Lettings::Year2024::RowParser validate :validate_all_charges_given, on: :after_log, if: proc { is_carehome.zero? } validate :validate_address_option_found, on: :after_log - validate :validate_nulls, on: :after_log - validate :validate_uprn_exists_if_any_key_address_fields_are_blank, on: :after_log, unless: -> { supported_housing? } + validate :validate_address_fields, on: :after_log validate :validate_incomplete_soft_validations, on: :after_log validate :validate_nationality, on: :after_log + validate :validate_nulls, on: :after_log + def self.question_for_field(field) QUESTIONS[field] end @@ -581,6 +582,18 @@ private end end + def validate_address_fields + if field_16.blank? || log.errors.attribute_names.include?(:uprn) + if field_17.blank? + errors.add(:field_17, I18n.t("validations.not_answered", question: "address line 1")) + end + + if field_19.blank? + errors.add(:field_19, I18n.t("validations.not_answered", question: "town or city")) + end + end + end + def validate_incomplete_soft_validations routed_to_soft_validation_questions = log.form.questions.filter { |q| q.type == "interruption_screen" && q.page.routed_to?(log, nil) }.compact routed_to_soft_validation_questions.each do |question| @@ -809,7 +822,7 @@ private if managing_organisation.nil? block_log_creation! - if errors[:field_2].blank? + if field_2.present? && errors[:field_2].blank? errors.add(:field_2, "The managing organisation code is incorrect", category: :setup) end end @@ -818,7 +831,7 @@ private def validate_managing_org_data_given if field_2.blank? block_log_creation! - errors.add(:field_2, "The managing organisation code is incorrect", category: :setup) + errors.add(:field_2, I18n.t("validations.not_answered", question: "managing organisation"), category: :setup) end end @@ -836,7 +849,7 @@ private if owning_organisation.nil? block_log_creation! - if errors[:field_1].blank? + if field_1.present? && errors[:field_1].blank? errors.add(:field_1, "The owning organisation code is incorrect", category: :setup) end end diff --git a/app/services/bulk_upload/sales/year2024/row_parser.rb b/app/services/bulk_upload/sales/year2024/row_parser.rb index e5f4f1d61..9b401e119 100644 --- a/app/services/bulk_upload/sales/year2024/row_parser.rb +++ b/app/services/bulk_upload/sales/year2024/row_parser.rb @@ -453,7 +453,6 @@ class BulkUpload::Sales::Year2024::RowParser validate :validate_buyer1_economic_status, on: :before_log validate :validate_address_option_found, on: :after_log validate :validate_buyer2_economic_status, on: :before_log - validate :validate_nulls, on: :after_log validate :validate_valid_radio_option, on: :before_log validate :validate_owning_org_data_given, on: :after_log @@ -474,6 +473,8 @@ class BulkUpload::Sales::Year2024::RowParser validate :validate_nationality, on: :after_log validate :validate_buyer_2_nationality, on: :after_log + validate :validate_nulls, on: :after_log + def self.question_for_field(field) QUESTIONS[field] end @@ -1259,7 +1260,7 @@ private block_log_creation! if errors[:field_1].blank? - errors.add(:field_1, "The owning organisation code is incorrect", category: :setup) + errors.add(:field_1, I18n.t("validations.not_answered", question: "owning organisation"), category: :setup) end end end @@ -1268,7 +1269,7 @@ private if owning_organisation.nil? block_log_creation! - if errors[:field_1].blank? + if field_1.present? && errors[:field_1].blank? errors.add(:field_1, "The owning organisation code is incorrect", category: :setup) end end diff --git a/spec/services/bulk_upload/lettings/year2023/row_parser_spec.rb b/spec/services/bulk_upload/lettings/year2023/row_parser_spec.rb index aba3b0670..6203b9c59 100644 --- a/spec/services/bulk_upload/lettings/year2023/row_parser_spec.rb +++ b/spec/services/bulk_upload/lettings/year2023/row_parser_spec.rb @@ -698,7 +698,7 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do context "when non-setup questions are null" do let(:attributes) { setup_section_params.merge({ field_18: "", field_19: "", field_21: "" }) } - it "fetches the question's check_answer_label if it exists, otherwise it gets the question's header" do + it "fetches the question's check_answer_label if it exists" do parser.valid? expect(parser.errors[:field_19]).to eql(["You must answer address line 1"]) expect(parser.errors[:field_21]).to eql(["You must answer town or city"]) diff --git a/spec/services/bulk_upload/lettings/year2024/row_parser_spec.rb b/spec/services/bulk_upload/lettings/year2024/row_parser_spec.rb index c19dbcbc0..9549cfe12 100644 --- a/spec/services/bulk_upload/lettings/year2024/row_parser_spec.rb +++ b/spec/services/bulk_upload/lettings/year2024/row_parser_spec.rb @@ -793,11 +793,20 @@ RSpec.describe BulkUpload::Lettings::Year2024::RowParser do context "when non-setup questions are null" do let(:attributes) { setup_section_params.merge({ field_43: "" }) } - it "fetches the question's check_answer_label if it exists, otherwise it gets the question's header" do + it "fetches the question's check_answer_label if it exists" do parser.valid? expect(parser.errors[:field_43]).to eql(["You must answer lead tenant’s gender identity"]) end end + + context "when other null error is added" do + let(:attributes) { setup_section_params.merge({ field_112: nil }) } + + it "only has one error added to the field" do + parser.valid? + expect(parser.errors[:field_112]).to eql(["You must answer was the letting made under the Choice-Based Lettings (CBL)?"]) + end + end end end @@ -1389,7 +1398,7 @@ RSpec.describe BulkUpload::Lettings::Year2024::RowParser do it "is not permitted as setup error" do setup_errors = parser.errors.select { |e| e.options[:category] == :setup } - expect(setup_errors.find { |e| e.attribute == :field_2 }.message).to eql("The managing organisation code is incorrect") + expect(setup_errors.find { |e| e.attribute == :field_2 }.message).to eql("You must answer managing organisation") end it "blocks log creation" do @@ -1486,10 +1495,10 @@ RSpec.describe BulkUpload::Lettings::Year2024::RowParser do field_1: "1" } end - it "does not add UPRN errors" do + it "does not add UPRN errors (but still adds missing address errors)" do expect(parser.errors[:field_16]).to be_empty - expect(parser.errors[:field_17]).to be_empty - expect(parser.errors[:field_19]).to be_empty + expect(parser.errors[:field_17]).to eql(["You must answer address line 1"]) + expect(parser.errors[:field_19]).to eql(["You must answer town or city"]) end end diff --git a/spec/services/bulk_upload/sales/year2024/row_parser_spec.rb b/spec/services/bulk_upload/sales/year2024/row_parser_spec.rb index d80aca533..1f3c4800c 100644 --- a/spec/services/bulk_upload/sales/year2024/row_parser_spec.rb +++ b/spec/services/bulk_upload/sales/year2024/row_parser_spec.rb @@ -282,6 +282,26 @@ RSpec.describe BulkUpload::Sales::Year2024::RowParser do expect(questions.map(&:id)).to eql([]) end end + + describe "#validate_nulls" do + context "when non-setup questions are null" do + let(:attributes) { setup_section_params.merge({ field_32: "" }) } + + it "fetches the question's check_answer_label if it exists" do + parser.valid? + expect(parser.errors[:field_32]).to eql(["You must answer buyer 1’s gender identity"]) + end + end + + context "when other null error is added" do + let(:attributes) { setup_section_params.merge({ field_23: nil }) } + + it "only has one error added to the field" do + parser.valid? + expect(parser.errors[:field_23]).to eql(["You must answer address line 1"]) + end + end + end end context "when setup section not complete and type is not given" do @@ -418,7 +438,7 @@ RSpec.describe BulkUpload::Sales::Year2024::RowParser do let(:attributes) { { bulk_upload:, field_1: "donotexist" } } it "is not permitted as a setup error" do - expect(parser.errors.where(:field_1, category: :setup).map(&:message)).to eql(["You must answer owning organisation"]) + expect(parser.errors.where(:field_1, category: :setup).map(&:message)).to eql(["The owning organisation code is incorrect"]) end it "blocks log creation" do