-
-
Notifications
You must be signed in to change notification settings - Fork 147
manage user permissions #514
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
base: main
Are you sure you want to change the base?
Conversation
|
Hey @dwradcliffe , just wanted to check in. Did you want me to review your PR, or is it still under development? |
backend/pkg/database/gorm_common.go
Outdated
| if result.Error != nil { | ||
| return result.Error | ||
| } | ||
| // Update User Permissions |
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.
ideally this would all be handed by the GORM Association logic & hooks
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.
Updated, please take a look
backend/pkg/models/user.go
Outdated
| Picture string `json:"picture"` | ||
| Email string `json:"email"` | ||
| Role pkg.UserRole `json:"role"` | ||
| Permissions map[string]map[string]bool `json:"permissions" gorm:"-:all"` |
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.
can we use GORM references for this - https://gorm.io/docs/has_many.html#Override-References
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'm not 100% sure what you're suggesting here. I'm using a different format for the api so that it's easier to handle the frontend concerns without leaking the database structure into the frontend. I think the current version is a good compromise but please take a look.
66b1310 to
a9bf9b2
Compare
Part of #57
This adds the ability for an admin user to set permissions for each user. This does not allow these permissions to be used, that will be in the next PR.