Extending Concept Set Items with Annotations #2403

alex-odysseus · 2024-10-29T11:50:18Z

Addressing #2318

A new Concept Set Annotation entity has been added being associated with a Concept Set to which the Concept Set Annotations relate to

When Concept Set is copied its Concept Set Annotations are preserved so that it will be known that some of the Concept Set Annotations were originated from a particular Concept Set of a specific Version

When a Concept Set deleted it is deleted together with the Concept Set Annotations

Concept Set Annotations can be deleted by the Administrators only or by those who have a specific permission assigned

…etadata

…tation, added vocabulary version and createdBy/createdDate

…eration, moved DB definition for the annotations feature into a single SQL file

Permitting ATLAS users to create Concept Set Annotations

…tation/{id}

…{id}

…d search data JSON to a human friendly format in annotations tab

…e format

…annotations)

alex-odysseus · 2024-10-29T12:13:10Z

src/main/java/org/ohdsi/webapi/service/ConceptSetService.java

+                    annotationDetailsDTO.setSearchData(newAnnotationData.getSearchData());
+                    conceptSetAnnotation.setAnnotationDetails(mapper.writeValueAsString(annotationDetailsDTO));
+                } catch (JsonProcessingException e) {
+                    throw new RuntimeException(e);


Let's add a log entry

alex-odysseus · 2024-10-29T12:15:35Z

src/main/java/org/ohdsi/webapi/service/ConceptSetService.java

+            }
+        }
+    }
+    private ConceptSetAnnotation copyAnnotation(ConceptSetAnnotation sourceConceptSetAnnotation, int sourceConceptSetId, int targetConceptSetId){


Please move it down below the public method where it is used

alex-odysseus · 2024-10-29T12:16:45Z

src/main/java/org/ohdsi/webapi/service/ConceptSetService.java

+        try {
+            annotationDetails = mapper.readValue(conceptSetAnnotation.getAnnotationDetails(), AnnotationDetailsDTO.class);
+        } catch (JsonProcessingException e) {
+            throw new RuntimeException(e);


A log entry should be added additionally

alex-odysseus · 2024-10-29T12:18:44Z

src/main/java/org/ohdsi/webapi/service/ConceptSetService.java

+    @PUT
+    @Path("/update/{id}/annotation")
+    @Produces(MediaType.APPLICATION_JSON)
+    public AnnotationDTO updateConceptSetAnnotation(@PathParam("id") final int id, AnnotationDTO annotationDTO) throws IOException {


This method is obsolete at this moment, if in the future it will be necessary to add/update a comment to a specific Concept Set Annotation it might be added

alex-odysseus · 2024-10-29T12:24:37Z

src/main/resources/db/migration/postgresql/V2.14.0.202407161000__conceptset_annotation.sql

@@ -0,0 +1,46 @@
+CREATE SEQUENCE ${ohdsiSchema}.concept_set_annotation_sequence;


Let's combine one migration script

Yes, let's combine into 1 script and also this is targeting v2.15 so if we do create 1 migrations cript let's just make a new one with all the others combined, call it V2.15 ...

alex-odysseus · 2024-10-29T12:31:09Z

src/main/resources/i18n/messages_en.json

@@ -1786,7 +1791,8 @@
        "includedConcepts": "Included Concepts",
        "includedSourceCodes": "Included Source Codes",
        "versions": "Versions",
-        "messages": "Messages"
+        "messages": "Messages",
+        "metadata": "Metadata"


To be removed

chrisknoll · 2024-11-04T16:52:43Z

src/main/java/org/ohdsi/webapi/conceptset/ConceptSet.java

@@ -36,7 +37,7 @@
 *
 * @author fdefalco
 */
-@Entity(name = "ConceptSet")
+@Entity


Please revert this as we have a convention to name the entity, you can see this in CohortDefinition, Source, etc.

chrisknoll · 2024-11-04T16:52:56Z

src/main/java/org/ohdsi/webapi/conceptset/ConceptSetGenerationInfo.java

@@ -21,7 +21,7 @@
 * @author Anthony Sena <https://github.com/ohdsi>
 */

-@Entity(name = "ConceptSetGenerationInfo")
+@Entity


Same as above, please revert this change.

chrisknoll · 2024-11-04T16:53:13Z

src/main/java/org/ohdsi/webapi/conceptset/ConceptSetItem.java

@@ -29,7 +29,7 @@
 * @author fdefalco
 */

-@Entity(name = "ConceptSetItem")
+@Entity


Same as above, we name Entities.

chrisknoll · 2024-11-04T16:53:25Z

src/main/java/org/ohdsi/webapi/conceptset/annotation/ConceptSetAnnotation.java

+import javax.persistence.Table;
+import java.io.Serializable;
+
+@Entity


Add name here.

chrisknoll · 2024-11-04T16:58:52Z

src/main/java/org/ohdsi/webapi/security/model/ConceptSetAnnotationPermissionSchema.java

+    private static Map<String, String> writePermissions = new HashMap<String, String>() {
+        {
+            put("conceptset:annotation:%s:delete", "Delete Concept Set Annotation with ID %s");
+        }
+    };
+
+    private static Map<String, String> readPermissions = new HashMap<String, String>() {
+        {
+        }
+    };


This is odd to me:

Shouldn't the writePermissions contain all permissions related to creating annotations and the read have the ones related to reading annotations?

chrisknoll · 2024-11-04T17:02:47Z

src/main/java/org/ohdsi/webapi/service/ConceptSetService.java

+import org.apache.commons.collections4.CollectionUtils;
+import org.apache.commons.lang3.StringUtils;


Just might want to check this import (collections4 is something I'm not familiar with). is there an existing package we depend on that has the function in CollectionUtils that you are using?

Just trying to keep extra dependencies down....when we get to the 3.x line we're going to be combing through the code locating redundant dependcies (ie: libraries that support string manipulation will be de-duplicated) So if we can remove a source of additional duplication, we should try to do it here.

chrisknoll · 2024-11-04T17:07:12Z

...urces/db/migration/postgresql/V2.14.0.202407261000__add_conceptset_version_to_annotation.sql

@@ -0,0 +1 @@
+ALTER TABLE ${ohdsiSchema}.concept_set_annotation ADD concept_set_version VARCHAR;


Are we sure VARCHAR is the correct type here? That will allow 2GB of text

chrisknoll · 2024-11-04T17:10:05Z

Hi, Everyone. I added comments and if you could address those I will work on pulling the branch down to test.

Specifically, the migration scripts being combined and re-labeled to 2.15 is soemthing I'd like to have changed before pulling it down, because otherwise I'd have to manually roll back the stuff in the 2.14 migrations and then let them re apply from the 2.15 migration, and it gets to be a headache. Once that's done I can play-test the functionality locally and provide feedback.

Reverting back entity names in @entity according to the codebase policy

chrisknoll · 2024-11-08T14:32:00Z

Thanks for addressing those requested changes. I'll pull it down and take it for a spin.

hernaldourbina and others added 14 commits October 29, 2024 12:10

ATL-17: Updating branch with latest release

e6c3857

ATL-17: Add feature about the metadata in concept set

633eb78

Fixing migration scripts and Concept Set Service to save and update m…

757929a

…etadata

[ATL-17] Refactored the annotations feature. Renamed metadata to anno…

83579dd

…tation, added vocabulary version and createdBy/createdDate

[ATL-17] Fixed issue with clashing endpoints for conceptset delete op…

7e9359c

…eration, moved DB definition for the annotations feature into a single SQL file

Correcting newly adjusted DELETE endpoint notation

25f44ae

Permitting ATLAS users to create Concept Set Annotations

Fixing the previous commit as the endpoint is DELETE /conceptset/anno…

948fde3

…tation/{id}

Fixing the notation as the endpoint is DELETE /conceptset/annotation/…

0fff858

…{id}

[ATL-58] Added concept set version to annotations

7adb69b

[ATL-58] Implemented copying of annotations along with ConceptSet

f40a666

[ATL-58] Fixed permissions issues for annotations feature, transforme…

2b9c35a

…d search data JSON to a human friendly format in annotations tab

Fix for annotation search data shown as JSON instead of human readabl…

485b41a

…e format

Added 'Copied From' list of concept set ids for concept set annotations

937c72e

Added i18n records for Origin Concept Sets ('Copied From' feature in …

6a88e5d

…annotations)

alex-odysseus commented Oct 29, 2024

View reviewed changes

alex-odysseus requested review from chrisknoll and anthonysena October 29, 2024 12:32

chrisknoll reviewed Nov 4, 2024

View reviewed changes

anthonysena linked an issue Nov 5, 2024 that may be closed by this pull request

Add annotations to Concept Set Items #2318

Open

A combined migration script has been assembled

deed559

Reverting back entity names in @entity according to the codebase policy

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Extending Concept Set Items with Annotations #2403

Extending Concept Set Items with Annotations #2403

alex-odysseus commented Oct 29, 2024 •

edited

Loading

alex-odysseus Oct 29, 2024

alex-odysseus Oct 29, 2024

alex-odysseus Oct 29, 2024

alex-odysseus Oct 29, 2024

alex-odysseus Oct 29, 2024

chrisknoll Nov 4, 2024

alex-odysseus Oct 29, 2024

chrisknoll Nov 4, 2024

chrisknoll Nov 4, 2024

chrisknoll Nov 4, 2024

chrisknoll Nov 4, 2024

chrisknoll Nov 4, 2024

chrisknoll Nov 4, 2024

chrisknoll Nov 4, 2024

chrisknoll commented Nov 4, 2024

chrisknoll commented Nov 8, 2024

		@@ -0,0 +1,46 @@
		CREATE SEQUENCE ${ohdsiSchema}.concept_set_annotation_sequence;

		import org.apache.commons.collections4.CollectionUtils;
		import org.apache.commons.lang3.StringUtils;

		@@ -0,0 +1 @@
		ALTER TABLE ${ohdsiSchema}.concept_set_annotation ADD concept_set_version VARCHAR;

Extending Concept Set Items with Annotations #2403

Are you sure you want to change the base?

Extending Concept Set Items with Annotations #2403

Conversation

alex-odysseus commented Oct 29, 2024 • edited Loading

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

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

chrisknoll commented Nov 4, 2024

chrisknoll commented Nov 8, 2024

alex-odysseus commented Oct 29, 2024 •

edited

Loading