From 2186e23f75bef0b66d40593698f24efb57c8f29b Mon Sep 17 00:00:00 2001
From: Phil Lee <asmega@users.noreply.github.com>
Date: Thu, 16 Mar 2023 13:15:10 +0000
Subject: [PATCH] bulk upload errors not referencing correct cells

- this is to handle different years where data is located in different
  columns
---
 .../bulk_upload/lettings/validator.rb         | 10 +++--
 .../lettings/year2022/row_parser.rb           |  8 ++++
 .../lettings/year2023/row_parser.rb           |  8 ++++
 .../bulk_upload/lettings/validator_spec.rb    | 40 +++++++++++++++++++
 4 files changed, 62 insertions(+), 4 deletions(-)

diff --git a/app/services/bulk_upload/lettings/validator.rb b/app/services/bulk_upload/lettings/validator.rb
index ea5015e56..ed03a05e9 100644
--- a/app/services/bulk_upload/lettings/validator.rb
+++ b/app/services/bulk_upload/lettings/validator.rb
@@ -23,14 +23,16 @@ class BulkUpload::Lettings::Validator
       row = index + row_offset + 1
 
       row_parser.errors.each do |error|
+        col = csv_parser.column_for_field(error.attribute.to_s)
+
         bulk_upload.bulk_upload_errors.create!(
           field: error.attribute,
           error: error.message,
-          tenant_code: row_parser.field_7,
-          property_ref: row_parser.field_100,
+          tenant_code: row_parser.tenant_code,
+          property_ref: row_parser.property_ref,
           row:,
-          cell: "#{cols[field_number_for_attribute(error.attribute) - col_offset + 1]}#{row}",
-          col: csv_parser.column_for_field(error.attribute.to_s),
+          cell: "#{col}#{row}",
+          col:,
           category: error.options[:category],
         )
       end
diff --git a/app/services/bulk_upload/lettings/year2022/row_parser.rb b/app/services/bulk_upload/lettings/year2022/row_parser.rb
index 69b7d9b4e..560ed82b5 100644
--- a/app/services/bulk_upload/lettings/year2022/row_parser.rb
+++ b/app/services/bulk_upload/lettings/year2022/row_parser.rb
@@ -372,6 +372,14 @@ class BulkUpload::Lettings::Year2022::RowParser
     block_log_creation
   end
 
+  def tenant_code
+    field_7
+  end
+
+  def property_ref
+    field_100
+  end
+
 private
 
   def validate_location_related
diff --git a/app/services/bulk_upload/lettings/year2023/row_parser.rb b/app/services/bulk_upload/lettings/year2023/row_parser.rb
index be20f6515..724b1c2ef 100644
--- a/app/services/bulk_upload/lettings/year2023/row_parser.rb
+++ b/app/services/bulk_upload/lettings/year2023/row_parser.rb
@@ -375,6 +375,14 @@ class BulkUpload::Lettings::Year2023::RowParser
     block_log_creation
   end
 
+  def tenant_code
+    field_13
+  end
+
+  def property_ref
+    field_14
+  end
+
 private
 
   def validate_needs_type_present
diff --git a/spec/services/bulk_upload/lettings/validator_spec.rb b/spec/services/bulk_upload/lettings/validator_spec.rb
index dda7722c6..1ffcc9691 100644
--- a/spec/services/bulk_upload/lettings/validator_spec.rb
+++ b/spec/services/bulk_upload/lettings/validator_spec.rb
@@ -59,6 +59,46 @@ RSpec.describe BulkUpload::Lettings::Validator do
       end
     end
 
+    context "with arbitrary ordered 23/24 csv" do
+      let(:bulk_upload) { create(:bulk_upload, user:, year: 2023) }
+      let(:log) { build(:lettings_log, :completed) }
+      let(:file) { Tempfile.new }
+      let(:path) { file.path }
+      let(:seed) { 321 }
+
+      around do |example|
+        FormHandler.instance.use_real_forms!
+
+        example.run
+
+        FormHandler.instance.use_fake_forms!
+      end
+
+      before do
+        file.write(BulkUpload::LogToCsv.new(log:, line_ending: "\r\n").default_2023_field_numbers_row(seed:))
+        file.write(BulkUpload::LogToCsv.new(log:, line_ending: "\r\n").to_2023_csv_row(seed:))
+        file.close
+      end
+
+      it "creates validation errors" do
+        expect { validator.call }.to change(BulkUploadError, :count)
+      end
+
+      it "create validation error with correct values" do
+        validator.call
+
+        error = BulkUploadError.find_by(field: "field_5")
+
+        expect(error.field).to eql("field_5")
+        expect(error.error).to eql("You must answer letting type")
+        expect(error.tenant_code).to eql(log.tenancycode)
+        expect(error.property_ref).to eql(log.propcode)
+        expect(error.row).to eql("2")
+        expect(error.cell).to eql("DD2")
+        expect(error.col).to eql("DD")
+      end
+    end
+
     context "with unix line endings" do
       let(:fixture_path) { file_fixture("2022_23_lettings_bulk_upload.csv") }
       let(:file) { Tempfile.new }