Skip to content
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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

JasonGrace2282
Copy link
Member

@JasonGrace2282 JasonGrace2282 commented Aug 21, 2024

This PR

  • adds an optional limit to the amount of submissions on a assignment
  • allows for a different limit after the assignment is due
  • allows per student overrides for specific pieces of data on the Assignment model
  • adds tests to verify behavior

It adds two new views:

  • Manage Students: allows for a listing of students in the course and a button to manage their individual overrides. Accessible from Manage Students in the assignments show view.
  • Manage a specific student: form view for AssignmentOverride.

TODO

  • Implement the feature
  • Add tests
  • Add more tests (I don't trust myself)
  • Improve form CSS

@JasonGrace2282 JasonGrace2282 marked this pull request as draft September 1, 2024 23:28
@JasonGrace2282 JasonGrace2282 marked this pull request as ready for review September 4, 2024 02:16
Copy link
Member

@krishnans2006 krishnans2006 left a 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")
Copy link
Member

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?

Copy link
Member Author

@JasonGrace2282 JasonGrace2282 Jan 9, 2025

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.

Copy link
Member

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.

Copy link
Member Author

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 🤔

@JasonGrace2282 JasonGrace2282 added the complex PRs or issues requiring substantial effort to implement and/or review label Feb 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
complex PRs or issues requiring substantial effort to implement and/or review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants