From b5407c8f1f3dd797c381020d3b8955497ef726cc Mon Sep 17 00:00:00 2001 From: Phil Lee Date: Mon, 6 Feb 2023 11:13:01 +0000 Subject: [PATCH] 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 | 9 +++------ app/services/bulk_upload/lettings/validator.rb | 2 +- config/locales/en.yml | 1 + spec/services/bulk_upload/lettings/validator_spec.rb | 10 +++++----- 4 files changed, 10 insertions(+), 12 deletions(-) diff --git a/app/services/bulk_upload/lettings/row_parser.rb b/app/services/bulk_upload/lettings/row_parser.rb index d9a4a3e95..1182f4b85 100644 --- a/app/services/bulk_upload/lettings/row_parser.rb +++ b/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 diff --git a/app/services/bulk_upload/lettings/validator.rb b/app/services/bulk_upload/lettings/validator.rb index 90da6efb3..6f37c0f3a 100644 --- a/app/services/bulk_upload/lettings/validator.rb +++ b/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:, diff --git a/config/locales/en.yml b/config/locales/en.yml index 2321446bd..0f3bcc923 100644 --- a/config/locales/en.yml +++ b/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" diff --git a/spec/services/bulk_upload/lettings/validator_spec.rb b/spec/services/bulk_upload/lettings/validator_spec.rb index ec9b3b7f7..263c83163 100644 --- a/spec/services/bulk_upload/lettings/validator_spec.rb +++ b/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.error).to eql("blank") + expect(error.field).to eql("field_11") + 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.col).to eql("CS") + expect(error.cell).to eql("L7") + expect(error.col).to eql("L") end end