diff --git a/app/resources/api/v2/qcable_resource.rb b/app/resources/api/v2/qcable_resource.rb index 3ffc30d7b7..e3622aa8cf 100644 --- a/app/resources/api/v2/qcable_resource.rb +++ b/app/resources/api/v2/qcable_resource.rb @@ -2,48 +2,127 @@ module Api module V2 - # @todo This documentation does not yet include a detailed description of what this resource represents. - # @todo This documentation does not yet include detailed descriptions for relationships, attributes and filters. - # @todo This documentation does not yet include any example usage of the API via cURL or similar. + # Provides a JSON:API representation of {Qcable} which represents an element of a lot which needs to be approved + # by QC before it can be used. # # @note Access this resource via the `/api/v2/qcables/` endpoint. # - # Provides a JSON:API representation of {Qcable}. + # @example POST request to create a {Qcable}. + # POST /api/v2/qcables/ + # { + # "data": { + # "type": "qc_files", + # "attributes": { + # "filename": "qc_file.csv", + # "contents": "A1,A2,A3\n1,2,3\n4,5,6\n" + # }, + # "relationships": { + # "asset": { + # "data": { + # "type": "labware", + # "id": 456 + # } + # } + # } + # } + # } + # + # @example PATCH request to change the {Asset} of a {Qcable}. + # PATCH /api/v2/qcables/ + # { + # "data": { + # "type": "qc_files", + # "id": 123 + # "relationships": { + # "asset": { + # "data": { + # "type": "labware", + # "id": 456 + # } + # } + # } + # } + # } + # + # @example GET request for all {Qcable} resources + # GET /api/v2/qcables/ + # + # @example GET request for a {Qcable} with ID 123 + # GET /api/v2/qcables/123/ # # For more information about JSON:API see the [JSON:API Specifications](https://jsonapi.org/format/) # or look at the [JSONAPI::Resources](http://jsonapi-resources.com/) package for Sequencescape's implementation # of the JSON:API standard. class QcableResource < BaseResource - # Constants... - - # model_name / model_hint if required - default_includes :uuid_object, :barcodes - # Associations: - has_one :lot - has_one :asset, polymorphic: true - + ### # Attributes - attribute :uuid, readonly: true - attribute :state, write_once: true - attribute :labware_barcode, write_once: true - - # Filters - filter :barcode, apply: ->(records, value, _options) { records.with_barcode(value) } + ### - # Custom methods - # These shouldn't be used for business logic, and are more about - # I/O and isolating implementation details. + # @!attribute [r] labware_barcode + # @return [Hash] the barcodes of the labware associated with this {Qcable}. + # This includes the EAN13 barcode, the machine barcode and the human barcode. + # Note however that some of these barcodes may be `nil`. + attribute :labware_barcode, readonly: true def labware_barcode { - 'ean13_barcode' => _model.ean13_barcode, - 'machine_barcode' => _model.machine_barcode, - 'human_barcode' => _model.human_barcode + 'ean13_barcode' => @model.ean13_barcode, + 'machine_barcode' => @model.machine_barcode, + 'human_barcode' => @model.human_barcode } end - # Class method overrides + # @!attribute [r] state + # @return [String] a string representation of the state this {Qcable} is in. + # The state is changed by a state machine via events that occur as the {Qcable} is processed. + # The possible states are: + # - `created` + # - `pending` + # - `failed` + # - `passed` + # - `available` + # - `destroyed` + # - `qc_in_progress` + # - `exhausted`. + attribute :state, readonly: true + + # @!attribute [r] uuid + # @return [String] the UUID of this {Qcable}. + attribute :uuid, readonly: true + + ### + # Relationships + ### + + # @!attribute [rw] asset + # @return [LabwareResource] the {Labware} resource associated with this {Qcable}. + # @deprecated Use the {#labware} relationship instead. + has_one :asset + + # @!attribute [rw] labware + # @return [LabwareResource] the {Labware} resource associated with this {Qcable}. + has_one :labware, relation_name: 'asset', foreign_key: :asset_id + + # @!attribute [rw] lot + # @return [LotResource] the {Lot} resource associated with this {Qcable}. + has_one :lot + + ### + # Filters + ### + + # @!method filter_barcode + # Apply a filter across all {Qcable} resource , matching by barcode. + # @example Get all {Qcable} resources with a specific barcode. + # /api/v2/qcables?filter[barcode]=1234567890123 + filter :barcode, apply: ->(records, value, _options) { records.with_barcode(value) } + + # @!method filter_uuid + # Apply a filter across all {Qcable} resources, matching by UUID. + # @example Get all {Qcable} resources with a specific UUID. + # /api/v2/qcables?filter[uuid]=12345678-1234-1234-1234-123456789012 + filter :uuid, apply: ->(records, value, _options) { records.with_uuid(value) } end end end diff --git a/spec/factories/z_tag_qc_factories.rb b/spec/factories/z_tag_qc_factories.rb index b2b9e98c84..50381ea43b 100644 --- a/spec/factories/z_tag_qc_factories.rb +++ b/spec/factories/z_tag_qc_factories.rb @@ -66,10 +66,11 @@ # callbacks. lot { create(:lot) } qcable_creator { create(:qcable_creator) } + transient { sanger_barcode { create(:sanger_ean13) } } factory :qcable_with_asset do state { 'created' } - asset { create(:full_plate) } + asset { create(:full_plate, sanger_barcode:) } end end diff --git a/spec/requests/api/v2/qcables_spec.rb b/spec/requests/api/v2/qcables_spec.rb index 03f2aff4c1..21a7be0b8e 100644 --- a/spec/requests/api/v2/qcables_spec.rb +++ b/spec/requests/api/v2/qcables_spec.rb @@ -2,56 +2,246 @@ require 'rails_helper' require './spec/requests/api/v2/shared_examples/api_key_authenticatable' +require './spec/requests/api/v2/shared_examples/requests' -describe 'Qcables API', with: :api_v2 do - let(:base_endpoint) { '/api/v2/qcables' } +describe 'QcFiles API', tags: :lighthouse, with: :api_v2 do + let(:model_class) { Qcable } + let(:base_endpoint) { "/api/v2/#{resource_type}" } + let(:resource_type) { model_class.name.demodulize.pluralize.underscore } it_behaves_like 'ApiKeyAuthenticatable' - context 'with multiple Qcables' do - before { create_list(:qcable, 5) } + context 'with a list of resources' do + let(:resource_count) { 5 } + let!(:resources) { Array.new(resource_count) { create(:qcable_with_asset) } } - it 'sends a list of qcables' do - api_get base_endpoint + describe '#GET all resources' do + before { api_get base_endpoint } - # test for the 200 status-code - expect(response).to have_http_status(:success) + it 'responds with a success http code' do + expect(response).to have_http_status(:success) + end - # check to make sure the right amount of messages are returned - expect(json['data'].length).to eq(5) + it 'returns all the resources' do + expect(json['data'].count).to eq(resource_count) + end end - # Check filters, ESPECIALLY if they aren't simple attribute filters + describe '#filter' do + let(:target_resource) { resources.sample } + let(:target_id) { target_resource.id } + + describe 'by ean13 barcode' do + before { api_get "#{base_endpoint}?filter[barcode]=#{target_resource.ean13_barcode}" } + + it_behaves_like 'has filtered to a resource with target_id correctly' + end + + describe 'by human barcode' do + before { api_get "#{base_endpoint}?filter[barcode]=#{target_resource.human_barcode}" } + + it_behaves_like 'has filtered to a resource with target_id correctly' + end + + describe 'by machine barcode' do + before { api_get "#{base_endpoint}?filter[barcode]=#{target_resource.machine_barcode}" } + + it_behaves_like 'has filtered to a resource with target_id correctly' + end + + describe 'by uuid' do + before { api_get "#{base_endpoint}?filter[uuid]=#{target_resource.uuid}" } + + it_behaves_like 'has filtered to a resource with target_id correctly' + end + end end - context 'with a Qcable' do - let(:resource_model) { create(:qcable) } + context 'with a single resource' do + describe '#GET resource by ID' do + let(:resource) { create(:qcable_with_asset) } + + context 'without included relationships' do + before { api_get "#{base_endpoint}/#{resource.id}" } + + it 'responds with a success http code' do + expect(response).to have_http_status(:success) + end + + it 'returns the resource with the correct id' do + expect(json.dig('data', 'id')).to eq(resource.id.to_s) + end - let(:payload) do - { - 'data' => { - 'id' => resource_model.id, - 'type' => 'qcables', - 'attributes' => { - # Set new attributes + it 'returns the resource with the correct type' do + expect(json.dig('data', 'type')).to eq(resource_type) + end + + it 'responds with the correct labware_barcode attribute value' do + barcode_hash = { + 'ean13_barcode' => resource.ean13_barcode, + 'machine_barcode' => resource.machine_barcode, + 'human_barcode' => resource.human_barcode } - } - } + expect(json.dig('data', 'attributes', 'labware_barcode')).to eq(barcode_hash) + end + + it 'responds with the correct state attribute value' do + expect(json.dig('data', 'attributes', 'state')).to eq(resource.state) + end + + it 'responds with the correct uuid attribute value' do + expect(json.dig('data', 'attributes', 'uuid')).to eq(resource.uuid) + end + + it 'returns a reference to the asset relationship' do + expect(json.dig('data', 'relationships', 'asset')).to be_present + end + + it 'returns a reference to the labware relationship' do + expect(json.dig('data', 'relationships', 'labware')).to be_present + end + + it 'returns a reference to the lot relationship' do + expect(json.dig('data', 'relationships', 'lot')).to be_present + end + + it 'does not include attributes for related resources' do + expect(json['included']).not_to be_present + end + end + + context 'with included relationships' do + it_behaves_like 'a GET request including a has_one relationship', 'asset' + it_behaves_like 'a GET request including a has_one relationship', 'labware' + it_behaves_like 'a GET request including a has_one relationship', 'lot' + end end + end + + describe '#PATCH a resource' do + let(:resource) { create(:qcable_with_asset) } + + context 'with a valid payload' do + let(:new_labware) { create(:full_plate) } - it 'sends an individual Qcable' do - api_get "#{base_endpoint}/#{resource_model.id}" - expect(response).to have_http_status(:success) - expect(json.dig('data', 'type')).to eq('qcables') + context 'with non-deprecated relationships' do + let(:new_lot) { create(:lot) } + let(:payload) do + { + data: { + id: resource.id, + type: resource_type, + relationships: { + labware: { + data: { + type: 'labware', + id: new_labware.id + } + }, + lot: { + data: { + type: 'lots', + id: new_lot.id + } + } + } + } + } + end + + before { api_patch "#{base_endpoint}/#{resource.id}", payload } + + it 'updates the labware/asset on the resource' do + expect(resource.reload.asset).to eq(new_labware) + end + + it 'updates the lot on the resource' do + expect(resource.reload.lot).to eq(new_lot) + end + end + + context 'with deprecated relationships' do + let(:payload) do + { + data: { + id: resource.id, + type: resource_type, + relationships: { + asset: { + data: { + type: 'labware', + id: new_labware.id + } + } + } + } + } + end + + before { api_patch "#{base_endpoint}/#{resource.id}", payload } + + it 'updates the labware/asset on the resource' do + expect(resource.reload.asset).to eq(new_labware) + end + end + end + + context 'with a read-only attribute in the payload' do + context 'with labware_barcode' do + let(:payload) do + { data: { id: resource.id, type: resource_type, attributes: { labware_barcode: { human_barcode: '1-2' } } } } + end + + it_behaves_like 'a PATCH request with a disallowed value', 'labware_barcode' + end + + context 'with state' do + let(:payload) { { data: { id: resource.id, type: resource_type, attributes: { state: 'pending' } } } } + + it_behaves_like 'a PATCH request with a disallowed value', 'state' + end + + context 'with uuid' do + let(:payload) { { data: { id: resource.id, type: resource_type, attributes: { uuid: 'new-uuid' } } } } + + it_behaves_like 'a PATCH request with a disallowed value', 'uuid' + end end + end + + describe '#POST a create request' do + # This test is a bit weird, because for whatever reason, the resource is currently incomplete for Qcables. Qcables + # validate the presence of a lot, asset, and qcable_creator, but the API doesn't allow you to set a qcable_creator. + # So, we can't actually create a valid Qcable through the API. + # + # I don't know why this is the case - it seems like it was an oversight on this endpoint, but the endpoint is in use + # already, so I don't want to modify it more than I have to. It's quite likely that the endpoint should be + # immutable, but it wasn't configured that way and I don't know what existing users may already be trying to do with + # this endpoint, so I'm going to leave it alone. For now, let's just test that the endpoint doesn't allow you to + # create a Qcable and it can be adjusted in future if we need to be able to create them at that time. + + let(:labware) { create(:plate) } + let(:lot) { create(:lot) } + + let(:labware_relationship) { { data: { id: labware.id, type: 'labware' } } } + let(:lot_relationship) { { data: { id: lot.id, type: 'lots' } } } + + let(:base_attributes) { {} } + let(:base_relationships) { { labware: labware_relationship, lot: lot_relationship } } + + context 'with a complete payload' do + let(:payload) do + { data: { type: resource_type, attributes: base_attributes, relationships: base_relationships } } + end + + it 'fails with an unprocessable entity status' do + api_post base_endpoint, payload + expect(response).to have_http_status(:unprocessable_entity) + end - # Remove if immutable - it 'allows update of a Qcable' do - api_patch "#{base_endpoint}/#{resource_model.id}", payload - expect(response).to have_http_status(:success) - expect(json.dig('data', 'type')).to eq('qcables') - # Double check at least one of the attributes - # eg. expect(json.dig('data', 'attributes', 'state')).to eq('started') + it 'does not create a new resource' do + expect { api_post base_endpoint, payload }.not_to change(model_class, :count) + end end end end diff --git a/spec/requests/api/v2/shared_examples/requests.rb b/spec/requests/api/v2/shared_examples/requests.rb index 829b65e79c..13b896be5d 100644 --- a/spec/requests/api/v2/shared_examples/requests.rb +++ b/spec/requests/api/v2/shared_examples/requests.rb @@ -113,3 +113,38 @@ expect(model_class.last.send(related_name)).to eq(value) end end + +shared_examples 'a PATCH request with a disallowed value' do |disallowed_value| + def do_patch + api_patch "#{base_endpoint}/#{resource.id}", payload + end + + it 'does not modify the resource attributes' do + # Note that attributes also includes IDs for relationships. + expect { do_patch }.not_to(change { resource.reload.attributes }) + end + + it 'responds with bad_request' do + do_patch + expect(response).to have_http_status(:bad_request) + end + + it 'specifies which value was not allowed' do + do_patch + expect(json.dig('errors', 0, 'detail')).to eq("#{disallowed_value} is not allowed.") + end +end + +shared_examples 'has filtered to a resource with target_id correctly' do + it 'responds with a success http code' do + expect(response).to have_http_status(:success) + end + + it 'returns one resource' do + expect(json['data'].count).to eq(1) + end + + it 'returns the correct resource' do + expect(json['data'].first['id']).to eq(target_id.to_s) + end +end diff --git a/spec/resources/api/v2/qcable_resource_spec.rb b/spec/resources/api/v2/qcable_resource_spec.rb index 3bd5e5d8a3..e4d9355966 100644 --- a/spec/resources/api/v2/qcable_resource_spec.rb +++ b/spec/resources/api/v2/qcable_resource_spec.rb @@ -12,14 +12,16 @@ it { is_expected.to have_model_name 'Qcable' } # Attributes - it { is_expected.to have_write_once_attribute :labware_barcode } - it { is_expected.to have_write_once_attribute :state } + it { is_expected.to have_readonly_attribute :labware_barcode } + it { is_expected.to have_readonly_attribute :state } it { is_expected.to have_readonly_attribute :uuid } # Relationships - it { is_expected.to have_a_writable_has_one(:lot).with_class_name('Lot') } it { is_expected.to have_a_writable_has_one(:asset).with_class_name('Labware') } + it { is_expected.to have_a_writable_has_one(:labware).with_class_name('Labware') } + it { is_expected.to have_a_writable_has_one(:lot).with_class_name('Lot') } # Filters it { is_expected.to filter(:barcode) } + it { is_expected.to filter(:uuid) } end