-
Notifications
You must be signed in to change notification settings - Fork 5
Index route for feedback #604
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
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.
Pull Request Overview
This PR adds functionality to list feedback on a project by introducing an index route for project feedback. The change enables teachers and students to retrieve all feedback associated with a specific project.
- Added
indexaction to the feedback controller with authorization checks - Updated routes to support both
indexandcreateactions for feedback resources - Added comprehensive test coverage for listing feedback with different user roles
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| config/routes.rb | Changed feedback from singular resource to plural resources to support index action |
| app/controllers/api/feedback_controller.rb | Added index action to retrieve and authorize feedback for a project |
| app/views/api/feedback/index.json.jbuilder | Created view template to render feedback list as JSON |
| app/models/ability.rb | Updated authorization rules to allow reading feedback for teachers and students |
| spec/models/ability_spec.rb | Added test cases verifying read permissions for feedback |
| spec/features/feedback/listing_feedback_spec.rb | Added comprehensive integration tests for listing feedback endpoint |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Just added a couple of things, I think the copilot suggestions are also worth looking at, I commented on one, but also the one about redundant authorization just in case of n+1s
Add authorization check for feedback access
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.
LGTM
Status
What's changed?
indexroute for project feedback