diff --git a/app/mailers/bulk_upload_mailer.rb b/app/mailers/bulk_upload_mailer.rb index 3d5fca769..ebc9df7a6 100644 --- a/app/mailers/bulk_upload_mailer.rb +++ b/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:, }, ) diff --git a/app/models/validations/sales/financial_validations.rb b/app/models/validations/sales/financial_validations.rb index 21f3743ca..255f12bf2 100644 --- a/app/models/validations/sales/financial_validations.rb +++ b/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 diff --git a/app/services/bulk_upload/lettings/validator.rb b/app/services/bulk_upload/lettings/validator.rb index ea5015e56..e60435f75 100644 --- a/app/services/bulk_upload/lettings/validator.rb +++ b/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! diff --git a/app/services/bulk_upload/lettings/year2022/csv_parser.rb b/app/services/bulk_upload/lettings/year2022/csv_parser.rb index 81977c3cb..c0d8d8900 100644 --- a/app/services/bulk_upload/lettings/year2022/csv_parser.rb +++ b/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 diff --git a/app/services/bulk_upload/lettings/year2022/row_parser.rb b/app/services/bulk_upload/lettings/year2022/row_parser.rb index 69b7d9b4e..560ed82b5 100644 --- a/app/services/bulk_upload/lettings/year2022/row_parser.rb +++ b/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 diff --git a/app/services/bulk_upload/lettings/year2023/csv_parser.rb b/app/services/bulk_upload/lettings/year2023/csv_parser.rb index 550ef1825..6f661922c 100644 --- a/app/services/bulk_upload/lettings/year2023/csv_parser.rb +++ b/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 diff --git a/app/services/bulk_upload/lettings/year2023/row_parser.rb b/app/services/bulk_upload/lettings/year2023/row_parser.rb index be20f6515..724b1c2ef 100644 --- a/app/services/bulk_upload/lettings/year2023/row_parser.rb +++ b/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 diff --git a/app/services/bulk_upload/processor.rb b/app/services/bulk_upload/processor.rb index ab4fe50a7..2d27464f0 100644 --- a/app/services/bulk_upload/processor.rb +++ b/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 diff --git a/config/locales/en.yml b/config/locales/en.yml index 4fe8bebc7..50574115e 100644 --- a/config/locales/en.yml +++ b/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: diff --git a/spec/mailers/bulk_upload_mailer_spec.rb b/spec/mailers/bulk_upload_mailer_spec.rb index 0e38617a3..323b49b8f 100644 --- a/spec/mailers/bulk_upload_mailer_spec.rb +++ b/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 diff --git a/spec/models/validations/sales/financial_validations_spec.rb b/spec/models/validations/sales/financial_validations_spec.rb index fae0e4536..d9406aec9 100644 --- a/spec/models/validations/sales/financial_validations_spec.rb +++ b/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) } diff --git a/spec/services/bulk_upload/lettings/validator_spec.rb b/spec/services/bulk_upload/lettings/validator_spec.rb index dda7722c6..f7048fa27 100644 --- a/spec/services/bulk_upload/lettings/validator_spec.rb +++ b/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 } diff --git a/spec/services/bulk_upload/processor_spec.rb b/spec/services/bulk_upload/processor_spec.rb index 8aaef8c36..2254e1b75 100644 --- a/spec/services/bulk_upload/processor_spec.rb +++ b/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