Feature/global sharing #2929
Feature/global sharing #2929
Conversation
…ranting write access to a global shared artifact reader role that would be given to all users. Next steps are to 1) make this configurable, 2) allow users to configure the global author role if they want only some users to be able to grant global read access, and 3) set up global read role for all users as a default assigned system role
…ole to a new roleid.
… then decided not to
…'public' role since this would remove the need to change WebAPI to add a new system role that pretty much duplicates 'public'
There was a problem hiding this comment.
thanks @rkboyce ! Please see two small comment improvements and one question below.
…rt to only persons who have been granted permission to change the same cohort.
|
thanks for merging in my proposed changes @rkboyce 👍 |
...to better reflect its real purpose
fix: adjust permission name to "artifact:global:share:put"
|
@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. |
|
thanks @chrisknoll . I found the PR where this was introduced: OHDSI/WebAPI#1273 The PR mentioned above, introduced a new PermissionController.java and a PermissionService.java. |
|
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! |
based on this discussion OHDSI#2929 (comment)
based on this discussion OHDSI#2929 (comment)
based on this discussion OHDSI#2929 (comment)
based on this discussion OHDSI#2929 (comment)
based on this discussion OHDSI#2929 (comment)
based on this discussion OHDSI#2929 (comment)
based on this discussion OHDSI#2929 (comment)
based on this discussion OHDSI#2929 (comment)
|
@rkboyce please merge in vinci-ohdsi#7 and then I think this PR is go (for testing phase). |
Fix: remove dead code
...previous name isPermittedGlobalShareCohort did not reflect the fact that it is about all kind of artifacts
Fix: better name for isPermittedGlobalShareArtifact
|
I think that we have completed the testing and discussion for this feature. I am closing the draft pull request and opening an actual one. |
based on this discussion OHDSI#2929 (comment)
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.