From a183f721e88987f158bfe538368611402ab655b5 Mon Sep 17 00:00:00 2001 From: Stuart McHattie Date: Wed, 24 Jul 2024 14:36:27 +0100 Subject: [PATCH 01/15] Populate Sample Metadata with YARD docs --- .../api/v2/sample_metadata_resource.rb | 48 +++++++++++++++---- 1 file changed, 39 insertions(+), 9 deletions(-) diff --git a/app/resources/api/v2/sample_metadata_resource.rb b/app/resources/api/v2/sample_metadata_resource.rb index 8c55c63897..cbcbe0f87b 100644 --- a/app/resources/api/v2/sample_metadata_resource.rb +++ b/app/resources/api/v2/sample_metadata_resource.rb @@ -4,20 +4,50 @@ 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 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 From 99f792e899aa5b8b14c4ba5cd76949b709eb1b1a Mon Sep 17 00:00:00 2001 From: Stuart McHattie Date: Wed, 24 Jul 2024 14:36:42 +0100 Subject: [PATCH 02/15] Add the gender attribute to the Sample Metadata --- app/resources/api/v2/sample_metadata_resource.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/resources/api/v2/sample_metadata_resource.rb b/app/resources/api/v2/sample_metadata_resource.rb index cbcbe0f87b..f96d48393e 100644 --- a/app/resources/api/v2/sample_metadata_resource.rb +++ b/app/resources/api/v2/sample_metadata_resource.rb @@ -28,6 +28,10 @@ class SampleMetadataResource < BaseResource # @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 From 1b77c9f3bb7f005900cb4ea62ea2f4adf7035d69 Mon Sep 17 00:00:00 2001 From: Stuart McHattie Date: Wed, 24 Jul 2024 14:36:52 +0100 Subject: [PATCH 03/15] Update tests for the resource class --- .../api/v2/sample_metadata_resource_spec.rb | 36 +++++++++++++------ 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/spec/resources/api/v2/sample_metadata_resource_spec.rb b/spec/resources/api/v2/sample_metadata_resource_spec.rb index 7781cef4c1..73431baef2 100644 --- a/spec/resources/api/v2/sample_metadata_resource_spec.rb +++ b/spec/resources/api/v2/sample_metadata_resource_spec.rb @@ -4,18 +4,32 @@ require './app/resources/api/v2/sample_metadata_resource' RSpec.describe Api::V2::SampleMetadataResource, type: :resource do - describe 'it works' do - subject { described_class.new(sample_metadata, {}) } + subject(:resource) { described_class.new(sample_metadata, {}) } - let(:sample_metadata) { create(:sample_metadata) } + let(:sample_metadata) { create(:sample_metadata) } - it 'has the expected attributes' do - expect(subject).to have_attribute :sample_common_name - expect(subject).to have_attribute :supplier_name - expect(subject).to have_attribute :collected_by - expect(subject).to have_attribute :donor_id - expect(subject).to have_attribute :concentration - expect(subject).to have_attribute :volume - end + # Test attributes + it 'has the expected attributes', :aggregate_failures do + expect(resource).to have_attribute :sample_common_name + expect(resource).to have_attribute :supplier_name + expect(resource).to have_attribute :collected_by + expect(resource).to have_attribute :donor_id + expect(resource).to have_attribute :concentration + expect(resource).to have_attribute :volume end + + # Updatable fields + it 'allows updating of read-write fields', :aggregate_failures do + expect(resource).to have_updatable_field :sample_common_name + expect(resource).to have_updatable_field :supplier_name + expect(resource).to have_updatable_field :collected_by + expect(resource).to have_updatable_field :donor_id + expect(resource).to have_updatable_field :concentration + expect(resource).to have_updatable_field :volume + end + + # Non-updatable fields -- uncomment to use + # it 'disallows updating of read only fields', :aggregate_failures do + # expect(resource).not_to have_updatable_field :uuid + # end end From 051117ba10b91255dff3d5dd18d417311e8c49d8 Mon Sep 17 00:00:00 2001 From: Stuart McHattie Date: Wed, 24 Jul 2024 15:06:31 +0100 Subject: [PATCH 04/15] Add a request unit test for Sample Metadata --- spec/factories/sample_metadata.rb | 1 + spec/requests/api/v2/sample_metadata_spec.rb | 199 +++++++++++++++++++ 2 files changed, 200 insertions(+) create mode 100644 spec/requests/api/v2/sample_metadata_spec.rb diff --git a/spec/factories/sample_metadata.rb b/spec/factories/sample_metadata.rb index c1d7d32217..824baf75ae 100644 --- a/spec/factories/sample_metadata.rb +++ b/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' } diff --git a/spec/requests/api/v2/sample_metadata_spec.rb b/spec/requests/api/v2/sample_metadata_spec.rb new file mode 100644 index 0000000000..55251c6b5b --- /dev/null +++ b/spec/requests/api/v2/sample_metadata_spec.rb @@ -0,0 +1,199 @@ +# frozen_string_literal: true + +require 'rails_helper' +require './spec/requests/api/v2/shared_examples/api_key_authenticatable' + +describe 'SampleMetadata API', with: :api_v2 do + let(:base_endpoint) { '/api/v2/sample_metadata' } + + it_behaves_like 'ApiKeyAuthenticatable' + + context 'with multiple metadata resources' do + before { create_list(:sample_metadata_for_api, 5) } + + it 'gets a list of metadata resources' do + api_get base_endpoint + + expect(response).to have_http_status(:success) + expect(json['data'].length).to eq(5) + end + end + + context 'with a single metadata resource' do + let(:resource_model) { create :sample_metadata_for_api } + + describe '#get' do + it 'generates a success response' do + api_get "#{base_endpoint}/#{resource_model.id}" + + expect(response).to have_http_status(:success) + end + + it 'returns the expected JSON' do + api_get "#{base_endpoint}/#{resource_model.id}" + + expect(json.dig('data', 'type')).to eq('sample_metadata') + expect(json.dig('data', 'attributes', 'cohort')).to eq resource_model.cohort + expect(json.dig('data', 'attributes', 'collected_by')).to eq resource_model.collected_by + expect(json.dig('data', 'attributes', 'concentration')).to eq resource_model.concentration + expect(json.dig('data', 'attributes', 'donor_id')).to eq resource_model.donor_id + expect(json.dig('data', 'attributes', 'gender')).to eq resource_model.gender + expect(json.dig('data', 'attributes', 'sample_common_name')).to eq resource_model.sample_common_name + expect(json.dig('data', 'attributes', 'sample_description')).to eq resource_model.sample_description + expect(json.dig('data', 'attributes', 'supplier_name')).to eq resource_model.supplier_name + expect(json.dig('data', 'attributes', 'volume')).to eq resource_model.volume + end + end + + describe '#patch' do + context 'with a valid payload' do + let(:payload) do + { + 'data' => { + 'id' => resource_model.id, + 'type' => 'sample_metadata', + 'attributes' => { + cohort: 'updated cohort', + collected_by: 'updated collected_by', + concentration: 'updated concentration', + donor_id: 'updated donor_id', + gender: 'female', + sample_common_name: 'updated sample_common_name', + sample_description: 'updated sample_description', + supplier_name: 'updated supplier_name', + volume: 'updated volume' + } + } + } + end + + it 'gives a success response' do + api_patch "#{base_endpoint}/#{resource_model.id}", payload + + expect(response).to have_http_status(:success) + end + + it 'responds with the expected attributes' do + api_patch "#{base_endpoint}/#{resource_model.id}", payload + + expect(response).to have_http_status(:success) + expect(json.dig('data', 'attributes', 'cohort')).to eq 'updated cohort' + expect(json.dig('data', 'attributes', 'collected_by')).to eq 'updated collected_by' + expect(json.dig('data', 'attributes', 'concentration')).to eq 'updated concentration' + expect(json.dig('data', 'attributes', 'donor_id')).to eq 'updated donor_id' + expect(json.dig('data', 'attributes', 'gender')).to eq 'Female' + expect(json.dig('data', 'attributes', 'sample_common_name')).to eq 'updated sample_common_name' + expect(json.dig('data', 'attributes', 'sample_description')).to eq 'updated sample_description' + expect(json.dig('data', 'attributes', 'supplier_name')).to eq 'updated supplier_name' + expect(json.dig('data', 'attributes', 'volume')).to eq 'updated volume' + end + + it 'updates the model correctly' do + # Apply the patch which replaced all the metadata + api_patch "#{base_endpoint}/#{resource_model.id}", payload + + # Check that the model was modified + resource_model.reload + expect(resource_model.cohort).to eq 'updated cohort' + expect(resource_model.collected_by).to eq 'updated collected_by' + expect(resource_model.concentration).to eq 'updated concentration' + expect(resource_model.donor_id).to eq 'updated donor_id' + expect(resource_model.gender).to eq 'Female' + expect(resource_model.sample_common_name).to eq 'updated sample_common_name' + expect(resource_model.sample_description).to eq 'updated sample_description' + expect(resource_model.supplier_name).to eq 'updated supplier_name' + expect(resource_model.volume).to eq 'updated volume' + end + end + + context 'with a missing type in the payload' do + let(:payload) { { 'data' => { 'id' => resource_model.id, 'attributes' => { cohort: 'updated cohort' } } } } + + it 'does not update the collection' do + original_cohort = resource_model.cohort + + api_patch "#{base_endpoint}/#{resource_model.id}", payload + expect(response).to have_http_status(:bad_request) + expect(json['errors'][0]['title']).to eq('Missing Parameter') + + # Check that the model was not modified + resource_model.reload + expect(resource_model.cohort).to eq original_cohort + end + end + end + + describe '#post' do + context 'with a valid payload' do + let(:payload) do + { + 'data' => { + 'type' => 'sample_metadata', + 'attributes' => { + cohort: 'posted cohort', + collected_by: 'posted collected_by', + concentration: 'posted concentration', + donor_id: 'posted donor_id', + gender: 'mixed', + sample_common_name: 'posted sample_common_name', + sample_description: 'posted sample_description', + supplier_name: 'posted supplier_name', + volume: 'posted volume' + } + } + } + end + + it 'gives a success response' do + api_post base_endpoint, payload + + expect(response).to have_http_status(:success) + end + + it 'creates the resource' do + expect { api_post base_endpoint, payload }.to change(Sample::Metadata, :count).by(1) + end + + it 'responds with the correct attributes' do + api_post base_endpoint, payload + + expect(json.dig('data', 'attributes', 'cohort')).to eq 'posted cohort' + expect(json.dig('data', 'attributes', 'collected_by')).to eq 'posted collected_by' + expect(json.dig('data', 'attributes', 'concentration')).to eq 'posted concentration' + expect(json.dig('data', 'attributes', 'donor_id')).to eq 'posted donor_id' + expect(json.dig('data', 'attributes', 'gender')).to eq 'Mixed' + expect(json.dig('data', 'attributes', 'sample_common_name')).to eq 'posted sample_common_name' + expect(json.dig('data', 'attributes', 'sample_description')).to eq 'posted sample_description' + expect(json.dig('data', 'attributes', 'supplier_name')).to eq 'posted supplier_name' + expect(json.dig('data', 'attributes', 'volume')).to eq 'posted volume' + end + + it 'populates the model correctly' do + api_post base_endpoint, payload + + new_model = Sample::Metadata.last + expect(new_model.cohort).to eq 'posted cohort' + expect(new_model.collected_by).to eq 'posted collected_by' + expect(new_model.concentration).to eq 'posted concentration' + expect(new_model.donor_id).to eq 'posted donor_id' + expect(new_model.gender).to eq 'Mixed' + expect(new_model.sample_common_name).to eq 'posted sample_common_name' + expect(new_model.sample_description).to eq 'posted sample_description' + expect(new_model.supplier_name).to eq 'posted supplier_name' + expect(new_model.volume).to eq 'posted volume' + end + end + + context 'with unexpected "birthday" attribute in the payload' do + let(:payload) { { 'data' => { 'type' => 'sample_metadata', 'attributes' => { birthday: '14-04-1954' } } } } + + it 'does not create the resource' do + expect { api_post base_endpoint, payload }.not_to change(Sample::Metadata, :count) + + expect(response).to have_http_status(:bad_request) + expect(json['errors'][0]['detail']).to eq('birthday is not allowed.') # Sad times :( + end + end + end + end +end From 0fdc51e167827f9e263a300ae9e663f95b98fdc2 Mon Sep 17 00:00:00 2001 From: Stuart McHattie Date: Wed, 24 Jul 2024 15:10:48 +0100 Subject: [PATCH 05/15] Remove todo for fixed test class --- .rubocop_todo.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 1b778979b1..22c06f6bf5 100644 --- a/.rubocop_todo.yml +++ b/.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' From b0cd83b4ffd969fde1ce183980223a4881f11db0 Mon Sep 17 00:00:00 2001 From: Stuart McHattie Date: Wed, 24 Jul 2024 15:13:12 +0100 Subject: [PATCH 06/15] Add missing attributes/fields from unit tests for Sample Metadata --- .../api/v2/sample_metadata_resource_spec.rb | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/spec/resources/api/v2/sample_metadata_resource_spec.rb b/spec/resources/api/v2/sample_metadata_resource_spec.rb index 73431baef2..aa113a2bf6 100644 --- a/spec/resources/api/v2/sample_metadata_resource_spec.rb +++ b/spec/resources/api/v2/sample_metadata_resource_spec.rb @@ -10,21 +10,27 @@ # Test attributes it 'has the expected attributes', :aggregate_failures do - expect(resource).to have_attribute :sample_common_name - expect(resource).to have_attribute :supplier_name + expect(resource).to have_attribute :cohort expect(resource).to have_attribute :collected_by - expect(resource).to have_attribute :donor_id expect(resource).to have_attribute :concentration + expect(resource).to have_attribute :donor_id + expect(resource).to have_attribute :gender + expect(resource).to have_attribute :sample_common_name + expect(resource).to have_attribute :sample_description + expect(resource).to have_attribute :supplier_name expect(resource).to have_attribute :volume end # Updatable fields it 'allows updating of read-write fields', :aggregate_failures do - expect(resource).to have_updatable_field :sample_common_name - expect(resource).to have_updatable_field :supplier_name + expect(resource).to have_updatable_field :cohort expect(resource).to have_updatable_field :collected_by - expect(resource).to have_updatable_field :donor_id expect(resource).to have_updatable_field :concentration + expect(resource).to have_updatable_field :donor_id + expect(resource).to have_updatable_field :gender + expect(resource).to have_updatable_field :sample_common_name + expect(resource).to have_updatable_field :sample_description + expect(resource).to have_updatable_field :supplier_name expect(resource).to have_updatable_field :volume end From 66b6f74ca1a086c928c4e9ee837842816b802d01 Mon Sep 17 00:00:00 2001 From: Stephen Hulme Date: Thu, 25 Jul 2024 09:06:49 +0100 Subject: [PATCH 07/15] fix: repair deprecated tags --- app/models/illumina_htp/mx_tube_purpose.rb | 2 +- app/models/illumina_htp/stock_tube_purpose.rb | 2 +- app/models/plate.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/models/illumina_htp/mx_tube_purpose.rb b/app/models/illumina_htp/mx_tube_purpose.rb index 99bea208ec..cd6a76307a 100644 --- a/app/models/illumina_htp/mx_tube_purpose.rb +++ b/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 # diff --git a/app/models/illumina_htp/stock_tube_purpose.rb b/app/models/illumina_htp/stock_tube_purpose.rb index dc17ef263c..5f35a29174 100644 --- a/app/models/illumina_htp/stock_tube_purpose.rb +++ b/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 # diff --git a/app/models/plate.rb b/app/models/plate.rb index cf599c54d3..0655990c19 100644 --- a/app/models/plate.rb +++ b/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 From e97b349ae37a352fd26036bf3f720f42a77f1fec Mon Sep 17 00:00:00 2001 From: Stephen Hulme Date: Thu, 25 Jul 2024 09:08:04 +0100 Subject: [PATCH 08/15] docs: repair broken parameters --- app/helpers/application_helper.rb | 6 +++--- app/jobs/asset_link/builder_job.rb | 4 ++-- app/models/cherrypick_task.rb | 4 +--- app/models/plate/quad_creator.rb | 2 +- lib/accession/accession/configuration.rb | 2 ++ lib/nested_validation.rb | 2 +- 6 files changed, 10 insertions(+), 10 deletions(-) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 005b9f1ae0..9db8efe1ab 100644 --- a/app/helpers/application_helper.rb +++ b/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] 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 diff --git a/app/jobs/asset_link/builder_job.rb b/app/jobs/asset_link/builder_job.rb index 75cc32edf9..67d745bbfe 100644 --- a/app/jobs/asset_link/builder_job.rb +++ b/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 diff --git a/app/models/cherrypick_task.rb b/app/models/cherrypick_task.rb index c641b3f603..4b41a71bd3 100644 --- a/app/models/cherrypick_task.rb +++ b/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 diff --git a/app/models/plate/quad_creator.rb b/app/models/plate/quad_creator.rb index 3444bf6266..46dc85f7af 100644 --- a/app/models/plate/quad_creator.rb +++ b/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 [] Location name of the well or tube. Eg. A3 + # @param [] locn_name name of the well or tube. Eg. A3 # # @return [Array] An array of two integers indicating column and row. eg. [0, 2] # diff --git a/lib/accession/accession/configuration.rb b/lib/accession/accession/configuration.rb index 31f00202a2..67dbac8d2d 100644 --- a/lib/accession/accession/configuration.rb +++ b/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] a list of symbols FILES = [:tags].freeze attr_accessor :folder, *FILES diff --git a/lib/nested_validation.rb b/lib/nested_validation.rb index 865854d85e..21122483ec 100644 --- a/lib/nested_validation.rb +++ b/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) From 10f8eed9ef8b2ad8fc57a1c4becec0eee5685f34 Mon Sep 17 00:00:00 2001 From: Stephen Hulme Date: Thu, 25 Jul 2024 09:29:26 +0100 Subject: [PATCH 09/15] build: add linting for yard docs to CI --- .github/workflows/lint_yard_docs.yml | 18 ++++++++++++++++++ Gemfile | 9 +++++---- Gemfile.lock | 6 ++++++ README.md | 8 ++++++++ 4 files changed, 37 insertions(+), 4 deletions(-) create mode 100644 .github/workflows/lint_yard_docs.yml diff --git a/.github/workflows/lint_yard_docs.yml b/.github/workflows/lint_yard_docs.yml new file mode 100644 index 0000000000..a4a3685061 --- /dev/null +++ b/.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 diff --git a/Gemfile b/Gemfile index 850f6d5d36..feb5fe24ae 100644 --- a/Gemfile +++ b/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 diff --git a/Gemfile.lock b/Gemfile.lock index d4e644d26e..6fe00f7f6e 100644 --- a/Gemfile.lock +++ b/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 diff --git a/README.md b/README.md index 50c46b789b..4630426d0e 100644 --- a/README.md +++ b/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: From e40223f288c7bc1a65be786af043a94e70afb75d Mon Sep 17 00:00:00 2001 From: Stephen Hulme Date: Thu, 25 Jul 2024 12:04:32 +0100 Subject: [PATCH 10/15] fix: encapsulate Aliquot::TagClash error to prevent calling on analysis --- app/api/core/service/error_handling.rb | 9 +++++++-- spec/models/transfer_request_spec.rb | 4 +++- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/app/api/core/service/error_handling.rb b/app/api/core/service/error_handling.rb index c5722ddcb6..cbce1a0529 100644 --- a/app/api/core/service/error_handling.rb +++ b/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 diff --git a/spec/models/transfer_request_spec.rb b/spec/models/transfer_request_spec.rb index 84747f7b74..662d6d9d02 100644 --- a/spec/models/transfer_request_spec.rb +++ b/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 From 878f81d977a323b60e31f5af9a86844f3585a93f Mon Sep 17 00:00:00 2001 From: Stephen Hulme Date: Thu, 25 Jul 2024 12:06:12 +0100 Subject: [PATCH 11/15] docs: update parameter names --- app/resources/api/v2/asset_audit_resource.rb | 2 +- app/resources/api/v2/volume_update_resource.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/resources/api/v2/asset_audit_resource.rb b/app/resources/api/v2/asset_audit_resource.rb index f7a03a4452..a68ec83fb0 100644 --- a/app/resources/api/v2/asset_audit_resource.rb +++ b/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 diff --git a/app/resources/api/v2/volume_update_resource.rb b/app/resources/api/v2/volume_update_resource.rb index 78db317197..9c8ea04f0c 100644 --- a/app/resources/api/v2/volume_update_resource.rb +++ b/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 From eb0b3bdb22c002db3dc72e527acfd8eb88ce1a3a Mon Sep 17 00:00:00 2001 From: Stephen Hulme Date: Thu, 25 Jul 2024 12:10:06 +0100 Subject: [PATCH 12/15] style: lint --- Gemfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile b/Gemfile index feb5fe24ae..51bb14aaab 100644 --- a/Gemfile +++ b/Gemfile @@ -158,7 +158,7 @@ group :development, :linting do # Automatically generate documentation gem 'yard', require: false gem 'yard-activerecord', '~> 0.0.16', require: false - gem "yard-junk", "~> 0.0.9", require: false + gem 'yard-junk', '~> 0.0.9', require: false end group :linting, :test do From 5e2410ec24541e145e1227b7fde666c9c35b9d67 Mon Sep 17 00:00:00 2001 From: Stephen Hulme Date: Thu, 25 Jul 2024 12:13:02 +0100 Subject: [PATCH 13/15] style: rubocop --- app/models/cherrypick_task.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/cherrypick_task.rb b/app/models/cherrypick_task.rb index 4b41a71bd3..13304cb801 100644 --- a/app/models/cherrypick_task.rb +++ b/app/models/cherrypick_task.rb @@ -36,7 +36,7 @@ def new_control_locator(batch_id, total_wells, num_control_wells, wells_to_leave # # @return [false,'Can only be accessed via the previous step'>] Array indicating this action can't be linked # - def can_link_directly?() + def can_link_directly? [false, 'Can only be accessed via the previous step'] end From e10385301e2a5333b054b05a5699711950ad56ce Mon Sep 17 00:00:00 2001 From: Andrew Sparkes Date: Fri, 26 Jul 2024 13:11:22 +0100 Subject: [PATCH 14/15] fixed typo --- app/sequencescape_excel/sequencescape_excel/validation.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/sequencescape_excel/sequencescape_excel/validation.rb b/app/sequencescape_excel/sequencescape_excel/validation.rb index 346526ddd8..ad0b7e0843 100644 --- a/app/sequencescape_excel/sequencescape_excel/validation.rb +++ b/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 From c125c64ba33b4befc8280ead59b7aeaa698773cc Mon Sep 17 00:00:00 2001 From: Andrew Sparkes Date: Fri, 26 Jul 2024 13:14:03 +0100 Subject: [PATCH 15/15] added version of specimen barcode column with mandatory text length range for anospp pipeline --- config/sample_manifest_excel/columns.yml | 20 +++++++++++++++++++ .../sample_manifest_excel/manifest_types.yml | 2 +- spec/data/sample_manifest_excel/columns.yml | 20 +++++++++++++++++++ .../sample_manifest_excel/manifest_types.yml | 2 +- 4 files changed, 42 insertions(+), 2 deletions(-) diff --git a/config/sample_manifest_excel/columns.yml b/config/sample_manifest_excel/columns.yml index 54f8febbcf..6bd16407f1 100644 --- a/config/sample_manifest_excel/columns.yml +++ b/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 diff --git a/config/sample_manifest_excel/manifest_types.yml b/config/sample_manifest_excel/manifest_types.yml index 3dd6ddd915..4c29a4c456 100644 --- a/config/sample_manifest_excel/manifest_types.yml +++ b/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 diff --git a/spec/data/sample_manifest_excel/columns.yml b/spec/data/sample_manifest_excel/columns.yml index 405efbb1dc..8199b9d64c 100644 --- a/spec/data/sample_manifest_excel/columns.yml +++ b/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 diff --git a/spec/data/sample_manifest_excel/manifest_types.yml b/spec/data/sample_manifest_excel/manifest_types.yml index 40e08b82c8..d6c5c5aeae 100644 --- a/spec/data/sample_manifest_excel/manifest_types.yml +++ b/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