-
Notifications
You must be signed in to change notification settings - Fork 5
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
Add role and permission guard decorators to some existing routes #112
Conversation
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.
Wow, this look like a massive amount of work. All tests passed!
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.
do we have tests for the guards?
yes we should have e2e tests on each endpoint for the permission, but I think we might need to go through each of them and add more soon, as i'm thinking to add more permissions But I don't think we have tests for the role guard |
…rd-be into chore/decorators
I decided to only add role check for forms (also did some refactoring while i'm there) because
Resources, sprints, users endpoints I have not touched so I don't intervere with the tests Josh, Curt, and Piero are working on - will need to add decorators to them - also because I want to add more permissions for these endpoints |
back to draft again - I want to add missing swagger docs to the forms endpoints |
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 like how you moved the utility functions to a separate file. We should properly do something similar to the function in the other test files.
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.
Successfully gave 403 for non-admin logged in user
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.
great job! looks really clean.
Description
Add role and permission guard decorators to all routes except sprints, resources, and part of ideations, as I would like to look at adding additional permissions for these routes.
TODO - for future, not in this PR
#112 (comment)
Issue link
Fixes # (issue)
Type of change
How Has This Been Tested?
Checklist: