search

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

Samplesheet editor cannot add existing rows to pooled material #1876

Closed sellth closed 8 months ago

sellth commented 8 months ago

Pooling samples is a valid use case of ISA-tabs and supported by the parser. In such a case, multiple assay rows would share a common material, such as Library Name, Extract Name, or * Data File. SODAR's samplesheet editor currently does not allow this and responds with an "Invalid value" error.

How to Reproduce

  1. Edit Sheets
  2. Go to material column in assay.
  3. Change 2nd row to value of first row.

grafik

Caused by this if block: https://github.com/bihealth/sodar-server/blob/2bc609331f218d963413ed53764b2fe4ff163bed/samplesheets/vueapp/src/components/editors/DataCellEditor.vue#L144-L151

Edit: This is only true for editing existing rows, but not for the "Insert row" functionality. Still an issue, because we often give partially pre-filled samplesheets to users.

mikkonie commented 8 months ago

This is not a bug but a feature which is not currently supported. For now, new rows can be inserted to get around this.

This is related to #852 and #855, will look into it some time after #994.

sellth commented 8 months ago

Maybe it's a missing feature of the editor, but the bug is in the UX then. @mmilek regularly encounters this with his users and agrees that it should be addressed. Reopening because it makes a different point compared to #855 .

mmilek commented 8 months ago

agree with @sellth here. sodar users should be able to input non-unique extract name (last material) via the edit sheets functionality. Otherwise, I always have to manually update the sample sheets and replace the isa-tab...

mikkonie commented 8 months ago

agree with @sellth here. sodar users should be able to input non-unique extract name (last material) via the edit sheets functionality. Otherwise, I always have to manually update the sample sheets and replace the isa-tab...

Yes, this should be allowed and it has been a plan to implement this all along. Pull requests are welcome :)

mikkonie commented 8 months ago

Reopening because it makes a different point compared to #855 .

To me it sure looks like a case of #855 (not supporting changing the node to refer to an already existing one). Please provide me an example ISA-Tab where this happens (privately and securely, if it's one containing actual data) and I'll see if this is some special case instead.

mikkonie commented 8 months ago

If this is indeed an ongoing problem with actual users, I will look into prioritizing this. It's slightly non-trivial thanks to the complexity of ISA-Tab, but I have an idea how to (hopefully) implementing it without too many problems, even if it has to be done before #994.

mmilek commented 8 months ago

hi @mikkonie, there is one user that has these issues but they work on many new projects so the problem reappears about once a month. not super urgent i guess but it would be nice to solve this... here is an example project in sodar where the last material extract name cannot be changed to the same string over all samples.... https://sodar.bihealth.org/samplesheets/25b47283-04d5-4027-a0c0-417359dfa9b7#/study/15aac681-9403-4423-ba55-1106286b21b7

mikkonie commented 8 months ago

Ok, so I took a look and this is absolutely a dupe of #855. I'd close this issue, but apparently that will just lead into it being immediately reopened by someone who is not the owner of this repo :) So let's just leave this open then.

sellth commented 8 months ago

duplicate of #855