Run extra checks (#312)

* chore: celery #206

* chore: support patch for project 2

* refactor!: rework checks and submissions

* chore: move is_valid to submission

* chore: hook structure checks

* chore: merge migrations

.dev.env

@@ -2,7 +2,7 @@
 PUID=1000
 PGID=1000
 TZ=Europe/Brussels
-FIXTURE=small # Options ["", small, medium, large]
+FIXTURE=small
 
 # File directories
 DATA_DIR="./data"
@@ -15,11 +15,11 @@ REDIS_IP=192.168.90.10
 REDIS_PORT=6379
 
 # Django
-DJANGO_SECRET_KEY="" # Set to a random string
-DJANGO_DEBUG=True # Django debug mode
-DJANGO_DOMAIN_NAME=localhost # Domain name for the Django server
-DJANGO_CAS_URL_PREFIX="" # URL prefix for the CAS server. Should be empty for development
-DJANGO_CAS_PORT=8080 # Port for the CAS server. Should be 8080 if DJANGO_DOMAIN_NAME is localhost
-DJANGO_DB_ENGINE=django.db.backends.sqlite3 # Database engine
-DJANGO_REDIS_HOST=${REDIS_IP} # Redis configuration
+DJANGO_SECRET_KEY=""
+DJANGO_DEBUG=True
+DJANGO_DOMAIN_NAME=localhost
+DJANGO_CAS_URL_PREFIX=""
+DJANGO_CAS_PORT=8080
+DJANGO_DB_ENGINE="django.db.backends.sqlite3"
+DJANGO_REDIS_HOST=${REDIS_IP}
 DJANGO_REDIS_PORT=${REDIS_PORT}

.prod.env

@@ -14,23 +14,23 @@ POSTGRES_IP=192.168.90.9
 POSTGRES_PORT=5432
 POSTGRES_DB=selab
 POSTGRES_USER=selab_user
-POSTGRES_PASSWORD="" # Set to desired password
+POSTGRES_PASSWORD=""
 
 # Redis
 REDIS_IP=192.168.90.10
 REDIS_PORT=6379
 
 # Django
-DJANGO_SECRET_KEY="" # Set to a random string
-DJANGO_DEBUG=False # Django debug mode
-DJANGO_DOMAIN_NAME="" # Domain name for the Django server
-DJANGO_CAS_URL_PREFIX="" # URL prefix for the CAS server
-DJANGO_CAS_PORT="" # Port for the CAS server
-DJANGO_DB_ENGINE=django.db.backends.postgresql # Database engine configuration
+DJANGO_SECRET_KEY=""
+DJANGO_DEBUG=False
+DJANGO_DOMAIN_NAME=""
+DJANGO_CAS_URL_PREFIX=""
+DJANGO_CAS_PORT=""
+DJANGO_DB_ENGINE=django.db.backends.postgresql
 DJANGO_DB_NAME=${POSTGRES_DB}
 DJANGO_DB_USER=${POSTGRES_USER}
 DJANGO_DB_PASSWORD=${POSTGRES_PASSWORD}
 DJANGO_DB_HOST=${POSTGRES_IP}
 DJANGO_DB_PORT=${POSTGRES_PORT}
-DJANGO_REDIS_HOST=${REDIS_IP} # Redis configuration
+DJANGO_REDIS_HOST=${REDIS_IP}
 DJANGO_REDIS_PORT=${REDIS_PORT}

backend/api/migrations/0012_errortemplate_alter_extrachecksresult_error_message_and_more.py

@@ -21,7 +21,8 @@ class Migration(migrations.Migration):
         migrations.AlterField(
             model_name='extrachecksresult',
             name='error_message',
-            field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, related_name='extra_checks_results', to='api.errortemplate'),
+            field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE,
+                                    related_name='extra_checks_results', to='api.errortemplate'),
         ),
         migrations.DeleteModel(
             name='ErrorTemplates',

backend/api/migrations/0016_remove_checkresult_is_valid_submission_is_valid_and_more.py

@@ -0,0 +1,40 @@
+# Generated by Django 5.0.4 on 2024-04-15 16:11
+
+import django.db.models.deletion
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('api', '0015_checkresult_remove_extrachecksresult_error_message_and_more'),
+    ]
+
+    operations = [
+        migrations.RemoveField(
+            model_name='checkresult',
+            name='is_valid',
+        ),
+        migrations.AddField(
+            model_name='submission',
+            name='is_valid',
+            field=models.BooleanField(default=True),
+        ),
+        migrations.AlterField(
+            model_name='checkresult',
+            name='submission',
+            field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='results', to='api.submission'),
+        ),
+        migrations.AlterField(
+            model_name='extracheckresult',
+            name='extra_check',
+            field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE,
+                                    related_name='extra_check', to='api.extracheck'),
+        ),
+        migrations.AlterField(
+            model_name='structurecheckresult',
+            name='structure_check',
+            field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE,
+                                    related_name='structure_check', to='api.structurecheck'),
+        ),
+    ]

backend/api/migrations/0017_merge_20240416_1054.py

@@ -0,0 +1,14 @@
+# Generated by Django 5.0.4 on 2024-04-16 10:54
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('api', '0016_alter_checkresult_submission_and_more'),
+        ('api', '0016_remove_checkresult_is_valid_submission_is_valid_and_more'),
+    ]
+
+    operations = [
+    ]

backend/api/models/submission.py

@@ -28,6 +28,14 @@ class Submission(models.Model):
     # Automatically set the submission time to the current time
     submission_time = models.DateTimeField(auto_now_add=True)
 
+    # Whether the checks results are still valid
+    # Becomes invalid after changing / adding a check
+    is_valid = models.BooleanField(
+        default=True,
+        blank=False,
+        null=False
+    )
+
     class Meta:
         # A group can only have one submission with a specific number
         unique_together = ("group", "submission_number")
@@ -103,14 +111,6 @@ class CheckResult(PolymorphicModel):
         null=True
     )  # type: ignore
 
-    # Whether the pass result is still valid
-    # Becomes invalid after changing / adding a check
-    is_valid = models.BooleanField(
-        default=True,
-        blank=False,
-        null=False
-    )
-
 
 class StructureCheckResult(CheckResult):
 

backend/api/serializers/submission_serializer.py

@@ -77,6 +77,9 @@ def create(self, validated_data):
         # Set the new submission number to the maximum value plus 1
         validated_data['submission_number'] = max_submission_number + 1
 
+        # Required otherwise the default value isn't used
+        validated_data["is_valid"] = True
+
         # Create the Submission instance without the files
         submission = Submission.objects.create(**validated_data)
 
@@ -89,4 +92,5 @@ def create(self, validated_data):
             #     passed = False
 
         submission.save()
+
         return submission

backend/api/signals.py

@@ -1,4 +1,4 @@
-from api.models.checks import ExtraCheck
+from api.models.checks import ExtraCheck, StructureCheck
 from api.models.student import Student
 from api.models.submission import (ExtraCheckResult, StateEnum,
                                    StructureCheckResult, Submission)
@@ -18,7 +18,6 @@
 # Receivers
 @receiver(user_created)
 def _user_creation(user: User, attributes: dict, **_):
-    print(user)
     """Upon user creation, auto-populate additional properties"""
     student_id: str = attributes.get("ugentStudentID")
 
@@ -29,13 +28,17 @@ def _user_creation(user: User, attributes: dict, **_):
 @receiver(run_structure_checks)
 def _run_structure_checks(submission: Submission, **kwargs):
     for structure_check in submission.group.project.structure_checks.all():
-        structure_check_result = StructureCheckResult(
-            submission=submission,
-            result=StateEnum.QUEUED,
-            error_message=None,
-            is_valid=True,
-            structure_check=structure_check
-        )
+        structure_check_result: StructureCheckResult
+        if submission.results.filter(structurecheckresult__structure_check__id=structure_check.id).exists():
+            structure_check_result = submission.results.get(structurecheckresult__structure_check__id=structure_check.id)
+            structure_check_result.result = StateEnum.QUEUED
+        else:
+            structure_check_result = StructureCheckResult(
+                submission=submission,
+                result=StateEnum.QUEUED,
+                error_message=None,
+                structure_check=structure_check
+            )
         structure_check_result.save()
         task_structure_check_start.apply_async((structure_check_result,))
     return True
@@ -44,25 +47,41 @@ def _run_structure_checks(submission: Submission, **kwargs):
 @receiver(run_extra_checks)
 def _run_extra_checks(submission: Submission, **kwargs):
     for extra_check in submission.group.project.extra_checks.all():
-        extra_check_result = ExtraCheckResult(
-            submission=submission,
-            result=StateEnum.QUEUED,
-            error_message=None,
-            is_valid=True,
-            extra_check=extra_check,
-            log_file=None
-        )
+        extra_check_result: ExtraCheckResult
+        if submission.results.filter(extracheckresult__extra_check__id=extra_check.id).exists():
+            extra_check_result = submission.results.get(extracheckresult__extra_check__id=extra_check.id)
+            extra_check_result.result = StateEnum.QUEUED
+        else:
+            extra_check_result = ExtraCheckResult(
+                submission=submission,
+                result=StateEnum.QUEUED,
+                error_message=None,
+                extra_check=extra_check,
+                log_file=None
+            )
         extra_check_result.save()
         task_extra_check_start.apply_async((extra_check_result,))
     return True
 
 
+@receiver(post_save, sender=StructureCheck)
+@receiver(post_delete, sender=StructureCheck)
+def hook_structure_check(sender, instance: StructureCheck, **kwargs):
+    for group in instance.project.groups.all():
+        submissions = group.submissions.order_by("-submission_time")
+        if submissions:
+            run_structure_checks.send(sender=StructureCheck, submission=submissions[0])
+
+            for submission in submissions[1:]:
+                submission.is_valid = False
+                submission.save()
+
+
 @receiver(post_save, sender=ExtraCheck)
-@receiver(post_delete, sender=ExtraCheck)  # TODO: Does this work post_delete
+@receiver(post_delete, sender=ExtraCheck)
 def hook_extra_check(sender, instance: ExtraCheck, **kwargs):
     for group in instance.project.groups.all():
-        submissions = group.submissions.order_by("submission_time")  # TODO: Ordered in the right way?
-        # TODO: Set to invalid old results
+        submissions = group.submissions.order_by("-submission_time")
         if submissions:
             run_extra_checks.send(sender=ExtraCheck, submission=submissions[0])
 
@@ -71,10 +90,8 @@ def hook_extra_check(sender, instance: ExtraCheck, **kwargs):
                 submission.save()
 
 
-# TODO: Hook structure_check
-
-
-@ receiver(post_save, sender=Submission)
-def hook_submission(sender, instance: Submission, **kwargs):
-    run_structure_checks.send(sender=Submission, submission=instance)
-    run_extra_checks.send(sender=Submission, submission=instance)
+@receiver(post_save, sender=Submission)
+def hook_submission(sender, instance: Submission, created: bool, **kwargs):
+    if created:
+        run_structure_checks.send(sender=Submission, submission=instance)
+        run_extra_checks.send(sender=Submission, submission=instance)