CLDC-4270: Fix text being coerced to 0 on integer fields (#3230)

* CLDC-4270: Add an error for fields with the wrong type for 2026 lettings

* CLDC-4270: Add tests

* CLDC-4270: Port changes to other years' parsers

sales 2025 2026
lettings 2026

* fixup! CLDC-4270: Add an error for fields with the wrong type for 2026 lettings

* fixup! CLDC-4270: Port changes to other years' parsers

* CLDC-4270: Fix tests post merge

* fixup! CLDC-4270: Add tests

* CLDC-4270: Add ? to boolean method

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,30 @@ 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|
+                      field_is_valid = value_is_valid_for_field?(field, value)
+
+                      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)
 
-      BulkUpload::Lettings::Year2025::RowParser.new(hash)
+      invalid_fields.each do |field|
+        row_parser.add_invalid_field(field)
+      end
+
+      row_parser
     }.compact
   end
 
@@ -110,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

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],

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

@@ -8,6 +8,8 @@ class BulkUpload::Lettings::Year2026::CsvParser
 
   attr_reader :path
 
+  ROW_PARSER_CLASS = BulkUpload::Lettings::Year2026::RowParser
+
   def initialize(path:)
     @path = path
   end
@@ -32,11 +34,30 @@ 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|
+                      field_is_valid = value_is_valid_for_field?(field, value)
+
+                      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)
 
-      BulkUpload::Lettings::Year2026::RowParser.new(hash)
+      invalid_fields.each do |field|
+        row_parser.add_invalid_field(field)
+      end
+
+      row_parser
     }.compact
   end
 
@@ -118,4 +139,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

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],

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,30 @@ 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|
+                      field_is_valid = value_is_valid_for_field?(field, value)
+
+                      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
 
@@ -112,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

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
@@ -503,6 +503,8 @@ class BulkUpload::Sales::Year2025::RowParser
       end
     end
 
+    add_errors_for_invalid_fields
+
     errors.blank?
   end
 
@@ -549,6 +551,10 @@ class BulkUpload::Sales::Year2025::RowParser
     end
   end
 
+  def add_invalid_field(field)
+    invalid_fields << field
+  end
+
 private
 
   def normalise_case_insensitive_fields
@@ -677,6 +683,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],

app/services/bulk_upload/sales/year2026/csv_parser.rb

@@ -8,6 +8,8 @@ class BulkUpload::Sales::Year2026::CsvParser
 
   attr_reader :path
 
+  ROW_PARSER_CLASS = BulkUpload::Sales::Year2026::RowParser
+
   def initialize(path:)
     @path = path
   end
@@ -32,10 +34,30 @@ 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|
+                      field_is_valid = value_is_valid_for_field?(field, value)
+
+                      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
 
@@ -111,6 +133,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

app/services/bulk_upload/sales/year2026/row_parser.rb

@@ -320,7 +320,7 @@ class BulkUpload::Sales::Year2026::RowParser
 
   attribute :field_127, :integer
   attribute :field_128, :decimal
-  attribute :field_129, :integer
+  attribute :field_129, :decimal
   attribute :field_130, :decimal
   attribute :field_131, :integer
   attribute :field_132, :decimal
@@ -552,6 +552,8 @@ class BulkUpload::Sales::Year2026::RowParser
       end
     end
 
+    add_errors_for_invalid_fields
+
     errors.blank?
   end
 
@@ -598,6 +600,10 @@ class BulkUpload::Sales::Year2026::RowParser
     end
   end
 
+  def add_invalid_field(field)
+    invalid_fields << field
+  end
+
 private
 
   def normalise_case_insensitive_fields
@@ -730,6 +736,17 @@ private
     [9, 14, 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],

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

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
 

spec/services/bulk_upload/lettings/year2026/csv_parser_spec.rb

@@ -251,4 +251,23 @@ RSpec.describe BulkUpload::Lettings::Year2026::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_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(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

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

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

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
 

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_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:))
+      file.rewind
+    end
+
+    it "sets the invalid data to nil" do
+      expect(service.row_parsers[0].field_34).to be_nil
+    end
+  end
 end

spec/services/bulk_upload/sales/year2026/row_parser_spec.rb

@@ -342,6 +342,22 @@ RSpec.describe BulkUpload::Sales::Year2026::RowParser do
             expect(parser.errors[:field_35]).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_34: 0 }) }
+
+          context "when a field has been marked as invalid" do
+            before do
+              parser.add_invalid_field("field_34")
+            end
+
+            it "sets a single error on that field" do
+              parser.valid?
+              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
       end
     end