Changed student query_key to a UUID. Closes #87

* Added stages to the subscription mechanism, meaning that
  a subscription is distingushed between feedback-creation,
  -validation and -conflict resolution
* Minor refactoring to the Feedback ViewSet and serializer
* Added a larger integration test

core/migrations/0001_initial.py

-# Generated by Django 2.0.1 on 2018-01-07 14:37
-
-import uuid
+# Generated by Django 2.0.1 on 2018-01-10 10:46
 
+import core.models
+from django.conf import settings
 import django.contrib.auth.models
 import django.contrib.auth.validators
+from django.db import migrations, models
 import django.db.models.deletion
 import django.utils.timezone
-from django.conf import settings
-from django.db import migrations, models
-
-import core.models
+import uuid
 
 
 class Migration(migrations.Migration):
@@ -113,13 +111,14 @@ class Migration(migrations.Migration):
                 ('subscription_id', models.UUIDField(default=uuid.uuid4, editable=False, primary_key=True, serialize=False)),
                 ('query_key', models.CharField(blank=True, max_length=75)),
                 ('query_type', models.CharField(choices=[('random', 'Query for any submission'), ('student', 'Query for submissions of student'), ('exam', 'Query for submissions of exam type'), ('submission_type', 'Query for submissions of submissions_type')], default='random', max_length=75)),
-                ('owner', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='susbscriptions', to=settings.AUTH_USER_MODEL)),
+                ('feedback_stage', models.CharField(choices=[('feedback-creation', 'No feedback was ever assigned'), ('feedback-validation', 'Feedback exists but is not validated'), ('feedback-conflict-resolution', 'Previous correctors disagree')], default='feedback-creation', max_length=40)),
+                ('owner', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='subscriptions', to=settings.AUTH_USER_MODEL)),
             ],
         ),
         migrations.CreateModel(
             name='StudentInfo',
             fields=[
-                ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
+                ('student_id', models.UUIDField(default=uuid.uuid4, editable=False, primary_key=True, serialize=False)),
                 ('has_logged_in', models.BooleanField(default=False)),
                 ('matrikel_no', models.CharField(default=core.models.random_matrikel_no, max_length=8, unique=True)),
                 ('exam', models.ForeignKey(null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='students', to='core.ExamType')),

core/models.py

@@ -205,6 +205,9 @@ class StudentInfo(models.Model):
         matrikel_no (CharField):
             The matriculation number of the student
     """
+    student_id = models.UUIDField(primary_key=True,
+                                  default=uuid.uuid4,
+                                  editable=False)
     has_logged_in = models.BooleanField(default=False)
     matrikel_no = models.CharField(unique=True,
                                    max_length=8,
@@ -446,7 +449,7 @@ class GeneralTaskSubscription(models.Model):
 
     type_query_mapper = {
         RANDOM: '__any',
-        STUDENT_QUERY: 'student__user__username',
+        STUDENT_QUERY: 'student__student_id',
         EXAM_TYPE_QUERY: 'student__examtype__module_reference',
         SUBMISSION_TYPE_QUERY: 'type__title',
     }
@@ -458,16 +461,35 @@ class GeneralTaskSubscription(models.Model):
         (SUBMISSION_TYPE_QUERY, 'Query for submissions of submissions_type'),
     )
 
+    FEEDBACK_CREATION = 'feedback-creation'
+    FEEDBACK_VALIDATION = 'feedback-validation'
+    FEEDBACK_CONFLICT_RESOLUTION = 'feedback-conflict-resolution'
+
+    type_to_assignment_count_map = {
+        FEEDBACK_CREATION: 0,
+        FEEDBACK_VALIDATION: 1,
+        FEEDBACK_CONFLICT_RESOLUTION: 2,
+    }
+
+    stages = (
+        (FEEDBACK_CREATION, 'No feedback was ever assigned'),
+        (FEEDBACK_VALIDATION, 'Feedback exists but is not validated'),
+        (FEEDBACK_CONFLICT_RESOLUTION, 'Previous correctors disagree'),
+    )
+
     subscription_id = models.UUIDField(primary_key=True,
                                        default=uuid.uuid4,
                                        editable=False)
     owner = models.ForeignKey(get_user_model(),
                               on_delete=models.CASCADE,
-                              related_name='susbscriptions')
+                              related_name='subscriptions')
     query_key = models.CharField(max_length=75, blank=True)
     query_type = models.CharField(max_length=75,
                                   choices=QUERY_CHOICE,
                                   default=RANDOM)
+    feedback_stage = models.CharField(choices=stages,
+                                      max_length=40,
+                                      default=FEEDBACK_CREATION)
 
     class Meta:
         unique_together = ('owner', 'query_key', 'query_type')
@@ -479,6 +501,24 @@ class GeneralTaskSubscription(models.Model):
         return Submission.objects.filter(
             **{self.type_query_mapper[self.query_type]: self.query_key})
 
+    def _find_submissions_based_on_completed_assignments(
+            self, done_assignments_count):
+        base = self._get_submission_base_query().filter(
+            Q(feedback__isnull=False),
+            Q(feedback__is_final=False),
+            ~Q(feedback__of_tutor=self.owner)
+        ).annotate(  # to display only manual
+            done_assignments_count=Count(
+                Case(When(assignments__is_done=True, then=Value(1)),
+                     output_field=IntegerField(),
+                     )
+            )
+        ).filter(done_assignments_count=self.type_to_assignment_count_map[
+            done_assignments_count])
+
+        log.debug('base %s', base)
+        return base
+
     def _find_unassigned_non_final_submissions(self):
         unassigned_non_final_submissions = \
             self._get_submission_base_query().filter(
@@ -492,31 +532,25 @@ class GeneralTaskSubscription(models.Model):
         return unassigned_non_final_submissions
 
     def _find_unassigned_unapproved_non_final_submissions(self):
-        unapproved_not_final_submissions = \
-            self._get_submission_base_query().filter(
-                Q(feedback__isnull=False),
-                Q(feedback__is_final=False),
-                ~Q(feedback__of_tutor=self.owner),
-                # TODO: prevent reassigning to the same tutor
-            )
-
-        log.debug('unapproved not final submissions %s',
-                  unapproved_not_final_submissions)
+        return self._find_submissions_based_on_completed_assignments(
+            done_assignments_count=self.FEEDBACK_VALIDATION)
 
-        return unapproved_not_final_submissions
+    def _find_submissions_with_confilcting_feedback(self):
+        return self._find_submissions_based_on_completed_assignments(
+            done_assignments_count=self.FEEDBACK_CONFLICT_RESOLUTION)
 
     def _get_next_assignment_in_subscription(self):
-        assignment_priority = (
+        candidates = (
             self._find_unassigned_non_final_submissions,
-            self._find_unassigned_unapproved_non_final_submissions
-        )
+            self._find_unassigned_unapproved_non_final_submissions,
+            self._find_submissions_with_confilcting_feedback
+        )[self.type_to_assignment_count_map[self.feedback_stage]]()
 
-        lazy_queries = (query_set() for query_set in assignment_priority)
-        for query in (q for q in lazy_queries if len(q) > 0):
-            return query.first()
+        if candidates.count() == 0:
+            raise SubscriptionEnded(
+                f'The task which user {self.owner} subscribed to is done')
 
-        raise SubscriptionEnded(
-            f'The task which user {self.owner} subscribed to is done')
+        return candidates.first()
 
     @transaction.atomic
     def get_or_create_work_assignment(self):

core/serializers.py

@@ -15,6 +15,14 @@ log = logging.getLogger(__name__)
 user_factory = GradyUserFactory()
 
 
+class NoValidAssignmentForFeedbackCreation(Exception):
+    pass
+
+
+class CannotSetFirstFeedbackFinal(Exception):
+    pass
+
+
 class DynamicFieldsModelSerializer(DynamicFieldsMixin,
                                    serializers.ModelSerializer):
     pass
@@ -29,6 +37,7 @@ class ExamSerializer(DynamicFieldsModelSerializer):
 
 
 class FeedbackForSubmissionLineSerializer(serializers.BaseSerializer):
+
     def to_representation(self, obj):
         return {feedback.of_line:
                 FeedbackCommentSerializer(
@@ -44,7 +53,7 @@ class FeedbackSerializer(DynamicFieldsModelSerializer):
     feedbacklines = FeedbackForSubmissionLineSerializer(
         required=False
     )
-    isFinal = serializers.BooleanField(source="is_final")
+    isFinal = serializers.BooleanField(source="is_final", required=False)
     ofSubmission = serializers.PrimaryKeyRelatedField(
         source='of_submission',
         required=False,
@@ -76,6 +85,7 @@ class FeedbackSerializer(DynamicFieldsModelSerializer):
         log.debug(data)
         assignment_id = data.pop('assignment_id')
         score = data.get('score')
+        is_final = data.get('is_final', False)
 
         try:
             assignment = TutorSubmissionAssignment.objects.get(
@@ -83,6 +93,16 @@ class FeedbackSerializer(DynamicFieldsModelSerializer):
         except ObjectDoesNotExist as err:
             raise serializers.ValidationError('No assignment for given id.')
 
+        requesting_tutor = self.context['request'].user
+        if assignment.subscription.owner != requesting_tutor:
+            raise NoValidAssignmentForFeedbackCreation()
+
+        http_method = self.context['request'].method
+        if (is_final and
+                http_method == 'POST' and
+                requesting_tutor.role == models.UserAccount.TUTOR):
+            raise CannotSetFirstFeedbackFinal()
+
         submission = assignment.submission
         if not 0 <= score <= submission.type.full_score:
             raise serializers.ValidationError(
@@ -251,4 +271,5 @@ class SubscriptionSerializer(DynamicFieldsModelSerializer):
             'owner',
             'query_type',
             'query_key',
-            'assignments')
+            'assignments',
+            'feedback_stage')

core/tests/test_subscription_assignment_service.py

 from rest_framework import status
 from rest_framework.test import APIClient, APITestCase
 
+from core import models
 from core.models import (GeneralTaskSubscription, Submission, SubmissionType,
                          SubscriptionEnded)
 from util.factories import GradyUserFactory, make_test_data
@@ -91,6 +92,9 @@ class TestApiEndpoints(APITestCase):
                 {'username': 'tutor01'},
                 {'username': 'tutor02'}
             ],
+            'reviewers': [
+                {'username': 'reviewer'}
+            ],
             'submissions': [
                 {
                     'text': 'function blabl\n'
@@ -145,7 +149,7 @@ class TestApiEndpoints(APITestCase):
 
         self.assertEqual('tutor01', response.data['owner'])
 
-    def test_subscription_has_next_assignment(self):
+    def test_subscription_has_next_and_current_assignment(self):
         client = APIClient()
         client.force_authenticate(user=self.data['tutors'][0])
 
@@ -154,22 +158,86 @@ class TestApiEndpoints(APITestCase):
         subscription_id = response_subs.data['subscription_id']
         assignment_id = response_subs.data['assignments'][0]['assignment_id']
 
-        response_current = client.get(
+        response = client.get(
             f'/api/subscription/{subscription_id}/assignments/current/')
 
         self.assertEqual(1, len(response_subs.data['assignments']))
         self.assertEqual(assignment_id,
-                         response_current.data['assignment_id'])
+                         response.data['assignment_id'])
+
+        response_next = client.get(
+            f'/api/subscription/{subscription_id}/assignments/next/')
+        response_detail_subs = \
+            client.get(f'/api/subscription/{subscription_id}/')
+
+        self.assertEqual(2, len(response_detail_subs.data['assignments']))
+        self.assertNotEqual(assignment_id, response_next.data['assignment_id'])
 
     def test_subscription_can_assign_to_student(self):
         client = APIClient()
-        client.force_authenticate(user=self.data['tutors'][0])
+        client.force_authenticate(user=self.data['reviewers'][0])
 
-        response_subs = client.post(
+        student01 = self.data['students'][0]
+
+        response = client.post(
             '/api/subscription/', {
                 'query_type': 'student',
-                'query_key': 'student01'
+                'query_key': student01.student.student_id,
+                'stage': 'feedback-creation'
             })
 
-        assignments = response_subs.data['assignments']
+        assignments = response.data['assignments']
         self.assertEqual(2, len(assignments))
+
+    def test_all_stages_of_the_subscription(self):
+        client = APIClient()
+        client.force_authenticate(user=self.data['tutors'][0])
+
+        # The tutor corrects something
+        response = client.post(
+            '/api/subscription/', {
+                'query_type': 'random',
+                'feedback_stage': 'feedback-creation'
+            })
+
+        self.assertEqual(response.status_code, status.HTTP_201_CREATED)
+
+        assignment_id = response.data['assignments'][0]['assignment_id']
+        response = client.post(
+            f'/api/feedback/', {
+                "score": 23,
+                "assignment_id": assignment_id,
+                "feedbacklines": {
+                    2: {"text": "< some string >"},
+                    3: {"text": "< some string >"}
+                }
+            }
+        )
+        print(response, response.data)
+        self.assertEqual(response.status_code, status.HTTP_201_CREATED)
+
+        # some other tutor reviews it
+        client.force_authenticate(user=self.data['tutors'][1])
+
+        response = client.post(
+            '/api/subscription/', {
+                'query_type': 'random',
+                'feedback_stage': 'feedback-validation'
+            })
+
+        self.assertEqual(response.status_code, status.HTTP_201_CREATED)
+
+        subscription_id = response.data['subscription_id']
+        response = client.get(
+            f'/api/subscription/{subscription_id}/assignments/current/')
+
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+
+        submissioin_id_in_database = models.Feedback.objects.filter(
+            is_final=False).first().of_submission.submission_id
+        submissioin_id_in_response = \
+            response.data['submission']['submission_id']
+
+        self.assertEqual(
+            str(submissioin_id_in_database),
+            submissioin_id_in_response)

core/views.py

@@ -6,7 +6,7 @@ from rest_framework import generics, mixins, status, viewsets
 from rest_framework.decorators import api_view, detail_route
 from rest_framework.response import Response
 
-from core import models
+from core import models, serializers
 from core.models import (ExamType, Feedback, GeneralTaskSubscription,
                          StudentInfo, SubmissionType,
                          TutorSubmissionAssignment)
@@ -67,9 +67,18 @@ class FeedbackApiView(
     lookup_url_kwarg = 'submission_id'
 
     def create(self, request, *args, **kwargs):
-        serializer = self.get_serializer(
-            data={**request.data, 'of_tutor': request.user})
-        serializer.is_valid(raise_exception=True)
+        serializer = self.get_serializer(data=request.data)
+
+        try:
+            serializer.is_valid(raise_exception=True)
+        except serializers.NoValidAssignmentForFeedbackCreation:
+            return Response(
+                {'This user has not permission to create this feedback'},
+                status=status.HTTP_403_FORBIDDEN)
+        except serializers.CannotSetFirstFeedbackFinal:
+            return Response(
+                {'Cannot set the first feedback final unless user reviewer'},
+                status=status.HTTP_403_FORBIDDEN)
 
         self.perform_create(serializer)
         headers = self.get_success_headers(serializer.data)