From 8ca9cbfb499ce455752587019794f3a9ccdc3b5b Mon Sep 17 00:00:00 2001
From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com>
Date: Wed, 25 Jan 2023 16:34:16 +0000
Subject: [PATCH 1/2] Change numeric validation to display prefixes in error
 message (#1227)

* change numeric validation

* Fix test, add delimiter and suffix to the numeric error mesage
---
 .../form/sales/questions/deposit_amount.rb    |  2 +-
 .../sales/sale_information_validations.rb     |  8 ----
 app/models/validations/shared_validations.rb  |  8 +++-
 .../sales/questions/deposit_amount_spec.rb    |  2 +-
 .../sale_information_validations_spec.rb      | 42 -------------------
 .../validations/shared_validations_spec.rb    | 19 +++++++++
 6 files changed, 27 insertions(+), 54 deletions(-)

diff --git a/app/models/form/sales/questions/deposit_amount.rb b/app/models/form/sales/questions/deposit_amount.rb
index 05403dbab..e4fd4ceb2 100644
--- a/app/models/form/sales/questions/deposit_amount.rb
+++ b/app/models/form/sales/questions/deposit_amount.rb
@@ -7,7 +7,7 @@ class Form::Sales::Questions::DepositAmount < ::Form::Question
     @type = "numeric"
     @min = 0
     @width = 5
-    @max = 9_999_999
+    @max = 999_999
     @prefix = "£"
     @hint_text = "Enter the total cash sum paid by the buyer towards the property that was not funded by the mortgage"
     @derived = true
diff --git a/app/models/validations/sales/sale_information_validations.rb b/app/models/validations/sales/sale_information_validations.rb
index 49e39d052..7b0b32206 100644
--- a/app/models/validations/sales/sale_information_validations.rb
+++ b/app/models/validations/sales/sale_information_validations.rb
@@ -1,12 +1,4 @@
 module Validations::Sales::SaleInformationValidations
-  def validate_deposit_range(record)
-    return if record.deposit.blank?
-
-    unless record.deposit >= 0 && record.deposit <= 999_999
-      record.errors.add :deposit, "Cash deposit must be £0 - £999,999"
-    end
-  end
-
   def validate_pratical_completion_date_before_saledate(record)
     return if record.saledate.blank? || record.hodate.blank?
 
diff --git a/app/models/validations/shared_validations.rb b/app/models/validations/shared_validations.rb
index 93a81e938..2d7ac0c9e 100644
--- a/app/models/validations/shared_validations.rb
+++ b/app/models/validations/shared_validations.rb
@@ -1,4 +1,6 @@
 module Validations::SharedValidations
+  include ActionView::Helpers::NumberHelper
+
   def validate_other_field(record, value_other = nil, main_field = nil, other_field = nil, main_label = nil, other_label = nil)
     return unless main_field || other_field
 
@@ -19,17 +21,19 @@ module Validations::SharedValidations
       next unless record[question.id]
 
       field = question.check_answer_label || question.id
+      min = [question.prefix, number_with_delimiter(question.min, delimiter: ","), question.suffix].join("")
+      max = [question.prefix, number_with_delimiter(question.max, delimiter: ","), question.suffix].join("")
 
       begin
         answer = Float(record.public_send("#{question.id}_before_type_cast"))
       rescue ArgumentError
-        record.errors.add question.id.to_sym, I18n.t("validations.numeric.valid", field:, min: question.min, max: question.max)
+        record.errors.add question.id.to_sym, I18n.t("validations.numeric.valid", field:, min:, max:)
       end
 
       next unless answer
 
       if (question.min && question.min > answer) || (question.max && question.max < answer)
-        record.errors.add question.id.to_sym, I18n.t("validations.numeric.valid", field:, min: question.min, max: question.max)
+        record.errors.add question.id.to_sym, I18n.t("validations.numeric.valid", field:, min:, max:)
       end
     end
   end
diff --git a/spec/models/form/sales/questions/deposit_amount_spec.rb b/spec/models/form/sales/questions/deposit_amount_spec.rb
index 961576d62..33a5dcf1d 100644
--- a/spec/models/form/sales/questions/deposit_amount_spec.rb
+++ b/spec/models/form/sales/questions/deposit_amount_spec.rb
@@ -48,6 +48,6 @@ RSpec.describe Form::Sales::Questions::DepositAmount, type: :model do
   end
 
   it "has correct max" do
-    expect(question.max).to eq(9_999_999)
+    expect(question.max).to eq(999_999)
   end
 end
diff --git a/spec/models/validations/sales/sale_information_validations_spec.rb b/spec/models/validations/sales/sale_information_validations_spec.rb
index fafe2786a..052e171be 100644
--- a/spec/models/validations/sales/sale_information_validations_spec.rb
+++ b/spec/models/validations/sales/sale_information_validations_spec.rb
@@ -5,48 +5,6 @@ RSpec.describe Validations::Sales::SaleInformationValidations do
 
   let(:validator_class) { Class.new { include Validations::Sales::SaleInformationValidations } }
 
-  describe "#validate_deposit_range" do
-    context "when within permitted bounds" do
-      let(:record) { build(:sales_log, deposit: 0) }
-
-      it "does not add an error" do
-        sale_information_validator.validate_deposit_range(record)
-
-        expect(record.errors[:deposit]).not_to be_present
-      end
-    end
-
-    context "when blank" do
-      let(:record) { build(:sales_log, deposit: nil) }
-
-      it "does not add an error" do
-        sale_information_validator.validate_deposit_range(record)
-
-        expect(record.errors[:deposit]).not_to be_present
-      end
-    end
-
-    context "when below lower bound" do
-      let(:record) { build(:sales_log, deposit: -1) }
-
-      it "adds an error" do
-        sale_information_validator.validate_deposit_range(record)
-
-        expect(record.errors[:deposit]).to be_present
-      end
-    end
-
-    context "when higher than upper bound" do
-      let(:record) { build(:sales_log, deposit: 1_000_000) }
-
-      it "adds an error" do
-        sale_information_validator.validate_deposit_range(record)
-
-        expect(record.errors[:deposit]).to be_present
-      end
-    end
-  end
-
   describe "#validate_pratical_completion_date_before_saledate" do
     context "when hodate blank" do
       let(:record) { build(:sales_log, hodate: nil) }
diff --git a/spec/models/validations/shared_validations_spec.rb b/spec/models/validations/shared_validations_spec.rb
index a88b5dda2..abdde5f45 100644
--- a/spec/models/validations/shared_validations_spec.rb
+++ b/spec/models/validations/shared_validations_spec.rb
@@ -5,6 +5,7 @@ RSpec.describe Validations::SharedValidations do
 
   let(:validator_class) { Class.new { include Validations::SharedValidations } }
   let(:record) { FactoryBot.create(:lettings_log) }
+  let(:sales_record) { FactoryBot.create(:sales_log) }
   let(:fake_2021_2022_form) { Form.new("spec/fixtures/forms/2021_2022.json") }
 
   describe "numeric min max validations" do
@@ -67,6 +68,24 @@ RSpec.describe Validations::SharedValidations do
         expect(record.errors["age6"]).to be_empty
       end
     end
+
+    context "when validating percent" do
+      it "validates that % suffix is added in the error message" do
+        sales_record.stairbought = "random"
+        shared_validator.validate_numeric_min_max(sales_record)
+        expect(sales_record.errors["stairbought"])
+          .to include(match I18n.t("validations.numeric.valid", field: "Percentage bought in this staircasing transaction", min: "0 percent", max: "100 percent"))
+      end
+    end
+
+    context "when validating price" do
+      it "validates that £ prefix  and , is added in the error message" do
+        sales_record.income1 = "random"
+        shared_validator.validate_numeric_min_max(sales_record)
+        expect(sales_record.errors["income1"])
+          .to include(match I18n.t("validations.numeric.valid", field: "Buyer 1’s gross annual income", min: "£0", max: "£999,999"))
+      end
+    end
   end
 
   describe "radio options validations" do

From 1a5a64e18ede90000ab296123429bf6e0771b644 Mon Sep 17 00:00:00 2001
From: Phil Lee <asmega@users.noreply.github.com>
Date: Wed, 25 Jan 2023 16:44:26 +0000
Subject: [PATCH 2/2] handle bulk upload files with boms (#1233)

---
 app/services/bulk_upload/lettings/csv_parser.rb |  2 +-
 .../bulk_upload/lettings/csv_parser_spec.rb     | 17 +++++++++++++++++
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/app/services/bulk_upload/lettings/csv_parser.rb b/app/services/bulk_upload/lettings/csv_parser.rb
index b8e66678b..ae29f4a6f 100644
--- a/app/services/bulk_upload/lettings/csv_parser.rb
+++ b/app/services/bulk_upload/lettings/csv_parser.rb
@@ -50,7 +50,7 @@ private
   def normalised_string
     return @normalised_string if @normalised_string
 
-    @normalised_string = File.read(path)
+    @normalised_string = File.read(path, encoding: "bom|utf-8")
     @normalised_string.gsub!("\r\n", "\n")
 
     @normalised_string
diff --git a/spec/services/bulk_upload/lettings/csv_parser_spec.rb b/spec/services/bulk_upload/lettings/csv_parser_spec.rb
index 6711d82cd..9d381a5a0 100644
--- a/spec/services/bulk_upload/lettings/csv_parser_spec.rb
+++ b/spec/services/bulk_upload/lettings/csv_parser_spec.rb
@@ -35,4 +35,21 @@ RSpec.describe BulkUpload::Lettings::CsvParser do
       expect(service.row_parsers[0].field_12).to eql(log.age1)
     end
   end
+
+  context "when parsing with BOM aka byte order mark" do
+    let(:file) { Tempfile.new }
+    let(:path) { file.path }
+    let(:log) { build(:lettings_log, :completed) }
+    let(:bom) { "\uFEFF" }
+
+    before do
+      file.write(bom)
+      file.write(BulkUpload::LogToCsv.new(log:, col_offset: 0).to_csv_row)
+      file.close
+    end
+
+    it "parses csv correctly" do
+      expect(service.row_parsers[0].field_12).to eql(log.age1)
+    end
+  end
 end