Move addComments to permissionClass #2263
Conversation
| @@ -38,6 +39,10 @@ export class Permissions { | |||
| return this.checkProjectFilterPermission(metric, "createMetrics"); | |||
| }; | |||
|
|
|||
| public canAddComment = (projects: string[]): boolean => { | |||
There was a problem hiding this comment.
open to suggestions on how to type the arguments here - we check for this permission across metrics (with have projects, along with ideas, experiments, and features, which only have a project property.
It'd be nice to standardize so the engineer doesn't have to have much context when calling canAddComment.
Currently, it still seems like a step in the right direction, as before it could be a string, or undefined, or string[], so this at least simplifies it a bit.
There was a problem hiding this comment.
One option is to type the argument like this:
type ObjWithProject = {project?: string; projects?: string[]};That way, you could call it directly with the thing you want to comment on:
canAddComment(feature);
canAddComment(metric);
Another option is to make separate canCommentOnFeature, canCommentOnMetric, etc. methods. They could all check the same addComments permission for now, but it leaves the door open for us to make this finer grained in the future.
There was a problem hiding this comment.
I'm also fine with how you have it
|
Your preview environment pr-2263-bttf has been deployed. Preview environment endpoints are available at: |
Features and Changes
This PR moves the
addCommentspermission check to the new permissionClass. This is the firstREAD_ONLYtype permission that we've migrated, so there are updates to thecheckProjectFilterPermissionsmethod.If the resource is in "All Projects", we'll add the user's project-specific roles to the
projectsarray, as the user only needs to have the permission globally, or in at least 1 project in order to have the permission.