-
-
Notifications
You must be signed in to change notification settings - Fork 729
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
feat(cjux-278): maintenance root roles #8875
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -41,6 +41,9 @@ export const CREATE_TAG_TYPE = 'CREATE_TAG_TYPE'; | |
export const UPDATE_TAG_TYPE = 'UPDATE_TAG_TYPE'; | ||
export const DELETE_TAG_TYPE = 'DELETE_TAG_TYPE'; | ||
|
||
export const UPDATE_MAINTENANCE_MODE = 'UPDATE_MAINTENANCE_MODE'; | ||
export const UPDATE_INSTANCE_BANNERS = 'UPDATE_INSTANCE_BANNERS'; | ||
|
||
// Project | ||
export const CREATE_FEATURE = 'CREATE_FEATURE'; | ||
export const UPDATE_FEATURE = 'UPDATE_FEATURE'; | ||
|
@@ -83,7 +86,7 @@ export const RELEASE_PLAN_TEMPLATE_DELETE = 'RELEASE_PLAN_TEMPLATE_DELETE'; | |
|
||
export const ROOT_PERMISSION_CATEGORIES = [ | ||
{ | ||
label: 'Addon', | ||
label: 'Integration', | ||
permissions: [CREATE_ADDON, UPDATE_ADDON, DELETE_ADDON], | ||
}, | ||
{ | ||
|
@@ -141,4 +144,17 @@ export const ROOT_PERMISSION_CATEGORIES = [ | |
RELEASE_PLAN_TEMPLATE_UPDATE, | ||
], | ||
}, | ||
{ | ||
label: 'Instance maintenance', | ||
permissions: [UPDATE_MAINTENANCE_MODE, UPDATE_INSTANCE_BANNERS], | ||
}, | ||
]; | ||
|
||
// Used on Frontend, to allow admin panel use for users with custom root roles | ||
export const MAINTENANCE_MODE_PERMISSIONS = [ | ||
ADMIN, | ||
READ_ROLE, | ||
READ_CLIENT_API_TOKEN, | ||
READ_FRONTEND_API_TOKEN, | ||
UPDATE_MAINTENANCE_MODE, | ||
Comment on lines
+154
to
+159
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How does this work? Do you need one of them or all of them? Seems a little strange to gain access to the full admin panel if you have read client API token only? Maybe I'm misunderstanding how this works? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
]; |
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 didn't know we could import stuff from the server? Should it go in
AccessProvider/permissions
to be consistent with how we're doing it today? I think only defining it in one place is cool, though, but why is it not something we're already doing?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.
Thanks for noticing this. It needs refactoring. I'll post another PR, because it turns out there are inconsistencies all over this.