refactored sufficient tubes validation

app/models/labware_creators/plate_split_to_tube_racks.rb

@@ -61,7 +61,7 @@ class PlateSplitToTubeRacks < Base
     validates_nested :contingency_csv_file, if: :contingency_file
 
     # validate there are sufficient tubes in the racks for the number of parent wells
-    validate :must_have_sufficient_tubes_in_rack, if: :contingency_file
+    validate :must_have_sufficient_tubes_in_rack_files, if: :contingency_file
 
     # validate that the tube barcodes do not already exist in the system
     validate :tube_barcodes_are_unique?
@@ -151,27 +151,40 @@ def num_parent_wells
       @num_parent_wells ||= well_filter.filtered.length
     end
 
-    # Validation method that checks whether there are enough tubes in the rack.
-    #
-    # @return [void]
-    def must_have_sufficient_tubes_in_rack
-      errors.add(:tube_rack_file, 'Must have sufficient tubes in rack') unless sufficient_tubes_in_racks?
-    end
-
-    # Checks if there are sufficient tubes in the child tube racks for all the parent wells.
+    # Validation that checks if there are sufficient tubes in the child tube racks for all the parent wells.
     # This depends on the number of unique samples in the parent plate, and the number of parent wells,
     # as well as whether they are using both sequencing tubes and contingency tubes or just contingency.
     #
-    # @return [Boolean] `true` if there are sufficient tubes, `false` otherwise.
-    def sufficient_tubes_in_racks?
+    # Sets errors if there are insufficient tubes.
+    def must_have_sufficient_tubes_in_rack_files
+      return unless files_correctly_parsed?
+
       if require_contingency_tubes_only?
-        num_contingency_tubes >= num_parent_wells
+        add_error_if_insufficient_tubes(:contingency_csv_file, num_contingency_tubes, num_parent_wells)
       else
-        (num_sequencing_tubes >= num_parent_unique_samples) &&
-          (num_contingency_tubes >= (num_parent_wells - num_parent_unique_samples))
+        add_error_if_insufficient_tubes(
+          :contingency_csv_file,
+          num_contingency_tubes,
+          num_parent_wells - num_parent_unique_samples
+        )
+        add_error_if_insufficient_tubes(:sequencing_csv_file, num_sequencing_tubes, num_parent_unique_samples)
       end
     end
 
+    # Checks the files parsed correctly
+    def files_correctly_parsed?
+      if require_contingency_tubes_only?
+        contingency_csv_file.correctly_parsed?
+      else
+        contingency_csv_file.correctly_parsed? && sequencing_csv_file.correctly_parsed?
+      end
+    end
+
+    # Adds an error when there are insufficient tubes in the given file
+    def add_error_if_insufficient_tubes(file, num_tubes, required_tubes)
+      errors.add(file, 'contains insufficient tubes') unless num_tubes >= required_tubes
+    end
+
     # Validation that the tube barcodes are unique and do not already exist in the system.
     # NB. this checks all the tube barcodes in the uploaded tube rack scan files, not just the
     # ones that will be used.

spec/models/labware_creators/plate_split_to_tube_racks_spec.rb

@@ -186,6 +186,14 @@
     )
   end
 
+  let(:sequencing_file) do
+    fixture_file_upload('spec/fixtures/files/scrna_core_sequencing_tube_rack_scan.csv', 'sequencescape/qc_file')
+  end
+
+  let(:contingency_file) do
+    fixture_file_upload('spec/fixtures/files/scrna_core_contingency_tube_rack_scan.csv', 'sequencescape/qc_file')
+  end
+
   before do
     # need both child tubes to have a purpose config here
     create(
@@ -220,7 +228,7 @@
     end
   end
 
-  context '#sufficient_tubes_in_racks?' do
+  context '#must_have_sufficient_tubes_in_rack_files' do
     let(:num_parent_wells) { 96 }
     let(:num_parent_unique_samples) { 48 }
     let(:num_sequencing_tubes) { 48 }
@@ -242,13 +250,11 @@
 
     context 'when a contingency file is not present' do
       it 'does not call the validation' do
-        expect(subject).not_to receive(:sufficient_tubes_in_racks?)
+        expect(subject).not_to receive(:must_have_sufficient_tubes_in_rack_files)
       end
     end
 
     context 'when require_contingency_tubes_only? is true' do
-      let(:contingency_file) { 'somefile' }
-
       let(:form_attributes) do
         {
           user_uuid: user_uuid,
@@ -258,29 +264,29 @@
         }
       end
 
-      before { allow(subject).to receive(:require_contingency_tubes_only?).and_return(true) }
+      before do
+        allow(subject).to receive(:require_contingency_tubes_only?).and_return(true)
+        subject.must_have_sufficient_tubes_in_rack_files
+      end
 
       context 'when there are enough contingency tubes' do
         let(:num_contingency_tubes) { 96 }
 
-        it 'returns true' do
-          expect(subject.sufficient_tubes_in_racks?).to be true
+        it 'is valid' do
+          expect(subject.errors).to be_empty
         end
       end
 
       context 'when there are not enough contingency tubes' do
         let(:num_contingency_tubes) { 47 }
 
-        it 'returns false' do
-          expect(subject.sufficient_tubes_in_racks?).to be false
+        it 'is not valid' do
+          expect(subject.errors.full_messages).to include('Contingency csv file contains insufficient tubes')
         end
       end
     end
 
     context 'when require_contingency_tubes_only? is false' do
-      let(:sequencing_file) { 'somefile' }
-      let(:contingency_file) { 'somefile' }
-
       let(:form_attributes) do
         {
           user_uuid: user_uuid,
@@ -291,27 +297,30 @@
         }
       end
 
-      before { allow(subject).to receive(:require_contingency_tubes_only?).and_return(false) }
+      before do
+        allow(subject).to receive(:require_contingency_tubes_only?).and_return(false)
+        subject.must_have_sufficient_tubes_in_rack_files
+      end
 
       context 'when there are enough tubes' do
-        it 'returns true' do
-          expect(subject.sufficient_tubes_in_racks?).to be true
+        it 'is valid' do
+          expect(subject.errors.full_messages).to be_empty
         end
       end
 
       context 'when there are not enough sequencing tubes' do
         let(:num_sequencing_tubes) { 47 }
 
-        it 'returns false' do
-          expect(subject.sufficient_tubes_in_racks?).to be false
+        it 'is not valid' do
+          expect(subject.errors.full_messages).to include('Sequencing csv file contains insufficient tubes')
         end
       end
 
       context 'when there are not enough contingency tubes' do
         let(:num_contingency_tubes) { 47 }
 
-        it 'returns false' do
-          expect(subject.sufficient_tubes_in_racks?).to be false
+        it 'is not valid' do
+          expect(subject.errors.full_messages).to include('Contingency csv file contains insufficient tubes')
         end
       end
     end
@@ -444,14 +453,6 @@
     end
 
     context 'with both sequencing and contingency files' do
-      let(:sequencing_file) do
-        fixture_file_upload('spec/fixtures/files/scrna_core_sequencing_tube_rack_scan.csv', 'sequencescape/qc_file')
-      end
-
-      let(:contingency_file) do
-        fixture_file_upload('spec/fixtures/files/scrna_core_contingency_tube_rack_scan.csv', 'sequencescape/qc_file')
-      end
-
       let(:form_attributes) do
         {
           user_uuid: user_uuid,