From c270ba3d10ef3244d237d05c161944e068b74c64 Mon Sep 17 00:00:00 2001 From: samyou-softwire Date: Wed, 11 Mar 2026 16:34:49 +0000 Subject: [PATCH] CLDC-4270: Port changes to other years' parsers sales 2025 2026 lettings 2026 --- .../lettings/year2025/csv_parser.rb | 30 ++++++++++++++++-- .../lettings/year2025/row_parser.rb | 17 ++++++++++ .../bulk_upload/sales/year2025/csv_parser.rb | 31 +++++++++++++++++-- .../bulk_upload/sales/year2025/row_parser.rb | 17 ++++++++++ .../bulk_upload/sales/year2026/csv_parser.rb | 31 +++++++++++++++++-- .../bulk_upload/sales/year2026/row_parser.rb | 17 ++++++++++ .../lettings/year2025/csv_parser_spec.rb | 19 ++++++++++++ .../lettings/year2025/row_parser_spec.rb | 16 ++++++++++ .../sales/year2025/csv_parser_spec.rb | 19 ++++++++++++ .../sales/year2025/row_parser_spec.rb | 16 ++++++++++ .../sales/year2026/csv_parser_spec.rb | 19 ++++++++++++ .../sales/year2026/row_parser_spec.rb | 16 ++++++++++ 12 files changed, 242 insertions(+), 6 deletions(-) diff --git a/app/services/bulk_upload/lettings/year2025/csv_parser.rb b/app/services/bulk_upload/lettings/year2025/csv_parser.rb index ec6c33b6d..8fb728ea1 100644 --- a/app/services/bulk_upload/lettings/year2025/csv_parser.rb +++ b/app/services/bulk_upload/lettings/year2025/csv_parser.rb @@ -9,6 +9,8 @@ class BulkUpload::Lettings::Year2025::CsvParser attr_reader :path + ROW_PARSER_CLASS = BulkUpload::Lettings::Year2025::RowParser + def initialize(path:) @path = path end @@ -33,11 +35,35 @@ class BulkUpload::Lettings::Year2025::CsvParser @row_parsers ||= body_rows.map { |row| next if row.empty? + invalid_fields = [] stripped_row = row[col_offset..] - hash = Hash[field_numbers.zip(stripped_row)] + hash_rows = field_numbers + .zip(stripped_row) + .map do |field, value| + # this is needed as a string passed to an int attribute is by default mapped to '0'. + # this is bad as some questions will accept a '0'. so you could enter something invalid and not be told about it + expected_is_integer = ROW_PARSER_CLASS.attribute_types[field].is_a?(ActiveModel::Type::Integer) + # we must be certain that the user entered a string that cannot be coerced to an integer + actual_is_non_empty_string = value.present? && Integer(value, exception: false).nil? && Float(value, exception: false).nil? + field_is_valid = !(expected_is_integer && actual_is_non_empty_string) + + correct_value = field_is_valid ? value : nil + + invalid_fields << field unless field_is_valid + + [field, correct_value] + end + + hash = Hash[hash_rows] + + row_parser = ROW_PARSER_CLASS.new(hash) + + invalid_fields.each do |field| + row_parser.add_invalid_field(field) + end - BulkUpload::Lettings::Year2025::RowParser.new(hash) + row_parser }.compact end diff --git a/app/services/bulk_upload/lettings/year2025/row_parser.rb b/app/services/bulk_upload/lettings/year2025/row_parser.rb index ea52aa9f9..d20a361e9 100644 --- a/app/services/bulk_upload/lettings/year2025/row_parser.rb +++ b/app/services/bulk_upload/lettings/year2025/row_parser.rb @@ -506,6 +506,8 @@ class BulkUpload::Lettings::Year2025::RowParser end end + add_errors_for_invalid_fields + @valid = errors.blank? end @@ -582,6 +584,10 @@ class BulkUpload::Lettings::Year2025::RowParser end end + def add_invalid_field(field) + invalid_fields << field + end + private def normalise_case_insensitive_fields @@ -1020,6 +1026,17 @@ private end end + def invalid_fields + @invalid_fields ||= [] + end + + def add_errors_for_invalid_fields + invalid_fields.each do |field| + errors.delete(field) # take precedence over any other errors as this is a BU format issue + errors.add(field, I18n.t("#{ERROR_BASE_KEY}.invalid_option", question: QUESTIONS[field.to_sym])) + end + end + def field_mapping_for_errors { lettype: [:field_11], diff --git a/app/services/bulk_upload/sales/year2025/csv_parser.rb b/app/services/bulk_upload/sales/year2025/csv_parser.rb index ec052dbfb..f1f22e2b4 100644 --- a/app/services/bulk_upload/sales/year2025/csv_parser.rb +++ b/app/services/bulk_upload/sales/year2025/csv_parser.rb @@ -9,6 +9,8 @@ class BulkUpload::Sales::Year2025::CsvParser attr_reader :path + ROW_PARSER_CLASS = BulkUpload::Sales::Year2025::RowParser + def initialize(path:) @path = path end @@ -33,10 +35,35 @@ class BulkUpload::Sales::Year2025::CsvParser @row_parsers ||= body_rows.map { |row| next if row.empty? + invalid_fields = [] stripped_row = row[col_offset..] - hash = Hash[field_numbers.zip(stripped_row)] - BulkUpload::Sales::Year2025::RowParser.new(hash) + hash_rows = field_numbers + .zip(stripped_row) + .map do |field, value| + # this is needed as a string passed to an int attribute is by default mapped to '0'. + # this is bad as some questions will accept a '0'. so you could enter something invalid and not be told about it + expected_is_integer = ROW_PARSER_CLASS.attribute_types[field].is_a?(ActiveModel::Type::Integer) + # we must be certain that the user entered a string that cannot be coerced to an integer + actual_is_non_empty_string = value.present? && Integer(value, exception: false).nil? && Float(value, exception: false).nil? + field_is_valid = !(expected_is_integer && actual_is_non_empty_string) + + correct_value = field_is_valid ? value : nil + + invalid_fields << field unless field_is_valid + + [field, correct_value] + end + + hash = Hash[hash_rows] + + row_parser = ROW_PARSER_CLASS.new(hash) + + invalid_fields.each do |field| + row_parser.add_invalid_field(field) + end + + row_parser }.compact end diff --git a/app/services/bulk_upload/sales/year2025/row_parser.rb b/app/services/bulk_upload/sales/year2025/row_parser.rb index 19ad2cb1b..b6ffbef2f 100644 --- a/app/services/bulk_upload/sales/year2025/row_parser.rb +++ b/app/services/bulk_upload/sales/year2025/row_parser.rb @@ -503,6 +503,8 @@ class BulkUpload::Sales::Year2025::RowParser end end + add_errors_for_invalid_fields + errors.blank? end @@ -547,6 +549,10 @@ class BulkUpload::Sales::Year2025::RowParser end end + def add_invalid_field(field) + invalid_fields << field + end + private def normalise_case_insensitive_fields @@ -675,6 +681,17 @@ private [9, 14, 27, 29].include?(field_11) end + def invalid_fields + @invalid_fields ||= [] + end + + def add_errors_for_invalid_fields + invalid_fields.each do |field| + errors.delete(field) # take precedence over any other errors as this is a BU format issue + errors.add(field, I18n.t("#{ERROR_BASE_KEY}.invalid_option", question: QUESTIONS[field.to_sym])) + end + end + def field_mapping_for_errors { purchid: %i[field_7], diff --git a/app/services/bulk_upload/sales/year2026/csv_parser.rb b/app/services/bulk_upload/sales/year2026/csv_parser.rb index c403a5aa0..868246eda 100644 --- a/app/services/bulk_upload/sales/year2026/csv_parser.rb +++ b/app/services/bulk_upload/sales/year2026/csv_parser.rb @@ -9,6 +9,8 @@ class BulkUpload::Sales::Year2026::CsvParser attr_reader :path + ROW_PARSER_CLASS = BulkUpload::Sales::Year2026::RowParser + def initialize(path:) @path = path end @@ -34,10 +36,35 @@ class BulkUpload::Sales::Year2026::CsvParser @row_parsers ||= body_rows.map { |row| next if row.empty? + invalid_fields = [] stripped_row = row[col_offset..] - hash = Hash[field_numbers.zip(stripped_row)] - BulkUpload::Sales::Year2026::RowParser.new(hash) + hash_rows = field_numbers + .zip(stripped_row) + .map do |field, value| + # this is needed as a string passed to an int attribute is by default mapped to '0'. + # this is bad as some questions will accept a '0'. so you could enter something invalid and not be told about it + expected_is_integer = ROW_PARSER_CLASS.attribute_types[field].is_a?(ActiveModel::Type::Integer) + # we must be certain that the user entered a string that cannot be coerced to an integer + actual_is_non_empty_string = value.present? && Integer(value, exception: false).nil? && Float(value, exception: false).nil? + field_is_valid = !(expected_is_integer && actual_is_non_empty_string) + + correct_value = field_is_valid ? value : nil + + invalid_fields << field unless field_is_valid + + [field, correct_value] + end + + hash = Hash[hash_rows] + + row_parser = ROW_PARSER_CLASS.new(hash) + + invalid_fields.each do |field| + row_parser.add_invalid_field(field) + end + + row_parser }.compact end diff --git a/app/services/bulk_upload/sales/year2026/row_parser.rb b/app/services/bulk_upload/sales/year2026/row_parser.rb index 8c76234e0..64acf19cd 100644 --- a/app/services/bulk_upload/sales/year2026/row_parser.rb +++ b/app/services/bulk_upload/sales/year2026/row_parser.rb @@ -551,6 +551,8 @@ class BulkUpload::Sales::Year2026::RowParser end end + add_errors_for_invalid_fields + errors.blank? end @@ -595,6 +597,10 @@ class BulkUpload::Sales::Year2026::RowParser end end + def add_invalid_field(field) + invalid_fields << field + end + private def normalise_case_insensitive_fields @@ -727,6 +733,17 @@ private [9, 14, 27, 29].include?(field_11) end + def invalid_fields + @invalid_fields ||= [] + end + + def add_errors_for_invalid_fields + invalid_fields.each do |field| + errors.delete(field) # take precedence over any other errors as this is a BU format issue + errors.add(field, I18n.t("#{ERROR_BASE_KEY}.invalid_option", question: QUESTIONS[field.to_sym])) + end + end + def field_mapping_for_errors { purchid: %i[field_7], diff --git a/spec/services/bulk_upload/lettings/year2025/csv_parser_spec.rb b/spec/services/bulk_upload/lettings/year2025/csv_parser_spec.rb index b53222f7b..098347e62 100644 --- a/spec/services/bulk_upload/lettings/year2025/csv_parser_spec.rb +++ b/spec/services/bulk_upload/lettings/year2025/csv_parser_spec.rb @@ -251,4 +251,23 @@ RSpec.describe BulkUpload::Lettings::Year2025::CsvParser do end end end + + context "when parsing csv with data of the wrong type" do + let(:log_to_csv) { BulkUpload::LettingsLogToCsv.new(log:) } + let(:field_numbers) { log_to_csv.default_2025_field_numbers } + let(:field_values) { log_to_csv.to_2025_row } + + before do + field_46_index = field_numbers.index(46) + field_values[field_46_index] = "GBR" # should be a 3 digit code + + file.write(log_to_csv.custom_field_numbers_row(field_numbers:)) + file.write(log_to_csv.to_custom_csv_row(field_values:)) + file.rewind + end + + it "sets the invalid data to nil" do + expect(service.row_parsers[0].field_46).to be_nil + end + end end diff --git a/spec/services/bulk_upload/lettings/year2025/row_parser_spec.rb b/spec/services/bulk_upload/lettings/year2025/row_parser_spec.rb index 7f4384410..a62685970 100644 --- a/spec/services/bulk_upload/lettings/year2025/row_parser_spec.rb +++ b/spec/services/bulk_upload/lettings/year2025/row_parser_spec.rb @@ -644,6 +644,22 @@ RSpec.describe BulkUpload::Lettings::Year2025::RowParser do expect(parser.errors[:field_116]).to include(match I18n.t("validations.lettings.2025.bulk_upload.invalid_option", question: "")) end end + + describe "invalid fields" do + let(:attributes) { setup_section_params.merge({ field_45: 0 }) } + + context "when a field has been marked as invalid" do + before do + parser.add_invalid_field("field_45") + end + + it "sets a single error on that field" do + parser.valid? + expect(parser.errors[:field_45].size).to eq(1) + expect(parser.errors[:field_45]).to include(I18n.t("validations.lettings.2025.bulk_upload.invalid_option", question: "What is the lead tenant’s nationality?")) + end + end + end end end diff --git a/spec/services/bulk_upload/sales/year2025/csv_parser_spec.rb b/spec/services/bulk_upload/sales/year2025/csv_parser_spec.rb index 6a46c16ad..30993c76c 100644 --- a/spec/services/bulk_upload/sales/year2025/csv_parser_spec.rb +++ b/spec/services/bulk_upload/sales/year2025/csv_parser_spec.rb @@ -188,4 +188,23 @@ RSpec.describe BulkUpload::Sales::Year2025::CsvParser do expect(service.row_parsers[0].field_16).to eql(log.uprn) end end + + context "when parsing csv with data of the wrong type" do + let(:log_to_csv) { BulkUpload::SalesLogToCsv.new(log:) } + let(:field_numbers) { log_to_csv.default_field_numbers_for_year(2025) } + let(:field_values) { log_to_csv.to_2025_row } + + before do + field_32_index = field_numbers.index(32) + field_values[field_32_index] = "abc" # should be an integer + + file.write(log_to_csv.custom_field_numbers_row(field_numbers:)) + file.write(log_to_csv.to_custom_csv_row(field_values:)) + file.rewind + end + + it "sets the invalid data to nil" do + expect(service.row_parsers[0].field_32).to be_nil + end + end end diff --git a/spec/services/bulk_upload/sales/year2025/row_parser_spec.rb b/spec/services/bulk_upload/sales/year2025/row_parser_spec.rb index ebebc38b8..523e55fbd 100644 --- a/spec/services/bulk_upload/sales/year2025/row_parser_spec.rb +++ b/spec/services/bulk_upload/sales/year2025/row_parser_spec.rb @@ -336,6 +336,22 @@ RSpec.describe BulkUpload::Sales::Year2025::RowParser do expect(parser.errors[:field_32]).to include(match I18n.t("validations.sales.2025.bulk_upload.invalid_option", question: "")) end end + + describe "invalid fields" do + let(:attributes) { setup_section_params.merge({ field_31: 0 }) } + + context "when a field has been marked as invalid" do + before do + parser.add_invalid_field("field_31") + end + + it "sets a single error on that field" do + parser.valid? + expect(parser.errors[:field_31].size).to eq(1) + expect(parser.errors[:field_31]).to include(match(I18n.t("validations.sales.2025.bulk_upload.invalid_option", question: "What is buyer 1’s nationality?"))) + end + end + end end end diff --git a/spec/services/bulk_upload/sales/year2026/csv_parser_spec.rb b/spec/services/bulk_upload/sales/year2026/csv_parser_spec.rb index 09e135400..05cfa03e9 100644 --- a/spec/services/bulk_upload/sales/year2026/csv_parser_spec.rb +++ b/spec/services/bulk_upload/sales/year2026/csv_parser_spec.rb @@ -188,4 +188,23 @@ RSpec.describe BulkUpload::Sales::Year2026::CsvParser do expect(service.row_parsers[0].field_16).to eql(log.uprn) end end + + context "when parsing csv with data of the wrong type" do + let(:log_to_csv) { BulkUpload::SalesLogToCsv.new(log:) } + let(:field_numbers) { log_to_csv.default_field_numbers_for_year(2026) } + let(:field_values) { log_to_csv.to_2026_row } + + before do + field_32_index = field_numbers.index(32) + field_values[field_32_index] = "abc" # should be an integer + + file.write(log_to_csv.custom_field_numbers_row(field_numbers:)) + file.write(log_to_csv.to_custom_csv_row(field_values:)) + file.rewind + end + + it "sets the invalid data to nil" do + expect(service.row_parsers[0].field_32).to be_nil + end + end end diff --git a/spec/services/bulk_upload/sales/year2026/row_parser_spec.rb b/spec/services/bulk_upload/sales/year2026/row_parser_spec.rb index 492d1b4b5..7874cd4c5 100644 --- a/spec/services/bulk_upload/sales/year2026/row_parser_spec.rb +++ b/spec/services/bulk_upload/sales/year2026/row_parser_spec.rb @@ -340,6 +340,22 @@ RSpec.describe BulkUpload::Sales::Year2026::RowParser do expect(parser.errors[:field_32]).to include(match I18n.t("validations.sales.2026.bulk_upload.invalid_option", question: "")) end end + + describe "invalid fields" do + let(:attributes) { setup_section_params.merge({ field_31: 0 }) } + + context "when a field has been marked as invalid" do + before do + parser.add_invalid_field("field_31") + end + + it "sets a single error on that field" do + parser.valid? + expect(parser.errors[:field_31].size).to eq(1) + expect(parser.errors[:field_31]).to include(match(I18n.t("validations.sales.2026.bulk_upload.invalid_option", question: "What is buyer 1’s nationality?"))) + end + end + end end end