chore: better submission status (#421)

* chore: better submission status

* chore: add tests + fix tests

* fix: backend tests

* chore: round status percentages

* Revert "chore: round status percentages"

This reverts commit cfb55c7.

* chore: rename test to make tybo happy

backend/api/serializers/project_serializer.py

@@ -1,7 +1,7 @@
 from api.logic.parse_zip_files import parse_zip
 from api.models.group import Group
 from api.models.project import Project
-from api.models.submission import Submission
+from api.models.submission import Submission, ExtraCheckResult, StructureCheckResult, StateEnum
 from api.serializers.course_serializer import CourseSerializer
 from django.core.files.uploadedfile import InMemoryUploadedFile
 from django.utils import timezone
@@ -14,28 +14,69 @@
 class SubmissionStatusSerializer(serializers.Serializer):
     non_empty_groups = serializers.IntegerField(read_only=True)
     groups_submitted = serializers.IntegerField(read_only=True)
-    submissions_passed = serializers.IntegerField(read_only=True)
+    structure_checks_passed = serializers.IntegerField(read_only=True)
+    extra_checks_passed = serializers.IntegerField(read_only=True)
 
     def to_representation(self, instance: Project):
         """Return the submission status of the project"""
         if not isinstance(instance, Project):
             raise ValidationError(gettext("project.errors.invalid_instance"))
 
-        non_empty_groups = instance.groups.filter(students__isnull=False).count()
-        groups_submitted = Submission.objects.filter(group__project=instance).count()
-        submissions_passed = Submission.objects.filter(group__project=instance, is_valid=True).count()
+        non_empty_groups = Group.objects.filter(project=instance, students__isnull=False).distinct().count()
+
+        groups_submitted_ids = Submission.objects.filter(group__project=instance).values_list('group__id', flat=True)
+        unique_groups = set(groups_submitted_ids)
+        groups_submitted = len(unique_groups)
+
+        #  The total amount of groups with at least one submission should never exceed the total number of non empty groups
+        # (the seeder does not account for this restriction)
+        if (groups_submitted > non_empty_groups):
+            non_empty_groups = groups_submitted
+
+        passed_structure_checks_submission_ids = StructureCheckResult.objects.filter(
+            submission__group__project=instance,
+            submission__is_valid=True,
+            result=StateEnum.SUCCESS
+        ).values_list('submission__id', flat=True)
+
+        passed_structure_checks_group_ids = Submission.objects.filter(
+            id__in=passed_structure_checks_submission_ids
+        ).values_list('group_id', flat=True)
+
+        unique_groups = set(passed_structure_checks_group_ids)
+        structure_checks_passed = len(unique_groups)
+
+        passed_extra_checks_submission_ids = ExtraCheckResult.objects.filter(
+            submission__group__project=instance,
+            submission__is_valid=True,
+            result=StateEnum.SUCCESS
+        ).values_list('submission__id', flat=True)
+
+        passed_extra_checks_group_ids = Submission.objects.filter(
+            id__in=passed_extra_checks_submission_ids
+        ).values_list('group_id', flat=True)
+
+        unique_groups = set(passed_extra_checks_group_ids)
+        extra_checks_passed = len(unique_groups)
+
+        # The total number of passed extra checks combined with the number of passed structure checks
+        # can never exceed the total number of submissions (the seeder does not account for this restriction)
+        if (structure_checks_passed + extra_checks_passed > groups_submitted):
+            extra_checks_passed = groups_submitted - structure_checks_passed
 
         return {
             "non_empty_groups": non_empty_groups,
             "groups_submitted": groups_submitted,
-            "submissions_passed": submissions_passed,
+            "structure_checks_passed": structure_checks_passed,
+            "extra_checks_passed": extra_checks_passed
         }
 
     class Meta:
         fields = [
             "non_empty_groups",
             "groups_submitted",
-            "submissions_passed",
+            "structure_checks_passed",
+            "extra_checks_passed"
         ]
 
 

backend/api/tests/test_project.py

@@ -802,10 +802,10 @@ def test_submission_status_non_empty_groups(self):
         # Only two of the three created groups contain at least one student
         self.assertEqual(
             content_json["status"],
-            {"non_empty_groups": 2, "groups_submitted": 0, "submissions_passed": 0},
+            {"non_empty_groups": 2, "groups_submitted": 0, "extra_checks_passed": 0, "structure_checks_passed": 0},
         )
 
-    def test_submission_status_groups_submitted_and_passed_checks(self):
+    def test_submission_status_groups_submitted_and_not_passed_checks(self):
         """Retrieve the submission status for a project."""
         course = create_course(name="test course", academic_startyear=2024)
         project = create_project(
@@ -866,7 +866,7 @@ def test_submission_status_groups_submitted_and_passed_checks(self):
 
         self.assertEqual(
             content_json["status"],
-            {"non_empty_groups": 3, "groups_submitted": 2, "submissions_passed": 2},
+            {"non_empty_groups": 3, "groups_submitted": 2, "extra_checks_passed": 0, "structure_checks_passed": 0},
         )
 
     def test_retrieve_list_submissions(self):

frontend/src/assets/lang/app/en.json

@@ -186,7 +186,8 @@
             "noSubmissions": "This project does not have any submissions",
             "submissions": "Submission | Submissions",
             "groups": "Group | Groups",
-            "testsSucceed": "Succeeded tests",
+            "structureTestsSucceed": "Succeeded structure tests",
+            "extraTestsSucceed": "Succeeded extra tests",
             "testsFail": "Failed tests",
             "submit": "Submit"
         },

frontend/src/assets/lang/app/nl.json

@@ -183,7 +183,8 @@
             "noSubmissions": "Dit project heeft geen indieningen",
             "submissions": "Indiening | Indieningen",
             "groups": "Groep | Groepen",
-            "testsSucceed": "Geslaagde testen",
+            "structureTestsSucceed": "Geslaagde structuur testen",
+            "extraTestsSucceed": "Geslaagde extra testen",
             "testsFail": "Gefaalde testen",
             "submit": "Indienen"
         },

frontend/src/components/projects/ProjectMeter.vue

@@ -17,15 +17,23 @@ const { t } = useI18n();
 const meterItems = computed(() => {
     const groups = props.project !== null ? props.project.status.non_empty_groups : 0;
     const groupsSubmitted = props.project !== null ? props.project.status.groups_submitted : 0;
-    const submissionsPassed = props.project !== null ? props.project.status.submissions_passed : 0;
-    const submissionsFailed = groupsSubmitted - submissionsPassed;
+    const structureChecksPassed = props.project !== null ? props.project.status.structure_checks_passed : 0;
+    const extraChecksPassed = props.project !== null ? props.project.status.extra_checks_passed : 0;
+    const submissionsFailed = groupsSubmitted - structureChecksPassed;
+
     return [
         {
-            value: (submissionsPassed / groups) * 100,
+            value: (extraChecksPassed / groups) * 100,
             color: '#749b68',
-            label: t('components.card.testsSucceed'),
+            label: t('components.card.extraTestsSucceed'),
             icon: 'pi pi-check',
         },
+        {
+            value: (structureChecksPassed / groups) * 100,
+            color: '#fa9746',
+            label: t('components.card.structureTestsSucceed'),
+            icon: 'pi pi-exclamation-circle',
+        },
         {
             value: (submissionsFailed / groups) * 100,
             color: '#FF5445',

frontend/src/test/unit/services/setup/delete_handlers.ts

@@ -29,4 +29,9 @@ export const deleteHandlers = [
         assistants.splice(index, 1);
         return HttpResponse.json(assistants);
     }),
+    http.delete(baseUrl + endpoints.structureChecks.retrieve.replace('{id}', ':id'), async ({ params }) => {
+        const index = structureChecks.findIndex((x) => x.id === params.id);
+        structureChecks.splice(index, 1);
+        return HttpResponse.json(structureChecks);
+    }),
 ];

frontend/src/test/unit/services/setup/get_handlers.ts

@@ -86,27 +86,10 @@ export const getHandlers = [
         const project = projects.find((x) => x.id === params.id);
         const groupIds = project !== null && project !== undefined ? project.groups : [];
         const submissionIds = project !== null && project !== undefined ? project.submissions : [];
-        const subGroupIds = Array.from(
-            new Set(submissions.filter((x) => submissionIds.includes(x.id)).map((x) => x.group)),
-        );
 
-        // Filter submissions for each subgroup and get the submission with the highest number
-        const subgroupSubmissions = subGroupIds.map((groupId) => {
-            const submissionsForGroup = submissions.filter((submission) => submission.group === groupId);
-            if (submissionsForGroup.length > 0) {
-                return submissionsForGroup.reduce((maxSubmission, currentSubmission) => {
-                    return currentSubmission.submission_number > maxSubmission.submission_number
-                        ? currentSubmission
-                        : maxSubmission;
-                });
-            } else {
-                return null;
-            }
-        });
         return HttpResponse.json({
             groups_submitted: new Set(submissions.filter((x) => submissionIds.includes(x.id)).map((x) => x.group)).size,
             non_empty_groups: groups.filter((x) => groupIds.includes(x.id) && x.students.length > 0).length,
-            submissions_passed: subgroupSubmissions.filter((x) => x?.structureChecks_passed).length,
         });
     }),
     http.get(baseUrl + endpoints.structureChecks.byProject.replace('{projectId}', ':id'), ({ params }) => {

frontend/src/test/unit/services/setup/post_handlers.ts

@@ -54,18 +54,29 @@ export const postHandlers = [
         faculties.push(newFaculty);
         return HttpResponse.json(faculties);
     }),
-    http.post(baseUrl + endpoints.groups.byProject.replace('{projectId}', ':id'), async ({ request, params }) => {
+    http.post(baseUrl + endpoints.groups.byProject.replace('{projectId}', ':id'), async ({ request }) => {
         const buffer = await request.arrayBuffer();
         const requestBody = new TextDecoder().decode(buffer);
         const newGroup = JSON.parse(requestBody);
         groups.push(newGroup);
         return HttpResponse.json(groups);
     }),
-    http.post(baseUrl + endpoints.projects.byCourse.replace('{courseId}', ':id'), async ({ request, params }) => {
+    http.post(baseUrl + endpoints.projects.byCourse.replace('{courseId}', ':id'), async ({ request }) => {
         const buffer = await request.arrayBuffer();
         const requestBody = new TextDecoder().decode(buffer);
         const newProject = JSON.parse(requestBody);
         projects.push(newProject);
         return HttpResponse.json(projects);
     }),
+    http.post(
+        baseUrl + endpoints.structureChecks.byProject.replace('{projectId}', ':id'),
+        async ({ request, params }) => {
+            const buffer = await request.arrayBuffer();
+            const requestBody = new TextDecoder().decode(buffer);
+            const newStructureCheck = JSON.parse(requestBody);
+            newStructureCheck.project = params.id;
+            structureChecks.push(newStructureCheck);
+            return HttpResponse.json(structureChecks);
+        },
+    ),
 ];

frontend/src/test/unit/services/structure_check.test.ts

@@ -1,6 +1,7 @@
 /* eslint-disable @typescript-eslint/no-unused-vars */
 import { describe, it, expect } from 'vitest';
 import { useStructureCheck } from '@/composables/services/structure_check.service.ts';
+import { StructureCheck } from '@/types/StructureCheck';
 
 const {
     structureChecks,
@@ -59,3 +60,52 @@ describe('structureCheck', (): void => {
         expect(structureChecks.value?.[3]?.blocked_extensions).toBeNull();
     });
 });
+
+it('create structureCheck', async () => {
+    resetService();
+
+    const exampleStructureCheck = new StructureCheck(
+        '', // id
+        'structure_check_name', // name
+        [], // blocked extensions
+        [], // obligated extensions
+        null, // project
+    );
+
+    await getStructureCheckByProject('123456');
+    expect(structureChecks).not.toBeNull();
+    expect(Array.isArray(structureChecks.value)).toBe(true);
+    const prevLength = structureChecks.value?.length ?? 0;
+
+    await createStructureCheck(exampleStructureCheck, '123456');
+    await getStructureCheckByProject('123456');
+
+    expect(structureChecks).not.toBeNull();
+    expect(Array.isArray(structureChecks.value)).toBe(true);
+    expect(structureChecks.value?.length).toBe(prevLength + 1);
+
+    // Only check for fields that are sent to the backend
+    expect(structureChecks.value?.[prevLength]?.name).toBe('structure_check_name');
+});
+
+it('delete structureCheck', async () => {
+    resetService();
+
+    await getStructureCheckByProject('123456');
+    expect(structureChecks.value).not.toBeNull();
+    expect(Array.isArray(structureChecks.value)).toBe(true);
+    const prevLength = structureChecks.value?.length ?? 0;
+
+    let structureCheckId = '';
+    if (structureChecks.value?.[2]?.id !== undefined && structureChecks.value?.[2].id !== null) {
+        structureCheckId = structureChecks.value?.[2]?.id;
+    }
+
+    await deleteStructureCheck(structureCheckId);
+    await getStructureCheckByProject('123456');
+
+    expect(structureChecks).not.toBeNull();
+    expect(Array.isArray(structureChecks.value)).toBe(true);
+    expect(structureChecks.value?.length).toBe(prevLength - 1);
+    expect(structureChecks.value?.[2]?.id).not.toBe(structureCheckId);
+});

frontend/src/test/unit/services/submission_status_service.test.ts

@@ -16,6 +16,6 @@ describe('submision_status', (): void => {
         expect(submissionStatus.value).not.toBeNull();
         expect(submissionStatus.value?.groups_submitted).toBe(1);
         expect(submissionStatus.value?.non_empty_groups).toBe(2);
-        expect(submissionStatus.value?.submissions_passed).toBe(1);
+        // No need to check for structure_check_passed and extra_checks_passed since those queries are not implemented in the frontend
     });
 });

frontend/src/test/unit/types/data.ts

@@ -123,8 +123,9 @@ export const structureCheckData = {
 
 export const submissionStatusData = {
     non_empty_groups: 5,
-    groups_submitted: 2,
-    submissions_passed: 1,
+    groups_submitted: 4,
+    structure_checks_passed: 3,
+    extra_checks_passed: 1,
 };
 
 export const submissionData = {

frontend/src/test/unit/types/helper.ts

@@ -156,7 +156,8 @@ export function createSubmissionStatus(submissionStatusData: any): SubmissionSta
     return new SubmissionStatus(
         submissionStatusData.non_empty_groups,
         submissionStatusData.groups_submitted,
-        submissionStatusData.submissions_passed,
+        submissionStatusData.structure_checks_passed,
+        submissionStatusData.extra_checks_passed,
     );
 }
 

frontend/src/test/unit/types/submissionStatus.test.ts

@@ -11,7 +11,8 @@ describe('submissionStatus type', () => {
         expect(submissionStatus).toBeInstanceOf(SubmissionStatus);
         expect(submissionStatus.non_empty_groups).toBe(submissionStatusData.non_empty_groups);
         expect(submissionStatus.groups_submitted).toBe(submissionStatusData.groups_submitted);
-        expect(submissionStatus.submissions_passed).toBe(submissionStatusData.submissions_passed);
+        expect(submissionStatus.structure_checks_passed).toBe(submissionStatusData.structure_checks_passed);
+        expect(submissionStatus.extra_checks_passed).toBe(submissionStatusData.extra_checks_passed);
     });
 
     it('create a submissionStatus instance from JSON data', () => {
@@ -21,6 +22,7 @@ describe('submissionStatus type', () => {
         expect(submissionStatus).toBeInstanceOf(SubmissionStatus);
         expect(submissionStatus.non_empty_groups).toBe(submissionStatusData.non_empty_groups);
         expect(submissionStatus.groups_submitted).toBe(submissionStatusData.groups_submitted);
-        expect(submissionStatus.submissions_passed).toBe(submissionStatusData.submissions_passed);
+        expect(submissionStatus.structure_checks_passed).toBe(submissionStatusData.structure_checks_passed);
+        expect(submissionStatus.extra_checks_passed).toBe(submissionStatusData.extra_checks_passed);
     });
 });

frontend/src/types/SubmisionStatus.ts

@@ -2,7 +2,8 @@ export class SubmissionStatus {
     constructor(
         public non_empty_groups: number,
         public groups_submitted: number,
-        public submissions_passed: number,
+        public structure_checks_passed: number,
+        public extra_checks_passed: number,
     ) {}
 
     /**
@@ -14,7 +15,8 @@ export class SubmissionStatus {
         return new SubmissionStatus(
             submissionStatus.non_empty_groups,
             submissionStatus.groups_submitted,
-            submissionStatus.submissions_passed,
+            submissionStatus.structure_checks_passed,
+            submissionStatus.extra_checks_passed,
         );
     }
 }