From 6bcb75c12e5a54af4ea066c5fdae15eb7f422817 Mon Sep 17 00:00:00 2001 From: Bram Meir Date: Tue, 12 Mar 2024 11:09:22 +0100 Subject: [PATCH 1/3] chore: score visible if set in project --- ...sible_alter_project_start_date_and_more.py | 29 ++++++++++++ backend/api/models/project.py | 3 ++ backend/api/serializers/group_serializer.py | 9 ++++ backend/api/serializers/project_serializer.py | 1 + backend/api/tests/test_group.py | 47 ++++++++++++++++++- backend/api/tests/test_submission.py | 2 +- 6 files changed, 88 insertions(+), 3 deletions(-) create mode 100644 backend/api/migrations/0006_project_score_visible_alter_project_start_date_and_more.py diff --git a/backend/api/migrations/0006_project_score_visible_alter_project_start_date_and_more.py b/backend/api/migrations/0006_project_score_visible_alter_project_start_date_and_more.py new file mode 100644 index 00000000..dd13125d --- /dev/null +++ b/backend/api/migrations/0006_project_score_visible_alter_project_start_date_and_more.py @@ -0,0 +1,29 @@ +# Generated by Django 5.0.2 on 2024-03-12 09:49 + +import django.utils.timezone +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('api', '0005_alter_project_max_score'), + ] + + operations = [ + migrations.AddField( + model_name='project', + name='score_visible', + field=models.BooleanField(default=False), + ), + migrations.AlterField( + model_name='project', + name='start_date', + field=models.DateTimeField(blank=True, default=django.utils.timezone.now), + ), + migrations.AlterField( + model_name='submission', + name='submission_number', + field=models.PositiveIntegerField(blank=True, null=True), + ), + ] diff --git a/backend/api/models/project.py b/backend/api/models/project.py index 0cecf456..5cd45ac4 100644 --- a/backend/api/models/project.py +++ b/backend/api/models/project.py @@ -35,6 +35,9 @@ class Project(models.Model): default=20 ) + # Score already visible to students + score_visible = models.BooleanField(default=False) + # Size of the groups than can be formed group_size = models.PositiveSmallIntegerField( blank=False, diff --git a/backend/api/serializers/group_serializer.py b/backend/api/serializers/group_serializer.py index 35a61402..80822583 100644 --- a/backend/api/serializers/group_serializer.py +++ b/backend/api/serializers/group_serializer.py @@ -25,6 +25,15 @@ class Meta: model = Group fields = ["id", "project", "students", "score", "submissions"] + def to_representation(self, instance): + data = super().to_representation(instance) + + # Check if the score should be visible, if not exclude it from the representation + if not instance.project.score_visible: + data.pop('score', None) + + return data + def validate(self, data): # Make sure the score of the group is lower or equal to the maximum score group: Group = self.instance diff --git a/backend/api/serializers/project_serializer.py b/backend/api/serializers/project_serializer.py index e05397e2..66393edf 100644 --- a/backend/api/serializers/project_serializer.py +++ b/backend/api/serializers/project_serializer.py @@ -38,6 +38,7 @@ class Meta: "start_date", "deadline", "max_score", + "score_visible", "group_size", "structure_checks", "extra_checks", diff --git a/backend/api/tests/test_group.py b/backend/api/tests/test_group.py index 16ffa2df..74abeee0 100644 --- a/backend/api/tests/test_group.py +++ b/backend/api/tests/test_group.py @@ -24,12 +24,12 @@ def create_course(name, academic_startyear, description=None, parent_course=None ) -def create_project(name, description, days, course, group_size=2, max_score=20): +def create_project(name, description, days, course, group_size=2, max_score=20, score_visible=True): """Create a Project with the given arguments.""" deadline = timezone.now() + timedelta(days=days) return Project.objects.create( name=name, description=description, deadline=deadline, course=course, - group_size=group_size, max_score=max_score + group_size=group_size, max_score=max_score, score_visible=score_visible ) @@ -86,6 +86,49 @@ def test_group_detail_view(self): self.assertEqual(content_json["project"], expected_project_url) self.assertEqual(content_json["score"], group.score) + def test_group_score_visibility(self): + """Only able to retrieve the score of a group if it is visible.""" + course = create_course(name="sel2", academic_startyear=2023) + + project = create_project( + name="Project 1", description="Description 1", days=7, course=course, score_visible=False + ) + group = create_group(project=project, score=10) + + response = self.client.get( + reverse("group-detail", args=[str(group.id)]), follow=True + ) + + self.assertEqual(response.status_code, 200) + + content_json = json.loads(response.content.decode("utf-8")) + + # Make sure that score is not included + self.assertNotIn("score", content_json) + + # Update that the score is visible + response = self.client.patch( + reverse("project-detail", args=[str(project.id)]), + {"score_visible": True}, + ) + + # Refresh the project from the database + project.refresh_from_db() + + # Make sure the project is updated + self.assertEqual(project.score_visible, True) + self.assertEqual(response.status_code, 200) + + response = self.client.get( + reverse("group-detail", args=[str(group.id)]), follow=True + ) + + self.assertEqual(response.status_code, 200) + content_json = json.loads(response.content.decode("utf-8")) + + # Make sure the score is included now + self.assertIn("score", content_json) + def test_group_project(self): """Able to retrieve details of a single group.""" course = create_course(name="sel2", academic_startyear=2023) diff --git a/backend/api/tests/test_submission.py b/backend/api/tests/test_submission.py index ec426a03..a9aa6c9f 100644 --- a/backend/api/tests/test_submission.py +++ b/backend/api/tests/test_submission.py @@ -28,7 +28,7 @@ def create_project(name, description, days, course): """Create a Project with the given arguments.""" deadline = timezone.now() + timedelta(days=days) return Project.objects.create( - name=name, description=description, deadline=deadline, course=course + name=name, description=description, deadline=deadline, course=course, score_visible=True ) From 5fe5626ecbae5977102300b40919ede20062e2f5 Mon Sep 17 00:00:00 2001 From: Bram Meir Date: Tue, 12 Mar 2024 11:37:12 +0100 Subject: [PATCH 2/3] chore: score not visible for students of other groups --- backend/api/serializers/group_serializer.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/backend/api/serializers/group_serializer.py b/backend/api/serializers/group_serializer.py index 80822583..ddbaa158 100644 --- a/backend/api/serializers/group_serializer.py +++ b/backend/api/serializers/group_serializer.py @@ -1,6 +1,7 @@ from django.utils.translation import gettext from rest_framework import serializers from rest_framework.exceptions import ValidationError +from api.permissions.role_permissions import is_student from api.models.group import Group from api.models.student import Student from api.serializers.student_serializer import StudentIDSerializer @@ -28,9 +29,13 @@ class Meta: def to_representation(self, instance): data = super().to_representation(instance) - # Check if the score should be visible, if not exclude it from the representation - if not instance.project.score_visible: - data.pop('score', None) + # If you are not a student, you can always see the score + if is_student(self.context["request"].user): + # Student can not see the score if they are not part of the group, or it is not visible yet + if not instance.students.filter(id=self.context["request"].user.student.id).exists() or\ + not instance.project.score_visible: + + data.pop("score") return data From 50f2ebdc2502a09a79f5de3aa165090ea091d36a Mon Sep 17 00:00:00 2001 From: Bram Meir Date: Tue, 12 Mar 2024 11:45:00 +0100 Subject: [PATCH 3/3] chore: test with student permissions --- backend/api/tests/test_group.py | 98 ++++++++++++++++++--------------- 1 file changed, 55 insertions(+), 43 deletions(-) diff --git a/backend/api/tests/test_group.py b/backend/api/tests/test_group.py index 74abeee0..63017054 100644 --- a/backend/api/tests/test_group.py +++ b/backend/api/tests/test_group.py @@ -86,49 +86,6 @@ def test_group_detail_view(self): self.assertEqual(content_json["project"], expected_project_url) self.assertEqual(content_json["score"], group.score) - def test_group_score_visibility(self): - """Only able to retrieve the score of a group if it is visible.""" - course = create_course(name="sel2", academic_startyear=2023) - - project = create_project( - name="Project 1", description="Description 1", days=7, course=course, score_visible=False - ) - group = create_group(project=project, score=10) - - response = self.client.get( - reverse("group-detail", args=[str(group.id)]), follow=True - ) - - self.assertEqual(response.status_code, 200) - - content_json = json.loads(response.content.decode("utf-8")) - - # Make sure that score is not included - self.assertNotIn("score", content_json) - - # Update that the score is visible - response = self.client.patch( - reverse("project-detail", args=[str(project.id)]), - {"score_visible": True}, - ) - - # Refresh the project from the database - project.refresh_from_db() - - # Make sure the project is updated - self.assertEqual(project.score_visible, True) - self.assertEqual(response.status_code, 200) - - response = self.client.get( - reverse("group-detail", args=[str(group.id)]), follow=True - ) - - self.assertEqual(response.status_code, 200) - content_json = json.loads(response.content.decode("utf-8")) - - # Make sure the score is included now - self.assertIn("score", content_json) - def test_group_project(self): """Able to retrieve details of a single group.""" course = create_course(name="sel2", academic_startyear=2023) @@ -539,3 +496,58 @@ def test_try_to_update_score_of_group(self): group.refresh_from_db() self.assertEqual(group.score, 10) + + def test_group_score_visibility(self): + """Only able to retrieve the score of a group if it is visible, and the student is part of the group.""" + course = create_course(name="sel2", academic_startyear=2023) + + project = create_project( + name="Project 1", description="Description 1", days=7, course=course, score_visible=True + ) + group = create_group(project=project, score=10) + course.students.add(self.user) + + response = self.client.get( + reverse("group-detail", args=[str(group.id)]), follow=True + ) + + self.assertEqual(response.status_code, 200) + + content_json = json.loads(response.content.decode("utf-8")) + + # Make sure that score is not included, because the student is not part of the group + self.assertNotIn("score", content_json) + + # Add the student to the group + group.students.add(self.user) + + # Set the visibility of the score to False, to make sure the score is not included if it is not visible + project.score_visible = False + project.save() + + response = self.client.get( + reverse("group-detail", args=[str(group.id)]), follow=True + ) + + self.assertEqual(response.status_code, 200) + + content_json = json.loads(response.content.decode("utf-8")) + + # Make sure that score is not included, because the teacher has set the visibility of the score to False + self.assertNotIn("score", content_json) + + # Update that the score is visible + project.score_visible = True + project.save() + + self.assertEqual(response.status_code, 200) + + response = self.client.get( + reverse("group-detail", args=[str(group.id)]), follow=True + ) + + self.assertEqual(response.status_code, 200) + content_json = json.loads(response.content.decode("utf-8")) + + # Make sure the score is included now + self.assertIn("score", content_json)