Extending Concept Set Items with Annotations by alex-odysseus · Pull Request #2403 · OHDSI/WebAPI

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

        "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

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

 */

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

 */

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

@entity

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.

…nership of the concept set for annotations

…dressed comments from the feature review on the community github # Conflicts: # src/main/java/org/ohdsi/webapi/conceptset/ConceptSet.java

…sion comments

chrisknoll · 2024-12-04T15:16:17Z

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

    private static Map<String, String> writePermissions = new HashMap<String, String>() {{
        put("conceptset:%s:put", "Update Concept Set with ID = %s");
        put("conceptset:%s:items:put", "Update Items of Concept Set with ID = %s");
+        put("conceptset:*:annotation:put", "Create Concept Set Annotation");


These should not be * permissions. What happens is that when you delete the entity, it finds the write permission schema and deletes it, thus deleting the wildcard permission' from all users, and then preventing access to the endpiont for all users.

However, this is what happens when you extend the 'CommonEntity' which you didn't do in this case, however, ou are using a permissionSchema as if it was a common entity, so there may be a little confusion here.

But, in any case, we shouldn't see * permissions in any writePermission schema because these permission schemas are targeting a specific entity ID.

chrisknoll · 2024-12-06T19:51:14Z

Hi, I am getting an error when I restore to a prior version of the DB and the migration scripts were applied.

Steps to reproduce:

Click Create concept Set (repository concept set)
Name it
Save it (empty concept set)
Observe error: There was an error saving your concept set'
Close editor with X to return to concept set list
Observe the new concept set was actually created.
Open Concept set
Search for a concept
Add concept to concept set
Try to save again: Observe same error 'There was an error saving your concept set'
11 close and re-open the concept set, and the concept set seems to be saved.

Note: annotation tab is 'disabled' in that clicking it does not swatch tab.
Also, there are no errors in the browser console, and no exceptions seen on the server log, making it hard to diagnose the issue.

chrisknoll · 2024-12-16T19:40:41Z

I've confirmed that I do not get this error when switching to master/master (atlas/webapi) so there is something related to this branch leading to the described error.

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 anthonysena and chrisknoll 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

Closed

A combined migration script has been assembled

deed559

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

oleg-odysseus and others added 4 commits December 3, 2024 11:13

Changed owner/permissions check for annotations delete to consider ow…

1f6476c

…nership of the concept set for annotations

Added migration script to re-create annotation-related permissions. A…

4ee6b48

…dressed comments from the feature review on the community github # Conflicts: # src/main/java/org/ohdsi/webapi/conceptset/ConceptSet.java

Changed SearchDataTransformer in accordance with the community discus…

57491c3

…sion comments

Permissions update, concept_set_version column type change to integer

4c3a304

chrisknoll requested changes Dec 4, 2024

View reviewed changes

anthonysena assigned chrisknoll Dec 17, 2024

chrisknoll approved these changes Dec 17, 2024

View reviewed changes

chrisknoll merged commit 67f8816 into master Dec 17, 2024

chrisknoll deleted the concept-set-annotations branch December 17, 2024 14:40

		@@ -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; No newline at end of file

Conversation

alex-odysseus commented Oct 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chrisknoll commented Nov 4, 2024

Uh oh!

chrisknoll commented Nov 8, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chrisknoll commented Dec 6, 2024

Uh oh!

chrisknoll commented Dec 16, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

alex-odysseus commented Oct 29, 2024 •

edited

Loading