Y24-190-3: Use SS v2 for TagLayoutTemplates #1835
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop-Y24-190 #1835 +/- ##
===================================================
+ Coverage 91.40% 91.47% +0.06%
===================================================
Files 377 379 +2
Lines 7657 7763 +106
===================================================
+ Hits 6999 7101 +102
- Misses 658 662 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| #### 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. |
There was a problem hiding this comment.
Thanks for writing this documentation.
Generally these are quite dynamic so you don't need to explicitly specify every property the API will respond with.
Not quite sure what this sentence means. How will the record have the property if you haven't specified it?
There was a problem hiding this comment.
Magic. Basically the resources mostly look like this:
# frozen_string_literal: true
class Sequencescape::Api::V2::Project < Sequencescape::Api::V2::Base
end
but even so, that's just a template for the client gem to know that it should load a resource from that endpoint and the attributes from the API will be available to you when you use this class. They are dynamically assigned.
The exception is for relationships. If the API supports a resource having associated other resources, you do need to specify has_one, has_many etc in order for it to make those accessible via the client gem.
You can also add custom logic to these, like I did for TagLayoutTemplate, so you can change the named algorithms into implementations of that algorithm on the Limber side.
| # 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. |
There was a problem hiding this comment.
I thought you were planning to redesign it so that you could call create! on TagLayout instead, and that pulled info from the TagLayoutTemplate?
There was a problem hiding this comment.
Yes, but we don't have TagLayout in V2 yet. I plan to do that in a soon pull request. This comment was written before we came across that solution. I'm not sure it's worth updating since it's coming out in the next week.
| 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 |
There was a problem hiding this comment.
So this method and the one below are taken from the existing app > models > limber > tag_layout_template.rb file... is that file needed any more?
There was a problem hiding this comment.
I'm leaving removal of V1 until the end. It's still unclear how tightly tied they are into the system and I do still need it until we remove the TODO items.
| sequence(:name) { |index| "TagGroup#{index}" } | ||
| tags { v2_tags.map { |t| { index: t.map_id, oligo: t.oligo } } } |
There was a problem hiding this comment.
So v2_tags get passed into this factory as instances of Sequencescape::Api::V2::Tag, but this data structure seems to change them into a simple hash? Is that how it appears in real API results too?
There was a problem hiding this comment.
Yes. I wouldn't have done it this way as it's not how V1 works, but the V2 endpoint was pre-existing with this and if I change it, who knows what else might break.
One of the biggest (and possibly least obvious) issues with APIs is that they're even harder to change than other aspects of systems. You're setting up a contract with people outside your team's scope for how they can interface with your application. Making decisions about API implementations should never be taken lightly. Deploying a bad decision has to be maintained until the next version of the API.
Closes #1818
Changes proposed in this pull request
Instructions for Reviewers
[All PRs] - Confirm PR template filled
[Feature Branches] - Review code