From 81ea7844f3fd4e93c1fc0fd98f93afdce52884da Mon Sep 17 00:00:00 2001
From: "robinwilliam.hundt" <robinwilliam.hundt@stud.uni-goettingen.de>
Date: Sun, 11 Mar 2018 19:29:33 +0100
Subject: [PATCH] Added VisibleCommentFeedbackSerializer
Only Comments that are `visible_to_student=True` will be serialized. For some weird reason i had to resort to a little hack in the serializer, see the comment inside the `get_feedback_lines()` method of the serializer for context.
I choose to not remove Feedback that is not final from the response of the student submissions endpoint (as outlined in #91) and will instead show a message in the frontend. This is easier to implement and potentially better for debugging in the frontend.
---
core/serializers/__init__.py | 3 ++-
core/serializers/feedback.py | 28 +++++++++++++++++++++++-
core/serializers/submission.py | 3 ++-
core/tests/test_student_page.py | 38 ++++++++++++++++++++++++++-------
4 files changed, 61 insertions(+), 11 deletions(-)
diff --git a/core/serializers/__init__.py b/core/serializers/__init__.py
index 0306ce60..1e598184 100644
--- a/core/serializers/__init__.py
+++ b/core/serializers/__init__.py
@@ -1,5 +1,6 @@
from .common_serializers import * # noqa
-from .feedback import FeedbackSerializer, FeedbackCommentSerializer # noqa
+from .feedback import (FeedbackSerializer, FeedbackCommentSerializer,
+ VisibleCommentFeedbackSerializer) # noqa
from .subscription import * # noqa
from .student import * # noqa
from .submission import * # noqa
diff --git a/core/serializers/feedback.py b/core/serializers/feedback.py
index d5e48833..3f854ebd 100644
--- a/core/serializers/feedback.py
+++ b/core/serializers/feedback.py
@@ -62,7 +62,7 @@ class FeedbackCommentDictionarySerializer(serializers.ListSerializer):
return ret
-class FeedbackCommentSerializer(serializers.ModelSerializer):
+class FeedbackCommentSerializer(DynamicFieldsModelSerializer):
of_tutor = serializers.StringRelatedField(source='of_tutor.username')
class Meta:
@@ -179,3 +179,29 @@ class FeedbackSerializer(DynamicFieldsModelSerializer):
model = Feedback
fields = ('pk', 'of_submission', 'is_final', 'score', 'feedback_lines',
'created', 'of_submission_type', 'feedback_stage_for_user')
+
+
+class VisibleCommentFeedbackSerializer(FeedbackSerializer):
+ feedback_lines = serializers.SerializerMethodField()
+ of_submission_type = serializers.ReadOnlyField(
+ source='of_submission.type.pk')
+
+ def get_feedback_lines(self, feedback):
+ comments = feedback.feedback_lines.filter(visible_to_student=True)
+ serializer = FeedbackCommentSerializer(
+ comments,
+ many=True,
+ fields=('pk', 'text', 'created', 'of_line',)
+ )
+ # this is a weird hack because, for some reason, serializer.data
+ # just won't contain the correct data. Instead .data returns a list
+ # containing just the `of_line` attr of the serialized comments
+ # after long debugging i found that for inexplicable reasons
+ # `data.serializer._data` contains the correct data. No clue why.
+ return serializer.data.serializer._data
+
+ class Meta:
+ model = Feedback
+ fields = ('pk', 'of_submission', 'is_final', 'score', 'feedback_lines',
+ 'created', 'of_submission_type')
+
diff --git a/core/serializers/submission.py b/core/serializers/submission.py
index 8286e58b..8e0b1d08 100644
--- a/core/serializers/submission.py
+++ b/core/serializers/submission.py
@@ -2,6 +2,7 @@ from rest_framework import serializers
from core.models import Submission
from core.serializers import (DynamicFieldsModelSerializer, FeedbackSerializer,
+ VisibleCommentFeedbackSerializer,
SubmissionTypeListSerializer,
SubmissionTypeSerializer, TestSerializer)
@@ -18,7 +19,7 @@ class SubmissionNoTextFieldsSerializer(DynamicFieldsModelSerializer):
class SubmissionSerializer(DynamicFieldsModelSerializer):
type = SubmissionTypeSerializer()
- feedback = FeedbackSerializer()
+ feedback = VisibleCommentFeedbackSerializer()
tests = TestSerializer(many=True)
class Meta:
diff --git a/core/tests/test_student_page.py b/core/tests/test_student_page.py
index c8a1365b..fa57aea9 100644
--- a/core/tests/test_student_page.py
+++ b/core/tests/test_student_page.py
@@ -139,9 +139,14 @@ class StudentSelfSubmissionsTests(APITestCase):
'students': [{
'username': 'user01',
}],
- 'tutors': [{
- 'username': 'tutor01'
- }],
+ 'tutors': [
+ {
+ 'username': 'tutor01'
+ },
+ {
+ 'username': 'tutor02'
+ }
+ ],
'submissions': [{
'user': 'user01',
'type': 'problem01',
@@ -150,10 +155,19 @@ class StudentSelfSubmissionsTests(APITestCase):
'text': 'Very bad!',
'score': 3,
'feedback_lines': {
- '1': [{
- 'text': 'This is very bad!',
- 'of_tutor': 'tutor01'
- }],
+ '1': [
+ {
+ 'text': 'This is very bad!',
+ 'of_tutor': 'tutor01',
+ # explicitness to required
+ # will also be set automatically
+ 'visible_to_student': False
+ },
+ {
+ 'text': 'This is good!',
+ 'of_tutor': 'tutor02'
+ }
+ ],
}
}
}]
@@ -210,12 +224,20 @@ class StudentSelfSubmissionsTests(APITestCase):
self.submission_list_first_entry['feedback']['score'],
self.student_info.submissions.first().feedback.score)
- def submssion_feedback_contains_submission_lines(self):
+ def test_submission_feedback_contains_submission_lines(self):
self.assertIn(
'feedback_lines',
self.submission_list_first_entry['feedback']
)
+ def test_feedback_contains_one_comment_per_line(self):
+ lines = self.submission_list_first_entry['feedback']['feedback_lines']
+ self.assertEqual(len(lines[1]), 1)
+
+ def test_feedback_comment_does_not_contain_tutor(self):
+ lines = self.submission_list_first_entry['feedback']['feedback_lines']
+ self.assertNotIn('of_tutor', lines[1][0])
+
# We don't want a matriculation number here
def test_matriculation_number_is_not_send(self):
self.assertNotIn('matrikel_no', self.submission_list_first_entry)
--
GitLab