Merge branch 'develop' into y24-177-scrna-modify-aggregation

.github/workflows/lint_yard_docs.yml

@@ -0,0 +1,18 @@
+name: Lint documentation
+on:
+  - push
+  - pull_request
+
+jobs:
+  yard-junk:
+    runs-on: ubuntu-latest
+    env:
+      BUNDLE_WITHOUT: "cucumber deployment profile development default test"
+    steps:
+      - uses: actions/checkout@v2
+      - name: Set up Ruby
+        uses: ruby/setup-ruby@v1
+        with:
+          bundler-cache: true # runs 'bundle install' and caches installed gems automatically
+      - name: Run yard-junk
+        run: bundle exec yard-junk --sanity

.rubocop_todo.yml

@@ -913,7 +913,6 @@ RSpec/NamedSubject:
     - 'spec/resources/api/v2/receptacle_resource_spec.rb'
     - 'spec/resources/api/v2/request_resource_spec.rb'
     - 'spec/resources/api/v2/request_type_resource_spec.rb'
-    - 'spec/resources/api/v2/sample_metadata_resource_spec.rb'
     - 'spec/resources/api/v2/sample_resource_spec.rb'
     - 'spec/resources/api/v2/study_resource_spec.rb'
     - 'spec/resources/api/v2/submission_resource_spec.rb'

Gemfile

@@ -132,10 +132,6 @@ group :development do
   # Detect n+1 queries
   gem 'bullet'
 
-  # Automatically generate documentation
-  gem 'yard', require: false
-  gem 'yard-activerecord', '~> 0.0.16'
-
   # MiniProfiler allows you to see the speed of a request conveniently on the page.
   # It also shows the SQL queries performed and allows you to profile a specific block of code.
   gem 'rack-mini-profiler'
@@ -158,6 +154,11 @@ group :development, :linting do
   gem 'syntax_tree', require: false
   gem 'syntax_tree-haml', require: false
   gem 'syntax_tree-rbs', require: false
+
+  # Automatically generate documentation
+  gem 'yard', require: false
+  gem 'yard-activerecord', '~> 0.0.16', require: false
+  gem 'yard-junk', '~> 0.0.9', require: false
 end
 
 group :linting, :test do

Gemfile.lock

@@ -115,6 +115,7 @@ GEM
     ast (2.4.2)
     avro (1.11.3)
       multi_json (~> 1.0)
+    backports (3.25.0)
     base64 (0.2.0)
     bigdecimal (3.1.8)
     bootsnap (1.18.3)
@@ -555,6 +556,10 @@ GEM
     yard (0.9.36)
     yard-activerecord (0.0.16)
       yard (>= 0.8.3)
+    yard-junk (0.0.9)
+      backports (>= 3.18)
+      rainbow
+      yard
     zeitwerk (2.6.16)
 
 PLATFORMS
@@ -652,6 +657,7 @@ DEPENDENCIES
   will_paginate-bootstrap
   yard
   yard-activerecord (~> 0.0.16)
+  yard-junk (~> 0.0.9)
 
 BUNDLED WITH
    2.5.9

README.md

@@ -74,6 +74,14 @@ You can then access the Sequencescape documentation through: [http://localhost:8
 
 Yard will also try and document the installed gems: [http://localhost:8808/docs](http://localhost:8808/docs)
 
+### Linting
+
+Yard-Junk is used to check for missing or incorrect documentation. To run the checks:
+
+```shell
+bundle exec yard-junk --sanity
+```
+
 ## Requirements
 
 The following tools are required for development:

app/api/core/service/error_handling.rb

@@ -99,5 +99,10 @@ class IllegalOperation < RuntimeError
   self.api_error_message = 'requested action is not supported on this resource'
 end
 
-Aliquot::TagClash.include Core::Service::Error::Behaviour
-Aliquot::TagClash.api_error_code = 422
+class Aliquot::TagClash
+  include Core::Service::Error::Behaviour
+
+  def self.api_error_code
+    422
+  end
+end

app/helpers/application_helper.rb

@@ -71,9 +71,9 @@ def render_flashes
   end
 
   # A helper method for render_flashes - If multiple messages, render them as a list, else render as a single div
-  # @param key [String] The type of flash message
-  def render_message(message)
-    messages = Array(message)
+  # @param messages [Array<String>, String] The flash message or messages to be rendered
+  def render_message(messages)
+    messages = Array(messages)
     if messages.size > 1
       tag.ul { messages.each { |m| concat tag.li(m) } }
     else

app/jobs/asset_link/builder_job.rb

@@ -2,8 +2,8 @@
 # Enables the bulk creation of the asset links defined by the pairs passed as edges.
 require_dependency 'asset_link'
 
-# An AssetLink::BuilderJob receives an array of [parent_id, child_id] and builds
-# asset links between them
+# An AssetLink::BuilderJob receives an array of [parent_id, child_id] and builds asset links between them
+# @return []
 AssetLink::BuilderJob =
   Struct.new(:links) do
     # For memory reasons we need to limit transaction size to 10 links at a time

app/models/cherrypick_task.rb

@@ -34,11 +34,9 @@ def new_control_locator(batch_id, total_wells, num_control_wells, wells_to_leave
   # Cherrypick tasks are directly coupled to the previous task, due to the awkward
   # way in which the WorkflowsController operates. See issues#2831 for aims to help improve some of this
   #
-  # @param batch [Batch] The batch on which the action will be performed
-  #
   # @return [false,'Can only be accessed via the previous step'>] Array indicating this action can't be linked
   #
-  def can_link_directly?(_batch)
+  def can_link_directly?
     [false, 'Can only be accessed via the previous step']
   end
 

app/models/illumina_htp/mx_tube_purpose.rb

@@ -20,7 +20,7 @@ class IlluminaHtp::MxTubePurpose < Tube::Purpose
   # limber pipelines this will actually return the plate on which you charge and pass.
   # See https://github.com/sanger/sequencescape/issues/3040 for more information
   #
-  # @deprecate Do not use this for new behaviour.
+  # @deprecated Do not use this for new behaviour.
   #
   # @param tube [Tube] The tube for which to find the stock_plate
   #

app/models/illumina_htp/stock_tube_purpose.rb

@@ -28,7 +28,7 @@ def create_with_request_options(_tube)
   # with that in plate, and deprecate it.
   # See https://github.com/sanger/sequencescape/issues/3040 for more information
   #
-  # @deprecate Do not use this for new behaviour.
+  # @deprecated Do not use this for new behaviour.
   #
   # @param tube [Tube] The tube for which to find the stock_plate
   #

app/models/plate.rb

@@ -326,7 +326,7 @@ def stock_plate?
   # JG: 2021-02-11:
   # See https://github.com/sanger/sequencescape/issues/3040 for more information
   #
-  # @deprecate Do not use this for new behaviour.
+  # @deprecated Do not use this for new behaviour.
   #
   #
   # @return [Plate, nil] The stock plate if found

app/models/plate/quad_creator.rb

@@ -125,7 +125,7 @@ def target_coordinate_for(source_coordinate_name, quadrant_index)
     #
     # Converts a well or tube location name to its co-ordinates
     #
-    # @param [<String>] Location name of the well or tube. Eg. A3
+    # @param [<String>] locn_name name of the well or tube. Eg. A3
     #
     # @return [Array<Integer>] An array of two integers indicating column and row. eg. [0, 2]
     #

app/resources/api/v2/asset_audit_resource.rb

@@ -33,7 +33,7 @@ class AssetAuditResource < BaseResource
 
       # Sets the Asset on the model using the UUID provided in the API create/update request.
       #
-      # @param name [String] the uuid of the associated asset.
+      # @param uuid [String] the uuid of the associated asset.
       # @return [void]
       def asset_uuid=(uuid)
         @model.asset = Uuid.with_external_id(uuid).include_resource.map(&:resource).first

app/resources/api/v2/sample_metadata_resource.rb

@@ -4,20 +4,54 @@ module Api
   module V2
     # SampleMetadataResource
     class SampleMetadataResource < BaseResource
-      attribute :sample_common_name
-      attribute :supplier_name
-      attribute :collected_by
+      # Set add_model_hint true to allow updates from Limber, otherwise get a
+      # 500 error as it looks for resource Api::V2::MetadatumResource
+      model_name 'Sample::Metadata', add_model_hint: true
+
+      ###
+      # Attributes
+      ###
+
+      # @!attribute [rw]
+      # @return [String] The sample cohort.
       attribute :cohort
-      attribute :sample_description
-      attribute :donor_id
+
+      # @!attribute [rw]
+      # @return [String] The name of the body collecting the sample.
+      attribute :collected_by
+
+      # @!attribute [rw]
+      # @return [String] The sample concentration.
       attribute :concentration
-      attribute :volume
 
-      # set add_model_hint true to allow updates from Limber, otherwise get a
-      # 500 error as it looks for resource Api::V2::MetadatumResource
-      model_name 'Sample::Metadata', add_model_hint: true
+      # @!attribute [rw]
+      # @return [String] The ID of the sample donor.
+      attribute :donor_id
+
+      # @!attribute [rw]
+      # @return [String] The gender of the organism providing the sample.
+      attribute :gender
 
+      # @!attribute [rw]
+      # @return [String] The common name for the sample.
+      attribute :sample_common_name
+
+      # @!attribute [rw]
+      # @return [String] A description of the sample.
+      attribute :sample_description
+
+      # @!attribute [rw]
+      # @return [String] The supplier name for the sample.
+      attribute :supplier_name
+
+      # @!attribute [rw]
+      # @return [String] The volume of the sample.
+      attribute :volume
+
+      ###
       # Filters
+      ###
+
       filter :sample_id
     end
   end

app/resources/api/v2/volume_update_resource.rb

@@ -21,7 +21,7 @@ class VolumeUpdateResource < BaseResource
 
       # Sets the target Labware on the model using the UUID provided in the API create/update request.
       #
-      # @param name [String] the uuid of the associated target Labware.
+      # @param uuid [String] the uuid of the associated target Labware.
       # @return [void]
       def target_uuid=(uuid)
         @model.target = Uuid.with_external_id(uuid).include_resource.map(&:resource).first

app/sequencescape_excel/sequencescape_excel/validation.rb

@@ -68,7 +68,7 @@ def inspect
     ##
     # formula1 is defined within the options, however it needs to be updated
     # with:
-    # 1) A provided rage in the case of lists
+    # 1) A provided range in the case of lists
     # 2) Proper cell names in the case of custom formulas
     # 3) AXLSX doesn't escape text fields for us, so we do that ourselves
     def formula1

config/sample_manifest_excel/columns.yml

@@ -731,6 +731,26 @@ sample_description_specimen_plate_barcode:
         operator: ">"
         operand: 20
     is_number:
+sample_description_specimen_plate_barcode_mandatory:
+  heading: SAMPLE DESCRIPTION
+  updates: sample_description
+  unlocked: true
+  validation:
+    options:
+      type: :custom
+      operator: :between
+      formula1: "=AND(LEN(A1)>=7,LEN(A1)<=11)"
+      allowBlank: false
+      showInputMessage: true
+      promptTitle: "Sample Description - Specimen Plate Barcode"
+      prompt: "Please enter the specimen plate barcode (NB. should be the same for all wells on the input plate, and between 7 and 11 characters in length)."
+      showErrorMessage: true
+      errorStyle: :stop
+      errorTitle: "Sample Description - Specimen Plate Barcode"
+      error: "The barcode length must be between 7 and 11 characters in length."
+  conditional_formattings:
+    empty_mandatory_cell:
+    is_number:
 sample_strain_att:
   heading: STRAIN
   unlocked: true

config/sample_manifest_excel/manifest_types.yml

@@ -273,7 +273,7 @@ plate_anospp:
     - :date_of_sample_collection
     - :sample_taxon_id
     - :sample_common_name
-    - :sample_description_specimen_plate_barcode
+    - :sample_description_specimen_plate_barcode_mandatory
     - :sample_type
     - :donor_id
     - :control_type

lib/accession/accession/configuration.rb

@@ -4,6 +4,8 @@ class Configuration
     include Accession::Helpers
     include Accession::Equality
 
+    # This constant defines a list of tags for loading
+    # @return [Array<Symbol>] a list of symbols
     FILES = [:tags].freeze
 
     attr_accessor :folder, *FILES

lib/nested_validation.rb

@@ -38,7 +38,7 @@ def validate_each(record, attribute, value)
   #
   # Records of this class will call valid? on any associations provided
   # as attr_names. Errors on these records will be propagated out
-  # @param *attr_names [Symbol] One or more associations to validate
+  # @param attr_names [Symbol] One or more associations to validate
   #
   # @return [NestedValidator]
   def validates_nested(*attr_names)

spec/data/sample_manifest_excel/columns.yml

@@ -686,6 +686,26 @@ sample_description_specimen_plate_barcode:
         operator: ">"
         operand: 20
     is_number:
+sample_description_specimen_plate_barcode_mandatory:
+  heading: SAMPLE DESCRIPTION
+  updates: sample_description
+  unlocked: true
+  validation:
+    options:
+      type: :custom
+      operator: :between
+      formula1: "=AND(LEN(A1)>=7,LEN(A1)<=11)"
+      allowBlank: false
+      showInputMessage: true
+      promptTitle: "Sample Description - Specimen Plate Barcode"
+      prompt: "Please enter the specimen plate barcode (NB. should be the same for all wells on the input plate, and between 7 and 11 characters in length)."
+      showErrorMessage: true
+      errorStyle: :stop
+      errorTitle: "Sample Description - Specimen Plate Barcode"
+      error: "The barcode length must be between 7 and 11 characters in length."
+  conditional_formattings:
+    empty_mandatory_cell:
+    is_number:
 sample_strain_att:
   heading: STRAIN
   unlocked: true

spec/data/sample_manifest_excel/manifest_types.yml

@@ -206,7 +206,7 @@ plate_anospp:
     - :date_of_sample_collection
     - :sample_taxon_id
     - :sample_common_name
-    - :sample_description_specimen_plate_barcode
+    - :sample_description_specimen_plate_barcode_mandatory
     - :sample_type
     - :donor_id
     - :control_type

spec/factories/sample_metadata.rb

@@ -12,6 +12,7 @@
       cohort { 'cohort' }
       country_of_origin { 'country_of_origin' }
       geographical_region { 'geographical_region' }
+      gender { :male }
       ethnicity { 'ethnicity' }
       volume { 'volume' }
       mother { 'mother' }

spec/models/transfer_request_spec.rb

@@ -232,7 +232,9 @@
         let(:merge) { false }
 
         it 'will throw a TagClash exception' do
-          expect { transfer_request.save }.to raise_error(Aliquot::TagClash)
+          expect { transfer_request.save }.to raise_error(Aliquot::TagClash) do |error|
+            expect(error.api_error_code).to eq(422)
+          end
         end
       end
     end