Fixed the update method in FeedbackSerializer

* Also made minor modifications to assignment endpoint

core/serializers/feedback.py

@@ -83,11 +83,14 @@ class FeedbackCommentSerializer(serializers.ModelSerializer):
 class FeedbackSerializer(DynamicFieldsModelSerializer):
     feedback_lines = FeedbackCommentSerializer(many=True, required=False)
 
+    def _set_assignment_done(self, submission):
+        assignment = submission.assignments.get(
+            subscription__owner=self.context['request'].user)
+        assignment.set_done()
+
     @transaction.atomic
     def create(self, validated_data) -> Feedback:
         submission = validated_data.pop('of_submission')
-        assignment = submission.assignments.get(
-            subscription__owner=self.context['request'].user)
         feedback_lines = validated_data.pop('feedback_lines', [])
 
         feedback = Feedback.objects.create(of_submission=submission,
@@ -100,19 +103,20 @@ class FeedbackSerializer(DynamicFieldsModelSerializer):
                 **comment
             )
 
-        assignment.set_done()
+        self._set_assignment_done(submission)
         return Feedback.objects.get(of_submission=submission)
 
     @transaction.atomic
-    def update(self, instance, validated_data):
+    def update(self, feedback, validated_data):
         for comment in validated_data.pop('feedback_lines', []):
             models.FeedbackComment.objects.update_or_create(
-                of_feedback=instance,
+                of_feedback=feedback,
                 of_tutor=self.context['request'].user,
                 of_line=comment.get('of_line'),
                 defaults={'text': comment.get('text')})
 
-        return super().update(instance, validated_data)
+        self._set_assignment_done(feedback.of_submission)
+        return super().update(feedback, validated_data)
 
     def validate_of_submission(self, submission):
         feedback = self.instance

core/serializers/subscription.py

@@ -19,8 +19,18 @@ class AssignmentSerializer(DynamicFieldsModelSerializer):
 
     class Meta:
         model = TutorSubmissionAssignment
-        fields = ('pk', 'submission', 'is_done', 'subscription',)
+        fields = ('pk', 'submission', 'is_done')
         read_only_fields = ('is_done', 'submission')
+
+
+class AssignmentDetailSerializer(AssignmentSerializer):
+    feedback = FeedbackSerializer(source='submission.feedback', read_only=True)
+    submission = SubmissionAssignmentSerializer(read_only=True)
+
+    class Meta:
+        model = TutorSubmissionAssignment
+        fields = ('pk', 'submission', 'feedback', 'is_done', 'subscription')
+        read_only_fields = ('is_done', 'submission', 'feedback')
         extra_kwargs = {
             'subscription': {'write_only': True},
         }
@@ -30,15 +40,6 @@ class AssignmentSerializer(DynamicFieldsModelSerializer):
         return subscription.get_or_create_work_assignment()
 
 
-class AssignmentDetailSerializer(AssignmentSerializer):
-    feedback = FeedbackSerializer(source='submission.feedback')
-    submission = SubmissionAssignmentSerializer()
-
-    class Meta:
-        model = TutorSubmissionAssignment
-        fields = ('pk', 'submission', 'feedback', 'is_done',)
-
-
 class SubscriptionSerializer(DynamicFieldsModelSerializer):
     owner = serializers.ReadOnlyField(source='owner.username')
     query_key = serializers.UUIDField(required=False)

core/tests/test_subscription_assignment_service.py

@@ -282,7 +282,6 @@ class TestApiEndpoints(APITestCase):
                 }
             }
         )
-        print(response, response.data)
         self.assertEqual(status.HTTP_201_CREATED, response.status_code)
 
         # some other tutor reviews it
@@ -298,16 +297,33 @@ class TestApiEndpoints(APITestCase):
 
         subscription_pk = response.data['pk']
         response = client.post(
-            f'/api/assignment/', {
+            '/api/assignment/', {
                 'subscription': subscription_pk
             })
 
         self.assertEqual(status.HTTP_201_CREATED, response.status_code)
         submission_id_in_database = models.Feedback.objects.filter(
             is_final=False).first().of_submission.submission_id
-        submission_id_in_response = \
-            response.data['submission']['pk']
+        submission_id_in_response = response.data['submission']['pk']
 
         self.assertEqual(
             str(submission_id_in_database),
             submission_id_in_response)
+
+        assignment = models.TutorSubmissionAssignment.objects.get(
+            pk=response.data['pk'])
+        self.assertFalse(assignment.is_done)
+        response = client.patch(
+            '/api/feedback/%s/' % submission_id_in_response, {
+                "score": 20,
+                "is_final": True,
+                "feedback_lines": {
+                    2: {"text": "< some addition by second tutor>"},
+                }
+            }
+        )
+
+        assignment.refresh_from_db()
+        self.assertEqual(status.HTTP_200_OK, response.status_code)
+        self.assertEqual(2, len(response.data['feedback_lines'][2]))
+        self.assertTrue(assignment.is_done)

core/views/subscription.py

@@ -52,7 +52,7 @@ class SubscriptionApiViewSet(
         serializer.is_valid(raise_exception=True)
         subscription = serializer.save()
 
-        if subscription.query_type == models.SubmissionSubscription.STUDENT_QUERY:  # noqa TODO refactor name of generaltasksubscription to SubmissionSubscription
+        if subscription.query_type == models.SubmissionSubscription.STUDENT_QUERY:  # noqa
             subscription.reserve_all_assignments_for_a_student()
 
         headers = self.get_success_headers(serializer.data)
@@ -81,7 +81,6 @@ class AssignmentApiViewSet(
         except models.NotMoreThanTwoOpenAssignmentsAllowed as err:
             return Response({'Error': str(err)},
                             status=status.HTTP_403_FORBIDDEN)
-        serializer = AssignmentDetailSerializer(serializer.instance)
         return Response(serializer.data, status=status.HTTP_201_CREATED)
 
     def get_queryset(self):
@@ -100,6 +99,13 @@ class AssignmentApiViewSet(
         return Response(status=status.HTTP_204_NO_CONTENT)
 
     def create(self, request, *args, **kwargs):
-        serializer = self.get_serializer(data=request.data)
+        context = self.get_serializer_context()
+        serializer = AssignmentDetailSerializer(data=request.data,
+                                                context=context)
         serializer.is_valid(raise_exception=True)
         return self._fetch_assignment(serializer)
+
+    def retrieve(self, request, *args, **kwargs):
+        assignment = self.get_object()
+        serializer = AssignmentDetailSerializer(assignment)
+        return Response(serializer.data)