From d34fe9975682ac350a66f606257e5cb025b47832 Mon Sep 17 00:00:00 2001 From: JasonGrace2282 Date: Fri, 16 Aug 2024 15:08:19 -0400 Subject: [PATCH] Add an optional submission cap and allow per-student overrides --- tin/apps/assignments/admin.py | 4 ++ tin/apps/assignments/forms.py | 14 ++++- ...ssignment_submission_cap_perstudentdata.py | 31 +++++++++++ tin/apps/assignments/models.py | 43 +++++++++++++++ .../assignments/tests/test_assignments.py | 55 +++++++++++++++++++ tin/apps/assignments/urls.py | 6 ++ tin/apps/assignments/views.py | 40 +++++++++++++- tin/apps/submissions/models.py | 2 +- tin/apps/submissions/tasks.py | 5 +- tin/static/css/base.css | 12 ++-- tin/templates/assignments/manage_student.html | 11 ++++ .../assignments/manage_students.html | 27 +++++++++ tin/templates/assignments/show.html | 3 +- 13 files changed, 242 insertions(+), 11 deletions(-) create mode 100644 tin/apps/assignments/migrations/0033_assignment_submission_cap_perstudentdata.py create mode 100644 tin/templates/assignments/manage_student.html create mode 100644 tin/templates/assignments/manage_students.html diff --git a/tin/apps/assignments/admin.py b/tin/apps/assignments/admin.py index 67d9285b..313deb92 100644 --- a/tin/apps/assignments/admin.py +++ b/tin/apps/assignments/admin.py @@ -10,6 +10,7 @@ FileAction, Folder, MossResult, + PerStudentData, Quiz, QuizLogMessage, ) @@ -140,3 +141,6 @@ def match(self, obj): elif obj.match_type == "C": return f"*{obj.match_value}*" return "" + + +admin.site.register(PerStudentData) diff --git a/tin/apps/assignments/forms.py b/tin/apps/assignments/forms.py index bca6c406..01644847 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, PerStudentData logger = getLogger(__name__) @@ -66,6 +66,7 @@ class Meta: "grader_timeout", "grader_has_network_access", "has_network_access", + "submission_cap", "submission_limit_count", "submission_limit_interval", "submission_limit_cooldown", @@ -136,6 +137,7 @@ class Meta: "submission_limit_count", "submission_limit_interval", "submission_limit_cooldown", + "submission_cap", ), "collapsed": True, }, @@ -171,6 +173,7 @@ class Meta: "instructions that need to be hidden until the student enters the monitored quiz environment.", "quiz_description_markdown": "This allows adding images, code blocks, or hyperlinks to the quiz " "description.", + "submission_cap": "The maximum number of submissions that can be made. It can be overridden on a per-student basis.", } widgets = { "description": forms.Textarea(attrs={"cols": 30, "rows": 8}), @@ -241,3 +244,12 @@ class Meta: "name", ] help_texts = {"name": "Note: Folders are ordered alphabetically."} + + +class PerStudentDataForm(forms.ModelForm): + class Meta: + model = PerStudentData + fields = [ + "submission_cap", + ] + help_texts = {"submission_cap": "The maximum number of submissions that can be made."} diff --git a/tin/apps/assignments/migrations/0033_assignment_submission_cap_perstudentdata.py b/tin/apps/assignments/migrations/0033_assignment_submission_cap_perstudentdata.py new file mode 100644 index 00000000..a09695e8 --- /dev/null +++ b/tin/apps/assignments/migrations/0033_assignment_submission_cap_perstudentdata.py @@ -0,0 +1,31 @@ +# Generated by Django 4.2.14 on 2024-08-16 19:44 + +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.AddField( + model_name='assignment', + name='submission_cap', + field=models.PositiveSmallIntegerField(null=True, validators=[django.core.validators.MinValueValidator(1)]), + ), + migrations.CreateModel( + name='PerStudentData', + fields=[ + ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('submission_cap', models.PositiveSmallIntegerField(default=None, null=True)), + ('assignment', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='student_data', to='assignments.assignment')), + ('student', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to=settings.AUTH_USER_MODEL)), + ], + ), + ] diff --git a/tin/apps/assignments/models.py b/tin/apps/assignments/models.py index 1b1dfcb6..b37e03fe 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)], @@ -172,6 +173,11 @@ class Assignment(models.Model): validators=[MinValueValidator(10)], ) + submission_cap = models.PositiveSmallIntegerField( + null=True, + validators=[MinValueValidator(1)], + ) + last_action_output = models.CharField(max_length=16 * 1024, default="", null=False, blank=True) is_quiz = models.BooleanField(default=False) @@ -192,6 +198,23 @@ 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.""" + # assignments can have infinite submissions, and so can teachers. + if not student.is_student: + return True + data, created = self.student_data.get_or_create(student=student) + if created: + data.submission_cap = self.submission_cap + data.save() + + # note that this doesn't care about killed/incomplete submissions + submission_count = self.submissions.filter(student=student).count() + return submission_count < (data.submission_cap or float("inf")) + + def find_student_data(self, student) -> PerStudentData: + return self.student_data.get_or_create(student=student)[0] + 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 +394,26 @@ def quiz_issues_for_student(self, student) -> bool: ) +class PerStudentData(models.Model): + """A collection of per-student data for each :class:`.Assignment`.""" + + assignment = models.ForeignKey( + Assignment, + on_delete=models.CASCADE, + related_name="student_data", + ) + + student = models.ForeignKey( + get_user_model(), + on_delete=models.CASCADE, + ) + + submission_cap = models.PositiveSmallIntegerField(null=True, default=None) + + def __str__(self): + return f"Override for {self.student} @ Assignment {self.assignment}" + + 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..b40ede3f 100644 --- a/tin/apps/assignments/tests/test_assignments.py +++ b/tin/apps/assignments/tests/test_assignments.py @@ -24,6 +24,7 @@ def test_create_assignment(client, course) -> None: "submission_limit_cooldown": "30", "is_quiz": False, "quiz_action": "2", + "submission_cap": "100", } response = client.post( reverse("assignments:add", args=[course.id]), @@ -103,3 +104,57 @@ def test_csv_of_missing_assignment_graded(client, assignment, student): assert float(raw) == max_points assert float(final) == max_points assert formatted == "150 / 300 (50.00%)" + + +@login("student") +@pytest.mark.parametrize("is_quiz", (True, False)) +def test_submission_cap(client, assignment, student, is_quiz): + assignment.is_quiz = is_quiz + assignment.submission_cap = 1 + assignment.save() + code = "print('hello, world')" + assignment.save_grader_file(code) + + def submit(): + url = "assignments:quiz" if is_quiz else "assignments:submit" + return client.post( + reverse(url, args=[assignment.id]), + {"text": "print('I hate fun')"}, + ) + + response = submit() + assert response.status_code == 302, f"Expected status code 302, 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("is_quiz", (True, False)) +def test_submission_cap_with_override(client, assignment, student, is_quiz): + assignment.is_quiz = is_quiz + assignment.submission_cap = 1 + assignment.save() + code = "print('hello, world')" + assignment.save_grader_file(code) + + # add student override + assignment.student_data.create(student=student, submission_cap=2) + + def submit(): + url = "assignments:quiz" if is_quiz else "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 response.status_code == 302, f"Expected status code 302, got {response}" + response = submit() + assert response.status_code == 302, f"Expected status code 302, 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}" diff --git a/tin/apps/assignments/urls.py b/tin/apps/assignments/urls.py index 13924cff..07387b9a 100644 --- a/tin/apps/assignments/urls.py +++ b/tin/apps/assignments/urls.py @@ -13,6 +13,12 @@ path("/delete", views.delete_view, name="delete"), path("/grader", views.manage_grader_view, name="manage_grader"), path("/grader/download", views.download_grader_view, name="download_grader"), + path("/manage_students", views.manage_students_view, name="manage_students"), + path( + "//manage", + views.manage_student, + name="edit_student_data", + ), path("/files", views.manage_files_view, name="manage_files"), path( "/files/download/", diff --git a/tin/apps/assignments/views.py b/tin/apps/assignments/views.py index 526444d3..79b397e8 100644 --- a/tin/apps/assignments/views.py +++ b/tin/apps/assignments/views.py @@ -30,6 +30,7 @@ FolderForm, GraderScriptUploadForm, MossForm, + PerStudentDataForm, TextSubmissionForm, ) from .models import Assignment, CooldownPeriod, QuizLogMessage @@ -71,6 +72,7 @@ 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), }, ) else: @@ -274,6 +276,38 @@ def delete_view(request, assignment_id): return redirect(reverse("courses:show", args=(course.id,))) +@teacher_or_superuser_required +def manage_students_view(request, assignment_id): + assignment = Assignment.objects.get(id=assignment_id) + course = assignment.course + return render( + request, + "assignments/manage_students.html", + {"students": course.students.all(), "assignment": assignment}, + ) + + +@teacher_or_superuser_required +def manage_student(request, assignment_id, student_id): + student = get_object_or_404( + get_user_model().objects.filter(is_student=True), + id=student_id, + ) + assignment = get_object_or_404(Assignment, id=assignment_id) + data = assignment.find_student_data(student=student) + form = PerStudentDataForm(instance=data) + if request.method == "POST": + form = PerStudentDataForm(data=request.POST, instance=data) + if form.is_valid(): + form.save() + return redirect("assignments:manage_students", assignment_id) + return render( + request, + "assignments/manage_student.html", + {"form": form, "user": student, "assignment": assignment}, + ) + + @teacher_or_superuser_required def manage_grader_view(request, assignment_id): """Uploads a grader for an assignment @@ -535,6 +569,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 @@ -704,6 +740,8 @@ def quiz_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 @@ -756,8 +794,6 @@ def quiz_view(request, assignment_id): run_submission.delay(submission.id) return redirect("assignments:quiz", assignment.id) - else: - text_errors = "Submission too large" quiz_color = assignment.quiz_issues_for_student(request.user) and assignment.quiz_action == "1" diff --git a/tin/apps/submissions/models.py b/tin/apps/submissions/models.py index 42649b74..a295976b 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..607f67a4 100644 --- a/tin/apps/submissions/tasks.py +++ b/tin/apps/submissions/tasks.py @@ -31,7 +31,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: @@ -80,7 +80,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/manage_student.html b/tin/templates/assignments/manage_student.html new file mode 100644 index 00000000..5e20985c --- /dev/null +++ b/tin/templates/assignments/manage_student.html @@ -0,0 +1,11 @@ +{% extends "base.html" %} + +{% block main %} +

Managing {{ request.user.username }}

+ +
+ {% csrf_token %} + {{ form.as_p }} + +
+{% endblock %} diff --git a/tin/templates/assignments/manage_students.html b/tin/templates/assignments/manage_students.html new file mode 100644 index 00000000..d64218de --- /dev/null +++ b/tin/templates/assignments/manage_students.html @@ -0,0 +1,27 @@ +{% extends "base.html" %} +{% load static %} + +{% block title %} + Manage Students +{% endblock %} + + +{% block main %} +

Manage Students

+ +
+
    + {% for student in students %} +
  • +
    + {{ student.last_name }}, {{ student.first_name }} +
    +
    + Manage Student +
    +
    • + + {% endfor %} +
+
+{% endblock %} diff --git a/tin/templates/assignments/show.html b/tin/templates/assignments/show.html index e07a5034..91784b76 100644 --- a/tin/templates/assignments/show.html +++ b/tin/templates/assignments/show.html @@ -36,12 +36,13 @@

{% if assignment.is_quiz %}[QUIZ] {% endif %}{{ assignment.name Upload grader {% endif %} Manage files + Manage Students {% if log_file_exists %} Download log {% endif %} {% endif %} {% if not course.archived or course.permission == "w" or is_teacher or request.user.is_superuser %} - {% if assignment.is_quiz %} + {% if assignment.is_quiz and within_submission_limit %} {% if quiz_accessible or is_teacher or request.user.is_superuser %}