Merge pull request #539 from sanger/develop

Release Gpl-674 - prevent merger pcr plates into pool with different barcode sets

Gemfile.lock

@@ -293,10 +293,6 @@ GEM
       rack (>= 1.1)
       rubocop (>= 0.87.0)
     ruby-progressbar (1.10.1)
-    ruby_jard (0.3.0)
-      byebug (>= 9.1, < 12.0)
-      pry (~> 0.13.0)
-      tty-screen (~> 0.8.1)
     rubyzip (2.3.0)
     sass-rails (6.0.0)
       sassc-rails (~> 2.1, >= 2.1.1)
@@ -346,7 +342,6 @@ GEM
       launchy (~> 2.1)
       pusher-client (~> 0.4)
       typhoeus (~> 0.6, >= 0.6.8)
-    tty-screen (0.8.1)
     typhoeus (0.8.0)
       ethon (>= 0.8.0)
     tzinfo (1.2.7)
@@ -412,7 +407,6 @@ DEPENDENCIES
   rubocop
   rubocop-performance
   rubocop-rails
-  ruby_jard
   sanger_barcode_format!
   sass-rails
   select2-rails

README.md

@@ -16,7 +16,7 @@ In addition to the [externally hosted YARD docs](https://www.rubydoc.info/github
 yard server -r --gems -m limber
 ```
 
-You can then access the Sequencescape documentation through: http://localhost:8808/docs/limber
+You can then access the Limber documentation through: http://localhost:8808/docs/limber
 Yard will also try and document the installed gems: http://localhost:8808/docs
 
 ## Configuring pipelines

app/assets/javascripts/lib/validator.js

@@ -1,9 +1,14 @@
+// accepts a `validation` function which returns true (valid) or false (invalid)
+// and a `message` string for when it's invalid
 var validator = function(validation, message) {
   this.validation = validation;
   this.message = message;
 };
 
 validator.prototype = {
+
+  // returns an object with `valid` boolean
+  // and a string message
   validate: function(target) {
     if (this.validation(target)) {
       return { valid: true, message: 'is valid' };

app/assets/javascripts/tag_by_tag_plate.js

@@ -7,39 +7,47 @@
   // TAG CREATION
   $(document).ready(function(){
     if ($('#tag-creation-page').length === 0) { return; }
-    var qcLookup;
+    var qcableLookup;
 
     //= require lib/ajax_support
 
     // Set up some null objects
     var unknownTemplate = { unknown: true, dual_index: false };
     var unkownQcable = { template_uuid: 'not-loaded' };
 
-    qcLookup = function(barcodeBox, collector) {
+    qcableLookup = function(barcodeBox, collector) {
       if (barcodeBox.length === 0) { return false; }
+
       var qc_lookup = this, status;
       this.inputBox = barcodeBox;
+
+      // the `data` attribute is set when declaring the element in tagged_plate,html.erb
       this.infoPanel = $('#'+barcodeBox.data('info-panel'));
       this.dualIndex = barcodeBox.data('dual-index');
       this.approvedTypes = SCAPE[barcodeBox.data('approved-list')];
       this.required = this.inputBox[0].required;
+
+      // add an onchange event to the box, to look up the plate
       this.inputBox.on('change', function(){
         qc_lookup.resetStatus();
         qc_lookup.requestPlate(this.value);
       });
       this.monitor = collector.register(!this.required, this);
+
+      // set initial values for the tag plate data
       this.qcable = unkownQcable;
       this.template = unknownTemplate;
     };
 
-    qcLookup.prototype = {
+    qcableLookup.prototype = {
       resetStatus: function() {
         this.monitor.fail();
         this.infoPanel.find('dd').text('');
         this.infoPanel.find('input').val(null);
       },
       requestPlate: function(barcode) {
         if ( this.inputBox.val()==="" && !this.required ) { return this.monitor.pass();}
+        // find the qcable (tag plate) based on the barcode scanned in by the user
         $.ajax({
           type: 'POST',
           dataType: "json",
@@ -53,6 +61,7 @@
           if (response.error) {
             qc_lookup.message(response.error,'danger');
           } else if (response.qcable) {
+            // if response is as expected, load some data
             qc_lookup.plateFound(response.qcable);
           } else {
             qc_lookup.message('An unexpected response was received. Please contact support.','danger');
@@ -66,17 +75,34 @@
         };
       },
       validators: [
+        // `t` is a qcableLookup object
+        // The data for t.template comes from app/models/labware_creators/tagging/tag_collection.rb
+        // t.template.dual_index is true if the scanned tag plate contains both i5 and i7 tags together in its wells (is a UDI plate)
+        // t.dualIndex is true if there are multiple source plates from the submission, which will be pooled...
+        // ... and therefore an i5 tag (tag 2) is needed (from either a tube or UDI plate)
         new validator(function(t) { return t.qcable.state == 'available'; }, 'The scanned item is not available.'),
         new validator(function(t) { return !t.template.unknown; }, 'It is an unrecognised template.'),
         new validator(function(t) { return t.template.approved; }, 'It is not approved for use with this pipeline.'),
         new validator(function(t) { return !(t.dualIndex && t.template.used && t.template.dual_index); }, 'This template has already been used.'),
         new validator(function(t) { return !(t.dualIndex && !t.template.dual_index); }, 'Pool has been tagged with a UDI plate. UDI plates must be used.'),
-        new validator(function(t) { return !(t.dualIndex == false && t.template.dual_index); }, 'Pool has been tagged with tube. Dual indexed plates are unsupported.')
+        new validator(function(t) { return !(t.dualIndex == false && t.template.dual_index); }, 'Pool has been tagged with tube. Dual indexed plates are unsupported.'),
+        new validator(
+          function(t) { return (SCAPE.enforceSameTemplateWithinPool ? t.template.matches_templates_in_pool : true) },
+          'It doesn\'t match those already used for other plates in this submission pool.'
+        )
       ],
+      //
+      // The major function that runs when a tag plate is scanned into the box
+      // Loads tag plate data into `qcable` and `template`
+      // Adds visible information to the information panel
+      // Validates whether the tag plate is suitable
+      // Updates the plate diagram with tag numbers
+      //
       plateFound: function(qcable) {
         this.qcable = qcable;
         this.template = this.approvedTypes[qcable.template_uuid] || unknownTemplate;
         this.populateData();
+
         if (this.validPlate()) {
           this.message('The ' + qcable.qcable_type + ' is suitable.', 'success');
           SCAPE.update_layout();
@@ -87,13 +113,15 @@
         }
       },
       populateData: function() {
+        // add information retrieved about the scanned tag plate to the information panel for the user to see
         this.infoPanel.find('dd.lot-number').text(this.qcable.lot_number);
         this.infoPanel.find('dd.template').text(this.qcable.tag_layout);
         this.infoPanel.find('dd.state').text(this.qcable.state);
         this.infoPanel.find('.asset_uuid').val(this.qcable.asset_uuid);
         this.infoPanel.find('.template_uuid').val(this.qcable.template_uuid);
       },
       validPlate: function() {
+        // run through the `validators`, and collect any errors
         this.errors = '';
         for (var i =0; i < this.validators.length; i+=1) {
           var response = this.validators[i].validate(this);
@@ -127,8 +155,8 @@
       }
     );
 
-    new qcLookup($('#plate_tag_plate_barcode'), qcCollector);
-    new qcLookup($('#plate_tag2_tube_barcode'), qcCollector);
+    new qcableLookup($('#plate_tag_plate_barcode'), qcCollector);
+    new qcableLookup($('#plate_tag2_tube_barcode'), qcCollector);
 
     /* Disables form submit (eg. by enter) if the button is disabled. Seems safari doesn't do this by default */
     $('form#plate_new').on('submit', function(){ return !$('input#plate_submit')[0].disabled; } );

app/models/labware_creators/tagged_plate.rb

@@ -1,5 +1,6 @@
 # frozen_string_literal: true
 
+# rubocop:todo Metrics/ClassLength
 module LabwareCreators
   # Handles transfer of material into a pre-existing tag plate, created via
   # Gatekeeper. It performs a few actions:
@@ -26,6 +27,10 @@ class TaggedPlate < Base
     validates :tag2_tube_barcode, :tag2_tube, presence: { if: :tag_tubes_used? }
 
     delegate :size, :number_of_columns, :number_of_rows, to: :labware
+
+    # If I call `tag_plates_used?`, it calls `tag_plates.used?`
+    # where `tag_plates` is a method in this class, returning an instance of TagCollection
+    # similar for `list` and `names`
     delegate :used?, :list, :names, to: :tag_plates, prefix: true
     delegate :used?, :list, :names, to: :tag_tubes, prefix: true
 
@@ -78,7 +83,7 @@ def requires_tag2?
     # - If we don't need a tag2, allow anything, it doesn't matter.
     # - If we've already started using one method, enforce it for the rest of the pool
     # - Otherwise, anything goes
-    # Note: The order matter here, as pools tagged with tubes will still list plates
+    # Note: The order matters here, as pools tagged with tubes will still list plates
     # for the i5 (tag) tag.
     #
     # @return [Array<String>] An array of acceptable sources, 'plate' and/or 'tube'
@@ -104,7 +109,7 @@ def allow_tag_tube?
     # Forbidden if part of a pool using tubes
     # Permitted, but not required in all other cases
     #
-    # @return [<Boolean] false: UDI plates are forbidden
+    # @return [Boolean] false: UDI plates are forbidden
     #                    true: UDI plates are required
     #                    nil: UDI plates are permitted, but not required
     #
@@ -123,6 +128,10 @@ def pool_index(_pool_index)
       nil
     end
 
+    def enforce_same_template_within_pool?
+      purpose_config.fetch(:enforce_same_template_within_pool, false)
+    end
+
     private
 
     def transfer_hash
@@ -165,3 +174,4 @@ def create_labware! # rubocop:todo Metrics/AbcSize
     # rubocop:enable Metrics/MethodLength
   end
 end
+# rubocop:enable Metrics/ClassLength

app/models/labware_creators/tagging/tag_collection.rb

@@ -18,23 +18,31 @@ def initialize(api, plate, purpose_uuid)
     #
     # Returns hash of usable tag layout templates, and the tags assigned to
     # each well:
-    # eg. { "tag-layout-template-0"=>{tags: [["A1", [1, 1]], ["B1", [1, 2]]], dual_index: true } }
+    # eg. { "tag-layout-template-0" => { tags: [["A1", [1, 1]], ["B1", [1, 2]]], dual_index: true } }
     # where { tag_template_uuid => { tags: [[well_name, [ pool_id, tag_id ]]], dual_index: dual_index? } }
     # @return [Hash] Tag layouts and their tags
     #
     def list
       @list ||= tag_layout_templates.each_with_object({}) do |layout, hash|
+        # the `throw` that this catches comes from `generate_tag_layout` method
         catch(:unacceptable_tag_layout) do
-          hash[layout.uuid] = {
-            tags: tags_by_column(layout),
-            dual_index: layout.dual_index?,
-            used: used.include?(layout.uuid),
-            approved: acceptable_template?(layout)
-          }
+          hash[layout.uuid] = layout_hash(layout)
         end
       end
     end
 
+    def layout_hash(layout)
+      {
+        tags: tags_by_column(layout),
+        dual_index: layout.dual_index?,
+        used: used.include?(layout.uuid),
+        matches_templates_in_pool: matches_templates_in_pool(layout.uuid),
+        approved: acceptable_template?(layout)
+      }
+    end
+
+    # Returns a list of the tag layout templates (their uuids) that have already been used on
+    # other plates in the relevant submission pools
     def used
       return [] if @plate.submission_pools.empty?
 
@@ -43,10 +51,29 @@ def used
       end
     end
 
+    # Have any tag layout templates already been used on other plates in the relevant submission pools?
     def used?
       used.present?
     end
 
+    #
+    # Used where the wells being pooled together originate from the same sample,
+    # so should have the same tags, so they are kept together when analysing sequencing data.
+    # (As opposed to when the pool will contain multiple samples, and therefore need to have different tags)
+    #
+    # @param [string] uuid - the uuid of the Tag Layout Template we are currently dealing with
+    #
+    # @return [Bool] true if either no other templates have been used in the submission pool, or
+    #                     if all the templates used are the same as this one
+    #
+    def matches_templates_in_pool(uuid)
+      # if there haven't been any templates used yet in the pool, we say it matches them
+      return true if used.empty?
+
+      # return true if this template has been used already in the pool
+      used.include?(uuid)
+    end
+
     private
 
     #

app/views/plate_creation/tagged_plate.html.erb

@@ -1,13 +1,15 @@
 <%= page(:'tag-creation-page', prevent_row: true) do -%>
   <script type="text/javascript" charset="utf-8">
     (function(exports, $, undefined){
-       $.extend(SCAPE, {
+      $.extend(SCAPE, {
         tag_plates_list: <%= @labware_creator.tag_plates_list.to_json.html_safe %>,
         tag_tubes_list: <%= @labware_creator.tag_tubes_list.to_json.html_safe %>,
-        dualRequired: <%= @labware_creator.requires_tag2? %>
-       });
+        dualRequired: <%= @labware_creator.requires_tag2? %>,
+        enforceSameTemplateWithinPool: <%= @labware_creator.enforce_same_template_within_pool? %>
+      });
      })(window,jQuery);
   </script>
+
   <% form_for(@labware_creator, as: :plate, url: limber_plate_children_path(@labware_creator.parent), html: { class: 'row' }) do |f| %>
     <%= content do %>
       <%= card without_block: true, id: 'main-content' do %>

config/purposes/heron_lthr_96.yml

@@ -19,6 +19,7 @@ LTHR Lib PCR 1:
   - TS_pWGSC_UDI96
   - TS_pWGSD_UDI96
   - TS_pWGSA_UDI96v2
+  :enforce_same_template_within_pool: true
 LTHR Lib PCR 2:
   :asset_type: plate
   :presenter_class: Presenters::MinimalPcrPlatePresenter
@@ -29,6 +30,7 @@ LTHR Lib PCR 2:
   - TS_pWGSC_UDI96
   - TS_pWGSD_UDI96
   - TS_pWGSA_UDI96v2
+  :enforce_same_template_within_pool: true
 LTHR Lib PCR pool:
   :asset_type: plate
   :creator_class: LabwareCreators::MergedPlate

docs/purposes_yaml_files.md

@@ -14,7 +14,7 @@ Limber automatically loads all `.yml` files within this directory into the
 refactored to use a PurposeConfig object in future, to bring it more in line
 with the {Pipeline} behaviour.
 
-In addition, limber will also register purposes in Sequencescape upon running
+In addition, Limber will also register purposes in Sequencescape upon running
 `rake config:generate`. This process is idempotent (ie. will only register
 each purpose once), although is subject to race conditions if run concurrently.
 `rake config:generate` is run automatically on deployment, and is run in series
@@ -27,9 +27,8 @@ Loading of yaml files is handled by {ConfigLoader::PurposesLoader} which
 loads all files and detects potential duplicates.
 
 > **TIP**
-> It is suggested that where you create a purposes.yml to match the
-> corresponding pipeline. However, purposes can be shared between different
-> pipelines.
+> It is suggested that when you create a new pipeline, you create a purposes.yml to match.
+> However, purposes can be shared between different pipelines.
 
 ## An example file
 
@@ -499,6 +498,22 @@ all layout templates are approved.
 - 'TS_RNAhWGS_UDI_96'
 ```
 
+#### :enforce_same_template_within_pool
+
+Boolean, specifies whether tagged plates which will end up being pooled together
+should use the same tag layout template as each other.
+For instance, they should use different tags if they are different samples
+and will therefore need to be 'de-plexed' during data analysis, but should
+use the same tags if they originate from the same plate and therefore
+contain the same samples.
+
+Used by {LabwareCreators::TaggedPlate}. The default behaviour is as if this setting
+is set to false, whether or not it exists.
+
+```yaml
+:enforce_same_template_within_pool: true
+```
+
 #### :merged_plate
 
 Hash, specifying:

spec/factories/tag_layout_template_factories.rb

@@ -65,7 +65,7 @@
     end
   end
 
-  # API V1 index of tag lauout templates
+  # API V1 index of tag layout templates
   factory :tag_layout_template_collection, class: Sequencescape::Api::PageOfResults, traits: [:api_object] do
     size { 2 }