Merge pull request #4227 from sanger/add-attributes-to-v2-sample-meta…

…data

Add gender attribute to v2 `Sample::Metadata` resource

.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'

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

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/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

spec/resources/api/v2/sample_metadata_resource_spec.rb

@@ -4,18 +4,38 @@
 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 :cohort
+    expect(resource).to have_attribute :collected_by
+    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 :cohort
+    expect(resource).to have_updatable_field :collected_by
+    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
+
+  # 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