Bulk upload validation tweaks (#1270)

* remove duplicate bulk validations

- validation was being performed both at CSV level and log level causing
  a duplicate validation to appear

* bulk upload valiation errors now store the message

- previouly this was storing just the error type which we do not have a
  mechanism to pipe these back to user readable error messages

app/services/bulk_upload/lettings/row_parser.rb

@@ -139,13 +139,10 @@ class BulkUpload::Lettings::RowParser
   attribute :field_133, :integer
   attribute :field_134, :integer
 
-  validates :field_1, presence: true, inclusion: { in: (1..12).to_a }
+  validates :field_1, presence: { message: I18n.t("validations.not_answered", question: "letting type") },
+                      inclusion: { in: (1..12).to_a, message: I18n.t("validations.invalid_option", question: "letting type") }
   validates :field_4, presence: { if: proc { [2, 4, 6, 8, 10, 12].include?(field_1) } }
 
-  validates :field_96, presence: true
-  validates :field_97, presence: true
-  validates :field_98, presence: true
-
   def valid?
     errors.clear
 
@@ -176,7 +173,7 @@ private
 
   def validate_data_types
     unless attribute_set["field_1"].value_before_type_cast&.match?(/\A\d+\z/)
-      errors.add(:field_1, :invalid)
+      errors.add(:field_1, I18n.t("validations.invalid_number", question: "letting type"))
     end
   end
 

app/services/bulk_upload/lettings/validator.rb

@@ -162,7 +162,7 @@ class BulkUpload::Lettings::Validator
       row_parser.errors.each do |error|
         bulk_upload.bulk_upload_errors.create!(
           field: error.attribute,
-          error: error.type,
+          error: error.message,
           tenant_code: row_parser.field_7,
           property_ref: row_parser.field_100,
           row:,

config/locales/en.yml

@@ -127,6 +127,7 @@ en:
         already_added: "You have already added this managing agent"
     not_answered: "You must answer %{question}"
     invalid_option: "Enter a valid value for %{question}"
+    invalid_number: "Enter a number for %{question}"
 
     other_field_missing: "If %{main_field_label} is other then %{other_field_label} must be provided"
     other_field_not_required: "%{other_field_label} must not be provided if %{main_field_label} was not other"

spec/services/bulk_upload/lettings/validator_spec.rb

@@ -42,15 +42,15 @@ RSpec.describe BulkUpload::Lettings::Validator do
       it "create validation error with correct values" do
         validator.call
 
-        error = BulkUploadError.first
+        error = BulkUploadError.order(:row, :field).first
 
-        expect(error.field).to eql("field_96")
+        expect(error.field).to eql("field_11")
-        expect(error.error).to eql("blank")
+        expect(error.error).to eql("You must only answer the length of the tenancy if it's fixed-term")
         expect(error.tenant_code).to eql("123")
         expect(error.property_ref).to be_nil
         expect(error.row).to eql("7")
-        expect(error.cell).to eql("CS7")
+        expect(error.cell).to eql("L7")
-        expect(error.col).to eql("CS")
+        expect(error.col).to eql("L")
       end
     end