Merge pull request #4017 from sanger/DPL-823-Import-PBMC-pool-plates-…

…into-Sequencescape

DPL-823 [Part 1 - download] Import PBMC pool plates into Sequencescape

app/controllers/sdb/sample_manifests_controller.rb

@@ -37,6 +37,7 @@ def show
     @study_id = @sample_manifest.study_id
     @samples = @sample_manifest.samples.paginate(page: params[:page])
   end
+
   def new # rubocop:todo Metrics/AbcSize
     params[:only_first_label] ||= false
     @sample_manifest = SampleManifest.new(new_manifest_params)

app/jobs/sample_manifest/generate_wells_job.rb

@@ -1,25 +1,31 @@
 # frozen_string_literal: true
-# Generates wells for plate sample manifests
+# Generates wells and sample manifest assets, for a plate sample manifest.
 SampleManifest::GenerateWellsJob =
-  Struct.new(:sample_manifest_id, :map_ids_to_sample_ids, :plate_id) do
+  Struct.new(:sample_manifest_id, :map_ids_to_sanger_sample_ids, :plate_id) do
     def perform
       ActiveRecord::Base.transaction do
-        # Ensure the order of the wells are maintained
-        maps = Map.find(map_ids).index_by(&:id)
-        well_data = map_ids_to_sample_ids.map { |map_id, sample_id| [maps[map_id], sample_id] }
+        map_ids_to_sanger_sample_ids.each { |map_id, sanger_sample_id| create_well(map_id, sanger_sample_id) }
 
-        sample_manifest.core_behaviour.generate_wells_job(well_data, plate)
+        RequestFactory.create_assets_requests(plate.wells, sample_manifest.study)
+
+        plate.events.created_using_sample_manifest!(sample_manifest.user)
       end
     end
 
-    def map_ids
-      map_ids_to_sample_ids.map(&:first)
+    def create_well(map_id, sanger_sample_ids)
+      plate.wells.create!(map: Map.find(map_id)) { |well| create_sample_manifest_assets(well, sanger_sample_ids) }
     end
 
     def plate
       Plate.find(plate_id)
     end
 
+    def create_sample_manifest_assets(well, sanger_sample_ids)
+      sanger_sample_ids.each do |sanger_sample_id|
+        SampleManifestAsset.create(sanger_sample_id: sanger_sample_id, asset: well, sample_manifest: sample_manifest)
+      end
+    end
+
     def sample_manifest
       SampleManifest.find(sample_manifest_id)
     end

app/models/sample_manifest.rb

@@ -6,7 +6,7 @@
 # for the potential samples. It also generates a {SampleManifestExcel}
 # spreadsheet which gets sent to the customer.
 #
-# The labware that gets generate is determined by the {#asset_type} which
+# The labware that gets generated is determined by the {#asset_type} which
 # switches out the {#core_behaviour} module {SampleManifest::CoreBehaviour}.
 # This is concerned with generating {Labware} and {Receptacle receptacles},
 # generating any event specific to the asset type, and setting manifest specific
@@ -49,6 +49,7 @@ def self.included(base)
   has_uploaded_document :generated, differentiator: 'generated'
 
   attr_accessor :override, :only_first_label
+  attr_writer :rows_per_well
 
   class_attribute :spreadsheet_offset
   class_attribute :spreadsheet_header_row
@@ -128,6 +129,13 @@ def default_filename
     "#{study_id}stdy_manifest_#{id}_#{created_at.to_formatted_s(:dmy)}"
   end
 
+  # Use a default value of 1 for rows_per_well if not set
+  def rows_per_well
+    1
+    # TODO: replace above line with below line to turn the rows_per_well feature on, when DPL-823 is complete
+    # @rows_per_well || 1
+  end
+
   scope :pending_manifests,
         -> {
           order(id: :desc).includes(:uploaded_document).references(:uploaded_document).where(documents: { id: nil })

app/models/sample_manifest/generator.rb

@@ -96,13 +96,21 @@ def execute_print_job
   end
 
   def attributes
-    params.except(:template, :barcode_printer, :only_first_label).merge(user: user, asset_type: asset_type)
+    params
+      .except(:template, :barcode_printer, :only_first_label)
+      .merge(user: user, asset_type: asset_type, rows_per_well: rows_per_well)
   end
 
   def asset_type
     configuration.manifest_types.find_by(params[:template]).asset_type
   end
 
+  # Retrieves the value of the rows_per_well attribute from the manifest_types.yml config.
+  # If the attribute is not set, it returns nil.
+  def rows_per_well
+    configuration.manifest_types.find_by(params[:template]).rows_per_well
+  end
+
   def only_first_label
     ActiveRecord::Type::Boolean.new.cast(params[:only_first_label])
   end

app/models/sample_manifest/plate_behaviour.rb

@@ -13,6 +13,15 @@ def initialize(manifest)
 
     def generate
       @plates = generate_plates(purpose)
+
+      sanger_sample_ids = insert_sanger_sample_ids
+      well_data = build_well_data(sanger_sample_ids)
+
+      build_wells_async(well_data)
+
+      @details_array = build_details_array(well_data)
+
+      @manifest.update!(barcodes: @plates.map(&:human_barcode))
     end
 
     def acceptable_purposes
@@ -27,24 +36,6 @@ def included_resources
       [{ sample: :sample_metadata, asset: { plate: :barcodes } }]
     end
 
-    def generate_wells(well_data, plates)
-      # Generate the wells, samples & requests asynchronously.
-      generate_wells_for_plates(well_data, plates) do |this_plates_well_data, plate|
-        generate_wells_asynchronously(this_plates_well_data.map { |map, sample_id| [map.id, sample_id] }, plate.id)
-      end
-
-      # Ensure we maintain the information we need for printing labels and generating
-      # the CSV file
-      @plates = plates.sort_by(&:human_barcode)
-
-      @details_array =
-        plates.flat_map do |plate|
-          well_data
-            .slice!(0, plate.size)
-            .map { |map, sample_id| { barcode: plate.human_barcode, position: map.description, sample_id: sample_id } }
-        end
-    end
-
     def io_samples
       samples.map do |sample|
         container = sample.primary_receptacle
@@ -83,65 +74,60 @@ def labware=(labware)
 
     # We use the barcodes here as we may need to reference the plates before the delayed job has passed
     def labware
-      plates | labware_from_barcodes
+      plates | Labware.with_barcode(barcodes)
     end
     alias printables labware
 
-    # Called by {SampleManifest::GenerateWellsJob} and builds the wells
-    def generate_wells_job(wells_for_plate, plate)
-      wells_for_plate.map do |map, sanger_sample_id|
-        plate
-          .wells
-          .create!(map: map) do |well|
-            SampleManifestAsset.create(sanger_sample_id: sanger_sample_id, asset: well, sample_manifest: @manifest)
-          end
-      end
-      RequestFactory.create_assets_requests(plate.wells, study)
-      plate.events.created_using_sample_manifest!(@manifest.user)
-    end
-
     private
 
-    # This method ensures that each of the plates is handled by an individual job.  If it doesn't do this we run
-    # the risk that the 'handler' column in the database for the delayed job will not be large enough and will
-    # truncate the data.
-    def generate_wells_for_plates(well_data, plates)
-      cloned_well_data = well_data.dup
-      plates.each { |plate| yield(cloned_well_data.slice!(0, plate.size), plate) }
-    end
-
-    def labware_from_barcodes
-      Labware.with_barcode(barcodes)
+    def generate_plates(purpose)
+      Array.new(count) { purpose.create!(:without_wells) }.sort_by(&:human_barcode)
     end
 
-    def generate_wells_asynchronously(map_ids_to_sample_ids, plate_id)
-      Delayed::Job.enqueue SampleManifest::GenerateWellsJob.new(@manifest.id, map_ids_to_sample_ids, plate_id)
+    def insert_sanger_sample_ids
+      sanger_sample_ids = generate_sanger_ids(@plates.sum(&:size) * @manifest.rows_per_well)
+      sanger_sample_ids.map do |sanger_sample_id|
+        SangerSampleId.generate_sanger_sample_id!(study.abbreviation, sanger_sample_id)
+      end
     end
 
-    # rubocop:todo Metrics/MethodLength
-    def generate_plates(purpose) # rubocop:todo Metrics/AbcSize
-      study_abbreviation = study.abbreviation
-
-      well_data = []
-      plates = Array.new(count) { purpose.create!(:without_wells) }.sort_by(&:human_barcode)
-
-      plates.each do |plate|
-        sanger_sample_ids = generate_sanger_ids(plate.size)
+    # output:
+    # plate_id => { map_id => [sanger_sample_id, sanger_sample_id, ...] }
+    def build_well_data(sanger_sample_ids)
+      @plates.each_with_object({}) do |plate, well_data|
+        well_data[plate.id] = {}
 
         plate.maps.in_column_major_order.each do |well_map|
-          sanger_sample_id = sanger_sample_ids.shift
-          generated_sanger_sample_id = SangerSampleId.generate_sanger_sample_id!(study_abbreviation, sanger_sample_id)
-
-          well_data << [well_map, generated_sanger_sample_id]
+          well_data[plate.id][well_map.id] = sanger_sample_ids.shift(@manifest.rows_per_well)
         end
       end
+    end
 
-      generate_wells(well_data, plates)
-      @manifest.update!(barcodes: plates.map(&:human_barcode))
+    # Each of the plates is handled by an individual job.
+    # If it doesn't do this we run the risk that the 'handler' column in the database
+    # for the delayed job will not be large enough and will truncate the data.
+    def build_wells_async(well_data)
+      @plates.each do |plate|
+        Delayed::Job.enqueue SampleManifest::GenerateWellsJob.new(@manifest.id, well_data[plate.id], plate.id)
+      end
+    end
 
-      plates
+    # output:
+    # [{barcode, position, sanger_sample_id}, {barcode, position, sanger_sample_id}, ...]
+    def build_details_array(well_data)
+      @details_array =
+        @plates.flat_map do |plate|
+          well_data[plate.id].flat_map do |map_id, sanger_sample_ids|
+            sanger_sample_ids.map do |sanger_sample_id|
+              {
+                barcode: plate.human_barcode,
+                position: plate.maps.find(map_id).description,
+                sample_id: sanger_sample_id
+              }
+            end
+          end
+        end
     end
-    # rubocop:enable Metrics/MethodLength
   end
 
   class Core < Base

app/sample_manifest_excel/sample_manifest_excel/manifest_type_list.rb

@@ -16,6 +16,7 @@ def each(&block)
       manifest_types.each(&block)
     end
 
+    # Hash version of the manifest_types.yml config file
     def manifest_types
       @manifest_types ||= {}
     end
@@ -49,7 +50,7 @@ def <=>(other)
     class ManifestType
       include SequencescapeExcel::Helpers::Attributes
 
-      setup_attributes :name, :columns, :heading, :asset_type
+      setup_attributes :name, :columns, :heading, :asset_type, :rows_per_well
 
       def initialize(attributes = {})
         super
@@ -62,7 +63,8 @@ def to_a
       def ==(other)
         return false unless other.is_a?(self.class)
 
-        name == other.name && columns == other.columns && heading == other.heading && asset_type == other.asset_type
+        name == other.name && columns == other.columns && heading == other.heading && asset_type == other.asset_type &&
+          rows_per_well == other.rows_per_well
       end
     end
 

config/default_records/plate_purposes/011_scrna_core_cdna_prep_plate_purposes.wip.yml

@@ -1,7 +1,15 @@
+# scRNA Core pipeline purposes
+# Most are defined in the Limber config, but the below also need to be defined in Sequencescape.
+# They are in a separate file so it can be 'feature flagged off' until needed.
+---
 # The 'LRC PBMC Pools' purpose is controlled by Limber. However, it has been
 # added here to create submission and request type records for scRNA Core cDNA
 # Prep stage.
----
 LRC PBMC Pools:
   stock_plate: false
   cherrypickable_target: false
+# The 'LRC PBMC Pools Input' purpose is included here so that it's available for
+# sample manifests.
+LRC PBMC Pools Input:
+  stock_plate: true
+  cherrypickable_target: false

config/sample_manifest_excel/manifest_types.yml

@@ -255,6 +255,16 @@ plate_bioscan:
     - :sample_type
     - :donor_id
     - :bioscan_control_type
+plate_scrna_core_pools:
+  heading: "scRNA Core Pools Plate"
+  asset_type: "plate"
+  rows_per_well: 20
+  columns:
+    - :sanger_plate_id
+    - :well
+    - :sanger_sample_id
+    - :supplier_name
+    - :donor_id_mandatory
 tube_default:
   heading: "Default Tube"
   asset_type: "1dtube"

db/migrate/20240214144515_add_rows_per_well_to_sample_manifests.rb

@@ -0,0 +1,6 @@
+# frozen_string_literal: true
+class AddRowsPerWellToSampleManifests < ActiveRecord::Migration[6.0]
+  def change
+    add_column :sample_manifests, :rows_per_well, :integer
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2023_12_04_163029) do
+ActiveRecord::Schema.define(version: 2024_02_14_144515) do
 
   create_table "aliquot_indices", id: :integer, options: "ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci ROW_FORMAT=DYNAMIC", force: :cascade do |t|
     t.integer "aliquot_id", null: false
@@ -1327,6 +1327,7 @@
     t.string "password"
     t.integer "purpose_id"
     t.integer "tube_rack_purpose_id"
+    t.integer "rows_per_well"
     t.index ["purpose_id"], name: "fk_rails_5627ab4aaa"
     t.index ["study_id"], name: "index_sample_manifests_on_study_id"
     t.index ["supplier_id"], name: "index_sample_manifests_on_supplier_id"

spec/data/sample_manifest_excel/extract/manifest_types.yml

@@ -31,6 +31,7 @@ test_2:
 test_3:
   heading: "Yet Another test"
   asset_type: "plate"
+  rows_per_well: 2
   columns:
     - :sanger_plate_id
     - :well

spec/data/sample_manifest_excel/manifest_types.yml

@@ -701,3 +701,12 @@ tube_extraction:
     - :sample_ebi_accession_number
     - :donor_id
     - :phenotype
+pools_plate:
+  heading: "Pools Plate"
+  asset_type: "plate"
+  rows_per_well: 2
+  columns:
+    - :sanger_plate_id
+    - :well
+    - :sanger_sample_id
+    - :supplier_name

spec/features/sample_manifests/create_manifest_spec.rb

@@ -24,7 +24,7 @@ def load_manifest_spec
       select(study.name, from: 'Study')
       select(supplier.name, from: 'Supplier')
       within('#sample_manifest_template') do
-        expect(page).to have_css('option', count: 7)
+        expect(page).to have_css('option', count: 8)
         expect(page).not_to have_css('option', text: 'Default Tube')
       end
       select('Default Plate', from: 'Template')
@@ -66,7 +66,7 @@ def load_manifest_spec
 
     it 'indicate the purpose field is used for plates only' do
       visit(new_sample_manifest_path)
-      within('#sample_manifest_template') { expect(page).to have_css('option', count: 22) }
+      within('#sample_manifest_template') { expect(page).to have_css('option', count: 23) }
       select(created_purpose.name, from: 'Purpose')
       expect(page).to have_text('Used for plate manifests only')
     end