chore: permission types #6417
Open
chore: permission types #6417
+369
−123
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…on-types # Conflicts: # frontend/common/providers/Permission.tsx # frontend/web/components/modals/create-feature/index.js
…on-types # Conflicts: # frontend/web/components/modals/create-feature/index.js
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
github-actions
bot
added
the
front-end
Contributor
Docker builds report
|
Open
4 tasks
Zaimwa9
requested changes
Jan 5, 2026
Contributor
Zaimwa9
left a comment
Zaimwa9
left a comment
There was a problem hiding this comment.
Looks neat. There are just 2 conditions that got truncated somehow and a couple of NITs
Comment on lines
+283
to
+284
| permissions[EnvironmentPermission.VIEW_IDENTITIES] | ||
| permissions.ADMIN |
Contributor
There was a problem hiding this comment.
Suggested change
| permissions[EnvironmentPermission.VIEW_IDENTITIES] | |
| permissions.ADMIN | |
| permissions[EnvironmentPermission.VIEW_IDENTITIES] || | |
| permissions.ADMIN |
Comment on lines
+1895
to
+1898
| (!savePermission, | ||
| isSaving || | ||
| !projectFlag.name || | ||
| invalid) |
Contributor
There was a problem hiding this comment.
Suggested change
| (!savePermission, | |
| isSaving || | |
| !projectFlag.name || | |
| invalid) | |
| (!savePermission || | |
| isSaving || | |
| !projectFlag.name || | |
| invalid) |
| }, | ||
| organisationPermissions: (perm: string) => | ||
| organisationPermissions: (perm: OrganisationPermissionDescription) => | ||
| `To manage this feature you need the <i>${perm}</i> permission for this organisastion.<br/>Please contact a member of this organisation who has administrator privileges.`, |
Contributor
There was a problem hiding this comment.
Suggested change
| `To manage this feature you need the <i>${perm}</i> permission for this organisastion.<br/>Please contact a member of this organisation who has administrator privileges.`, | |
| `To manage this feature you need the <i>${perm}</i> permission for this organisation.<br/>Please contact a member of this organisation who has administrator privileges.`, |
|
|
||
| const finalPermission = hasPermission || AccountStore.isAdmin() | ||
|
|
||
| let permissionDescriptionFunc: (perm: any) => any = |
Contributor
There was a problem hiding this comment.
What do you think about renaming to something like getPermissionDescription?
Comment on lines
+200
to
+203
| permissionName || | ||
| permissionDescriptionFunc( | ||
| PermissionDescriptions[permission as ProjectPermission], | ||
| ), |
Contributor
There was a problem hiding this comment.
Can we put this into a variable and re-use it in the 2 places?
| '#F57C78', | ||
| ], | ||
| projectPermissions: (perm: string) => | ||
| projectPermissions: (perm: ProjectPermissionDescription) => |
Contributor
There was a problem hiding this comment.
Actually, can't we use directly the enum here and get the description within this function ?
const description = ProjectPermissionDescriptions[perm];
Comment on lines
27
to
+29
| type PermissionType = { | ||
| id: number | string | ||
| permission: string | ||
| permission: PermissionEnum |
Contributor
There was a problem hiding this comment.
NIT:
Here we could use a union type like:
type OrganisationLevelProps = BasePermissionProps & {
level: 'organisation';
permission: OrganisationPermission;
};
same with env and project and then
export type PermissionType =
| ProjectLevelProps
| EnvironmentLevelProps
| OrganisationLevelProps;
so the permission would directly be inferred and we can get invalid combinations directly
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Thanks for submitting a PR! Please check the boxes below:
docs/if required so people know about the feature!Changes
How did you test this code?
E2E, checking each usage of permission
Note: due to amount of refactoring I've done, I've pointed this to the latest branch I've been working on