Merge pull request sanger#587 from sanger/issue583/dpl-028-{bug}-on-t…

…he-pacbio-samples-page-i-need-to-see-the-original-barcode-for-the-sample-source

issue583/dpl 028 {bug} on the pacbio samples page i need to see the original barcode for the sample source

.release-version

@@ -1 +1 @@
-3.6.7
+3.6.8

app/models/concerns/tube_material.rb

@@ -8,10 +8,7 @@ module TubeMaterial
   include Material
 
   included do
-    def tube
-      return nil unless container.is_a?(Tube)
-
-      container
-    end
+    has_one :tube, through: :container_material, source: :container,
+                   source_type: 'Tube', class_name: '::Tube'
   end
 end

app/models/concerns/well_material.rb

@@ -0,0 +1,12 @@
+# frozen_string_literal: true
+
+# WellMaterial -- A material that is contained in a well.
+module WellMaterial
+  extend ActiveSupport::Concern
+  include Material
+
+  included do
+    has_one :well, through: :container_material, source: :container,
+                   source_type: 'Well', class_name: '::Well'
+  end
+end

app/models/pacbio/plate_creator.rb

@@ -196,7 +196,7 @@ class SampleWrapper
       SAMPLE_ATTRIBUTES = %i[external_id name species].freeze
       REQUEST_ATTRIBUTES = %i[
         library_type estimate_of_gb_required number_of_smrt_cells cost_code
-        external_study_id source_barcode
+        external_study_id
       ].freeze
 
       attr_reader :sample, :well, :pacbio_request, :request, :container_material

app/models/tube.rb

@@ -13,6 +13,10 @@ class Tube < ApplicationRecord
           )
         }
 
+  def identifier
+    barcode
+  end
+
   def self.includes_args(except = nil)
     args = []
     unless except == :container_materials

app/models/well.rb

@@ -16,6 +16,10 @@ def column
     position[1..].to_i
   end
 
+  def identifier
+    "#{plate&.barcode}:#{position}"
+  end
+
   def self.includes_args(except = nil)
     args = []
     args << { plate: Plate.includes_args(:wells) } unless except == :plate

app/pacbio/pacbio.rb

@@ -9,7 +9,7 @@ def self.table_name_prefix
   def self.request_attributes
     %i[
       library_type estimate_of_gb_required number_of_smrt_cells cost_code
-      external_study_id source_barcode
+      external_study_id
     ]
   end
 

app/pipelines/pipelines/requestor/controller.rb

@@ -88,12 +88,18 @@ def pipeline_request
       end
 
       # Permitted parameters for create and edit actions
-      # @return [ActionController::Parameters] - whitelisted
+      # @return [Hash] - hash of whitelisted parameters
       def params_names
-        params.require(:data).require(:attributes)[:requests].map do |param|
-          param.permit(*self.class.pipeline_const.request_attributes,
-                       :name, :external_id, :species).to_h
-        end
+        params.require(:data).require(:attributes)
+              .permit(
+                requests: [
+                  {
+                    request: self.class.pipeline_const.request_attributes,
+                    sample: %i[name external_id species],
+                    tube: :barcode
+                  }
+                ]
+              ).to_h[:requests]
       end
     end
   end

app/pipelines/pipelines/requestor/factory.rb

@@ -38,26 +38,20 @@ def initialize(attributes = [])
 
       # @return [Array of ActiveRecord Requests] for the chosen pipeline
       def requests
-        @requests ||= []
-      end
-
-      def container_materials
-        @container_materials ||= []
+        @request_wrappers.collect(&:request)
       end
 
       # @return [Array of ActiveRecord Requestables] for the chosen pipeline
       def requestables
-        requests.collect(&:requestable)
+        @request_wrappers.collect(&:requestable)
       end
 
       # checks if the factory is valid, if so will save all of the requests
       # @return [Boolean] if the requests have been successfully saved or not
       def save
         return false unless valid?
 
-        requests.collect(&:save)
-        container_materials.collect(&:save)
-        true
+        @request_wrappers.all?(&:save)
       end
 
       # Takes each set of request attributes:
@@ -68,26 +62,46 @@ def save
       # @param attributes [Array of ActionController::Parameters] list of request parameters
       # @return [Array of ActiveRecord Requests] for the chosen pipeline
       def build_requests(attributes)
-        attributes.each do |request_attributes|
-          build_request(request_attributes)
+        @request_wrappers = attributes.map do |request_attributes|
+          RequestWrapper.new(request_model: self.class.request_model, **request_attributes)
         end
       end
 
-      # TODO: needs to be refactored.
-      def build_request(request_attributes)
-        sample_attributes = request_attributes.extract!(:name, :external_id, :species)
-        add_request(request_attributes, sample_attributes)
-        add_container_material
-      end
+      # Handles the creation of the request, containers and samples for each request
+      # passed to the factory
+      class RequestWrapper
+        include ActiveModel::Model
 
-      def add_request(request_attributes, sample_attributes)
-        requests << ::Request.new(requestable: self.class.request_model.new(request_attributes),
-                                  sample: Sample.find_or_initialize_by(sample_attributes))
-      end
+        attr_reader :sample, :requestable
+        attr_accessor :request_model
 
-      def add_container_material
-        container_materials << ContainerMaterial.new(container: Tube.new,
-                                                     material: requests.last.requestable)
+        def tube
+          @tube ||= Tube.new
+        end
+
+        def request
+          @request ||= ::Request.new(requestable: requestable, sample: sample)
+        end
+
+        def container_material
+          @container_material ||= ContainerMaterial.new(container: tube, material: requestable)
+        end
+
+        def save
+          request.save && container_material.save
+        end
+
+        def sample=(sample_attributes)
+          @sample = Sample.find_or_initialize_by(sample_attributes)
+        end
+
+        def request=(request_attributes)
+          @requestable = request_model.new(request_attributes)
+        end
+
+        def tube=(tube_attributes)
+          @tube = Tube.new(tube_attributes)
+        end
       end
 
       # Validates the requests:

app/pipelines/pipelines/requestor/model.rb

@@ -13,6 +13,7 @@ module Model
       extend ActiveSupport::Concern
 
       include TubeMaterial
+      include WellMaterial
 
       included do
         has_one :request, class_name: '::Request', as: :requestable, dependent: :nullify
@@ -22,6 +23,17 @@ module Model
         delegate :species, to: :sample, prefix: :sample
 
         validates(*to_s.deconstantize.constantize.required_request_attributes, presence: true)
+
+        # Note, this shouldn't be moved outside the included block, as otherwise the
+        # more generalize delegate method in ::Material will take precedence.
+        # This approach allows us to pre-load tubes and wells, increasing performance
+        def container
+          tube || well
+        end
+      end
+
+      def source_identifier
+        container&.identifier
       end
     end
   end

app/pipelines/pipelines/requestor/resource.rb

@@ -26,21 +26,25 @@ def pipeline_const
         def request_model
           @request_model ||= "#{pipeline}::Request"
         end
+
+        def records_for_populate(*_args)
+          super.preload(:sample, :tube, well: :plate)
+        end
       end
 
       included do
         model_name request_model, add_model_hint: false
 
         attributes(*pipeline_const.request_attributes, :sample_name, :barcode,
-                   :sample_species, :created_at)
+                   :sample_species, :created_at, :source_identifier)
+      end
 
-        def barcode
-          @model&.tube&.barcode
-        end
+      def barcode
+        @model&.tube&.barcode
+      end
 
-        def created_at
-          @model.created_at.to_s(:us)
-        end
+      def created_at
+        @model.created_at.to_s(:us)
       end
     end
   end

app/resources/v1/pacbio/container_material_resource.rb

@@ -15,7 +15,7 @@ class ContainerMaterialResource < JSONAPI::Resource
 
       # Request attributes
       attributes :library_type, :estimate_of_gb_required, :number_of_smrt_cells, :cost_code,
-                 :external_study_id, :source_barcode, :sample_name, :sample_species
+                 :external_study_id, :sample_name, :sample_species
 
       # Shared attributes
       attributes :barcode, :created_at, :material_type
@@ -27,87 +27,43 @@ def fetchable_fields
              created_at deactivated_at sample_names material_type]
         when ::Pacbio::Request
           %i[library_type estimate_of_gb_required number_of_smrt_cells cost_code external_study_id
-             source_barcode sample_name barcode sample_species created_at material_type]
+             sample_name barcode sample_species created_at material_type]
         else
           super
         end
       end
 
+      def self.records_for_populate(*_args)
+        super.preload(:container, material: %i[material_type sample])
+      end
+
       def material_type
         @model.material_type.demodulize.downcase
       end
 
-      # Delegations to Container
+      delegate :container, :material, to: :@model
+
+      # Delegations to container
       def barcode
         @model.container.try(:barcode)
       end
 
-      # Delegations to Material
-      def library_type
-        @model.material.library_type
-      end
-
-      def estimate_of_gb_required
-        @model.material.estimate_of_gb_required
-      end
-
-      def number_of_smrt_cells
-        @model.material.number_of_smrt_cells
-      end
-
-      def cost_code
-        @model.material.cost_code
-      end
-
-      def external_study_id
-        @model.material.external_study_id
-      end
-
-      def source_barcode
-        @model.material.source_barcode
-      end
-
-      def sample_name
-        @model.material.sample_name
-      end
-
-      def sample_species
-        @model.material.sample_species
-      end
+      # Delegations to material
+      delegate :library_type, :estimate_of_gb_required, :number_of_smrt_cells,
+               :cost_code, :external_study_id,
+               :sample_name, :sample_species, :state, :volume,
+               :concentration, :template_prep_kit_box_barcode,
+               :fragment_size, :sample_names, to: :material
 
       def created_at
         @model.material.created_at.strftime('%Y/%m/%d %H:%M')
       end
 
-      def state
-        @model.material.state
-      end
-
-      def volume
-        @model.material.volume
-      end
-
-      def concentration
-        @model.material.concentration
-      end
-
-      def template_prep_kit_box_barcode
-        @model.material.template_prep_kit_box_barcode
-      end
-
-      def fragment_size
-        @model.material.fragment_size
-      end
-
       def deactivated_at
         return nil if @model.material.deactivated_at.nil?
 
         @model.material.deactivated_at.strftime('%Y/%m/%d %H:%M')
       end
-
-      def sample_names
-        @model.material.sample_names
-      end
     end
   end
 end

app/resources/v1/pacbio/plate_resource.rb

@@ -14,7 +14,7 @@ class PlateResource < JSONAPI::Resource
       filter :barcode, apply: ->(records, value, _options) { records.by_barcode(value) }
 
       def self.records(_options = {})
-        ::Plate.by_pipeline(:pacbio)
+        super.by_pipeline(:pacbio)
       end
 
       def created_at

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

@@ -8,6 +8,10 @@ class WellLibraryResource < JSONAPI::Resource
         model_name 'Pacbio::WellLibrary'
 
         attributes :barcode
+
+        def self.records_for_populate(*_args)
+          super.preload(library: :tube)
+        end
       end
     end
   end