From 00ea2ff69d9115f1e391c6dd19305e07d2a84579 Mon Sep 17 00:00:00 2001 From: janmax <j.michal@stud.uni-goettingen.de> Date: Fri, 26 Jan 2018 18:41:20 +0100 Subject: [PATCH] Refactores views and serializers added patch methods * Added more tests for feedback view * Now each view is included in one file that are held in the package views instead of using one big file * Did the same this for serializers * Now using ListSerializer to implement the custom behaviour * Also refactored the feedback serializers and models and removed the FeedbackLine model * The serializers are simpler now * Renamed 'is_final' on FeedbackComment to 'visible_to_student' --- core/migrations/0001_initial.py | 151 ++++++----- core/migrations/0002_auto_20180114_1057.py | 35 --- core/migrations/0002_auto_20180202_1421.py | 17 ++ core/migrations/0003_auto_20180207_1922.py | 18 ++ core/models.py | 64 +++-- core/permissions.py | 2 +- core/serializers.py | 299 --------------------- core/serializers/__init__.py | 5 + core/serializers/common_serializers.py | 54 ++++ core/serializers/feedback.py | 160 +++++++++++ core/serializers/generic.py | 20 ++ core/serializers/student.py | 28 ++ core/serializers/submission.py | 36 +++ core/serializers/subscription.py | 75 ++++++ core/tests/test_feedback.py | 233 +++++++++++++--- core/views.py | 205 -------------- core/views/__init__.py | 3 + core/views/common_views.py | 84 ++++++ core/views/feedback.py | 100 +++++++ core/views/subscription.py | 104 +++++++ util/factories.py | 9 +- util/importer.py | 2 +- 22 files changed, 1023 insertions(+), 681 deletions(-) delete mode 100644 core/migrations/0002_auto_20180114_1057.py create mode 100644 core/migrations/0002_auto_20180202_1421.py create mode 100644 core/migrations/0003_auto_20180207_1922.py delete mode 100644 core/serializers.py create mode 100644 core/serializers/__init__.py create mode 100644 core/serializers/common_serializers.py create mode 100644 core/serializers/feedback.py create mode 100644 core/serializers/generic.py create mode 100644 core/serializers/student.py create mode 100644 core/serializers/submission.py create mode 100644 core/serializers/subscription.py delete mode 100644 core/views.py create mode 100644 core/views/__init__.py create mode 100644 core/views/common_views.py create mode 100644 core/views/feedback.py create mode 100644 core/views/subscription.py diff --git a/core/migrations/0001_initial.py b/core/migrations/0001_initial.py index 8eb1e6be..77e1634f 100644 --- a/core/migrations/0001_initial.py +++ b/core/migrations/0001_initial.py @@ -1,4 +1,4 @@ -# Generated by Django 2.0.1 on 2018-01-10 10:46 +# Generated by Django 2.0.1 on 2018-02-01 15:08 import uuid @@ -24,22 +24,37 @@ class Migration(migrations.Migration): migrations.CreateModel( name='UserAccount', fields=[ - ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), - ('password', models.CharField(max_length=128, verbose_name='password')), - ('last_login', models.DateTimeField(blank=True, null=True, verbose_name='last login')), - ('is_superuser', models.BooleanField(default=False, help_text='Designates that this user has all permissions without explicitly assigning them.', verbose_name='superuser status')), - ('username', models.CharField(error_messages={'unique': 'A user with that username already exists.'}, help_text='Required. 150 characters or fewer. Letters, digits and @/./+/-/_ only.', max_length=150, unique=True, validators=[django.contrib.auth.validators.UnicodeUsernameValidator()], verbose_name='username')), - ('first_name', models.CharField(blank=True, max_length=30, verbose_name='first name')), - ('last_name', models.CharField(blank=True, max_length=150, verbose_name='last name')), - ('email', models.EmailField(blank=True, max_length=254, verbose_name='email address')), - ('is_staff', models.BooleanField(default=False, help_text='Designates whether the user can log into this admin site.', verbose_name='staff status')), - ('is_active', models.BooleanField(default=True, help_text='Designates whether this user should be treated as active. Unselect this instead of deleting accounts.', verbose_name='active')), - ('date_joined', models.DateTimeField(default=django.utils.timezone.now, verbose_name='date joined')), - ('fullname', models.CharField(blank=True, max_length=70, verbose_name='full name')), + ('id', models.AutoField(auto_created=True, + primary_key=True, serialize=False, verbose_name='ID')), + ('password', models.CharField( + max_length=128, verbose_name='password')), + ('last_login', models.DateTimeField( + blank=True, null=True, verbose_name='last login')), + ('is_superuser', models.BooleanField(default=False, + help_text='Designates that this user has all permissions without explicitly assigning them.', verbose_name='superuser status')), + ('username', models.CharField(error_messages={'unique': 'A user with that username already exists.'}, help_text='Required. 150 characters or fewer. Letters, digits and @/./+/-/_ only.', + max_length=150, unique=True, validators=[django.contrib.auth.validators.UnicodeUsernameValidator()], verbose_name='username')), + ('first_name', models.CharField(blank=True, + max_length=30, verbose_name='first name')), + ('last_name', models.CharField(blank=True, + max_length=150, verbose_name='last name')), + ('email', models.EmailField(blank=True, + max_length=254, verbose_name='email address')), + ('is_staff', models.BooleanField(default=False, + help_text='Designates whether the user can log into this admin site.', verbose_name='staff status')), + ('is_active', models.BooleanField( + default=True, help_text='Designates whether this user should be treated as active. Unselect this instead of deleting accounts.', verbose_name='active')), + ('date_joined', models.DateTimeField( + default=django.utils.timezone.now, verbose_name='date joined')), + ('fullname', models.CharField(blank=True, + max_length=70, verbose_name='full name')), ('is_admin', models.BooleanField(default=False)), - ('role', models.CharField(choices=[('Student', 'student'), ('Tutor', 'tutor'), ('Reviewer', 'reviewer')], max_length=50)), - ('groups', models.ManyToManyField(blank=True, help_text='The groups this user belongs to. A user will get all permissions granted to each of their groups.', related_name='user_set', related_query_name='user', to='auth.Group', verbose_name='groups')), - ('user_permissions', models.ManyToManyField(blank=True, help_text='Specific permissions for this user.', related_name='user_set', related_query_name='user', to='auth.Permission', verbose_name='user permissions')), + ('role', models.CharField(choices=[ + ('Student', 'student'), ('Tutor', 'tutor'), ('Reviewer', 'reviewer')], max_length=50)), + ('groups', models.ManyToManyField(blank=True, help_text='The groups this user belongs to. A user will get all permissions granted to each of their groups.', + related_name='user_set', related_query_name='user', to='auth.Group', verbose_name='groups')), + ('user_permissions', models.ManyToManyField(blank=True, help_text='Specific permissions for this user.', + related_name='user_set', related_query_name='user', to='auth.Permission', verbose_name='user permissions')), ], options={ 'verbose_name': 'user', @@ -53,7 +68,8 @@ class Migration(migrations.Migration): migrations.CreateModel( name='ExamType', fields=[ - ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('id', models.AutoField(auto_created=True, + primary_key=True, serialize=False, verbose_name='ID')), ('module_reference', models.CharField(max_length=50, unique=True)), ('total_score', models.PositiveIntegerField()), ('pass_score', models.PositiveIntegerField()), @@ -67,13 +83,13 @@ class Migration(migrations.Migration): migrations.CreateModel( name='Feedback', fields=[ - ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), - ('text', models.TextField()), + ('id', models.AutoField(auto_created=True, + primary_key=True, serialize=False, verbose_name='ID')), ('score', models.PositiveIntegerField(default=0)), ('created', models.DateTimeField(auto_now_add=True)), - ('modified', models.DateTimeField(auto_now=True)), ('is_final', models.BooleanField(default=False)), - ('origin', models.IntegerField(choices=[(0, 'was empty'), (1, 'passed unittests'), (2, 'did not compile'), (3, 'could not link'), (4, 'created by a human. yak!')], default=4)), + ('origin', models.IntegerField(choices=[(0, 'was empty'), (1, 'passed unittests'), ( + 2, 'did not compile'), (3, 'could not link'), (4, 'created by a human. yak!')], default=4)), ], options={ 'verbose_name': 'Feedback', @@ -83,11 +99,17 @@ class Migration(migrations.Migration): migrations.CreateModel( name='FeedbackComment', fields=[ - ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('id', models.AutoField(auto_created=True, + primary_key=True, serialize=False, verbose_name='ID')), ('text', models.TextField()), ('created', models.DateTimeField(auto_now_add=True)), ('modified', models.DateTimeField(auto_now=True)), ('is_final', models.BooleanField(default=True)), + ('of_line', models.PositiveIntegerField(default=0)), + ('of_feedback', models.ForeignKey(null=True, on_delete=django.db.models.deletion.CASCADE, + related_name='feedback_lines', to='core.Feedback')), + ('of_tutor', models.ForeignKey(on_delete=django.db.models.deletion.PROTECT, + related_name='comment_list', to=settings.AUTH_USER_MODEL)), ], options={ 'verbose_name': 'Feedback Comment', @@ -95,36 +117,32 @@ class Migration(migrations.Migration): 'ordering': ('created',), }, ), - migrations.CreateModel( - name='FeedbackForSubmissionLine', - fields=[ - ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), - ('of_line', models.PositiveIntegerField()), - ('of_feedback', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='feedbacklines', to='core.Feedback')), - ], - options={ - 'verbose_name': 'Feedback for Submissionline', - 'verbose_name_plural': 'Feedback Submissionlines', - }, - ), migrations.CreateModel( name='GeneralTaskSubscription', fields=[ - ('subscription_id', models.UUIDField(default=uuid.uuid4, editable=False, primary_key=True, serialize=False)), + ('subscription_id', models.UUIDField(default=uuid.uuid4, + editable=False, primary_key=True, serialize=False)), ('query_key', models.CharField(blank=True, max_length=75)), - ('query_type', models.CharField(choices=[('random', 'Query for any submission'), ('student', 'Query for submissions of student'), ('exam', 'Query for submissions of exam type'), ('submission_type', 'Query for submissions of submissions_type')], default='random', max_length=75)), - ('feedback_stage', models.CharField(choices=[('feedback-creation', 'No feedback was ever assigned'), ('feedback-validation', 'Feedback exists but is not validated'), ('feedback-conflict-resolution', 'Previous correctors disagree')], default='feedback-creation', max_length=40)), - ('owner', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='subscriptions', to=settings.AUTH_USER_MODEL)), + ('query_type', models.CharField(choices=[('random', 'Query for any submission'), ('student', 'Query for submissions of student'), ( + 'exam', 'Query for submissions of exam type'), ('submission_type', 'Query for submissions of submissions_type')], default='random', max_length=75)), + ('feedback_stage', models.CharField(choices=[('feedback-creation', 'No feedback was ever assigned'), ('feedback-validation', + 'Feedback exists but is not validated'), ('feedback-conflict-resolution', 'Previous correctors disagree')], default='feedback-creation', max_length=40)), + ('owner', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, + related_name='subscriptions', to=settings.AUTH_USER_MODEL)), ], ), migrations.CreateModel( name='StudentInfo', fields=[ - ('student_id', models.UUIDField(default=uuid.uuid4, editable=False, primary_key=True, serialize=False)), + ('student_id', models.UUIDField(default=uuid.uuid4, + editable=False, primary_key=True, serialize=False)), ('has_logged_in', models.BooleanField(default=False)), - ('matrikel_no', models.CharField(default=core.models.random_matrikel_no, max_length=8, unique=True)), - ('exam', models.ForeignKey(null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='students', to='core.ExamType')), - ('user', models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, related_name='student', to=settings.AUTH_USER_MODEL)), + ('matrikel_no', models.CharField( + default=core.models.random_matrikel_no, max_length=8, unique=True)), + ('exam', models.ForeignKey(null=True, on_delete=django.db.models.deletion.SET_NULL, + related_name='students', to='core.ExamType')), + ('user', models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, + related_name='student', to=settings.AUTH_USER_MODEL)), ], options={ 'verbose_name': 'Student', @@ -134,10 +152,12 @@ class Migration(migrations.Migration): migrations.CreateModel( name='Submission', fields=[ - ('submission_id', models.UUIDField(default=uuid.uuid4, editable=False, primary_key=True, serialize=False)), + ('submission_id', models.UUIDField(default=uuid.uuid4, + editable=False, primary_key=True, serialize=False)), ('seen_by_student', models.BooleanField(default=False)), ('text', models.TextField(blank=True)), - ('student', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='submissions', to='core.StudentInfo')), + ('student', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, + related_name='submissions', to='core.StudentInfo')), ], options={ 'verbose_name': 'Submission', @@ -148,7 +168,8 @@ class Migration(migrations.Migration): migrations.CreateModel( name='SubmissionType', fields=[ - ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('id', models.AutoField(auto_created=True, + primary_key=True, serialize=False, verbose_name='ID')), ('name', models.CharField(max_length=50, unique=True)), ('full_score', models.PositiveIntegerField(default=0)), ('description', models.TextField()), @@ -162,11 +183,13 @@ class Migration(migrations.Migration): migrations.CreateModel( name='Test', fields=[ - ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('id', models.AutoField(auto_created=True, + primary_key=True, serialize=False, verbose_name='ID')), ('name', models.CharField(max_length=30)), ('label', models.CharField(max_length=50)), ('annotation', models.TextField()), - ('submission', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='tests', to='core.Submission')), + ('submission', models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, related_name='tests', to='core.Submission')), ], options={ 'verbose_name': 'Test', @@ -176,37 +199,27 @@ class Migration(migrations.Migration): migrations.CreateModel( name='TutorSubmissionAssignment', fields=[ - ('assignment_id', models.UUIDField(default=uuid.uuid4, editable=False, primary_key=True, serialize=False)), + ('assignment_id', models.UUIDField(default=uuid.uuid4, + editable=False, primary_key=True, serialize=False)), ('is_done', models.BooleanField(default=False)), ('created', models.DateTimeField(auto_now_add=True)), - ('submission', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='assignments', to='core.Submission')), - ('subscription', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='assignments', to='core.GeneralTaskSubscription')), + ('submission', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, + related_name='assignments', to='core.Submission')), + ('subscription', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, + related_name='assignments', to='core.GeneralTaskSubscription')), ], ), migrations.AddField( model_name='submission', name='type', - field=models.ForeignKey(on_delete=django.db.models.deletion.PROTECT, related_name='submissions', to='core.SubmissionType'), - ), - migrations.AddField( - model_name='feedbackcomment', - name='of_line', - field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='comment_list', to='core.FeedbackForSubmissionLine'), - ), - migrations.AddField( - model_name='feedbackcomment', - name='of_tutor', - field=models.ForeignKey(on_delete=django.db.models.deletion.PROTECT, related_name='comment_list', to=settings.AUTH_USER_MODEL), + field=models.ForeignKey(on_delete=django.db.models.deletion.PROTECT, + related_name='submissions', to='core.SubmissionType'), ), migrations.AddField( model_name='feedback', name='of_submission', - field=models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, related_name='feedback', to='core.Submission'), - ), - migrations.AddField( - model_name='feedback', - name='of_tutor', - field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='feedback_list', to=settings.AUTH_USER_MODEL), + field=models.OneToOneField( + on_delete=django.db.models.deletion.CASCADE, related_name='feedback', to='core.Submission'), ), migrations.AlterUniqueTogether( name='test', @@ -220,4 +233,8 @@ class Migration(migrations.Migration): name='generaltasksubscription', unique_together={('owner', 'query_key', 'query_type')}, ), + migrations.AlterUniqueTogether( + name='feedbackcomment', + unique_together={('of_line', 'of_tutor', 'of_feedback')}, + ), ] diff --git a/core/migrations/0002_auto_20180114_1057.py b/core/migrations/0002_auto_20180114_1057.py deleted file mode 100644 index e10404b1..00000000 --- a/core/migrations/0002_auto_20180114_1057.py +++ /dev/null @@ -1,35 +0,0 @@ -# Generated by Django 2.0.1 on 2018-01-14 10:57 - -from django.db import migrations, models -import django.db.models.deletion - - -class Migration(migrations.Migration): - - dependencies = [ - ('core', '0001_initial'), - ] - - operations = [ - migrations.AlterModelOptions( - name='feedbackforsubmissionline', - options={'verbose_name': 'Feedback for Submission line', 'verbose_name_plural': 'Feedback Submission lines'}, - ), - migrations.RemoveField( - model_name='feedback', - name='modified', - ), - migrations.RemoveField( - model_name='feedback', - name='of_tutor', - ), - migrations.RemoveField( - model_name='feedback', - name='text', - ), - migrations.AlterField( - model_name='feedbackforsubmissionline', - name='of_feedback', - field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='feedback_lines', to='core.Feedback'), - ), - ] diff --git a/core/migrations/0002_auto_20180202_1421.py b/core/migrations/0002_auto_20180202_1421.py new file mode 100644 index 00000000..f06014c1 --- /dev/null +++ b/core/migrations/0002_auto_20180202_1421.py @@ -0,0 +1,17 @@ +# Generated by Django 2.0.2 on 2018-02-02 14:21 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('core', '0001_initial'), + ] + + operations = [ + migrations.AlterUniqueTogether( + name='generaltasksubscription', + unique_together={('owner', 'query_key', 'query_type', 'feedback_stage')}, + ), + ] diff --git a/core/migrations/0003_auto_20180207_1922.py b/core/migrations/0003_auto_20180207_1922.py new file mode 100644 index 00000000..b76df083 --- /dev/null +++ b/core/migrations/0003_auto_20180207_1922.py @@ -0,0 +1,18 @@ +# Generated by Django 2.0.2 on 2018-02-07 19:22 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('core', '0002_auto_20180202_1421'), + ] + + operations = [ + migrations.RenameField( + model_name='feedbackcomment', + old_name='is_final', + new_name='visible_to_student', + ), + ] diff --git a/core/models.py b/core/models.py index 6eff3dd6..08e852ee 100644 --- a/core/models.py +++ b/core/models.py @@ -449,7 +449,7 @@ class GeneralTaskSubscription(models.Model): FEEDBACK_VALIDATION = 'feedback-validation' FEEDBACK_CONFLICT_RESOLUTION = 'feedback-conflict-resolution' - type_to_assignment_count_map = { + stage_to_assignment_count_map = { FEEDBACK_CREATION: 0, FEEDBACK_VALIDATION: 1, FEEDBACK_CONFLICT_RESOLUTION: 2, @@ -476,7 +476,10 @@ class GeneralTaskSubscription(models.Model): default=FEEDBACK_CREATION) class Meta: - unique_together = ('owner', 'query_key', 'query_type') + unique_together = ('owner', + 'query_key', + 'query_type', + 'feedback_stage') def _get_submission_base_query(self) -> QuerySet: if self.query_type == self.RANDOM: @@ -487,22 +490,18 @@ class GeneralTaskSubscription(models.Model): def _find_submissions_based_on_completed_assignments( self, done_assignments_count): - base = self._get_submission_base_query().filter( + return self._get_submission_base_query().filter( Q(feedback__isnull=False), Q(feedback__is_final=False), ~Q(assignments__subscription__owner=self.owner) ).annotate( # to display only manual done_assignments_count=Count( Case(When(assignments__is_done=True, then=Value(1)), - output_field=IntegerField(), - ) + output_field=IntegerField(),) ) - ).filter(done_assignments_count=self.type_to_assignment_count_map[ + ).filter(done_assignments_count=self.stage_to_assignment_count_map[ done_assignments_count]) - log.debug('base %s', base) - return base - def _find_unassigned_non_final_submissions(self): unassigned_non_final_submissions = \ self._get_submission_base_query().filter( @@ -528,7 +527,7 @@ class GeneralTaskSubscription(models.Model): self._find_unassigned_non_final_submissions, self._find_unassigned_unapproved_non_final_submissions, self._find_submissions_with_conflicting_feedback - )[self.type_to_assignment_count_map[self.feedback_stage]]() + )[self.stage_to_assignment_count_map[self.feedback_stage]]() if candidates.count() == 0: raise SubscriptionEnded( @@ -601,44 +600,43 @@ class TutorSubmissionAssignment(models.Model): f' (done={self.is_done})') -class FeedbackForSubmissionLine(models.Model): - - of_line = models.PositiveIntegerField() - of_feedback = models.ForeignKey( - Feedback, - related_name="feedback_lines", - on_delete=models.CASCADE - ) - - class Meta: - verbose_name = "Feedback for Submission line" - verbose_name_plural = "Feedback Submission lines" - - class FeedbackComment(models.Model): - """This Class contains the Feedback for a specific line of a Submission - """ + """This Class contains the Feedback for a specific line of a Submission""" text = models.TextField() created = models.DateTimeField(auto_now_add=True) modified = models.DateTimeField(auto_now=True) - of_line = models.ForeignKey( - FeedbackForSubmissionLine, - related_name="comment_list", - on_delete=models.CASCADE - ) - is_final = models.BooleanField(default=True) + + visible_to_student = models.BooleanField(default=True) + + of_line = models.PositiveIntegerField(default=0) of_tutor = models.ForeignKey( get_user_model(), related_name="comment_list", on_delete=models.PROTECT ) + of_feedback = models.ForeignKey( + Feedback, + related_name="feedback_lines", + on_delete=models.CASCADE, + null=True + ) def save(self, *args, **kwargs): - self.of_line.comment_list.update(is_final=False) + comments_on_the_same_line = FeedbackComment.objects.filter( + of_line=self.of_line, + of_feedback=self.of_feedback, + ) + comments_on_the_same_line.update(visible_to_student=False) super().save(*args, **kwargs) class Meta: verbose_name = "Feedback Comment" verbose_name_plural = "Feedback Comments" ordering = ('created',) + unique_together = ('of_line', 'of_tutor', 'of_feedback') + + def __str__(self): + return 'Comment on line {} of tutor {}: "{}"'.format(self.of_line, + self.of_tutor, + self.text) diff --git a/core/permissions.py b/core/permissions.py index 7c5f5cf0..21299ccc 100644 --- a/core/permissions.py +++ b/core/permissions.py @@ -26,7 +26,7 @@ class IsUserRoleGenericPermission(permissions.BasePermission): log.warn('User "%s" has no permission to view %s', user.username, view.__class__.__name__) - return is_authorized + return is_authorized or user.is_superuser class IsStudent(IsUserRoleGenericPermission): diff --git a/core/serializers.py b/core/serializers.py deleted file mode 100644 index 64c50f5a..00000000 --- a/core/serializers.py +++ /dev/null @@ -1,299 +0,0 @@ -import logging - -from django.core.exceptions import ObjectDoesNotExist -from drf_dynamic_fields import DynamicFieldsMixin -from rest_framework import serializers - -from core import models -from core.models import (ExamType, Feedback, GeneralTaskSubscription, - StudentInfo, Submission, SubmissionType, Test, - TutorSubmissionAssignment, UserAccount) -from util.factories import GradyUserFactory - -log = logging.getLogger(__name__) -user_factory = GradyUserFactory() - - -class NoValidAssignmentForFeedbackCreation(Exception): - pass - - -class CannotSetFirstFeedbackFinal(Exception): - pass - - -class DynamicFieldsModelSerializer(DynamicFieldsMixin, - serializers.ModelSerializer): - - def __init__(self, *args, **kwargs): - # Don't pass the 'fields' arg up to the superclass - fields = kwargs.pop('fields', None) - - # Instantiate the superclass normally - super(DynamicFieldsModelSerializer, self).__init__(*args, **kwargs) - - if fields is not None: - # Drop any fields that are not specified in the `fields` argument. - allowed = set(fields) - existing = set(self.fields.keys()) - for field_name in existing - allowed: - self.fields.pop(field_name) - - -class ExamSerializer(DynamicFieldsModelSerializer): - - class Meta: - model = ExamType - fields = ('pk', 'module_reference', 'total_score', - 'pass_score', 'pass_only',) - - -class FeedbackForSubmissionLineSerializer(serializers.BaseSerializer): - - def to_representation(self, obj): - return {feedback.of_line: - FeedbackCommentSerializer( - feedback.comment_list, many=True).data - for feedback in obj.all()} - - def to_internal_value(self, data): - return data - - -class FeedbackSerializer(DynamicFieldsModelSerializer): - assignment_pk = serializers.UUIDField(write_only=True) - feedback_lines = FeedbackForSubmissionLineSerializer( - required=False - ) - - def create(self, validated_data) -> Feedback: - feedback = Feedback.objects.create( - score=validated_data['score'], - is_final=validated_data.get('is_final', False), - of_submission=validated_data['of_submission'] - ) - assignment = validated_data.pop('assignment') - if 'feedback_lines' in validated_data: - for line_no, comment in validated_data['feedback_lines'].items(): - line_obj = models.FeedbackForSubmissionLine.objects.create( - of_line=line_no, - of_feedback=feedback - ) - models.FeedbackComment.objects.create( - of_line=line_obj, - of_tutor=self.context['request'].user, - **comment - ) - assignment.set_done() - return feedback - - def validate(self, data): - log.debug(data) - assignment_pk = data.pop('assignment_pk') - score = data.get('score') - is_final = data.get('is_final', False) - - try: - assignment = TutorSubmissionAssignment.objects.get( - pk=assignment_pk) - except ObjectDoesNotExist as err: - raise serializers.ValidationError('No assignment for given id.') - - requesting_tutor = self.context['request'].user - if assignment.subscription.owner != requesting_tutor: - raise NoValidAssignmentForFeedbackCreation() - - http_method = self.context['request'].method - if (is_final and - http_method == 'POST' and - requesting_tutor.role == models.UserAccount.TUTOR): - raise CannotSetFirstFeedbackFinal() - - submission = assignment.submission - if not 0 <= score <= submission.type.full_score: - raise serializers.ValidationError( - f'Score has to be in range [0..{submission.type.full_score}].') - - if hasattr(submission, 'feedback'): - raise serializers.ValidationError( - 'Feedback for this submission already exists') - - return { - **data, - 'assignment': assignment, - 'of_submission': submission - } - - class Meta: - model = Feedback - fields = ('pk', 'assignment_pk', 'is_final', 'score', - 'of_submission', 'feedback_lines') - read_only_fields = ('of_submission', ) - - -class FeedbackCommentSerializer(serializers.ModelSerializer): - of_tutor = serializers.StringRelatedField(source='of_tutor.username') - - def to_internal_value(self, data): - return data - - class Meta: - model = models.FeedbackComment - fields = ('text', 'of_tutor', 'created', 'is_final') - read_only_fields = ('created',) - - -class TestSerializer(DynamicFieldsModelSerializer): - - class Meta: - model = Test - fields = ('pk', 'name', 'label', 'annotation') - - -class SubmissionTypeListSerializer(DynamicFieldsModelSerializer): - - class Meta: - model = SubmissionType - fields = ('pk', 'name', 'full_score') - - -class SubmissionTypeSerializer(SubmissionTypeListSerializer): - - class Meta: - model = SubmissionType - fields = ('pk', 'name', 'full_score', 'description', 'solution') - - -class SubmissionSerializer(DynamicFieldsModelSerializer): - type = SubmissionTypeSerializer() - feedback = FeedbackSerializer() - tests = TestSerializer(many=True) - - class Meta: - model = Submission - fields = ('pk', 'type', 'text', 'feedback', 'tests') - - -class SubmissionListSerializer(DynamicFieldsModelSerializer): - type = SubmissionTypeListSerializer(fields=('pk', 'name', 'full_score')) - # TODO change this according to new feedback model - feedback = FeedbackSerializer(fields=('score',)) - - class Meta: - model = Submission - fields = ('pk', 'type', 'feedback') - - -class StudentInfoSerializer(DynamicFieldsModelSerializer): - name = serializers.ReadOnlyField(source='user.fullname') - matrikel_no = serializers.ReadOnlyField(source='user.matrikel_no') - exam = ExamSerializer() - submissions = SubmissionListSerializer(many=True) - - class Meta: - model = StudentInfo - fields = ('pk', 'name', 'user', 'matrikel_no', 'exam', 'submissions') - - -class SubmissionNoTextFieldsSerializer(DynamicFieldsModelSerializer): - score = serializers.ReadOnlyField(source='feedback.score') - type = serializers.ReadOnlyField(source='type.name') - full_score = serializers.ReadOnlyField(source='type.full_score') - - class Meta: - model = Submission - fields = ('pk', 'type', 'score', 'full_score') - - -class StudentInfoSerializerForListView(DynamicFieldsModelSerializer): - name = serializers.ReadOnlyField(source='user.fullname') - user = serializers.ReadOnlyField(source='user.username') - exam = serializers.ReadOnlyField(source='exam.module_reference') - submissions = SubmissionNoTextFieldsSerializer(many=True) - - class Meta: - model = StudentInfo - fields = ('pk', 'name', 'user', 'exam', 'submissions') - - -class TutorSerializer(DynamicFieldsModelSerializer): - done_assignments_count = serializers.IntegerField( - read_only=True) - - def create(self, validated_data) -> UserAccount: - log.info("Crating tutor from data %s", validated_data) - return user_factory.make_tutor( - username=validated_data['username']) - - class Meta: - model = UserAccount - fields = ('pk', 'username', 'done_assignments_count') - - -class AssignmentSerializer(DynamicFieldsModelSerializer): - submission_pk = serializers.ReadOnlyField( - source='submission.pk') - - class Meta: - model = TutorSubmissionAssignment - fields = ('pk', 'submission_pk', 'is_done',) - - -class SubmissionAssignmentSerializer(DynamicFieldsModelSerializer): - text = serializers.ReadOnlyField() - type_pk = serializers.ReadOnlyField(source='type.pk') - full_score = serializers.ReadOnlyField(source='type.full_score') - - class Meta: - model = Submission - fields = ('pk', 'type_pk', 'text', 'full_score') - - -class AssignmentDetailSerializer(DynamicFieldsModelSerializer): - submission = SubmissionAssignmentSerializer() - feedback = FeedbackSerializer(source='submission.feedback') - - class Meta: - model = TutorSubmissionAssignment - fields = ('pk', 'feedback', 'submission', 'is_done',) - - -class SubscriptionSerializer(DynamicFieldsModelSerializer): - owner = serializers.ReadOnlyField(source='owner.username') - query_key = serializers.CharField(required=False) - assignments = AssignmentSerializer(read_only=True, many=True) - - def validate(self, data): - data['owner'] = self.context['request'].user - - if 'query_key' in data != \ - data['query_type'] == GeneralTaskSubscription.RANDOM: - raise serializers.ValidationError( - f'The {data["query_type"]} query_type does not work with the' - f'provided key') - - try: - GeneralTaskSubscription.objects.get( - owner=data['owner'], - query_type=data['query_type'], - query_key=data.get('query_key', None)) - except ObjectDoesNotExist: - pass - else: - raise serializers.ValidationError( - 'The user already has the subscription') - - return data - - def create(self, validated_data) -> GeneralTaskSubscription: - return GeneralTaskSubscription.objects.create(**validated_data) - - class Meta: - model = GeneralTaskSubscription - fields = ( - 'pk', - 'owner', - 'query_type', - 'query_key', - 'assignments', - 'feedback_stage') diff --git a/core/serializers/__init__.py b/core/serializers/__init__.py new file mode 100644 index 00000000..5cafb28c --- /dev/null +++ b/core/serializers/__init__.py @@ -0,0 +1,5 @@ +from .common_serializers import * # noqa +from .feedback import FeedbackSerializer # noqa +from .subscription import * # noqa +from .student import * # noqa +from .submission import * # noqa diff --git a/core/serializers/common_serializers.py b/core/serializers/common_serializers.py new file mode 100644 index 00000000..72c0a437 --- /dev/null +++ b/core/serializers/common_serializers.py @@ -0,0 +1,54 @@ +import logging + +from rest_framework import serializers + +from core.models import ExamType, SubmissionType, Test, UserAccount +from util.factories import GradyUserFactory + +from .generic import DynamicFieldsModelSerializer + +log = logging.getLogger(__name__) +user_factory = GradyUserFactory() + + +class ExamSerializer(DynamicFieldsModelSerializer): + + class Meta: + model = ExamType + fields = ('pk', 'module_reference', 'total_score', + 'pass_score', 'pass_only',) + + +class TestSerializer(DynamicFieldsModelSerializer): + + class Meta: + model = Test + fields = ('pk', 'name', 'label', 'annotation') + + +class SubmissionTypeListSerializer(DynamicFieldsModelSerializer): + + class Meta: + model = SubmissionType + fields = ('pk', 'name', 'full_score') + + +class SubmissionTypeSerializer(SubmissionTypeListSerializer): + + class Meta: + model = SubmissionType + fields = ('pk', 'name', 'full_score', 'description', 'solution') + + +class TutorSerializer(DynamicFieldsModelSerializer): + done_assignments_count = serializers.IntegerField( + read_only=True) + + def create(self, validated_data) -> UserAccount: + log.info("Crating tutor from data %s", validated_data) + return user_factory.make_tutor( + username=validated_data['username']) + + class Meta: + model = UserAccount + fields = ('pk', 'username', 'done_assignments_count') diff --git a/core/serializers/feedback.py b/core/serializers/feedback.py new file mode 100644 index 00000000..95e0e0d9 --- /dev/null +++ b/core/serializers/feedback.py @@ -0,0 +1,160 @@ +import logging +from collections import defaultdict + +from django.core.exceptions import ObjectDoesNotExist +from django.db import transaction +from django.db.models.manager import Manager +from rest_framework import serializers +from rest_framework.utils import html + +from core import models +from core.models import Feedback, TutorSubmissionAssignment +from util.factories import GradyUserFactory + +from .generic import DynamicFieldsModelSerializer + +log = logging.getLogger(__name__) +user_factory = GradyUserFactory() + + +class FeedbackCommentDictionarySerializer(serializers.ListSerializer): + + def to_internal_value(self, comment_dict): + """ Converts a line_no -> comment list dictionary back to a list + of comments. Currently we do not have any information about the + feedback since it is not availiable in this scope. Feedback is + responsible to add it later on update/creation """ + if html.is_html_input(comment_dict): + comment_dict = html.parse_html_list(comment_dict) + + if not isinstance(comment_dict, dict): + raise serializers.ValidationError( + 'Comments have to be provided as a dict' + 'with: line -> list of comments' + ) + + ret = [] + errors = [] + + for line, comment in comment_dict.items(): + try: + comment['of_line'] = line + validated = self.child.run_validation(comment) + except serializers.ValidationError as err: + errors.append(err.detail) + else: + ret.append(validated) + errors.append({}) + + if any(errors): + raise serializers.ValidationError(errors) + + return ret + + def to_representation(self, comments): + """ Provides a dict where all the keys correspond to lines and contain + a list of comments on that line. """ + if isinstance(comments, Manager): + comments = comments.all() + + ret = defaultdict(list) + for comment in comments: + ret[comment.of_line].append(self.child.to_representation(comment)) + return ret + + +class FeedbackCommentSerializer(serializers.ModelSerializer): + of_tutor = serializers.StringRelatedField(source='of_tutor.username') + + class Meta: + model = models.FeedbackComment + fields = ('text', + 'created', + 'of_tutor', + 'of_line', + 'visible_to_student') + read_only_fields = ('created', 'of_tutor') + extra_kwargs = { + 'of_feedback': {'write_only': True}, + 'of_line': {'write_only': True}, + } + list_serializer_class = FeedbackCommentDictionarySerializer + + +class FeedbackSerializer(DynamicFieldsModelSerializer): + pk = serializers.ReadOnlyField(source='of_submission.pk') + assignment_pk = serializers.UUIDField(write_only=True) + feedback_lines = FeedbackCommentSerializer(many=True, required=False) + + @transaction.atomic + def create(self, validated_data) -> Feedback: + assignment = validated_data.pop('assignment_pk') + feedback_lines = validated_data.pop('feedback_lines', []) + + feedback = Feedback.objects.create(of_submission=assignment.submission, + **validated_data) + + for comment in feedback_lines: + models.FeedbackComment.objects.create( + of_feedback=feedback, + of_tutor=self.context['request'].user, + **comment + ) + + assignment.set_done() + return Feedback.objects.get(of_submission=assignment.submission) + + @transaction.atomic + def update(self, instance, validated_data): + for comment in validated_data.pop('feedback_lines', []): + models.FeedbackComment.objects.update_or_create( + of_feedback=instance, + of_tutor=self.context['request'].user, + of_line=comment.get('of_line'), + defaults={'text': comment.get('text')}) + + return super().update(instance, validated_data) + + def validate_assignment_pk(self, assignment_pk): + try: + assignment = TutorSubmissionAssignment.objects.get( + pk=assignment_pk) + except ObjectDoesNotExist as err: + raise serializers.ValidationError('No assignment for given id.') + + return assignment + + def validate(self, data): + log.debug("Validate feedback data: %s", data) + score = data.get('score') + assignment = data.get('assignment_pk') + + submission = assignment.submission + if not 0 <= score <= submission.type.full_score: + raise serializers.ValidationError( + f'Score has to be in range [0..{submission.type.full_score}].') + + has_full_score = score == submission.type.full_score + has_feedback_lines = ('feedback_lines' in data and + len(data['feedback_lines']) > 0) + if not has_full_score and not has_feedback_lines: + raise serializers.ValidationError( + 'Sorry, you have to explain why this does not get full score') + + http_method = self.context['request'].method + if hasattr(submission, 'feedback') and http_method == 'POST': + raise serializers.ValidationError( + 'Feedback for this submission already exists') + + for comment in data.get('feedback_lines', {}): + lines_in_submission = len(submission.text.split('\n')) + if not 0 < comment['of_line'] <= lines_in_submission: + raise serializers.ValidationError( + "Cannot comment line number %d of %d" % ( + comment['of_line'], lines_in_submission)) + + return data + + class Meta: + model = Feedback + fields = ('pk', 'assignment_pk', 'is_final', 'score', 'feedback_lines') diff --git a/core/serializers/generic.py b/core/serializers/generic.py new file mode 100644 index 00000000..20edc3c6 --- /dev/null +++ b/core/serializers/generic.py @@ -0,0 +1,20 @@ +from drf_dynamic_fields import DynamicFieldsMixin +from rest_framework import serializers + + +class DynamicFieldsModelSerializer(DynamicFieldsMixin, + serializers.ModelSerializer): + + def __init__(self, *args, **kwargs): + # Don't pass the 'fields' arg up to the superclass + fields = kwargs.pop('fields', None) + + # Instantiate the superclass normally + super(DynamicFieldsModelSerializer, self).__init__(*args, **kwargs) + + if fields is not None: + # Drop any fields that are not specified in the `fields` argument. + allowed = set(fields) + existing = set(self.fields.keys()) + for field_name in existing - allowed: + self.fields.pop(field_name) diff --git a/core/serializers/student.py b/core/serializers/student.py new file mode 100644 index 00000000..6f71ef4b --- /dev/null +++ b/core/serializers/student.py @@ -0,0 +1,28 @@ +from rest_framework import serializers + +from core.models import StudentInfo +from core.serializers import DynamicFieldsModelSerializer, ExamSerializer +from core.serializers.submission import (SubmissionListSerializer, + SubmissionNoTextFieldsSerializer) + + +class StudentInfoSerializer(DynamicFieldsModelSerializer): + name = serializers.ReadOnlyField(source='user.fullname') + matrikel_no = serializers.ReadOnlyField(source='user.matrikel_no') + exam = ExamSerializer() + submissions = SubmissionListSerializer(many=True) + + class Meta: + model = StudentInfo + fields = ('pk', 'name', 'user', 'matrikel_no', 'exam', 'submissions') + + +class StudentInfoSerializerForListView(DynamicFieldsModelSerializer): + name = serializers.ReadOnlyField(source='user.fullname') + user = serializers.ReadOnlyField(source='user.username') + exam = serializers.ReadOnlyField(source='exam.module_reference') + submissions = SubmissionNoTextFieldsSerializer(many=True) + + class Meta: + model = StudentInfo + fields = ('pk', 'name', 'user', 'exam', 'submissions') diff --git a/core/serializers/submission.py b/core/serializers/submission.py new file mode 100644 index 00000000..f8d777e4 --- /dev/null +++ b/core/serializers/submission.py @@ -0,0 +1,36 @@ +from rest_framework import serializers + +from core.models import Submission +from core.serializers import (DynamicFieldsModelSerializer, FeedbackSerializer, + SubmissionTypeListSerializer, + SubmissionTypeSerializer, TestSerializer) + + +class SubmissionNoTextFieldsSerializer(DynamicFieldsModelSerializer): + score = serializers.ReadOnlyField(source='feedback.score') + type = serializers.ReadOnlyField(source='type.name') + full_score = serializers.ReadOnlyField(source='type.full_score') + + class Meta: + model = Submission + fields = ('pk', 'type', 'score', 'full_score') + + +class SubmissionSerializer(DynamicFieldsModelSerializer): + type = SubmissionTypeSerializer() + feedback = FeedbackSerializer() + tests = TestSerializer(many=True) + + class Meta: + model = Submission + fields = ('pk', 'type', 'text', 'feedback', 'tests') + + +class SubmissionListSerializer(DynamicFieldsModelSerializer): + type = SubmissionTypeListSerializer(fields=('pk', 'name', 'full_score')) + # TODO change this according to new feedback model + feedback = FeedbackSerializer() + + class Meta: + model = Submission + fields = ('pk', 'type', 'feedback') diff --git a/core/serializers/subscription.py b/core/serializers/subscription.py new file mode 100644 index 00000000..9bbeaab5 --- /dev/null +++ b/core/serializers/subscription.py @@ -0,0 +1,75 @@ +from django.core.exceptions import ObjectDoesNotExist +from rest_framework import serializers + +from core.models import (GeneralTaskSubscription, Submission, + TutorSubmissionAssignment) +from core.serializers import DynamicFieldsModelSerializer, FeedbackSerializer + + +class AssignmentSerializer(DynamicFieldsModelSerializer): + submission_pk = serializers.ReadOnlyField( + source='submission.pk') + + class Meta: + model = TutorSubmissionAssignment + fields = ('pk', 'submission_pk', 'is_done',) + + +class SubmissionAssignmentSerializer(DynamicFieldsModelSerializer): + text = serializers.ReadOnlyField() + type_pk = serializers.ReadOnlyField(source='type.pk') + full_score = serializers.ReadOnlyField(source='type.full_score') + + class Meta: + model = Submission + fields = ('pk', 'type_pk', 'text', 'full_score') + + +class AssignmentDetailSerializer(DynamicFieldsModelSerializer): + submission = SubmissionAssignmentSerializer() + feedback = FeedbackSerializer(source='submission.feedback') + + class Meta: + model = TutorSubmissionAssignment + fields = ('pk', 'feedback', 'submission', 'is_done',) + + +class SubscriptionSerializer(DynamicFieldsModelSerializer): + owner = serializers.ReadOnlyField(source='owner.username') + query_key = serializers.CharField(required=False) + assignments = AssignmentSerializer(read_only=True, many=True) + + def validate(self, data): + data['owner'] = self.context['request'].user + + if 'query_key' in data != \ + data['query_type'] == GeneralTaskSubscription.RANDOM: + raise serializers.ValidationError( + f'The {data["query_type"]} query_type does not work with the' + f'provided key') + + try: + GeneralTaskSubscription.objects.get( + owner=data['owner'], + query_type=data['query_type'], + query_key=data.get('query_key', None)) + except ObjectDoesNotExist: + pass + else: + raise serializers.ValidationError( + 'The user already has the subscription') + + return data + + def create(self, validated_data) -> GeneralTaskSubscription: + return GeneralTaskSubscription.objects.create(**validated_data) + + class Meta: + model = GeneralTaskSubscription + fields = ( + 'pk', + 'owner', + 'query_type', + 'query_key', + 'assignments', + 'feedback_stage') diff --git a/core/tests/test_feedback.py b/core/tests/test_feedback.py index 2512b6bc..17b3cbfd 100644 --- a/core/tests/test_feedback.py +++ b/core/tests/test_feedback.py @@ -3,10 +3,9 @@ from rest_framework.test import (APIRequestFactory, APITestCase, force_authenticate) from core import models -from core.models import (Feedback, FeedbackComment, FeedbackForSubmissionLine, - Submission, SubmissionType) +from core.models import Feedback, FeedbackComment, Submission, SubmissionType from core.views import FeedbackApiView -from util.factories import GradyUserFactory +from util.factories import GradyUserFactory, make_test_data class FeedbackRetrieveTestCase(APITestCase): @@ -28,13 +27,11 @@ class FeedbackRetrieveTestCase(APITestCase): type=cls.submission_type) cls.feedback = Feedback.objects.create(score=23, is_final=False, of_submission=cls.sub) - cls.linelist = [] - for line in range(2): - cls.linelist.append(FeedbackForSubmissionLine.objects.create( - of_line=line + 1, of_feedback=cls.feedback)) - for line in cls.linelist: + + for line in range(1, 3): for tutor in cls.tutors: FeedbackComment.objects.create(text='fortytwo', + of_feedback=cls.feedback, of_tutor=tutor, of_line=line) @@ -42,18 +39,15 @@ class FeedbackRetrieveTestCase(APITestCase): self.request = self.request_factory.get(f'/api/feedback/{self.sub.pk}') force_authenticate(self.request, self.tutor) self.view = FeedbackApiView.as_view({'get': 'retrieve'}) - self.response = self.view( - self.request, - submission_id=str(self.sub.pk) - ) + self.response = self.view(self.request, + submission_pk=str(self.sub.pk)) self.data = self.response.data def test_only_one_final_comment_per_line(self): - comments_on_first_line = FeedbackForSubmissionLine.objects \ - .first().comment_list.all() - self.assertEqual(2, len(comments_on_first_line)) - final_comments = [comment for comment in comments_on_first_line - if comment.is_final] + comments_on_first_line = FeedbackComment.objects.filter(of_line=1) + self.assertEqual(2, comments_on_first_line.count()) + final_comments = [comment for comment in comments_on_first_line.all() + if comment.visible_to_student] self.assertEqual(1, len(final_comments)) def test_can_retrieve_feedback_via_endpoint(self): @@ -88,9 +82,9 @@ class FeedbackRetrieveTestCase(APITestCase): self.data['feedback_lines'][1][0]['of_tutor']) def test_if_comment_has_final(self): - self.assertIn('is_final', self.data['feedback_lines'][1][0]) + self.assertIn('visible_to_student', self.data['feedback_lines'][1][0]) self.assertIsNotNone( - self.data['feedback_lines'][1][0]['is_final']) + self.data['feedback_lines'][1][0]['visible_to_student']) class FeedbackCreateTestCase(APITestCase): @@ -105,8 +99,13 @@ class FeedbackCreateTestCase(APITestCase): name='Cooking some crystal with Jesse', full_score=100 ) + text = ''' First line of defense + We do not have a second line + security via obscurity + is very bad. ''' cls.sub = Submission.objects.create(student=cls.student.student, - type=cls.submission_type) + type=cls.submission_type, + text=text) def setUp(self): self.client.force_authenticate(user=self.tutor) @@ -117,7 +116,7 @@ class FeedbackCreateTestCase(APITestCase): ) self.assignment = self.subscription.get_or_create_work_assignment() - def test_can_create_feedback_without_feedback_lines(self): + def test_cannot_create_feedback_without_feedback_lines(self): # TODO this test has to be adapted to test the various constraints # e.g. feedback without lines can only be given if the score is equal # to the max Score for this submission @@ -129,8 +128,8 @@ class FeedbackCreateTestCase(APITestCase): } self.assertEqual(Feedback.objects.count(), 0) response = self.client.post(self.url, data, format='json') - self.assertEqual(response.status_code, status.HTTP_201_CREATED) - self.assertEqual(Feedback.objects.count(), 1) + self.assertEqual(status.HTTP_400_BAD_REQUEST, response.status_code) + self.assertEqual(Feedback.objects.count(), 0) def test_cannot_create_feedback_with_score_higher_than_max(self): data = { @@ -143,6 +142,26 @@ class FeedbackCreateTestCase(APITestCase): self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) self.assertEqual(Feedback.objects.count(), 0) + def test_tutor_cannot_set_feedback_final_on_creation(self): + data = { + 'score': 100, + 'is_final': True, + 'assignment_pk': self.assignment.assignment_id + } + response = self.client.post(self.url, data, format='json') + self.assertEqual(status.HTTP_403_FORBIDDEN, response.status_code) + self.assertEqual(Feedback.objects.count(), 0) + + def test_tutor_has_to_write_a_line_if_score_is_not_100_percent(self): + data = { + 'score': 50, + 'is_final': False, + 'assignment_pk': self.assignment.assignment_id + } + response = self.client.post(self.url, data, format='json') + self.assertEqual(status.HTTP_400_BAD_REQUEST, response.status_code) + self.assertEqual(Feedback.objects.count(), 0) + def test_cannot_create_feedback_with_score_less_than_zero(self): data = { 'score': -1, @@ -158,7 +177,12 @@ class FeedbackCreateTestCase(APITestCase): data = { 'score': 5, 'is_final': False, - 'assignment_pk': self.assignment.pk + 'assignment_pk': self.assignment.pk, + 'feedback_lines': { + '4': { + 'text': 'Why you no learn how to code, man?' + } + } } self.client.post(self.url, data, format='json') object_score = self.sub.feedback.score @@ -170,7 +194,7 @@ class FeedbackCreateTestCase(APITestCase): 'is_final': False, 'assignment_pk': self.assignment.pk, 'feedback_lines': { - '5': { + '4': { 'text': 'Nice meth!' } } @@ -186,7 +210,7 @@ class FeedbackCreateTestCase(APITestCase): 'is_final': False, 'assignment_pk': self.assignment.pk, 'feedback_lines': { - '5': { + '3': { 'text': 'Nice meth!' } } @@ -196,8 +220,37 @@ class FeedbackCreateTestCase(APITestCase): self.assertEqual(comment.of_tutor, self.tutor) self.assertEqual(comment.text, 'Nice meth!') self.assertIsNotNone(comment.created) - self.assertEqual(comment.of_line.of_line, 5) - self.assertTrue(comment.is_final) + self.assertEqual(comment.of_line, 3) + self.assertTrue(comment.visible_to_student) + + def test_cannot_create_without_assignment(self): + data = { + 'score': 0, + 'assignment_pk': self.assignment.assignment_id, + 'feedback_lines': { + '2': { + 'text': 'Well, at least you tried.' + }, + } + } + self.assignment.delete() + response = self.client.post(self.url, data, format='json') + self.assertEqual(status.HTTP_400_BAD_REQUEST, response.status_code) + + def test_cannot_create_with_someoneelses_assignment(self): + data = { + 'score': 0, + 'assignment_pk': self.assignment.assignment_id, + 'feedback_lines': { + '1': { + 'text': 'Well, at least you tried.' + }, + } + } + other_tutor = self.user_factory.make_tutor('Berta') + self.client.force_authenticate(other_tutor) + response = self.client.post(self.url, data, format='json') + self.assertEqual(status.HTTP_403_FORBIDDEN, response.status_code) def test_can_create_multiple_feedback_comments(self): data = { @@ -205,10 +258,10 @@ class FeedbackCreateTestCase(APITestCase): 'is_final': False, 'assignment_pk': self.assignment.pk, 'feedback_lines': { - '5': { + '1': { 'text': 'Nice meth!' }, - '7': { + '3': { 'text': 'Good one!' } } @@ -217,11 +270,123 @@ class FeedbackCreateTestCase(APITestCase): first_comment = FeedbackComment.objects.get(text='Nice meth!') self.assertEqual(first_comment.of_tutor, self.tutor) self.assertIsNotNone(first_comment.created) - self.assertEqual(first_comment.of_line.of_line, 5) - self.assertTrue(first_comment.is_final) + self.assertEqual(first_comment.of_line, 1) + self.assertTrue(first_comment.visible_to_student) second_comment = FeedbackComment.objects.get(text='Good one!') self.assertEqual(second_comment.of_tutor, self.tutor) self.assertIsNotNone(second_comment.created) - self.assertEqual(second_comment.of_line.of_line, 7) - self.assertTrue(second_comment.is_final) + self.assertEqual(second_comment.of_line, 3) + self.assertTrue(second_comment.visible_to_student) + + +class FeedbackPatchTestCase(APITestCase): + + @classmethod + def setUpTestData(cls): + cls.burl = '/api/feedback/' + cls.data = make_test_data({ + 'submission_types': [ + { + 'name': '01. Sort this or that', + 'full_score': 35, + 'description': 'Very complicated', + 'solution': 'Trivial!' + }], + 'students': [ + {'username': 'student01'} + ], + 'tutors': [ + {'username': 'tutor01'}, + {'username': 'tutor02'} + ], + 'reviewers': [{ + 'username': 'reviewer01', + }], + 'submissions': [{ + 'text': 'function blabl\n' + ' on multi lines\n' + ' for blabla in bla:\n', + 'type': '01. Sort this or that', + 'user': 'student01' + }] + }) + + def setUp(self): + self.tutor01 = self.data['tutors'][0] + self.tutor02 = self.data['tutors'][1] + self.client.force_authenticate(user=self.tutor01) + self.subscription = models.GeneralTaskSubscription.objects.create( + owner=self.tutor01, + query_type='random', + query_key='' + ) + self.assignment = self.subscription.get_or_create_work_assignment() + data = { + 'score': 35, + 'is_final': False, + 'assignment_pk': self.assignment.assignment_id, + 'feedback_lines': { + '2': {'text': 'Very good.'}, + } + } + response = self.client.post(self.burl, data, format='json') + self.feedback = Feedback.objects.get(of_submission=response.data['pk']) + self.url = f'{self.burl}{self.feedback.of_submission.submission_id}/' + + def test_can_patch_onto_the_own_feedback(self): + data = { + 'feedback_lines': { + '1': {'text': 'Spam spam spam'}, + } + } + response = self.client.patch(self.url, data, format='json') + self.assertEqual(status.HTTP_200_OK, response.status_code) + self.assertEqual( + 'Spam spam spam', + response.data['feedback_lines'][1][0]['text'] + ) + self.assertEqual( + 'Very good.', + response.data['feedback_lines'][2][0]['text'] + ) + + def test_can_update_a_single_line(self): + data = { + 'feedback_lines': { + '2': {'text': 'Turns out this is rather bad.'}, + } + } + + response = self.client.patch(self.url, data, format='json') + self.assertEqual(status.HTTP_200_OK, response.status_code) + + def test_tutor_can_not_update_when_there_is_a_new_assignment(self): + # Step 1 - Create a new assignment + second_subs = models.GeneralTaskSubscription.objects.create( + owner=self.tutor02, + query_type='random', + feedback_stage='feedback-validation' + ) + second_subs.get_or_create_work_assignment() + + # Step 2 - Tutor 2 tries to patch + data = { + 'feedback_lines': { + '2': {'text': 'Turns out this is rather bad.'}, + } + } + + response = self.client.patch(self.url, data, format='json') + self.assertEqual(status.HTTP_403_FORBIDDEN, response.status_code) + + def test_cannot_patch_first_feedback_final(self): + data = { + 'feedback_lines': { + '2': {'text': 'Turns out this is rather bad.'}, + }, + 'is_final': True + } + + response = self.client.patch(self.url, data, format='json') + self.assertEqual(status.HTTP_403_FORBIDDEN, response.status_code) diff --git a/core/views.py b/core/views.py deleted file mode 100644 index 10142562..00000000 --- a/core/views.py +++ /dev/null @@ -1,205 +0,0 @@ -""" All API views that are used to retrieve data from the database. They -can be categorized by the permissions they require. All views require a -user to be authenticated and most are only accessible by one user group """ -from django.conf import settings -from rest_framework import generics, mixins, status, viewsets -from rest_framework.decorators import api_view, detail_route -from rest_framework.response import Response - -from core import models, serializers -from core.models import (ExamType, Feedback, GeneralTaskSubscription, - StudentInfo, SubmissionType, - TutorSubmissionAssignment) -from core.permissions import IsReviewer, IsStudent, IsTutorOrReviewer -from core.serializers import (AssignmentDetailSerializer, AssignmentSerializer, - ExamSerializer, FeedbackSerializer, - StudentInfoSerializer, - StudentInfoSerializerForListView, - SubmissionSerializer, SubmissionTypeSerializer, - SubscriptionSerializer, TutorSerializer) - - -@api_view() -def get_jwt_expiration_delta(request): - return Response({'timeDelta': settings.JWT_AUTH['JWT_EXPIRATION_DELTA']}) - - -@api_view() -def get_user_role(request): - return Response({'role': request.user.role}) - - -class StudentSelfApiView(generics.RetrieveAPIView): - """ Gets all data that belongs to one student """ - permission_classes = (IsStudent,) - serializer_class = StudentInfoSerializer - - def get_object(self) -> StudentInfo: - """ The object in question is the student associated with the requests - user. Since the permission IsStudent is satisfied the member exists """ - return self.request.user.student - - -class StudentSelfSubmissionsApiView(generics.ListAPIView): - permission_classes = (IsStudent, ) - serializer_class = SubmissionSerializer - - def get_queryset(self): - return self.request.user.student.submissions - - -class ExamApiViewSet(viewsets.ReadOnlyModelViewSet): - """ Gets a list of an individual exam by Id if provided """ - permission_classes = (IsTutorOrReviewer,) - queryset = ExamType.objects.all() - serializer_class = ExamSerializer - - -class FeedbackApiView( - mixins.CreateModelMixin, - mixins.RetrieveModelMixin, - viewsets.GenericViewSet): - """ Gets a list of an individual exam by Id if provided """ - permission_classes = (IsTutorOrReviewer,) - queryset = Feedback.objects.all() - serializer_class = FeedbackSerializer - lookup_field = 'of_submission__submission_id' - lookup_url_kwarg = 'submission_id' - - def create(self, request, *args, **kwargs): - serializer = self.get_serializer(data=request.data) - - try: - serializer.is_valid(raise_exception=True) - except serializers.NoValidAssignmentForFeedbackCreation: - return Response( - {'This user has not permission to create this feedback'}, - status=status.HTTP_403_FORBIDDEN) - except serializers.CannotSetFirstFeedbackFinal: - return Response( - {'Cannot set the first feedback final unless user reviewer'}, - status=status.HTTP_403_FORBIDDEN) - - self.perform_create(serializer) - headers = self.get_success_headers(serializer.data) - return Response(serializer.data, - status=status.HTTP_201_CREATED, - headers=headers) - - -class TutorApiViewSet( - mixins.RetrieveModelMixin, - mixins.CreateModelMixin, - mixins.DestroyModelMixin, - mixins.ListModelMixin, - viewsets.GenericViewSet): - """ Api endpoint for creating, listing, viewing or deleteing tutors """ - permission_classes = (IsReviewer,) - queryset = models.UserAccount.objects.filter(role='Tutor') - serializer_class = TutorSerializer - lookup_field = 'username' - lookup_url_kwarg = 'username' - - -class StudentReviewerApiViewSet(viewsets.ReadOnlyModelViewSet): - """ Gets a list of all students without individual submissions """ - permission_classes = (IsReviewer,) - queryset = StudentInfo.objects.all() - serializer_class = StudentInfoSerializerForListView - - -class SubmissionTypeApiView(viewsets.ReadOnlyModelViewSet): - """ Gets a list or a detail view of a single SubmissionType """ - queryset = SubmissionType.objects.all() - serializer_class = SubmissionTypeSerializer - - -class SubscriptionApiViewSet( - mixins.RetrieveModelMixin, - mixins.CreateModelMixin, - mixins.ListModelMixin, - viewsets.GenericViewSet): - permission_classes = (IsTutorOrReviewer,) - queryset = GeneralTaskSubscription.objects.all() - serializer_class = SubscriptionSerializer - - @detail_route(methods=['get'], url_path='assignments/current') - def current_assignment(self, request, pk=None): - subscription = self.get_object() - try: - assignment = subscription.get_oldest_unfinished_assignment() - except models.SubscriptionEnded as err: - return Response( - {'Error': 'This subscription has ended'}, - status=status.HTTP_410_GONE) - serializer = AssignmentDetailSerializer(assignment) - return Response(serializer.data) - - @detail_route(methods=['get'], url_path='assignments/next') - def next_assignment(self, request, pk=None): - subscription = self.get_object() - try: - assignment = subscription.get_youngest_unfinished_assignment() - except models.SubscriptionEnded as err: - return Response( - {'Error': 'Seems there is nothing left to prefetch'}, - status=status.HTTP_410_GONE) - serializer = AssignmentDetailSerializer(assignment) - return Response(serializer.data) - - def get_queryset(self): - return GeneralTaskSubscription.objects.filter(owner=self.request.user) - - def destroy(self, request, pk=None): - instance = self.get_object() - - # todo: prevent this via deactivation - - instance.delete() - return Response(status=status.HTTP_204_NO_CONTENT) - - def create(self, request, *args, **kwargs): - serializer = self.get_serializer(data=request.data) - serializer.is_valid(raise_exception=True) - subscription = serializer.save() - - try: - if subscription.query_type == \ - GeneralTaskSubscription.STUDENT_QUERY: - subscription.reserve_all_assignments_for_a_student() - else: - subscription.get_oldest_unfinished_assignment() - except models.SubscriptionEnded as err: - return Response( - {'Error': 'This subscription has no available submissions'}, - status.HTTP_410_GONE - ) - - headers = self.get_success_headers(serializer.data) - return Response(serializer.data, - status=status.HTTP_201_CREATED, - headers=headers) - - -class AssignmentApiViewSet( - mixins.RetrieveModelMixin, - mixins.ListModelMixin, - viewsets.GenericViewSet): - permission_classes = (IsTutorOrReviewer,) - queryset = TutorSubmissionAssignment.objects.all() - serializer_class = AssignmentSerializer - - def get_queryset(self): - """ Get only assignments of that user """ - return TutorSubmissionAssignment.objects.filter( - subscription__owner=self.request.user) - - def destroy(self, request, pk=None): - """ Stop working on the assignment before it is finished """ - instance = self.get_object() - - if instance.is_done: - return Response(status=status.HTTP_403_FORBIDDEN) # test - - instance.delete() - return Response(status=status.HTTP_204_NO_CONTENT) # test diff --git a/core/views/__init__.py b/core/views/__init__.py new file mode 100644 index 00000000..990e8c5c --- /dev/null +++ b/core/views/__init__.py @@ -0,0 +1,3 @@ +from .feedback import FeedbackApiView # noqa +from .subscription import SubscriptionApiViewSet, AssignmentApiViewSet # noqa +from .common_views import * # noqa diff --git a/core/views/common_views.py b/core/views/common_views.py new file mode 100644 index 00000000..344db1a2 --- /dev/null +++ b/core/views/common_views.py @@ -0,0 +1,84 @@ +""" All API views that are used to retrieve data from the database. They +can be categorized by the permissions they require. All views require a +user to be authenticated and most are only accessible by one user group """ +import logging + +from django.conf import settings +from rest_framework import generics, mixins, viewsets +from rest_framework.decorators import api_view +from rest_framework.response import Response + +from core import models +from core.models import ExamType, StudentInfo, SubmissionType +from core.permissions import IsReviewer, IsStudent +from core.serializers import (ExamSerializer, StudentInfoSerializer, + StudentInfoSerializerForListView, + SubmissionSerializer, SubmissionTypeSerializer, + TutorSerializer) + +log = logging.getLogger(__name__) + + +@api_view() +def get_jwt_expiration_delta(request): + return Response({'timeDelta': settings.JWT_AUTH['JWT_EXPIRATION_DELTA']}) + + +@api_view() +def get_user_role(request): + return Response({'role': request.user.role}) + + +class StudentSelfApiView(generics.RetrieveAPIView): + """ Gets all data that belongs to one student """ + permission_classes = (IsStudent,) + serializer_class = StudentInfoSerializer + + def get_object(self) -> StudentInfo: + """ The object in question is the student associated with the requests + user. Since the permission IsStudent is satisfied the member exists """ + if self.request.user.is_superuser: + return StudentInfo.objects.last() + return self.request.user.student + + +class StudentSelfSubmissionsApiView(generics.ListAPIView): + permission_classes = (IsStudent, ) + serializer_class = SubmissionSerializer + + def get_queryset(self): + return self.request.user.student.submissions + + +class StudentReviewerApiViewSet(viewsets.ReadOnlyModelViewSet): + """ Gets a list of all students without individual submissions """ + permission_classes = (IsReviewer,) + queryset = StudentInfo.objects.all() + serializer_class = StudentInfoSerializerForListView + + +class ExamApiViewSet(viewsets.ReadOnlyModelViewSet): + """ Gets a list of an individual exam by Id if provided """ + permission_classes = (IsReviewer,) + queryset = ExamType.objects.all() + serializer_class = ExamSerializer + + +class TutorApiViewSet( + mixins.RetrieveModelMixin, + mixins.CreateModelMixin, + mixins.DestroyModelMixin, + mixins.ListModelMixin, + viewsets.GenericViewSet): + """ Api endpoint for creating, listing, viewing or deleteing tutors """ + permission_classes = (IsReviewer,) + queryset = models.UserAccount.objects.filter(role='Tutor') + serializer_class = TutorSerializer + lookup_field = 'username' + lookup_url_kwarg = 'username' + + +class SubmissionTypeApiView(viewsets.ReadOnlyModelViewSet): + """ Gets a list or a detail view of a single SubmissionType """ + queryset = SubmissionType.objects.all() + serializer_class = SubmissionTypeSerializer diff --git a/core/views/feedback.py b/core/views/feedback.py new file mode 100644 index 00000000..4bdf36f6 --- /dev/null +++ b/core/views/feedback.py @@ -0,0 +1,100 @@ +import logging + +from rest_framework import mixins, status, viewsets +from rest_framework.response import Response + +from core import models, permissions, serializers + +log = logging.getLogger(__name__) + + +class FeedbackApiView( + mixins.CreateModelMixin, + mixins.RetrieveModelMixin, + viewsets.GenericViewSet): + """ Gets a list of an individual exam by Id if provided """ + permission_classes = (permissions.IsTutorOrReviewer,) + queryset = models.Feedback.objects.all() + serializer_class = serializers.FeedbackSerializer + lookup_field = 'of_submission__pk' + lookup_url_kwarg = 'submission_pk' + + def _tutor_attempts_to_change_final_feedback(self, serializer): + feedback_is_final = serializer.instance.is_final + user_is_tutor = self.request.user.role == models.UserAccount.TUTOR + return feedback_is_final and user_is_tutor + + def _request_user_does_not_own_assignment(self, serializer): + assignment = serializer.validated_data['assignment_pk'] + return assignment.subscription.owner != self.request.user + + def _tutor_attempts_to_set_first_feedback_final(self, serializer): + is_final_set = serializer.validated_data.get('is_final', False) + user_is_tutor = self.request.user.role == models.UserAccount.TUTOR + return is_final_set and user_is_tutor + + def _tutor_is_allowed_to_change_own_feedback(self, serializer): + assignment = serializer.validated_data['assignment_pk'] + youngest = models.TutorSubmissionAssignment.objects\ + .filter(submission=assignment.submission)\ + .order_by('-created')\ + .first() + + return assignment == youngest + + def _tutor_attempts_to_patch_first_feedback_final(self, serializer): + is_final_set = serializer.validated_data.get('is_final', False) + assignment = serializer.validated_data.get('assignment_pk') + in_creation = assignment.subscription.feedback_stage == models.GeneralTaskSubscription.FEEDBACK_CREATION # noqa + return is_final_set and in_creation + + def create(self, request, *args, **kwargs): + serializer = self.get_serializer(data=request.data) + serializer.is_valid(raise_exception=True) + + if self._tutor_attempts_to_set_first_feedback_final(serializer): + return Response( + {'It is not allowed to create feedback final for tutors'}, + status=status.HTTP_403_FORBIDDEN) + + if self._request_user_does_not_own_assignment(serializer): + return Response( + {'This user has not permission to create this feedback'}, + status=status.HTTP_403_FORBIDDEN) + + self.perform_create(serializer) + return Response(serializer.data, + status=status.HTTP_201_CREATED) + + def partial_update(self, request, **kwargs): + feedback = self.get_object() + assignment = models.TutorSubmissionAssignment.objects.get( + subscription__owner=request.user, + submission=feedback.of_submission + ) + serializer = self.get_serializer( + feedback, + data={'assignment_pk': assignment.pk, + 'score': feedback.score, + **request.data}, + partial=True) + + serializer.is_valid(raise_exception=True) + + if self._tutor_attempts_to_change_final_feedback(serializer): + return Response( + {'Cannot set the first feedback final unless user reviewer'}, + status=status.HTTP_403_FORBIDDEN) + + if self._tutor_attempts_to_patch_first_feedback_final(serializer): + return Response( + {'Cannot set the first feedback final unless user reviewer'}, + status=status.HTTP_403_FORBIDDEN) + + if not self._tutor_is_allowed_to_change_own_feedback(serializer): + return Response( + {'Sorry, but somebody else is already working on this on'}, + status=status.HTTP_403_FORBIDDEN) + + serializer.save() + return Response(serializer.data) diff --git a/core/views/subscription.py b/core/views/subscription.py new file mode 100644 index 00000000..e1920b3b --- /dev/null +++ b/core/views/subscription.py @@ -0,0 +1,104 @@ +import logging + +from rest_framework import mixins, status, viewsets +from rest_framework.decorators import detail_route +from rest_framework.response import Response + +from core import models, permissions, serializers +from core.models import TutorSubmissionAssignment +from core.permissions import IsTutorOrReviewer +from core.serializers import AssignmentSerializer + +log = logging.getLogger(__name__) + + +class SubscriptionApiViewSet( + mixins.RetrieveModelMixin, + mixins.CreateModelMixin, + mixins.ListModelMixin, + viewsets.GenericViewSet): + permission_classes = (permissions.IsTutorOrReviewer,) + queryset = models.GeneralTaskSubscription.objects.all() + serializer_class = serializers.SubscriptionSerializer + + def fetch_assignment(self, + request, + assignment_fetcher, + error_message): + try: + assignment = assignment_fetcher() + except models.SubscriptionEnded as err: + return Response( + {'Error': error_message}, + status=status.HTTP_410_GONE) + serializer = serializers.AssignmentDetailSerializer(assignment) + return Response(serializer.data) + + @detail_route(methods=['get'], url_path='assignments/current') + def current_assignment(self, request, pk=None): + subscription = self.get_object() + + return self.fetch_assignment( + request, + subscription.get_oldest_unfinished_assignment, + 'Seems this subscription has ended') + + @detail_route(methods=['get'], url_path='assignments/next') + def next_assignment(self, request, pk=None): + subscription = self.get_object() + + return self.fetch_assignment( + request, + subscription.get_youngest_unfinished_assignment, + 'Seems there is nothing left to prefetch') + + def get_queryset(self): + return models.GeneralTaskSubscription.objects.filter( + owner=self.request.user) + + def destroy(self, request, pk=None): + instance = self.get_object() + + raise NotImplemented # todo: just impl. deactivate and reactivate + + instance.delete() + return Response(status=status.HTTP_204_NO_CONTENT) + + def create(self, request, *args, **kwargs): + serializer = self.get_serializer(data=request.data) + serializer.is_valid(raise_exception=True) + subscription = serializer.save() + + if subscription.query_type == models.GeneralTaskSubscription.STUDENT_QUERY: # noqa TODO refactor name of generaltasksubscription to SubmissionSubscription + subscription.reserve_all_assignments_for_a_student() + else: + subscription.get_oldest_unfinished_assignment() + + headers = self.get_success_headers(serializer.data) + return Response(serializer.data, + status=status.HTTP_201_CREATED, + headers=headers) + + +class AssignmentApiViewSet( + mixins.RetrieveModelMixin, + mixins.ListModelMixin, + viewsets.GenericViewSet): + permission_classes = (IsTutorOrReviewer,) + queryset = TutorSubmissionAssignment.objects.all() + serializer_class = AssignmentSerializer + + def get_queryset(self): + """ Get only assignments of that user """ + return TutorSubmissionAssignment.objects.filter( + subscription__owner=self.request.user) + + def destroy(self, request, pk=None): + """ Stop working on the assignment before it is finished """ + instance = self.get_object() + + if instance.is_done: + return Response(status=status.HTTP_403_FORBIDDEN) # test + + instance.delete() + return Response(status=status.HTTP_204_NO_CONTENT) # test diff --git a/util/factories.py b/util/factories.py index 655f9720..89957819 100644 --- a/util/factories.py +++ b/util/factories.py @@ -150,15 +150,12 @@ def make_feedback(feedback, submission_object): is_final=feedback.get('is_final', False) )[0] for line_index, comment_list in feedback['feedback_lines'].items(): - line = models.FeedbackForSubmissionLine.objects.get_or_create( - of_line=int(line_index), - of_feedback=feedback_obj - )[0] for comment in comment_list: tutor = models.UserAccount.objects.get( username=comment.pop('of_tutor')) - models.FeedbackComment.objects.update_or_create( - of_line=line, + ret, c = models.FeedbackComment.objects.update_or_create( + of_line=line_index, + of_feedback=feedback_obj, of_tutor=tutor, defaults=comment ) diff --git a/util/importer.py b/util/importer.py index f2e0b0e1..9f60042b 100644 --- a/util/importer.py +++ b/util/importer.py @@ -8,8 +8,8 @@ from django.db import transaction import util.convert import util.processing -from core.models import UserAccount as User from core.models import ExamType, Feedback, Submission, SubmissionType, Test +from core.models import UserAccount as User from util.factories import GradyUserFactory from util.messages import info, warn -- GitLab