-
Notifications
You must be signed in to change notification settings - Fork 8
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
Quiz submission limit and Per Student Overrides #76
base: master
Are you sure you want to change the base?
Conversation
d34fe99
to
bb81025
Compare
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.
Nice feature! Here are some initial "gut reaction" questions/comments for you to consider:
@@ -535,6 +585,8 @@ def submit_view(request, assignment_id): | |||
raise http.Http404 | |||
|
|||
student = request.user | |||
if not assignment.within_submission_limit(student): | |||
return http.HttpResponseForbidden("Submission limit exceeded") |
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.
The 403 for exceeding the limit seems harsh - maybe an error message would be nicer?
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.
In theory, they shouldn't be able to even get to that screen without attempting to POST that URL or manualy entering the url.
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.
Yeah, but I figured there's a possibility that someone has Tin open on two devices (e.g. phone, pc). So, they would submit until they reach the cap on pc, forget about it, then open the page and submit on phone.
Very obscure/rare scenario and it's probably too much effort to fix the way I described, but probably replacing it with a redirect to the assignment page would be a better solution.
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.
Finally had time to come back to this PR - that would be nice, but it makes it harder to actually test the submit view. Maybe it is worth it to just create an error page 🤔
6b79415
to
7ed750a
Compare
646aa75
to
ac16e69
Compare
27f26e9
to
4698e52
Compare
This PR
Assignment
modelIt adds two new views:
Manage Students
in the assignments show view.AssignmentOverride
.TODO