Fixed FeedbackComment visibility bug

Also removed path endpoint for this model since it wasnt used

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

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
+

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)

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()

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/')
 }