search

OHDSI / Atlas

ATLAS is an open source software tool for researchers to conduct scientific analyses on standardized observational data
http://atlas-demo.ohdsi.org/
Apache License 2.0
258 stars 126 forks source link

Feature/global sharing #2929

Open rkboyce opened 2 months ago

rkboyce commented 2 months ago

This is a draft pull request that shows a test implementation of the enhancement described in this Atlas ticket .

Here is a video of the functionality

This changes in the draft pull request implement the general functionality needed in the configure-access-modal and show the role dependent visibility of the lock icon for the concept set and cohort definition managers. The final pull request will apply the latter to all relevant sub-apps in Atlas.

pieterlukasse commented 1 month ago

thanks for merging in my proposed changes @rkboyce 👍 You can mark all my comments above as resolved.

pieterlukasse commented 1 month ago

@rkboyce FYI - I found one issue in this solution where the "lock icon" only appears to the owner of the cohort definition, regardless of our global sharing configuration. I am trying to work around this (see this commit https://github.com/uc-cdis/Atlas/pull/47/commits/b8fa9399de36fa5a6b697ee922d6693ba9943dbf added to https://github.com/uc-cdis/Atlas/pull/47). However, this seems to reveal some more issues. Please take a look and let me know if you're able to reproduce it as well.

chrisknoll commented 1 month ago

@rkboyce FYI - I found one issue in this solution where the "lock icon" only appears to the owner of the cohort definition, regardless of our global sharing configuration. I am trying to work around this (see this commit uc-cdis@b8fa939 added to uc-cdis#47). However, this seems to reveal some more issues. Please take a look and let me know if you're able to reproduce it as well.

I think that's correct: I think the current decision is that only owner can control access to an asset. I don't think that this is documented, but I can't recall anything about permissions that are granted that grant permissions to control access rights. Apologies if I am mistaken.

pieterlukasse commented 1 month ago

thanks @chrisknoll . I found the PR where this was introduced: https://github.com/OHDSI/WebAPI/pull/1273 This is the endpoint that is called when the permissions modal UI is opened: AtlasSecurity.java - Line214 In my case, the call WebAPI/permission/access/COHORT_DEFINITION/445/READ returns 403 (probably from here).

The PR mentioned above, introduced a new PermissionController.java and a PermissionService.java. @rkboyce let's discuss with the business today and see if they are OK with this restriction.

pieterlukasse commented 1 month ago

This might actually work as a solution for now, based on our current "teamproject" WebAPI code: https://github.com/uc-cdis/WebAPI/pull/137/files

pieterlukasse commented 1 month ago

This might actually work as a solution for now, based on our current "teamproject" WebAPI code: https://github.com/uc-cdis/WebAPI/pull/137/files

update: this solution works and allows me to share a cohort created by someone else in my team. Let me know what you think!