Merge pull request #4242 from sanger/develop

Develop into master

.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

.release-version

@@ -1 +1 @@
-14.38.0
+14.39.0

.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/controllers/api/v2/plate_purposes_controller.rb

@@ -2,10 +2,10 @@
 
 module Api
   module V2
-    # Provides a JSON API controller for receptacle
+    # Provides a JSON API controller for plate purposes.
     # See: http://jsonapi-resources.com/ for JSONAPI::Resource documentation
     class PlatePurposesController < JSONAPI::ResourceController
-      # By default JSONAPI::ResourceController provides most the standard
+      # By default JSONAPI::ResourceController provides most of the standard
       # behaviour, and in many cases this file may be left empty.
     end
   end

app/controllers/api/v2/submission_templates_controller.rb

@@ -0,0 +1,12 @@
+# frozen_string_literal: true
+
+module Api
+  module V2
+    # Provides a JSON API controller for submission templates.
+    # See: http://jsonapi-resources.com/ for JSONAPI::Resource documentation.
+    class SubmissionTemplatesController < JSONAPI::ResourceController
+      # By default JSONAPI::ResourceController provides most of the standard
+      # behaviour, and in many cases this file may be left empty.
+    end
+  end
+end

app/controllers/api/v2/transfer_templates_controller.rb

@@ -0,0 +1,12 @@
+# frozen_string_literal: true
+
+module Api
+  module V2
+    # Provides a JSON API controller for transfer templates.
+    # See: http://jsonapi-resources.com/ for JSONAPI::Resource documentation.
+    class TransferTemplatesController < JSONAPI::ResourceController
+      # By default JSONAPI::ResourceController provides most of the standard
+      # behaviour, and in many cases this file may be left empty.
+    end
+  end
+end

app/controllers/api/v2/tube_purposes_controller.rb

@@ -0,0 +1,12 @@
+# frozen_string_literal: true
+
+module Api
+  module V2
+    # Provides a JSON API controller for tube purposes.
+    # See: http://jsonapi-resources.com/ for JSONAPI::Resource documentation
+    class TubePurposesController < JSONAPI::ResourceController
+      # By default JSONAPI::ResourceController provides most of the standard
+      # behaviour, and in many cases this file may be left empty.
+    end
+  end
+end

app/controllers/tag_layout_templates_controller.rb

@@ -6,18 +6,6 @@
 # that they should be laid out column by column across a plate.
 # NB. Not all combinations will be valid.
 class TagLayoutTemplatesController < ApplicationController
-  DIRECTIONS = {
-    'InColumns (A1,B1,C1...)': 'TagLayout::InColumns',
-    'InRows (A1,A2,A3...)': 'TagLayout::InRows',
-    'InInverseColumns (H12,G12,F12...)': 'TagLayout::InInverseColumns',
-    'InInverseRows (H12,H11,H10...)': 'TagLayout::InInverseRows',
-    # These next two directions are used with the 'quadrants' walking by algorithm and the layout
-    # described refers to the 4 wells adjacent to one another, one from each of the 4 quadrants.
-    # The two directions order the tag distribution to the 4 quadrants differently.
-    'InColumnsThenRows (A1,A2,B1,B2...)': 'TagLayout::InColumnsThenRows',
-    'InColumnsThenColumns (A1,B1,A2,B2...)': 'TagLayout::InColumnsThenColumns'
-  }.freeze
-
   authorize_resource
 
   def index
@@ -36,7 +24,8 @@ def show
   # Allows for the passing in of tag group id using a link from the tag group show page.
   def new
     @tag_layout_template = TagLayoutTemplate.new(tag_group_id: params[:tag_group_id])
-    @direction_algorithms = DIRECTIONS
+    @direction_algorithms = TagLayout::DIRECTION_ALGORITHMS
+    @walking_algorithms = TagLayout::WALKING_ALGORITHMS
 
     respond_to { |format| format.html }
   end
@@ -49,7 +38,8 @@ def create
         flash[:notice] = I18n.t('tag_groups.success')
         format.html { redirect_to(@tag_layout_template) }
       else
-        @direction_algorithms = DIRECTIONS
+        @direction_algorithms = TagLayout::DIRECTION_ALGORITHMS
+        @walking_algorithms = TagLayout::WALKING_ALGORITHMS
         format.html { render action: 'new' }
       end
     end

app/frontend/stylesheets/all/sequencescape.scss

@@ -134,6 +134,10 @@ h3.card-header-custom {
   }
 }
 
+.alert-error ul {
+  margin: 0;
+}
+
 .alert-notice,
 .alert-passed {
   @extend .alert-success;

app/helpers/application_helper.rb

@@ -66,12 +66,21 @@ def apple_icon
   end
 
   def render_flashes
-    flash.each do |key, message|
-      concat(alert(key, id: "message_#{key}") { Array(message).each { |m| concat tag.div(m) } })
-    end
+    flash.each { |key, message| concat(alert(key, id: "message_#{key}") { render_message(message) }) }
     nil
   end
 
+  # A helper method for render_flashes - If multiple messages, render them as a list, else render as a single div
+  # @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
+      tag.div(messages.first)
+    end
+  end
+
   def api_data
     { api_version: RELEASE.api_version }
   end

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/models/tag_layout.rb

@@ -15,7 +15,7 @@ class TagLayout < ApplicationRecord
   UnknownDirection = Struct.new(:direction)
   UnknownWalking = Struct.new(:walking_by)
 
-  DIRECTIONS = {
+  DIRECTION_ALGORITHMS = {
     'column' => 'TagLayout::InColumns',
     'row' => 'TagLayout::InRows',
     'inverse column' => 'TagLayout::InInverseColumns',
@@ -50,7 +50,7 @@ class TagLayout < ApplicationRecord
   # The plate we'll be laying out the tags into
   belongs_to :plate, optional: false
 
-  validates :direction, inclusion: { in: DIRECTIONS.keys }
+  validates :direction, inclusion: { in: DIRECTION_ALGORITHMS.keys }
   validates :walking_by, inclusion: { in: WALKING_ALGORITHMS.keys }
 
   validates :direction_algorithm, presence: true
@@ -65,7 +65,7 @@ class TagLayout < ApplicationRecord
   delegate :walking_by, :walk_wells, :apply_tags, to: :walking_algorithm_helper
 
   def direction=(new_direction)
-    self.direction_algorithm = DIRECTIONS.fetch(new_direction) { UnknownDirection.new(new_direction) }
+    self.direction_algorithm = DIRECTION_ALGORITHMS.fetch(new_direction) { UnknownDirection.new(new_direction) }
   end
 
   def walking_by=(walk)

app/models/tube/purpose.rb

@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 # Base class for the all tube purposes, describes the role the associated
-# {Tube} is playing within the lab, and my modify its behaviour.
+# {Tube} is playing within the lab, and modifies its behaviour.
 # This is not an abstract class, and can be used directly.
 # @see Purpose
 class Tube::Purpose < Purpose

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/plate_purpose_resource.rb

@@ -13,34 +13,34 @@ class PlatePurposeResource < BaseResource
 
       # The following attributes are sent by Limber for a new plate purpose.
 
-      # @!attribute name
-      #  @return [String] gets or sets the name of the plate purpose
+      # @!attribute [rw]
+      # @return [String] The name of the plate purpose.
       attribute :name
 
-      # @!attribute stock_plate
-      #  @return [Boolean] gets or sets whether the plates of this purpose are stock plates
+      # @!attribute [rw]
+      # @return [Boolean] Whether the plates of this purpose are stock plates.
       attribute :stock_plate
 
-      # @!attribute cherrypickable_target
-      #  @return [Boolean] gets or sets whether the plates of this purpose are cherrypickable
+      # @!attribute [rw]
+      # @return [Boolean] Whether the plates of this purpose are cherrypickable.
       attribute :cherrypickable_target
 
-      # @!attribute input_plate
-      #  @return [Boolean] gets or sets whether the plates of this purpose are input plates
+      # @!attribute [rw]
+      # @return [Boolean] Whether the plates of this purpose are input plates.
       attribute :input_plate
 
-      # @!attribute size
-      #  @return [Integer] gets or sets the size of the plates of this purpose
+      # @!attribute [rw]
+      # @return [Integer] The size of the plates of this purpose.
       attribute :size
 
-      # @!attribute asset_shape
-      #  @return [String] gets or sets the name of the shape of the plates of this purpose
+      # @!attribute [rw]
+      # @return [String] The name of the shape of the plates of this purpose.
       attribute :asset_shape
 
       # The following attribute is required by Limber to store purposes.
 
-      # @!attribute [r] uuid
-      #  @return [String] gets the UUID of the plate purpose
+      # @!attribute [r]
+      # @return [String] gets the UUID of the plate purpose.
       attribute :uuid
 
       # Sets the asset shape of the plate purpose by name if given.