DPL-057 Allow editing of batch information [OKR]

[JamesGlover]

User story We need to be a bit careful about how we approach this one, as certain piplines are very unhappy if you try to edit completed steps. However there is no real reason for sequencing to be one of these. However well will need to ensure:

• [ ] 'Editing' a descriptor should create a new set of descriptors
• [ ] We must always use the latest set of descriptors (such as in flowcell messages)

We should also:

• [ ] Expose historic data clearly where appropriate
• [ ] Allow the addition of RT numbers to edited information

Neil S:

In the process of loading flowcells most details (events) are manually entered. Some are for 'in team' tracking e.g. Operator, Pipette Carousel, PhiX lot whilst others are required for downstream analysis e.g. phix barcode, loading concentration, workflow

Presently once the batch is released these events cannot be updated by the team and require the generation of an RT ticket to update the event(s). There is potential for downstream analysis to be delayed.

The number of RT tickets generated for this action is consistently the highest in any month than any other area we support.

Who are the primary contacts for this story e.g. John S (don't include surnames in public repos)

Acceptance criteria To be considered successful the solution must allow:

• [ ] add a list of acceptance criteria here
• [ ] ...

Dependencies This story is blocked by the following dependencies:

• sanger/#

References This story has a non-blocking relationship with:

• sanger/#

Additional context Add any other context or screenshots about the feature request here.

[JamesGlover]

PR with annotations of existing code: https://github.com/sanger/sequencescape/pull/3242

[JamesGlover]

Improve the batch summary page to better handle updated data:

Before After

[neilsycamore]

James I can see pipette carousel in the view twice. Removing it from the top row would free up some space

[JamesGlover]

Its recorded for different steps, they don't necessarily use the same carousel each time. (Sequencescape even supports different carousels for each request, but I'm not sure if this functionality is ever used.)

[KatyTaylor]

I think a nice addition to the UI you've posted would be for there to be a label somewhere saying what the things in the cards / tables are. Our expert users will know, but new users (and developers!) might not. I think the cards are lanes, and each table is an event, or something? Can the word 'lane' and 'events' go somewhere? Apologies for annoying not-related-to-your-task request :)

Edit: also not quite sure about the table format - are we expecting there to be multiple rows in the tables? If not, might be better as cards or something?

[JamesGlover]

I did try a descriptive list approach, but it was a bit hard to control space, so tables were an attempt to keep things compact. I can prototype another version and see what Tris thinks.

[JamesGlover]

Quickly prototyped a cards version. For some batches there will be more columns...

[JamesGlover]

Batch with more steps

[JamesGlover]

The same batch with a table view

[JamesGlover]

Card deck at the request level It completely falls over for 8 lane batched though, and still looks cluttered if grouped by 4.

[JamesGlover]

Tris likes the first protoype, but agrees that it falls apart with more columns. I'll look at improving that approach. A few ideas:

• Limit ourselves to just 2-3 cards per line, to avoid getting too compact
• Put the field titles above their values, rather than besides them
• Drop the font size (As Tris suggests)
• Could also consider hiding ‘empty’ fields by default.

I'm probably going to avoid the 'masonry' style approach of tessellating the columns, as:

• Support has been dropped from bootstrap 5
• It can sometimes split a card over multiple columns
• While pretty, I think it overall reduces clarity with respect to card order, and even card divisions.

Tris also suggested abbreviating some of the fields, but as they get pulled straight from the database, this isn't really viable without changing the historical data.

[JamesGlover]

Tris was worried that hiding empty fields could make it unclear if a field was missed.

[JamesGlover]

Got something that seems to work. Not quite as responsive as I'd like, due to limitations in bootstrap card groups. But that does seem to have been improved in bootstrap 5, so might be able to revisit it soom.

[JamesGlover]

Question to Tris:

I’ve got a question about some existing functionality in the Sequencing pipelines, that I suspect is almost entirely unused.

Most pipeline steps can be in one of two modes: Per item mode (Mostly for setting concentrations) where each request has its own boxes Per batch mode where a single set of fields are shared for the entire batch (most other steps)

PhiX has its own behaviour, and I’m not really covering it here.

In both modes it is possible to de-select requests as the top of the page, and only apply the values you filled in to the rest of the requests within the batch. When you do this, you’ll be returned to the same page.

In the case of per-batch mode, the fields will be filled in with the values on the previous page. You then need to adjust the selected requests, update the details as required and click update again. Once all the requests have been updated in this way, you get taken to the next page.

This approach already feels clunky, and will be incredibly confusing if you wish to edit a request. As a result I’m proposing the following:

If you choose to only update some requests in the batch, the screen will flip to per-item mode for the remaining requests. Per item mode will also be selected if you choose to edit a batch where the information is different for the various requests.

One slight issues this raises though, is whether we should still populate the empty fields with the previous contents:

Advantages: Less typing, especially if its only one field that differs. Disadvantages: More error prone, as it isn’t immediately apparent which fields haven’t been filled yet.

Do you have a preference? Keeping them blank is marginally simpler from our perspective, although more from a risk-of-bugs perspective than actual time to implement.

Running with not-populate for now