Merge branch 'master' into fix_lti_issues

aplus/celery.py

@@ -5,6 +5,7 @@
 import logging
 from dateutil.relativedelta import relativedelta
 from time import sleep
+from random import choice
 
 from django.conf import settings
 
@@ -49,13 +50,23 @@ def retry_submissions():
     from exercise.submission_models import PendingSubmission
 
     # Recovery state: only send one grading request to probe the state of grader
-    # We pick the one with most attempts, so that total_retries comes down more quickly
-    # when things are back to normal, and we can return to normal state
     if not PendingSubmission.objects.is_grader_stable():
-        pending = PendingSubmission.objects.order_by('-num_retries').first()
-        # pylint: disable-next=logging-fstring-interpolation
-        logging.info(f"Recovery state: retrying expired submission {pending.submission}")
-        pending.submission.exercise.grade(pending.submission)
+        # Get ids of all pending submissions and randomly load one to be retried
+        # (do not load all the submissions objects to save memory)
+        submission_ids = PendingSubmission.objects.values_list('id',flat=True)
+        random_choice = choice(submission_ids)
+        pending = PendingSubmission.objects.get(pk=random_choice)
+        if pending.num_retries >= settings.SUBMISSION_RETRY_LIMIT and settings.SUBMISSION_RETRY_LIMIT > 0:
+            logger.info("Recovery state: submission retry limit exceeded for submission %s - removing from pending",
+                        pending.submission)
+            pending.submission.set_error()
+            pending.submission.save()
+            pending.delete()
+        else:
+            if pending.submission.exercise.can_regrade:
+                logger.info("Recovery state: retrying expired submission %s (retries: %s)",
+                            pending.submission, pending.num_retries)
+                pending.submission.exercise.grade(pending.submission)
         return
 
     # Stable state: retry all expired submissions
@@ -66,7 +77,21 @@ def retry_submissions():
 
     for pending in expired:
         if pending.submission.exercise.can_regrade:
-            # pylint: disable-next=logging-fstring-interpolation
-            logger.info(f"Retrying expired submission {pending.submission}")
-            pending.submission.exercise.grade(pending.submission)
-            sleep(0.5)  # Delay 500 ms to avoid choking grader
+            # Do not retry submission until SUBMISSION_EXPIRY_TIMEOUT * num_retries has passed
+            pending_timelimit = datetime.datetime.now(datetime.timezone.utc) - relativedelta(
+                seconds=settings.SUBMISSION_EXPIRY_TIMEOUT*pending.num_retries
+            )
+            if pending.num_retries < settings.SUBMISSION_RETRY_LIMIT:
+                if pending.submission_time < pending_timelimit:
+                    logger.info("Retrying expired submission %s (retries: %s)",
+                                pending.submission, pending.num_retries)
+                    pending.submission.exercise.grade(pending.submission)
+                    sleep(0.5)  # Delay 500 ms to avoid choking grader
+                else:
+                    logger.info("Not yet retrying submission %s (retries: %s)",
+                                pending.submission, pending.num_retries)
+            else:
+                logger.info("Could not grade submission %s (maximum retries exceeded).", pending.submission)
+                pending.submission.set_error()
+                pending.submission.save()
+                pending.delete()

aplus/settings.py

@@ -280,6 +280,9 @@
 # Network location is sufficient, e.g. "localhost:8080" or "grader.cs.aalto.fi"
 SUBMISSION_RETRY_SERVICES = []
 
+# Maximum number of retries to automatically grade a given submission
+SUBMISSION_RETRY_LIMIT = 3
+
 # Number of unresponded retries beyond which we move to recovery state.
 # In recovery state there likely is more persistent problem with the grader
 # or network that needs fixing.

assets/sass/legacy/_main.scss

@@ -234,8 +234,8 @@ file for all the components */
 }
 .progress .required-points {
     position: absolute;
-    height: 20px;
-    border-left: 1px solid black;
+    height: 100%;
+    border-left: 2px solid black;
 }
 .glyphicon.red {
   color: #ff7070;

course/migrations/0060_studentmodulegoal.py

@@ -0,0 +1,24 @@
+# Generated by Django 4.2.13 on 2024-10-08 08:17
+
+from django.db import migrations, models
+import django.db.models.deletion
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('userprofile', '0006_auto_20210812_1536'),
+        ('course', '0059_submissiontag'),
+    ]
+
+    operations = [
+        migrations.CreateModel(
+            name='StudentModuleGoal',
+            fields=[
+                ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
+                ('goal_points', models.IntegerField(default=0)),
+                ('module', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='course.coursemodule')),
+                ('student', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='userprofile.userprofile')),
+            ],
+        ),
+    ]

course/models.py

@@ -1482,6 +1482,11 @@ def number_of_submitters(self):
             .filter(submissions__exercise__course_module=self).distinct().count()
 
 
+class StudentModuleGoal(models.Model):
+    student = models.ForeignKey(UserProfile, on_delete=models.CASCADE)
+    module = models.ForeignKey(CourseModule, on_delete=models.CASCADE)
+    goal_points = models.IntegerField(default=0)
+
 class LearningObjectCategory(models.Model):
     """
     Learning objects may be grouped to different categories.

e2e_tests/test_points_goal_set.py

@@ -0,0 +1,42 @@
+from playwright.sync_api import Page, expect
+
+from e2e_tests.helpers import login
+
+
+def test_points_goal_set(page: Page) -> None:
+    login(page, "student", "student")
+
+    page.get_by_role("link", name="Def. Course Current DEF000 1.").click()
+    page.get_by_role("link", name="Your points").click()
+    page.locator("#progress-questionnaires").get_by_role("button", name="Points goal").click()
+    page.get_by_label("Input personalized goal as").fill("50")
+    page.get_by_label("Input personalized goal as").press("Tab")
+    page.get_by_role("button", name="Save").click()
+    expect(page.locator("#success-alert")).to_contain_text("Succesfully set personalized points goal")
+    page.get_by_role("button", name="Close", exact=True).click()
+    expect(page.get_by_text("Points goal: 50"))
+    expect(page.locator("#goal-points"))
+
+
+def test_points_goal_reached(page: Page) -> None:
+    login(page, "student", "student")
+    page.get_by_role("link", name="Def. Course Current DEF000 1.").click()
+    page.get_by_role("link", name="Creating questionnaire exercises").click()
+    page.locator("label").filter(has_text="2").first.click()
+    page.locator("label").filter(has_text="an integer").first.click()
+    page.locator("div:nth-child(4) > div:nth-child(6) > label").click()
+    page.locator("label").filter(has_text="-").nth(1).click()
+    page.locator("label").filter(has_text="an integer").nth(1).click()
+    page.locator("div:nth-child(5) > div:nth-child(6) > label").click()
+    page.locator("label").filter(has_text="-").nth(3).click()
+    page.locator("#chapter-exercise-1").get_by_role("button", name="Submit").click()
+    expect(page.get_by_label("5.1.1 Single-choice and")).to_contain_text("30 / 40")
+    page.get_by_role("link", name="Your points").click()
+    page.locator("#progress-questionnaires").get_by_role("button", name="Points goal").click()
+    page.get_by_label("Input personalized goal as").fill("30")
+    page.get_by_role("button", name="Save").click()
+    expect(page.locator("#success-alert")).to_contain_text("Succesfully set personalized points goal")
+    page.get_by_role("button", name="Close", exact=True).click()
+    progress_bar_locator = page.locator("#progress-questionnaires > .progress > .aplus-progress-bar")
+    expect(progress_bar_locator).\
+        to_have_class("aplus-progress-bar aplus-progress-bar-striped aplus-progress-bar-primary")

edit_course/exercise_forms.py

@@ -96,12 +96,23 @@ class RevealRuleForm(FieldsetModelForm):
 
     class Meta:
         model = RevealRule
-        fields = ['trigger', 'delay_minutes', 'time', 'currently_revealed']
+        fields = ['trigger', 'delay_minutes', 'time', 'currently_revealed', 'show_zero_points_immediately']
         widgets = {'time': DateTimeLocalInput}
 
-    def __init__(self, *args: Any, **kwargs: Any) -> None:
+    def __init__(self, *args: Any, is_submission_feedback: bool = False, **kwargs: Any) -> None:
         super().__init__(*args, **kwargs)
         self.fields['trigger'].widget.attrs['data-trigger'] = True
+
+        zero_feedback_triggers = [
+            RevealRule.TRIGGER.DEADLINE.value,
+            RevealRule.TRIGGER.DEADLINE_ALL.value,
+            RevealRule.TRIGGER.DEADLINE_OR_FULL_POINTS.value,
+            RevealRule.TRIGGER.TIME.value,
+            RevealRule.TRIGGER.MANUAL.value,
+            RevealRule.TRIGGER.COMPLETION.value,
+        ] if is_submission_feedback else []
+        self.fields['show_zero_points_immediately'].widget.attrs['data-visible-triggers'] = zero_feedback_triggers
+
         # Visibility rules for the form fields. Each of the following fields is
         # only visible when one of their specified values is selected from the
         # trigger dropdown. See edit_model.html.
@@ -147,11 +158,13 @@ def __init__(self, *args: Any, **kwargs: Any) -> None:
             data=kwargs.get('data'),
             instance=self.instance.active_submission_feedback_reveal_rule,
             prefix='submission_feedback',
+            is_submission_feedback=True,
         )
         self.model_solutions_form = RevealRuleForm(
             data=kwargs.get('data'),
             instance=self.instance.active_model_solutions_reveal_rule,
             prefix='model_solutions',
+            is_submission_feedback=False,
         )
 
     def get_fieldsets(self) -> List[Dict[str, Any]]:

edit_course/operations/configure.py

@@ -132,6 +132,8 @@ def parse_reveal_rule(
         rule.time = parse_date(rule_config["time"], errors)
     if "delay_minutes" in rule_config:
         rule.delay_minutes = parse_int(rule_config["delay_minutes"], errors)
+    if "show_zero_points_immediately" in rule_config:
+        rule.show_zero_points_immediately = rule_config["show_zero_points_immediately"]
     rule.save()
     return rule
 

exercise/api/csv/views.py

@@ -158,6 +158,8 @@ def submitted_field(submission, name):
         response = Response(data)
         if isinstance(getattr(request, 'accepted_renderer'), CSVRenderer):
             response['Content-Disposition'] = 'attachment; filename="submissions.csv"'
+        else:
+            response['Content-Disposition'] = 'attachment; filename="submissions.json"'
         return response
 
     def get_renderer_context(self):

exercise/api/views.py

@@ -1,11 +1,13 @@
+from io import BytesIO
+import zipfile
+
 from aplus_auth.payload import Permission
 from django.core.exceptions import PermissionDenied
-from django.http.response import HttpResponse
+from django.http.response import HttpResponse, FileResponse
 from django.utils import timezone
 from django.utils.translation import gettext_lazy as _
 from wsgiref.util import FileWrapper
-from rest_framework import mixins, permissions, viewsets
-from rest_framework import status
+from rest_framework import mixins, permissions, status, viewsets
 from rest_framework.response import Response
 from rest_framework.reverse import reverse
 from rest_framework.decorators import action
@@ -28,6 +30,7 @@
 from course.api.mixins import CourseResourceMixin
 from exercise.async_views import _post_async_submission
 
+from ..cache.points import ExercisePoints
 from ..models import (
     Submission,
     SubmittedFile,
@@ -322,6 +325,107 @@ def submit(self, request, *args, **kwargs):
 
         return Response(data, status=status_code, headers=headers)
 
+    @action(
+        detail=False,
+        url_path='zip',
+        url_name='zip',
+        methods=['get'],
+    )
+    def zip(self, request, exercise_id, *args, **kwargs): # noqa: MC0001
+        if not self.instance.is_teacher(request.user):
+            return Response(
+                'Only a teacher can download submissions via this API',
+                status=status.HTTP_403_FORBIDDEN,
+        )
+        exercise = None
+        try:
+            exercise = BaseExercise.objects.get(id=exercise_id)
+        except BaseExercise.DoesNotExist:
+            return Response('Exercise not found', status=status.HTTP_404_NOT_FOUND)
+
+        best = request.query_params.get('best') == 'yes'
+        submissions = Submission.objects.filter(exercise__id=exercise_id).order_by('submission_time')
+
+        def get_group_id(submission):
+            group_id = None
+            if 'group' in submission.meta_data:
+                group_id = submission.meta_data['group']
+            if group_id is None:
+                for lst in submission.submission_data:
+                    if '_aplus_group' in lst:
+                        group_id = lst[1]
+                        break
+            return group_id
+
+        def handle_submission(submission, submitters, info_csv):
+            group_id = None
+            if len(submitters) > 1:
+                group_id = get_group_id(submission)
+                if group_id is not None:
+                    try:
+                        group_id = int(group_id)
+                    except ValueError:
+                        return info_csv
+            submission_time = submission.submission_time.strftime('%Y-%m-%d %H:%M:%S %z')
+            submitted_files = SubmittedFile.objects.filter(submission=submission)
+            student_ids = sorted([submitter.student_id for submitter in submitters])
+            submitters_string = '+'.join(student_ids)
+            submission_num = list(
+                dict.fromkeys( # Remove duplicates
+                    Submission.objects.filter(exercise__id=exercise_id, submitters__in=submitters)
+                    .order_by('submission_time')
+                )
+            ).index(submission) + 1
+            for i, submitted_file in enumerate(submitted_files, start=1):
+                filename = f"{submitters_string}_file{i}_submission{submission_num}"
+                try:
+                    with submitted_file.file_object.file.open('rb') as file:
+                        zip_file.writestr(f'{filename}', file.read())
+                    if group_id is not None:
+                        info_csv += f"{filename},group{group_id},{submission_time}\n"
+                    else:
+                        info_csv += f"{filename},{submitters_string},{submission_time}\n"
+                except OSError:
+                    pass
+            return info_csv
+
+        # Create a zip file in memory
+        zip_buffer = BytesIO()
+        with zipfile.ZipFile(zip_buffer, 'w') as zip_file:
+            info_csv = "filename,label,created_at\n"
+            if best:
+                unique_submitters = []
+                for submission in submissions:
+                    submitters = submission.submitters.all()
+                    if any(self.instance.is_course_staff(submitter.user) for submitter in submitters):
+                        # Skip staff submissions
+                        continue
+                    for submitter in submitters:
+                        if submitter not in unique_submitters:
+                            unique_submitters.append(submitter)
+                unique_submissions = []
+                for submitter in unique_submitters:
+                    submission_entry = ExercisePoints.get(exercise, submitter).best_submission
+                    if submission_entry is None:
+                        continue
+                    submission = submissions.filter(id=submission_entry.id).first()
+                    # Prevent duplicate best submissions due to group submissions
+                    if submission not in unique_submissions:
+                        unique_submissions.append(submission)
+                        info_csv = handle_submission(submission, submission.submitters.all(), info_csv)
+            else:
+                for submission in submissions:
+                    submitters = submission.submitters.all()
+                    if any(self.instance.is_course_staff(submitter.user) for submitter in submitters):
+                        # Skip staff submissions
+                        continue
+                    info_csv = handle_submission(submission, submitters, info_csv)
+            zip_file.writestr('info.csv', info_csv)
+        zip_buffer.seek(0)
+
+        response = FileResponse(zip_buffer, as_attachment=True, filename='submissions.zip')
+        return response
+
     def get_access_mode(self):
         # The API is not supposed to use the access mode permission in views,
         # but this is currently required so that enrollment exercises work in