Enabling publishing of library asserts to stock_resource table.#5430
Conversation
…or library asset types.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #5430 +/- ##
===========================================
- Coverage 87.40% 87.38% -0.02%
===========================================
Files 1457 1457
Lines 32705 32734 +29
Branches 3423 3427 +4
===========================================
+ Hits 28586 28606 +20
- Misses 4104 4113 +9
Partials 15 15 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Some notes on the implementationTip This comment is a way for myself to keep copious notes regarding this story. So, this comment is primarily for my benefit but I hope this would be useful for others and for code reviews as well! This is going to be a long comment so brace yourselves 🚤
That is essentially the solution for the first acceptance criteria. For the second part (i.e., changing the
It would be worthwhile to mention how the Procedural flow for sending receptacles to
|
… sequencing source labware
…type assignment for library plates
…usting item retrieval
…broadcast_consumer
…t subject type assignment for library plates
…a-developer-ben-i-would-like-sample-manifest-library-assets-to-appear-in-the-stock_resource-table
…-like-sample-manifest-library-assets-to-appear-in-the-stock_resource-table
…re type for library plates
…n-i-would-like-sample-manifest-library-assets-to-appear-in-the-stock_resource-table' into y25-544-and-697
…-like-sample-manifest-library-assets-to-appear-in-the-stock_resource-table
…97---as-a-developer-ben-i-would-like-sample-manifest-library-assets-to-appear-in-the-stock_resource-table
…a-developer-ben-i-would-like-sample-manifest-library-assets-to-appear-in-the-stock_resource-table
…-like-sample-manifest-library-assets-to-appear-in-the-stock_resource-table
…a-developer-ben-i-would-like-sample-manifest-library-assets-to-appear-in-the-stock_resource-table
…n-i-would-like-sample-manifest-library-assets-to-appear-in-the-stock_resource-table' into 5382-y25-697---as-a-developer-ben-i-would-like-sample-manifest-library-assets-to-appear-in-the-stock_resource-table
…-like-sample-manifest-library-assets-to-appear-in-the-stock_resource-table
BenTopping
left a comment
There was a problem hiding this comment.
Thanks for the issue notes and description. Couple small comments, my main concern is around the library_plate_well logic.
| class Core < SampleManifest::PlateBehaviour::Base | ||
| include SampleManifest::CoreBehaviour::LibraryAssets | ||
|
|
||
| def stocks? |
There was a problem hiding this comment.
Could you update this line https://github.com/sanger/sequencescape/blob/develop/app/models/sample_manifest/core_behaviour.rb#L76 instead of overwriting each instance separately?
| { root => render_class.to_hash(target), 'lims' => configatron.amqp.lims_id! } | ||
| message = render_class.to_hash(target) | ||
| Rails.logger.info("Publishing message: #{message}") | ||
| { root => process_receptacles(message), 'lims' => configatron.amqp.lims_id! } |
There was a problem hiding this comment.
Is this messenger just used for receptacles? If its a shared class Im not sure this logic should be here.
There was a problem hiding this comment.
No it is not. It is a shared class. The problem is that the register_stock! method pushes messages to the broadcast queue with the Messenger model (e.g., [Messenger, 12123]). It doesn't push messages like [Well, 12123] which makes the whole thing a bit weird and difficult. It felt weird having to put this logic here but I couldn't figure out any other way.
There was a problem hiding this comment.
I will try to instantiate the wells with the subject_type = "libarary_plate_well" upon manifest upload.
| return message unless receptacle_target? | ||
|
|
||
| asset_type = fetch_asset_type | ||
| message['labware_type'] = 'library_plate_well' if library_plate?(asset_type) |
There was a problem hiding this comment.
is library_plate_well the only affected labware type here, are the other library types okay?
…a-developer-ben-i-would-like-sample-manifest-library-assets-to-appear-in-the-stock_resource-table
Closes #
Changes proposed in this pull request
stock?method to returntrueforLibraryAsset.messenger.rbto set thelabware_typein the MLWH message.Instructions 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