diff --git a/.github/workflows/check_release_version.yml b/.github/workflows/check_release_version.yml index bd49a9ec3c..d85e1525f5 100644 --- a/.github/workflows/check_release_version.yml +++ b/.github/workflows/check_release_version.yml @@ -5,12 +5,21 @@ on: pull_request: branches: - master + types: + # defaults + - opened + - synchronize + - reopened + # custom + - ready_for_review # required for Github-created PRs jobs: check: runs-on: ubuntu-latest + # only run when PR is not draft + if: ${{ !github.event.pull_request.draft }} steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v4 - name: Get specific changed files id: changed-files-specific diff --git a/.github/workflows/create_release_pr.yml b/.github/workflows/create_release_pr.yml new file mode 100644 index 0000000000..805b990a15 --- /dev/null +++ b/.github/workflows/create_release_pr.yml @@ -0,0 +1,10 @@ +# Create or update merge-to-master pull requests for production releases +# Note that by design, creating or editing a PR will not trigger a downstream `pull_request` event as this could lead to recursion +name: Release +on: + push: + branches: + - develop +jobs: + pull_request: + uses: sanger/.github/.github/workflows/create-release-pr.yml@master diff --git a/.nvmrc b/.nvmrc index b8e593f521..7af24b7ddb 100644 --- a/.nvmrc +++ b/.nvmrc @@ -1 +1 @@ -20.15.1 +22.11.0 diff --git a/.release-version b/.release-version index ce140f3641..4cc29b46f3 100644 --- a/.release-version +++ b/.release-version @@ -1 +1 @@ -14.45.0 +14.48.0 diff --git a/Gemfile.lock b/Gemfile.lock index 9dd2a685e4..20d5d306c4 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -34,28 +34,28 @@ GEM specs: aasm (5.5.0) concurrent-ruby (~> 1.0) - actioncable (6.1.7.8) - actionpack (= 6.1.7.8) - activesupport (= 6.1.7.8) + actioncable (6.1.7.10) + actionpack (= 6.1.7.10) + activesupport (= 6.1.7.10) nio4r (~> 2.0) websocket-driver (>= 0.6.1) - actionmailbox (6.1.7.8) - actionpack (= 6.1.7.8) - activejob (= 6.1.7.8) - activerecord (= 6.1.7.8) - activestorage (= 6.1.7.8) - activesupport (= 6.1.7.8) + actionmailbox (6.1.7.10) + actionpack (= 6.1.7.10) + activejob (= 6.1.7.10) + activerecord (= 6.1.7.10) + activestorage (= 6.1.7.10) + activesupport (= 6.1.7.10) mail (>= 2.7.1) - actionmailer (6.1.7.8) - actionpack (= 6.1.7.8) - actionview (= 6.1.7.8) - activejob (= 6.1.7.8) - activesupport (= 6.1.7.8) + actionmailer (6.1.7.10) + actionpack (= 6.1.7.10) + actionview (= 6.1.7.10) + activejob (= 6.1.7.10) + activesupport (= 6.1.7.10) mail (~> 2.5, >= 2.5.4) rails-dom-testing (~> 2.0) - actionpack (6.1.7.8) - actionview (= 6.1.7.8) - activesupport (= 6.1.7.8) + actionpack (6.1.7.10) + actionview (= 6.1.7.10) + activesupport (= 6.1.7.10) rack (~> 2.0, >= 2.0.9) rack-test (>= 0.6.3) rails-dom-testing (~> 2.0) @@ -63,44 +63,44 @@ GEM actionpack-xml_parser (2.0.1) actionpack (>= 5.0) railties (>= 5.0) - actiontext (6.1.7.8) - actionpack (= 6.1.7.8) - activerecord (= 6.1.7.8) - activestorage (= 6.1.7.8) - activesupport (= 6.1.7.8) + actiontext (6.1.7.10) + actionpack (= 6.1.7.10) + activerecord (= 6.1.7.10) + activestorage (= 6.1.7.10) + activesupport (= 6.1.7.10) nokogiri (>= 1.8.5) - actionview (6.1.7.8) - activesupport (= 6.1.7.8) + actionview (6.1.7.10) + activesupport (= 6.1.7.10) builder (~> 3.1) erubi (~> 1.4) rails-dom-testing (~> 2.0) rails-html-sanitizer (~> 1.1, >= 1.2.0) - activejob (6.1.7.8) - activesupport (= 6.1.7.8) + activejob (6.1.7.10) + activesupport (= 6.1.7.10) globalid (>= 0.3.6) - activemodel (6.1.7.8) - activesupport (= 6.1.7.8) + activemodel (6.1.7.10) + activesupport (= 6.1.7.10) activemodel-serializers-xml (1.0.2) activemodel (> 5.x) activesupport (> 5.x) builder (~> 3.1) - activerecord (6.1.7.8) - activemodel (= 6.1.7.8) - activesupport (= 6.1.7.8) + activerecord (6.1.7.10) + activemodel (= 6.1.7.10) + activesupport (= 6.1.7.10) activerecord-import (1.7.0) activerecord (>= 4.2) activeresource (6.1.0) activemodel (>= 6.0) activemodel-serializers-xml (~> 1.0) activesupport (>= 6.0) - activestorage (6.1.7.8) - actionpack (= 6.1.7.8) - activejob (= 6.1.7.8) - activerecord (= 6.1.7.8) - activesupport (= 6.1.7.8) + activestorage (6.1.7.10) + actionpack (= 6.1.7.10) + activejob (= 6.1.7.10) + activerecord (= 6.1.7.10) + activesupport (= 6.1.7.10) marcel (~> 1.0) mini_mime (>= 1.1.0) - activesupport (6.1.7.8) + activesupport (6.1.7.10) concurrent-ruby (~> 1.0, >= 1.0.2) i18n (>= 1.6, < 2) minitest (>= 5.1) @@ -198,7 +198,7 @@ GEM activerecord (>= 5.a) database_cleaner-core (~> 2.0.0) database_cleaner-core (2.0.1) - date (3.3.4) + date (3.4.0) delayed_job (4.1.11) activesupport (>= 3.0, < 8.0) delayed_job_active_record (4.1.8) @@ -259,7 +259,7 @@ GEM rb-fsevent (~> 0.10, >= 0.10.3) rb-inotify (~> 0.9, >= 0.9.10) logger (1.6.1) - loofah (2.22.0) + loofah (2.23.1) crass (~> 1.0.2) nokogiri (>= 1.12.0) mail (2.8.1) @@ -287,7 +287,7 @@ GEM mustermann (3.0.0) ruby2_keywords (~> 0.0.1) mysql2 (0.5.6) - net-imap (0.4.14) + net-imap (0.5.0) date net-protocol net-ldap (0.19.0) @@ -298,7 +298,7 @@ GEM net-smtp (0.5.0) net-protocol netrc (0.11.0) - nio4r (2.7.3) + nio4r (2.7.4) nokogiri (1.16.7-arm64-darwin) racc (~> 1.4) nokogiri (1.16.7-x86_64-darwin) @@ -324,7 +324,7 @@ GEM puma (6.4.3) nio4r (~> 2.0) racc (1.8.1) - rack (2.2.9) + rack (2.2.10) rack-acceptable (0.1.0) rack (>= 1.1.0) rack-cors (2.0.2) @@ -338,20 +338,20 @@ GEM rack rack-test (2.1.0) rack (>= 1.3) - rails (6.1.7.8) - actioncable (= 6.1.7.8) - actionmailbox (= 6.1.7.8) - actionmailer (= 6.1.7.8) - actionpack (= 6.1.7.8) - actiontext (= 6.1.7.8) - actionview (= 6.1.7.8) - activejob (= 6.1.7.8) - activemodel (= 6.1.7.8) - activerecord (= 6.1.7.8) - activestorage (= 6.1.7.8) - activesupport (= 6.1.7.8) + rails (6.1.7.10) + actioncable (= 6.1.7.10) + actionmailbox (= 6.1.7.10) + actionmailer (= 6.1.7.10) + actionpack (= 6.1.7.10) + actiontext (= 6.1.7.10) + actionview (= 6.1.7.10) + activejob (= 6.1.7.10) + activemodel (= 6.1.7.10) + activerecord (= 6.1.7.10) + activestorage (= 6.1.7.10) + activesupport (= 6.1.7.10) bundler (>= 1.15.0) - railties (= 6.1.7.8) + railties (= 6.1.7.10) sprockets-rails (>= 2.0.0) rails-controller-testing (1.0.5) actionpack (>= 5.0.1.rc1) @@ -370,9 +370,9 @@ GEM loofah (~> 2.21) nokogiri (~> 1.14) rails-perftest (0.0.7) - railties (6.1.7.8) - actionpack (= 6.1.7.8) - activesupport (= 6.1.7.8) + railties (6.1.7.10) + actionpack (= 6.1.7.10) + activesupport (= 6.1.7.10) method_source rake (>= 12.2) thor (~> 1.0) @@ -390,7 +390,7 @@ GEM http-cookie (>= 1.0.2, < 2.0) mime-types (>= 1.16, < 4.0) netrc (~> 0.8) - rexml (3.3.7) + rexml (3.3.9) roo (2.10.1) nokogiri (~> 1) rubyzip (>= 1.3.0, < 3.0.0) @@ -502,7 +502,7 @@ GEM sprockets (4.2.1) concurrent-ruby (~> 1.0) rack (>= 2.2.4, < 4) - sprockets-rails (3.5.1) + sprockets-rails (3.5.2) actionpack (>= 6.1) activesupport (>= 6.1) sprockets (>= 3.0.0) @@ -562,7 +562,7 @@ GEM ostruct rainbow yard - zeitwerk (2.6.18) + zeitwerk (2.7.1) PLATFORMS arm64-darwin diff --git a/app/controllers/api/v2/bait_library_layouts_controller.rb b/app/controllers/api/v2/bait_library_layouts_controller.rb index f518fdb834..4e1edd66d5 100644 --- a/app/controllers/api/v2/bait_library_layouts_controller.rb +++ b/app/controllers/api/v2/bait_library_layouts_controller.rb @@ -33,7 +33,7 @@ def preview respond_with_errors('Validation failed', e.record.errors.full_messages, :unprocessable_entity) and return end - json = { data: { type: 'bait_library_layouts', attributes: { layout: preview.layout } } } + json = { data: { type: 'bait_library_layouts', attributes: { well_layout: preview.well_layout } } } render json: json, status: :ok end diff --git a/app/models/accessionable/base.rb b/app/models/accessionable/base.rb index 16afd5819e..0093d815ca 100644 --- a/app/models/accessionable/base.rb +++ b/app/models/accessionable/base.rb @@ -95,6 +95,8 @@ class FieldSerializer MISSING_DATA_AGGREEMENT_PRE2023 = 'missing: data agreement established pre-2023' MISSING_ENDANGERED_SPECIES = 'missing: endangered species' MISSING_HUMAN_IDENTIFIABLE = 'missing: human-identifiable' + MISSING_CONTROL_SAMPLE = 'missing: control sample' + MISSING_SAMPLE_GROUP = 'missing: sample group' OTHER_DEFAULT_SETTINGS = [ NOT_COLLECTED, @@ -107,7 +109,9 @@ class FieldSerializer MISING_THIRD_PARTY_DATA, MISSING_DATA_AGGREEMENT_PRE2023, MISSING_ENDANGERED_SPECIES, - MISSING_HUMAN_IDENTIFIABLE + MISSING_HUMAN_IDENTIFIABLE, + MISSING_CONTROL_SAMPLE, + MISSING_SAMPLE_GROUP ].freeze def value_for(value) diff --git a/app/models/cherrypick_task.rb b/app/models/cherrypick_task.rb index 7fb4b9ce50..031a599eb0 100644 --- a/app/models/cherrypick_task.rb +++ b/app/models/cherrypick_task.rb @@ -21,8 +21,16 @@ class CherrypickTask < Task # rubocop:todo Metrics/ClassLength # # @return [CherrypickTask::ControlLocator] A generator of control locations # - def new_control_locator(batch_id, total_wells, num_control_wells, wells_to_leave_free: DEFAULT_WELLS_TO_LEAVE_FREE) - CherrypickTask::ControlLocator.new(batch_id:, total_wells:, num_control_wells:, wells_to_leave_free:) + + def new_control_locator(params) + CherrypickTask::ControlLocator.new( + batch_id: params[:batch_id], + total_wells: params[:total_wells], + num_control_wells: params[:num_control_wells], + wells_to_leave_free: params[:wells_to_leave_free] || DEFAULT_WELLS_TO_LEAVE_FREE, + control_source_plate: params[:control_source_plate], + template: params[:template] + ) end # @@ -38,7 +46,7 @@ def can_link_directly? # rubocop:todo Metrics/ParameterLists def pick_new_plate(requests, template, robot, plate_purpose, control_source_plate = nil, workflow_controller = nil) target_type = PickTarget.for(plate_purpose) - perform_pick(requests, robot, control_source_plate, workflow_controller) do + perform_pick(requests, robot, control_source_plate, workflow_controller, template) do target_type.new(template, plate_purpose.try(:asset_shape)) end end @@ -54,7 +62,7 @@ def pick_onto_partial_plate( purpose = partial_plate.plate_purpose target_type = PickTarget.for(purpose) - perform_pick(requests, robot, control_source_plate, workflow_controller) do + perform_pick(requests, robot, control_source_plate, workflow_controller, template) do target_type .new(template, purpose.try(:asset_shape), partial_plate) .tap do @@ -66,7 +74,7 @@ def pick_onto_partial_plate( # rubocop:enable Metrics/ParameterLists # rubocop:todo Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity, Metrics/MethodLength - def perform_pick(requests, robot, control_source_plate, workflow_controller) # rubocop:todo Metrics/AbcSize + def perform_pick(requests, robot, control_source_plate, workflow_controller, template) # rubocop:todo Metrics/AbcSize max_plates = robot.max_beds raise StandardError, 'The chosen robot has no beds!' if max_plates.zero? @@ -80,7 +88,22 @@ def perform_pick(requests, robot, control_source_plate, workflow_controller) # r num_plate = 0 batch = requests.first.batch control_assets = control_source_plate.wells.joins(:samples) - control_locator = new_control_locator(batch.id, current_destination_plate.size, control_assets.count) + control_locator = + new_control_locator( + { + batch_id: batch.id, + total_wells: current_destination_plate.size, + num_control_wells: control_assets.count, + template: template, + control_source_plate: control_source_plate + } + ) + + if control_locator.handle_incompatible_plates + message = 'The control plate and plate template are incompatible' + workflow_controller.send(:flash)[:error] = message unless workflow_controller.nil? + workflow_controller.redirect_to action: 'stage', batch_id: batch.id, workflow_id: workflow.id + end control_posns = control_locator.control_positions(num_plate) # If is an incomplete plate, or a plate with a template applied, copy all the controls missing into the diff --git a/app/models/cherrypick_task/control_locator.rb b/app/models/cherrypick_task/control_locator.rb index 91313fb121..2811dc3b89 100644 --- a/app/models/cherrypick_task/control_locator.rb +++ b/app/models/cherrypick_task/control_locator.rb @@ -29,23 +29,34 @@ class CherrypickTask::ControlLocator # limit ourself to primes to simplify validation. BETWEEN_PLATE_OFFSETS = [53, 59].freeze - attr_reader :batch_id, :total_wells, :wells_to_leave_free, :num_control_wells, :available_positions + attr_reader :batch_id, + :total_wells, + :wells_to_leave_free, + :num_control_wells, + :available_positions, + :control_source_plate # @note wells_to_leave_free was originally hardcoded for 96 well plates at 24, in order to avoid # control wells being missed in cDNA quant QC. This requirement was removed in # https://github.com/sanger/sequencescape/issues/2967 however I've avoided stripping out the behaviour # completely in case controls are used in other pipelines. # - # @param batch_id [Integer] The id of the batch, used to generate a starting position - # @param total_wells [Integer] The total number of wells on the plate - # @param num_control_wells [Integer] The number of control wells to lay out - # @param wells_to_leave_free [Enumerable] Array or range indicating the wells to leave free from controls - def initialize(batch_id:, total_wells:, num_control_wells:, wells_to_leave_free: []) - @batch_id = batch_id - @total_wells = total_wells - @num_control_wells = num_control_wells - @wells_to_leave_free = wells_to_leave_free.to_a - @available_positions = (0...total_wells).to_a - @wells_to_leave_free + # @param params [Hash] A hash containing the following keys: + # - :batch_id [Integer] The id of the batch, used to generate a starting position + # - :total_wells [Integer] The total number of wells on the plate + # - :num_control_wells [Integer] The number of control wells to lay out + # - :wells_to_leave_free [Enumerable] Array or range indicating the wells to leave free from controls + # - :control_source_plate [ControlPlate] The plate to source controls from + # - :template [PlateTemplate] The template of the destination plate + + def initialize(params) + @batch_id = params[:batch_id] + @total_wells = params[:total_wells] + @num_control_wells = params[:num_control_wells] + @wells_to_leave_free = params[:wells_to_leave_free].to_a || [] + @available_positions = (0...@total_wells).to_a - @wells_to_leave_free + @control_source_plate = params[:control_source_plate] + @plate_template = params[:template] end # @@ -55,7 +66,7 @@ def initialize(batch_id:, total_wells:, num_control_wells:, wells_to_leave_free: # @param num_plate [Integer] The plate number within the batch # # @return [Array] The indexes of the control well positions - # + def control_positions(num_plate) raise StandardError, 'More controls than free wells' if num_control_wells > total_available_positions @@ -65,9 +76,25 @@ def control_positions(num_plate) # If num plate is equal to the available positions, the cycle is going to be repeated. # To avoid it, every num_plate=available_positions we start a new cycle with a new seed. - seed = seed_for(num_plate) - initial_positions = random_positions_from_available(seed) - control_positions_for_plate(num_plate, initial_positions) + + placement_type = control_placement_type + if placement_type.nil? || %w[fixed random].exclude?(placement_type) + raise StandardError, 'Control placement type is not set or is invalid' + end + + handle_control_placement_type(placement_type, num_plate) + end + + def handle_incompatible_plates + return false if control_placement_type == 'random' + return false if @plate_template.wells.empty? + + control_assets = control_source_plate.wells.joins(:samples) + + converted_control_assets = convert_assets(control_assets.map(&:map_id)) + converted_template_assets = convert_assets(@plate_template.wells.map(&:map_id)) + + converted_control_assets.intersect?(converted_template_assets) end private @@ -96,6 +123,49 @@ def random_positions_from_available(seed) available_positions.sample(num_control_wells, random: Random.new(seed)) end + def control_placement_type + @control_source_plate.custom_metadatum_collection.metadata['control_placement_type'] + end + + def handle_control_placement_type(placement_type, num_plate) + if placement_type == 'random' + control_positions_for_plate(num_plate, random_positions_from_available(seed_for(num_plate))) + else + fixed_positions_from_available + end + end + + # Because the control source plate wells are ordered inversely to the destination plate wells, + # the control asset ids need to be converted to the corresponding destination plate well indexes. + + def convert_assets(control_assets) + valid_map, invalid_map = create_plate_maps + + control_assets.map do |id| + invalid_location = valid_map[id] + invalid_map.key(invalid_location) - 1 + end + end + + def fixed_positions_from_available + control_assets = @control_source_plate.wells.joins(:samples) + control_wells = control_assets.map(&:map_id) + convert_assets(control_wells) + end + + # The invalid and valid maps are hash maps to represent a plate that maps A1 -> 1, A2 -> 2, etc, + # whereas the other map is the inverse of this, mapping 1 -> A1, 2 -> B1, etc. + + def create_plate_maps + rows = ('A'..'H').to_a + columns = (1..12).to_a + + valid_map = rows.product(columns).each_with_index.to_h { |(row, col), i| [i + 1, "#{row}#{col}"] } + invalid_map = columns.product(rows).each_with_index.to_h { |(col, row), i| [i + 1, "#{row}#{col}"] } + + [valid_map, invalid_map] + end + # Works out which offset to use based on the number of available wells and ensures we use # all wells before looping. Will select the first suitable value from BETWEEN_PLATE_OFFSETS # excluding any numbers that are a factor of the available wells. In the incredibly unlikely diff --git a/app/models/delegate_validation.rb b/app/models/delegate_validation.rb index c9a3cb1fe3..bef99c546f 100644 --- a/app/models/delegate_validation.rb +++ b/app/models/delegate_validation.rb @@ -101,7 +101,11 @@ def valid? return true if @validators.map(&:valid?).all?(true) @validators.each do |validator| - validator.errors.each { |attrib, message| errors.add(attrib, message) unless errors.include?(attrib) } + validator.errors.each do |error| + attrib = error.attribute + message = error.message + errors.add(attrib, message) unless errors.include?(attrib) + end end false diff --git a/app/models/messenger.rb b/app/models/messenger.rb index 20528b4d70..f12e3bf46b 100644 --- a/app/models/messenger.rb +++ b/app/models/messenger.rb @@ -18,6 +18,13 @@ def as_json(_options = {}) { root => render_class.to_hash(target), 'lims' => configatron.amqp.lims_id! } end + def template + # Replace IO with Io to match the class name + # This is a consequence of the zeitwerk renaming for the message modules from IO to Io + # This ensures that the correct class is loaded for historical messages + read_attribute(:template).gsub(/IO$/, 'Io') + end + def resend Warren.handler << Warren::Message::Short.new(self) end diff --git a/app/models/plate_purpose.rb b/app/models/plate_purpose.rb index 712b4ef55a..00ed583209 100644 --- a/app/models/plate_purpose.rb +++ b/app/models/plate_purpose.rb @@ -88,7 +88,11 @@ def pool_wells(wells) # rubocop:todo Metrics/MethodLength has_many :plates def self.stock_plate_purpose - PlatePurpose.create_with(stock_plate: true, cherrypickable_target: true).find_or_create_by!(name: 'Stock Plate') + PlatePurpose.create_with( + stock_plate: true, + cherrypickable_target: true, + type: 'PlatePurpose::Input' + ).find_or_create_by!(name: 'Stock Plate') end def size diff --git a/app/models/submission/validations_by_template_name.rb b/app/models/submission/validations_by_template_name.rb index e3ed8e1517..36ee7710a4 100644 --- a/app/models/submission/validations_by_template_name.rb +++ b/app/models/submission/validations_by_template_name.rb @@ -6,7 +6,9 @@ module Submission::ValidationsByTemplateName # Column headers HEADER_TEMPLATE_NAME = 'template name' HEADER_STUDY_NAME = 'study name' + HEADER_PROJECT_NAME = 'project name' HEADER_NUM_SAMPLES = 'scrna core number of samples per pool' + HEADER_CELLS_PER_CHIP_WELL = 'scrna core cells per chip well' # Applies additional validations based on the submission template type. # @@ -30,38 +32,38 @@ def apply_additional_validations_by_template_name case submission_template_name # this validation is for the scRNA pipeline cDNA submission when SCRNA_CORE_CDNA_PREP_GEM_X_5P - validate_scrna_core_samples_per_pool + validate_consistent_column_value(HEADER_NUM_SAMPLES) + validate_consistent_column_value(HEADER_CELLS_PER_CHIP_WELL) end end - # Validates that the scrna core number of samples per pool is consistent for all rows with the same study name. + # Validates that the specified column is consistent for all rows with the same study and project name. # - # This method groups the rows in the CSV data by the study name and checks if the scrna core number of samples - # per pool is the same for all rows within each study group. If inconsistencies are found, an error is added to - # the errors collection. + # This method groups the rows in the CSV data by the study name and project name, and checks if the specified column + # has the same value for all rows within each group. If inconsistencies are found, an error is + # added to the errors collection. # + # @param column_header [String] The header of the column to validate. # @return [void] - # rubocop:disable Metrics/MethodLength - def validate_scrna_core_samples_per_pool - # Group rows by study name + # rubocop:disable Metrics/MethodLength,Metrics/AbcSize + def validate_consistent_column_value(column_header) index_of_study_name = headers.index(HEADER_STUDY_NAME) - grouped_rows = csv_data_rows.group_by { |row| row[index_of_study_name] } + index_of_project_name = headers.index(HEADER_PROJECT_NAME) + index_of_column = headers.index(column_header) - # Iterate through each study group - grouped_rows.each do |study_name, rows| - # Get the unique values of scrna core number of samples per pool for the group - index_of_num_samples = headers.index(HEADER_NUM_SAMPLES) - list_of_uniq_number_of_samples_per_pool = rows.pluck(index_of_num_samples).uniq + grouped_rows = csv_data_rows.group_by { |row| [row[index_of_study_name], row[index_of_project_name]] } - # Check if there is more than one unique value - next unless list_of_uniq_number_of_samples_per_pool.size > 1 + grouped_rows.each do |study_project, rows| + unique_values = rows.pluck(index_of_column).uniq + + next unless unique_values.size > 1 errors.add( :spreadsheet, - "Inconsistent values for column 'scRNA Core Number of Samples per Pool' for Study name '#{study_name}', " \ - 'all rows for a specific study must have the same value' + "Inconsistent values for column '#{column_header}' for Study name '#{study_project[0]}' and Project name " \ + "'#{study_project[1]}', " \ + 'all rows for a specific study and project must have the same value' ) end end - - # rubocop:enable Metrics/MethodLength + # rubocop:enable Metrics/MethodLength,Metrics/AbcSize end diff --git a/app/models/tag_layout.rb b/app/models/tag_layout.rb index 55bf3c9228..691886bc4f 100644 --- a/app/models/tag_layout.rb +++ b/app/models/tag_layout.rb @@ -64,8 +64,8 @@ def walking_by validates :walking_by, presence: { message: 'must define a valid algorithm' } # After creating the instance we can layout the tags into the wells. - after_create :layout_tags_into_wells, if: :valid? + after_create :layout_tags_into_wells, if: :valid? set_target_for_owner(:plate) delegate :direction, to: :direction_algorithm_module diff --git a/app/models/transfer_request.rb b/app/models/transfer_request.rb index 7d0c9166cc..6bf33efb03 100644 --- a/app/models/transfer_request.rb +++ b/app/models/transfer_request.rb @@ -80,7 +80,7 @@ class TransferRequest < ApplicationRecord # rubocop:todo Metrics/ClassLength end event :process_1 do - transitions to: :processed_1, from: [:pending] + transitions to: :processed_1, from: [:pending], after: :on_started end event :process_2 do diff --git a/app/views/receptacles/new_request.html.erb b/app/views/receptacles/new_request.html.erb index b1b1d489d9..55283f9ce2 100644 --- a/app/views/receptacles/new_request.html.erb +++ b/app/views/receptacles/new_request.html.erb @@ -9,7 +9,9 @@ <% instance_variable_or_id_param(:request_type) do |field_name, value| %>
<%= label_tag(field_name, 'Request type', class: 'col-md-4') %> - <%= select_field_sorted_by_name(field_name, @request_types, value, can_edit) %> + <%= select_field_sorted_by_name(field_name, @request_types, value, can_edit, prompt: 'Select a request type', required: true, + data: { novaseq_ids: RequestType.where(key: %w[illumina_htp_novaseq_6000_paired_end_sequencing illumina_htp_novaseqx_paired_end_sequencing]).pluck(:id).join(',') } + ) %>
<% end %> <% if @asset.studies.uniq.count > 1 -%> @@ -104,6 +106,17 @@ var disable_inputs = function(context) { $('input,select,textarea', context).attr('disabled', true); } var enable_inputs = function(context) { $('input,select,textarea', context).attr('disabled', false); } + + + setFlowcellTypeRequiredForNovaseq = function() { + var selected_request_type_id = $('option:selected', request_type_element).attr('value'); + var flowcell_type_element = $('[name="request[request_metadata_attributes][requested_flowcell_type]"]:not(:disabled)'); + if(flowcell_type_element && flowcell_type_element.length > 0) { + var novaseq_ids = $(request_type_element).data('novaseq-ids').split(','); + flowcell_type_element.attr('required', novaseq_ids.includes(selected_request_type_id)); + } + } + handler = function() { var selected_request_type_id = $('option:selected', request_type_element).attr('value'); @@ -111,6 +124,7 @@ // the parameters passed to the server are incorrect. disable_inputs(request_options.hide()); enable_inputs($('#request_type_options_for_' + selected_request_type_id).show()); + setFlowcellTypeRequiredForNovaseq() } request_type_element.change(handler); diff --git a/app/views/shared/metadata/edit/_request.html.erb b/app/views/shared/metadata/edit/_request.html.erb index ee380c1a54..b62a5318cc 100644 --- a/app/views/shared/metadata/edit/_request.html.erb +++ b/app/views/shared/metadata/edit/_request.html.erb @@ -5,7 +5,7 @@ <%- request.request_metadata.field_infos.each do |field_info| %> <%- if field_info.selection %> - <%= metadata_fields.select(field_info.key, field_info.selection) %> + <%= metadata_fields.select(field_info.key, field_info.selection, { prompt: "Select a #{field_info.key.to_s.gsub( '_', ' ')}" }) %> <%- elsif field_info.kind == FieldInfo::BOOLEAN %> <%= metadata_fields.check_box(field_info.key) %> <%- elsif field_info.kind == FieldInfo::NUMERIC %> diff --git a/app/views/workflows/_plate_template_batches.html.erb b/app/views/workflows/_plate_template_batches.html.erb index dac8d76df3..e3fbeb9b77 100644 --- a/app/views/workflows/_plate_template_batches.html.erb +++ b/app/views/workflows/_plate_template_batches.html.erb @@ -19,8 +19,15 @@ <%= select(:plate_template, "0", @plate_templates.map { |pt| [pt.name, pt.id ] }, {}, class: 'form-control select2') %>

Templates define which wells to leave empty on a plate when you cherrypick samples. You can add to an existing partial plate by scanning the barcode, or entering the plate ID. The plate must have been previously picked in Sequencescape. Wells can be rearranged in the next step.

- - <%= select("Control", "plate_id", ControlPlate.all.collect {|p| [ "#{p.human_barcode} - #{p.name}", p.id ] }, { include_blank: true }, class: 'form-control select2') %> + + <%= select("Control", "plate_id", + ControlPlate.all.collect do |p| + placement_type = p.custom_metadatum_collection&.metadata&.[]('control_placement_type') + if placement_type.present? + [ "#{p.human_barcode} - #{p.name} (#{placement_type.capitalize})", p.id ] + end + end.compact, + { include_blank: true }, class: 'form-control select2') %> diff --git a/config/default_records/plate_purposes/005_limber_purposes.yml b/config/default_records/plate_purposes/005_limber_purposes.yml index e0fa1efcc2..fa603dd415 100644 --- a/config/default_records/plate_purposes/005_limber_purposes.yml +++ b/config/default_records/plate_purposes/005_limber_purposes.yml @@ -18,6 +18,8 @@ LTN Cherrypick: *limber_input_96 LHR RT: *limber_input_96 LTHR RT: *limber_input_96 LTHR Cherrypick: *limber_input_96 +Stock Plate: *limber_input_96 + PF Cherrypicked: <<: *limber_input_96 default_state: passed @@ -70,3 +72,8 @@ LTN AL Lib: LTN Lib PCR XP: type: PlatePurpose stock_plate: false + +# RVI BCL plate for reISC +RVI Lib PCR XP: + type: PlatePurpose + stock_plate: false diff --git a/config/default_records/product_catalogues/007_novaseqx_product_catalogue.yml b/config/default_records/product_catalogues/007_novaseqx_product_catalogue.yml index 0f438fda0c..a22dbb236f 100644 --- a/config/default_records/product_catalogues/007_novaseqx_product_catalogue.yml +++ b/config/default_records/product_catalogues/007_novaseqx_product_catalogue.yml @@ -18,6 +18,8 @@ RNAAG: selection_behaviour: SingleProduct RNARG: selection_behaviour: SingleProduct +RNAF: + selection_behaviour: SingleProduct PFHSqX: selection_behaviour: SingleProduct GnT Picoplex: diff --git a/config/default_records/request_types/004_limber_high_throughput.yml b/config/default_records/request_types/004_limber_high_throughput.yml index 11bb84fa84..fb0291e6bf 100644 --- a/config/default_records/request_types/004_limber_high_throughput.yml +++ b/config/default_records/request_types/004_limber_high_throughput.yml @@ -37,6 +37,7 @@ limber_reisc: acceptable_purposes: - LB Lib PCR-XP - LTN Lib PCR XP # for Targeted NanoSeq ReISC + - RVI Lib PCR XP # for RVI Bait Capture Library library_types: - Agilent Pulldown - Twist Pulldown @@ -44,6 +45,7 @@ limber_reisc: - Targeted NanoSeq Pulldown Twist - Targeted NanoSeq Pulldown Agilent - BGE + - RVI-BCL limber_pcr_free: <<: *limber_htp_library name: Limber PCR Free @@ -199,3 +201,10 @@ limber_targeted_nanoseq: - LTN Cherrypick library_types: - Targeted Nanoseq +limber_rnaf: + <<: *limber_htp_library + name: Limber RNAF + acceptable_purposes: + - LBR Cherrypick + library_types: + - RNA FFPE diff --git a/config/default_records/request_types/005_limber_bespoke.yml b/config/default_records/request_types/005_limber_bespoke.yml index 060bd2364f..36969088f0 100644 --- a/config/default_records/request_types/005_limber_bespoke.yml +++ b/config/default_records/request_types/005_limber_bespoke.yml @@ -10,6 +10,7 @@ limber_pcr_bespoke: acceptable_purposes: - LBB Cherrypick - LBC Cherrypick + - Stock Plate library_types: - ChIP-Seq Auto - Chromium single cell HTO diff --git a/config/default_records/submission_templates/015_rnaf_submission_templates.wip.yml b/config/default_records/submission_templates/015_rnaf_submission_templates.wip.yml new file mode 100644 index 0000000000..a4fd4dd4c5 --- /dev/null +++ b/config/default_records/submission_templates/015_rnaf_submission_templates.wip.yml @@ -0,0 +1,33 @@ +Limber-Htp - RNAF: + submission_class_name: "LinearSubmission" + related_records: + product_line_name: Illumina-HTP + product_catalogue_name: RNAF + request_type_keys: ["limber_rnaf"] +Limber-Htp - RNAF - Pool: + submission_class_name: "LinearSubmission" + related_records: + product_line_name: Illumina-HTP + product_catalogue_name: RNAF + request_type_keys: ["limber_rnaf", "limber_multiplexing"] +Limber-Htp - RNAF - NovaSeqX paired end sequencing: + name: "Limber-Htp - RNAF - NovaSeqX paired end sequencing" + submission_class_name: "LinearSubmission" + related_records: + product_line_name: Illumina-HTP + product_catalogue_name: RNAF + request_type_keys: ["limber_rnaf", "limber_multiplexing", "illumina_htp_novaseqx_paired_end_sequencing"] +Limber-Htp - RNAF - NovaSeq 6000 paired end sequencing: + name: "Limber-Htp - RNAF - NovaSeq 6000 paired end sequencing" + submission_class_name: "LinearSubmission" + related_records: + product_line_name: Illumina-HTP + product_catalogue_name: RNAF + request_type_keys: ["limber_rnaf", "limber_multiplexing", "illumina_htp_novaseq_6000_paired_end_sequencing"] +Limber-Htp - RNAF - MiSeq sequencing: + name: "Limber-Htp - RNAF - MiSeq sequencing" + submission_class_name: "LinearSubmission" + related_records: + product_line_name: Illumina-HTP + product_catalogue_name: RNAF + request_type_keys: ["limber_rnaf", "limber_multiplexing", "illumina_b_miseq_sequencing"] diff --git a/db/migrate/20241018131928_disable_invalid_insdc_countries.rb b/db/migrate/20241018131928_disable_invalid_insdc_countries.rb new file mode 100644 index 0000000000..143c3ed69b --- /dev/null +++ b/db/migrate/20241018131928_disable_invalid_insdc_countries.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +# This migration corrects the country list for missing samples/sample groups +# following EBI's checklist https://www.ebi.ac.uk/ena/browser/view/ERC000011 +class DisableInvalidInsdcCountries < ActiveRecord::Migration[6.1] + def change + # Disable existing invalid countries + # We don't want to delete them yet in case they are used in existing records and existing manifests. + ['not applicable: control sample', 'not applicable: sample group'].each do |name| + Insdc::Country.find_by(name:)&.invalid! + end + + # Add missing countries + ['missing: control sample', 'missing: sample group'].each do |name| + Insdc::Country.find_or_create_by(name: name, sort_priority: -2, validation_state: 0) + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 2f924b36c3..940e4e5f29 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: 2024_09_17_133813) do +ActiveRecord::Schema.define(version: 2024_10_18_131928) do create_table "aliquot_indices", id: :integer, charset: "utf8mb4", collation: "utf8mb4_unicode_ci", options: "ENGINE=InnoDB ROW_FORMAT=DYNAMIC", force: :cascade do |t| t.integer "aliquot_id", null: false diff --git a/db/seeds/0001_snp_plate_purposes.rb b/db/seeds/0001_snp_plate_purposes.rb index 3d57f61f9c..2693e3e51a 100644 --- a/db/seeds/0001_snp_plate_purposes.rb +++ b/db/seeds/0001_snp_plate_purposes.rb @@ -8,9 +8,6 @@ cherrypickable_target: true stock_plate: false prefix: WD - - name: Stock Plate - stock_plate: true - cherrypickable_target: true - name: 40ng - name: Whole Genome Amplification cherrypickable_target: true diff --git a/features/api/tag_layout_templates.feature b/features/api/tag_layout_templates.feature index b9c5a38776..903143beb7 100644 --- a/features/api/tag_layout_templates.feature +++ b/features/api/tag_layout_templates.feature @@ -1,4 +1,4 @@ -@api @json @tag_layout_template @single-sign-on @new-api +@wip @api @json @tag_layout_template @single-sign-on @new-api Feature: Access tag layout templates through the API In order to actually be able to do anything useful As an authenticated user of the API diff --git a/features/api/tag_layouts.feature b/features/api/tag_layouts.feature index d095561f1a..e50694c94e 100644 --- a/features/api/tag_layouts.feature +++ b/features/api/tag_layouts.feature @@ -55,46 +55,6 @@ Feature: Access tag layouts through the API } """ - @tag_layout @create @barcode-service - Scenario: Creating a tag layout of an entire plate using 96 tags by pools - Given the Baracoda barcode service returns "SQPD-1000001" - Given the Baracoda barcode service returns "SQPD-1000002" - Given the tag group "Example Tag Group" exists - And the UUID for the tag group "Example Tag Group" is "00000000-1111-2222-3333-444444444444" - And the tag group "Example Tag Group" has 20 tags - - Given a "Stock plate" plate called "Testing the API" exists - And the UUID for the plate "Testing the API" is "11111111-2222-3333-4444-000000000002" - And all wells on the plate "Testing the API" have unique samples - - Given a "Stock plate" plate called "Testing the tagging" exists - And the UUID for the plate "Testing the tagging" is "11111111-2222-3333-4444-000000000001" - And the wells for the plate "Testing the API" have been pooled to the plate "Testing the tagging" according to the pooling strategy 12, 8, 20, 12, 8, 20, 16 - - When I make an authorised POST with the following JSON to the API path "/tag_layouts": - """ - { - "tag_layout": { - "plate": "11111111-2222-3333-4444-000000000001", - "user": "99999999-8888-7777-6666-555555555555", - "tag_group": "00000000-1111-2222-3333-444444444444", - "direction": "column", - "walking_by": "manual by pool", - "initial_tag": 0 - } - } - """ - Then the HTTP response should be "201 Created" - - Then the tag layout on the plate "Testing the tagging" should be: - | TAG1 | TAG9 | TAG5 | TAG5 | TAG13 | TAG1 | TAG9 | TAG5 | TAG5 | TAG13 | TAG1 | TAG9 | - | TAG2 | TAG10 | TAG6 | TAG6 | TAG14 | TAG2 | TAG10 | TAG6 | TAG6 | TAG14 | TAG2 | TAG10 | - | TAG3 | TAG11 | TAG7 | TAG7 | TAG15 | TAG3 | TAG11 | TAG7 | TAG7 | TAG15 | TAG3 | TAG11 | - | TAG4 | TAG12 | TAG8 | TAG8 | TAG16 | TAG4 | TAG12 | TAG8 | TAG8 | TAG16 | TAG4 | TAG12 | - | TAG5 | TAG1 | TAG1 | TAG9 | TAG17 | TAG5 | TAG1 | TAG1 | TAG9 | TAG17 | TAG5 | TAG13 | - | TAG6 | TAG2 | TAG2 | TAG10 | TAG18 | TAG6 | TAG2 | TAG2 | TAG10 | TAG18 | TAG6 | TAG14 | - | TAG7 | TAG3 | TAG3 | TAG11 | TAG19 | TAG7 | TAG3 | TAG3 | TAG11 | TAG19 | TAG7 | TAG15 | - | TAG8 | TAG4 | TAG4 | TAG12 | TAG20 | TAG8 | TAG4 | TAG4 | TAG12 | TAG20 | TAG8 | TAG16 | @tag_layout @create @barcode-service Scenario: Creating a tag layout of an entire plate using 96 tags by pools @@ -179,47 +139,6 @@ Feature: Access tag layouts through the API | TAG8 | TAG16 | TAG24 | TAG32 | TAG40 | TAG47 | TAG55 | TAG63 | TAG71 | TAG79 | TAG87 | TAG95 | - @tag_layout @create @barcode-service - Scenario: Creating a tag layout of an entire plate using 96 tags by pools with an offset - Given the Baracoda barcode service returns "SQPD-1000001" - Given the Baracoda barcode service returns "SQPD-1000002" - Given the tag group "Example Tag Group" exists - And the UUID for the tag group "Example Tag Group" is "00000000-1111-2222-3333-444444444444" - And the tag group "Example Tag Group" has 30 tags - - Given a "Stock plate" plate called "Testing the API" exists - And the UUID for the plate "Testing the API" is "11111111-2222-3333-4444-000000000002" - And all wells on the plate "Testing the API" have unique samples - - Given a "Stock plate" plate called "Testing the tagging" exists - And the UUID for the plate "Testing the tagging" is "11111111-2222-3333-4444-000000000001" - And the wells for the plate "Testing the API" have been pooled to the plate "Testing the tagging" according to the pooling strategy 12, 8, 20, 12, 8, 20, 16 - - When I make an authorised POST with the following JSON to the API path "/tag_layouts": - """ - { - "tag_layout": { - "plate": "11111111-2222-3333-4444-000000000001", - "user": "99999999-8888-7777-6666-555555555555", - "tag_group": "00000000-1111-2222-3333-444444444444", - "direction": "column", - "walking_by": "manual by pool", - "initial_tag": 10 - } - } - """ - Then the HTTP response should be "201 Created" - - Then the tag layout on the plate "Testing the tagging" should be: - | TAG11 | TAG19 | TAG15 | TAG15 | TAG23 | TAG11 | TAG19 | TAG15 | TAG15 | TAG23 | TAG11 | TAG19 | - | TAG12 | TAG20 | TAG16 | TAG16 | TAG24 | TAG12 | TAG20 | TAG16 | TAG16 | TAG24 | TAG12 | TAG20 | - | TAG13 | TAG21 | TAG17 | TAG17 | TAG25 | TAG13 | TAG21 | TAG17 | TAG17 | TAG25 | TAG13 | TAG21 | - | TAG14 | TAG22 | TAG18 | TAG18 | TAG26 | TAG14 | TAG22 | TAG18 | TAG18 | TAG26 | TAG14 | TAG22 | - | TAG15 | TAG11 | TAG11 | TAG19 | TAG27 | TAG15 | TAG11 | TAG11 | TAG19 | TAG27 | TAG15 | TAG23 | - | TAG16 | TAG12 | TAG12 | TAG20 | TAG28 | TAG16 | TAG12 | TAG12 | TAG20 | TAG28 | TAG16 | TAG24 | - | TAG17 | TAG13 | TAG13 | TAG21 | TAG29 | TAG17 | TAG13 | TAG13 | TAG21 | TAG29 | TAG17 | TAG25 | - | TAG18 | TAG14 | TAG14 | TAG22 | TAG30 | TAG18 | TAG14 | TAG14 | TAG22 | TAG30 | TAG18 | TAG26 | - @tag_layout @create @barcode-service Scenario: Creating a tag layout of an entire plate using 96 tags by pools with an offset Given the Baracoda barcode service returns "SQPD-1000001" diff --git a/features/support/step_definitions/tag_layout_steps.rb b/features/support/step_definitions/tag_layout_steps.rb index 8a829ebedc..d2272fc273 100644 --- a/features/support/step_definitions/tag_layout_steps.rb +++ b/features/support/step_definitions/tag_layout_steps.rb @@ -59,7 +59,6 @@ def check_tag_layout(name, well_range, expected_wells_to_oligos) # rubocop:todo .wells .filter_map do |w| next unless well_range.include?(w) - [w.map.description, w.primary_aliquot.try(:tag).try(:oligo) || ''] end .to_h diff --git a/lib/accession/accession/tag.rb b/lib/accession/accession/tag.rb index 72d48842cd..4339051347 100644 --- a/lib/accession/accession/tag.rb +++ b/lib/accession/accession/tag.rb @@ -72,6 +72,8 @@ module HelperTagValue MISSING_DATA_AGGREEMENT_PRE2023 = 'missing: data agreement established pre-2023' MISSING_ENDANGERED_SPECIES = 'missing: endangered species' MISSING_HUMAN_IDENTIFIABLE = 'missing: human-identifiable' + MISSING_CONTROL_SAMPLE = 'missing: control sample' + MISSING_SAMPLE_GROUP = 'missing: sample group' OTHER_DEFAULT_SETTINGS = [ NOT_COLLECTED, @@ -84,7 +86,9 @@ module HelperTagValue MISING_THIRD_PARTY_DATA, MISSING_DATA_AGGREEMENT_PRE2023, MISSING_ENDANGERED_SPECIES, - MISSING_HUMAN_IDENTIFIABLE + MISSING_HUMAN_IDENTIFIABLE, + MISSING_CONTROL_SAMPLE, + MISSING_SAMPLE_GROUP ].freeze def incorrect_format_value diff --git a/spec/data/submission/scrna_additional_validations_invalid.csv b/spec/data/submission/scrna_additional_validations_invalid_cells_per_chip_well.csv similarity index 96% rename from spec/data/submission/scrna_additional_validations_invalid.csv rename to spec/data/submission/scrna_additional_validations_invalid_cells_per_chip_well.csv index 6af3f0c985..c483aa5a04 100644 --- a/spec/data/submission/scrna_additional_validations_invalid.csv +++ b/spec/data/submission/scrna_additional_validations_invalid_cells_per_chip_well.csv @@ -1,3 +1,3 @@ User Login,template name,study name,project name,submission name,asset names,asset group name,read length,fragment size from,fragment size to,library type,comments,pre-capture plex level,pre-capture group,gigabases expected,scrna core number of samples per pool,scrna core cells per chip well -user,Limber-Htp - scRNA Core cDNA Prep GEM-X 5p,abc123_study,Test project,sub1,,assetgroup123,100,,,Standard,hello there,,,1.35,15, -user,Limber-Htp - scRNA Core cDNA Prep GEM-X 5p,abc123_study,Test project,sub2,,assetgroup123,100,,,Standard,hello there,,,1.35,10, +user,Limber-Htp - scRNA Core cDNA Prep GEM-X 5p,abc123_study,Test project,sub1,,assetgroup123,100,,,Standard,hello there,,,1.35,15,10000 +user,Limber-Htp - scRNA Core cDNA Prep GEM-X 5p,abc123_study,Test project,sub2,,assetgroup123,100,,,Standard,hello there,,,1.35,15,20000 diff --git a/spec/data/submission/scrna_additional_validations_invalid_samples_per_pool.csv b/spec/data/submission/scrna_additional_validations_invalid_samples_per_pool.csv new file mode 100644 index 0000000000..0f2d1911b5 --- /dev/null +++ b/spec/data/submission/scrna_additional_validations_invalid_samples_per_pool.csv @@ -0,0 +1,3 @@ +User Login,template name,study name,project name,submission name,asset names,asset group name,read length,fragment size from,fragment size to,library type,comments,pre-capture plex level,pre-capture group,gigabases expected,scrna core number of samples per pool,scrna core cells per chip well +user,Limber-Htp - scRNA Core cDNA Prep GEM-X 5p,abc123_study,Test project,sub1,,assetgroup123,100,,,Standard,hello there,,,1.35,15,10000 +user,Limber-Htp - scRNA Core cDNA Prep GEM-X 5p,abc123_study,Test project,sub2,,assetgroup123,100,,,Standard,hello there,,,1.35,10,10000 diff --git a/spec/data/submission/scrna_additional_validations_valid.csv b/spec/data/submission/scrna_additional_validations_valid.csv index 46da271f46..abebd6f92c 100644 --- a/spec/data/submission/scrna_additional_validations_valid.csv +++ b/spec/data/submission/scrna_additional_validations_valid.csv @@ -1,3 +1,4 @@ User Login,template name,study name,project name,submission name,asset names,asset group name,read length,fragment size from,fragment size to,library type,comments,pre-capture plex level,pre-capture group,gigabases expected,scrna core number of samples per pool,scrna core cells per chip well -user,Limber-Htp - scRNA Core cDNA Prep GEM-X 5p,abc123_study,Test project,sub1,,assetgroup123,100,,,Standard,hello there,,,1.35,15, -user,Limber-Htp - scRNA Core cDNA Prep GEM-X 5p,abc123_study,Test project,sub2,,assetgroup123,100,,,Standard,hello there,,,1.35,15, +user,Limber-Htp - scRNA Core cDNA Prep GEM-X 5p,abc123_study,Test project,sub1,,assetgroup123,100,,,Standard,hello there,,,1.35,15,10000 +user,Limber-Htp - scRNA Core cDNA Prep GEM-X 5p,abc123_study,Test project,sub2,,assetgroup123,100,,,Standard,hello there,,,1.35,15,10000 +user,Limber-Htp - scRNA Core cDNA Prep GEM-X 5p,abc123_study,Project 1,sub2,,assetgroup2,100,,,Standard,hello there,,,1.35,10,20000 diff --git a/spec/factories/plate_factories.rb b/spec/factories/plate_factories.rb index a700925935..455d2b9a3f 100644 --- a/spec/factories/plate_factories.rb +++ b/spec/factories/plate_factories.rb @@ -242,6 +242,16 @@ transient { well_factory { :untagged_well } } after(:create) do |plate, _evaluator| + custom_metadatum = CustomMetadatum.new + custom_metadatum.key = 'control_placement_type' + custom_metadatum.value = 'random' + custom_metadatum_collection = CustomMetadatumCollection.new + custom_metadatum_collection.custom_metadata = [custom_metadatum] + custom_metadatum_collection.asset = plate + custom_metadatum_collection.user = User.new(id: 1) + custom_metadatum_collection.save! + custom_metadatum.save! + plate.wells.each_with_index do |well, index| next if well.aliquots.empty? diff --git a/spec/features/assets/asset_submission_spec.rb b/spec/features/assets/asset_submission_spec.rb index 637555bc85..ca99f5276e 100644 --- a/spec/features/assets/asset_submission_spec.rb +++ b/spec/features/assets/asset_submission_spec.rb @@ -7,12 +7,126 @@ let(:study) { create(:study) } let(:request_factory) { :sequencing_request } let(:asset) { create(:library_tube) } + let(:flowcell_types) { create_list(:flowcell_type, 2) } let(:request_types) { create_list(:sequencing_request_type, 2) } let(:original_request_type) { request_types.first } let(:selected_request_type) { original_request_type } let(:selected_read_length) { '76' } + let(:request_flowcell_type_validator) do + RequestType::Validator.create( + request_type: selected_request_type, + request_option: 'requested_flowcell_type', + valid_options: flowcell_types.map(&:name) + ) + end let!(:original_request) do - create(request_factory, study: study, project: project, asset: asset, request_type: original_request_type) + create(request_factory, study: study, project: project, asset: asset, request_type: selected_request_type) + end + + describe 'The request form does not set default values' do + let(:user) { create(:admin) } + + before do + login_user user + visit labware_path(asset) + click_link 'Request additional sequencing' + end + + describe 'when the form is loaded' do + it 'does not set request type' do + expect(page).to have_select('Request type', selected: 'Select a request type') + end + end + + describe 'when the user selects a request type' do + before { select 'Request Type 1', from: 'Request type' } + + it 'does not set flowcell type to default value' do + expect(page).to have_select('Flowcell type', selected: 'Select a requested flowcell type') + end + + it 'does not set read length to default value' do + expect(page).to have_select('Read length', selected: 'Select a read length') + end + end + end + + describe 'Validation of Flowcell Type field for request types that require a Flowcell Type' do + let(:user) { create(:admin) } + + # Mock the validator for the selected request type to return the predefined flowcell type validator + # the predefined flowcell type validator require a specific flowcell type to be selected + before do + allow(selected_request_type).to receive(:validator_for).with('requested_flowcell_type').and_return( + request_flowcell_type_validator + ) + login_user user + visit labware_path(asset) + click_link 'Request additional sequencing' + end + + it 'displays an error if Flowcell Type is not set' do + select(selected_request_type.name, from: 'Request type') + select(study.name, from: 'Study') + select(project.name, from: 'Project') + fill_in 'Fragment size required (from)', with: '100' + fill_in 'Fragment size required (to)', with: '200' + select(selected_read_length, from: 'Read length') + click_button 'Create' + + # The JS native validation error 'Please select an item in the list' is being displayed but cannot be inspected. + expect(page).to have_no_text 'Created request' + + redirect_path = + new_request_receptacle_path( + asset.receptacle, + study_id: study.id, + project_id: project.id, + request_type_id: selected_request_type.id + ) + expect(page).to have_current_path(redirect_path) + end + + it 'creates a new request successfully when Flowcell Type is correctly set' do + select(selected_request_type.name, from: 'Request type') + select(study.name, from: 'Study') + select(project.name, from: 'Project') + fill_in 'Fragment size required (from)', with: '100' + fill_in 'Fragment size required (to)', with: '200' + select(selected_read_length, from: 'Read length') + select('Flowcell 1', from: 'Flowcell type') + fill_in 'Fragment size required (from)', with: '100' + fill_in 'Fragment size required (to)', with: '200' + select(selected_read_length, from: 'Read length') + click_button 'Create' + + expect(page).to have_text 'Created request' + end + end + + describe 'Validation of Flowcell Type field for request types that do not require a Flowcell Type' do + let(:user) { create(:admin) } + + before do + login_user user + visit labware_path(asset) + click_link 'Request additional sequencing' + end + + it 'creates a new request successfully even when Flowcell Type is not specified' do + select(selected_request_type.name, from: 'Request type') + select(study.name, from: 'Study') + select(project.name, from: 'Project') + fill_in 'Fragment size required (from)', with: '100' + fill_in 'Fragment size required (to)', with: '200' + select(selected_read_length, from: 'Read length') + fill_in 'Fragment size required (from)', with: '100' + fill_in 'Fragment size required (to)', with: '200' + select(selected_read_length, from: 'Read length') + click_button 'Create' + + expect(page).to have_text 'Created request' + end end shared_examples 'it allows additional sequencing' do diff --git a/spec/models/bulk_submission_spec.rb b/spec/models/bulk_submission_spec.rb index b222344c6c..7dc83a2147 100644 --- a/spec/models/bulk_submission_spec.rb +++ b/spec/models/bulk_submission_spec.rb @@ -20,6 +20,7 @@ let!(:study) { create(:study, name: 'abc123_study') } let!(:asset_group) { create(:asset_group, name: 'assetgroup123', study: study, asset_count: 2) } + let!(:asset_group_2) { create(:asset_group, name: 'assetgroup2', study: study, asset_count: 1) } let!(:library_type) { create(:library_type, name: 'Standard') } before do @@ -277,7 +278,7 @@ end end - context 'when invalid for scRNA template' do + context 'when invalid for scRNA template on samples per pool' do let(:submission_template_hash) do { name: 'Limber-Htp - scRNA Core cDNA Prep GEM-X 5p', @@ -290,15 +291,41 @@ } } end - let(:spreadsheet_filename) { 'scrna_additional_validations_invalid.csv' } + let(:spreadsheet_filename) { 'scrna_additional_validations_invalid_samples_per_pool.csv' } before { SubmissionSerializer.construct!(submission_template_hash) } it 'raises an error and sets an error message' do expect { subject.process }.to raise_error(ActiveRecord::RecordInvalid) expect(subject.errors.messages[:spreadsheet][0]).to eq( - "Inconsistent values for column 'scRNA Core Number of Samples per Pool' for " \ - "Study name 'abc123_study', all rows for a specific study must have the same value" + "Inconsistent values for column 'scrna core number of samples per pool' for Study name 'abc123_study' " \ + "and Project name 'Test project', all rows for a specific study and project must have the same value" + ) + end + end + + context 'when invalid for scRNA template on cells per chip well' do + let(:submission_template_hash) do + { + name: 'Limber-Htp - scRNA Core cDNA Prep GEM-X 5p', + submission_class_name: 'LinearSubmission', + product_catalogue: 'Generic', + submission_parameters: { + request_options: { + }, + request_types: request_types.map(&:key) + } + } + end + let(:spreadsheet_filename) { 'scrna_additional_validations_invalid_cells_per_chip_well.csv' } + + before { SubmissionSerializer.construct!(submission_template_hash) } + + it 'raises an error and sets an error message' do + expect { subject.process }.to raise_error(ActiveRecord::RecordInvalid) + expect(subject.errors.messages[:spreadsheet][0]).to eq( + "Inconsistent values for column 'scrna core cells per chip well' for Study name 'abc123_study' " \ + "and Project name 'Test project', all rows for a specific study and project must have the same value" ) end end diff --git a/spec/models/cherrypick_task/control_locator_spec.rb b/spec/models/cherrypick_task/control_locator_spec.rb index 37cd0645f1..ee13130967 100644 --- a/spec/models/cherrypick_task/control_locator_spec.rb +++ b/spec/models/cherrypick_task/control_locator_spec.rb @@ -3,7 +3,16 @@ require 'rails_helper' RSpec.describe CherrypickTask::ControlLocator do - let(:instance) { described_class.new(batch_id:, total_wells:, num_control_wells:, wells_to_leave_free:) } + let(:instance) do + described_class.new( + batch_id: batch_id, + total_wells: total_wells, + num_control_wells: num_control_wells, + wells_to_leave_free: wells_to_leave_free, + control_source_plate: create(:control_plate), + template: create(:plate_template_with_well) + ) + end shared_examples 'an invalid ControlLocator' do |plate_number, error = 'More controls than free wells'| it 'throws a "More controls than free wells" exception' do @@ -50,6 +59,11 @@ this_plate.each_with_index { |position, index| expect(position).not_to be_within(5).of(previous_plate[index]) } end end + + it 'uses a control plate that is valid' do + placement_type = instance.send(:control_placement_type) + expect(placement_type.nil? || %w[fixed random].exclude?(placement_type)).to be_falsey + end end # Control positions will be our only public method, sand perhaps some attr_readers @@ -126,7 +140,16 @@ let(:range) { (1...1000) } let(:control_positions) do range.map do |batch_id| - described_class.new(batch_id: batch_id, total_wells: 96, num_control_wells: 1).control_positions(0).first + described_class + .new( + batch_id: batch_id, + total_wells: 96, + num_control_wells: 1, + control_source_plate: create(:control_plate), + template: create(:plate_template) + ) + .control_positions(0) + .first end end @@ -145,5 +168,66 @@ expect(tally.values).to all be_between(2, 25) end end + + context 'when the control placement type is not valid' do + let(:batch_id) { 1 } + let(:total_wells) { 96 } + let(:num_control_wells) { 2 } + let(:wells_to_leave_free) { [] } + + before { allow(instance).to receive(:control_placement_type).and_return('invalid_type') } + + it 'raises an error about invalid placement type' do + expect { instance.control_positions(0) }.to raise_error( + StandardError, + 'Control placement type is not set or is invalid' + ) + end + end + + context 'when the control plate and plate template are incompatible' do + let(:batch_id) { 1 } + let(:total_wells) { 96 } + let(:num_control_wells) { 2 } + let(:wells_to_leave_free) { [] } + + before do + allow(instance).to receive_messages( + control_placement_type: 'fixed', + convert_assets: [1, 2, 3], + control_source_plate: create(:control_plate) + ) + end + + it 'returns and displays an error message' do + expect(instance.handle_incompatible_plates).to be_truthy + end + end + + context 'when assets are converted using the maps' do + let(:batch_id) { 1 } + let(:total_wells) { 96 } + let(:num_control_wells) { 2 } + let(:wells_to_leave_free) { [] } + + before { allow(instance).to receive_messages(control_placement_type: 'fixed') } + + it 'they are given the correct position IDs' do + expect(instance.send(:convert_assets, [94, 95, 96])).to eq([79, 87, 95]) + end + end + + context 'when the control placement type is fixed' do + let(:batch_id) { 1 } + let(:total_wells) { 96 } + let(:num_control_wells) { 2 } + let(:wells_to_leave_free) { [] } + + before { allow(instance).to receive_messages(control_placement_type: 'fixed') } + + it 'passes as intended' do + expect(instance.send(:fixed_positions_from_available)).to eq([]) + end + end end end diff --git a/spec/models/state_changer/standard_plate_spec.rb b/spec/models/state_changer/standard_plate_spec.rb index 59e44f42fc..cfac90d0b3 100644 --- a/spec/models/state_changer/standard_plate_spec.rb +++ b/spec/models/state_changer/standard_plate_spec.rb @@ -21,12 +21,25 @@ context 'when the plate is the initial plate in the pipeline' do include_context 'a limber target plate with submissions', 'pending' - let(:target_state) { 'started' } + context 'when the target state is started' do + let(:target_state) { 'started' } - it 'starts the requests', :aggregate_failures do - # Requests are started and we create one event per order. - expect { state_changer.update_labware_state }.to change(BroadcastEvent::LibraryStart, :count).by(1) - expect(library_requests).to all(be_started) + it 'starts the requests', :aggregate_failures do + # Requests are started and we create one event per order. + expect { state_changer.update_labware_state }.to change(BroadcastEvent::LibraryStart, :count).by(1) + expect(library_requests).to all(be_started) + end + end + + # RVI BCL uses the processed_1 state as the first state for the the initial plate + context 'when the target state is processed_1' do + let(:target_state) { 'processed_1' } + + it 'starts the requests', :aggregate_failures do + # Requests are started and we create one event per order. + expect { state_changer.update_labware_state }.to change(BroadcastEvent::LibraryStart, :count).by(1) + expect(library_requests).to all(be_started) + end end end diff --git a/spec/models/tasks/cherrypick_task_spec.rb b/spec/models/tasks/cherrypick_task_spec.rb index 887de3e81f..97e9f98796 100644 --- a/spec/models/tasks/cherrypick_task_spec.rb +++ b/spec/models/tasks/cherrypick_task_spec.rb @@ -36,8 +36,11 @@ def requests_for_plate(plate) batch_id: 1235, total_wells: 6, num_control_wells: 2, - wells_to_leave_free: wells_to_leave_free + wells_to_leave_free: wells_to_leave_free, + control_source_plate: control_plate, + template: template ).and_return(locator) + allow(locator).to receive(:handle_incompatible_plates) end let(:instance) { described_class.new } @@ -89,6 +92,7 @@ def requests_for_plate(plate) locator = instance_double(CherrypickTask::ControlLocator) allow(locator).to receive(:control_positions).and_return([2, 5], [0, 2]) allow(CherrypickTask::ControlLocator).to receive(:new).and_return(locator) + allow(locator).to receive(:handle_incompatible_plates) end it 'places controls in a different position' do @@ -117,6 +121,7 @@ def requests_for_plate(plate) before do locator = instance_double(CherrypickTask::ControlLocator, control_positions: [2, 3]) allow(CherrypickTask::ControlLocator).to receive(:new).and_return(locator) + allow(locator).to receive(:handle_incompatible_plates) end let(:instance) { described_class.new } @@ -145,6 +150,7 @@ def requests_for_plate(plate) locator = instance_double(CherrypickTask::ControlLocator) allow(locator).to receive(:control_positions).and_return([2, 4], [0, 2]) allow(CherrypickTask::ControlLocator).to receive(:new).and_return(locator) + allow(locator).to receive(:handle_incompatible_plates) end let(:instance) { described_class.new } diff --git a/spec/requests/api/v2/bait_library_layouts_spec.rb b/spec/requests/api/v2/bait_library_layouts_spec.rb index 8ff389389a..2da1c7dfd5 100644 --- a/spec/requests/api/v2/bait_library_layouts_spec.rb +++ b/spec/requests/api/v2/bait_library_layouts_spec.rb @@ -308,8 +308,8 @@ def perform_post expect(json.dig('data', 'type')).to eq(resource_type) end - it 'returns a layout as an attribute' do - expect(json.dig('data', 'attributes', 'layout')).not_to be_nil + it 'returns a well_layout as an attribute' do + expect(json.dig('data', 'attributes', 'well_layout')).not_to be_nil end end diff --git a/spec/support/download_helper.rb b/spec/support/download_helper.rb index f9af020af6..c5dc2c49ac 100644 --- a/spec/support/download_helper.rb +++ b/spec/support/download_helper.rb @@ -14,7 +14,6 @@ def self.downloads end def self.downloaded_file(file, timeout: TIMEOUT) - # binding.pry wait_for_download(file, timeout) File.read(path_to(file)) ensure diff --git a/test/unit/messaging/messenger_test.rb b/test/unit/messaging/messenger_test.rb index c9d24317ae..d9ef54396a 100644 --- a/test/unit/messaging/messenger_test.rb +++ b/test/unit/messaging/messenger_test.rb @@ -18,6 +18,11 @@ class MessengerTest < ActiveSupport::TestCase should 'render the json' do assert_equal '{"example":{"example":"hash"},"lims":"SQSCP"}', @messenger.to_json end + + should 'render the json when template is historical (ends in IO)' do + messenger = Messenger.new(target: @target, template: 'FlowcellIO', root: 'example') + assert_equal '{"example":{"example":"hash"},"lims":"SQSCP"}', messenger.to_json + end end should 'provide a routing key' do