CLDC-1889 Bulk upload setup (#1399)

* bulk upload setup errors only for missing data

* bulk upload setup error for scheme

* bulk upload setup error for location

* ensure missing startdate is a setup error

app/services/bulk_upload/lettings/row_parser.rb

@@ -174,15 +174,19 @@ class BulkUpload::Lettings::RowParser
   validate :validate_owning_org_permitted
   validate :validate_owning_org_owns_stock
   validate :validate_owning_org_exists
+  validate :validate_owning_org_data_given
 
   validate :validate_managing_org_related
   validate :validate_managing_org_exists
+  validate :validate_managing_org_data_given
 
   validate :validate_scheme_related
   validate :validate_scheme_exists
+  validate :validate_scheme_data_given
 
   validate :validate_location_related
   validate :validate_location_exists
+  validate :validate_location_data_given
 
   def valid?
     errors.clear
@@ -222,10 +226,6 @@ class BulkUpload::Lettings::RowParser
     block_log_creation
   end
 
-  def setup_section_incomplete?
-    log.form.setup_sections[0].subsections[0].is_incomplete?(log)
-  end
-
 private
 
   def validate_location_related
@@ -245,7 +245,13 @@ private
 
   def validate_location_exists
     if scheme && field_5.present? && location.nil?
-      errors.add(:field_5, "Location could be found with provided scheme code", category: :setup)
+      errors.add(:field_5, "Location could be found with provided scheme code")
+    end
+  end
+
+  def validate_location_data_given
+    if bulk_upload.supported_housing? && field_5.blank?
+      errors.add(:field_5, "The scheme code must be present", category: "setup")
     end
   end
 
@@ -263,7 +269,13 @@ private
 
   def validate_scheme_exists
     if field_4.present? && scheme.nil?
-      errors.add(:field_4, "The management group code is not correct", category: :setup)
+      errors.add(:field_4, "The management group code is not correct")
+    end
+  end
+
+  def validate_scheme_data_given
+    if bulk_upload.supported_housing? && field_4.blank?
+      errors.add(:field_4, "The management group code is not correct", category: "setup")
     end
   end
 
@@ -277,6 +289,12 @@ private
   def validate_managing_org_exists
     if managing_organisation.nil?
       errors.delete(:field_113)
+      errors.add(:field_113, "The managing organisation code is incorrect")
+    end
+  end
+
+  def validate_managing_org_data_given
+    if field_113.blank?
       errors.add(:field_113, "The managing organisation code is incorrect", category: :setup)
     end
   end
@@ -292,6 +310,12 @@ private
   def validate_owning_org_exists
     if owning_organisation.nil?
       errors.delete(:field_111)
+      errors.add(:field_111, "The owning organisation code is incorrect")
+    end
+  end
+
+  def validate_owning_org_data_given
+    if field_111.blank?
       errors.add(:field_111, "The owning organisation code is incorrect", category: :setup)
     end
   end

app/services/bulk_upload/lettings/validator.rb

@@ -175,7 +175,7 @@ class BulkUpload::Lettings::Validator
   end
 
   def create_logs?
-    return false if any_setup_sections_incomplete?
+    return false if any_setup_errors?
     return false if over_column_error_threshold?
     return false if row_parsers.any?(&:block_log_creation?)
 
@@ -186,8 +186,12 @@ class BulkUpload::Lettings::Validator
     QUESTIONS[field]
   end
 
-  def any_setup_sections_incomplete?
+  def any_setup_errors?
-    row_parsers.any?(&:setup_section_incomplete?)
+    bulk_upload
+      .bulk_upload_errors
+      .where(category: "setup")
+      .count
+      .positive?
   end
 
 private

app/services/bulk_upload/processor.rb

@@ -12,7 +12,7 @@ class BulkUpload::Processor
 
     validator.call
 
-    if validator.any_setup_sections_incomplete?
+    if validator.any_setup_errors?
       send_setup_errors_mail
     elsif validator.create_logs?
       create_logs

spec/services/bulk_upload/lettings/row_parser_spec.rb

@@ -298,10 +298,13 @@ RSpec.describe BulkUpload::Lettings::RowParser do
 
     describe "#field_4" do
       context "when nullable not permitted" do
+        let(:bulk_upload) { create(:bulk_upload, :lettings, user:, needstype: 2) }
         let(:attributes) { { bulk_upload:, field_1: "2", field_4: nil } }
 
         it "cannot be nulled" do
-          expect(parser.errors[:field_4]).to be_present
+          setup_errors = parser.errors.select { |e| e.options[:category] == "setup" }
+
+          expect(setup_errors.find { |e| e.attribute == :field_4 }).to be_present
         end
       end
 
@@ -350,6 +353,17 @@ RSpec.describe BulkUpload::Lettings::RowParser do
     end
 
     describe "#field_5" do
+      context "when not nullable" do
+        let(:bulk_upload) { create(:bulk_upload, :lettings, user:, needstype: 2) }
+        let(:attributes) { { bulk_upload:, field_1: "2", field_5: nil } }
+
+        it "cannot be nulled" do
+          setup_errors = parser.errors.select { |e| e.options[:category] == "setup" }
+
+          expect(setup_errors.find { |e| e.attribute == :field_5 }).to be_present
+        end
+      end
+
       context "when location does not exist" do
         let(:scheme) { create(:scheme, :with_old_visible_id, owning_organisation: owning_org) }
         let(:attributes) do
@@ -544,12 +558,12 @@ RSpec.describe BulkUpload::Lettings::RowParser do
       context "when all of these fields are blank" do
         let(:attributes) { { bulk_upload:, field_1: "1", field_96: nil, field_97: nil, field_98: nil } }
 
-        it "returns an error" do
+        it "returns them as setup errors" do
-          parser.valid?
+          setup_errors = parser.errors.select { |e| e.options[:category] == :setup }
 
-          expect(parser.errors[:field_96]).to be_present
+          expect(setup_errors.find { |e| e.attribute == :field_96 }).to be_present
-          expect(parser.errors[:field_97]).to be_present
+          expect(setup_errors.find { |e| e.attribute == :field_97 }).to be_present
-          expect(parser.errors[:field_98]).to be_present
+          expect(setup_errors.find { |e| e.attribute == :field_98 }).to be_present
         end
       end
 
@@ -557,8 +571,6 @@ RSpec.describe BulkUpload::Lettings::RowParser do
         let(:attributes) { { bulk_upload:, field_1: "1", field_96: "1", field_97: "1", field_98: nil } }
 
         it "returns an error only on blank field" do
-          parser.valid?
-
           expect(parser.errors[:field_96]).to be_blank
           expect(parser.errors[:field_97]).to be_blank
           expect(parser.errors[:field_98]).to be_present
@@ -569,8 +581,6 @@ RSpec.describe BulkUpload::Lettings::RowParser do
         let(:attributes) { { bulk_upload:, field_98: "2022" } }
 
         it "returns an error" do
-          parser.valid?
-
           expect(parser.errors[:field_98]).to include("Tenancy start year must be 2 digits")
         end
       end
@@ -589,8 +599,6 @@ RSpec.describe BulkUpload::Lettings::RowParser do
         let(:bulk_upload) { create(:bulk_upload, :lettings, user:, year: 2022) }
 
         it "does not return errors" do
-          parser.valid?
-
           expect(parser.errors[:field_96]).not_to be_present
           expect(parser.errors[:field_97]).not_to be_present
           expect(parser.errors[:field_98]).not_to be_present
@@ -609,8 +617,6 @@ RSpec.describe BulkUpload::Lettings::RowParser do
         let(:bulk_upload) { create(:bulk_upload, :lettings, user:, year: 2022) }
 
         it "returns errors" do
-          parser.valid?
-
           expect(parser.errors[:field_96]).to be_present
           expect(parser.errors[:field_97]).to be_present
           expect(parser.errors[:field_98]).to be_present
@@ -619,6 +625,16 @@ RSpec.describe BulkUpload::Lettings::RowParser do
     end
 
     describe "#field_111" do # owning org
+      context "when no data given" do
+        let(:attributes) { { bulk_upload:, field_1: "1", field_111: "" } }
+
+        it "is not permitted as setup error" do
+          setup_errors = parser.errors.select { |e| e.options[:category] == :setup }
+
+          expect(setup_errors.find { |e| e.attribute == :field_111 }.message).to eql("The owning organisation code is incorrect")
+        end
+      end
+
       context "when cannot find owning org" do
         let(:attributes) { { bulk_upload:, field_111: "donotexist" } }
 

spec/services/bulk_upload/lettings/validator_spec.rb

@@ -53,7 +53,7 @@ RSpec.describe BulkUpload::Lettings::Validator do
         expect(error.col).to eql("L")
         expect(error.category).to be_nil
 
-        error = BulkUploadError.order(:row, :field).find_by(field: "field_111")
+        error = BulkUploadError.find_by(row: "7", category: "setup", field: "field_111")
 
         expect(error.category).to eql("setup")
       end

spec/services/bulk_upload/processor_spec.rb

@@ -106,7 +106,7 @@ RSpec.describe BulkUpload::Processor do
           BulkUpload::Lettings::Validator,
           invalid?: false,
           call: nil,
-          any_setup_sections_incomplete?: true,
+          any_setup_errors?: true,
         )
       end
 
@@ -142,7 +142,7 @@ RSpec.describe BulkUpload::Processor do
           BulkUpload::Lettings::Validator,
           invalid?: false,
           call: nil,
-          any_setup_sections_incomplete?: false,
+          any_setup_errors?: false,
           create_logs?: true,
         )
       end
@@ -193,7 +193,7 @@ RSpec.describe BulkUpload::Processor do
           BulkUpload::Lettings::Validator,
           invalid?: false,
           call: nil,
-          any_setup_sections_incomplete?: false,
+          any_setup_errors?: false,
           create_logs?: false,
         )
       end
@@ -254,7 +254,7 @@ RSpec.describe BulkUpload::Processor do
           BulkUpload::Lettings::Validator,
           call: nil,
           create_logs?: true,
-          any_setup_sections_incomplete?: false,
+          any_setup_errors?: false,
           invalid?: false,
         )
       end