diff --git a/tin/apps/assignments/admin.py b/tin/apps/assignments/admin.py index 67d9285b..3d4a62f6 100644 --- a/tin/apps/assignments/admin.py +++ b/tin/apps/assignments/admin.py @@ -12,6 +12,7 @@ MossResult, Quiz, QuizLogMessage, + SubmissionCap, ) @@ -140,3 +141,9 @@ def match(self, obj): elif obj.match_type == "C": return f"*{obj.match_value}*" return "" + + +@admin.register(SubmissionCap) +class SubmissionCapAdmin(admin.ModelAdmin): + list_display = ("assignment", "submission_cap", "student") + save_as = True diff --git a/tin/apps/assignments/forms.py b/tin/apps/assignments/forms.py index bca6c406..260b52c9 100644 --- a/tin/apps/assignments/forms.py +++ b/tin/apps/assignments/forms.py @@ -7,7 +7,7 @@ from django.conf import settings from ..submissions.models import Submission -from .models import Assignment, Folder, MossResult +from .models import Assignment, Folder, MossResult, SubmissionCap logger = getLogger(__name__) @@ -15,6 +15,9 @@ class AssignmentForm(forms.ModelForm): due = forms.DateTimeInput() + submission_cap = forms.IntegerField(min_value=1) + submission_cap_after_due = forms.IntegerField(min_value=1, required=False) + def __init__(self, course, *args, **kwargs): super().__init__(*args, **kwargs) self.fields["folder"].queryset = Folder.objects.filter(course=course) @@ -136,6 +139,8 @@ class Meta: "submission_limit_count", "submission_limit_interval", "submission_limit_cooldown", + "submission_cap", + "submission_cap_after_due", ), "collapsed": True, }, @@ -150,6 +155,10 @@ class Meta: 'internet access" below. If set, it increases the amount ' "of time it takes to start up the grader (to about 1.5 " "seconds). This is not recommended unless necessary.", + "use_submission_cap": "This enables setting a limit on the number of submissions that can be made on assignments." + "It has no effect on quizzes", + "submission_cap": "The maximum number of submissions that can be made, or empty for unlimited.", + "submission_cap_after_due": "The maximum number of submissions that can be made after the due date, or empty for unlimited.", "submission_limit_count": "", "submission_limit_interval": "Tin sets rate limits on submissions. If a student tries " "to submit too many submissions in a given interval, " @@ -180,6 +189,21 @@ class Meta: def __str__(self) -> str: return f"AssignmentForm(\"{self['name'].value()}\")" + def save(self, **kwargs) -> Assignment: + assignment = super().save(**kwargs) + sub_cap = self.cleaned_data.get("submission_cap") + sub_cap_after_due = self.cleaned_data.get("submission_cap_after_due") + if sub_cap is not None or sub_cap_after_due is not None: + SubmissionCap.objects.update_or_create( + assignment=assignment, + student=None, + defaults={ + "submission_cap": sub_cap, + "submission_cap_after_due": sub_cap_after_due, + }, + ) + return assignment + class GraderScriptUploadForm(forms.Form): grader_file = forms.FileField( @@ -241,3 +265,21 @@ class Meta: "name", ] help_texts = {"name": "Note: Folders are ordered alphabetically."} + + +class SubmissionCapForm(forms.ModelForm): + def __init__(self, **kwargs) -> None: + super().__init__(**kwargs) + + nonrequired = ("submission_cap", "submission_cap_after_due") + for f in nonrequired: + self.fields[f].required = False + + class Meta: + model = SubmissionCap + fields = ["submission_cap", "submission_cap_after_due"] + help_texts = { + "submission_cap_after_due": "The submission cap after the due date (or empty for unlimited)", + "student": "The student to apply the cap to.", + } + labels = {"submission_cap_after_due": "Submission cap after due date"} diff --git a/tin/apps/assignments/migrations/0033_submissioncap_submissioncap_unique_type_and_more.py b/tin/apps/assignments/migrations/0033_submissioncap_submissioncap_unique_type_and_more.py new file mode 100644 index 00000000..794473ca --- /dev/null +++ b/tin/apps/assignments/migrations/0033_submissioncap_submissioncap_unique_type_and_more.py @@ -0,0 +1,35 @@ +# Generated by Django 4.2.16 on 2025-01-10 01:57 + +from django.conf import settings +import django.core.validators +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ('assignments', '0032_assignment_quiz_description_and_more'), + ] + + operations = [ + migrations.CreateModel( + name='SubmissionCap', + fields=[ + ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('submission_cap', models.PositiveSmallIntegerField(null=True, validators=[django.core.validators.MinValueValidator(1)])), + ('submission_cap_after_due', models.PositiveSmallIntegerField(null=True)), + ('assignment', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='submission_caps', to='assignments.assignment')), + ('student', models.ForeignKey(null=True, on_delete=django.db.models.deletion.CASCADE, to=settings.AUTH_USER_MODEL)), + ], + ), + migrations.AddConstraint( + model_name='submissioncap', + constraint=models.UniqueConstraint(fields=('student', 'assignment'), name='unique_type'), + ), + migrations.AddConstraint( + model_name='submissioncap', + constraint=models.CheckConstraint(check=models.Q(('submission_cap__isnull', False), ('submission_cap_after_due__isnull', False), _connector='OR'), name='has_submission_cap', violation_error_message='Either the submission cap before or after the due date has to be set'), + ), + ] diff --git a/tin/apps/assignments/models.py b/tin/apps/assignments/models.py index 1b1dfcb6..dbc95fac 100644 --- a/tin/apps/assignments/models.py +++ b/tin/apps/assignments/models.py @@ -159,6 +159,7 @@ class Assignment(models.Model): has_network_access = models.BooleanField(default=False) + # WARNING: this is the rate limit submission_limit_count = models.PositiveIntegerField( default=90, validators=[MinValueValidator(10)], @@ -183,6 +184,8 @@ class Assignment(models.Model): objects = AssignmentQuerySet.as_manager() + submission_caps: models.QuerySet[SubmissionCap] + def __str__(self): return self.name @@ -192,6 +195,61 @@ def get_absolute_url(self): def __repr__(self): return self.name + def within_submission_limit(self, student) -> bool: + """Check if a student is within the submission limit for an assignment.""" + # teachers should have infinite submissions + if not student.is_student or not self.submission_caps.exists(): + return True + + # note that this doesn't care about killed/incomplete submissions + submission_count = self.submissions.filter(student=student).count() + + cap = self.find_submission_cap(student) + return submission_count < cap + + def find_submission_cap(self, student) -> float: + """Given a student, find the submission cap. + + This takes into account student overrides, and due dates. + """ + if student.is_superuser or student.is_teacher: + return float("inf") + cap = self.find_student_override(student) + if cap is not None: + if timezone.localtime() > self.due: + return cap.submission_cap_after_due + return cap.submission_cap + elif timezone.localtime() > self.due: + return self.submission_cap_after_due() + return self.before_submission_cap() + + def find_student_override(self, student) -> SubmissionCap | None: + """Find an :class:`.SubmissionCap` for a student. + + Returns ``None`` if no override exists. + """ + return self.submission_caps.filter(student=student).first() + + def before_submission_cap(self) -> float: + """Get the submission cap for an assignment before the due date. + + Returns ``float("inf")`` if no cap is found. + """ + cap = self.submission_caps.filter(student__isnull=True).first() + if cap is not None: + return cap.submission_cap + return float("inf") + + def submission_cap_after_due(self) -> float: + """Get the submission cap after the due date. + + Returns ``float("inf")`` if no cap is found. + """ + cap = self.submission_caps.filter(student__isnull=True).first() + if cap is not None: + return cap.submission_cap + return float("inf") + def make_assignment_dir(self) -> None: """Creates the directory where the assignment grader scripts go.""" assignment_path = os.path.join(settings.MEDIA_ROOT, f"assignment-{self.id}") @@ -371,6 +429,39 @@ def quiz_issues_for_student(self, student) -> bool: ) +class SubmissionCap(models.Model): + """Submission cap information""" + + student = models.ForeignKey( + settings.AUTH_USER_MODEL, + on_delete=models.CASCADE, + null=True, + ) + + assignment = models.ForeignKey( + "Assignment", + on_delete=models.CASCADE, + related_name="submission_caps", + ) + + submission_cap = models.PositiveSmallIntegerField(null=True, validators=[MinValueValidator(1)]) + submission_cap_after_due = models.PositiveSmallIntegerField(null=True) + + class Meta: + constraints = [ + # TODO: In django 5.0+ add nulls_distinct=False + models.UniqueConstraint(fields=["student", "assignment"], name="unique_type"), + models.CheckConstraint( + check=Q(submission_cap__isnull=False) | Q(submission_cap_after_due__isnull=False), + violation_error_message="Either the submission cap before or after the due date has to be set", + name="has_submission_cap", + ), + ] + + def __str__(self) -> str: + return f"{type(self).__name__}(submission_cap={self.submission_cap})" + + class CooldownPeriod(models.Model): assignment = models.ForeignKey( Assignment, diff --git a/tin/apps/assignments/tests/test_assignments.py b/tin/apps/assignments/tests/test_assignments.py index 2e456587..53977a6d 100644 --- a/tin/apps/assignments/tests/test_assignments.py +++ b/tin/apps/assignments/tests/test_assignments.py @@ -24,6 +24,9 @@ def test_create_assignment(client, course) -> None: "submission_limit_cooldown": "30", "is_quiz": False, "quiz_action": "2", + "submission_cap": "100", + "submission_cap_after_due": "100", + "use_submission_cap": False, } response = client.post( reverse("assignments:add", args=[course.id]), diff --git a/tin/apps/assignments/tests/test_submission_cap.py b/tin/apps/assignments/tests/test_submission_cap.py new file mode 100644 index 00000000..b0e06817 --- /dev/null +++ b/tin/apps/assignments/tests/test_submission_cap.py @@ -0,0 +1,153 @@ +from __future__ import annotations + +from datetime import datetime, timedelta + +import pytest +from django.urls import reverse + +from tin.tests import is_redirect, login + + +@login("student") +def test_submission_cap(client, assignment): + assignment.use_submission_cap = True + assignment.submission_cap = 1 + assignment.submission_cap_after_due = 1 + assignment.due = datetime.today() + timedelta(days=1) + assignment.save() + code = "print('hello, world')" + assignment.save_grader_file(code) + + def submit(): + url = "assignments:submit" + return client.post( + reverse(url, args=[assignment.id]), + {"text": "print('I hate fun')"}, + ) + + response = submit() + assert is_redirect(response), f"Expected redirect, got {response}" + + # but now we've passed the cap + response = submit() + assert response.status_code == 403, f"Expected status code 403, got {response}" + + +@login("student") +def test_submission_cap_overdue(client, assignment): + assignment.use_submission_cap = True + assignment.submission_cap = 10000 + assignment.submission_cap_after_due = 1 + assignment.due = datetime.today() - timedelta(days=1) + assignment.save() + code = "print('hello, world')" + assignment.save_grader_file(code) + + def submit(): + url = "assignments:submit" + return client.post( + reverse(url, args=[assignment.id]), + {"text": "print('I hate fun')"}, + ) + + response = submit() + assert is_redirect(response), f"Expected redirect, got {response}" + + # but now we've passed the cap + response = submit() + assert response.status_code == 403, f"Expected status code 403, got {response}" + + +@login("student") +@pytest.mark.parametrize( + "due", (datetime.today() - timedelta(days=1), datetime.today() + timedelta(days=1)) +) +def test_submission_cap_with_override(client, assignment, student, due): + """Test the submission cap with a per student override. + + The override should take place regardless of the due date of the :class:`.Assignment`. + """ + assignment.due = due + assignment.use_submission_cap = True + assignment.submission_cap = 1 + assignment.submission_cap_after_due = 1 + assignment.save() + code = "print('hello, world')" + assignment.save_grader_file(code) + + # add student override + assignment.student_overrides.create(student=student, submission_cap=2) + + def submit(): + url = "assignments:submit" + return client.post( + reverse(url, args=[assignment.id]), + {"text": "print('I hate fun')"}, + ) + + # first and second submission should be fine + response = submit() + assert is_redirect(response), f"Expected redirect, got {response}" + response = submit() + assert is_redirect(response), f"Expected redirect, got {response}" + + # but now we've passed the cap for the student + response = submit() + assert response.status_code == 403, f"Expected status code 403, got {response}" + + +@login("student") +def test_submission_cap_on_quiz(client, quiz): + quiz.use_submission_cap = True + quiz.submission_cap = 1 + quiz.submission_cap_after_due = 1 + quiz.save() + code = "print('hello, world')" + quiz.save_grader_file(code) + + def submit(): + url = "assignments:quiz" + return client.post( + reverse(url, args=[quiz.id]), + {"text": "print('I hate fun')"}, + ) + + response = submit() + assert is_redirect(response), f"Expected redirect, got {response}" + response = submit() + assert is_redirect(response), f"Expected submission cap to not affect quizzes (got {response})" + + +@login("student") +def test_submission_cap_not_used(client, assignment): + assignment.use_submission_cap = False + assignment.submission_cap = 1 + assignment.submission_cap_after_due = 1 + assignment.due = datetime.today() + timedelta(days=1) + assignment.save() + + def submit(): + url = "assignments:submit" + return client.post( + reverse(url, args=[assignment.id]), + {"text": "print('I hate fun')"}, + ) + + # all submissions should be fine because use_submission_cap is False + response = submit() + assert is_redirect(response), f"Expected redirect, got {response}" + response = submit() + assert is_redirect(response), f"Expected redirect, got {response}" + + +@login("teacher") +def test_create_student_override(client, assignment, student): + response = client.post( + reverse("assignments:create_student_override", args=[assignment.id, student.id]), + {"submission_cap": 2}, + ) + assert is_redirect(response), f"Expected redirect, got {response}" + + override = assignment.find_student_override(student) + assert override is not None + assert override.submission_cap == 2 diff --git a/tin/apps/assignments/views.py b/tin/apps/assignments/views.py index eaf93ef3..6f93abd3 100644 --- a/tin/apps/assignments/views.py +++ b/tin/apps/assignments/views.py @@ -30,6 +30,7 @@ FolderForm, GraderScriptUploadForm, MossForm, + SubmissionCapForm, TextSubmissionForm, ) from .models import Assignment, CooldownPeriod, QuizLogMessage @@ -58,6 +59,15 @@ def show_view(request, assignment_id): publishes = PublishedSubmission.objects.filter(student=request.user, assignment=assignment) graded_submission = publishes.latest().submission if publishes else latest_submission + submission_limit = assignment.find_submission_cap(request.user) + submissions_left = submission_limit - len(submissions) + submissions_left = max(submissions_left, 0) + + if submissions_left == float("inf"): + submissions_left = None + if submission_limit == float("inf"): + submission_limit = None + return render( request, "assignments/show.html", @@ -71,6 +81,9 @@ def show_view(request, assignment_id): "is_student": course.is_student_in_course(request.user), "is_teacher": request.user in course.teacher.all(), "quiz_accessible": quiz_accessible, + "within_submission_limit": assignment.within_submission_limit(request.user), + "submissions_left": submissions_left, + "submission_limit": submission_limit, }, ) else: @@ -173,11 +186,25 @@ def show_view(request, assignment_id): publishes = PublishedSubmission.objects.filter(student=request.user, assignment=assignment) latest_submission = submissions.latest() if submissions else None graded_submission = publishes.latest().submission if publishes else latest_submission + + submission_limit = assignment.find_submission_cap(request.user) + submissions_left = submission_limit - len(submissions) + + # render properly after submission cap is lowered (such as when the due date is passed) + if submission_limit == float("inf"): + submission_limit = None + submissions_left = max(submissions_left, 0) + if submissions_left == float("inf"): + submissions_left = None + context.update( { "submissions": submissions.order_by("-date_submitted"), "latest_submission": latest_submission, "graded_submission": graded_submission, + "within_submission_limit": assignment.within_submission_limit(request.user), + "submissions_left": submissions_left, + "submission_limit": submission_limit, } ) @@ -491,6 +518,15 @@ def student_submissions_view(request, assignment_id, student_id): ) student = get_object_or_404(User, id=student_id) + cap = assignment.submission_caps.filter(student=student).first() + form = SubmissionCapForm(instance=cap) + if request.method == "POST": + form = SubmissionCapForm(data=request.POST, instance=cap) + if form.is_valid(): + form.instance.assignment = assignment + form.instance.student = student + form.save() + submissions = Submission.objects.filter(student=student, assignment=assignment) publishes = PublishedSubmission.objects.filter(student=student, assignment=assignment) latest_submission = submissions.latest() if submissions else None @@ -514,6 +550,7 @@ def student_submissions_view(request, assignment_id, student_id): "latest_submission": latest_submission, "published_submission": published_submission, "log_messages": log_messages, + "form": form, }, ) @@ -535,6 +572,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") submissions = Submission.objects.filter(student=student, assignment=assignment) latest_submission = submissions.latest() if submissions else None diff --git a/tin/apps/submissions/models.py b/tin/apps/submissions/models.py index 513a9350..601e04a3 100644 --- a/tin/apps/submissions/models.py +++ b/tin/apps/submissions/models.py @@ -199,7 +199,7 @@ def file_text(self): return None try: - with open(self.backup_file_path) as f: + with open(self.backup_file_path, encoding="utf-8") as f: file_text = f.read() except OSError: file_text = "[Error accessing submission file]" diff --git a/tin/apps/submissions/tasks.py b/tin/apps/submissions/tasks.py index 50f56156..9e997bd1 100644 --- a/tin/apps/submissions/tasks.py +++ b/tin/apps/submissions/tasks.py @@ -7,6 +7,7 @@ import shutil import signal import subprocess +import sys import time import traceback from decimal import Decimal @@ -31,7 +32,7 @@ def truncate_output(text, field_name): @shared_task -def run_submission(submission_id): +def run_submission(submission_id: int) -> None: submission = Submission.objects.get(id=submission_id) try: @@ -65,7 +66,7 @@ def run_submission(submission_id): python_exe = ( os.path.join(submission.assignment.venv.path, "bin", "python") if submission.assignment.venv_fully_created - else "/usr/bin/python3.10" + else sys.executable ) if not settings.DEBUG or shutil.which("bwrap") is not None: @@ -80,7 +81,8 @@ def run_submission(submission_id): "wrappers", folder_name, f"{submission.assignment.language}.txt", - ) + ), + encoding="utf-8", ) as wrapper_file: wrapper_text = wrapper_file.read().format( has_network_access=bool(submission.assignment.has_network_access), diff --git a/tin/static/css/base.css b/tin/static/css/base.css index b7c2a7f9..dd4d462e 100644 --- a/tin/static/css/base.css +++ b/tin/static/css/base.css @@ -150,7 +150,8 @@ ul.no-list { ul#course-list, ul#assignment-list, ul#venv-list, -ul#archive-list { +ul#archive-list, +ul.linebreak-list { padding-left: 0px; margin-left: 0px; } @@ -158,7 +159,8 @@ ul#archive-list { ul#course-list > li, ul#assignment-list > li, ul#venv-list > li, -ul#archive-list > li { +ul#archive-list > li, +ul.linebreak-list > li { display: flex; padding: 10px 0px; border-top: 1px solid lightgray; @@ -166,7 +168,8 @@ ul#archive-list > li { ul#course-list > li:last-child, ul#assignment-list > li:last-child, -ul#venv-list > li:last-child { +ul#venv-list > li:last-child, +ul.linebreak-last > li:last-child { border-bottom: 1px solid lightgray; } @@ -204,7 +207,8 @@ ul#venv-list > li .left { ul#course-list > li .right, ul#assignment-list > li .right, ul#venv-list > li .right, -ul#archive-list > li .right { +ul#archive-list > li .right, +ul.linebreak-list > li .right { margin-left: auto; flex: 0 0 auto; text-align: right; diff --git a/tin/templates/assignments/show.html b/tin/templates/assignments/show.html index 460fd9ab..44e32a2e 100644 --- a/tin/templates/assignments/show.html +++ b/tin/templates/assignments/show.html @@ -47,7 +47,7 @@

{% if assignment.is_quiz %}[QUIZ] {% endif %}{{ assignment.name onclick="return confirm('You are about to begin an assessment. All activity on your device will be monitored. If you open any other tabs or attempt to use any other applications, your quiz might be stopped and your teacher will be notified.')" href="{% url 'assignments:quiz' assignment.id %}">Begin quiz {% endif %} - {% else %} + {% elif within_submission_limit %} Submit {% endif %} {% endif %} @@ -87,6 +87,12 @@

{% if assignment.is_quiz %}[QUIZ] {% endif %}{{ assignment.name {% endif %} {% endif %} + {% if submissions_left is not None %} + + Submissions: + {{ submissions_left }} out of {{ submission_limit }} + + {% endif %} {% if is_teacher or request.user.is_superuser %} diff --git a/tin/templates/assignments/student_submissions.html b/tin/templates/assignments/student_submissions.html index 3d0976f5..aecdc3e6 100644 --- a/tin/templates/assignments/student_submissions.html +++ b/tin/templates/assignments/student_submissions.html @@ -111,4 +111,29 @@

All Submissions

+ +

Edit Submission Cap

+ To change the submission cap for this student, enter a new cap below. +
+ {% csrf_token %} + {{ form.errors }} +
+ {% for field in form.visible_fields %} +
+ {{ field.label }} + + {{ field }} + {% if field.help_text %} +
{{ field.help_text }} + {% endif %} +
+
+ {% endfor %} + {% for field in form.hidden_fields %} + {{ field }} + {% endfor %} +
+ +
+ {% endblock %}