Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Y24-268 - Create a presenter for automated submission for WGS branch on Adp Lig plate #1982

Open
wants to merge 21 commits into
base: develop
Choose a base branch
from

Conversation

StephenHulme
Copy link
Contributor

@StephenHulme StephenHulme commented Oct 8, 2024

Closes sanger/sequencescape#4290

Changes proposed in this pull request

  • Refactors sidebar rendering
  • Adds a new PermissiveSubmissionPlatePresenter adding elements of the PermissivePlatePresenter to the SubmissionPlatePresenter
  • Adds a new permissive submission state machine
  • Rewords workflow message
  • Prevents new submissions from being created if any are currently active for a plate

Current issues

  • Pooling affects availability of Add an empty ___ plate button - I think this is expected?

Current screenshots

Screenshot 2024-10-15 at 15 57 41 Screenshot 2024-10-15 at 15 55 34 Screenshot 2024-10-15 at 15 55 46 Screenshot 2024-10-15 at 15 56 14

Previous screenshots

Screenshot 2024-10-11 at 14 27 38 Screenshot 2024-10-11 at 14 28 26 Screenshot 2024-10-14 at 12 43 29

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

Copy link

codecov bot commented Oct 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.18%. Comparing base (0c7e26e) to head (44817ff).
Report is 37 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1982      +/-   ##
===========================================
+ Coverage    77.97%   78.18%   +0.21%     
===========================================
  Files          459      466       +7     
  Lines        17699    17989     +290     
  Branches       225      225              
===========================================
+ Hits         13800    14065     +265     
- Misses        3897     3922      +25     
  Partials         2        2              
Flag Coverage Δ
javascript 69.69% <ø> (ø)
pull_request 78.05% <100.00%> (+0.08%) ⬆️
push 78.04% <100.00%> (+0.07%) ⬆️
ruby 91.16% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@StephenHulme StephenHulme marked this pull request as draft October 11, 2024 13:30
Comment on lines +66 to +73
state :passed do
include StateAllowsChildCreation
include DoesNotAllowLibraryPassing

def sidebar_partial
'submission_default'
end
end
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'submission_default' shows the newly combined sidebar

include Presenters::StateChangeless
# TODO: confirm Presenters::StateChangeless is not required here
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is almost certainly incorrect, but as StateChangeless says "Include in presenters that suppress state changes under all circumstances", this feels like a good start. If State Changes are prevented the Manual Transfer button is not displayed (and I'm assuming the plate cannot enter passed state).
I suspect the way forward is to specify under what circumstances the state may change or not.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Solved by creating a new SubmissionBehaviour in d15c05e that can be used where required

@StephenHulme
Copy link
Contributor Author

@yoldas @KatyTaylor @andrewsparkes thanks for the conversation this morning - it gave me a much better idea of what we are trying to achieve, the various ways of doing so, and why.
Would you please have a look at this PR to confirm I'm on the correct track? I've highlighted some areas of interest in the comments.

Comment on lines 4 to 5
<%# TODO: confirm that this is a suitable check and does not disrupt workflows besides LCMT WGS branch %>
<% if not presenter.labware.any_complete_requests? %>
Copy link
Contributor Author

@StephenHulme StephenHulme Oct 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How can I check and confirm this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see any checking of requests working on a labware created via permissive presenters, because I think that labware will be in pending state and have no aliquots/samples or requests because nothing has been transferred into it yet.
(we discussed this the other day, but I forget the conclusion! am I wrong?)

In theory a plate in a pipeline could have both completed and active requests. e.g. Bespoke chromium splits into 3 library preps each with a different submission and requests. So, including the closed request for the DNA prep part of the pipeline that ends with creation of the branch point plate, the branch plate could have 3 more to make 4 in total, e.g. 2 completed sets of library prep requests per well plus a third set of library prep requests still active (3 different library types). (NB. that one does not have automated submissions)

@StephenHulme StephenHulme marked this pull request as ready for review October 14, 2024 15:01
@StephenHulme
Copy link
Contributor Author

From issue story:

We should restrict submission button to only display if there’s not already an active submission of configured type

Copy link

codeclimate bot commented Oct 15, 2024

Code Climate has analyzed commit 44817ff and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (50% is the threshold).

This pull request will bring the total coverage in the repository to 91.2% (0.1% change).

View more on Code Climate.

@StephenHulme StephenHulme requested review from andrewsparkes and KatyTaylor and removed request for andrewsparkes and KatyTaylor October 16, 2024 10:15
Copy link
Member

@andrewsparkes andrewsparkes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Just check Submission values for WGS.

template_name: 'Limber-Htp - LCM Triomics WGS'
allowed_extra_barcodes: false
request_options:
# TODO: replace with the correct request options
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assume you've asked Scott about those fragment sizes?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Y24-268 - Create a presenter for automated submission for WGS branch on Adp Lig plate
3 participants