diff --git a/core/migrations/0014_feedbacklabel.py b/core/migrations/0014_feedbacklabel.py new file mode 100644 index 0000000000000000000000000000000000000000..14b9d385a957821ca66f102d1d51f87e11a78379 --- /dev/null +++ b/core/migrations/0014_feedbacklabel.py @@ -0,0 +1,23 @@ +# Generated by Django 2.1.4 on 2019-04-25 15:28 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('core', '0013_auto_20190308_1448'), + ] + + operations = [ + migrations.CreateModel( + name='FeedbackLabel', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('name', models.CharField(max_length=50)), + ('description', models.TextField()), + ('feedback', models.ManyToManyField(related_name='labels', to='core.Feedback')), + ('feedback_comments', models.ManyToManyField(related_name='labels', to='core.FeedbackComment')), + ], + ), + ] diff --git a/core/models/__init__.py b/core/models/__init__.py index 36b8767e32f067b20940f8e2ed04938681652d75..cb9f27cfed799b0ba652d7f69a23601a6f5681de 100644 --- a/core/models/__init__.py +++ b/core/models/__init__.py @@ -8,4 +8,4 @@ from .feedback import Feedback, FeedbackComment # noqa from .subscription import (NotMoreThanTwoOpenAssignmentsAllowed, SubmissionSubscription, # noqa SubscriptionTemporarilyEnded, SubscriptionEnded) # noqa from .assignment import DeletionOfDoneAssignmentsNotPermitted, TutorSubmissionAssignment # noqa -from .label import Label +from .label import FeedbackLabel # noqa diff --git a/core/models/exam_type.py b/core/models/exam_type.py index 9036fbbacd2aa189c3f1c75cc9f699538b4646c8..bf9ab0a79d752fb9d59597510c37fe9ddc5141d5 100644 --- a/core/models/exam_type.py +++ b/core/models/exam_type.py @@ -4,7 +4,6 @@ import uuid import constance from django.db import models -from django.db.models import BooleanField log = logging.getLogger(__name__) config = constance.config diff --git a/core/models/feedback.py b/core/models/feedback.py index 45e070637a0c9185e4c6bd0594857b230837b30a..fb933be3e84b643c86b32539443c7c9329762edd 100644 --- a/core/models/feedback.py +++ b/core/models/feedback.py @@ -4,7 +4,6 @@ import uuid import constance from django.contrib.auth import get_user_model from django.db import models -from django.db.models import (IntegerField,) from core.models.submission import Submission diff --git a/core/models/label.py b/core/models/label.py index 5e83a32fb043e938261368d0939a9902b4b135c2..3ee53eb0ef3610b97cbefe70d10acd8ca9b28ec4 100644 --- a/core/models/label.py +++ b/core/models/label.py @@ -7,10 +7,8 @@ from core.models.feedback import Feedback, FeedbackComment log = logging.getLogger(__name__) -class Label(models.Model): +class FeedbackLabel(models.Model): name = models.CharField(max_length=50) description = models.TextField() feedback = models.ManyToManyField(Feedback, related_name='labels') feedback_comments = models.ManyToManyField(FeedbackComment, related_name='labels') - - diff --git a/core/models/submission.py b/core/models/submission.py index a2169d08597b8b06a29030e35f7c294209d447ee..3687fb679af0458ee21f0e8d9d2575ba40c6ff82 100644 --- a/core/models/submission.py +++ b/core/models/submission.py @@ -4,7 +4,6 @@ import uuid import constance from django.contrib.auth import get_user_model from django.db import models -from django.db.models import (BooleanField) from core.models.submission_type import SubmissionType diff --git a/core/permissions.py b/core/permissions.py index e52f68a21432ab2e0c6369dac0487ea8376a60fa..04385e84d7dc6d2ea9485db681df89af7bb9009b 100644 --- a/core/permissions.py +++ b/core/permissions.py @@ -24,8 +24,8 @@ class IsUserRoleGenericPermission(permissions.BasePermission): user.role in self.roles) if not is_authorized: - log.warn('User "%s" has no permission to view %s', - user.username, view.__class__.__name__) + log.warning('User "%s" has no permission to view %s', + user.username, view.__class__.__name__) return is_authorized diff --git a/core/serializers/__init__.py b/core/serializers/__init__.py index 9f6fce12b55a083e2eaec48ad1219413b6877052..2edb1d054fe8d2c63d7440519d4c07b11f2dffd4 100644 --- a/core/serializers/__init__.py +++ b/core/serializers/__init__.py @@ -5,3 +5,4 @@ from .subscription import * # noqa from .student import * # noqa from .submission import * # noqa from .tutor import TutorSerializer # noqa +from .label import LabelSerializer # noqa diff --git a/core/serializers/feedback.py b/core/serializers/feedback.py index 808fa07d1892304e136913daec1bc40ffebd3e6f..e5095da56b76ca01eec746ed30048a8b2eb7c7b0 100644 --- a/core/serializers/feedback.py +++ b/core/serializers/feedback.py @@ -72,6 +72,7 @@ class FeedbackCommentSerializer(DynamicFieldsModelSerializer): 'created', 'of_tutor', 'of_line', + 'labels', 'visible_to_student') read_only_fields = ('created', 'of_tutor') extra_kwargs = { @@ -86,6 +87,8 @@ class FeedbackSerializer(DynamicFieldsModelSerializer): of_submission_type = serializers.ReadOnlyField( source='of_submission.type.pk') feedback_stage_for_user = serializers.SerializerMethodField() + labels = serializers.PrimaryKeyRelatedField(many=True, required=False, + queryset=models.FeedbackLabel.objects.all()) def get_feedback_stage_for_user(self, obj): """ Search for the assignment of this feedback and report in which @@ -114,28 +117,35 @@ class FeedbackSerializer(DynamicFieldsModelSerializer): def create(self, validated_data) -> Feedback: submission = validated_data.pop('of_submission') feedback_lines = validated_data.pop('feedback_lines', []) - + labels = validated_data.pop('labels', []) feedback = Feedback.objects.create(of_submission=submission, **validated_data) + for label in labels: + feedback.labels.add(label) + submission.meta.feedback_authors.add(self.context['request'].user) for comment in feedback_lines: - models.FeedbackComment.objects.create( + labels = comment.pop('labels', []) + comment_instance = models.FeedbackComment.objects.create( of_feedback=feedback, of_tutor=self.context['request'].user, **comment ) + comment_instance.labels.set(labels) return Feedback.objects.get(of_submission=submission) @transaction.atomic def update(self, feedback, validated_data): for comment in validated_data.pop('feedback_lines', []): - models.FeedbackComment.objects.update_or_create( + labels = comment.pop('labels', []) + comment_instance, _ = models.FeedbackComment.objects.update_or_create( of_feedback=feedback, of_tutor=self.context['request'].user, of_line=comment.get('of_line'), defaults={'text': comment.get('text')}) + comment_instance.labels.set(labels) return super().update(feedback, validated_data) @@ -196,7 +206,7 @@ class FeedbackSerializer(DynamicFieldsModelSerializer): class Meta: model = Feedback fields = ('pk', 'of_submission', 'is_final', 'score', 'feedback_lines', - 'created', 'of_submission_type', 'feedback_stage_for_user') + 'created', 'of_submission_type', 'feedback_stage_for_user', 'labels') class VisibleCommentFeedbackSerializer(FeedbackSerializer): diff --git a/core/serializers/label.py b/core/serializers/label.py index 36bdb84861d9d8ce2e00c050289ffd48311d88ca..3d358c8a6f4ba37d0c315fc48b59a65c1d256e19 100644 --- a/core/serializers/label.py +++ b/core/serializers/label.py @@ -1,11 +1,11 @@ from rest_framework import serializers -from core.models import Label +from core.models import FeedbackLabel class LabelSerializer(serializers.ModelSerializer): class Meta: - model = Label + model = FeedbackLabel fields = ( 'pk', 'name', diff --git a/core/tests/test_feedback.py b/core/tests/test_feedback.py index da2efe5582d822041d000b4a07025f3378ae3c35..def5acb1ae72f03bb12654dcd3e50493a92493f3 100644 --- a/core/tests/test_feedback.py +++ b/core/tests/test_feedback.py @@ -4,7 +4,7 @@ from rest_framework import status from rest_framework.test import APIRequestFactory, APITestCase from core import models -from core.models import Feedback, FeedbackComment, Submission, SubmissionType +from core.models import Feedback, FeedbackComment, Submission, SubmissionType, FeedbackLabel from util.factories import GradyUserFactory, make_test_data, make_exams @@ -113,9 +113,13 @@ class FeedbackCreateTestCase(APITestCase): cls.sub = Submission.objects.create(student=cls.student.student, type=cls.submission_type, text=text) + cls.fst_label = FeedbackLabel.objects.create(name='Label1', description='Bla') + cls.snd_label = FeedbackLabel.objects.create(name='Label2', description='Bla') def setUp(self): self.sub.refresh_from_db() + self.fst_label.refresh_from_db() + self.snd_label.refresh_from_db() self.client.force_authenticate(user=self.tutor) self.subscription = models.SubmissionSubscription.objects.create( owner=self.tutor, @@ -191,6 +195,28 @@ class FeedbackCreateTestCase(APITestCase): self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) self.assertEqual(Feedback.objects.count(), 0) + def test_can_create_with_labels(self): + data = { + 'score': 0, + 'is_final': False, + 'of_submission': self.assignment.submission.pk, + 'labels': [self.fst_label.pk, self.snd_label.pk], + 'feedback_lines': { + '2': { + 'text': 'Why you no learn how to code, man?', + 'labels': [] + } + } + } + self.assertEqual(self.fst_label.feedback.count(), 0) + response = self.client.post(self.url, data, format='json') + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + self.fst_label.refresh_from_db() + self.snd_label.refresh_from_db() + self.assertEqual(self.fst_label.feedback.count(), 1) + self.assertEqual(self.snd_label.feedback.count(), 1) + self.assertEqual(Feedback.objects.first().labels.count(), 2) + def test_can_create_feedback_with_half_points(self): data = { 'score': 0.5, @@ -198,7 +224,8 @@ class FeedbackCreateTestCase(APITestCase): 'of_submission': self.assignment.submission.pk, 'feedback_lines': { '2': { - 'text': 'Why you no learn how to code, man?' + 'text': 'Why you no learn how to code, man?', + 'labels': [] } } } @@ -213,7 +240,8 @@ class FeedbackCreateTestCase(APITestCase): 'of_submission': self.assignment.submission.pk, 'feedback_lines': { '4': { - 'text': 'Why you no learn how to code, man?' + 'text': 'Why you no learn how to code, man?', + 'labels': [] } } } @@ -228,7 +256,8 @@ class FeedbackCreateTestCase(APITestCase): 'of_submission': self.assignment.submission.pk, 'feedback_lines': { '3': { - 'text': 'Nice meth!' + 'text': 'Nice meth!', + 'labels': [] } } } @@ -244,7 +273,8 @@ class FeedbackCreateTestCase(APITestCase): 'of_submission': self.assignment.submission.pk, 'feedback_lines': { '3': { - 'text': 'Nice meth!' + 'text': 'Nice meth!', + 'labels': [] } } } @@ -262,7 +292,8 @@ class FeedbackCreateTestCase(APITestCase): 'of_submission': self.assignment.submission.pk, 'feedback_lines': { '2': { - 'text': 'Well, at least you tried.' + 'text': 'Well, at least you tried.', + 'labels': [] }, } } @@ -276,7 +307,8 @@ class FeedbackCreateTestCase(APITestCase): 'of_submission': self.assignment.submission.pk, 'feedback_lines': { '1': { - 'text': 'Well, at least you tried.' + 'text': 'Well, at least you tried.', + 'labels': [] }, } } @@ -292,10 +324,12 @@ class FeedbackCreateTestCase(APITestCase): 'of_submission': self.assignment.submission.pk, 'feedback_lines': { '1': { - 'text': 'Nice meth!' + 'text': 'Nice meth!', + 'labels': [] }, '3': { - 'text': 'Good one!' + 'text': 'Good one!', + 'labels': [] } } } @@ -353,6 +387,9 @@ class FeedbackPatchTestCase(APITestCase): }] }) + cls.fst_label = FeedbackLabel.objects.create(name='Label1', description='Bla') + cls.snd_label = FeedbackLabel.objects.create(name='Label2', description='Bla') + def setUp(self): self.tutor01 = self.data['tutors'][0] self.tutor02 = self.data['tutors'][1] @@ -367,7 +404,10 @@ class FeedbackPatchTestCase(APITestCase): 'is_final': False, 'of_submission': self.assignment.submission.pk, 'feedback_lines': { - '2': {'text': 'Very good.'}, + '2': { + 'text': 'Very good.', + 'labels': [] + }, } } response = self.client.post(self.burl, data, format='json') @@ -375,10 +415,16 @@ class FeedbackPatchTestCase(APITestCase): of_submission=response.data['of_submission']) self.url = f'{self.burl}{self.feedback.of_submission.submission_id}/' + self.fst_label.refresh_from_db() + self.snd_label.refresh_from_db() + def test_can_patch_onto_the_own_feedback(self): data = { 'feedback_lines': { - '1': {'text': 'Spam spam spam'}, + '1': { + 'text': 'Spam spam spam', + 'labels': [] + }, } } response = self.client.patch(self.url, data, format='json') @@ -395,7 +441,10 @@ class FeedbackPatchTestCase(APITestCase): def test_can_update_a_single_line(self): data = { 'feedback_lines': { - '2': {'text': 'Turns out this is rather bad.'}, + '2': { + 'text': 'Turns out this is rather bad.', + 'labels': [] + }, } } @@ -415,7 +464,7 @@ class FeedbackPatchTestCase(APITestCase): # Step 2 - Tutor 1 tries to patch data = { 'feedback_lines': { - '2': {'text': 'Turns out this is rather bad.'}, + '2': {'text': 'Turns out this is rather bad.', 'labels': []}, } } @@ -425,7 +474,7 @@ class FeedbackPatchTestCase(APITestCase): def test_cannot_patch_first_feedback_final(self): data = { 'feedback_lines': { - '2': {'text': 'Turns out this is rather bad.'}, + '2': {'text': 'Turns out this is rather bad.', 'labels': []}, }, 'is_final': True } @@ -433,6 +482,21 @@ class FeedbackPatchTestCase(APITestCase): response = self.client.patch(self.url, data, format='json') self.assertEqual(status.HTTP_403_FORBIDDEN, response.status_code) + def tutor_can_patch_labels(self): + data = { + 'feedback_lines': { + '2': { + 'text': 'Turns out this is rather bad.', + 'labels': [self.fst_label.pk, self.snd_label.pk] + }, + } + } + + self.assertEqual(FeedbackComment.objects.first().labels.count(), 0) + response = self.client.patch(self.url, data, format='json') + self.assertEqual(status.HTTP_200_OK, response.status_code) + self.assertEqual(FeedbackComment.objects.first().labels.count(), 2) + class FeedbackCommentApiEndpointTest(APITestCase): diff --git a/core/tests/test_labels.py b/core/tests/test_labels.py new file mode 100644 index 0000000000000000000000000000000000000000..8af7ba2201de60651691671a0b51f3b62125f475 --- /dev/null +++ b/core/tests/test_labels.py @@ -0,0 +1,48 @@ +from rest_framework import status +from rest_framework.test import APITestCase + +from core.models import FeedbackLabel +from util.factories import GradyUserFactory, make_exams + + +class LabelsTestCases(APITestCase): + @classmethod + def setUpTestData(cls) -> None: + cls.factory = GradyUserFactory() + cls.exam = make_exams(exams=[{ + 'module_reference': 'Test Exam 01', + 'total_score': 100, + 'pass_score': 60, + }])[0] + cls.student = cls.factory.make_student(exam=cls.exam) + cls.tutor = cls.factory.make_tutor() + cls.reviewer = cls.factory.make_reviewer() + cls.label_post_data = { + 'name': 'A label', + 'description': 'with a description...' + } + cls.label_url = '/api/label/' + + def test_student_can_not_read_labels(self): + self.client.force_authenticate(user=self.student) + response = self.client.get(self.label_url) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + self.assertEqual(FeedbackLabel.objects.count(), 0) + + def test_student_can_not_write_labels(self): + self.client.force_authenticate(user=self.student) + response = self.client.post(self.label_url, data=self.label_post_data) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + self.assertEqual(FeedbackLabel.objects.count(), 0) + + def test_tutor_can_create_label(self): + self.client.force_authenticate(user=self.tutor) + response = self.client.post(self.label_url, data=self.label_post_data) + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + self.assertEqual(FeedbackLabel.objects.count(), 1) + + def test_reviewer_can_create_label(self): + self.client.force_authenticate(user=self.reviewer) + response = self.client.post(self.label_url, data=self.label_post_data) + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + self.assertEqual(FeedbackLabel.objects.count(), 1) diff --git a/core/urls.py b/core/urls.py index 18efbbd135b346226faa7cd55d4ee190c2e1bd81..ecdca6469797922d942ac56662e7cae62901f6c8 100644 --- a/core/urls.py +++ b/core/urls.py @@ -22,6 +22,7 @@ router.register('subscription', views.SubscriptionApiViewSet, router.register('assignment', views.AssignmentApiViewSet) router.register('statistics', views.StatisticsEndpoint, basename='statistics') router.register('user', views.UserAccountViewSet, basename='user') +router.register('label', views.LabelApiViewSet, basename='label') schema_view = get_schema_view( openapi.Info( diff --git a/core/views/__init__.py b/core/views/__init__.py index 04a6d8386d6023b90861ebbe9aafdfe34580c1f4..96c0465ab8f4b28b174100e09cbed5c0a8a43ab2 100644 --- a/core/views/__init__.py +++ b/core/views/__init__.py @@ -1,4 +1,5 @@ from .feedback import FeedbackApiView, FeedbackCommentApiView # noqa from .subscription import SubscriptionApiViewSet, AssignmentApiViewSet # noqa from .common_views import * # noqa -from .export import StudentJSONExport, InstanceExport # noqa +from .export import StudentJSONExport, InstanceExport # noqa +from .label import LabelApiViewSet # noqa diff --git a/core/views/label.py b/core/views/label.py new file mode 100644 index 0000000000000000000000000000000000000000..c1b4a02a6266a4056d6234bf3b9997b5afbe98e5 --- /dev/null +++ b/core/views/label.py @@ -0,0 +1,16 @@ +import logging + +from rest_framework import mixins, viewsets + +from core import models, permissions, serializers + +log = logging.getLogger(__name__) + + +class LabelApiViewSet(viewsets.GenericViewSet, + mixins.CreateModelMixin, + mixins.UpdateModelMixin, + mixins.ListModelMixin): + permission_classes = (permissions.IsTutorOrReviewer, ) + queryset = models.FeedbackLabel.objects.all() + serializer_class = serializers.LabelSerializer