Y24-190: Use SS API v2 for searches #2169
Conversation
It seems to be returning 100 results per page when we actually requested 30! It doesn't populate the library_type_name It doesn't populate the insert_size
harrietc52
changed the title
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop-Y24-190 #2169 +/- ##
================================================
Coverage 80.78% 80.79%
================================================
Files 481 481
Lines 18167 18173 +6
Branches 269 269
================================================
+ Hits 14677 14683 +6
Misses 3488 3488
Partials 2 2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| context 'without parameters' do | ||
| let(:search_parameters) do | ||
| { | ||
| states: %w[pending started passed qc_complete failed cancelled], | ||
| plate_purpose_uuids: %w[uuid-1 uuid-2], | ||
| show_my_plates_only: false, |
There was a problem hiding this comment.
As mentioned above, just checking show_my_plates_only has been removed because it is not needed anymore
| page: 1 | ||
| state: %w[pending started passed qc_complete failed cancelled], | ||
| purpose_name: %w[purpose-config minimal-purpose-config], | ||
| include_used: true |
There was a problem hiding this comment.
When include_used is true, is there anything else to check for?
There was a problem hiding this comment.
Could us the total_count to check how many plates are returned
There was a problem hiding this comment.
@harrietc52 looking into this
There was a problem hiding this comment.
@StephenHulme I’ve been looking into this, and I’m not sure if there is a way to access the total_count. It is added in the method which stubs find_all, and the Mock response doesnt have it in it, which I was expecting it too!
| include_used: '1' | ||
| } | ||
| } | ||
| expect(expected_search).to have_been_made.once | ||
| expected_search.once |
There was a problem hiding this comment.
Again, maybe worth checking that the correct plate is returned (sorry know this work is just to update, not add!) just a thought :)
There was a problem hiding this comment.
As above, this might be hard to confirm without some non-trivial refactoring of the test. Would definitely be a good thing to check though! Possibly a GitHub Issue?
| @@ -76,6 +76,8 @@ | |||
| # Library request with primer panel information | |||
| factory :gbs_library_request do | |||
| primer_panel | |||
|
|
|||
| after(:build) { |request, evaluator| request._cached_relationship(:primer_panel) { evaluator.primer_panel } } | |||
There was a problem hiding this comment.
Can see why this would be needed, just wondering how did it come about?
There was a problem hiding this comment.
This was from the includes refactoring to make the ongoing plates & tubes load in under 60 seconds. The code works, but the test-suite doesn't really like how JSON-API handles relationships. Fortunately there was a note in the readme.
| }, | ||
| [associated(:tube, barcode_number: 2)] | ||
| { page: 1, per_page: 30 }, |
There was a problem hiding this comment.
Again, not 100%, but not sure whether we should be passing in per_page as this is a constant, so maybe if this was something other than 30, it would still return 30 because of the constant?
There was a problem hiding this comment.
No, in this context the per_page: 30 is being used for incoming request recognition, rather than returning a hard-coded response. The mocking of the pagination response by the JsonApiClient is handled in stub_find_all_with_pagination.
| if asset.is_a?(Array) | ||
| allow(Sequencescape::Api::V2::Labware).to receive(:find).with(barcode:).and_return(asset) | ||
| else | ||
| # TODO: Y24-190 to test |
There was a problem hiding this comment.
Is this todo outstanding?
Closes #2171
Changes proposed in this pull request
show_my_plates_onlyInstructions for Reviewers
[All PRs] - Confirm PR template filled
[Feature Branches] - Review code
[Production Merges to
main]- Check story numbers included
- Check for debug code
- Check version