feat(directory): add studies associated to collections

svituz commented 4 months ago

What are the main changes you did:

I added, to the Biobank Directory module, the possibility of linking a clinical study to one or more collections. The clinical study is the context in which the collection of samples was generated. This PR is needed to link BBMRI ERIC collections with ECRIN MDR clinical studies

The PR implements the following changes:

Model:
- adds the Study entity with some attributes for the studies
- adds to the Collection entity the attribute study that references a study
- changes the attribute national_node of the AlsoKnowIn entity, to be optional. Indeed, in case the also_known entity is used to store information for a study, the national node is null, since the concept is not related to a study
UI
- adds to the Collection page, in the info card on the right, a link to the details of the Study, if present
- adds a Study page with the information of the Study. The page contains study attributes, the list of collections with the samples of the study, and an info card that just contains the link to the external catalog if present

how to test:

As the usual biobank_directory:
- run molgenis-emx2
- create a new database named Directory, using "Biobank_Directory" as a template setting "load data" to true
- navigate to the details of the first collection of Biobank 1 (Collection 1 of biobank 1)
- in the info card on the right there is a "Study" section with a link that brings to the Study details page

todo:

[ ] updated docs in case of new feature
[ ] added/updated tests
[ ] added/updated testplan to include a test for this fix, including ref to bug using # notation

sonarcloud[bot] commented 3 months ago

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

esthervanenckevort commented 2 months ago

The data model should also include a National Node column for studies. @dtroelofsprins can explain.

dtroelofsprins commented 2 months ago

Discussed this with Esther.

In your comments you say that studies can contain collections from multiple national nodes and therefore no NN can be attached to it. We have similar cases with 'inter'national Networks, and these are included in a EU national node. We would like to propose to handle the studies in a similar way, so then the required=false in the AlsoKnownIN can be true again and a national_node column should be added to the Studies table.
Second argument for this is, that it can be that national nodes want to include studies by themselves and that cannot be done on the proposed Studies table itself, but needs to go through a NN's own studies table which are copied into the ERIC studies tables overnight => and for this the NN column is relevant.
As a result of this I think it might also be good to broaden the description of the study ID in the molgenis.csv to a more general one as it's now only focussing on ECRIN.
And related to that: In your example you have id: bbmri_eric:ID:DE:112233, I would suggest to keep this in line with the IDs of the other tables, so something like: bbmri-eric:studyID:DE_112233

svituz commented 2 weeks ago

@dtroelofsprins I removed the study from the card details. You were right it was unnecessary. Regarding the breadcrumbs, I thought about adding the collection to the path. The problem, though, is that the relation Study-Collections is 1-N, so a study can be associated with multiple collections. When navigating from the collection page it is possible to know the collection but this is not possible when accessing the page via direct URL. So to keep the same behavior in the two cases I preferred not to include the breadcrumb in all situations. The same thing happens for the Networks.

dtroelofsprins commented 2 weeks ago

@dtroelofsprins I removed the study from the card details. You were right it was unnecessary. Regarding the breadcrumbs, I thought about adding the collection to the path. The problem, though, is that the relation Study-Collections is 1-N, so a study can be associated with multiple collections. When navigating from the collection page it is possible to know the collection but this is not possible when accessing the page via direct URL. So to keep the same behavior in the two cases I preferred not to include the breadcrumb in all situations. The same thing happens for the Networks.

Ah of course you're right about that, studies can have more collections, which make me wonder: currently a collection can only be in one study, is this indeed true? Or could it potentially be linked to multiple studies?

esthervanenckevort commented 2 weeks ago

Ah of course you're right about that, studies can have more collections, which make me wonder: currently a collection can only be in one study, is this indeed true? Or could it potentially be linked to multiple studies?

A collection can be linked to multiple studies

esthervanenckevort commented 2 weeks ago

Can you have a look at how you can harmonise the data model with MIABIS v3 Study (researchresource).

https://docs.google.com/spreadsheets/d/1RrNHSNWotFl0F4P3u2cb-o_z-20bJGUn/edit?gid=1358611155#gid=1358611155

svituz commented 2 weeks ago

@esthervanenckevort The harmonization with MIABIS 3 needs further discussions with the MIABIS team, so it was decided to use the current model to start linking the MDR with the Directory and leave the harmonization for future work.

dtroelofsprins commented 1 week ago

Ah of course you're right about that, studies can have more collections, which make me wonder: currently a collection can only be in one study, is this indeed true? Or could it potentially be linked to multiple studies?

A collection can be linked to multiple studies

If so, then the datatype should be ref_array instead of ref

svituz commented 1 week ago

I changed the datatype of studies to ref_array as suggested and changed the frontend to support multiple studies

dtroelofsprins commented 3 days ago

I changed the datatype of studies to ref_array as suggested and changed the frontend to support multiple studies

Thanks! One (last) small remark, as multiple studies now can be linked to the collections it would perhaps be better to name the column studies instead of study in the collections table?

sonarcloud[bot] commented 22 hours ago

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

molgenis / molgenis-emx2