From 1c03a70fa2268e293c59453ec7789c6c94349452 Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Wed, 11 Dec 2024 17:20:28 +0000 Subject: [PATCH] CLDC-3769 Fix some check errors flows (#2826) * Fix some check errors flows * Only show check errors journey for question on different pages * Update restore_error_field_values --- app/controllers/form_controller.rb | 11 ++++++++--- app/helpers/form_page_error_helper.rb | 5 +++-- .../form/sales/pages/owning_organisation.rb | 2 ++ app/models/validations/property_validations.rb | 2 ++ .../validations/sales/property_validations.rb | 2 ++ app/views/form/page.html.erb | 4 ++-- spec/features/form/page_routing_spec.rb | 16 +++++++++++++++- spec/requests/check_errors_controller_spec.rb | 2 +- 8 files changed, 35 insertions(+), 9 deletions(-) diff --git a/app/controllers/form_controller.rb b/app/controllers/form_controller.rb index 7ce63e609..3e029371d 100644 --- a/app/controllers/form_controller.rb +++ b/app/controllers/form_controller.rb @@ -105,8 +105,13 @@ private def restore_error_field_values(previous_responses) return unless previous_responses - previous_responses_to_reset = previous_responses.reject do |key, _| - @log.form.get_question(key, @log)&.type == "date" + previous_responses_to_reset = previous_responses.reject do |key, value| + if @log.form.get_question(key, @log)&.type == "date" && value.present? + year = value.split("-").first.to_i + year&.zero? + else + false + end end @log.assign_attributes(previous_responses_to_reset) @@ -433,7 +438,7 @@ private @log.valid? @log.reload error_attributes = @log.errors.map(&:attribute) - @questions = @log.form.questions.select { |q| error_attributes.include?(q.id.to_sym) } + @questions = @log.form.questions.select { |q| error_attributes.include?(q.id.to_sym) && q.page.routed_to?(@log, current_user) } end render "form/check_errors" end diff --git a/app/helpers/form_page_error_helper.rb b/app/helpers/form_page_error_helper.rb index 2c90bfd2a..bf4e1db08 100644 --- a/app/helpers/form_page_error_helper.rb +++ b/app/helpers/form_page_error_helper.rb @@ -13,7 +13,8 @@ module FormPageErrorHelper end end - def all_questions_affected_by_errors(log) - (log.errors.map(&:attribute) - [:base]).uniq + def all_pages_affected_by_errors(log) + question_ids = (log.errors.map(&:attribute) - [:base]).uniq + question_ids.map { |id| log.form.get_question(id, log)&.page&.id }.compact.uniq end end diff --git a/app/models/form/sales/pages/owning_organisation.rb b/app/models/form/sales/pages/owning_organisation.rb index f0c9e4e68..f1875d52d 100644 --- a/app/models/form/sales/pages/owning_organisation.rb +++ b/app/models/form/sales/pages/owning_organisation.rb @@ -20,11 +20,13 @@ class Form::Sales::Pages::OwningOrganisation < ::Form::Page if current_user.organisation.holds_own_stock? return true if current_user.organisation.absorbed_organisations.any?(&:holds_own_stock?) return true if stock_owners.count >= 1 + return false if log.owning_organisation == current_user.organisation log.update!(owning_organisation: current_user.organisation) else return false if stock_owners.count.zero? return true if stock_owners.count > 1 + return false if log.owning_organisation == stock_owners.first log.update!(owning_organisation: stock_owners.first) end diff --git a/app/models/validations/property_validations.rb b/app/models/validations/property_validations.rb index e9eba8184..1cf710857 100644 --- a/app/models/validations/property_validations.rb +++ b/app/models/validations/property_validations.rb @@ -41,6 +41,8 @@ module Validations::PropertyValidations def validate_property_postcode(record) postcode = record.postcode_full + return unless postcode + if record.postcode_known? && (postcode.blank? || !postcode.match(POSTCODE_REGEXP)) error_message = I18n.t("validations.lettings.property.postcode_full.invalid") record.errors.add :postcode_full, :wrong_format, message: error_message diff --git a/app/models/validations/sales/property_validations.rb b/app/models/validations/sales/property_validations.rb index 2238a634a..7fd4d2440 100644 --- a/app/models/validations/sales/property_validations.rb +++ b/app/models/validations/sales/property_validations.rb @@ -31,6 +31,8 @@ module Validations::Sales::PropertyValidations def validate_property_postcode(record) postcode = record.postcode_full + return unless postcode + if record.postcode_known? && (postcode.blank? || !postcode.match(POSTCODE_REGEXP)) error_message = I18n.t("validations.sales.property_information.postcode_full.invalid") record.errors.add :postcode_full, :wrong_format, message: error_message diff --git a/app/views/form/page.html.erb b/app/views/form/page.html.erb index 95dc38ec6..7379de0eb 100644 --- a/app/views/form/page.html.erb +++ b/app/views/form/page.html.erb @@ -16,7 +16,7 @@ <%= form_with model: @log, url: request.original_url, method: "post", local: true do |f| %>
- <% all_questions_with_errors = all_questions_affected_by_errors(@log) %> + <% all_pages_with_errors = all_pages_affected_by_errors(@log) %> <% remove_other_page_errors(@log, @page) %> <%= f.govuk_error_summary %> @@ -76,7 +76,7 @@ <%= f.hidden_field :check_errors, value: @check_errors %> <% end %> - <% if all_questions_with_errors.count > 1 %> + <% if all_pages_with_errors.count > 1 %>
<%= f.submit "See all related answers", name: "check_errors", class: "govuk-body govuk-link submit-button-link" %>
diff --git a/spec/features/form/page_routing_spec.rb b/spec/features/form/page_routing_spec.rb index 42a2c25fb..118b52543 100644 --- a/spec/features/form/page_routing_spec.rb +++ b/spec/features/form/page_routing_spec.rb @@ -92,7 +92,7 @@ RSpec.describe "Form Page Routing" do expect(find("#lettings-log-postcode-full-field-error").value).to eq("FAKE_POSTCODE") end - it "does not reset the displayed date" do + it "does not reset the displayed date if it's an invalid date" do lettings_log.update!(startdate: "2021/10/13") visit("/lettings-logs/#{id}/tenancy-start-date") fill_in("lettings_log[startdate(1i)]", with: "202") @@ -106,6 +106,20 @@ RSpec.describe "Form Page Routing" do expect(find_field("lettings_log[startdate(1i)]").value).to eq("2021") end + it "displays the entered date if it's in a valid format" do + lettings_log.update!(startdate: "2021/10/13") + visit("/lettings-logs/#{id}/tenancy-start-date") + fill_in("lettings_log[startdate(1i)]", with: "202") + fill_in("lettings_log[startdate(2i)]", with: "12") + fill_in("lettings_log[startdate(3i)]", with: "1") + click_button("Save and continue") + + expect(page).to have_current_path("/lettings-logs/#{id}/tenancy-start-date") + expect(find_field("lettings_log[startdate(3i)]").value).to eq("1") + expect(find_field("lettings_log[startdate(2i)]").value).to eq("12") + expect(find_field("lettings_log[startdate(1i)]").value).to eq("202") + end + it "does not reset the displayed date if it's empty" do lettings_log.update!(startdate: nil) visit("/lettings-logs/#{id}/tenancy-start-date") diff --git a/spec/requests/check_errors_controller_spec.rb b/spec/requests/check_errors_controller_spec.rb index 29130f547..9c4d3f8c1 100644 --- a/spec/requests/check_errors_controller_spec.rb +++ b/spec/requests/check_errors_controller_spec.rb @@ -84,7 +84,7 @@ RSpec.describe CheckErrorsController, type: :request do end it "displays correct clear and change links" do - expect(page.all(:button, value: "Clear").count).to eq(2) + expect(page.all(:button, value: "Clear").count).to eq(1) expect(page).to have_link("Change", count: 1) expect(page).to have_button("Clear all") end