diff --git a/app/controllers/sdb/sample_manifests_controller.rb b/app/controllers/sdb/sample_manifests_controller.rb index ce718177b1..da67ea471f 100644 --- a/app/controllers/sdb/sample_manifests_controller.rb +++ b/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) diff --git a/app/jobs/sample_manifest/generate_wells_job.rb b/app/jobs/sample_manifest/generate_wells_job.rb index d4e27933fa..a2e090c863 100644 --- a/app/jobs/sample_manifest/generate_wells_job.rb +++ b/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 diff --git a/app/models/sample_manifest.rb b/app/models/sample_manifest.rb index 1254a30f9c..943d935ef9 100644 --- a/app/models/sample_manifest.rb +++ b/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 }) diff --git a/app/models/sample_manifest/generator.rb b/app/models/sample_manifest/generator.rb index 86c81ee471..0781656d57 100644 --- a/app/models/sample_manifest/generator.rb +++ b/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 diff --git a/app/models/sample_manifest/plate_behaviour.rb b/app/models/sample_manifest/plate_behaviour.rb index f5ab83b901..70e2eaea54 100644 --- a/app/models/sample_manifest/plate_behaviour.rb +++ b/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 diff --git a/app/sample_manifest_excel/sample_manifest_excel/manifest_type_list.rb b/app/sample_manifest_excel/sample_manifest_excel/manifest_type_list.rb index 0c8dc8f166..1588b04645 100644 --- a/app/sample_manifest_excel/sample_manifest_excel/manifest_type_list.rb +++ b/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 diff --git a/config/default_records/plate_purposes/011_scrna_core_cdna_prep_plate_purposes.wip.yml b/config/default_records/plate_purposes/011_scrna_core_cdna_prep_plate_purposes.wip.yml index a50ce87f06..faec61f135 100644 --- a/config/default_records/plate_purposes/011_scrna_core_cdna_prep_plate_purposes.wip.yml +++ b/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 diff --git a/config/sample_manifest_excel/manifest_types.yml b/config/sample_manifest_excel/manifest_types.yml index ecfec186a0..288af10a2d 100644 --- a/config/sample_manifest_excel/manifest_types.yml +++ b/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" diff --git a/db/migrate/20240214144515_add_rows_per_well_to_sample_manifests.rb b/db/migrate/20240214144515_add_rows_per_well_to_sample_manifests.rb new file mode 100644 index 0000000000..5fcd675472 --- /dev/null +++ b/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 diff --git a/db/schema.rb b/db/schema.rb index 45dc7c6275..d1b9c24d03 100644 --- a/db/schema.rb +++ b/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" diff --git a/spec/data/sample_manifest_excel/extract/manifest_types.yml b/spec/data/sample_manifest_excel/extract/manifest_types.yml index 18d12f2734..84f3d6e611 100644 --- a/spec/data/sample_manifest_excel/extract/manifest_types.yml +++ b/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 diff --git a/spec/data/sample_manifest_excel/manifest_types.yml b/spec/data/sample_manifest_excel/manifest_types.yml index fd308eb404..55214e10d6 100644 --- a/spec/data/sample_manifest_excel/manifest_types.yml +++ b/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 diff --git a/spec/features/sample_manifests/create_manifest_spec.rb b/spec/features/sample_manifests/create_manifest_spec.rb index 92e168706b..7ec31d0f9c 100644 --- a/spec/features/sample_manifests/create_manifest_spec.rb +++ b/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 diff --git a/spec/models/sample_manifest/generator_spec.rb b/spec/models/sample_manifest/generator_spec.rb index dea5c17c3e..2123cd0624 100644 --- a/spec/models/sample_manifest/generator_spec.rb +++ b/spec/models/sample_manifest/generator_spec.rb @@ -24,9 +24,10 @@ let!(:supplier) { create(:supplier) } let!(:barcode_printer) { create(:barcode_printer) } let(:configuration) { SampleManifestExcel.configuration } + let(:template) { 'plate_full' } let(:attributes) do - { template: 'plate_full', study_id: study.id, supplier_id: supplier.id, count: '4' }.with_indifferent_access + { template: template, study_id: study.id, supplier_id: supplier.id, count: '4' }.with_indifferent_access end after(:all) { SampleManifestExcel.reset! } @@ -39,7 +40,7 @@ expect(described_class.new(attributes, nil, configuration)).not_to be_valid end - it 'is not be unless all of the attributes are present' do + it 'is not valid unless all of the attributes are present' do SampleManifest::Generator::REQUIRED_ATTRIBUTES.each do |attribute| expect(described_class.new(attributes.except(attribute), user, configuration)).not_to be_valid end @@ -123,4 +124,24 @@ it 'does not have a print job if printer name has not been provided' do expect(described_class.new(attributes, user, configuration)).not_to be_print_job_required end + + context 'with rows_per_well set' do + let(:template) { 'pools_plate' } + + # TODO: un-skip when rows_per_well feature is enabled (when DPL-823 is complete) + skip 'generates a details array with more than one entry per well' do + generator = described_class.new(attributes, user, configuration) + generator.execute + expect(generator.sample_manifest.details_array.size).to eq(4 * 96 * 2) + end + end + + context 'with rows_per_well not set' do + # TODO: un-skip when rows_per_well feature is enabled (when DPL-823 is complete) + skip 'generates a details array with one entry per well' do + generator = described_class.new(attributes, user, configuration) + generator.execute + expect(generator.sample_manifest.details_array.size).to eq(4 * 96) + end + end end diff --git a/spec/sample_manifest_excel/manifest_type_list_spec.rb b/spec/sample_manifest_excel/manifest_type_list_spec.rb index 456deb8459..4f2f1ae814 100644 --- a/spec/sample_manifest_excel/manifest_type_list_spec.rb +++ b/spec/sample_manifest_excel/manifest_type_list_spec.rb @@ -20,6 +20,9 @@ expect(manifest_type.heading).to eq(v['heading']) expect(manifest_type.columns).to eq(v['columns']) expect(manifest_type.asset_type).to eq(v['asset_type']) + + # if the rows_per_well attribute isn't present, just compares nil == nil + expect(manifest_type.rows_per_well).to eq(v['rows_per_well']) end end