Atlas icon indicating copy to clipboard operation
Atlas copied to clipboard

Published 20 hours ago • verified publisher icon OHDSI

Feature/global sharing

Open rkboyce opened this issue 10 months ago • 7 comments

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.

rkboyce avatar Apr 23 '24 17:04 rkboyce

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

pieterlukasse avatar May 16 '24 14:05 pieterlukasse

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

pieterlukasse avatar May 23 '24 10:05 pieterlukasse

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

chrisknoll avatar May 23 '24 12:05 chrisknoll

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 avatar May 23 '24 12:05 pieterlukasse

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 avatar May 23 '24 13:05 pieterlukasse

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!

pieterlukasse avatar May 23 '24 13:05 pieterlukasse

@rkboyce please merge in https://github.com/vinci-ohdsi/Atlas-2.15-dev/pull/7 and then I think this PR is go (for testing phase).

pieterlukasse avatar Aug 23 '24 17:08 pieterlukasse