.gitignore

@@ -38,6 +38,7 @@ coverage_html/
 anon-export/
 public/
 geckodriver.log
+.screenshots
 
 # node
 node_modules

.gitlab-ci.yml

@@ -25,7 +25,7 @@ build_test_env:
   artifacts:
     paths:
       - .venv/
-    expire_in: 20 minutes
+    expire_in: 1 days
   cache:
     key: "$CI_JOB_NAME"
     paths:
@@ -44,7 +44,7 @@ build_frontend:
     paths:
       - frontend/dist
       - frontend/node_modules/
-    expire_in: 20 minutes
+    expire_in: 1 days
   cache:
     key: "$CI_JOB_NAME"
     paths:
@@ -106,6 +106,11 @@ test_frontend:
     - python util/format_index.py
     - python manage.py collectstatic --no-input
     - HEADLESS_TESTS=True pytest --ds=grady.settings.test functional_tests
+  artifacts:
+    paths:
+      - functional_tests/screenshots/
+    when: on_failure
+    expire_in: 30 days
 
 test_frontend_unit:
   image: node:carbon

Makefile

@@ -28,13 +28,13 @@ install:
 	pip install -Ur requirements.dev.txt
 
 test:
-	DJANGO_SETTINGS_MODULE=grady.settings pytest
+	pytest --ds=grady.settings core/tests
 
 teste2e:
-	cd frontend && yarn build && cp dist/index.html ../core/templates && cd .. && python util/format_index.py && python manage.py collectstatic --no-input && HEADLESS_TESTS=$(headless) pytest --ds=grady.settings $(path); git checkout core/templates/index.html
+	cd frontend && yarn build && cp dist/index.html ../core/templates && cd .. && python util/format_index.py && python manage.py collectstatic --no-input && HEADLESS_TESTS=$(headless) pytest  --ds=grady.settings $(path); git checkout core/templates/index.html
 
 teste2e-nc:
-	cp frontend/dist/index.html ./core/templates && python util/format_index.py && python manage.py collectstatic --no-input && HEADLESS_TESTS=$(headless) pytest --ds=grady.settings $(path); git checkout core/templates/index.html
+	cp frontend/dist/index.html ./core/templates && python util/format_index.py && python manage.py collectstatic --no-input && HEADLESS_TESTS=$(headless) pytest -n 4 --ds=grady.settings $(path); git checkout core/templates/index.html
 
 
 coverage:

core/migrations/0018_auto_20190709_1526.py

+# Generated by Django 2.2 on 2019-07-09 15:26
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('core', '0017_auto_20190604_1631'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='feedback',
+            name='final_by_reviewer',
+            field=models.BooleanField(default=False),
+        ),
+        migrations.AlterField(
+            model_name='feedbacklabel',
+            name='name',
+            field=models.CharField(max_length=50, unique=True),
+        ),
+    ]

core/migrations/0018_auto_20190814_1324.py

+# Generated by Django 2.1.11 on 2019-08-14 13:24
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('core', '0017_auto_20190604_1631'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='feedbacklabel',
+            name='name',
+            field=models.CharField(max_length=50, unique=True),
+        ),
+    ]

core/migrations/0019_merge_20190814_1437.py

+# Generated by Django 2.1.11 on 2019-08-14 14:37
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('core', '0018_auto_20190709_1526'),
+        ('core', '0018_auto_20190814_1324'),
+    ]
+
+    operations = [
+    ]

core/migrations/0020_auto_20190831_1417.py

+# Generated by Django 2.1.11 on 2019-08-31 14:17
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('core', '0019_merge_20190814_1437'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='submissiontype',
+            name='programming_language',
+            field=models.CharField(choices=[('c', 'C syntax highlighting'), ('java', 'Java syntax highlighting'), ('mipsasm', 'Mips syntax highlighting'), ('haskell', 'Haskell syntax highlighting'), ('plaintext', 'No syntax highlighting')], default='c', max_length=25),
+        ),
+    ]

core/models/assignment.py

@@ -14,6 +14,10 @@ class DeletionOfDoneAssignmentsNotPermitted(Exception):
     pass
 
 
+class CanOnlyCallFinishOnUnfinishedAssignments(Exception):
+    pass
+
+
 class TutorSubmissionAssignment(models.Model):
 
     assignment_id = models.UUIDField(primary_key=True,
@@ -32,6 +36,19 @@ class TutorSubmissionAssignment(models.Model):
         return (f'{self.subscription.owner} assigned to {self.submission}'
                 f' (done={self.is_done})')
 
+    def finish(self):
+        self.refresh_from_db()
+        if self.is_done:
+            raise CanOnlyCallFinishOnUnfinishedAssignments()
+
+        meta = self.submission.meta
+        meta.feedback_authors.add(self.subscription.owner)
+        meta.done_assignments += 1
+        meta.has_active_assignment = False
+        self.is_done = True
+        self.save()
+        meta.save()
+
     def delete(self, *args, **kwargs):
         if self.is_done:
             raise DeletionOfDoneAssignmentsNotPermitted()

core/models/feedback.py

@@ -25,10 +25,13 @@ class Feedback(models.Model):
         points a student receives for his submission.
     origin : IntegerField
         Of whom was this feedback originally created. She below for the choices
+    final_by_reviewer: BooleanField
+        Whether or not this feedback was set to final by a reviewer once
     """
     score = models.DecimalField(max_digits=5, decimal_places=2, default=0)
     created = models.DateTimeField(auto_now_add=True)
     is_final = models.BooleanField(default=False)
+    final_by_reviewer = models.BooleanField(default=False)
 
     of_submission = models.OneToOneField(
         Submission,

core/models/submission.py

@@ -63,7 +63,9 @@ class MetaSubmission(models.Model):
     done_assignments = models.PositiveIntegerField(default=0)
     has_active_assignment = models.BooleanField(default=False)
 
+    # Managed by signal!
     has_feedback = models.BooleanField(default=False)
+    # Managed by signal!
     has_final_feedback = models.BooleanField(default=False)
 
     feedback_authors = models.ManyToManyField(get_user_model())

core/models/submission_type.py

@@ -35,12 +35,14 @@ class SubmissionType(models.Model):
     JAVA = 'java'
     MIPS = 'mipsasm'
     HASKELL = 'haskell'
+    TEXT = 'plaintext'
 
     LANGUAGE_CHOICES = (
         (C, 'C syntax highlighting'),
         (JAVA, 'Java syntax highlighting'),
         (MIPS, 'Mips syntax highlighting'),
         (HASKELL, 'Haskell syntax highlighting'),
+        (TEXT, 'No syntax highlighting')
     )
 
     submission_type_id = models.UUIDField(primary_key=True,

core/serializers/feedback.py

@@ -7,7 +7,7 @@ from rest_framework import serializers
 from rest_framework.utils import html
 
 from core import models
-from core.models import Feedback
+from core.models import Feedback, UserAccount
 from util.factories import GradyUserFactory
 
 from .generic import DynamicFieldsModelSerializer
@@ -64,6 +64,8 @@ class FeedbackCommentDictionarySerializer(serializers.ListSerializer):
 
 class FeedbackCommentSerializer(DynamicFieldsModelSerializer):
     of_tutor = serializers.StringRelatedField(source='of_tutor.username')
+    labels = serializers.PrimaryKeyRelatedField(many=True, required=False,
+                                                queryset=models.FeedbackLabel.objects.all())
 
     class Meta:
         model = models.FeedbackComment
@@ -121,7 +123,11 @@ class FeedbackSerializer(DynamicFieldsModelSerializer):
         submission = validated_data.pop('of_submission')
         feedback_lines = validated_data.pop('feedback_lines', [])
         labels = validated_data.pop('labels', [])
+        user = self.context['request'].user
+        final_by_reviewer = validated_data.get('is_final', False) and \
+            user.role == UserAccount.REVIEWER
         feedback = Feedback.objects.create(of_submission=submission,
+                                           final_by_reviewer=final_by_reviewer,
                                            **validated_data)
         for label in labels:
             feedback.labels.add(label)
@@ -137,18 +143,25 @@ class FeedbackSerializer(DynamicFieldsModelSerializer):
             )
             comment_instance.labels.set(labels)
 
-        return Feedback.objects.get(of_submission=submission)
+        return feedback
 
     @transaction.atomic
     def update(self, feedback, validated_data):
+        user = self.context['request'].user
+
+        if user.role == UserAccount.REVIEWER:
+            feedback.final_by_reviewer = self.context['request'].data['is_final']
+
         for comment in validated_data.pop('feedback_lines', []):
-            labels = comment.pop('labels', [])
+            labels = comment.pop('labels', None)
             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)
+
+            if labels is not None:
+                comment_instance.labels.set(labels)
 
         return super().update(feedback, validated_data)
 
@@ -192,8 +205,7 @@ class FeedbackSerializer(DynamicFieldsModelSerializer):
             raise serializers.ValidationError(
                 'Sorry, you have to explain why this does not get full score')
 
-        http_method = self.context['request'].method
-        if hasattr(submission, 'feedback') and http_method == 'POST':
+        if hasattr(submission, 'feedback') and not self.instance:
             raise serializers.ValidationError(
                 'Feedback for this submission already exists')
 

core/signals.py

@@ -49,17 +49,7 @@ def update_after_feedback_save(sender, instance, created, **kwargs):
     log.debug('SIGNAL -- update_after_feedback_save')
     meta = instance.of_submission.meta
     meta.has_feedback = True
-    meta.has_final_feedback = instance.is_final
-
-    undone_assignment = meta.submission.assignments.filter(is_done=False)
-    assert undone_assignment.count() <= 1
-    if undone_assignment.count() > 0:
-        log.debug('SIGNAL -- Completed: %s' % undone_assignment.first())
-        meta.feedback_authors.add(undone_assignment.first().subscription.owner)
-        meta.done_assignments += 1
-        meta.has_active_assignment = False
-        undone_assignment.update(is_done=True)
-
+    meta.has_final_feedback = instance.is_final or instance.final_by_reviewer
     meta.save()
 
 

core/tests/test_custom_subscription_filter.py

@@ -140,6 +140,7 @@ class StopOnPass(APITestCase):
         self.assertEqual(35, self.data['students'][0].student.total_score)
         self.assertTrue(self.data['students'][0].student.passes_exam)
 
+    # TODO why is this code commented?!?
     # def test_submissions_left_after_not_pass_only_student_passed_exam(self):
     #     Feedback.objects.create(
     #         of_submission=self.data['submissions'][3], score=20)
@@ -164,8 +165,10 @@ class StopOnPass(APITestCase):
         # signals recognize the open assignments
         Feedback.objects.create(
             of_submission=a1.submission, score=20)
+        a1.finish()
         Feedback.objects.create(
             of_submission=a2.submission, score=15)
+        a2.finish()
 
         subscription_other_tutor = SubmissionSubscription.objects.create(
             owner=self.tutor02,
@@ -188,8 +191,10 @@ class StopOnPass(APITestCase):
         # signals recognize the open assignments
         Feedback.objects.create(
             of_submission=a1.submission, score=20)
+        a1.finish()
         Feedback.objects.create(
             of_submission=a2.submission, score=15)
+        a2.finish()
 
         subscription_other_tutor = SubmissionSubscription.objects.create(
             owner=self.tutor02,

core/tests/test_feedback.py

@@ -93,7 +93,7 @@ class FeedbackCreateTestCase(APITestCase):
 
     @classmethod
     def setUpTestData(cls):
-        cls.url = '/api/feedback/'
+        cls.url = lambda self: f'/api/assignment/{self.assignment.pk}/finish/'
         cls.user_factory = GradyUserFactory()
         cls.tutor = cls.user_factory.make_tutor(password='p')
         cls.exam = make_exams(exams=[{
@@ -138,7 +138,7 @@ class FeedbackCreateTestCase(APITestCase):
 
         }
         self.assertEqual(Feedback.objects.count(), 0)
-        response = self.client.post(self.url, data, format='json')
+        response = self.client.post(self.url(), data, format='json')
         self.assertEqual(status.HTTP_400_BAD_REQUEST, response.status_code)
         self.assertEqual(Feedback.objects.count(), 0)
 
@@ -149,7 +149,7 @@ class FeedbackCreateTestCase(APITestCase):
             'of_submission': self.assignment.submission.pk
         }
         self.assertEqual(Feedback.objects.count(), 0)
-        response = self.client.post(self.url, data, format='json')
+        response = self.client.post(self.url(), data, format='json')
         self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
         self.assertEqual(Feedback.objects.count(), 0)
 
@@ -159,7 +159,7 @@ class FeedbackCreateTestCase(APITestCase):
             'is_final': True,
             'of_submission': self.assignment.submission.pk
         }
-        response = self.client.post(self.url, data, format='json')
+        response = self.client.post(self.url(), data, format='json')
         self.assertEqual(status.HTTP_403_FORBIDDEN, response.status_code)
         self.assertEqual(Feedback.objects.count(), 0)
 
@@ -169,7 +169,7 @@ class FeedbackCreateTestCase(APITestCase):
             'is_final': False,
             'of_submission': self.assignment.submission.pk
         }
-        response = self.client.post(self.url, data, format='json')
+        response = self.client.post(self.url(), data, format='json')
         self.assertEqual(status.HTTP_400_BAD_REQUEST, response.status_code)
         self.assertEqual(Feedback.objects.count(), 0)
 
@@ -180,7 +180,7 @@ class FeedbackCreateTestCase(APITestCase):
             'of_submission': self.assignment.submission.pk
         }
         self.assertEqual(Feedback.objects.count(), 0)
-        response = self.client.post(self.url, data, format='json')
+        response = self.client.post(self.url(), data, format='json')
         self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
         self.assertEqual(Feedback.objects.count(), 0)
 
@@ -191,7 +191,7 @@ class FeedbackCreateTestCase(APITestCase):
             'of_submission': self.assignment.submission.pk
         }
         self.assertEqual(Feedback.objects.count(), 0)
-        response = self.client.post(self.url, data, format='json')
+        response = self.client.post(self.url(), data, format='json')
         self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
         self.assertEqual(Feedback.objects.count(), 0)
 
@@ -209,7 +209,7 @@ class FeedbackCreateTestCase(APITestCase):
             }
         }
         self.assertEqual(self.fst_label.feedback.count(), 0)
-        response = self.client.post(self.url, data, format='json')
+        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()
@@ -229,7 +229,7 @@ class FeedbackCreateTestCase(APITestCase):
                 }
             }
         }
-        self.client.post(self.url, data, format='json')
+        self.client.post(self.url(), data, format='json')
         object_score = self.sub.feedback.score
         self.assertEqual(object_score, 0.5)
 
@@ -245,7 +245,7 @@ class FeedbackCreateTestCase(APITestCase):
                 }
             }
         }
-        self.client.post(self.url, data, format='json')
+        self.client.post(self.url(), data, format='json')
         object_score = self.sub.feedback.score
         self.assertEqual(object_score, 5)
 
@@ -262,7 +262,7 @@ class FeedbackCreateTestCase(APITestCase):
             }
         }
         self.assertEqual(FeedbackComment.objects.count(), 0)
-        response = self.client.post(self.url, data, format='json')
+        response = self.client.post(self.url(), data, format='json')
         self.assertEqual(response.status_code, status.HTTP_201_CREATED)
         self.assertEqual(FeedbackComment.objects.count(), 1)
 
@@ -278,7 +278,7 @@ class FeedbackCreateTestCase(APITestCase):
                 }
             }
         }
-        self.client.post(self.url, data, format='json')
+        self.client.post(self.url(), data, format='json')
         comment = FeedbackComment.objects.first()
         self.assertEqual(comment.of_tutor, self.tutor)
         self.assertEqual(comment.text, 'Nice meth!')
@@ -298,8 +298,8 @@ class FeedbackCreateTestCase(APITestCase):
             }
         }
         self.assignment.delete()
-        response = self.client.post(self.url, data, format='json')
-        self.assertEqual(status.HTTP_403_FORBIDDEN, response.status_code)
+        response = self.client.post(self.url(), data, format='json')
+        self.assertEqual(status.HTTP_404_NOT_FOUND, response.status_code)
 
     def test_cannot_create_with_someoneelses_assignment(self):
         data = {
@@ -314,8 +314,9 @@ class FeedbackCreateTestCase(APITestCase):
         }
         other_tutor = self.user_factory.make_tutor('Berta')
         self.client.force_authenticate(other_tutor)
-        response = self.client.post(self.url, data, format='json')
-        self.assertEqual(status.HTTP_403_FORBIDDEN, response.status_code)
+        response = self.client.post(self.url(), data, format='json')
+        # returns 404 since the other users assignment is not visible to this one
+        self.assertEqual(status.HTTP_404_NOT_FOUND, response.status_code)
 
     def test_can_create_multiple_feedback_comments(self):
         data = {
@@ -333,7 +334,7 @@ class FeedbackCreateTestCase(APITestCase):
                 }
             }
         }
-        self.client.post(self.url, data, format='json')
+        self.client.post(self.url(), data, format='json')
         first_comment = FeedbackComment.objects.get(text='Nice meth!')
         self.assertEqual(first_comment.of_tutor, self.tutor)
         self.assertIsNotNone(first_comment.created)

core/tests/test_subscription_assignment_service.py

@@ -235,7 +235,6 @@ class TestApiEndpoints(APITestCase):
 
         response_subscription_create = self.client.post(
             '/api/subscription/', {'query_type': 'random'})
-        subscription_pk = response_subscription_create.data['pk']
 
         subscription_pk = response_subscription_create.data['pk']
         response_assignment = self.client.post(
@@ -374,7 +373,7 @@ class TestApiEndpoints(APITestCase):
             })
         self.assertEqual(status.HTTP_201_CREATED, response.status_code)
         response = self.client.post(
-            f'/api/feedback/', {
+            f'/api/assignment/{response.data["pk"]}/finish/', {
                 "score": 23,
                 "of_submission": response.data['submission']['pk'],
                 "feedback_lines": {
@@ -415,8 +414,8 @@ class TestApiEndpoints(APITestCase):
         assignment = models.TutorSubmissionAssignment.objects.get(
             pk=response.data['pk'])
         self.assertFalse(assignment.is_done)
-        response = self.client.patch(
-            '/api/feedback/%s/' % submission_id_in_response, {
+        response = self.client.post(
+            f'/api/assignment/{assignment.pk}/finish/', {
                 "score": 20,
                 "is_final": True,
                 "feedback_lines": {

core/tests/test_tutor_api_endpoints.py

@@ -107,6 +107,7 @@ class TutorListTests(APITestCase):
             Feedback.objects.update_or_create(
                 of_submission=assignment.submission,
                 score=35)
+            assignment.finish()
 
         tutor01 = data['tutors'][0]
         tutor02 = data['tutors'][1]

core/urls.py

@@ -7,6 +7,7 @@ from rest_framework.permissions import AllowAny
 from core import views
 
 # Create a router and register our viewsets with it.
+
 router = DefaultRouter()
 router.register('student', views.StudentReviewerApiViewSet,
                 basename='student')

core/views/feedback.py

@@ -4,8 +4,11 @@ from multiprocessing import Lock
 from rest_framework import mixins, status, viewsets
 from rest_framework.exceptions import PermissionDenied
 from rest_framework.response import Response
+from rest_framework import decorators
 
 from core import models, permissions, serializers
+from core.views.util import tutor_attempts_to_patch_first_feedback_final, \
+    get_implicit_assignment_for_user
 
 log = logging.getLogger(__name__)
 
@@ -17,62 +20,26 @@ class FeedbackApiView(
         viewsets.GenericViewSet):
     """ Gets a list of an individual exam by Id if provided """
     permission_classes = (permissions.IsTutorOrReviewer,)
-    queryset = models.Feedback.objects\
-        .select_related('of_submission')\
-        .select_related('of_submission__type')\
-        .select_related('of_submission__student')\
-        .select_related('of_submission__student__user')\
+    queryset = models.Feedback.objects \
+        .select_related('of_submission') \
+        .select_related('of_submission__type') \
+        .select_related('of_submission__student') \
+        .select_related('of_submission__student__user') \
         .all()
     serializer_class = serializers.FeedbackSerializer
     lookup_field = 'of_submission__pk'
     lookup_url_kwarg = 'submission_pk'
 
     def _tutor_attempts_to_change_final_feedback_of_reviewer(self, serializer):
-        feedback_is_final = serializer.instance.is_final
+        feedback_final_by_reviewer = serializer.instance.final_by_reviewer
         user_is_tutor = self.request.user.role == models.UserAccount.TUTOR
-        authors = serializer.instance.of_submission.meta.feedback_authors
-        set_by_reviewer = authors.filter(
-            role=models.UserAccount.REVIEWER).exists()
-        return feedback_is_final and set_by_reviewer and user_is_tutor
-
-    def _get_implicit_assignment_for_user(self, submission):
-        """ Check for tutor if it exists. Not relevant for reviewer """
-        try:
-            return models.TutorSubmissionAssignment.objects.get(
-                subscription__owner=self.request.user,
-                submission=submission
-            )
-        except models.TutorSubmissionAssignment.DoesNotExist:
-            if self.request.user.role == models.UserAccount.REVIEWER:
-                return None
-            raise PermissionDenied(
-                detail='This user has no permission to create this feedback')
+        return feedback_final_by_reviewer and user_is_tutor
 
     def _tutor_attempts_to_set_first_feedback_final(self, serializer):
         is_final_set = serializer.validated_data.get('is_final', False)
         user_is_tutor = self.request.user.role == models.UserAccount.TUTOR
         return is_final_set and user_is_tutor
 
-    # unused
-    def _tutor_is_allowed_to_change_own_feedback(self, serializer):
-        submission = self.get_object().of_submission
-        assignment = self._get_implicit_assignment_for_user(submission)
-        youngest = models.TutorSubmissionAssignment.objects \
-            .filter(submission=submission) \
-            .order_by('-created') \
-            .first()
-
-        return assignment == youngest
-
-    def _tutor_attempts_to_patch_first_feedback_final(self, serializer):
-        if self.request.user.role == models.UserAccount.REVIEWER:
-            return False
-        is_final_set = serializer.validated_data.get('is_final', False)
-        submission = self.get_object().of_submission
-        assignment = self._get_implicit_assignment_for_user(submission)
-        in_creation = assignment.subscription.feedback_stage == models.SubmissionSubscription.FEEDBACK_CREATION  # noqa
-        return is_final_set and in_creation
-
     def get_queryset(self):
         if self.request.user.is_reviewer():
             return self.queryset \
@@ -84,19 +51,15 @@ class FeedbackApiView(
             of_submission__assignments__subscription__owner=self.request.user
         )
 
+    @decorators.permission_classes((permissions.IsReviewer,))
     def create(self, request, *args, **kwargs):
         serializer = self.get_serializer(data=request.data)
         serializer.is_valid(raise_exception=True)
-
-        self._get_implicit_assignment_for_user(
-            serializer.validated_data['of_submission'])
-
-        if self._tutor_attempts_to_set_first_feedback_final(serializer):
-            return Response(
-                {'For tutors it is not allowed to create feedback final.'},
-                status=status.HTTP_403_FORBIDDEN)
-
         self.perform_create(serializer)
+
+        # update MetaSubmission information
+        meta = serializer.validated_data.get('of_submission').meta
+        meta.feedback_authors.add(self.request.user)
         return Response(serializer.data,
                         status=status.HTTP_201_CREATED)
 
@@ -106,16 +69,15 @@ class FeedbackApiView(
                                          partial=True)
         serializer.is_valid(raise_exception=True)
 
-        self._get_implicit_assignment_for_user(feedback.of_submission)
+        assignment = get_implicit_assignment_for_user(feedback.of_submission, self.request.user)
 
         if self._tutor_attempts_to_change_final_feedback_of_reviewer(serializer):  # noqa
             raise PermissionDenied(
-                detail="Changing final feedback is not allowed.")
+                detail="Changing feedback set to final by a reviewer is not allowed.")
 
-        if self._tutor_attempts_to_patch_first_feedback_final(serializer):
+        if tutor_attempts_to_patch_first_feedback_final(serializer, self.request.user, assignment):
             raise PermissionDenied(
                 detail='Cannot set the first feedback final.')
-
         serializer.save()
         return Response(serializer.data)
 

core/views/subscription.py

@@ -2,7 +2,8 @@ import logging
 
 from django.core.exceptions import ObjectDoesNotExist
 from rest_framework import mixins, status, viewsets
-from rest_framework.decorators import action, permission_classes
+from rest_framework import decorators
+from rest_framework.exceptions import PermissionDenied
 from rest_framework.response import Response
 
 from core import models, permissions, serializers
@@ -12,6 +13,8 @@ from core.serializers import AssignmentDetailSerializer, AssignmentSerializer
 
 from multiprocessing import Lock
 
+from core.views.util import tutor_attempts_to_patch_first_feedback_final
+
 log = logging.getLogger(__name__)
 
 
@@ -76,6 +79,7 @@ class AssignmentApiViewSet(
     queryset = TutorSubmissionAssignment.objects\
         .select_related('subscription').all()
     serializer_class = AssignmentSerializer
+    permission_classes = (IsTutorOrReviewer, )
 
     def get_queryset(self):
         if self.action in ['list', 'active', 'destroy']:
@@ -97,11 +101,11 @@ class AssignmentApiViewSet(
                             status=status.HTTP_403_FORBIDDEN)
         return Response(serializer.data, status=status.HTTP_201_CREATED)
 
-    @permission_classes((IsReviewer,))
+    @decorators.permission_classes((IsReviewer,))
     def list(self, *args, **kwargs):
         return super().list(*args, **kwargs)
 
-    @action(detail=False, permission_classes=(IsReviewer,), methods=['get', 'delete'])
+    @decorators.action(detail=False, permission_classes=(IsReviewer,), methods=['get', 'delete'])
     def active(self, request):
         if request.method == 'GET':
             queryset = self.get_queryset().filter(is_done=False)
@@ -111,7 +115,39 @@ class AssignmentApiViewSet(
             self.get_queryset().filter(is_done=False).delete()
             return Response(status=status.HTTP_204_NO_CONTENT)
 
-    @permission_classes((IsTutorOrReviewer,))
+    @decorators.action(detail=True, methods=['post'])
+    def finish(self, request, *args, **kwargs):
+        context = self.get_serializer_context()
+        instance = self.get_object()
+        if instance.is_done or (instance.subscription.owner != request.user):
+            return Response(status=status.HTTP_403_FORBIDDEN)
+        try:
+            orig_feedback = instance.submission.feedback
+            serializer = serializers.FeedbackSerializer(
+                orig_feedback,
+                data=request.data,
+                context=context,
+                partial=True)
+            if orig_feedback.final_by_reviewer and request.user.role == models.UserAccount.TUTOR:
+                raise PermissionDenied(detail="Unfortunately you won't be able to finish this"
+                                              "assignment since a reviewer has marked it as "
+                                              "final while you were assigned.")
+        except models.Feedback.DoesNotExist:
+            serializer = serializers.FeedbackSerializer(
+                data=request.data,
+                context=context)
+
+        serializer.is_valid(raise_exception=True)
+        if tutor_attempts_to_patch_first_feedback_final(serializer, self.request.user, instance):
+            raise PermissionDenied(
+                detail='Cannot set the first feedback final.')
+        serializer.save()
+        instance.finish()
+        response_status = status.HTTP_201_CREATED if \
+            instance.subscription.feedback_stage == \
+            models.SubmissionSubscription.FEEDBACK_CREATION else status.HTTP_200_OK
+        return Response(serializer.data, status=response_status)
+
     def destroy(self, request, pk=None):
         """ Stop working on the assignment before it is finished """
         instance = self.get_object()
@@ -123,7 +159,6 @@ class AssignmentApiViewSet(
         instance.delete()
         return Response(status=status.HTTP_204_NO_CONTENT)
 
-    @permission_classes((IsTutorOrReviewer,))
     def create(self, request, *args, **kwargs):
         with Lock():
             context = self.get_serializer_context()
@@ -133,7 +168,6 @@ class AssignmentApiViewSet(
             assignment = self._fetch_assignment(serializer)
         return assignment
 
-    @permission_classes((IsTutorOrReviewer,))
     def retrieve(self, request, *args, **kwargs):
         assignment = self.get_object()
         if assignment.subscription.owner != request.user: