From 6d5f61cb1bd001543824497479323085330ed3a4 Mon Sep 17 00:00:00 2001
From: janmax <j.michal@stud.uni-goettingen.de>
Date: Wed, 10 Jan 2018 11:13:30 +0100
Subject: [PATCH] Changed the Feedback model to use lines instead of text
* Feedback consists of lines which can hold up to three comments
* The Feedback model contains almost no logic since all has been
migrated to the assignemnt mechanism
---
.gitignore | 1 +
core/migrations/0001_initial.py | 79 +++---
core/migrations/0002_auto_20171222_1116.py | 18 --
core/migrations/0003_auto_20180104_1631.py | 18 --
core/migrations/0004_feedback_is_final.py | 18 --
core/migrations/0005_auto_20180104_1851.py | 26 --
core/migrations/0006_auto_20180104_2001.py | 20 --
core/migrations/0007_auto_20180105_1136.py | 41 ----
core/models.py | 49 +++-
core/serializers.py | 63 ++++-
...actory_and_feedback.py => test_factory.py} | 0
core/tests/test_feedback.py | 228 ++++++++++++++++++
core/views.py | 7 +-
13 files changed, 383 insertions(+), 185 deletions(-)
delete mode 100644 core/migrations/0002_auto_20171222_1116.py
delete mode 100644 core/migrations/0003_auto_20180104_1631.py
delete mode 100644 core/migrations/0004_feedback_is_final.py
delete mode 100644 core/migrations/0005_auto_20180104_1851.py
delete mode 100644 core/migrations/0006_auto_20180104_2001.py
delete mode 100644 core/migrations/0007_auto_20180105_1136.py
rename core/tests/{test_factory_and_feedback.py => test_factory.py} (100%)
create mode 100644 core/tests/test_feedback.py
diff --git a/.gitignore b/.gitignore
index 5209d363..94f4e488 100644
--- a/.gitignore
+++ b/.gitignore
@@ -32,6 +32,7 @@ public/
.importer*
*.sublime-*
.idea/
+.vscode/
# node
node_modules
diff --git a/core/migrations/0001_initial.py b/core/migrations/0001_initial.py
index e28dd8e6..9102abb2 100644
--- a/core/migrations/0001_initial.py
+++ b/core/migrations/0001_initial.py
@@ -1,6 +1,4 @@
-# -*- coding: utf-8 -*-
-# Generated by Django 1.11.8 on 2017-12-22 10:51
-from __future__ import unicode_literals
+# Generated by Django 2.0.1 on 2018-01-07 14:37
import uuid
@@ -19,7 +17,7 @@ class Migration(migrations.Migration):
initial = True
dependencies = [
- ('auth', '0008_alter_user_username_max_length'),
+ ('auth', '0009_alter_user_last_name_max_length'),
]
operations = [
@@ -32,13 +30,14 @@ class Migration(migrations.Migration):
('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=30, verbose_name='last 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')),
],
@@ -73,6 +72,7 @@ class Migration(migrations.Migration):
('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)),
],
options={
@@ -81,23 +81,43 @@ class Migration(migrations.Migration):
},
),
migrations.CreateModel(
- name='GeneralTaskSubscription',
+ name='FeedbackComment',
fields=[
- ('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)),
- ('owner', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='susbscriptions', to=settings.AUTH_USER_MODEL)),
+ ('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)),
],
+ options={
+ 'verbose_name': 'Feedback Comment',
+ 'verbose_name_plural': 'Feedback Comments',
+ 'ordering': ('created',),
+ },
),
migrations.CreateModel(
- name='Reviewer',
+ name='FeedbackForSubmissionLine',
fields=[
('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
- ('user', models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, related_name='reviewer', to=settings.AUTH_USER_MODEL)),
+ ('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)),
+ ('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)),
+ ('owner', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='susbscriptions', to=settings.AUTH_USER_MODEL)),
],
),
migrations.CreateModel(
- name='Student',
+ name='StudentInfo',
fields=[
('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
('has_logged_in', models.BooleanField(default=False)),
@@ -116,7 +136,7 @@ class Migration(migrations.Migration):
('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.Student')),
+ ('student', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='submissions', to='core.StudentInfo')),
],
options={
'verbose_name': 'Submission',
@@ -152,19 +172,13 @@ class Migration(migrations.Migration):
'verbose_name_plural': 'Tests',
},
),
- migrations.CreateModel(
- name='Tutor',
- fields=[
- ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
- ('user', models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, related_name='tutor', to=settings.AUTH_USER_MODEL)),
- ],
- ),
migrations.CreateModel(
name='TutorSubmissionAssignment',
fields=[
('assignment_id', models.UUIDField(default=uuid.uuid4, editable=False, primary_key=True, serialize=False)),
- ('active', models.BooleanField(default=False)),
- ('submission', models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, related_name='assignment', to='core.Submission')),
+ ('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')),
],
),
@@ -174,9 +188,14 @@ class Migration(migrations.Migration):
field=models.ForeignKey(on_delete=django.db.models.deletion.PROTECT, related_name='submissions', to='core.SubmissionType'),
),
migrations.AddField(
- model_name='feedback',
- name='of_reviewer',
- field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='reviewed_submissions', to='core.Reviewer'),
+ 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),
),
migrations.AddField(
model_name='feedback',
@@ -186,18 +205,18 @@ class Migration(migrations.Migration):
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='core.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),
),
migrations.AlterUniqueTogether(
name='test',
- unique_together=set([('submission', 'name')]),
+ unique_together={('submission', 'name')},
),
migrations.AlterUniqueTogether(
name='submission',
- unique_together=set([('type', 'student')]),
+ unique_together={('type', 'student')},
),
migrations.AlterUniqueTogether(
name='generaltasksubscription',
- unique_together=set([('owner', 'query_key', 'query_type')]),
+ unique_together={('owner', 'query_key', 'query_type')},
),
]
diff --git a/core/migrations/0002_auto_20171222_1116.py b/core/migrations/0002_auto_20171222_1116.py
deleted file mode 100644
index 47abe7a7..00000000
--- a/core/migrations/0002_auto_20171222_1116.py
+++ /dev/null
@@ -1,18 +0,0 @@
-# Generated by Django 2.0 on 2017-12-22 11:16
-
-from django.db import migrations, models
-
-
-class Migration(migrations.Migration):
-
- dependencies = [
- ('core', '0001_initial'),
- ]
-
- operations = [
- migrations.AlterField(
- model_name='useraccount',
- name='last_name',
- field=models.CharField(blank=True, max_length=150, verbose_name='last name'),
- ),
- ]
diff --git a/core/migrations/0003_auto_20180104_1631.py b/core/migrations/0003_auto_20180104_1631.py
deleted file mode 100644
index f87e50dd..00000000
--- a/core/migrations/0003_auto_20180104_1631.py
+++ /dev/null
@@ -1,18 +0,0 @@
-# Generated by Django 2.0.1 on 2018-01-04 16:31
-
-from django.db import migrations
-
-
-class Migration(migrations.Migration):
-
- dependencies = [
- ('core', '0002_auto_20171222_1116'),
- ]
-
- operations = [
- migrations.RenameField(
- model_name='tutorsubmissionassignment',
- old_name='active',
- new_name='is_done',
- ),
- ]
diff --git a/core/migrations/0004_feedback_is_final.py b/core/migrations/0004_feedback_is_final.py
deleted file mode 100644
index 7aa6f9ca..00000000
--- a/core/migrations/0004_feedback_is_final.py
+++ /dev/null
@@ -1,18 +0,0 @@
-# Generated by Django 2.0.1 on 2018-01-04 16:58
-
-from django.db import migrations, models
-
-
-class Migration(migrations.Migration):
-
- dependencies = [
- ('core', '0003_auto_20180104_1631'),
- ]
-
- operations = [
- migrations.AddField(
- model_name='feedback',
- name='is_final',
- field=models.BooleanField(default=False),
- ),
- ]
diff --git a/core/migrations/0005_auto_20180104_1851.py b/core/migrations/0005_auto_20180104_1851.py
deleted file mode 100644
index 96eea1d2..00000000
--- a/core/migrations/0005_auto_20180104_1851.py
+++ /dev/null
@@ -1,26 +0,0 @@
-# Generated by Django 2.0.1 on 2018-01-04 18:51
-
-import django.db.models.deletion
-import django.utils.timezone
-from django.db import migrations, models
-
-
-class Migration(migrations.Migration):
-
- dependencies = [
- ('core', '0004_feedback_is_final'),
- ]
-
- operations = [
- migrations.AddField(
- model_name='tutorsubmissionassignment',
- name='created',
- field=models.DateTimeField(auto_now_add=True, default=django.utils.timezone.now),
- preserve_default=False,
- ),
- migrations.AlterField(
- model_name='tutorsubmissionassignment',
- name='submission',
- field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='assignments', to='core.Submission'),
- ),
- ]
diff --git a/core/migrations/0006_auto_20180104_2001.py b/core/migrations/0006_auto_20180104_2001.py
deleted file mode 100644
index f790ca9e..00000000
--- a/core/migrations/0006_auto_20180104_2001.py
+++ /dev/null
@@ -1,20 +0,0 @@
-# Generated by Django 2.0.1 on 2018-01-04 20:01
-
-import django.db.models.deletion
-from django.conf import settings
-from django.db import migrations, models
-
-
-class Migration(migrations.Migration):
-
- dependencies = [
- ('core', '0005_auto_20180104_1851'),
- ]
-
- operations = [
- migrations.AlterField(
- 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),
- ),
- ]
diff --git a/core/migrations/0007_auto_20180105_1136.py b/core/migrations/0007_auto_20180105_1136.py
deleted file mode 100644
index 2b3aa7ee..00000000
--- a/core/migrations/0007_auto_20180105_1136.py
+++ /dev/null
@@ -1,41 +0,0 @@
-# Generated by Django 2.0 on 2018-01-05 11:36
-
-from django.db import migrations, models
-
-
-class Migration(migrations.Migration):
-
- dependencies = [
- ('core', '0006_auto_20180104_2001'),
- ]
-
- operations = [
- migrations.RenameModel(
- old_name='Student',
- new_name='StudentInfo',
- ),
- migrations.RemoveField(
- model_name='reviewer',
- name='user',
- ),
- migrations.RemoveField(
- model_name='tutor',
- name='user',
- ),
- migrations.RemoveField(
- model_name='feedback',
- name='of_reviewer',
- ),
- migrations.AddField(
- model_name='useraccount',
- name='role',
- field=models.CharField(choices=[('Student', 'student'), ('Tutor', 'tutor'), ('Reviewer', 'reviewer')], default='Student', max_length=50),
- preserve_default=False,
- ),
- migrations.DeleteModel(
- name='Reviewer',
- ),
- migrations.DeleteModel(
- name='Tutor',
- ),
- ]
diff --git a/core/models.py b/core/models.py
index 40af9775..47da2b27 100644
--- a/core/models.py
+++ b/core/models.py
@@ -319,7 +319,7 @@ class Submission(models.Model):
----------
seen_by_student : BooleanField
True if the student saw his accepted feedback.
- student : OneToOneField
+ student : ForgeignKey
The student how cause all of this
text : TextField
The code/text submitted by the student
@@ -579,5 +579,48 @@ class TutorSubmissionAssignment(models.Model):
self.save()
def __str__(self):
- return (f'{self.assignee} assigned to {self.submission}'
- f' (active={self.active})')
+ return (f'{self.subscription.owner} assigned to {self.submission}'
+ f' (done={self.is_done})')
+
+
+class FeedbackForSubmissionLine(models.Model):
+
+ of_line = models.PositiveIntegerField()
+ of_feedback = models.ForeignKey(
+ Feedback,
+ related_name="feedbacklines",
+ on_delete=models.CASCADE
+ )
+
+ class Meta:
+ verbose_name = "Feedback for Submissionline"
+ verbose_name_plural = "Feedback Submissionlines"
+
+
+class FeedbackComment(models.Model):
+
+ """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)
+ of_tutor = models.ForeignKey(
+ get_user_model(),
+ related_name="comment_list",
+ on_delete=models.PROTECT
+ )
+
+ def save(self, *args, **kwargs):
+ self.of_line.comment_list.update(is_final=False)
+ super().save(*args, **kwargs)
+
+ class Meta:
+ verbose_name = "Feedback Comment"
+ verbose_name_plural = "Feedback Comments"
+ ordering = ('created',)
diff --git a/core/serializers.py b/core/serializers.py
index 83c8bf1c..778c1b90 100644
--- a/core/serializers.py
+++ b/core/serializers.py
@@ -3,6 +3,7 @@ import logging
from django.core.exceptions import ObjectDoesNotExist
from drf_dynamic_fields import DynamicFieldsMixin
from rest_framework import serializers
+from rest_framework.validators import UniqueValidator
from core import models
from core.models import (ExamType, Feedback, GeneralTaskSubscription,
@@ -27,8 +28,49 @@ class ExamSerializer(DynamicFieldsModelSerializer):
'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_id = serializers.UUIDField(write_only=True)
+ feedbacklines = FeedbackForSubmissionLineSerializer(
+ required=False
+ )
+ isFinal = serializers.BooleanField(source="is_final")
+ ofSubmission = serializers.PrimaryKeyRelatedField(
+ source='of_submission',
+ required=False,
+ queryset=Submission.objects.all(),
+ validators=[UniqueValidator(queryset=Feedback.objects.all()), ])
+
+ 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 'feedbacklines' in validated_data:
+ for line_no, comment in validated_data['feedbacklines'].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)
@@ -56,15 +98,24 @@ class FeedbackSerializer(DynamicFieldsModelSerializer):
'of_submission': submission
}
- def create(self, validated_data) -> Feedback:
- assignment = validated_data.pop('assignment')
- assignment.set_done()
+ class Meta:
+ model = Feedback
+ fields = ('assignment_id', 'isFinal', 'score',
+ 'ofSubmission', 'feedbacklines')
+
- return Feedback.objects.create(**validated_data)
+class FeedbackCommentSerializer(serializers.ModelSerializer):
+
+ ofTutor = serializers.StringRelatedField(source='of_tutor.username')
+ isFinalComment = serializers.BooleanField(source='is_final')
+
+ def to_internal_value(self, data):
+ return data
class Meta:
- model = Feedback
- fields = ('assignment_id', 'text', 'score')
+ model = models.FeedbackComment
+ fields = ('text', 'ofTutor', 'created', 'isFinalComment')
+ read_only_fields = ('created',)
class SubmissionTypeSerializer(DynamicFieldsModelSerializer):
diff --git a/core/tests/test_factory_and_feedback.py b/core/tests/test_factory.py
similarity index 100%
rename from core/tests/test_factory_and_feedback.py
rename to core/tests/test_factory.py
diff --git a/core/tests/test_feedback.py b/core/tests/test_feedback.py
new file mode 100644
index 00000000..d7e52b51
--- /dev/null
+++ b/core/tests/test_feedback.py
@@ -0,0 +1,228 @@
+from rest_framework import status
+from rest_framework.test import (APIRequestFactory, APITestCase,
+ force_authenticate)
+
+from core import models
+from core.models import (Feedback, FeedbackComment, FeedbackForSubmissionLine,
+ Submission, SubmissionType)
+from core.views import FeedbackApiView
+from util.factories import GradyUserFactory
+
+
+class FeedbackRetrieveTestCase(APITestCase):
+
+ factory = GradyUserFactory()
+ EXPECTED_SCORE = 23
+
+ @classmethod
+ def setUpTestData(cls):
+ cls.score = 23
+ cls.tutor = cls.factory.make_tutor()
+ cls.student = cls.factory.make_student()
+ cls.tutor2 = cls.factory.make_tutor()
+ cls.tutors = [cls.tutor, cls.tutor2]
+ cls.request_factory = APIRequestFactory()
+ cls.submission_type = SubmissionType.objects.create(
+ name='Cooking some crystal with Jesse')
+ cls.sub = Submission.objects.create(student=cls.student.student,
+ type=cls.submission_type)
+ cls.feedback = Feedback.objects.create(of_tutor=cls.tutor,
+ 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 tutor in cls.tutors:
+ FeedbackComment.objects.create(text='fortytwo',
+ of_tutor=tutor,
+ of_line=line)
+
+ def setUp(self):
+ 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.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]
+ self.assertEqual(1, len(final_comments))
+
+ def test_can_retrieve_feedback_via_endpoint(self):
+ self.assertEqual(self.response.status_code, status.HTTP_200_OK)
+
+ def test_if_feedback_contains_correct_score(self):
+ self.assertIn('score', self.data)
+ self.assertEqual(self.data.get('score'), self.EXPECTED_SCORE)
+
+ def test_if_feedback_contains_linekeys(self):
+ self.assertIn('feedbacklines', self.data)
+ self.assertIn(1, self.data['feedbacklines'])
+ self.assertIn(2, self.data['feedbacklines'])
+
+ def test_if_feedback_contains_final(self):
+ self.assertIn('isFinal', self.data)
+ self.assertIsNotNone(self.data['isFinal'])
+
+ def test_if_comment_contains_text(self):
+ self.assertIn('text', self.data['feedbacklines'][1][0])
+ self.assertEqual(
+ 'fortytwo', self.data['feedbacklines'][1][0]['text'])
+
+ def test_if_comment_contains_created(self):
+ self.assertIn('created', self.data['feedbacklines'][1][0])
+ self.assertIsNotNone(self.data['feedbacklines'][1][0]['created'])
+
+ def test_if_comment_has_tutor(self):
+ self.assertIn('ofTutor', self.data['feedbacklines'][1][0])
+ self.assertEqual(
+ self.tutor.username,
+ self.data['feedbacklines'][1][0]['ofTutor'])
+
+ def test_if_comment_has_final(self):
+ self.assertIn('isFinalComment', self.data['feedbacklines'][1][0])
+ self.assertIsNotNone(
+ self.data['feedbacklines'][1][0]['isFinalComment'])
+
+
+class FeedbackCreateTestCase(APITestCase):
+
+ @classmethod
+ def setUpTestData(cls):
+ cls.url = '/api/feedback/'
+ cls.user_factory = GradyUserFactory()
+ cls.tutor = cls.user_factory.make_tutor(password='p')
+ cls.student = cls.user_factory.make_student()
+ cls.submission_type = SubmissionType.objects.create(
+ name='Cooking some crystal with Jesse',
+ full_score=100
+ )
+ cls.sub = Submission.objects.create(student=cls.student.student,
+ type=cls.submission_type)
+
+ def setUp(self):
+ self.client.force_authenticate(user=self.tutor)
+ self.subscription = models.GeneralTaskSubscription.objects.create(
+ owner=self.tutor,
+ query_type='random',
+ query_key=''
+ )
+ self.assignment = self.subscription.get_or_create_work_assignment()
+
+ def test_can_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
+ data = {
+ 'score': 10,
+ 'isFinal': False,
+ 'assignment_id': self.assignment.assignment_id
+
+ }
+ 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)
+
+ def test_cannot_create_feedback_with_score_higher_than_max(self):
+ data = {
+ 'score': 101,
+ 'isFinal': False,
+ 'assignment_id': self.assignment.assignment_id
+ }
+ self.assertEqual(Feedback.objects.count(), 0)
+ response = self.client.post(self.url, data, format='json')
+ self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
+ self.assertEqual(Feedback.objects.count(), 0)
+
+ def test_cannot_create_feedback_with_score_less_than_zero(self):
+ data = {
+ 'score': -1,
+ 'isFinal': False,
+ 'assignment_id': self.assignment.assignment_id
+ }
+ self.assertEqual(Feedback.objects.count(), 0)
+ response = self.client.post(self.url, data, format='json')
+ self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
+ self.assertEqual(Feedback.objects.count(), 0)
+
+ def test_check_score_is_set_accordingly(self):
+ data = {
+ 'score': 5,
+ 'isFinal': False,
+ 'assignment_id': self.assignment.assignment_id
+ }
+ self.client.post(self.url, data, format='json')
+ object_score = self.sub.feedback.score
+ self.assertEqual(object_score, 5)
+
+ def test_can_create_feedback_with_comment(self):
+ data = {
+ 'score': 0,
+ 'isFinal': False,
+ 'assignment_id': self.assignment.assignment_id,
+ 'feedbacklines': {
+ '5': {
+ 'text': 'Nice meth!'
+ }
+ }
+ }
+ self.assertEqual(FeedbackComment.objects.count(), 0)
+ response = self.client.post(self.url, data, format='json')
+ self.assertEqual(response.status_code, status.HTTP_201_CREATED)
+ self.assertEqual(FeedbackComment.objects.count(), 1)
+
+ def test_feedback_comment_is_created_correctly(self):
+ data = {
+ 'score': 0,
+ 'isFinal': False,
+ 'assignment_id': self.assignment.assignment_id,
+ 'feedbacklines': {
+ '5': {
+ 'text': 'Nice meth!'
+ }
+ }
+ }
+ self.client.post(self.url, data, format='json')
+ comment = FeedbackComment.objects.first()
+ 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)
+
+ def test_can_create_multiple_feedback_comments(self):
+ data = {
+ 'score': 0,
+ 'isFinal': False,
+ 'assignment_id': self.assignment.assignment_id,
+ 'feedbacklines': {
+ '5': {
+ 'text': 'Nice meth!'
+ },
+ '7': {
+ 'text': 'Good one!'
+ }
+ }
+ }
+ self.client.post(self.url, data, format='json')
+ 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)
+
+ 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)
diff --git a/core/views.py b/core/views.py
index edb9840e..886d41c5 100644
--- a/core/views.py
+++ b/core/views.py
@@ -63,17 +63,14 @@ class FeedbackApiView(
permission_classes = (IsTutorOrReviewer,)
queryset = Feedback.objects.all()
serializer_class = FeedbackSerializer
- lookup_field = 'submission__submission_id'
+ lookup_field = 'of_submission__submission_id'
+ lookup_url_kwarg = 'submission_id'
def create(self, request, *args, **kwargs):
serializer = self.get_serializer(
data={**request.data, 'of_tutor': request.user})
serializer.is_valid(raise_exception=True)
- if serializer.data['assignment'].subscription.owner != request.user:
- return Response({'You do not have permission to edit this'},
- status=status.HTTP_403_FORBIDDEN)
-
self.perform_create(serializer)
headers = self.get_success_headers(serializer.data)
return Response(serializer.data,
--
GitLab