From a183f721e88987f158bfe538368611402ab655b5 Mon Sep 17 00:00:00 2001 From: Stuart McHattie Date: Wed, 24 Jul 2024 14:36:27 +0100 Subject: [PATCH 1/6] 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 2/6] 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 3/6] 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 4/6] 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 5/6] 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 6/6] 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