bihealth / sodar-server

SODAR: System for Omics Data Access and Retrieval
https://github.com/bihealth/sodar-server
MIT License
14 stars 3 forks source link

Process not linked to existing node by name when adding new row in sheet editor #1904

Closed sellth closed 6 months ago

sellth commented 7 months ago

Assay Name can be used to define single instances of a process, i.e. individual runs of a sequencer, CyTOF, etc. For the ISA graph this is similar to pooling on a material level which means following annotations (Parameter Value, Comment) need to be the same for all rows in question. The parser correctly identifies this as invalid, but not the online samplesheets editor. This means, the user can create an invalid ISA-tab which doesn't re-validate after export & import.

Steps to reproduce

  1. Create new ISA-tab from mass_cytometry template.
  2. Fill multiple assay rows and set the same Assay Name in the Mass Cytometry process.
  3. Add different annotations for the Mass Cytometry process.
  4. Export the ISA-tab. Import the ISA-tab.
mikkonie commented 7 months ago

Thanks for the report, I will look into it.

mikkonie commented 7 months ago

I'm able to reproduce locally. While we normally detect existing nodes and update the references accordingly when creating a new row, this apparently fails to take "Assay Name" into consideration.

Some extra problems might be caused by Assay Name being empty (I guess it's optional?), but let's try fixing it with names included first..

mikkonie commented 7 months ago

Digging into it, it kind of seems this is an erroneously missing feature. Which would make it a bug :)

This also affects name fields other than Assay Name, namely all fields in the AltamISA PROCESS_NAME_HEADERS list. They are correctly tagged as process_name by SampleSheetTableBuilder, but on the client side the UI fails to consider this.

In addition to manually setting the name, this also doesn't work if we set a suffix for the column and "auto-generate" the node in enableNextNodes(). At least there it seems the relevant code is simply missing, hence "erroneously missing feature".

Tracking progress in the task list below.

Tasks

sellth commented 7 months ago

Some extra problems might be caused by Assay Name being empty (I guess it's optional?), but let's try fixing it with names included first..

Assay Name is optional. If empty, Altamisa assigns one ID per row.

mikkonie commented 6 months ago

Fixed, at least for all cases I could figure out. If there are some special cases where this still fails, we'll have to open new tickets. Sadly this isn't fully tested yet because vue & ag-grid testing is.. complicated.

While implementing this I discovered some other editor bugs/issues, one of which (#1928) should be handled in this release. Back to the drawing board..