Don't allow admins to submit to invalid exercises #5997
Conversation
WalkthroughThe pull request introduces updates to the Changes
Possibly related PRs
Suggested reviewers
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
test/controllers/submissions_controller_test.rb (1)
501-508: Rename the test to better reflect its purpose.The test name "should not be able to submit to valid exercise" is misleading as it actually verifies that submissions to valid exercises are successful.
Consider renaming to better reflect the test's purpose:
- test 'should not be able to submit to valid exercise' do + test 'should be able to submit to valid exercise' do
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
app/policies/activity_policy.rb(1 hunks)test/controllers/submissions_controller_test.rb(1 hunks)test/system/activities_test.rb(1 hunks)
🔇 Additional comments (3)
app/policies/activity_policy.rb (1)
61-61: LGTM! The implementation effectively prevents submissions to invalid exercises.
The placement of the not_valid? check before the admin check ensures that no user, regardless of their role, can submit to invalid exercises. This aligns perfectly with the PR objective.
test/system/activities_test.rb (1)
72-77: LGTM! The system test effectively verifies the UI behavior.
The test ensures that the submission button is not rendered when an exercise is invalid, providing good coverage for the UI implementation of this feature.
test/controllers/submissions_controller_test.rb (1)
490-499: LGTM! The test verifies the restriction for invalid exercises.
The test properly verifies that submissions to invalid exercises are rejected with an unprocessable_entity response, even for staff users.
There was a problem hiding this comment.
Copilot reviewed 3 out of 3 changed files in this pull request and generated no suggestions.
Comments skipped due to low confidence (1)
test/controllers/submissions_controller_test.rb:501
- The test name should be 'should be able to submit to valid exercise' instead of 'should not be able to submit to valid exercise'.
test 'should not be able to submit to valid exercise' do
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
test/system/activities_test.rb(1 hunks)test/system/scratchpad_test.rb(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- test/system/activities_test.rb
🔇 Additional comments (1)
test/system/scratchpad_test.rb (1)
38-43: LGTM! The setup code looks correct.
The reintroduced setup code properly initializes the scratchpad interface for testing.
This pull request blocks all users from submitting to invalid exercises.
For some reason this was still allowed for admins, which is odd, as a text was already shown mentioned disabling submit:
This fixes some internal errors, as submitting to an exercise with an invalid config evidently causes bugs.
Eg: https://dodona.be/en/submissions/20838290/