From d0046256cbb35a879bd0043df62c6b4cd756fdf4 Mon Sep 17 00:00:00 2001
From: Phil Lee <asmega@users.noreply.github.com>
Date: Thu, 6 Apr 2023 10:20:51 +0100
Subject: [PATCH 1/7] CLDC-2170 Bulk upload summary tweaks (#1522)

* show summary when on error threshold

* remove bulk upload mailer template

- this template is no longer used

* setup errors do not consider threshold

- return approprate intro test depending if there are setup errors or
  not
---
 ...oad_error_summary_table_component.html.erb |  4 ++
 ...lk_upload_error_summary_table_component.rb | 11 ++++-
 app/mailers/bulk_upload_mailer.rb             | 43 ++-----------------
 app/services/bulk_upload/processor.rb         |  6 ---
 .../summary.html.erb                          |  4 --
 ...load_error_summary_table_component_spec.rb | 27 ++++++++++++
 spec/mailers/bulk_upload_mailer_spec.rb       | 38 +---------------
 7 files changed, 44 insertions(+), 89 deletions(-)

diff --git a/app/components/bulk_upload_error_summary_table_component.html.erb b/app/components/bulk_upload_error_summary_table_component.html.erb
index 2f9643271..ab5254c0f 100644
--- a/app/components/bulk_upload_error_summary_table_component.html.erb
+++ b/app/components/bulk_upload_error_summary_table_component.html.erb
@@ -1,3 +1,7 @@
+<p class="govuk-body">
+  <%= intro %>
+</p>
+
 <% sorted_errors.each do |error| %>
   <%= govuk_table do |table| %>
     <% table.head do |head| %>
diff --git a/app/components/bulk_upload_error_summary_table_component.rb b/app/components/bulk_upload_error_summary_table_component.rb
index 8dddb61d2..7b483077f 100644
--- a/app/components/bulk_upload_error_summary_table_component.rb
+++ b/app/components/bulk_upload_error_summary_table_component.rb
@@ -15,7 +15,7 @@ class BulkUploadErrorSummaryTableComponent < ViewComponent::Base
     @sorted_errors ||= setup_errors.presence || bulk_upload
       .bulk_upload_errors
       .group(:col, :field, :error)
-      .having("count(*) > ?", display_threshold)
+      .having("count(*) >= ?", display_threshold)
       .count
       .sort_by { |el| el[0][0].rjust(3, "0") }
   end
@@ -24,6 +24,14 @@ class BulkUploadErrorSummaryTableComponent < ViewComponent::Base
     sorted_errors.present?
   end
 
+  def intro
+    if setup_errors.present?
+      "This summary shows important questions that have errors. See full error report for more details."
+    else
+      "This summary shows questions that have more than #{BulkUploadErrorSummaryTableComponent::DISPLAY_THRESHOLD - 1} errors. See full error report for more details."
+    end
+  end
+
 private
 
   def setup_errors
@@ -31,7 +39,6 @@ private
       .bulk_upload_errors
       .where(category: "setup")
       .group(:col, :field, :error)
-      .having("count(*) > ?", display_threshold)
       .count
       .sort_by { |el| el[0][0].rjust(3, "0") }
   end
diff --git a/app/mailers/bulk_upload_mailer.rb b/app/mailers/bulk_upload_mailer.rb
index 2d0b20a92..b6388213a 100644
--- a/app/mailers/bulk_upload_mailer.rb
+++ b/app/mailers/bulk_upload_mailer.rb
@@ -5,7 +5,6 @@ class BulkUploadMailer < NotifyMailer
   FAILED_CSV_ERRORS_TEMPLATE_ID = "e27abcd4-5295-48c2-b127-e9ee4b781b75".freeze
   FAILED_FILE_SETUP_ERROR_TEMPLATE_ID = "24c9f4c7-96ad-470a-ba31-eb51b7cbafd9".freeze
   FAILED_SERVICE_ERROR_TEMPLATE_ID = "c3f6288c-7a74-4e77-99ee-6c4a0f6e125a".freeze
-  WITH_ERRORS_TEMPLATE_ID = "eb539005-6234-404e-812d-167728cf4274".freeze
   HOW_FIX_UPLOAD_TEMPLATE_ID = "21a07b26-f625-4846-9f4d-39e30937aa24".freeze
 
   def send_how_fix_upload_mail(bulk_upload:)
@@ -73,25 +72,12 @@ class BulkUploadMailer < NotifyMailer
   end
 
   def send_bulk_upload_failed_file_setup_error_mail(bulk_upload:)
-    bulk_upload_link = if bulk_upload.lettings?
-                         start_bulk_upload_lettings_logs_url
+    bulk_upload_link = if BulkUploadErrorSummaryTableComponent.new(bulk_upload:).errors?
+                         summary_bulk_upload_lettings_result_url(bulk_upload)
                        else
-                         start_bulk_upload_sales_logs_url
+                         bulk_upload_lettings_result_url(bulk_upload)
                        end
 
-    row_parser_class = bulk_upload.prefix_namespace::RowParser
-
-    errors = bulk_upload
-      .bulk_upload_errors
-      .where(category: "setup")
-      .group(:col, :field)
-      .count
-      .keys
-      .sort_by { |_col, field| field }
-      .map do |col, field|
-        "- #{row_parser_class.question_for_field(field.to_sym)} (Column #{col})"
-      end
-
     send_email(
       bulk_upload.user.email,
       FAILED_FILE_SETUP_ERROR_TEMPLATE_ID,
@@ -100,7 +86,6 @@ class BulkUploadMailer < NotifyMailer
         upload_timestamp: bulk_upload.created_at.to_fs(:govuk_date_and_time),
         lettings_or_sales: bulk_upload.log_type,
         year_combo: bulk_upload.year_combo,
-        errors_list: errors.join("\n"),
         bulk_upload_link:,
       },
     )
@@ -126,26 +111,4 @@ class BulkUploadMailer < NotifyMailer
       },
     )
   end
-
-  def send_bulk_upload_with_errors_mail(bulk_upload:)
-    count = bulk_upload.logs.where.not(status: %w[completed]).count
-
-    n_logs = pluralize(count, "log")
-
-    title = "We found #{n_logs} with errors"
-
-    error_description = "We created logs from your #{bulk_upload.year_combo} #{bulk_upload.log_type} data. There was a problem with #{count} of the logs. Click the below link to fix these logs."
-
-    send_email(
-      bulk_upload.user.email,
-      WITH_ERRORS_TEMPLATE_ID,
-      {
-        title:,
-        filename: bulk_upload.filename,
-        upload_timestamp: bulk_upload.created_at.to_fs(:govuk_date_and_time),
-        error_description:,
-        summary_report_link: resume_bulk_upload_lettings_result_url(bulk_upload),
-      },
-    )
-  end
 end
diff --git a/app/services/bulk_upload/processor.rb b/app/services/bulk_upload/processor.rb
index 4fe449348..49d44261f 100644
--- a/app/services/bulk_upload/processor.rb
+++ b/app/services/bulk_upload/processor.rb
@@ -59,12 +59,6 @@ private
       .deliver_later
   end
 
-  def send_fix_errors_mail
-    BulkUploadMailer
-      .send_bulk_upload_with_errors_mail(bulk_upload:)
-      .deliver_later
-  end
-
   def send_success_mail
     BulkUploadMailer
       .send_bulk_upload_complete_mail(user:, bulk_upload:)
diff --git a/app/views/bulk_upload_lettings_results/summary.html.erb b/app/views/bulk_upload_lettings_results/summary.html.erb
index a6072fe89..533e2af05 100644
--- a/app/views/bulk_upload_lettings_results/summary.html.erb
+++ b/app/views/bulk_upload_lettings_results/summary.html.erb
@@ -16,10 +16,6 @@
 <div class="govuk-grid-row">
   <%= govuk_tabs(title: "Error reports") do |c| %>
     <% c.with_tab(label: "Summary") do %>
-      <p class="govuk-body">
-        This summary shows questions that have at least <%= BulkUploadErrorSummaryTableComponent::DISPLAY_THRESHOLD %> errors or more. See full error report for more details.
-      </p>
-
       <%= render BulkUploadErrorSummaryTableComponent.new(bulk_upload: @bulk_upload) %>
     <% end %>
 
diff --git a/spec/components/bulk_upload_error_summary_table_component_spec.rb b/spec/components/bulk_upload_error_summary_table_component_spec.rb
index 32da38119..79be32901 100644
--- a/spec/components/bulk_upload_error_summary_table_component_spec.rb
+++ b/spec/components/bulk_upload_error_summary_table_component_spec.rb
@@ -30,6 +30,25 @@ RSpec.describe BulkUploadErrorSummaryTableComponent, type: :component do
       end
     end
 
+    context "when on threshold" do
+      before do
+        stub_const("BulkUploadErrorSummaryTableComponent::DISPLAY_THRESHOLD", 1)
+
+        create(:bulk_upload_error, bulk_upload:, col: "A", row: 1)
+      end
+
+      it "renders tables" do
+        result = render_inline(component)
+        expect(result).to have_selector("table", count: 1)
+      end
+
+      it "renders intro with threshold" do
+        result = render_inline(component)
+
+        expect(result).to have_content("This summary shows questions that have more than 0 errors. See full error report for more details.")
+      end
+    end
+
     context "when there are 2 independent errors" do
       let!(:error_2) { create(:bulk_upload_error, bulk_upload:, col: "B", row: 2) }
       let!(:error_1) { create(:bulk_upload_error, bulk_upload:, col: "A", row: 1) }
@@ -99,6 +118,8 @@ RSpec.describe BulkUploadErrorSummaryTableComponent, type: :component do
 
       before do
         create(:bulk_upload_error, bulk_upload:, col: "B", row: 2, category: nil)
+
+        stub_const("BulkUploadErrorSummaryTableComponent::DISPLAY_THRESHOLD", 16)
       end
 
       it "only returns the setup errors" do
@@ -117,6 +138,12 @@ RSpec.describe BulkUploadErrorSummaryTableComponent, type: :component do
           "1 error",
         ])
       end
+
+      it "renders intro with setup errors" do
+        result = render_inline(component)
+
+        expect(result).to have_content("This summary shows important questions that have errors. See full error report for more details.")
+      end
     end
   end
 
diff --git a/spec/mailers/bulk_upload_mailer_spec.rb b/spec/mailers/bulk_upload_mailer_spec.rb
index cd2c4767d..20f78de4d 100644
--- a/spec/mailers/bulk_upload_mailer_spec.rb
+++ b/spec/mailers/bulk_upload_mailer_spec.rb
@@ -19,13 +19,6 @@ RSpec.describe BulkUploadMailer do
       create(:bulk_upload_error, bulk_upload:, col: "F", field: "field_5")
     end
 
-    let(:expected_errors) do
-      [
-        "- What is the letting type? (Column A)",
-        "- Management group code (Column E)",
-      ]
-    end
-
     it "sends correctly formed email" do
       expect(notify_client).to receive(:send_email).with(
         email_address: bulk_upload.user.email,
@@ -35,8 +28,7 @@ RSpec.describe BulkUploadMailer do
           upload_timestamp: bulk_upload.created_at.to_fs(:govuk_date_and_time),
           lettings_or_sales: bulk_upload.log_type,
           year_combo: bulk_upload.year_combo,
-          errors_list: expected_errors.join("\n"),
-          bulk_upload_link: start_bulk_upload_lettings_logs_url,
+          bulk_upload_link: summary_bulk_upload_lettings_result_url(bulk_upload),
         },
       )
 
@@ -81,34 +73,6 @@ RSpec.describe BulkUploadMailer do
     end
   end
 
-  context "when bulk upload has log which is not completed" do
-    before do
-      create(:lettings_log, :in_progress, bulk_upload:)
-    end
-
-    describe "#send_bulk_upload_with_errors_mail" do
-      let(:error_description) do
-        "We created logs from your 2022/23 lettings data. There was a problem with 1 of the logs. Click the below link to fix these logs."
-      end
-
-      it "sends correctly formed email" do
-        expect(notify_client).to receive(:send_email).with(
-          email_address: bulk_upload.user.email,
-          template_id: described_class::WITH_ERRORS_TEMPLATE_ID,
-          personalisation: {
-            title: "We found 1 log with errors",
-            filename: bulk_upload.filename,
-            upload_timestamp: bulk_upload.created_at.to_fs(:govuk_date_and_time),
-            error_description:,
-            summary_report_link: "http://localhost:3000/lettings-logs/bulk-upload-results/#{bulk_upload.id}/resume",
-          },
-        )
-
-        mailer.send_bulk_upload_with_errors_mail(bulk_upload:)
-      end
-    end
-  end
-
   describe "#send_correct_and_upload_again_mail" do
     context "when 2 columns with errors" do
       before do

From 9bbc2c2e1712c370eaeedaaf638276cc1f5b5640 Mon Sep 17 00:00:00 2001
From: Arthur Campbell <51094020+arfacamble@users.noreply.github.com>
Date: Thu, 6 Apr 2023 11:09:03 +0100
Subject: [PATCH 2/7] CLDC-1687 allow deactivation or reactivation of last year
 schemes and locations in crossover period (#1396)

* create method to test whether we are currently in the crossover period and associated tests

* update copy, use method for testing whether we are in the crossover period, remove focus from test file

* reuse existing method to determine whether we are in a collection period

* use the existing method and update validations

* fix test broken by changes

* update location in same way as scheme

* create method on FormHandler that finds the start date of the earliest collection period

* ensure that default deactivation and reactivation dates also reflect the changes

* create tests for the new validations

* lint correction

* minor copy change

* minor logic change

* amend naming error after rebase conflict
remove test that is no longer correct after change on another branch to functionality
update a test after a copy change
---
 app/controllers/locations_controller.rb       |  2 +-
 app/controllers/schemes_controller.rb         |  2 +-
 app/models/form_handler.rb                    |  4 +
 app/models/location_deactivation_period.rb    | 12 +--
 app/models/scheme_deactivation_period.rb      | 10 ++-
 app/views/locations/toggle_active.html.erb    |  6 +-
 app/views/schemes/toggle_active.html.erb      |  6 +-
 config/locales/en.yml                         |  6 +-
 spec/features/schemes_spec.rb                 |  4 +-
 .../location_deactivation_period_spec.rb      | 83 +++++++++++++++++++
 .../models/scheme_deactivation_period_spec.rb | 68 +++++++++++++++
 11 files changed, 182 insertions(+), 21 deletions(-)
 create mode 100644 spec/models/location_deactivation_period_spec.rb
 create mode 100644 spec/models/scheme_deactivation_period_spec.rb

diff --git a/app/controllers/locations_controller.rb b/app/controllers/locations_controller.rb
index 50102ee17..876f0e405 100644
--- a/app/controllers/locations_controller.rb
+++ b/app/controllers/locations_controller.rb
@@ -269,7 +269,7 @@ private
     if params[:location_deactivation_period].blank?
       return
     elsif params[:location_deactivation_period]["#{key}_type".to_sym] == "default"
-      return FormHandler.instance.current_collection_start_date
+      return FormHandler.instance.start_date_of_earliest_open_collection_period
     elsif params[:location_deactivation_period][key.to_sym].present?
       return params[:location_deactivation_period][key.to_sym]
     end
diff --git a/app/controllers/schemes_controller.rb b/app/controllers/schemes_controller.rb
index f248bffe5..e60620bc5 100644
--- a/app/controllers/schemes_controller.rb
+++ b/app/controllers/schemes_controller.rb
@@ -296,7 +296,7 @@ private
     if params[:scheme_deactivation_period].blank?
       return
     elsif params[:scheme_deactivation_period]["#{key}_type".to_sym] == "default"
-      return FormHandler.instance.current_collection_start_date
+      return FormHandler.instance.start_date_of_earliest_open_collection_period
     elsif params[:scheme_deactivation_period][key.to_sym].present?
       return params[:scheme_deactivation_period][key.to_sym]
     end
diff --git a/app/models/form_handler.rb b/app/models/form_handler.rb
index ab22e6bc5..ee84c3e32 100644
--- a/app/models/form_handler.rb
+++ b/app/models/form_handler.rb
@@ -77,6 +77,10 @@ class FormHandler
     form_mappings[current_collection_start_year - year]
   end
 
+  def start_date_of_earliest_open_collection_period
+    in_crossover_period? ? previous_collection_start_date : current_collection_start_date
+  end
+
   def in_crossover_period?(now: Time.zone.now)
     lettings_in_crossover_period?(now:) || sales_in_crossover_period?(now:)
   end
diff --git a/app/models/location_deactivation_period.rb b/app/models/location_deactivation_period.rb
index dcf347d24..c9a24bdc9 100644
--- a/app/models/location_deactivation_period.rb
+++ b/app/models/location_deactivation_period.rb
@@ -1,4 +1,6 @@
 class LocationDeactivationPeriodValidator < ActiveModel::Validator
+  include CollectionTimeHelper
+
   def validate(record)
     location = record.location
     recent_deactivation = location.location_deactivation_periods.deactivations_without_reactivation.first
@@ -16,7 +18,7 @@ class LocationDeactivationPeriodValidator < ActiveModel::Validator
       elsif record.reactivation_date_type == "other"
         record.errors.add(:reactivation_date, message: I18n.t("validations.location.toggle_date.invalid"))
       end
-    elsif !record.reactivation_date.between?(location.available_from, Time.zone.local(2200, 1, 1))
+    elsif record.reactivation_date.before? location.available_from
       record.errors.add(:reactivation_date, message: I18n.t("validations.location.toggle_date.out_of_range", date: location.available_from.to_formatted_s(:govuk_date)))
     elsif record.reactivation_date < recent_deactivation.deactivation_date
       record.errors.add(:reactivation_date, message: I18n.t("validations.location.reactivation.before_deactivation", date: recent_deactivation.deactivation_date.to_formatted_s(:govuk_date)))
@@ -32,10 +34,10 @@ class LocationDeactivationPeriodValidator < ActiveModel::Validator
       end
     elsif location.location_deactivation_periods.any? { |period| period.reactivation_date.present? && record.deactivation_date.between?(period.deactivation_date, period.reactivation_date - 1.day) }
       record.errors.add(:deactivation_date, message: I18n.t("validations.location.deactivation.during_deactivated_period"))
-    else
-      unless record.deactivation_date.between?(location.available_from, Time.zone.local(2200, 1, 1))
-        record.errors.add(:deactivation_date, message: I18n.t("validations.location.toggle_date.out_of_range", date: location.available_from.to_formatted_s(:govuk_date)))
-      end
+    elsif record.deactivation_date.before? FormHandler.instance.start_date_of_earliest_open_collection_period
+      record.errors.add(:deactivation_date, message: I18n.t("validations.location.toggle_date.out_of_range", date: FormHandler.instance.start_date_of_earliest_open_collection_period.to_formatted_s(:govuk_date)))
+    elsif record.deactivation_date.before? location.available_from
+      record.errors.add(:deactivation_date, message: I18n.t("validations.location.toggle_date.before_creation", date: location.available_from.to_formatted_s(:govuk_date)))
     end
   end
 end
diff --git a/app/models/scheme_deactivation_period.rb b/app/models/scheme_deactivation_period.rb
index f716cbc32..01aafbcb4 100644
--- a/app/models/scheme_deactivation_period.rb
+++ b/app/models/scheme_deactivation_period.rb
@@ -1,4 +1,6 @@
 class SchemeDeactivationPeriodValidator < ActiveModel::Validator
+  include CollectionTimeHelper
+
   def validate(record)
     scheme = record.scheme
     recent_deactivation = scheme.scheme_deactivation_periods.deactivations_without_reactivation.first
@@ -32,10 +34,10 @@ class SchemeDeactivationPeriodValidator < ActiveModel::Validator
       end
     elsif scheme.scheme_deactivation_periods.any? { |period| period.reactivation_date.present? && record.deactivation_date.between?(period.deactivation_date, period.reactivation_date - 1.day) }
       record.errors.add(:deactivation_date, message: I18n.t("validations.scheme.deactivation.during_deactivated_period"))
-    else
-      unless record.deactivation_date.between?(scheme.available_from, Time.zone.local(2200, 1, 1))
-        record.errors.add(:deactivation_date, message: I18n.t("validations.scheme.toggle_date.out_of_range", date: scheme.available_from.to_formatted_s(:govuk_date)))
-      end
+    elsif record.deactivation_date.before? FormHandler.instance.start_date_of_earliest_open_collection_period
+      record.errors.add(:deactivation_date, message: I18n.t("validations.scheme.toggle_date.out_of_range", date: FormHandler.instance.start_date_of_earliest_open_collection_period.to_formatted_s(:govuk_date)))
+    elsif record.deactivation_date.before? scheme.available_from
+      record.errors.add(:deactivation_date, message: I18n.t("validations.scheme.toggle_date.before_creation", date: scheme.available_from.to_formatted_s(:govuk_date)))
     end
   end
 end
diff --git a/app/views/locations/toggle_active.html.erb b/app/views/locations/toggle_active.html.erb
index 5c030de89..7b995b2b5 100644
--- a/app/views/locations/toggle_active.html.erb
+++ b/app/views/locations/toggle_active.html.erb
@@ -11,16 +11,16 @@
 <%= form_with model: @location_deactivation_period, url: toggle_location_form_path(action, @location), method: "patch", local: true do |f| %>
   <div class="govuk-grid-row">
     <div class="govuk-grid-column-two-thirds">
-    <% collection_start_date = FormHandler.instance.earliest_open_collection_start_date(now: @location.available_from) %>
+    <% start_date = FormHandler.instance.earliest_open_collection_start_date(now: @location.available_from) %>
     <%= f.govuk_error_summary %>
         <%= f.govuk_radio_buttons_fieldset date_type_question(action),
                                             legend: { text: I18n.t("questions.location.toggle_active.apply_from") },
                                             caption: { text: title },
-                                            hint: { text: I18n.t("hints.location.toggle_active", date: collection_start_date.to_formatted_s(:govuk_date)) } do %>
+                                            hint: { text: I18n.t("hints.location.toggle_active", date: start_date.to_formatted_s(:govuk_date)) } do %>
             <%= govuk_warning_text text: I18n.t("warnings.location.#{action}.existing_logs") %>
             <%= f.govuk_radio_button date_type_question(action),
                                         "default",
-                                        label: { text: "From the start of the current collection period (#{collection_start_date.to_formatted_s(:govuk_date)})" } %>
+                                        label: { text: "From the start of the open collection period (#{start_date.to_formatted_s(:govuk_date)})" } %>
 
             <%= f.govuk_radio_button date_type_question(action),
                                         "other",
diff --git a/app/views/schemes/toggle_active.html.erb b/app/views/schemes/toggle_active.html.erb
index 1b7507375..f2ecc4a1a 100644
--- a/app/views/schemes/toggle_active.html.erb
+++ b/app/views/schemes/toggle_active.html.erb
@@ -11,16 +11,16 @@
 <%= form_with model: @scheme_deactivation_period, url: toggle_scheme_form_path(action, @scheme), method: "patch", local: true do |f| %>
   <div class="govuk-grid-row">
     <div class="govuk-grid-column-two-thirds">
-      <% collection_start_date = FormHandler.instance.current_collection_start_date %>
+      <% start_date = FormHandler.instance.start_date_of_earliest_open_collection_period %>
       <%= f.govuk_error_summary %>
       <%= f.govuk_radio_buttons_fieldset date_type_question(action),
                                          legend: { text: I18n.t("questions.scheme.toggle_active.apply_from") },
                                          caption: { text: title },
-                                         hint: { text: I18n.t("hints.scheme.toggle_active", date: collection_start_date.to_formatted_s(:govuk_date)) } do %>
+                                         hint: { text: I18n.t("hints.scheme.toggle_active", date: start_date.to_formatted_s(:govuk_date)) } do %>
         <%= govuk_warning_text text: I18n.t("warnings.scheme.#{action}.existing_logs") %>
         <%= f.govuk_radio_button date_type_question(action),
                                  "default",
-                                 label: { text: "From the start of the current collection period (#{collection_start_date.to_formatted_s(:govuk_date)})" } %>
+                                 label: { text: "From the start of the open collection period (#{start_date.to_formatted_s(:govuk_date)})" } %>
         <%= f.govuk_radio_button date_type_question(action),
                                  "other",
                                  label: { text: "For tenancies starting after a certain date" },
diff --git a/config/locales/en.yml b/config/locales/en.yml
index b6735e3b5..1e7ed58af 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -456,6 +456,7 @@ en:
       toggle_date:
         not_selected: "Select one of the options"
         invalid: "Enter a valid day, month and year"
+        before_creation: "The scheme cannot be deactivated before %{date}, the start of the collection year when it was created"
         out_of_range: "The date must be on or after the %{date}"
       reactivation:
         before_deactivation: "This scheme was deactivated on %{date}. The reactivation date must be on or after deactivation date"
@@ -474,6 +475,7 @@ en:
       toggle_date:
         not_selected: "Select one of the options"
         invalid: "Enter a valid day, month and year"
+        before_creation: "The location cannot be deactivated before %{date}, the date when it was first available"
         out_of_range: "The date must be on or after the %{date}"
       reactivation:
         before_deactivation: "This location was deactivated on %{date}. The reactivation date must be on or after deactivation date"
@@ -594,10 +596,10 @@ en:
       postcode: "For example, SW1P 4DF."
       name: "This is how you refer to this location within your organisation"
       units: "A unit is the space being let. For example, the property might be a block of flats and the unit would be the specific flat being let. A unit can also be a bedroom in a shared house or flat. Do not include spaces used for staff."
-      toggle_active: "If the date is before %{date}, select ‘From the start of the current collection period’ because the previous period has now closed."
+      toggle_active: "If the date is before %{date}, select ‘From the start of the open collection period’ because the previous period has now closed."
       startdate: "For example, 27 3 2021"
     scheme:
-      toggle_active: "If the date is before %{date}, select ‘From the start of the current collection period’ because the previous period has now closed."
+      toggle_active: "If the date is before %{date}, select ‘From the start of the open collection period’ because the previous period has now closed."
     bulk_upload:
       needstype: "General needs housing includes both self-contained and shared housing without support or specific adaptations. Supported housing can include direct access hostels, group homes, residential care and nursing homes."
     offered: "Do not include the offer that led to this letting. This is after the last tenancy ended. If the property is being offered for let for the first time, enter 0."
diff --git a/spec/features/schemes_spec.rb b/spec/features/schemes_spec.rb
index f43dc67eb..1e45634c6 100644
--- a/spec/features/schemes_spec.rb
+++ b/spec/features/schemes_spec.rb
@@ -742,7 +742,7 @@ RSpec.describe "Schemes scheme Features" do
               expect(page).to have_current_path("/schemes/#{scheme.id}/locations")
             end
 
-            context "when location is incative" do
+            context "when location is inactive" do
               context "and I click to view the location" do
                 before do
                   click_link(deactivated_location.postcode)
@@ -766,7 +766,7 @@ RSpec.describe "Schemes scheme Features" do
                   expect(page).to have_current_path("/schemes/#{scheme.id}/locations/#{deactivated_location.id}/new-reactivation")
                   expect(page).to have_content("Reactivate #{deactivated_location.name}")
                   expect(page).to have_content("You’ll be able to add logs with this location if their tenancy start date is on or after the date you enter.")
-                  expect(page).to have_content("If the date is before 1 April 2022, select ‘From the start of the current collection period’ because the previous period has now closed.")
+                  expect(page).to have_content("If the date is before 1 April 2022, select ‘From the start of the open collection period’ because the previous period has now closed.")
                 end
 
                 context "when I press the back button" do
diff --git a/spec/models/location_deactivation_period_spec.rb b/spec/models/location_deactivation_period_spec.rb
new file mode 100644
index 000000000..4e0bb3384
--- /dev/null
+++ b/spec/models/location_deactivation_period_spec.rb
@@ -0,0 +1,83 @@
+require "rails_helper"
+
+RSpec.describe LocationDeactivationPeriod do
+  let(:validator) { LocationDeactivationPeriodValidator.new }
+  let(:location) { FactoryBot.create(:location, startdate: now - 2.years) }
+  let(:record) { FactoryBot.create(:location_deactivation_period, deactivation_date: now, location:) }
+
+  describe "#validate" do
+    around do |example|
+      Timecop.freeze(now) do
+        example.run
+      end
+    end
+
+    context "when not in a crossover period" do
+      let(:now) { Time.utc(2023, 3, 1) }
+
+      context "with a deactivation date before the current collection period" do
+        it "adds an error" do
+          record.deactivation_date = now - 1.year
+          location.location_deactivation_periods.clear
+          validator.validate(record)
+          expect(record.errors[:deactivation_date]).to include "The date must be on or after the 1 April 2022"
+        end
+      end
+
+      context "with a deactivation date in the current collection period" do
+        it "does not add an error" do
+          record.deactivation_date = now - 1.day
+          location.location_deactivation_periods.clear
+          validator.validate(record)
+          expect(record.errors).to be_empty
+        end
+      end
+    end
+
+    context "when in a crossover period" do
+      let(:now) { Time.utc(2023, 5, 1) }
+
+      context "with a deactivation date before the previous collection period" do
+        it "does not add an error" do
+          record.deactivation_date = now - 2.years
+          location.location_deactivation_periods.clear
+          validator.validate(record)
+          expect(record.errors[:deactivation_date]).to include "The date must be on or after the 1 April 2022"
+        end
+      end
+
+      context "with a deactivation date in the previous collection period" do
+        it "does not add an error" do
+          record.deactivation_date = now - 1.year
+          location.location_deactivation_periods.clear
+          validator.validate(record)
+          expect(record.errors).to be_empty
+        end
+      end
+
+      context "with a deactivation date in the current collection period" do
+        it "does not add an error" do
+          record.deactivation_date = now - 1.day
+          location.location_deactivation_periods.clear
+          validator.validate(record)
+          expect(record.errors).to be_empty
+        end
+      end
+
+      context "but the location was created in the current collection period" do
+        let(:location) { FactoryBot.create(:location, startdate:) }
+        let(:startdate) { now - 2.days }
+
+        context "with a deactivation date in the previous collection period" do
+          it "adds an error" do
+            record.deactivation_date = now - 1.year
+            location.location_deactivation_periods.clear
+            validator.validate(record)
+            start_date = startdate.to_formatted_s(:govuk_date)
+            expect(record.errors[:deactivation_date]).to include "The location cannot be deactivated before #{start_date}, the date when it was first available"
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/spec/models/scheme_deactivation_period_spec.rb b/spec/models/scheme_deactivation_period_spec.rb
new file mode 100644
index 000000000..eb46ee62f
--- /dev/null
+++ b/spec/models/scheme_deactivation_period_spec.rb
@@ -0,0 +1,68 @@
+require "rails_helper"
+
+RSpec.describe SchemeDeactivationPeriod do
+  let(:validator) { SchemeDeactivationPeriodValidator.new }
+  let(:scheme) { FactoryBot.create(:scheme, created_at: now - 2.years) }
+  let(:record) { FactoryBot.create(:scheme_deactivation_period, deactivation_date: now, scheme:) }
+
+  describe "#validate" do
+    around do |example|
+      Timecop.freeze(now) do
+        example.run
+      end
+    end
+
+    context "when not in a crossover period" do
+      let(:now) { Time.utc(2023, 3, 1) }
+
+      context "with a deactivation date before the current collection period" do
+        it "adds an error" do
+          record.deactivation_date = now - 1.year
+          scheme.scheme_deactivation_periods.clear
+          validator.validate(record)
+          expect(record.errors[:deactivation_date]).to include("The date must be on or after the 1 April 2022")
+        end
+      end
+
+      context "with a deactivation date in the current collection period" do
+        it "does not add an error" do
+          record.deactivation_date = now - 1.day
+          scheme.scheme_deactivation_periods.clear
+          validator.validate(record)
+          expect(record.errors[:deactivation_date]).to be_empty
+        end
+      end
+    end
+
+    context "when in a crossover period" do
+      let(:now) { Time.utc(2023, 5, 1) }
+
+      context "with a deactivation date before the previous collection period" do
+        it "does not add an error" do
+          record.deactivation_date = now - 2.years
+          scheme.scheme_deactivation_periods.clear
+          validator.validate(record)
+          expect(record.errors[:deactivation_date]).to include("The date must be on or after the 1 April 2022")
+        end
+      end
+
+      context "with a deactivation date in the previous collection period" do
+        it "does not add an error" do
+          record.deactivation_date = now - 1.year
+          scheme.scheme_deactivation_periods.clear
+          validator.validate(record)
+          expect(record.errors[:deactivation_date]).to be_empty
+        end
+      end
+
+      context "with a deactivation date in the current collection period" do
+        it "does not add an error" do
+          record.deactivation_date = now - 1.day
+          scheme.scheme_deactivation_periods.clear
+          validator.validate(record)
+          expect(record.errors[:deactivation_date]).to be_empty
+        end
+      end
+    end
+  end
+end

From 97e1463fda292d6fbcf128fe4b71b1a0ac53a8e8 Mon Sep 17 00:00:00 2001
From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com>
Date: Thu, 6 Apr 2023 11:59:25 +0100
Subject: [PATCH 3/7] Re-render page with errors (#1523)

---
 app/controllers/form_controller.rb      | 29 ++++++--------
 app/views/form/page.html.erb            |  2 +-
 config/routes.rb                        |  2 +
 spec/features/form/page_routing_spec.rb |  4 +-
 spec/requests/form_controller_spec.rb   | 51 ++++++++++++-------------
 5 files changed, 40 insertions(+), 48 deletions(-)

diff --git a/app/controllers/form_controller.rb b/app/controllers/form_controller.rb
index 43a805d9f..c584e40ee 100644
--- a/app/controllers/form_controller.rb
+++ b/app/controllers/form_controller.rb
@@ -1,7 +1,7 @@
 class FormController < ApplicationController
   before_action :authenticate_user!
-  before_action :find_resource, only: %i[submit_form review]
-  before_action :find_resource_by_named_id, except: %i[submit_form review]
+  before_action :find_resource, only: %i[review]
+  before_action :find_resource_by_named_id, except: %i[review]
   before_action :check_collection_period, only: %i[submit_form show_page]
 
   def submit_form
@@ -11,16 +11,15 @@ class FormController < ApplicationController
       mandatory_questions_with_no_response = mandatory_questions_with_no_response(responses_for_page)
 
       if mandatory_questions_with_no_response.empty? && @log.update(responses_for_page.merge(updated_by: current_user))
-        session[:errors] = session[:fields] = nil
         redirect_to(successful_redirect_path)
       else
-        redirect_path = "#{@log.model_name.param_key}_#{@page.id}_path"
         mandatory_questions_with_no_response.map do |question|
           @log.errors.add question.id.to_sym, question.unanswered_error_message
         end
-        session[:errors] = @log.errors.to_json
         Rails.logger.info "User triggered validation(s) on: #{@log.errors.map(&:attribute).join(', ')}"
-        redirect_to(send(redirect_path, @log))
+        @subsection = form.subsection_for_page(@page)
+        restore_error_field_values(@page&.questions)
+        render "form/page"
       end
     else
       render_not_found
@@ -47,7 +46,6 @@ class FormController < ApplicationController
 
   def show_page
     if @log
-      restore_error_field_values
       page_id = request.path.split("/")[-1].underscore
       @page = form.get_page(page_id)
       @subsection = form.subsection_for_page(@page)
@@ -63,17 +61,12 @@ class FormController < ApplicationController
 
 private
 
-  def restore_error_field_values
-    if session["errors"]
-      JSON(session["errors"]).each do |field, messages|
-        messages.each { |message| @log.errors.add field.to_sym, message }
-      end
-    end
-    if session["fields"]
-      session["fields"].each do |field, value|
-        if form.get_question(field, @log)&.type != "date" && @log.attributes.key?(field)
-          @log[field] = value
-        end
+  def restore_error_field_values(questions)
+    return unless questions
+
+    questions.each do |question|
+      if question&.type == "date" && @log.attributes.key?(question.id)
+        @log[question.id] = @log.send("#{question.id}_was")
       end
     end
   end
diff --git a/app/views/form/page.html.erb b/app/views/form/page.html.erb
index 5469219e3..45711b0f1 100644
--- a/app/views/form/page.html.erb
+++ b/app/views/form/page.html.erb
@@ -5,7 +5,7 @@
 <% end %>
 
 <div data-controller="govukfrontend"></div>
-<%= form_with model: @log, url: form_lettings_log_path(@log), method: "post", local: true do |f| %>
+<%= form_with model: @log, url: request.original_url, method: "post", local: true do |f| %>
   <div class="govuk-grid-row">
     <div class="govuk-grid-column-<%= @page.questions[0].type == "interruption_screen" ? "full-from-desktop" : "two-thirds-from-desktop" %>">
       <% remove_other_page_errors(@log, @page) %>
diff --git a/config/routes.rb b/config/routes.rb
index 1f00815c8..c01f38027 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -164,6 +164,7 @@ Rails.application.routes.draw do
     FormHandler.instance.lettings_forms.each do |_key, form|
       form.pages.map do |page|
         get page.id.to_s.dasherize, to: "form#show_page"
+        post page.id.to_s.dasherize, to: "form#submit_form"
       end
 
       form.subsections.map do |subsection|
@@ -190,6 +191,7 @@ Rails.application.routes.draw do
     FormHandler.instance.sales_forms.each do |_key, form|
       form.pages.map do |page|
         get page.id.to_s.dasherize, to: "form#show_page"
+        post page.id.to_s.dasherize, to: "form#submit_form"
       end
 
       form.subsections.map do |subsection|
diff --git a/spec/features/form/page_routing_spec.rb b/spec/features/form/page_routing_spec.rb
index 0ba8ece6e..98c370cff 100644
--- a/spec/features/form/page_routing_spec.rb
+++ b/spec/features/form/page_routing_spec.rb
@@ -85,11 +85,11 @@ RSpec.describe "Form Page Routing" do
     context "when answer is invalid" do
       it "shows error with invalid value in the field" do
         visit("/lettings-logs/#{id}/property-postcode")
-        fill_in("lettings-log-postcode-full-field", with: "fake_postcode")
+        fill_in("lettings-log-postcode-full-field", with: "FAKE_POSTCODE")
         click_button("Save and continue")
 
         expect(page).to have_current_path("/lettings-logs/#{id}/property-postcode")
-        expect(find("#lettings-log-postcode-full-field-error").value).to eq("fake_postcode")
+        expect(find("#lettings-log-postcode-full-field-error").value).to eq("FAKE_POSTCODE")
       end
 
       it "does not reset the displayed date" do
diff --git a/spec/requests/form_controller_spec.rb b/spec/requests/form_controller_spec.rb
index eb47fdb87..9b109ef36 100644
--- a/spec/requests/form_controller_spec.rb
+++ b/spec/requests/form_controller_spec.rb
@@ -57,7 +57,7 @@ RSpec.describe FormController, type: :request do
 
     describe "POST" do
       it "does not let you post form answers to lettings logs you don't have access to" do
-        post "/lettings-logs/#{lettings_log.id}/form", params: {}
+        post "/lettings-logs/#{lettings_log.id}/net-income", params: {}
         expect(response).to redirect_to("/account/sign-in")
       end
     end
@@ -102,7 +102,7 @@ RSpec.describe FormController, type: :request do
       end
 
       it "resets created by and renders the next page" do
-        post "/lettings-logs/#{lettings_log.id}/form", params: params
+        post "/lettings-logs/#{lettings_log.id}/net-income", params: params
         expect(response).to redirect_to("/lettings-logs/#{lettings_log.id}/created-by")
         follow_redirect!
         lettings_log.reload
@@ -127,7 +127,7 @@ RSpec.describe FormController, type: :request do
       end
 
       it "does not reset created by" do
-        post "/lettings-logs/#{lettings_log.id}/form", params: params
+        post "/lettings-logs/#{lettings_log.id}/net-income", params: params
         expect(response).to redirect_to("/lettings-logs/#{lettings_log.id}/created-by")
         follow_redirect!
         lettings_log.reload
@@ -152,7 +152,7 @@ RSpec.describe FormController, type: :request do
       end
 
       it "does not reset created by" do
-        post "/lettings-logs/#{lettings_log.id}/form", params: params
+        post "/lettings-logs/#{lettings_log.id}/stock-owner", params: params
         expect(response).to redirect_to("/lettings-logs/#{lettings_log.id}/managing-organisation")
         follow_redirect!
         lettings_log.reload
@@ -177,7 +177,7 @@ RSpec.describe FormController, type: :request do
       end
 
       it "does not reset created by" do
-        post "/lettings-logs/#{lettings_log.id}/form", params: params
+        post "/lettings-logs/#{lettings_log.id}/stock-owner", params: params
         expect(response).to redirect_to("/lettings-logs/#{lettings_log.id}/managing-organisation")
         follow_redirect!
         lettings_log.reload
@@ -400,22 +400,21 @@ RSpec.describe FormController, type: :request do
           end
 
           it "re-renders the same page with errors if validation fails" do
-            post "/lettings-logs/#{lettings_log.id}/form", params: params
-            expect(response).to redirect_to("/lettings-logs/#{lettings_log.id}/#{page_id.dasherize}")
-            follow_redirect!
+            post "/lettings-logs/#{lettings_log.id}/#{page_id.dasherize}", params: params
             expect(page).to have_content("There is a problem")
+            expect(page).to have_content("Error: What is the tenant’s age?")
           end
 
           it "resets errors when fixed" do
-            post "/lettings-logs/#{lettings_log.id}/form", params: params
-            post "/lettings-logs/#{lettings_log.id}/form", params: valid_params
+            post "/lettings-logs/#{lettings_log.id}/#{page_id.dasherize}", params: params
+            post "/lettings-logs/#{lettings_log.id}/#{page_id.dasherize}", params: valid_params
             get "/lettings-logs/#{lettings_log.id}/#{page_id.dasherize}"
             expect(page).not_to have_content("There is a problem")
           end
 
           it "logs that validation was triggered" do
             expect(Rails.logger).to receive(:info).with("User triggered validation(s) on: age1").once
-            post "/lettings-logs/#{lettings_log.id}/form", params:
+            post "/lettings-logs/#{lettings_log.id}/#{page_id.dasherize}", params:
           end
 
           context "when the number of days is too high for the month" do
@@ -433,8 +432,7 @@ RSpec.describe FormController, type: :request do
             end
 
             it "validates the date correctly" do
-              post "/lettings-logs/#{lettings_log.id}/form", params: params
-              follow_redirect!
+              post "/lettings-logs/#{lettings_log.id}/#{page_id.dasherize}", params: params
               expect(page).to have_content("There is a problem")
             end
           end
@@ -465,10 +463,9 @@ RSpec.describe FormController, type: :request do
           end
 
           it "re-renders the same page with errors if validation fails" do
-            post "/lettings-logs/#{lettings_log.id}/form", params: params
-            expect(response).to redirect_to("/lettings-logs/#{lettings_log.id}/managing-organisation")
-            follow_redirect!
+            post "/lettings-logs/#{lettings_log.id}/managing-organisation", params: params
             expect(page).to have_content("There is a problem")
+            expect(page).to have_content("Error: Which organisation manages this letting?")
           end
         end
 
@@ -486,7 +483,7 @@ RSpec.describe FormController, type: :request do
           end
 
           before do
-            post "/lettings-logs/#{lettings_log.id}/form", params:
+            post "/lettings-logs/#{lettings_log.id}/#{page_id.dasherize}", params:
           end
 
           it "re-renders the same page with errors if validation fails" do
@@ -524,7 +521,7 @@ RSpec.describe FormController, type: :request do
 
               before do
                 lettings_log.update!(postcode_known: 1, postcode_full: "NW1 8RR")
-                post "/lettings-logs/#{lettings_log.id}/form", params: valid_params
+                post "/lettings-logs/#{lettings_log.id}/#{page_id.dasherize}", params: valid_params
               end
 
               it "does not require you to answer that question" do
@@ -558,14 +555,14 @@ RSpec.describe FormController, type: :request do
         end
 
         it "sets checked items to true" do
-          post "/lettings-logs/#{lettings_log.id}/form", params: lettings_log_form_params
+          post "/lettings-logs/#{lettings_log.id}/accessibility-requirements", params: lettings_log_form_params
           lettings_log.reload
 
           expect(lettings_log.housingneeds_b).to eq(1)
         end
 
         it "sets previously submitted items to false when resubmitted with new values" do
-          post "/lettings-logs/#{lettings_log.id}/form", params: new_lettings_log_form_params
+          post "/lettings-logs/#{lettings_log.id}/accessibility-requirements", params: new_lettings_log_form_params
           lettings_log.reload
 
           expect(lettings_log.housingneeds_b).to eq(0)
@@ -609,7 +606,7 @@ RSpec.describe FormController, type: :request do
 
           it "updates both question fields" do
             allow(page).to receive(:questions).and_return(questions_for_page)
-            post "/lettings-logs/#{lettings_log.id}/form", params: lettings_log_form_params
+            post "/lettings-logs/#{lettings_log.id}/#{page.id.dasherize}", params: lettings_log_form_params
             lettings_log.reload
 
             expect(lettings_log.housingneeds_a).to eq(1)
@@ -650,16 +647,16 @@ RSpec.describe FormController, type: :request do
         end
 
         it "routes to the appropriate conditional page based on the question answer of the current page" do
-          post "/lettings-logs/#{lettings_log.id}/form", params: lettings_log_form_conditional_question_yes_params
+          post "/lettings-logs/#{lettings_log.id}/property-wheelchair-accessible", params: lettings_log_form_conditional_question_yes_params
           expect(response).to redirect_to("/lettings-logs/#{lettings_log.id}/conditional-question-yes-page")
 
-          post "/lettings-logs/#{lettings_log.id}/form", params: lettings_log_form_conditional_question_no_params
+          post "/lettings-logs/#{lettings_log.id}/property-wheelchair-accessible", params: lettings_log_form_conditional_question_no_params
           expect(response).to redirect_to("/lettings-logs/#{lettings_log.id}/conditional-question-no-page")
         end
 
         it "routes to the page if at least one of the condition sets is met" do
-          post "/lettings-logs/#{lettings_log.id}/form", params: lettings_log_form_conditional_question_wchair_yes_params
-          post "/lettings-logs/#{lettings_log.id}/form", params: lettings_log_form_conditional_question_no_params
+          post "/lettings-logs/#{lettings_log.id}/property-wheelchair-accessible", params: lettings_log_form_conditional_question_wchair_yes_params
+          post "/lettings-logs/#{lettings_log.id}/property-wheelchair-accessible", params: lettings_log_form_conditional_question_no_params
           expect(response).to redirect_to("/lettings-logs/#{lettings_log.id}/conditional-question-yes-page")
         end
       end
@@ -690,7 +687,7 @@ RSpec.describe FormController, type: :request do
             completed_lettings_log.update!(ecstat1: 1, earnings: 130, hhmemb: 1) # we're not routing to that page, so it gets cleared?
             allow(completed_lettings_log).to receive(:net_income_soft_validation_triggered?).and_return(true)
             allow(completed_lettings_log.form).to receive(:end_date).and_return(Time.zone.today + 1.day)
-            post "/lettings-logs/#{completed_lettings_log.id}/form", params: interrupt_params, headers: headers.merge({ "HTTP_REFERER" => referrer })
+            post "/lettings-logs/#{completed_lettings_log.id}/net-income-value-check", params: interrupt_params, headers: headers.merge({ "HTTP_REFERER" => referrer })
           end
 
           context "when yes is answered" do
@@ -722,7 +719,7 @@ RSpec.describe FormController, type: :request do
         end
 
         before do
-          post "/lettings-logs/#{unauthorized_lettings_log.id}/form", params: {}
+          post "/lettings-logs/#{unauthorized_lettings_log.id}/net-income", params: {}
         end
 
         it "does not let you post form answers to lettings logs you don't have access to" do

From 69aa90a1cd05cc13aa2bd27a6da6eb76a92d9195 Mon Sep 17 00:00:00 2001
From: Jack <113976590+bibblobcode@users.noreply.github.com>
Date: Thu, 6 Apr 2023 12:12:15 +0100
Subject: [PATCH 4/7] CLDC-1975 Format money values with 2 decimals (#1493)

* Format errors on money amounts with 2 decimals

* CLDC-1975 Format money input and error messages

* format discounted_ownership_value

* Format more locales

* Update lettings_log tests

* Fix import service spec
---
 app/helpers/money_formatting_helper.rb        | 23 +++++++
 app/models/lettings_log.rb                    |  5 +-
 app/models/log.rb                             |  4 --
 app/models/sales_log.rb                       |  3 +-
 .../validations/financial_validations.rb      | 29 ++++++---
 .../sales/sale_information_validations.rb     |  9 ++-
 .../form/_numeric_output_question.html.erb    |  3 +-
 app/views/form/_numeric_question.html.erb     | 11 +++-
 config/locales/en.yml                         | 16 ++---
 spec/helpers/money_formatting_helper_spec.rb  | 41 ++++++++++++
 spec/models/lettings_log_spec.rb              |  8 +--
 spec/models/sales_log_spec.rb                 |  2 +-
 .../validations/financial_validations_spec.rb | 40 ++++++------
 .../sale_information_validations_spec.rb      | 63 +++++++++++--------
 .../lettings_logs_import_service_spec.rb      |  8 +--
 15 files changed, 184 insertions(+), 81 deletions(-)
 create mode 100644 app/helpers/money_formatting_helper.rb
 create mode 100644 spec/helpers/money_formatting_helper_spec.rb

diff --git a/app/helpers/money_formatting_helper.rb b/app/helpers/money_formatting_helper.rb
new file mode 100644
index 000000000..246d910fe
--- /dev/null
+++ b/app/helpers/money_formatting_helper.rb
@@ -0,0 +1,23 @@
+module MoneyFormattingHelper
+  include ActionView::Helpers::NumberHelper
+
+  def format_money_input(log:, question:)
+    value = log[question.id]
+
+    return unless value
+    return value unless question.prefix == "£"
+
+    number_with_precision(
+      value,
+      precision: 2,
+    )
+  end
+
+  def format_as_currency(num_string)
+    number_to_currency(
+      num_string,
+      unit: "£",
+      precision: 2,
+    )
+  end
+end
diff --git a/app/models/lettings_log.rb b/app/models/lettings_log.rb
index 4a0651a29..0ddc921be 100644
--- a/app/models/lettings_log.rb
+++ b/app/models/lettings_log.rb
@@ -20,6 +20,7 @@ class LettingsLog < Log
   include DerivedVariables::LettingsLogVariables
   include Validations::DateValidations
   include Validations::FinancialValidations
+  include MoneyFormattingHelper
 
   has_paper_trail
 
@@ -137,7 +138,7 @@ class LettingsLog < Log
   def weekly_to_value_per_period(field_value)
     num_of_weeks = NUM_OF_WEEKS_FROM_PERIOD[period]
 
-    ((field_value * 52) / num_of_weeks).round(2)
+    format_as_currency((field_value * 52) / num_of_weeks)
   end
 
   def applicable_income_range
@@ -638,7 +639,7 @@ private
     num_of_weeks = NUM_OF_WEEKS_FROM_PERIOD[period]
     return "" unless value && num_of_weeks
 
-    (value * 52 / num_of_weeks).round(2)
+    format_as_currency((value * 52 / num_of_weeks))
   end
 
   def fully_wheelchair_accessible?
diff --git a/app/models/log.rb b/app/models/log.rb
index bc036d356..6a36b948d 100644
--- a/app/models/log.rb
+++ b/app/models/log.rb
@@ -217,8 +217,4 @@ private
     self[is_inferred_key] = false
     self[postcode_key] = nil
   end
-
-  def format_as_currency(num_string)
-    ActionController::Base.helpers.number_to_currency(num_string, unit: "£")
-  end
 end
diff --git a/app/models/sales_log.rb b/app/models/sales_log.rb
index b0aed712a..b86124a2f 100644
--- a/app/models/sales_log.rb
+++ b/app/models/sales_log.rb
@@ -18,6 +18,7 @@ class SalesLog < Log
   include DerivedVariables::SalesLogVariables
   include Validations::Sales::SoftValidations
   include Validations::SoftValidations
+  include MoneyFormattingHelper
 
   self.inheritance_column = :_type_disabled
 
@@ -215,7 +216,7 @@ class SalesLog < Log
   def expected_shared_ownership_deposit_value
     return unless value && equity
 
-    (value * equity / 100).round(2)
+    format_as_currency(value * equity / 100)
   end
 
   def process_postcode(postcode, postcode_known_key, la_inferred_key, la_key)
diff --git a/app/models/validations/financial_validations.rb b/app/models/validations/financial_validations.rb
index 5fba288bb..3a0014119 100644
--- a/app/models/validations/financial_validations.rb
+++ b/app/models/validations/financial_validations.rb
@@ -1,5 +1,6 @@
 module Validations::FinancialValidations
   include Validations::SharedValidations
+  include MoneyFormattingHelper
   # Validations methods need to be called 'validate_<page_name>' to run on model save
   # or 'validate_' to run on submit as well
   def validate_outstanding_rent_amount(record)
@@ -24,12 +25,26 @@ module Validations::FinancialValidations
   def validate_net_income(record)
     if record.ecstat1 && record.weekly_net_income
       if record.weekly_net_income > record.applicable_income_range.hard_max
-        record.errors.add :earnings, :over_hard_max, message: I18n.t("validations.financial.earnings.over_hard_max", hard_max: record.applicable_income_range.hard_max)
-        record.errors.add :ecstat1, :over_hard_max, message: I18n.t("validations.financial.ecstat.over_hard_max", hard_max: record.applicable_income_range.hard_max)
+        hard_max = format_as_currency(record.applicable_income_range.hard_max)
+        record.errors.add(
+          :earnings,
+          :over_hard_max,
+          message: I18n.t("validations.financial.earnings.over_hard_max", hard_max:),
+        )
+        record.errors.add(
+          :ecstat1,
+          :over_hard_max,
+          message: I18n.t("validations.financial.ecstat.over_hard_max", hard_max:),
+        )
       end
 
       if record.weekly_net_income < record.applicable_income_range.hard_min
-        record.errors.add :earnings, :under_hard_min, message: I18n.t("validations.financial.earnings.under_hard_min", hard_min: record.applicable_income_range.hard_min)
+        hard_min = format_as_currency(record.applicable_income_range.hard_min)
+        record.errors.add(
+          :earnings,
+          :under_hard_min,
+          message: I18n.t("validations.financial.earnings.under_hard_min", hard_min:),
+        )
       end
     end
 
@@ -120,10 +135,10 @@ module Validations::FinancialValidations
       elsif !weekly_value_in_range(record, "chcharge", 10, 1000)
         max_chcharge = record.weekly_to_value_per_period(1000)
         min_chcharge = record.weekly_to_value_per_period(10)
-        max_chcharge = [record.form.get_question("chcharge", record).prefix, max_chcharge].join("") if record.form.get_question("chcharge", record).present?
-        min_chcharge = [record.form.get_question("chcharge", record).prefix, min_chcharge].join("") if record.form.get_question("chcharge", record).present?
-        record.errors.add :period, I18n.t("validations.financial.carehome.out_of_range", period:, min_chcharge:, max_chcharge:)
-        record.errors.add :chcharge, :out_of_range, message: I18n.t("validations.financial.carehome.out_of_range", period:, min_chcharge:, max_chcharge:)
+        message = I18n.t("validations.financial.carehome.out_of_range", period:, min_chcharge:, max_chcharge:)
+
+        record.errors.add :period, message
+        record.errors.add :chcharge, :out_of_range, message: message
       end
     end
   end
diff --git a/app/models/validations/sales/sale_information_validations.rb b/app/models/validations/sales/sale_information_validations.rb
index fcd1854f4..e4714e81a 100644
--- a/app/models/validations/sales/sale_information_validations.rb
+++ b/app/models/validations/sales/sale_information_validations.rb
@@ -1,5 +1,6 @@
 module Validations::Sales::SaleInformationValidations
   include CollectionTimeHelper
+  include MoneyFormattingHelper
 
   def validate_practical_completion_date_before_saledate(record)
     return if record.saledate.blank? || record.hodate.blank?
@@ -54,7 +55,13 @@ module Validations::Sales::SaleInformationValidations
 
     if record.mortgage_deposit_and_grant_total != record.value_with_discount && record.discounted_ownership_sale?
       %i[mortgage deposit grant value discount ownershipsch].each do |field|
-        record.errors.add field, I18n.t("validations.sale_information.discounted_ownership_value", value_with_discount: sprintf("%.2f", record.value_with_discount))
+        record.errors.add(
+          field,
+          I18n.t(
+            "validations.sale_information.discounted_ownership_value",
+            value_with_discount: format_as_currency(record.value_with_discount),
+          ),
+        )
       end
     end
   end
diff --git a/app/views/form/_numeric_output_question.html.erb b/app/views/form/_numeric_output_question.html.erb
index 8892523aa..7d681f7ea 100644
--- a/app/views/form/_numeric_output_question.html.erb
+++ b/app/views/form/_numeric_output_question.html.erb
@@ -16,7 +16,8 @@
             type="number"
             name="lettings_log[tcharge]"
             for="<%= question.fields_added.present? ? question.fields_added.map { |x| "lettings-log-#{x}-field" }.join(" ") : "" %>">
-      <%= lettings_log[question.id] %></output>
+      <%= format_money_input(log: lettings_log, question:) %>
+    </output>
     <span class="govuk-input__suffix"><%= question.suffix_label(lettings_log) %></span>
   </div>
 </div>
diff --git a/app/views/form/_numeric_question.html.erb b/app/views/form/_numeric_question.html.erb
index 0aeb63801..b92d0f736 100644
--- a/app/views/form/_numeric_question.html.erb
+++ b/app/views/form/_numeric_question.html.erb
@@ -1,14 +1,19 @@
 <%= render partial: "form/guidance/#{question.guidance_partial}" if question.top_guidance? %>
 
-<%= f.govuk_number_field question.id.to_sym,
+<%= f.govuk_number_field(
+  question.id.to_sym,
   caption: caption(caption_text, page_header, conditional),
   label: legend(question, page_header, conditional),
   hint: { text: question.hint_text&.html_safe },
-  min: question.min, max: question.max, step: question.step,
+  min: question.min,
+  max: question.max,
+  step: question.step,
   width: question.width,
   readonly: question.read_only?,
   prefix_text: question.prefix.to_s,
   suffix_text: question.suffix_label(@log),
-  **stimulus_html_attributes(question) %>
+  value: format_money_input(log: @log, question:),
+  **stimulus_html_attributes(question),
+) %>
 
 <%= render partial: "form/guidance/#{question.guidance_partial}" if question.bottom_guidance? %>
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 1e7ed58af..68e1d72fc 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -248,15 +248,15 @@ en:
       benefits:
         part_or_full_time: "Answer cannot be ‘all’ for income from Universal Credit, state pensions or benefits if the tenant or their partner works part-time or full-time"
       earnings:
-        over_hard_max: "Net income cannot be greater than £%{hard_max} per week given the tenant’s working situation"
-        under_hard_min: "Net income cannot be less than £%{hard_min} per week given the tenant’s working situation"
+        over_hard_max: "Net income cannot be greater than %{hard_max} per week given the tenant’s working situation"
+        under_hard_min: "Net income cannot be less than %{hard_min} per week given the tenant’s working situation"
         freq_missing: "Select how often the household receives income"
         earnings_missing: "Enter how much income the household has in total"
       income:
-        over_hard_max_for_london: "Income must not exceed £90,000 for properties within London local authorities"
-        over_hard_max_for_outside_london: "Income must not exceed £80,000 for properties outside London local authorities"
-        combined_over_hard_max_for_london: "Combined income must not exceed £90,000 for properties within London local authorities"
-        combined_over_hard_max_for_outside_london: "Combined income must not exceed £80,000 for properties outside London local authorities"
+        over_hard_max_for_london: "Income must not exceed £90,000.00 for properties within London local authorities"
+        over_hard_max_for_outside_london: "Income must not exceed £80,000.00 for properties outside London local authorities"
+        combined_over_hard_max_for_london: "Combined income must not exceed £90,000.00 for properties within London local authorities"
+        combined_over_hard_max_for_outside_london: "Combined income must not exceed £80,000.00 for properties outside London local authorities"
         child_has_income: "Child's income must be £0"
       negative_currency: "Enter an amount above 0"
       rent:
@@ -498,9 +498,9 @@ en:
         must_be_after_hodate: "Sale completion date must be after practical completion or handover date"
       previous_property_type:
         property_type_bedsit: "A bedsit cannot have more than 1 bedroom"
-      discounted_ownership_value: "Mortgage, deposit, and grant total must equal £%{value_with_discount}"
+      discounted_ownership_value: "Mortgage, deposit, and grant total must equal %{value_with_discount}"
       monthly_rent:
-        higher_than_expected: "Basic monthly rent must be between £0 and £9,999"
+        higher_than_expected: "Basic monthly rent must be between £0.00 and £9,999.00"
 
   soft_validations:
     net_income:
diff --git a/spec/helpers/money_formatting_helper_spec.rb b/spec/helpers/money_formatting_helper_spec.rb
new file mode 100644
index 000000000..cd16ea86c
--- /dev/null
+++ b/spec/helpers/money_formatting_helper_spec.rb
@@ -0,0 +1,41 @@
+require "rails_helper"
+
+RSpec.describe MoneyFormattingHelper do
+  describe "#format_money_input" do
+    let!(:log) { create(:lettings_log, :completed, brent: 1000) }
+    let(:question) { instance_double(Form::Question, id: "brent", prefix:) }
+
+    context "with £ prefix" do
+      let(:prefix) { "£" }
+
+      it "returns formatted input" do
+        expect(format_money_input(log:, question:)).to eq("1000.00")
+      end
+    end
+
+    context "with other prefix" do
+      let(:prefix) { "other" }
+
+      it "does not format the input" do
+        expect(format_money_input(log:, question:)).to eq(BigDecimal(1000))
+      end
+    end
+
+    context "without prefix" do
+      let(:prefix) { nil }
+
+      it "does not format the input" do
+        expect(format_money_input(log:, question:)).to eq(BigDecimal(1000))
+      end
+    end
+
+    context "when value is nil" do
+      let(:prefix) { "£" }
+      let(:log) { create(:lettings_log, brent: nil) }
+
+      it "does not format the input" do
+        expect(format_money_input(log:, question:)).to be_nil
+      end
+    end
+  end
+end
diff --git a/spec/models/lettings_log_spec.rb b/spec/models/lettings_log_spec.rb
index 2358317af..1e6fd3244 100644
--- a/spec/models/lettings_log_spec.rb
+++ b/spec/models/lettings_log_spec.rb
@@ -2605,24 +2605,24 @@ RSpec.describe LettingsLog do
     context "when period is weekly for 52 weeks" do
       it "returns weekly soft min for 52 weeks" do
         lettings_log.period = 1
-        expect(lettings_log.soft_min_for_period).to eq("100.0 every week")
+        expect(lettings_log.soft_min_for_period).to eq("£100.00 every week")
       end
 
       it "returns weekly soft max for 52 weeks" do
         lettings_log.period = 1
-        expect(lettings_log.soft_max_for_period).to eq("400.0 every week")
+        expect(lettings_log.soft_max_for_period).to eq("£400.00 every week")
       end
     end
 
     context "when period is weekly for 47 weeks" do
       it "returns weekly soft min for 47 weeks" do
         lettings_log.period = 8
-        expect(lettings_log.soft_min_for_period).to eq("110.64 every week")
+        expect(lettings_log.soft_min_for_period).to eq("£110.64 every week")
       end
 
       it "returns weekly soft max for 47 weeks" do
         lettings_log.period = 8
-        expect(lettings_log.soft_max_for_period).to eq("442.55 every week")
+        expect(lettings_log.soft_max_for_period).to eq("£442.55 every week")
       end
     end
   end
diff --git a/spec/models/sales_log_spec.rb b/spec/models/sales_log_spec.rb
index ca72794a5..0632af547 100644
--- a/spec/models/sales_log_spec.rb
+++ b/spec/models/sales_log_spec.rb
@@ -498,7 +498,7 @@ RSpec.describe SalesLog, type: :model do
     let!(:completed_sales_log) { create(:sales_log, :completed, ownershipsch: 1, type: 2, value: 1000, equity: 50) }
 
     it "is set to completed for a completed sales log" do
-      expect(completed_sales_log.expected_shared_ownership_deposit_value).to eq(500)
+      expect(completed_sales_log.expected_shared_ownership_deposit_value).to eq("£500.00")
     end
   end
 
diff --git a/spec/models/validations/financial_validations_spec.rb b/spec/models/validations/financial_validations_spec.rb
index 8cb777029..6ae2a4607 100644
--- a/spec/models/validations/financial_validations_spec.rb
+++ b/spec/models/validations/financial_validations_spec.rb
@@ -202,7 +202,7 @@ RSpec.describe Validations::FinancialValidations do
         record.ecstat1 = 1
         financial_validator.validate_net_income(record)
         expect(record.errors["earnings"])
-          .to include(match I18n.t("validations.financial.earnings.over_hard_max", hard_max: 1230))
+          .to eq(["Net income cannot be greater than £1,230.00 per week given the tenant’s working situation"])
       end
     end
 
@@ -213,7 +213,7 @@ RSpec.describe Validations::FinancialValidations do
         record.ecstat1 = 1
         financial_validator.validate_net_income(record)
         expect(record.errors["earnings"])
-          .to include(match I18n.t("validations.financial.earnings.under_hard_min", hard_min: 90))
+          .to eq(["Net income cannot be less than £90.00 per week given the tenant’s working situation"])
       end
     end
   end
@@ -913,15 +913,15 @@ RSpec.describe Validations::FinancialValidations do
         record.is_carehome = 1
       end
 
-      context "and charges are over the valid limit (£1000 per week)" do
+      context "and charges are over the valid limit (£1,000 per week)" do
         it "validates charge when period is weekly for 52 weeks" do
           record.period = 1
           record.chcharge = 1001
           financial_validator.validate_care_home_charges(record)
           expect(record.errors["chcharge"])
-            .to include(match I18n.t("validations.financial.carehome.out_of_range", min_chcharge: 10, period: "weekly for 52 weeks", max_chcharge: 1000))
+            .to include("Household rent and other charges must be between £10.00 and £1,000.00 if paying weekly for 52 weeks")
           expect(record.errors["period"])
-            .to include(match I18n.t("validations.financial.carehome.out_of_range", min_chcharge: 10, period: "weekly for 52 weeks", max_chcharge: 1000))
+            .to include("Household rent and other charges must be between £10.00 and £1,000.00 if paying weekly for 52 weeks")
         end
 
         it "validates charge when period is monthly" do
@@ -929,9 +929,9 @@ RSpec.describe Validations::FinancialValidations do
           record.chcharge = 4334
           financial_validator.validate_care_home_charges(record)
           expect(record.errors["chcharge"])
-            .to include(match I18n.t("validations.financial.carehome.out_of_range", min_chcharge: 43, period: "every calendar month", max_chcharge: 4333))
+            .to include("Household rent and other charges must be between £43.00 and £4,333.00 if paying every calendar month")
           expect(record.errors["period"])
-            .to include(match I18n.t("validations.financial.carehome.out_of_range", min_chcharge: 43, period: "every calendar month", max_chcharge: 4333))
+            .to include("Household rent and other charges must be between £43.00 and £4,333.00 if paying every calendar month")
         end
 
         it "validates charge when period is every 2 weeks" do
@@ -939,9 +939,9 @@ RSpec.describe Validations::FinancialValidations do
           record.chcharge = 2001
           financial_validator.validate_care_home_charges(record)
           expect(record.errors["chcharge"])
-            .to include(match I18n.t("validations.financial.carehome.out_of_range", min_chcharge: 20, period: "every 2 weeks", max_chcharge: 2000))
+            .to include("Household rent and other charges must be between £20.00 and £2,000.00 if paying every 2 weeks")
           expect(record.errors["period"])
-            .to include(match I18n.t("validations.financial.carehome.out_of_range", min_chcharge: 20, period: "every 2 weeks", max_chcharge: 2000))
+            .to include("Household rent and other charges must be between £20.00 and £2,000.00 if paying every 2 weeks")
         end
 
         it "validates charge when period is every 4 weeks" do
@@ -949,13 +949,13 @@ RSpec.describe Validations::FinancialValidations do
           record.chcharge = 4001
           financial_validator.validate_care_home_charges(record)
           expect(record.errors["chcharge"])
-            .to include(match I18n.t("validations.financial.carehome.out_of_range", min_chcharge: 40, period: "every 4 weeks", max_chcharge: 4000))
+            .to include("Household rent and other charges must be between £40.00 and £4,000.00 if paying every 4 weeks")
           expect(record.errors["period"])
-            .to include(match I18n.t("validations.financial.carehome.out_of_range", min_chcharge: 40, period: "every 4 weeks", max_chcharge: 4000))
+            .to include("Household rent and other charges must be between £40.00 and £4,000.00 if paying every 4 weeks")
         end
       end
 
-      context "and charges are within the valid limit (£1000 per week)" do
+      context "and charges are within the valid limit (£1,000 per week)" do
         it "does not throw error when period is weekly for 52 weeks" do
           record.period = 1
           record.chcharge = 999
@@ -1007,9 +1007,9 @@ RSpec.describe Validations::FinancialValidations do
           record.chcharge = 9
           financial_validator.validate_care_home_charges(record)
           expect(record.errors["chcharge"])
-            .to include(match I18n.t("validations.financial.carehome.out_of_range", min_chcharge: 10, period: "weekly for 52 weeks", max_chcharge: 1000))
+            .to include("Household rent and other charges must be between £10.00 and £1,000.00 if paying weekly for 52 weeks")
           expect(record.errors["period"])
-            .to include(match I18n.t("validations.financial.carehome.out_of_range", min_chcharge: 10, period: "weekly for 52 weeks", max_chcharge: 1000))
+            .to include("Household rent and other charges must be between £10.00 and £1,000.00 if paying weekly for 52 weeks")
         end
 
         it "validates charge when period is monthly" do
@@ -1017,9 +1017,9 @@ RSpec.describe Validations::FinancialValidations do
           record.chcharge = 42
           financial_validator.validate_care_home_charges(record)
           expect(record.errors["chcharge"])
-            .to include(match I18n.t("validations.financial.carehome.out_of_range", min_chcharge: 43, period: "every calendar month", max_chcharge: 4333))
+            .to include("Household rent and other charges must be between £43.00 and £4,333.00 if paying every calendar month")
           expect(record.errors["period"])
-            .to include(match I18n.t("validations.financial.carehome.out_of_range", min_chcharge: 43, period: "every calendar month", max_chcharge: 4333))
+            .to include("Household rent and other charges must be between £43.00 and £4,333.00 if paying every calendar month")
         end
 
         it "validates charge when period is every 2 weeks" do
@@ -1027,9 +1027,9 @@ RSpec.describe Validations::FinancialValidations do
           record.chcharge = 19
           financial_validator.validate_care_home_charges(record)
           expect(record.errors["chcharge"])
-            .to include(match I18n.t("validations.financial.carehome.out_of_range", min_chcharge: 20, period: "every 2 weeks", max_chcharge: 2000))
+            .to include("Household rent and other charges must be between £20.00 and £2,000.00 if paying every 2 weeks")
           expect(record.errors["period"])
-            .to include(match I18n.t("validations.financial.carehome.out_of_range", min_chcharge: 20, period: "every 2 weeks", max_chcharge: 2000))
+            .to include("Household rent and other charges must be between £20.00 and £2,000.00 if paying every 2 weeks")
         end
 
         it "validates charge when period is every 4 weeks" do
@@ -1037,9 +1037,9 @@ RSpec.describe Validations::FinancialValidations do
           record.chcharge = 39
           financial_validator.validate_care_home_charges(record)
           expect(record.errors["chcharge"])
-            .to include(match I18n.t("validations.financial.carehome.out_of_range", min_chcharge: 40, period: "every 4 weeks", max_chcharge: 4000))
+            .to include("Household rent and other charges must be between £40.00 and £4,000.00 if paying every 4 weeks")
           expect(record.errors["period"])
-            .to include(match I18n.t("validations.financial.carehome.out_of_range", min_chcharge: 40, period: "every 4 weeks", max_chcharge: 4000))
+            .to include("Household rent and other charges must be between £40.00 and £4,000.00 if paying every 4 weeks")
         end
       end
     end
diff --git a/spec/models/validations/sales/sale_information_validations_spec.rb b/spec/models/validations/sales/sale_information_validations_spec.rb
index 3a8944e79..cccb19d40 100644
--- a/spec/models/validations/sales/sale_information_validations_spec.rb
+++ b/spec/models/validations/sales/sale_information_validations_spec.rb
@@ -246,11 +246,13 @@ RSpec.describe Validations::Sales::SaleInformationValidations do
           it "adds an error if mortgage, deposit and grant total does not equal market value" do
             record.grant = 3_000
             sale_information_validator.validate_discounted_ownership_value(record)
-            expect(record.errors[:mortgage]).to include(I18n.t("validations.sale_information.discounted_ownership_value", value_with_discount: "30000.00"))
-            expect(record.errors[:deposit]).to include(I18n.t("validations.sale_information.discounted_ownership_value", value_with_discount: "30000.00"))
-            expect(record.errors[:grant]).to include(I18n.t("validations.sale_information.discounted_ownership_value", value_with_discount: "30000.00"))
-            expect(record.errors[:value]).to include(I18n.t("validations.sale_information.discounted_ownership_value", value_with_discount: "30000.00"))
-            expect(record.errors[:discount]).to include(I18n.t("validations.sale_information.discounted_ownership_value", value_with_discount: "30000.00"))
+            expected_message = ["Mortgage, deposit, and grant total must equal £30,000.00"]
+
+            expect(record.errors[:mortgage]).to eq(expected_message)
+            expect(record.errors[:deposit]).to eq(expected_message)
+            expect(record.errors[:grant]).to eq(expected_message)
+            expect(record.errors[:value]).to eq(expected_message)
+            expect(record.errors[:discount]).to eq(expected_message)
           end
 
           it "does not add an error if mortgage, deposit and grant total equals market value" do
@@ -280,11 +282,14 @@ RSpec.describe Validations::Sales::SaleInformationValidations do
           it "adds an error if mortgage and deposit total does not equal market value - discount" do
             record.discount = 10
             sale_information_validator.validate_discounted_ownership_value(record)
-            expect(record.errors[:mortgage]).to include(I18n.t("validations.sale_information.discounted_ownership_value", value_with_discount: "27000.00"))
-            expect(record.errors[:deposit]).to include(I18n.t("validations.sale_information.discounted_ownership_value", value_with_discount: "27000.00"))
-            expect(record.errors[:grant]).to include(I18n.t("validations.sale_information.discounted_ownership_value", value_with_discount: "27000.00"))
-            expect(record.errors[:value]).to include(I18n.t("validations.sale_information.discounted_ownership_value", value_with_discount: "27000.00"))
-            expect(record.errors[:discount]).to include(I18n.t("validations.sale_information.discounted_ownership_value", value_with_discount: "27000.00"))
+
+            expected_message = ["Mortgage, deposit, and grant total must equal £27,000.00"]
+
+            expect(record.errors[:mortgage]).to eq(expected_message)
+            expect(record.errors[:deposit]).to eq(expected_message)
+            expect(record.errors[:grant]).to eq(expected_message)
+            expect(record.errors[:value]).to eq(expected_message)
+            expect(record.errors[:discount]).to eq(expected_message)
           end
 
           it "does not add an error if mortgage and deposit total equals market value - discount" do
@@ -301,11 +306,14 @@ RSpec.describe Validations::Sales::SaleInformationValidations do
         it "adds an error if mortgage and deposit total does not equal market value" do
           record.deposit = 2_000
           sale_information_validator.validate_discounted_ownership_value(record)
-          expect(record.errors[:mortgage]).to include(I18n.t("validations.sale_information.discounted_ownership_value", value_with_discount: "30000.00"))
-          expect(record.errors[:deposit]).to include(I18n.t("validations.sale_information.discounted_ownership_value", value_with_discount: "30000.00"))
-          expect(record.errors[:grant]).to include(I18n.t("validations.sale_information.discounted_ownership_value", value_with_discount: "30000.00"))
-          expect(record.errors[:value]).to include(I18n.t("validations.sale_information.discounted_ownership_value", value_with_discount: "30000.00"))
-          expect(record.errors[:discount]).to include(I18n.t("validations.sale_information.discounted_ownership_value", value_with_discount: "30000.00"))
+
+          expected_message = ["Mortgage, deposit, and grant total must equal £30,000.00"]
+
+          expect(record.errors[:mortgage]).to eq(expected_message)
+          expect(record.errors[:deposit]).to eq(expected_message)
+          expect(record.errors[:grant]).to eq(expected_message)
+          expect(record.errors[:value]).to eq(expected_message)
+          expect(record.errors[:discount]).to eq(expected_message)
         end
 
         it "does not add an error if mortgage and deposit total equals market value" do
@@ -334,11 +342,14 @@ RSpec.describe Validations::Sales::SaleInformationValidations do
           it "adds an error if mortgage, grant and deposit total does not equal market value - discount" do
             record.mortgage = 10
             sale_information_validator.validate_discounted_ownership_value(record)
-            expect(record.errors[:mortgage]).to include(I18n.t("validations.sale_information.discounted_ownership_value", value_with_discount: "18000.00"))
-            expect(record.errors[:deposit]).to include(I18n.t("validations.sale_information.discounted_ownership_value", value_with_discount: "18000.00"))
-            expect(record.errors[:grant]).to include(I18n.t("validations.sale_information.discounted_ownership_value", value_with_discount: "18000.00"))
-            expect(record.errors[:value]).to include(I18n.t("validations.sale_information.discounted_ownership_value", value_with_discount: "18000.00"))
-            expect(record.errors[:discount]).to include(I18n.t("validations.sale_information.discounted_ownership_value", value_with_discount: "18000.00"))
+
+            expected_message = ["Mortgage, deposit, and grant total must equal £18,000.00"]
+
+            expect(record.errors[:mortgage]).to eq(expected_message)
+            expect(record.errors[:deposit]).to eq(expected_message)
+            expect(record.errors[:grant]).to eq(expected_message)
+            expect(record.errors[:value]).to eq(expected_message)
+            expect(record.errors[:discount]).to eq(expected_message)
           end
 
           it "does not add an error if mortgage, grant and deposit total equals market value - discount" do
@@ -354,11 +365,13 @@ RSpec.describe Validations::Sales::SaleInformationValidations do
 
         it "adds an error if grant and deposit total does not equal market value - discount" do
           sale_information_validator.validate_discounted_ownership_value(record)
-          expect(record.errors[:mortgage]).to include(I18n.t("validations.sale_information.discounted_ownership_value", value_with_discount: "18000.00"))
-          expect(record.errors[:deposit]).to include(I18n.t("validations.sale_information.discounted_ownership_value", value_with_discount: "18000.00"))
-          expect(record.errors[:grant]).to include(I18n.t("validations.sale_information.discounted_ownership_value", value_with_discount: "18000.00"))
-          expect(record.errors[:value]).to include(I18n.t("validations.sale_information.discounted_ownership_value", value_with_discount: "18000.00"))
-          expect(record.errors[:discount]).to include(I18n.t("validations.sale_information.discounted_ownership_value", value_with_discount: "18000.00"))
+
+          expected_message = ["Mortgage, deposit, and grant total must equal £18,000.00"]
+          expect(record.errors[:mortgage]).to eq(expected_message)
+          expect(record.errors[:deposit]).to eq(expected_message)
+          expect(record.errors[:grant]).to eq(expected_message)
+          expect(record.errors[:value]).to eq(expected_message)
+          expect(record.errors[:discount]).to eq(expected_message)
         end
 
         it "does not add an error if mortgage, grant and deposit total equals market value - discount" do
diff --git a/spec/services/imports/lettings_logs_import_service_spec.rb b/spec/services/imports/lettings_logs_import_service_spec.rb
index 50a285ed9..deccde0c0 100644
--- a/spec/services/imports/lettings_logs_import_service_spec.rb
+++ b/spec/services/imports/lettings_logs_import_service_spec.rb
@@ -223,8 +223,8 @@ RSpec.describe Imports::LettingsLogsImportService do
         end
 
         it "intercepts the relevant validation error" do
-          expect(logger).to receive(:warn).with(/Removing earnings with error: Net income cannot be less than £10 per week given the tenant’s working situation/)
-          expect(logger).to receive(:warn).with(/Removing incfreq with error: Net income cannot be less than £10 per week given the tenant’s working situation/)
+          expect(logger).to receive(:warn).with(/Removing earnings with error: Net income cannot be less than £10.00 per week given the tenant’s working situation/)
+          expect(logger).to receive(:warn).with(/Removing incfreq with error: Net income cannot be less than £10.00 per week given the tenant’s working situation/)
           expect { lettings_log_service.send(:create_log, lettings_log_xml) }
             .not_to raise_error
         end
@@ -428,7 +428,7 @@ RSpec.describe Imports::LettingsLogsImportService do
         end
 
         it "intercepts the relevant validation error" do
-          expect(logger).to receive(:warn).with(/Removing ecstat1 with error: Net income cannot be greater than £890 per week given the tenant’s working situation/)
+          expect(logger).to receive(:warn).with(/Removing ecstat1 with error: Net income cannot be greater than £890.00 per week given the tenant’s working situation/)
           expect { lettings_log_service.send(:create_log, lettings_log_xml) }
             .not_to raise_error
         end
@@ -762,7 +762,7 @@ RSpec.describe Imports::LettingsLogsImportService do
         end
 
         it "intercepts the relevant validation error" do
-          expect(logger).to receive(:warn).with(/Removing chcharge with error: Household rent and other charges must be between £10 and £1000 if paying weekly for 52 weeks/)
+          expect(logger).to receive(:warn).with(/Removing chcharge with error: Household rent and other charges must be between £10.00 and £1,000.00 if paying weekly for 52 weeks/)
           expect { lettings_log_service.send(:create_log, lettings_log_xml) }
             .not_to raise_error
         end

From 98060176d21b78b8bf229a48fe797e1a0bc520e7 Mon Sep 17 00:00:00 2001
From: Phil Lee <asmega@users.noreply.github.com>
Date: Tue, 11 Apr 2023 15:17:09 +0100
Subject: [PATCH 5/7] hide blank bulk upload error meta labels (#1526)

---
 .../bulk_upload_error_row_component.html.erb  |  4 +--
 .../bulk_upload_error_row_component.rb        | 24 +++++++++++++++
 .../bulk_upload_error_row_component_spec.rb   | 29 +++++++++++++++++++
 3 files changed, 55 insertions(+), 2 deletions(-)

diff --git a/app/components/bulk_upload_error_row_component.html.erb b/app/components/bulk_upload_error_row_component.html.erb
index 6f8de6919..54a2c6c93 100644
--- a/app/components/bulk_upload_error_row_component.html.erb
+++ b/app/components/bulk_upload_error_row_component.html.erb
@@ -1,9 +1,9 @@
 <div class="x-govuk-summary-card govuk-!-margin-bottom-6">
   <div class="x-govuk-summary-card__header">
     <% if lettings? %>
-      <h3 class="x-govuk-summary-card__title"><strong>Row <%= row %></strong> <span class="govuk-!-margin-left-3">Tenant code: <%= tenant_code %></span> <span class="govuk-!-margin-left-3">Property reference: <%= property_ref %></span></h3>
+      <h3 class="x-govuk-summary-card__title"><strong>Row <%= row %></strong> <%= tenant_code_html %> <%= property_ref_html %></h3>
     <% else %>
-      <h3 class="x-govuk-summary-card__title"><strong>Row <%= row %></strong> <span class="govuk-!-margin-left-3">Purchaser code: <%= purchaser_code %></span></h3>
+      <h3 class="x-govuk-summary-card__title"><strong>Row <%= row %></strong> <%= purchaser_code_html %></h3>
     <% end %>
   </div>
 
diff --git a/app/components/bulk_upload_error_row_component.rb b/app/components/bulk_upload_error_row_component.rb
index 13ac326ef..f4f129cbc 100644
--- a/app/components/bulk_upload_error_row_component.rb
+++ b/app/components/bulk_upload_error_row_component.rb
@@ -15,14 +15,38 @@ class BulkUploadErrorRowComponent < ViewComponent::Base
     bulk_upload_errors.first.tenant_code
   end
 
+  def tenant_code_html
+    return if tenant_code.blank?
+
+    content_tag :span, class: "govuk-!-margin-left-3" do
+      "Tenant code: #{tenant_code}"
+    end
+  end
+
   def purchaser_code
     bulk_upload_errors.first.purchaser_code
   end
 
+  def purchaser_code_html
+    return if purchaser_code.blank?
+
+    content_tag :span, class: "govuk-!-margin-left-3" do
+      "Purchaser code: #{purchaser_code}"
+    end
+  end
+
   def property_ref
     bulk_upload_errors.first.property_ref
   end
 
+  def property_ref_html
+    return if property_ref.blank?
+
+    content_tag :span, class: "govuk-!-margin-left-3" do
+      "Property reference: #{property_ref}"
+    end
+  end
+
   def question_for_field(field)
     bulk_upload.prefix_namespace::RowParser.question_for_field(field.to_sym)
   end
diff --git a/spec/components/bulk_upload_error_row_component_spec.rb b/spec/components/bulk_upload_error_row_component_spec.rb
index 4950817f0..665a630c8 100644
--- a/spec/components/bulk_upload_error_row_component_spec.rb
+++ b/spec/components/bulk_upload_error_row_component_spec.rb
@@ -5,6 +5,7 @@ RSpec.describe BulkUploadErrorRowComponent, type: :component do
     let(:row) { rand(9_999) }
     let(:tenant_code) { SecureRandom.hex(4) }
     let(:property_ref) { SecureRandom.hex(4) }
+    let(:purchaser_code) { nil }
     let(:field) { :field_134 }
     let(:error) { "some error" }
     let(:bulk_upload) { create(:bulk_upload, :lettings) }
@@ -16,6 +17,7 @@ RSpec.describe BulkUploadErrorRowComponent, type: :component do
           row:,
           tenant_code:,
           property_ref:,
+          purchaser_code:,
           field:,
           error:,
         ),
@@ -49,6 +51,33 @@ RSpec.describe BulkUploadErrorRowComponent, type: :component do
       expect(result).to have_content(expected)
     end
 
+    context "when tenant_code not present" do
+      let(:tenant_code) { nil }
+
+      it "does not render tenant code label" do
+        result = render_inline(described_class.new(bulk_upload_errors:))
+        expect(result).not_to have_content("Tenant code")
+      end
+    end
+
+    context "when property_ref not present" do
+      let(:property_ref) { nil }
+
+      it "does not render the property_ref label" do
+        result = render_inline(described_class.new(bulk_upload_errors:))
+        expect(result).not_to have_content("Property reference")
+      end
+    end
+
+    context "when purchaser_code not present" do
+      let(:bulk_upload) { create(:bulk_upload, :sales) }
+
+      it "does not render the purchaser_code label" do
+        result = render_inline(described_class.new(bulk_upload_errors:))
+        expect(result).not_to have_content("Purchaser code")
+      end
+    end
+
     context "when multiple errors for a row" do
       subject(:component) { described_class.new(bulk_upload_errors:) }
 

From 8e8362a74b88bbc030a299afe60ccb1435aeaa60 Mon Sep 17 00:00:00 2001
From: Jack <113976590+bibblobcode@users.noreply.github.com>
Date: Wed, 12 Apr 2023 09:17:32 +0100
Subject: [PATCH 6/7] CLDC-1856 Add housing question flow validation (#1508)

* CLDC-1856 Add housing question flow validation

* Update copy and methods
---
 app/models/validations/household_validations.rb  |  8 ++++++++
 config/locales/en.yml                            |  2 ++
 .../validations/household_validations_spec.rb    | 16 ++++++++++++++++
 3 files changed, 26 insertions(+)

diff --git a/app/models/validations/household_validations.rb b/app/models/validations/household_validations.rb
index 8f28bff74..47d094be8 100644
--- a/app/models/validations/household_validations.rb
+++ b/app/models/validations/household_validations.rb
@@ -121,6 +121,14 @@ module Validations::HouseholdValidations
     end
   end
 
+  def validate_combination_of_housing_needs_responses(record)
+    if record.housingneeds == 1 && record.housingneeds_type == 3 && record.housingneeds_other&.zero?
+      record.errors.add :housingneeds, I18n.t("validations.household.housingneeds.invalid")
+      record.errors.add :housingneeds_type, I18n.t("validations.household.housingneeds.invalid")
+      record.errors.add :housingneeds_other, I18n.t("validations.household.housingneeds.invalid")
+    end
+  end
+
 private
 
   def household_no_illness?(record)
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 68e1d72fc..e223c7fc9 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -389,6 +389,8 @@ en:
       housingneeds_type:
         only_one_option_permitted: "Only one disabled access need: fully wheelchair-accessible housing, wheelchair access to essential rooms or level access housing, can be selected"
       housingneeds:
+        invalid:
+          If somebody in the household has disabled access needs, they must have the access needs listed, or other access needs
         no_disabled_needs_conjunction: "No disabled access needs can’t be selected if you have selected fully wheelchair-accessible housing, wheelchair access to essential rooms, level access housing or other disabled access needs"
         dont_know_disabled_needs_conjunction: "Don’t know disabled access needs can’t be selected if you have selected fully wheelchair-accessible housing, wheelchair access to essential rooms, level access housing or other disabled access needs"
         no_and_dont_know_disabled_needs_conjunction: "No disabled access needs and don’t know disabled access needs cannot be selected together"
diff --git a/spec/models/validations/household_validations_spec.rb b/spec/models/validations/household_validations_spec.rb
index a79db2541..b1c01b7a5 100644
--- a/spec/models/validations/household_validations_spec.rb
+++ b/spec/models/validations/household_validations_spec.rb
@@ -637,4 +637,20 @@ RSpec.describe Validations::HouseholdValidations do
       end
     end
   end
+
+  describe "housing needs validations" do
+    it "is invalid when a combination of housingneeds == 1 (yes) && housingneeds_type == 3 (none of listed) && housingneeds_other == 0 (no)" do
+      record.housingneeds = 1
+      record.housingneeds_type = 3
+      record.housingneeds_other = 0
+
+      household_validator.validate_combination_of_housing_needs_responses(record)
+
+      error_message = ["If somebody in the household has disabled access needs, they must have the access needs listed, or other access needs"]
+
+      expect(record.errors["housingneeds"]).to eq(error_message)
+      expect(record.errors["housingneeds_type"]).to eq(error_message)
+      expect(record.errors["housingneeds_other"]).to eq(error_message)
+    end
+  end
 end

From ca2aa82af6c25961333552d7498ba8bbef7f0a6f Mon Sep 17 00:00:00 2001
From: Jack <113976590+bibblobcode@users.noreply.github.com>
Date: Wed, 12 Apr 2023 12:44:00 +0100
Subject: [PATCH 7/7] Bump nokogiri (#1530)

---
 Gemfile.lock | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Gemfile.lock b/Gemfile.lock
index 9083ba67e..78fe1e157 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -233,11 +233,11 @@ GEM
     net-smtp (0.3.3)
       net-protocol
     nio4r (2.5.8)
-    nokogiri (1.14.2-arm64-darwin)
+    nokogiri (1.14.3-arm64-darwin)
       racc (~> 1.4)
-    nokogiri (1.14.2-x86_64-darwin)
+    nokogiri (1.14.3-x86_64-darwin)
       racc (~> 1.4)
-    nokogiri (1.14.2-x86_64-linux)
+    nokogiri (1.14.3-x86_64-linux)
       racc (~> 1.4)
     notifications-ruby-client (5.4.0)
       jwt (>= 1.5, < 3)