From 37faca4a120b23af5a2bc278076bd47e990fe5b7 Mon Sep 17 00:00:00 2001 From: samyou-softwire Date: Tue, 10 Mar 2026 16:49:24 +0000 Subject: [PATCH 1/6] CLDC-4270: Add an error for fields with the wrong type for 2026 lettings --- .../lettings/year2026/csv_parser.rb | 30 +++++++++++++++++-- .../lettings/year2026/row_parser.rb | 17 +++++++++++ 2 files changed, 45 insertions(+), 2 deletions(-) diff --git a/app/services/bulk_upload/lettings/year2026/csv_parser.rb b/app/services/bulk_upload/lettings/year2026/csv_parser.rb index d2f1ab7b1..37182c269 100644 --- a/app/services/bulk_upload/lettings/year2026/csv_parser.rb +++ b/app/services/bulk_upload/lettings/year2026/csv_parser.rb @@ -9,6 +9,8 @@ class BulkUpload::Lettings::Year2026::CsvParser attr_reader :path + ROW_PARSER_CLASS = BulkUpload::Lettings::Year2026::RowParser + def initialize(path:) @path = path end @@ -34,11 +36,35 @@ class BulkUpload::Lettings::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)] + 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::Year2026::RowParser.new(hash) + row_parser }.compact end diff --git a/app/services/bulk_upload/lettings/year2026/row_parser.rb b/app/services/bulk_upload/lettings/year2026/row_parser.rb index 577a48190..c05bfe667 100644 --- a/app/services/bulk_upload/lettings/year2026/row_parser.rb +++ b/app/services/bulk_upload/lettings/year2026/row_parser.rb @@ -541,6 +541,8 @@ class BulkUpload::Lettings::Year2026::RowParser end end + add_errors_for_invalid_fields + @valid = errors.blank? end @@ -620,6 +622,10 @@ class BulkUpload::Lettings::Year2026::RowParser end end + def add_invalid_field(field) + invalid_fields << field + end + private def normalise_case_insensitive_fields @@ -1098,6 +1104,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], From 34cd70f2ea97afeb276334dd610865bf45a56980 Mon Sep 17 00:00:00 2001 From: samyou-softwire Date: Wed, 11 Mar 2026 12:21:31 +0000 Subject: [PATCH 2/6] CLDC-4270: Add tests --- .../lettings/year2026/csv_parser_spec.rb | 20 +++++++++++++++++++ .../lettings/year2026/row_parser_spec.rb | 16 +++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/spec/services/bulk_upload/lettings/year2026/csv_parser_spec.rb b/spec/services/bulk_upload/lettings/year2026/csv_parser_spec.rb index 0452116be..0b74cdde2 100644 --- a/spec/services/bulk_upload/lettings/year2026/csv_parser_spec.rb +++ b/spec/services/bulk_upload/lettings/year2026/csv_parser_spec.rb @@ -251,4 +251,24 @@ RSpec.describe BulkUpload::Lettings::Year2026::CsvParser do end end end + + context "when parsing csv with data of the wrong type" do + let(:seed) { rand } + let(:log_to_csv) { BulkUpload::LettingsLogToCsv.new(log:) } + let(:field_numbers) { log_to_csv.default_2026_field_numbers } + let(:field_values) { log_to_csv.to_2026_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(seed:, field_numbers:)) + file.write(log_to_csv.to_custom_csv_row(seed:, 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/year2026/row_parser_spec.rb b/spec/services/bulk_upload/lettings/year2026/row_parser_spec.rb index 27c8b7672..b312f0b5e 100644 --- a/spec/services/bulk_upload/lettings/year2026/row_parser_spec.rb +++ b/spec/services/bulk_upload/lettings/year2026/row_parser_spec.rb @@ -536,6 +536,22 @@ RSpec.describe BulkUpload::Lettings::Year2026::RowParser do end end end + + describe "invalid fields" do + let(:attributes) { setup_section_params.merge({ field_46: 0 }) } + + context "when a field has been marked as invalid" do + before do + parser.add_invalid_field("field_46") + end + + it "sets a single error on that field" do + parser.valid? + expect(parser.errors[:field_46].size).to eq(1) + expect(parser.errors[:field_46]).to include(match(I18n.t("validations.lettings.2026.bulk_upload.invalid_option", question: "What is the lead tenant’s nationality?"))) + end + end + end end context "when setup section not complete" do From c270ba3d10ef3244d237d05c161944e068b74c64 Mon Sep 17 00:00:00 2001 From: samyou-softwire Date: Wed, 11 Mar 2026 16:34:49 +0000 Subject: [PATCH 3/6] 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 From 35cbe23973decffc02578dae02b2a3e5455d2c00 Mon Sep 17 00:00:00 2001 From: samyou-softwire Date: Mon, 16 Mar 2026 19:18:34 +0000 Subject: [PATCH 4/6] fixup! CLDC-4270: Add an error for fields with the wrong type for 2026 lettings --- .../lettings/year2026/csv_parser.rb | 21 +++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/app/services/bulk_upload/lettings/year2026/csv_parser.rb b/app/services/bulk_upload/lettings/year2026/csv_parser.rb index 37182c269..44d112ff2 100644 --- a/app/services/bulk_upload/lettings/year2026/csv_parser.rb +++ b/app/services/bulk_upload/lettings/year2026/csv_parser.rb @@ -42,12 +42,7 @@ class BulkUpload::Lettings::Year2026::CsvParser 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) + field_is_valid = value_is_valid_for_field(field, value) correct_value = field_is_valid ? value : nil @@ -146,4 +141,18 @@ private Date.new(year, rows.first[8].to_i, rows.first[7].to_i) end end + + # 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 + def value_is_valid_for_field(field, value) + field_type = ROW_PARSER_CLASS.attribute_types[field] + + if field_type.is_a?(ActiveModel::Type::Integer) + value.nil? || Integer(value, exception: false).present? + elsif field_type.is_a?(ActiveModel::Type::Decimal) + value.nil? || Float(value, exception: false).present? + else + true + end + end end From 4fe2e5b4bcff88e513e55cb7b951f661130d0eea Mon Sep 17 00:00:00 2001 From: samyou-softwire Date: Mon, 16 Mar 2026 19:18:51 +0000 Subject: [PATCH 5/6] fixup! CLDC-4270: Port changes to other years' parsers --- .../lettings/year2025/csv_parser.rb | 21 +++++++++++++------ .../bulk_upload/sales/year2025/csv_parser.rb | 21 +++++++++++++------ .../bulk_upload/sales/year2026/csv_parser.rb | 21 +++++++++++++------ 3 files changed, 45 insertions(+), 18 deletions(-) diff --git a/app/services/bulk_upload/lettings/year2025/csv_parser.rb b/app/services/bulk_upload/lettings/year2025/csv_parser.rb index 8fb728ea1..21750b352 100644 --- a/app/services/bulk_upload/lettings/year2025/csv_parser.rb +++ b/app/services/bulk_upload/lettings/year2025/csv_parser.rb @@ -41,12 +41,7 @@ class BulkUpload::Lettings::Year2025::CsvParser 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) + field_is_valid = value_is_valid_for_field(field, value) correct_value = field_is_valid ? value : nil @@ -136,6 +131,20 @@ private @normalised_string end + # 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 + def value_is_valid_for_field(field, value) + field_type = ROW_PARSER_CLASS.attribute_types[field] + + if field_type.is_a?(ActiveModel::Type::Integer) + value.nil? || Integer(value, exception: false).present? + elsif field_type.is_a?(ActiveModel::Type::Decimal) + value.nil? || Float(value, exception: false).present? + else + true + end + end + def first_record_start_date if with_headers? year = row_parsers.first.field_10.to_s.strip.length.between?(1, 2) ? row_parsers.first.field_10.to_i + 2000 : row_parsers.first.field_10.to_i diff --git a/app/services/bulk_upload/sales/year2025/csv_parser.rb b/app/services/bulk_upload/sales/year2025/csv_parser.rb index f1f22e2b4..0bdb756d7 100644 --- a/app/services/bulk_upload/sales/year2025/csv_parser.rb +++ b/app/services/bulk_upload/sales/year2025/csv_parser.rb @@ -41,12 +41,7 @@ class BulkUpload::Sales::Year2025::CsvParser 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) + field_is_valid = value_is_valid_for_field(field, value) correct_value = field_is_valid ? value : nil @@ -139,6 +134,20 @@ private @normalised_string end + # 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 + def value_is_valid_for_field(field, value) + field_type = ROW_PARSER_CLASS.attribute_types[field] + + if field_type.is_a?(ActiveModel::Type::Integer) + value.nil? || Integer(value, exception: false).present? + elsif field_type.is_a?(ActiveModel::Type::Decimal) + value.nil? || Float(value, exception: false).present? + else + true + end + end + def first_record_start_date if with_headers? year = row_parsers.first.field_3.to_s.strip.length.between?(1, 2) ? row_parsers.first.field_3.to_i + 2000 : row_parsers.first.field_3.to_i diff --git a/app/services/bulk_upload/sales/year2026/csv_parser.rb b/app/services/bulk_upload/sales/year2026/csv_parser.rb index 44d067faa..94c3f8223 100644 --- a/app/services/bulk_upload/sales/year2026/csv_parser.rb +++ b/app/services/bulk_upload/sales/year2026/csv_parser.rb @@ -42,12 +42,7 @@ class BulkUpload::Sales::Year2026::CsvParser 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) + field_is_valid = value_is_valid_for_field(field, value) correct_value = field_is_valid ? value : nil @@ -140,6 +135,20 @@ private @normalised_string end + # 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 + def value_is_valid_for_field(field, value) + field_type = ROW_PARSER_CLASS.attribute_types[field] + + if field_type.is_a?(ActiveModel::Type::Integer) + value.nil? || Integer(value, exception: false).present? + elsif field_type.is_a?(ActiveModel::Type::Decimal) + value.nil? || Float(value, exception: false).present? + else + true + end + end + def first_record_start_date if with_headers? year = row_parsers.first.field_3.to_s.strip.length.between?(1, 2) ? row_parsers.first.field_3.to_i + 2000 : row_parsers.first.field_3.to_i From 09af58c78521ebe86a607706017e33ff0a7e1200 Mon Sep 17 00:00:00 2001 From: samyou-softwire Date: Mon, 16 Mar 2026 20:51:13 +0000 Subject: [PATCH 6/6] CLDC-4270: Fix tests post merge --- app/services/bulk_upload/sales/year2025/row_parser.rb | 2 +- .../bulk_upload/sales/year2026/csv_parser_spec.rb | 6 +++--- .../bulk_upload/sales/year2026/row_parser_spec.rb | 8 ++++---- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/app/services/bulk_upload/sales/year2025/row_parser.rb b/app/services/bulk_upload/sales/year2025/row_parser.rb index 83266b388..bcb00ffd9 100644 --- a/app/services/bulk_upload/sales/year2025/row_parser.rb +++ b/app/services/bulk_upload/sales/year2025/row_parser.rb @@ -288,7 +288,7 @@ class BulkUpload::Sales::Year2025::RowParser attribute :field_112, :integer attribute :field_113, :decimal - attribute :field_114, :integer + attribute :field_114, :decimal attribute :field_115, :decimal attribute :field_116, :integer attribute :field_117, :decimal 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 065e20224..f347f6500 100644 --- a/spec/services/bulk_upload/sales/year2026/csv_parser_spec.rb +++ b/spec/services/bulk_upload/sales/year2026/csv_parser_spec.rb @@ -195,8 +195,8 @@ RSpec.describe BulkUpload::Sales::Year2026::CsvParser do 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 + field_34_index = field_numbers.index(34) + field_values[field_34_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:)) @@ -204,7 +204,7 @@ RSpec.describe BulkUpload::Sales::Year2026::CsvParser do end it "sets the invalid data to nil" do - expect(service.row_parsers[0].field_32).to be_nil + expect(service.row_parsers[0].field_34).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 19ad8b095..605674914 100644 --- a/spec/services/bulk_upload/sales/year2026/row_parser_spec.rb +++ b/spec/services/bulk_upload/sales/year2026/row_parser_spec.rb @@ -344,17 +344,17 @@ RSpec.describe BulkUpload::Sales::Year2026::RowParser do end describe "invalid fields" do - let(:attributes) { setup_section_params.merge({ field_31: 0 }) } + let(:attributes) { setup_section_params.merge({ field_34: 0 }) } context "when a field has been marked as invalid" do before do - parser.add_invalid_field("field_31") + parser.add_invalid_field("field_34") 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?"))) + expect(parser.errors[:field_34].size).to eq(1) + expect(parser.errors[:field_34]).to include(match(I18n.t("validations.sales.2026.bulk_upload.invalid_option", question: "What is buyer 1's nationality?"))) end end end