sanger / sequencescape

Web based LIMS
MIT License
87 stars 33 forks source link

GPL-570 Refactor cherrypick task #2831

Closed KatyTaylor closed 2 months ago

KatyTaylor commented 4 years ago

User story As a developer I would like to refactor the code in the cherrypick_task class, so that it's easier to maintain. We found it difficult to read and modify while developing these two stories, during the 'bulk cherrypicking' epic - https://github.com/sanger/sequencescape/issues/2763 & https://github.com/sanger/sequencescape/issues/2783

The main method to refactor is called perform_pick. It adds the wells onto the destination plates in one big loop. This gets complicated when certain wells are pre-allocated, like through controls, templates and partials. You have to do things like 'look ahead' in the loop and see if you have enough wells left to fit all the remaining controls in. We think it would be simpler to fill in the pre-allocated wells first, and then go through and fill in the normal samples.

Who are the primary contacts for this story Eduardo, Katy

Acceptance criteria To be considered successful the solution must allow:

JamesGlover commented 4 years ago

Have a long term desire to reimplement cherrypicking to get away from the tangled batch/pipelines/workflows stuff

1) Add the ability to form 'pick-lists' easily, without the need to go through all the paperwork of making submissions. 2) Avoid the need to go via the tangled batch/pipleines/workflows stuff, either as a new add, or just something else. 3) Improve flexibility, such as allowing changes right up until actually picked.

If we approach this sensibly we'll have some nice components to use in other contexts to allow gradual improvements of cherrypicking.

emrojo commented 4 years ago

First version:

Second version:

Third version (optional):

Fourth (optional):

rl15 commented 4 years ago

On 4th August agreed that as a team to Book a meeting Generate options on what we could do Agree as a team one of the options Do it.

JamesGlover commented 3 years ago

CherrypickTask#can_link_directly is currently set to return false, as the current WorkflowsController tightly couples the first and second steps. This is as performing the previous action, and rendering the next are performed in the same controller action, and share instance variables. These variables then get rendered as hidden fields in the CherrypickTask template.

We could break this coupling by storing all the results of the form, not just the robot id in the descriptor. You'd still need to fill out the previous step once, but then could skip to the second easily.

Mostly though the advantage of this is that stage can be separated into two distinct actions, a create for performing the action, and a get for rendering it. (Other pipelines permitting)