Merge branch 'main' into CLDC-1917-startdate-validation

# Conflicts:
#	config/locales/en.yml

app/mailers/bulk_upload_mailer.rb

@@ -100,7 +100,7 @@ class BulkUploadMailer < NotifyMailer
     )
   end
 
-  def send_bulk_upload_failed_service_error_mail(bulk_upload:)
+  def send_bulk_upload_failed_service_error_mail(bulk_upload:, errors: [])
     bulk_upload_link = if bulk_upload.lettings?
                          start_bulk_upload_lettings_logs_url
                        else
@@ -115,6 +115,7 @@ class BulkUploadMailer < NotifyMailer
         upload_timestamp: bulk_upload.created_at,
         lettings_or_sales: bulk_upload.log_type,
         year_combo: bulk_upload.year_combo,
+        errors: errors.map { |e| "- #{e}" }.join("\n"),
         bulk_upload_link:,
       },
     )

app/models/validations/sales/financial_validations.rb

@@ -52,6 +52,21 @@ module Validations::Sales::FinancialValidations
     end
   end
 
+  def validate_percentage_bought_at_least_threshold(record)
+    return unless record.stairbought && record.type
+
+    threshold = if [2, 16, 18, 24].include? record.type
+                  10
+                else
+                  1
+                end
+
+    if threshold && record.stairbought < threshold
+      record.errors.add :stairbought, I18n.t("validations.financial.staircasing.percentage_bought_must_be_at_least_threshold", threshold:)
+      record.errors.add :type, I18n.t("validations.setup.type.percentage_bought_must_be_at_least_threshold", threshold:)
+    end
+  end
+
   def validate_child_income(record)
     return unless record.income2 && record.ecstat2
 

app/services/bulk_upload/lettings/validator.rb

@@ -9,6 +9,7 @@ class BulkUpload::Lettings::Validator
   attr_reader :bulk_upload, :path
 
   validate :validate_file_not_empty
+  validate :validate_min_columns
   validate :validate_max_columns
 
   def initialize(bulk_upload:, path:)
@@ -23,14 +24,16 @@ class BulkUpload::Lettings::Validator
       row = index + row_offset + 1
 
       row_parser.errors.each do |error|
+        col = csv_parser.column_for_field(error.attribute.to_s)
+
         bulk_upload.bulk_upload_errors.create!(
           field: error.attribute,
           error: error.message,
-          tenant_code: row_parser.field_7,
-          property_ref: row_parser.field_100,
+          tenant_code: row_parser.tenant_code,
+          property_ref: row_parser.property_ref,
           row:,
-          cell: "#{cols[field_number_for_attribute(error.attribute) - col_offset + 1]}#{row}",
-          col: csv_parser.column_for_field(error.attribute.to_s),
+          cell: "#{col}#{row}",
+          col:,
           category: error.options[:category],
         )
       end
@@ -127,12 +130,20 @@ private
     end
   end
 
+  def validate_min_columns
+    return if halt_validations?
+
+    column_count = rows.map(&:size).min
+
+    errors.add(:base, :under_min_column_count) if column_count < csv_parser.class::MIN_COLUMNS
+  end
+
   def validate_max_columns
     return if halt_validations?
 
     column_count = rows.map(&:size).max
 
-    errors.add(:file, :column_count) if column_count > csv_parser.class::MAX_COLUMNS
+    errors.add(:base, :over_max_column_count) if column_count > csv_parser.class::MAX_COLUMNS
   end
 
   def halt_validations!

app/services/bulk_upload/lettings/year2022/csv_parser.rb

@@ -1,6 +1,7 @@
 require "csv"
 
 class BulkUpload::Lettings::Year2022::CsvParser
+  MIN_COLUMNS = 134
   MAX_COLUMNS = 136
 
   attr_reader :path

app/services/bulk_upload/lettings/year2022/row_parser.rb

@@ -372,6 +372,14 @@ class BulkUpload::Lettings::Year2022::RowParser
     block_log_creation
   end
 
+  def tenant_code
+    field_7
+  end
+
+  def property_ref
+    field_100
+  end
+
 private
 
   def validate_location_related

app/services/bulk_upload/lettings/year2023/csv_parser.rb

@@ -1,6 +1,7 @@
 require "csv"
 
 class BulkUpload::Lettings::Year2023::CsvParser
+  MIN_COLUMNS = 141
   MAX_COLUMNS = 143
 
   attr_reader :path

app/services/bulk_upload/lettings/year2023/row_parser.rb

@@ -375,6 +375,14 @@ class BulkUpload::Lettings::Year2023::RowParser
     block_log_creation
   end
 
+  def tenant_code
+    field_13
+  end
+
+  def property_ref
+    field_14
+  end
+
 private
 
   def validate_needs_type_present

app/services/bulk_upload/processor.rb

@@ -8,7 +8,7 @@ class BulkUpload::Processor
   def call
     download
 
-    return send_failure_mail if validator.invalid?
+    return send_failure_mail(errors: validator.errors.full_messages) if validator.invalid?
 
     validator.call
 
@@ -62,9 +62,9 @@ private
     validator.create_logs? && bulk_upload.logs.group(:status).count.keys == %w[completed]
   end
 
-  def send_failure_mail
+  def send_failure_mail(errors: [])
     BulkUploadMailer
-      .send_bulk_upload_failed_service_error_mail(bulk_upload:)
+      .send_bulk_upload_failed_service_error_mail(bulk_upload:, errors:)
       .deliver_later
   end
 

config/locales/en.yml

@@ -41,6 +41,11 @@ en:
   activemodel:
     errors:
       models:
+        bulk_upload/lettings/validator:
+          attributes:
+            base:
+              over_max_column_count: "too many columns, please ensure you have used the correct template"
+              under_min_column_count: "too few columns, please ensure you have used the correct template"
         forms/bulk_upload_lettings/year:
           attributes:
             year:
@@ -154,6 +159,8 @@ en:
           Enter a date within the %{current_start_year_short}/%{current_end_year_short} collection year, which is between %{current_start_year_long} and %{current_end_year_long}
         previous_and_current_financial_year:
           "Enter a date within the %{previous_start_year_short}/%{previous_end_year_short} or %{previous_end_year_short}/%{current_end_year_short} collection years, which is between %{previous_start_year_long} and %{current_end_year_long}"
+      type:
+        percentage_bought_must_be_at_least_threshold: "The minimum increase in equity while staircasing is %{threshold}% for this shared ownership type"
 
       startdate:
         current_financial_year:
@@ -301,6 +308,7 @@ en:
       staircasing:
         percentage_bought_must_be_greater_than_percentage_owned: "Total percentage buyer now owns must be more than percentage bought in this transaction"
         older_person_percentage_owned_maximum_75: "Percentage cannot be above 75% under Older Person's Shared Ownership"
+        percentage_bought_must_be_at_least_threshold: "The minimum increase in equity while staircasing is %{threshold}%"
 
     household:
       reasonpref:

spec/mailers/bulk_upload_mailer_spec.rb

@@ -72,11 +72,12 @@ RSpec.describe BulkUploadMailer do
           upload_timestamp: bulk_upload.created_at,
           lettings_or_sales: bulk_upload.log_type,
           year_combo: bulk_upload.year_combo,
+          errors: "- foo\n- bar",
           bulk_upload_link: start_bulk_upload_lettings_logs_url,
         },
       )
 
-      mailer.send_bulk_upload_failed_service_error_mail(bulk_upload:)
+      mailer.send_bulk_upload_failed_service_error_mail(bulk_upload:, errors: %w[foo bar])
     end
   end
 

spec/models/validations/sales/financial_validations_spec.rb

@@ -160,6 +160,48 @@ RSpec.describe Validations::Sales::FinancialValidations do
     end
   end
 
+  describe "#validate_percentage_bought_at_least_threshold" do
+    let(:record) { FactoryBot.create(:sales_log) }
+
+    it "adds an error to stairbought and type if the percentage bought is less than the threshold (which is 1% by default, but higher for some shared ownership types)" do
+      record.stairbought = 9
+      [2, 16, 18, 24].each do |type|
+        record.type = type
+        financial_validator.validate_percentage_bought_at_least_threshold(record)
+        expect(record.errors["stairbought"]).to eq(["The minimum increase in equity while staircasing is 10%"])
+        expect(record.errors["type"]).to eq(["The minimum increase in equity while staircasing is 10% for this shared ownership type"])
+        record.errors.clear
+      end
+
+      record.stairbought = 0
+      [28, 30, 31, 32].each do |type|
+        record.type = type
+        financial_validator.validate_percentage_bought_at_least_threshold(record)
+        expect(record.errors["stairbought"]).to eq(["The minimum increase in equity while staircasing is 1%"])
+        expect(record.errors["type"]).to eq(["The minimum increase in equity while staircasing is 1% for this shared ownership type"])
+        record.errors.clear
+      end
+    end
+
+    it "doesn't add an error to stairbought and type if the percentage bought is less than the threshold (which is 1% by default, but higher for some shared ownership types)" do
+      record.stairbought = 10
+      [2, 16, 18, 24].each do |type|
+        record.type = type
+        financial_validator.validate_percentage_bought_at_least_threshold(record)
+        expect(record.errors).to be_empty
+        record.errors.clear
+      end
+
+      record.stairbought = 1
+      [28, 30, 31, 32].each do |type|
+        record.type = type
+        financial_validator.validate_percentage_bought_at_least_threshold(record)
+        expect(record.errors).to be_empty
+        record.errors.clear
+      end
+    end
+  end
+
   describe "#validate_percentage_owned_not_too_much_if_older_person" do
     let(:record) { FactoryBot.create(:sales_log) }
 

spec/services/bulk_upload/lettings/validator_spec.rb

@@ -28,6 +28,18 @@ RSpec.describe BulkUpload::Lettings::Validator do
       end
     end
 
+    context "when file has too few columns" do
+      before do
+        file.write("a," * 132)
+        file.write("\n")
+        file.rewind
+      end
+
+      it "is not valid" do
+        expect(validator).not_to be_valid
+      end
+    end
+
     context "when incorrect headers"
   end
 
@@ -59,6 +71,46 @@ RSpec.describe BulkUpload::Lettings::Validator do
       end
     end
 
+    context "with arbitrary ordered 23/24 csv" do
+      let(:bulk_upload) { create(:bulk_upload, user:, year: 2023) }
+      let(:log) { build(:lettings_log, :completed) }
+      let(:file) { Tempfile.new }
+      let(:path) { file.path }
+      let(:seed) { 321 }
+
+      around do |example|
+        FormHandler.instance.use_real_forms!
+
+        example.run
+
+        FormHandler.instance.use_fake_forms!
+      end
+
+      before do
+        file.write(BulkUpload::LogToCsv.new(log:, line_ending: "\r\n").default_2023_field_numbers_row(seed:))
+        file.write(BulkUpload::LogToCsv.new(log:, line_ending: "\r\n").to_2023_csv_row(seed:))
+        file.close
+      end
+
+      it "creates validation errors" do
+        expect { validator.call }.to change(BulkUploadError, :count)
+      end
+
+      it "create validation error with correct values" do
+        validator.call
+
+        error = BulkUploadError.find_by(field: "field_5")
+
+        expect(error.field).to eql("field_5")
+        expect(error.error).to eql("You must answer letting type")
+        expect(error.tenant_code).to eql(log.tenancycode)
+        expect(error.property_ref).to eql(log.propcode)
+        expect(error.row).to eql("2")
+        expect(error.cell).to eql("DD2")
+        expect(error.col).to eql("DD")
+      end
+    end
+
     context "with unix line endings" do
       let(:fixture_path) { file_fixture("2022_23_lettings_bulk_upload.csv") }
       let(:file) { Tempfile.new }

spec/services/bulk_upload/processor_spec.rb

@@ -21,6 +21,7 @@ RSpec.describe BulkUpload::Processor do
           BulkUpload::Lettings::Validator,
           invalid?: true,
           call: nil,
+          errors: [],
         )
       end