From 9b6052fc3e4f77093dfe33f03ea7552efd776093 Mon Sep 17 00:00:00 2001
From: "robinwilliam.hundt" <robinwilliam.hundt@stud.uni-goettingen.de>
Date: Tue, 4 Jun 2019 16:58:02 +0200
Subject: [PATCH] Fixed FeedbackComment visibility bug
Also removed path endpoint for this model since it wasnt used
---
core/serializers/feedback.py | 7 +++++--
core/signals.py | 2 ++
core/tests/test_feedback.py | 16 ----------------
core/views/feedback.py | 11 -----------
frontend/src/api.ts | 5 -----
5 files changed, 7 insertions(+), 34 deletions(-)
diff --git a/core/serializers/feedback.py b/core/serializers/feedback.py
index e5095da5..92416e1e 100644
--- a/core/serializers/feedback.py
+++ b/core/serializers/feedback.py
@@ -70,11 +70,14 @@ class FeedbackCommentSerializer(DynamicFieldsModelSerializer):
fields = ('pk',
'text',
'created',
+ 'modified',
'of_tutor',
'of_line',
'labels',
'visible_to_student')
- read_only_fields = ('created', 'of_tutor')
+ # visible_to_student is kept in sync with modified, such that the latest modified
+ # comment is the one that is visible
+ read_only_fields = ('created', 'of_tutor', 'visible_to_student')
extra_kwargs = {
'of_feedback': {'write_only': True},
'of_line': {'write_only': True},
@@ -219,7 +222,7 @@ class VisibleCommentFeedbackSerializer(FeedbackSerializer):
serializer = FeedbackCommentSerializer(
comments,
many=True,
- fields=('pk', 'text', 'created', 'of_line',)
+ fields=('pk', 'text', 'created', 'modified', '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
diff --git a/core/signals.py b/core/signals.py
index d5d32b8f..342532e7 100644
--- a/core/signals.py
+++ b/core/signals.py
@@ -80,3 +80,5 @@ def set_comment_visibility_after_conflict(sender, instance, **kwargs):
of_feedback=instance.of_feedback,
)
comments_on_the_same_line.update(visible_to_student=False)
+ instance.visible_to_student = True
+
diff --git a/core/tests/test_feedback.py b/core/tests/test_feedback.py
index def5acb1..dea21a72 100644
--- a/core/tests/test_feedback.py
+++ b/core/tests/test_feedback.py
@@ -585,19 +585,3 @@ class FeedbackCommentApiEndpointTest(APITestCase):
pass
else:
self.fail('No exception raised')
-
- def test_reviewer_can_set_comment_visibility(self):
- reviewer = self.data['reviewers'][0]
- self.client.force_authenticate(user=reviewer)
- comment = FeedbackComment.objects.get(of_tutor=self.tutor01)
- self.assertTrue(comment.visible_to_student)
-
- data = {
- 'visible_to_student': False
- }
-
- response = self.client.patch(self.url % comment.pk, data)
-
- self.assertFalse(response.data['visible_to_student'])
- comment.refresh_from_db()
- self.assertFalse(comment.visible_to_student)
diff --git a/core/views/feedback.py b/core/views/feedback.py
index f6330096..27211eb7 100644
--- a/core/views/feedback.py
+++ b/core/views/feedback.py
@@ -134,17 +134,6 @@ class FeedbackCommentApiView(
return self.queryset
return self.queryset.filter(of_tutor=user)
- def partial_update(self, request, **kwargs):
- keys = self.request.data.keys()
- if keys - {'visible_to_student', 'of_line', 'text'}:
- raise PermissionDenied('These fields cannot be changed.')
-
- comment = self.get_object()
- serializer = self.get_serializer(comment, request.data, partial=True)
- serializer.is_valid()
- serializer.save()
- return Response(serializer.data)
-
def destroy(self, request, *args, **kwargs):
with Lock():
instance = self.get_object()
diff --git a/frontend/src/api.ts b/frontend/src/api.ts
index e34d47a3..57bd8ded 100644
--- a/frontend/src/api.ts
+++ b/frontend/src/api.ts
@@ -189,11 +189,6 @@ export async function deleteComment (comment: FeedbackComment): Promise<AxiosRes
return ax.delete(url)
}
-export async function patchComment (comment: FeedbackComment): Promise<FeedbackComment> {
- const url = `/api/feedback-comment/${comment.pk}/`
- return (await ax.patch(url, comment)).data
-}
-
export async function activateAllStudentAccess (): Promise<AxiosResponse<void>> {
return ax.post('/api/student/activate/')
}
--
GitLab