feat(directory): add studies associated to collections #3837

svituz · 2024-06-03T09:31:59Z

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

sonarqubecloud · 2024-06-11T08:47:57Z

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

Please implement a feature flag to make it possible to enable / disable this feature

esthervanenckevort · 2024-07-15T09:30:01Z

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

dtroelofsprins · 2024-07-17T10:39:25Z

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

chinook25

Please make sure new files are written in typescript and for .vue files use the setup tag.

apps/directory/src/property-config/initialCollectionColumns.js

chinook25 · 2024-07-26T10:02:13Z

apps/directory/src/stores/studyStore.js

+  function getStudyColumns() {
+    const properties = studyColumns
+      .filter((column) => column.column)
+      .flatMap((studyColumn) => studyColumn.column);
+
+    const rangeProperties = studyColumns.filter(
+      (column) => column.type === "range"
+    );
+
+    for (const property of rangeProperties) {
+      properties.push(property.min, property.max, property.unit_column);
+    }
+
+    return properties;
+  }
+


Please split this out to a a util file and write a unittest for it.

This is meant for internal use of studyStore so I think it fits here, I removed it from the export indeed. The collectionStore also has a similar function.
Regarding the unit test, I couldn't find any unit test in the app. Can you point me to their location?

apps/directory/src/functions/viewmodelMapper.js

apps/directory/src/views/StudyReport.vue

apps/directory/src/components/report-components/StudyReportInfoCard.vue

apps/directory/src/views/StudyReport.vue

chinook25

Works for me! Just tidy up the code a bit (see comments) and I think it's done.

chinook25 · 2024-09-17T12:53:49Z

apps/directory/vite.config.js

@@ -5,7 +5,7 @@ import { createHtmlPlugin } from "vite-plugin-html";
 import monacoEditorPlugin from "vite-plugin-monaco-editor";

 const HOST =
-  process.env.MOLGENIS_APPS_HOST || "https://bbmri-emx2-test.molgeniscloud.org";
+  process.env.MOLGENIS_APPS_HOST || "http://localhost:8080/" // "https://bbmri-emx2-test.molgeniscloud.org";


don't forget to undo

chinook25 · 2024-09-17T12:59:08Z

apps/directory/src/stores/studyStore.ts

+  }
+
+  async function getStudyReport(id: string) {
+    console.log(getStudyColumns());


don't forget to remove

chinook25 · 2024-09-17T13:34:10Z

apps/directory/src/stores/studyStore.ts

+      .orderBy("Studies", "id", "asc")
+      .where("id")
+      .like(id);
+    console.log(studyReportQuery);


also remove

esthervanenckevort · 2024-09-18T07:30:21Z

data/biobank-directory/molgenis.csv

+Studies,,title,Title,text,,true,,The title of the study,,,,,,,,,
+Studies,,description,Description,text,,false,,A text describing the study,,,,,,,,,
+Studies,,type,Type,string,,false,,"The type of study (Observational, Interventional). This is a string imported from ECRIN",,,,,,,,,


Could type not be a categorical value? This would make manual data entry easier

old version

dtroelofsprins

Thanks for making all the adjustment after our first review, in general data model etc looks fine now, except from some small remarks => see comments in the files.

Also the app looks fine:

Link to study card in collection detail view works OK.
Separate study card looks fine => All information that can be entered in the studies table is displayed if available and if not then it's also not displayed.

One small remark: why is study id displayed in the details on the collection card itself?
. Because the information is also available via the right pane? Like f.e. biobanks and for networks these IDs are also not in the collection details.

Second thing is more a feature request and I have no idea if it's difficult to implement, but I think it might be nice if you could navigate from the study card back to the collection card. So that the breadcrumb gets similar to the one of the collection card:

But then "Back to the catalogue"/"Collection1 of biobank1"/Dummy study? Instead of "Back to the catalogue"/Dummy study?

dtroelofsprins · 2024-09-18T11:10:22Z

data/biobank-directory/molgenis.csv

+Persons,,national_node,National Node,ref,,true,,The person originates from this national node.,,NationalNodes,,${description},,,,,
+Studies,,,,,,,,Table with information about the studies in which context the collection was generated,,,,,,,,,
+Studies,,id,ID,string,1,true,,"Unique ID of the study, prefixed with bbmri-eric:ID:<ecrin_mdr_id>",,,,,,,,,


Please don't make the ID prefixed with bbmri-eric:ID, but preferably something like bbmri-eric:studyID.
Also remove <ecrin_mdr_id> or include that as a suggestion. As the use-case is now ECRIN, but in the future studies from other sources might be included.

removed all ECRIN reference in the model

dtroelofsprins · 2024-09-18T11:10:37Z

data/biobank-directory/molgenis.csv

@@ -115,6 +115,7 @@ Collections,,access_joint_project,Access via Join Projects to,ontology_array,,fa
 Collections,,access_description,Access Description,text,,false,,Short description of the access rules.,,,,,,,,,
 Collections,,access_uri,Access URI,hyperlink,,false,,URI describing the access policy.,,,,,,,,,
 Collections,,sop,PD/SOPs,ontology_array,,false,,Availability of Process Descriptions (PDs) and/or Standard Operating Procedures (SOPs).,DirectoryOntologies,SOPs,,${label},,,,,
+Collections,,study,Study,ref,,false,,A link to a Study during which the collection was generated,,Studies,,,,,,,


Adding refLabel = ${title} will show the title instead of the id in the collections table

svituz · 2024-09-19T08:49:52Z

@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 · 2024-09-19T11:05:29Z

@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?

dtroelofsprins · 2024-09-19T11:08:01Z

data/biobank-directory/molgenis.csv

@@ -115,6 +115,7 @@ Collections,,access_joint_project,Access via Join Projects to,ontology_array,,fa
 Collections,,access_description,Access Description,text,,false,,Short description of the access rules.,,,,,,,,,
 Collections,,access_uri,Access URI,hyperlink,,false,,URI describing the access policy.,,,,,,,,,
 Collections,,sop,PD/SOPs,ontology_array,,false,,Availability of Process Descriptions (PDs) and/or Standard Operating Procedures (SOPs).,DirectoryOntologies,SOPs,,${label},,,,,
+Collections,,study,Study,ref,,false,,A link to a Study during which the collection was generated,,Studies,,${title},,,,,


Sorry, forgot to mention it earlier, but now the study is shown under the Policies header. I think it's better to move this to the "Belongs to' one. So after also_known.

esthervanenckevort · 2024-09-19T11:24:26Z

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 · 2024-09-19T11:27:10Z

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 · 2024-10-08T11:01:39Z

@dtroelofsprins can you check this again?

dtroelofsprins · 2024-10-09T09:52:04Z

@dtroelofsprins can you check this again?

I'm a bit confused now, not sure whether you've seen the comment I made last week?

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?

Looking at the molgenis.csv the column in collections.csv is still named Study, instead of Studies, but I do see that new commits were pushed recently.

…ciate it to Collection

…es in the Directory

…ge the id the dummy study according to the review

…uired and adds it to Study table

…ard. Left only in the right pane

…rence to ECRIN in the comments and id

…d changed the frontend accordingly

sonarqubecloud · 2024-10-21T08:44:10Z

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

svituz · 2024-10-21T08:57:02Z

@dtroelofsprins can you check this again?

I'm a bit confused now, not sure whether you've seen the comment I made last week?

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?

Looking at the molgenis.csv the column in collections.csv is still named Study, instead of Studies, but I do see that new commits were pushed recently.

I must have missed the comment. I changed the name of the column to studies now

dtroelofsprins

Thanks for the latest change.
As far as I can see all things look and work fine now 🙂

esthervanenckevort requested review from dtroelofsprins and konstantina-gkp June 4, 2024 11:14

svituz force-pushed the feat/add_studies_in_biobank_directory branch from 5abb2e3 to 69e48be Compare June 11, 2024 08:26

esthervanenckevort previously requested changes Jul 2, 2024

View reviewed changes

dtroelofsprins changed the title ~~Biobank Directory: add studies associated to collections~~ Biobank Directory: add studies associated to collections ==== DISCUSS WITH ESTHER ==== Jul 12, 2024

esthervanenckevort changed the title ~~Biobank Directory: add studies associated to collections ==== DISCUSS WITH ESTHER ====~~ Biobank Directory: add studies associated to collections Jul 15, 2024

mswertz changed the title ~~Biobank Directory: add studies associated to collections~~ feat(directory): add studies associated to collections Jul 15, 2024

EleanorHyde-UMCG changed the title ~~feat(directory): add studies associated to collections~~ feat(directory): add studies associated to collections - blocked on changes by Italian team Jul 24, 2024

chinook25 changed the title ~~feat(directory): add studies associated to collections - blocked on changes by Italian team~~ feat(directory): add studies associated to collections Jul 24, 2024

chinook25 requested changes Jul 26, 2024

View reviewed changes

svituz force-pushed the feat/add_studies_in_biobank_directory branch from 69e48be to 0bf1a78 Compare September 9, 2024 16:29

svituz force-pushed the feat/add_studies_in_biobank_directory branch from 0d15013 to 2977179 Compare September 17, 2024 10:42

svituz requested review from chinook25 and esthervanenckevort September 17, 2024 12:23

chinook25 approved these changes Sep 17, 2024

View reviewed changes

esthervanenckevort removed the request for review from konstantina-gkp September 18, 2024 07:28

esthervanenckevort reviewed Sep 18, 2024

View reviewed changes

dtroelofsprins requested changes Sep 19, 2024

View reviewed changes

svituz force-pushed the feat/add_studies_in_biobank_directory branch from 2977179 to b16cc2e Compare September 19, 2024 08:35

svituz requested a review from dtroelofsprins September 19, 2024 09:17

dtroelofsprins reviewed Sep 19, 2024

View reviewed changes

svituz force-pushed the feat/add_studies_in_biobank_directory branch from dcdf79f to 2b32b30 Compare September 20, 2024 12:49

svituz force-pushed the feat/add_studies_in_biobank_directory branch 2 times, most recently from 7547367 to 6d2e31e Compare October 8, 2024 10:49

svituz force-pushed the feat/add_studies_in_biobank_directory branch from 6d2e31e to 20dd31e Compare October 9, 2024 07:55

esthervanenckevort added the priority Features/bugs that we should not let wait label Oct 21, 2024

svituz added 18 commits October 21, 2024 09:50

feat(data/biobank-directory): updates model to include Study and asso…

2451e68

…ciate it to Collection

feat(data/biobank_directory): updates demo data to include Study

fc278d5

feat(data/biobank-directory): fixes Study model

4f2b222

feat(apps/directory): adds viewing of studies related to collections

7bff877

test(datamodels): updates the test that checks for the number of tabl…

a19b0ab

…es in the Directory

feat: changes description of Study entity to be more generic and chan…

69f9741

…ge the id the dummy study according to the review

feat: restores NationalNode attribute for AlsoKnownIn table to be req…

55edfe1

…uired and adds it to Study table

feat: adds EXT national node to the demo

4a08c4e

feat(directory): implements changes suggested in the review

ba9c1ef

test(directory): fixes directory's loader test

3c3b3e9

refactor(app/directory): convert the studyStore to typescript

a3e53d0

feat(apps/directory): removes watch of route from StudyReport

b21151a

refactor: removes console logging and revert const HOST

d910bd4

refactor: remove study from the information shown on the collection c…

1c3301f

…ard. Left only in the right pane

refactor(directory): adds refLabel to Collection and removes all refe…

5de8478

…rence to ECRIN in the comments and id

refactor: moves the study column under the Belongs To header

18db96f

feat: changes study attribute of Collection entity to be ref_array an…

afb9f1f

…d changed the frontend accordingly

feat: change study column name to studies

ab775d9

svituz force-pushed the feat/add_studies_in_biobank_directory branch from 20dd31e to ab775d9 Compare October 21, 2024 08:29

dtroelofsprins approved these changes Oct 21, 2024

View reviewed changes

svituz merged commit a5e8310 into master Oct 22, 2024
6 checks passed

svituz deleted the feat/add_studies_in_biobank_directory branch October 22, 2024 12:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(directory): add studies associated to collections #3837

feat(directory): add studies associated to collections #3837

svituz commented Jun 3, 2024

sonarqubecloud bot commented Jun 11, 2024

esthervanenckevort left a comment

esthervanenckevort commented Jul 15, 2024

dtroelofsprins commented Jul 17, 2024

chinook25 left a comment

chinook25 Jul 26, 2024

svituz Sep 10, 2024

chinook25 left a comment

chinook25 Sep 17, 2024

svituz Sep 19, 2024

chinook25 Sep 17, 2024

svituz Sep 19, 2024

chinook25 Sep 17, 2024

svituz Sep 19, 2024

esthervanenckevort Sep 18, 2024

dtroelofsprins left a comment

dtroelofsprins Sep 18, 2024

svituz Sep 19, 2024

dtroelofsprins Sep 18, 2024

svituz Sep 19, 2024

svituz commented Sep 19, 2024

dtroelofsprins commented Sep 19, 2024

dtroelofsprins Sep 19, 2024

svituz Sep 20, 2024

esthervanenckevort commented Sep 19, 2024

esthervanenckevort commented Sep 19, 2024

svituz commented Oct 8, 2024

dtroelofsprins commented Oct 9, 2024

sonarqubecloud bot commented Oct 21, 2024

svituz commented Oct 21, 2024

dtroelofsprins left a comment

feat(directory): add studies associated to collections #3837

feat(directory): add studies associated to collections #3837

Conversation

svituz commented Jun 3, 2024

What are the main changes you did:

how to test:

sonarqubecloud bot commented Jun 11, 2024

Quality Gate passed

esthervanenckevort left a comment

Choose a reason for hiding this comment

esthervanenckevort commented Jul 15, 2024

dtroelofsprins commented Jul 17, 2024

chinook25 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chinook25 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dtroelofsprins left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

svituz commented Sep 19, 2024

dtroelofsprins commented Sep 19, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

esthervanenckevort commented Sep 19, 2024

esthervanenckevort commented Sep 19, 2024

svituz commented Oct 8, 2024

dtroelofsprins commented Oct 9, 2024

sonarqubecloud bot commented Oct 21, 2024

Quality Gate passed

svituz commented Oct 21, 2024

dtroelofsprins left a comment

Choose a reason for hiding this comment