Y25-263 - Bug - fix to various classes to facilitate loop back to XP plate - #5982
andrewsparkes wants to merge 17 commits into
Conversation
…te a new multiplexing submission
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #5982 +/- ##
===========================================
- Coverage 83.79% 83.65% -0.15%
===========================================
Files 1297 1298 +1
Lines 31820 31827 +7
Branches 3520 3521 +1
===========================================
- Hits 26665 26625 -40
- Misses 4336 4382 +46
- Partials 819 820 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
StephenHulme
left a comment
There was a problem hiding this comment.
It looks good, a fair bit of complexity there but thank you for the good doc-strings they make it much easier to understand.
…for request with transfer request submission id
KatyTaylor
left a comment
There was a problem hiding this comment.
Sorry for slowness, finding the code quite dense (not very familiar with this area) and getting distracted by other things. I've only looked at the first class in detail, but have a few concerns. Let me know if you want to talk through. Obviously seems to be behaving well in your tests though 🤔 Thanks.
yoldas
left a comment
There was a problem hiding this comment.
There are a few naming suggestions in other comments. The tests cover the path and the scenario from the ticket (old completed request + new active one). I have highlighted a few hot spots with notes. I think this PR resolves the issue.
| ACTIVE_STATES = %w[pending started passed qc_complete].freeze | ||
|
|
||
| # States considered to be transferable | ||
| TRANSFERABLE_STATES = %w[pending started].freeze |
There was a problem hiding this comment.
note: this is no longer used anywhere
yoldas
left a comment
There was a problem hiding this comment.
Yes, the latest change that adds LB Cap Lib PCR-XP record in SS will set the purpose type correctly for a fresh database. For existing databases, it needs a post-deployment procedure to set it.
To fix bug preventing creation of additional Multiplexing submissions from XP plate.
Closes #5981
Changes proposed in this pull request
Instructions for Reviewers
The story description explains how to replicate the original issue.
Essentially you run a pipeline once, then want to go back to the XP plate and create a new Multiplexing and Seq submission.
It was failing because it was using the submission id from the first run (via the outer request) when trying to create transfer requests, instead of filtering to the newer, active submission with it's requests in well.requests_as_source. This causes it to try to re-use the MX pool tube from the first run, and triggers a tag clash error as it tries to put new aliquots (with the same tags) into that previously used tube.
The fixes in this PR look for that new active submission, and defaults to the current way of looking in outer request if not present.
Integration test PR
https://gitlab.internal.sanger.ac.uk/psd/integration-suite/-/merge_requests/328