Merge pull request #1019 from sanger/976-dpl-740-modify-the-pacbio-run-to-have-more-than-one-plate-for-multi-plate-support

976 dpl 740 modify the pacbio run to have more than one plate for multi plate support

Author: stevieing

app/csv_generator/pacbio_sample_sheet.rb

@@ -30,7 +30,7 @@ def generate
   private
 
   def wells
-    run.plate.wells
+    run.plates.first.wells
   end
 
   # Returns a list of wells associated with the plate in column order

app/models/pacbio/plate.rb

@@ -6,51 +6,14 @@ class Plate < ApplicationRecord
     include Uuidable
 
     belongs_to :run, foreign_key: :pacbio_run_id,
-                     inverse_of: :plate
+                     inverse_of: :plates
     has_many :wells, class_name: 'Pacbio::Well', foreign_key: :pacbio_plate_id,
-                     inverse_of: :plate, dependent: :destroy
+                     inverse_of: :plate, dependent: :destroy, autosave: true
 
-    accepts_nested_attributes_for :wells
+    accepts_nested_attributes_for :wells, allow_destroy: true
 
-    validates :wells, length: {
-      minimum: 1,
-      message: :plate_min_wells
-    }
-
-    def well_attributes=(well_options)
-      # Delete wells if attributes are not given
-      delete_removed_wells(well_options)
-
-      create_or_update_wells(well_options)
-    end
-
-    def delete_removed_wells(well_options)
-      # Here we need to map the json ids to integers to make sure the exclude comparison uses ints
-      options_ids = well_options.pluck(:id).compact.map(&:to_i)
-
-      # This loop here preloads the wells
-      # This ensures the wells.find(attributes[:id]) block
-      # in well_attributes=
-      # is successful because wells have been preloaded
-      # Otherwise, access wells[i] before calling find
-      wells.each do |well|
-        wells.delete(well) if options_ids.exclude? well.id
-      end
-    end
-
-    def create_or_update_wells(well_options)
-      well_options.map do |well_attributes|
-        # Assuming attributes['pools'] and the given pool id's exists
-        # If not, there is a problem and throw a 5**
-        pools = well_attributes['pools'].map { |pool_id| Pacbio::Pool.find(pool_id) }
-        well_attributes['pools'] = pools
-
-        if well_attributes[:id]
-          wells.find(well_attributes[:id]).assign_attributes(well_attributes)
-        else
-          wells.build(well_attributes)
-        end
-      end
-    end
+    # we maybe still need this in case someone tries to create a
+    # non sequel IIe or Revio run
+    validates :sequencing_kit_box_barcode, :plate_number, presence: true
   end
 end

app/models/pacbio/run.rb

@@ -11,12 +11,10 @@ class Run < ApplicationRecord
     # Sequel II and Sequel I are now deprecated
     enum system_name: { 'Sequel II' => 0, 'Sequel I' => 1, 'Sequel IIe' => 2, 'Revio' => 3 }
 
-    delegate :wells, to: :plate, allow_nil: true
-
     after_create :generate_name
 
-    has_one :plate, foreign_key: :pacbio_run_id,
-                    dependent: :destroy, inverse_of: :run
+    has_many :plates, foreign_key: :pacbio_run_id,
+                      dependent: :destroy, inverse_of: :run, autosave: true
 
     # This association creates the link to the SmrtLinkVersion. Run belongs
     # to a SmrtLinkVersion. We set the default SmrtLinkVersion for the run
@@ -27,29 +25,23 @@ class Run < ApplicationRecord
                inverse_of: :runs,
                default: -> { SmrtLinkVersion.default }
 
-    validates :sequencing_kit_box_barcode,
-              :system_name, presence: true
-
-    # it would be sensible to move this to dependent validation as with wells
-    # and SMRT Link. Something to ponder on ...
-    validates :dna_control_complex_box_barcode, presence: true, unless: lambda {
-                                                                          system_name == 'Revio'
-                                                                        }
+    validates :system_name, presence: true
 
-    # if it is a Revio run we need to check if the wells are in the correct positions
-    validates_with WellPositionValidator, if: lambda {
-                                                system_name == 'Revio'
-                                              }
+    validates_with InstrumentTypeValidator,
+                   instrument_types: Rails.configuration.pacbio_instrument_types,
+                   if: lambda {
+                         system_name.present?
+                       }
 
     validates :name, uniqueness: { case_sensitive: false }
 
     scope :active, -> { where(deactivated_at: nil) }
 
-    accepts_nested_attributes_for :plate
+    accepts_nested_attributes_for :plates, allow_destroy: true
 
     # This will return an empty list
-    # If well data is required via the run, use ?include=plate.wells
-    attr_reader :well_attributes
+    # If plate/well data is required via the run, use ?include=plates.wells
+    attr_reader :plates_attributes
 
     # if comments are nil this blows up so add try.
     def comments
@@ -69,10 +61,9 @@ def instrument_name
       system_name
     end
 
-    def well_attributes=(well_options)
-      self.plate = build_plate(run: self) unless plate
-
-      plate.well_attributes = well_options
+    # This is needed to generate the comments
+    def wells
+      plates.collect(&:wells).flatten
     end
 
     private

app/models/pacbio/well.rb

@@ -11,10 +11,9 @@ class Well < ApplicationRecord
 
     belongs_to :plate, class_name: 'Pacbio::Plate', foreign_key: :pacbio_plate_id,
                        inverse_of: :wells
-
     has_many :well_pools, class_name: 'Pacbio::WellPool', foreign_key: :pacbio_well_id,
-                          dependent: :destroy, inverse_of: :well
-    has_many :pools, class_name: 'Pacbio::Pool', through: :well_pools, autosave: true
+                          dependent: :destroy, inverse_of: :well, autosave: true
+    has_many :pools, class_name: 'Pacbio::Pool', through: :well_pools
 
     has_many :libraries, through: :pools
     has_many :tag_sets, through: :libraries

app/resources/v1/pacbio/run_resource.rb

@@ -6,11 +6,12 @@ module Pacbio
     class RunResource < JSONAPI::Resource
       model_name 'Pacbio::Run'
 
-      attributes :name, :sequencing_kit_box_barcode, :dna_control_complex_box_barcode,
+      attributes :name, :dna_control_complex_box_barcode,
                  :system_name, :created_at, :state, :comments,
-                 :pacbio_smrt_link_version_id, :well_attributes
-      has_one :plate, foreign_key_on: :related, foreign_key: 'pacbio_run_id',
-                      class_name: 'Runs::Plate'
+                 :pacbio_smrt_link_version_id, :plates_attributes
+
+      has_many :plates, foreign_key_on: :related, foreign_key: 'pacbio_run_id',
+                        class_name: 'Runs::Plate'
 
       has_one :smrt_link_version, foreign_key: 'pacbio_smrt_link_version_id'
 
@@ -57,14 +58,25 @@ def self.resource_klass_for(type)
       end
 
       def publish_messages
-        Messages.publish(@model.plate, Pipelines.pacbio.message)
+        Messages.publish(@model.plates.first, Pipelines.pacbio.message)
       end
 
       private
 
-      def well_attributes=(wells_parameters)
-        @model.well_attributes = wells_parameters.map do |well|
-          well.permit(PERMITTED_WELL_PARAMETERS, pools: [])
+      def plates_attributes=(plates_parameters)
+        @model.plates_attributes = plates_parameters.map do |plate|
+          plate.permit(
+            :id,
+            :sequencing_kit_box_barcode,
+            :plate_number,
+            wells_attributes: [
+              # the following is needed to allow the _destroy parameter which
+              # is used to mark wells for destruction
+              :_destroy,
+              PERMITTED_WELL_PARAMETERS,
+              { pool_ids: [] }
+            ]
+          )
         end
       end
     end

app/resources/v1/pacbio/runs/plate_resource.rb

@@ -7,7 +7,7 @@ module Runs
       class PlateResource < JSONAPI::Resource
         model_name 'Pacbio::Plate'
 
-        attributes :pacbio_run_id
+        attributes :pacbio_run_id, :plate_number, :sequencing_kit_box_barcode
 
         has_many :wells, class_name: 'Well'
       end

app/validators/has_filters.rb

@@ -0,0 +1,25 @@
+# frozen_string_literal: true
+
+# Concern for filtering records
+# currently will filter out records which are marked for destruction
+# could be extended to filter out other records
+module HasFilters
+  extend ActiveSupport::Concern
+
+  # @return [Boolean]
+  # records which are marked for destruction should be excluded from the validation
+  def exclude_marked_for_destruction
+    @exclude_marked_for_destruction ||= options[:exclude_marked_for_destruction] || false
+  end
+
+  private
+
+  # @param [ActiveRecord::Base] record
+  # @return [Array]
+  # filter out records which are marked for destruction
+  def filtered(records)
+    return records unless exclude_marked_for_destruction
+
+    records.filter { |record| !record.marked_for_destruction? }
+  end
+end

app/validators/instrument_type_validator.rb

@@ -0,0 +1,104 @@
+# frozen_string_literal: true
+
+# Validator for InstrumentType
+# Validates the InstrumentType and its associated Plates and Wells
+# Validates the presence of required attributes
+# Validates the number of Plates and Wells
+# Validates the positions of Wells
+# Validates the combinations of Wells
+class InstrumentTypeValidator < ActiveModel::Validator
+  attr_reader :instrument_types, :instrument_type
+
+  # @param [Hash] options
+  # @option options [Hash] :instrument_types
+  def initialize(options)
+    super
+    @instrument_types = options[:instrument_types].with_indifferent_access
+  end
+
+  # @param [Run] record
+  # validates the InstrumentType and its associated Plates and Wells
+  def validate(record)
+    self.instrument_type = record
+
+    return if instrument_type.blank?
+
+    # e.g. if record is a Run, then root is :run
+    validate_model(record.model_name.element.to_sym, record, instrument_type['models'])
+
+    bubble_errors(record)
+  end
+
+  private
+
+  # @param [Symbol] root - the root of the model e.g. :run
+  # @param [Model] record - the model to validate
+  # @param [Hash] models - the validations
+  # validates the model and its children
+  def validate_model(root, record, models)
+    model = models[root]
+
+    return if model[:validations].blank?
+
+    # if the model is a has_many relationship
+    if model[:validate_each]
+      run_child_validations(record, model, models)
+    else
+      run_validations(record, model[:validations])
+      # recursively validate the children
+      if model[:children].present?
+        validate_model(model[:children], record.send(model[:children]),
+                       models)
+      end
+    end
+  end
+
+  # @param [Model] record - the model to validate
+  # @param [Hash] validations - the validations
+  # run each validation by calling the corresponding validator
+  def run_validations(record, validations)
+    validations.each do |key, validation|
+      # e.g. my_simple_validator = MySimpleValidator.new(validation[:options])
+      validator = "#{key.camelize}Validator".constantize
+      instance = validator.new(validation[:options])
+      instance.validate(record)
+    end
+  end
+
+  # @param [Model] record - the model to validate
+  # @param [Hash] model - the specific validations for the record
+  # @param [Hash] models - all of the validations
+  # run each validation by calling the corresponding validator
+  def run_child_validations(record, model, models)
+    record.each do |child|
+      run_validations(child, model[:validations])
+      validate_model(model[:children], child, models) if model[:children].present?
+    end
+  end
+
+  # @param [Run] record
+  # adds the errors from the Plates and Wells to the Run
+  # It would be better to use the configuration to make this more flexible
+  # but I need a bit more time to get it right
+  # if we don't do this the run could be marked as valid
+  # because it will not recognise nested errors
+  def bubble_errors(record)
+    record.plates.each do |plate|
+      next if plate.errors.empty?
+
+      plate.errors.each do |error|
+        record.errors.add(:plates,
+                          "plate #{plate.plate_number} #{error.attribute} #{error.message}")
+      end
+    end
+  end
+
+  # @param [Run] record
+  # sets the instrument_type
+  # @return [Hash]
+  def instrument_type=(record)
+    @instrument_type = instrument_types.select do |_key, value|
+      value['name'] == record.system_name
+    end.values.first
+  end
+end