CLDC-2474 Allow numeric fields to accept decimal representations in bulk upload (#1753)

* feat: allow decimals for lettings type when they only have trailing 0s and add tests

* refactor: linting

* refactor: DRY tests

* refactor: linting

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

@@ -773,7 +773,7 @@ private
   end
 
   def validate_data_types
-    unless attribute_set["field_1"].value_before_type_cast&.match?(/\A\d+\z/)
+    unless attribute_set["field_1"].value_before_type_cast&.match?(/^\d+\.?0*$/)
       errors.add(:field_1, I18n.t("validations.invalid_number", question: "letting type"))
     end
   end

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

@@ -702,7 +702,7 @@ private
   end
 
   def validate_data_types
-    unless attribute_set["field_5"].value_before_type_cast&.match?(/\A\d+\z/)
+    unless attribute_set["field_5"].value_before_type_cast&.match?(/^\d+\.?0*$/)
       errors.add(:field_5, I18n.t("validations.invalid_number", question: "letting type"))
     end
   end

spec/services/bulk_upload/lettings/year2022/row_parser_spec.rb

@@ -339,6 +339,30 @@ RSpec.describe BulkUpload::Lettings::Year2022::RowParser do
           end
         end
       end
+
+      context "when valid row with valid decimal (integer) field_1" do
+        before do
+          allow(FeatureToggle).to receive(:bulk_upload_duplicate_log_check_enabled?).and_return(true)
+        end
+
+        let(:attributes) { valid_attributes.merge(field_1: "1.00") }
+
+        it "returns true" do
+          expect(parser).to be_valid
+        end
+      end
+
+      context "when valid row with invalid decimal (non-integer) field_1" do
+        before do
+          allow(FeatureToggle).to receive(:bulk_upload_duplicate_log_check_enabled?).and_return(true)
+        end
+
+        let(:attributes) { valid_attributes.merge(field_1: "1.56") }
+
+        it "returns false" do
+          expect(parser).not_to be_valid
+        end
+      end
     end
 
     context "when setup section not complete" do

spec/services/bulk_upload/lettings/year2023/row_parser_spec.rb

@@ -120,12 +120,8 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do
         end
       end
 
-      context "when valid row" do
+      context "when testing valid/invalid attributes" do
-        before do
+        let(:valid_attributes) do
-          allow(FeatureToggle).to receive(:bulk_upload_duplicate_log_check_enabled?).and_return(true)
-        end
-
-        let(:attributes) do
           {
             bulk_upload:,
             field_5: "1",
@@ -250,128 +246,160 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do
           }
         end
 
-        it "returns true" do
+        context "when valid row" do
-          expect(parser).to be_valid
+          before do
-        end
+            allow(FeatureToggle).to receive(:bulk_upload_duplicate_log_check_enabled?).and_return(true)
-
-        xit "instantiates a log with everything completed", aggregate_failures: true do
-          questions = parser.send(:questions).reject do |q|
-            parser.send(:log).optional_fields.include?(q.id) || q.completed?(parser.send(:log))
           end
 
-          expect(questions.map(&:id).size).to eq(0)
+          let(:attributes) { valid_attributes }
-          expect(questions.map(&:id)).to eql([])
-        end
-
-        context "when a general needs log already exists in the db" do
-          let(:attributes) { { bulk_upload:, field_4: "1" } }
 
-          before do
+          it "returns true" do
-            parser.log.save!
+            expect(parser).to be_valid
-            parser.instance_variable_set(:@valid, nil)
           end
 
-          it "is not a valid row" do
+          xit "instantiates a log with everything completed", aggregate_failures: true do
-            expect(parser).not_to be_valid
+            questions = parser.send(:questions).reject do |q|
+              parser.send(:log).optional_fields.include?(q.id) || q.completed?(parser.send(:log))
+            end
+
+            expect(questions.map(&:id).size).to eq(0)
+            expect(questions.map(&:id)).to eql([])
           end
 
-          it "adds an error to all (and only) the fields used to determine duplicates" do
+          context "when a general needs log already exists in the db" do
-            parser.valid?
+            let(:attributes) { { bulk_upload:, field_4: "1" } }
+
+            before do
+              parser.log.save!
+              parser.instance_variable_set(:@valid, nil)
+            end
 
-            error_message = "This is a duplicate log"
+            it "is not a valid row" do
-
+              expect(parser).not_to be_valid
-            [
-              :field_1, # owning_organisation
-              :field_7, # startdate
-              :field_8, # startdate
-              :field_9, # startdate
-              :field_13, # tenancycode
-              :field_14, # propcode
-              :field_23, # postcode_full
-              :field_24, # postcode_full
-              :field_25, # postcode_full
-              :field_46, # age1
-              :field_47, # sex1
-              :field_50, # ecstat1
-              :field_132, # tcharge
-            ].each do |field|
-              expect(parser.errors[field]).to include(error_message)
             end
 
-            expect(parser.errors[:field_17]).not_to include(error_message)
+            it "adds an error to all (and only) the fields used to determine duplicates" do
+              parser.valid?
+
+              error_message = "This is a duplicate log"
+
+              [
+                :field_1, # owning_organisation
+                :field_7, # startdate
+                :field_8, # startdate
+                :field_9, # startdate
+                :field_13, # tenancycode
+                :field_14, # propcode
+                :field_23, # postcode_full
+                :field_24, # postcode_full
+                :field_25, # postcode_full
+                :field_46, # age1
+                :field_47, # sex1
+                :field_50, # ecstat1
+                :field_132, # tcharge
+              ].each do |field|
+                expect(parser.errors[field]).to include(error_message)
+              end
+
+              expect(parser.errors[:field_17]).not_to include(error_message)
+            end
           end
-        end
 
-        context "when a supported housing log already exists in the db" do
+          context "when a supported housing log already exists in the db" do
-          let(:attributes) { { bulk_upload:, field_4: "2" } }
+            let(:attributes) { { bulk_upload:, field_4: "2" } }
 
-          before do
+            before do
-            parser.log.save!
+              parser.log.save!
-            parser.instance_variable_set(:@valid, nil)
+              parser.instance_variable_set(:@valid, nil)
-          end
+            end
 
-          it "is not a valid row" do
+            it "is not a valid row" do
-            expect(parser).not_to be_valid
+              expect(parser).not_to be_valid
+            end
+
+            it "adds an error to all the fields used to determine duplicates" do
+              parser.valid?
+
+              error_message = "This is a duplicate log"
+
+              [
+                :field_1, # owning_organisation
+                :field_7, # startdate
+                :field_8, # startdate
+                :field_9, # startdate
+                :field_13, # tenancycode
+                :field_14, # propcode
+                :field_17, # location
+                :field_46, # age1
+                :field_47, # sex1
+                :field_50, # ecstat1
+                :field_132, # tcharge
+              ].each do |field|
+                expect(parser.errors[field]).to include(error_message)
+              end
+
+              expect(parser.errors[:field_23]).not_to include(error_message)
+              expect(parser.errors[:field_24]).not_to include(error_message)
+              expect(parser.errors[:field_25]).not_to include(error_message)
+            end
           end
 
-          it "adds an error to all the fields used to determine duplicates" do
+          context "when a hidden log already exists in db" do
-            parser.valid?
+            before do
+              parser.log.status = "pending"
+              parser.log.skip_update_status = true
+              parser.log.save!
+            end
 
-            error_message = "This is a duplicate log"
+            it "is a valid row" do
-
+              expect(parser).to be_valid
-            [
-              :field_1, # owning_organisation
-              :field_7, # startdate
-              :field_8, # startdate
-              :field_9, # startdate
-              :field_13, # tenancycode
-              :field_14, # propcode
-              :field_17, # location
-              :field_46, # age1
-              :field_47, # sex1
-              :field_50, # ecstat1
-              :field_132, # tcharge
-            ].each do |field|
-              expect(parser.errors[field]).to include(error_message)
             end
 
-            expect(parser.errors[:field_23]).not_to include(error_message)
+            it "does not add duplicate errors" do
-            expect(parser.errors[:field_24]).not_to include(error_message)
+              parser.valid?
-            expect(parser.errors[:field_25]).not_to include(error_message)
+
+              [
+                :field_1, # owning_organisation
+                :field_7, # startdate
+                :field_8, # startdate
+                :field_9, # startdate
+                :field_14, # propcode
+                :field_17, # location
+                :field_23, # postcode_full
+                :field_24, # postcode_full
+                :field_25, # postcode_full
+                :field_46, # age1
+                :field_47, # sex1
+                :field_50, # ecstat1
+                :field_132, # tcharge
+              ].each do |field|
+                expect(parser.errors[field]).to be_blank
+              end
+            end
           end
         end
 
-        context "when a hidden log already exists in db" do
+        context "when valid row with valid decimal (integer) field_5" do
           before do
-            parser.log.status = "pending"
+            allow(FeatureToggle).to receive(:bulk_upload_duplicate_log_check_enabled?).and_return(true)
-            parser.log.skip_update_status = true
-            parser.log.save!
           end
 
-          it "is a valid row" do
+          let(:attributes) { valid_attributes.merge(field_5: "1.00") }
+
+          it "returns true" do
             expect(parser).to be_valid
           end
+        end
 
-          it "does not add duplicate errors" do
+        context "when valid row with invalid decimal (non-integer) field_5" do
-            parser.valid?
+          before do
+            allow(FeatureToggle).to receive(:bulk_upload_duplicate_log_check_enabled?).and_return(true)
+          end
 
-            [
+          let(:attributes) { valid_attributes.merge(field_5: "1.56") }
-              :field_1, # owning_organisation
+
-              :field_7, # startdate
+          it "returns false" do
-              :field_8, # startdate
+            expect(parser).not_to be_valid
-              :field_9, # startdate
-              :field_14, # propcode
-              :field_17, # location
-              :field_23, # postcode_full
-              :field_24, # postcode_full
-              :field_25, # postcode_full
-              :field_46, # age1
-              :field_47, # sex1
-              :field_50, # ecstat1
-              :field_132, # tcharge
-            ].each do |field|
-              expect(parser.errors[field]).to be_blank
-            end
           end
         end
       end