bulk upload checks column count of CSV (#1423)

app/mailers/bulk_upload_mailer.rb

@@ -100,7 +100,7 @@ class BulkUploadMailer < NotifyMailer
     )
   end
 
-  def send_bulk_upload_failed_service_error_mail(bulk_upload:)
+  def send_bulk_upload_failed_service_error_mail(bulk_upload:, errors: [])
     bulk_upload_link = if bulk_upload.lettings?
                          start_bulk_upload_lettings_logs_url
                        else
@@ -115,6 +115,7 @@ class BulkUploadMailer < NotifyMailer
         upload_timestamp: bulk_upload.created_at,
         lettings_or_sales: bulk_upload.log_type,
         year_combo: bulk_upload.year_combo,
+        errors: errors.map { |e| "- #{e}" }.join("\n"),
         bulk_upload_link:,
       },
     )

app/services/bulk_upload/lettings/validator.rb

@@ -9,6 +9,7 @@ class BulkUpload::Lettings::Validator
   attr_reader :bulk_upload, :path
 
   validate :validate_file_not_empty
+  validate :validate_min_columns
   validate :validate_max_columns
 
   def initialize(bulk_upload:, path:)
@@ -127,12 +128,20 @@ private
     end
   end
 
+  def validate_min_columns
+    return if halt_validations?
+
+    column_count = rows.map(&:size).min
+
+    errors.add(:base, :under_min_column_count) if column_count < csv_parser.class::MIN_COLUMNS
+  end
+
   def validate_max_columns
     return if halt_validations?
 
     column_count = rows.map(&:size).max
 
-    errors.add(:file, :column_count) if column_count > csv_parser.class::MAX_COLUMNS
+    errors.add(:base, :over_max_column_count) if column_count > csv_parser.class::MAX_COLUMNS
   end
 
   def halt_validations!

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

@@ -1,6 +1,7 @@
 require "csv"
 
 class BulkUpload::Lettings::Year2022::CsvParser
+  MIN_COLUMNS = 134
   MAX_COLUMNS = 136
 
   attr_reader :path

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

@@ -1,6 +1,7 @@
 require "csv"
 
 class BulkUpload::Lettings::Year2023::CsvParser
+  MIN_COLUMNS = 141
   MAX_COLUMNS = 143
 
   attr_reader :path

app/services/bulk_upload/processor.rb

@@ -8,7 +8,7 @@ class BulkUpload::Processor
   def call
     download
 
-    return send_failure_mail if validator.invalid?
+    return send_failure_mail(errors: validator.errors.full_messages) if validator.invalid?
 
     validator.call
 
@@ -62,9 +62,9 @@ private
     validator.create_logs? && bulk_upload.logs.group(:status).count.keys == %w[completed]
   end
 
-  def send_failure_mail
+  def send_failure_mail(errors: [])
     BulkUploadMailer
-      .send_bulk_upload_failed_service_error_mail(bulk_upload:)
+      .send_bulk_upload_failed_service_error_mail(bulk_upload:, errors:)
       .deliver_later
   end
 

config/locales/en.yml

@@ -41,6 +41,11 @@ en:
   activemodel:
     errors:
       models:
+        bulk_upload/lettings/validator:
+          attributes:
+            base:
+              over_max_column_count: "too many columns, please ensure you have used the correct template"
+              under_min_column_count: "too few columns, please ensure you have used the correct template"
         forms/bulk_upload_lettings/year:
           attributes:
             year:

spec/mailers/bulk_upload_mailer_spec.rb

@@ -72,11 +72,12 @@ RSpec.describe BulkUploadMailer do
           upload_timestamp: bulk_upload.created_at,
           lettings_or_sales: bulk_upload.log_type,
           year_combo: bulk_upload.year_combo,
+          errors: "- foo\n- bar",
           bulk_upload_link: start_bulk_upload_lettings_logs_url,
         },
       )
 
-      mailer.send_bulk_upload_failed_service_error_mail(bulk_upload:)
+      mailer.send_bulk_upload_failed_service_error_mail(bulk_upload:, errors: %w[foo bar])
     end
   end
 

spec/services/bulk_upload/lettings/validator_spec.rb

@@ -28,6 +28,18 @@ RSpec.describe BulkUpload::Lettings::Validator do
       end
     end
 
+    context "when file has too few columns" do
+      before do
+        file.write("a," * 132)
+        file.write("\n")
+        file.rewind
+      end
+
+      it "is not valid" do
+        expect(validator).not_to be_valid
+      end
+    end
+
     context "when incorrect headers"
   end
 

spec/services/bulk_upload/processor_spec.rb

@@ -21,6 +21,7 @@ RSpec.describe BulkUpload::Processor do
           BulkUpload::Lettings::Validator,
           invalid?: true,
           call: nil,
+          errors: [],
         )
       end