-
Notifications
You must be signed in to change notification settings - Fork 18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor: Extract ObjectAccess and Administrative permissions into Permission model in admin slice #3152
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3152 +/- ##
==========================================
- Coverage 88.79% 88.74% -0.05%
==========================================
Files 265 266 +1
Lines 22260 22274 +14
==========================================
+ Hits 19765 19767 +2
- Misses 2495 2507 +12 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good, thanks!
webapi/src/main/scala/org/knora/webapi/responders/admin/PermissionsResponderADM.scala
Outdated
Show resolved
Hide resolved
package org.knora.webapi.slice.admin.domain.model | ||
|
||
sealed trait Permission { | ||
def token: String |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
comment (non-blocking): I find the term token
somewhat confusing because it makes me think of JWTs. But I can't think of anything better for now. Maybe in the future we can try to think of something more descriptive.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am happy to hear any suggestions.
The token
contains now the value which is actually written as a token in the hasPermission
property:
... knora-api:hasPermissions "CR knora-admin:Creator|M knora-admin:ProjectMember|V knora-admin:KnownUser|RV knora-admin:UnknownUser"
... knora-base:hasPermissions "ProjectAdminGroupAllPermission|ProjectResourceCreateRestrictedPermission http://www.knora.org/ontology/0001/example#person,http://www.knora.org/ontology/0001/example#event"
For example the full name for all administrative Permissions like ProjectAdminGroupAllPermission
or the abbreviation for the object access permissions like CR
. Other property names I have considered were: abbreviation
, code
, symbol
, name
, wdyt?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I spontaneously like code
best. But at the end of the day, I'm ok with either of those as well as with token
…sionsResponderADM.scala Co-authored-by: Balduin Landolt <[email protected]>
Pull Request Checklist
Task Description/Number
Issue Number: DEV-
PR Type
Basic requirements for bug fixes and features
Does this PR introduce a breaking change?
Does this PR change client-test-data?