Merge pull request #1835 from sanger/Y24-190-3-use-ss-v2-for-labware-…

…creators

Y24-190-3: Use SS v2 for TagLayoutTemplates

README.md

@@ -220,12 +220,68 @@ There are a few tools available to assist with writing specs:
 
 - Helpers: `with_has_many_associations` and `with_belongs_to_associations` can be used in factories to set up the relevant json. They won't actually mock up the relevant requests, but ensure that things like actions are defined so that the api knows where to find them.
 
-#### Request stubbing
+#### Request stubbing for the Sequencescape v1 API
 
 Request stubs are provided by webmock. Two helper methods will assist with the majority of mocking requests to the api, `stub_api_get` and `stub_api_post`. See `spec/support/api_url_helper.rb` for details.
 
 **Note**: Due to the way the api functions, the factories don't yet support nested associations.
 
+#### Request stubbing for the Sequencescape v2 API
+
+The V2 API uses `JsonApiClient::Resource` sub-classes to represent the records in memory.
+Generally these are quite dynamic so you don't need to explicitly specify every property the API will respond with.
+The base class also provides us with methods that are familiar to Rails for finding one or more records that match criteria.
+So to stub the API, the easiest thing to do is to get FactoryBot to make up resources using the specific resource sub-class for the V2 API, and then mock the calls to those lookup methods.
+Many of these have already been done for you in `spec/support/api_url_helper.rb` such as `stub_v2_study` and `stub_v2_tag_layout_templates` which sets up the `find` method for studies by name and the `all` method for tag layout templates, respectively.
+However there's also `stub_api_v2_post`, `stub_api_v2_patch` and `stub_api_v2_save` which ensures that any calls to the `create`, `update` and the `save` method for resources of a particular type are expected and give a return value.
+If none of the existing method suit your needs, you should add new ones.
+
+##### FactoryBot is not mocking my related resources correctly
+
+Nested relationships, such as Wells inside a Plate, the resource should indicate this with keywords like `has_one`, `has_many`, `belongs_to`, etc.
+See the [json_api_client repository](https://github.com/JsonApiClient/json_api_client) for this topic and more.
+However, FactoryBot does not get on well with some of these relationship methods and will not mock them properly using standard FactoryBot definitions.
+
+If you find that FactoryBot is not giving you the expected resource for a related record, you can inject the related resource directly into the `JsonApiClient::Resource`'s cache of relationships.
+To do that, define the related resource as a `transient` variable and use an `after(:build)` block to assign the resource to the `_cached_relationship` method.
+For example, where the Resource might be defined as the following class:
+
+```ruby
+class Sequencescape::Api::V2::RootRecord < JsonApiClient::Resource
+  has_one :related_thing
+end
+```
+
+You might expect to be able to use FactoryBot in the following way:
+
+```ruby
+FactoryBot.define do
+  factory :root_record, class: Sequencescape::Api::V2::RootRecord do
+    skip_create
+
+    related_thing { create :related_thing }
+  end
+end
+```
+
+But the related thing will not be the one you defined to be generated by another factory.
+It appears the `has_one` method in the resource over-rides the mocked value and you get an empty record back instead.
+So, instead, you should create the `related_thing` record as a transient and assign it to the `root_record` in an `after(:build)` block as shown here.
+
+```ruby
+FactoryBot.define do
+  factory :root_record, class: Sequencescape::Api::V2::RootRecord do
+    skip_create
+
+    transient { related_thing { create :v2_tag_group_with_tags } }
+
+    after(:build) do |record, factory|
+      record._cached_relationship(:related_thing) { factory.related_thing } if evaluator.related_thing
+    end
+  end
+end
+```
+
 #### Feature debugging
 
 To help with debugging feature specs, temporarily comment out the line `options.add_argument('--headless')` in `spec/spec_helper.rb`. This will allow you to see the browser as the tests run. To pause the execution at certain point, possibly before an expected failure, insert `binding.pry` at the appropriate place in the spec.

app/models/concerns/labware_creators/tagged_plate_behaviour.rb

@@ -30,7 +30,7 @@ def transfer_hash
   end
 
   def tag_plates
-    @tag_plates ||= LabwareCreators::Tagging::TagCollection.new(api, labware, purpose_uuid)
+    @tag_plates ||= LabwareCreators::Tagging::TagCollection.new(labware, purpose_uuid)
   end
 
   #

app/models/labware_creators/tagged_plate.rb

@@ -79,6 +79,10 @@ def convert_tag_plate_to_new_purpose
 
     def create_labware!
       create_plate! do |plate_uuid|
+        # TODO: {Y24-190} Work out a way to call the `create!` method on TagLayoutTemplate model in Sequencescape
+        #       using the V2 API. I think either we need to misuse the PATCH method with some kind of
+        #       attributes telling Sequencescape to run the `create!` method, or we need to create a new
+        #       endpoint associated with a TagLayoutTemplate that will run the `create!` method.
         api
           .tag_layout_template
           .find(tag_plate.template_uuid)

app/models/labware_creators/tagging/tag_collection.rb

@@ -5,12 +5,10 @@ class TagCollection # rubocop:todo Style/Documentation
     #
     # Create a tag collection
     #
-    # @param [Sequencescape::Client::Api] api an api object used to retrieve tag templates
     # @param [Limber::Plate] plate The plate from which the tag layout will be generated
     # @param [String] purpose_uuid The uuid of the purpose which is about to be created
     #
-    def initialize(api, plate, purpose_uuid)
-      @api = api
+    def initialize(plate, purpose_uuid)
       @plate = plate
       @purpose_uuid = purpose_uuid
     end
@@ -105,7 +103,8 @@ def tags_by_column(layout)
     end
 
     def tag_layout_templates
-      @api.tag_layout_template.all.map(&:coerce)
+      query = Sequencescape::Api::V2::TagLayoutTemplate.paginate(per_page: 100)
+      Sequencescape::Api::V2.merge_page_results(query).map(&:coerce)
     end
   end
 end

app/sequencescape/sequencescape/api/v2/tag_layout_template.rb

@@ -0,0 +1,46 @@
+# frozen_string_literal: true
+
+# tag layout template resource
+class Sequencescape::Api::V2::TagLayoutTemplate < Sequencescape::Api::V2::Base
+  has_one :tag_group
+  has_one :tag2_group, class_name: 'TagGroup'
+
+  def dual_index?
+    tag2_group.present?
+  end
+
+  # Performs the coercion of this instance so that it behaves appropriately given the direction
+  # and walking algorithm information.
+  def coerce
+    extend("limber/tag_layout_template/in_#{direction.gsub(/\s+/, '_')}s".camelize.constantize)
+    extend("limber/tag_layout_template/walk_#{walking_by.gsub(/\s+/, '_')}".camelize.constantize)
+  rescue NameError => e
+    Rails.logger.warn("Unrecognised layout options: #{e.message}")
+    extend Unsupported
+  ensure
+    self
+  end
+
+  # This returns an array of well location to pool pairs.  The 'walker' is responsible for actually doing the walking
+  # of the wells that are acceptable, and it calls back with the location of the well being processed.
+  def group_wells(plate)
+    well_to_pool = plate.wells.each_with_object({}) { |well, store| store[well.location] = well.pool_id }
+
+    # We assume that if a well is unpooled then it is in the same pool as the previous pool.
+    prior_pool = nil
+    callback =
+      lambda do |row_column|
+        prior_pool = pool = well_to_pool[row_column] || prior_pool # or next
+        well_empty = well_to_pool[row_column].nil?
+        well = pool.nil? ? nil : row_column
+        [well, pool, well_empty] # Triplet: [ A1, pool_id, well_empty ]
+      end
+    yield(callback)
+  end
+  private :group_wells
+
+  def tag_ids
+    tag_group.tags.map { |t| t['index'].to_i }.sort
+  end
+  private :tag_ids
+end

spec/factories/asset_factories.rb

@@ -18,7 +18,7 @@
     purpose_uuid { 'example-purpose-uuid' }
     purpose { create :v2_purpose, name: purpose_name, uuid: purpose_uuid }
 
-    # Mock the relationships. Should probably handle this all a bit differently
+    # See the README.md for an explanation under "FactoryBot is not mocking my related resources correctly"
     after(:build) { |asset, evaluator| asset._cached_relationship(:purpose) { evaluator.purpose } }
   end
 end

spec/factories/labware_factories.rb

@@ -22,9 +22,8 @@
     factory(:labware_tube) { type { 'tubes' } }
     factory(:labware_tube_rack) { type { 'tube_racks' } }
 
+    # See the README.md for an explanation under "FactoryBot is not mocking my related resources correctly"
     after(:build) do |labware, evaluator|
-      # see plate_factories -> v2_plate factory -> after(:build) for an explanation of _cached_relationship
-      # basically, it allows you to call .purpose on the labware and get a Sequencescape::Api::V2::Purpose
       labware._cached_relationship(:purpose) { evaluator.purpose } if evaluator.purpose
       labware._cached_relationship(:ancestors) { evaluator.ancestors } if evaluator.ancestors
     end

spec/factories/plate_factories.rb

@@ -134,17 +134,7 @@
     updated_at { '2017-06-29T09:31:59.000+01:00' }
     custom_metadatum_collection { nil }
 
-    # Set up the relationships.
-    # json_client_api handles assigning of relationship information in a frustrating manner
-    # which isn't amenable to setting up objects for testing. Instead it tends to strip
-    # the attributes off the associated records, leaving just a type and an id. This is not
-    # useful if you want to use this data later.
-    # Even more frustratingly is that if you attempt to bypass this and set the attribute directly
-    # the getter attempts to fetch the object via a cache instead.
-    # Here we populate the cache directly with the objects we want. This is *MUCH* faster
-    # than achieving the same through mocks.
-    # We could probably avoid needing to do anything sneaky at all if we instead generated
-    # json-api data and generated the objects from that.
+    # See the README.md for an explanation under "FactoryBot is not mocking my related resources correctly"
     after(:build) do |plate, evaluator|
       Sequencescape::Api::V2::Plate.associations.each do |association|
         plate._cached_relationship(association.attr_name) { evaluator.send(association.attr_name) }

spec/factories/receptacle_factories.rb

@@ -12,6 +12,7 @@
 
     transient { qc_results { [] } }
 
+    # See the README.md for an explanation under "FactoryBot is not mocking my related resources correctly"
     after(:build) do |receptacle, evaluator|
       receptacle._cached_relationship(:qc_results) { evaluator.qc_results || [] }
       receptacle._cached_relationship(:requests_as_source) { evaluator.requests_as_source || [] }

spec/factories/request_factories.rb

@@ -70,6 +70,7 @@
     factory :library_request do
       request_type { create :library_request_type }
 
+      # See the README.md for an explanation under "FactoryBot is not mocking my related resources correctly"
       after(:build) { |request, evaluator| request._cached_relationship(:request_type) { evaluator.request_type } }
 
       # Library request with primer panel information

spec/factories/sample_factories.rb

@@ -15,6 +15,7 @@
     sample_manifest { create(:v2_sample_manifest) }
     uuid { SecureRandom.uuid }
 
+    # See the README.md for an explanation under "FactoryBot is not mocking my related resources correctly"
     after(:build) { |sample, evaluator| sample._cached_relationship(:sample_metadata) { evaluator.sample_metadata } }
   end
 

spec/factories/tag_factories.rb

@@ -12,13 +12,23 @@ def generate_oligo
   factory :v2_tag, class: Sequencescape::Api::V2::Tag do
     skip_create
 
+    sequence(:map_id) { |i| i }
     sequence(:oligo) { |_index| generate_oligo }
-    tag_group { create :v2_tag_group }
+    tag_group { create :v2_tag_group, v2_tags: [instance] }
   end
 
-  factory :v2_tag_group, class: Sequencescape::Api::V2::Tag do
+  factory :v2_tag_group, class: Sequencescape::Api::V2::TagGroup do
     skip_create
 
+    transient { v2_tags { [] } }
     sequence(:name) { |index| "TagGroup#{index}" }
+    tags { v2_tags.map { |t| { index: t.map_id, oligo: t.oligo } } }
+
+    factory :v2_tag_group_with_tags do
+      transient do
+        size { 96 }
+        v2_tags { (1..size).map { |i| create(:v2_tag, map_id: i, tag_group: instance) } }
+      end
+    end
   end
 end

spec/factories/tag_layout_template_factories.rb

@@ -1,6 +1,29 @@
 # frozen_string_literal: true
 
 FactoryBot.define do
+  factory :v2_tag_layout_template, class: Sequencescape::Api::V2::TagLayoutTemplate do
+    skip_create
+
+    uuid
+    sequence(:name) { |index| "TagLayoutTemplate#{index}" }
+    direction { 'column' }
+    walking_by { 'wells of plate' }
+    transient do
+      tag_group { create :v2_tag_group_with_tags }
+      tag2_group { nil }
+    end
+
+    # See the README.md for an explanation under "FactoryBot is not mocking my related resources correctly"
+    after(:build) do |tag_layout_template, evaluator|
+      tag_layout_template._cached_relationship(:tag_group) { evaluator.tag_group } if evaluator.tag_group
+      tag_layout_template._cached_relationship(:tag2_group) { evaluator.tag2_group } if evaluator.tag2_group
+    end
+
+    factory :v2_dual_index_tag_layout_template do
+      transient { tag2_group { create :v2_tag_group_with_tags } }
+    end
+  end
+
   # API V1 tag layout template. The inheriting factories set up the patterns
   # commonly seen by Limber
   factory :tag_layout_template, class: Limber::TagLayoutTemplate, traits: [:api_object] do

spec/factories/tube_factories.rb

@@ -119,7 +119,7 @@
       purpose { create :v2_purpose, name: purpose_name, uuid: purpose_uuid }
     end
 
-    # Mock the relationships. Should probably handle this all a bit differently
+    # See the README.md for an explanation under "FactoryBot is not mocking my related resources correctly"
     after(:build) do |asset, evaluator|
       asset._cached_relationship(:purpose) { evaluator.purpose }
       ancestors_scope = JsonApiClient::Query::Builder.new(Sequencescape::Api::V2::Asset)

spec/factories/tube_rack_factories.rb

@@ -34,6 +34,7 @@
     created_at { '2017-06-29T09:31:59.000+01:00' }
     updated_at { '2017-06-29T09:31:59.000+01:00' }
 
+    # See the README.md for an explanation under "FactoryBot is not mocking my related resources correctly"
     after(:build) do |tube_rack, evaluator|
       Sequencescape::Api::V2::TubeRack.associations.each do |association|
         tube_rack._cached_relationship(association.attr_name) { evaluator.send(association.attr_name) }

spec/factories/well_factories.rb

@@ -105,6 +105,7 @@
     sub_pool { nil }
     coverage { nil }
 
+    # See the README.md for an explanation under "FactoryBot is not mocking my related resources correctly"
     after(:build) do |well, evaluator|
       well._cached_relationship(:qc_results) { evaluator.qc_results || [] }
       well._cached_relationship(:aliquots) { evaluator.aliquots || [] }
@@ -273,8 +274,8 @@
     sample { create :v2_sample, sample_attributes }
     request { outer_request }
 
+    # See the README.md for an explanation under "FactoryBot is not mocking my related resources correctly"
     after(:build) do |aliquot, evaluator|
-      # Set up relationships downstream
       Sequencescape::Api::V2::Aliquot.associations.each do |association|
         aliquot._cached_relationship(association.attr_name) { evaluator.send(association.attr_name) }
       end