Fixes a nasty race condition

core/apps.py

@@ -4,3 +4,6 @@ from django.apps import AppConfig
 class CoreConfig(AppConfig):
     name = 'core'
     verbose_name = 'where everything comes together'
+
+    def ready(self):
+        import core.signals  # noqa

core/migrations/0003_submissiondoneassignmentscount.py

+# Generated by Django 2.0.2 on 2018-02-17 12:01
+
+from django.db import migrations, models
+import django.db.models.deletion
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('core', '0002_auto_20180210_1727'),
+    ]
+
+    operations = [
+        migrations.CreateModel(
+            name='SubmissionDoneAssignmentsCount',
+            fields=[
+                ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
+                ('done_assignments', models.PositiveIntegerField(default=0)),
+                ('submission', models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, related_name='counter', to='core.Submission')),
+            ],
+        ),
+    ]

core/models.py

@@ -502,6 +502,7 @@ class SubmissionSubscription(models.Model):
         if self.query_type == self.RANDOM:
             return Submission.objects.all()
 
+        TutorSubmissionAssignment.objects.select_for_update()
         return Submission.objects.filter(
             **{self.type_query_mapper[self.query_type]: self.query_key})
 
@@ -521,11 +522,6 @@ class SubmissionSubscription(models.Model):
             Q(assignments__subscription__owner=self.owner)
         ).exclude(
             assignments__is_done=False
-        ).annotate(
-            done_assignments_count=Count(
-                Case(When(assignments__is_done=True, then=Value(1)),
-                     output_field=IntegerField(),)
-            )
         )
 
     def _get_available_submissions_in_subscription_stage(self) -> QuerySet:
@@ -546,7 +542,7 @@ class SubmissionSubscription(models.Model):
 
         done_assignments_count = self.assignment_count_on_stage[self.feedback_stage]  # noqa
         stage_candiates = candidates.filter(
-            done_assignments_count=done_assignments_count,
+            counter__done_assignments=done_assignments_count,
         )
 
         if stage_candiates.count() == 0:
@@ -607,6 +603,14 @@ class DeletionOfDoneAssignmentsNotPermitted(Exception):
     pass
 
 
+class SubmissionDoneAssignmentsCount(models.Model):
+
+    submission = models.OneToOneField('submission',
+                                      related_name='counter',
+                                      on_delete=models.CASCADE)
+    done_assignments = models.PositiveIntegerField(default=0)
+
+
 class TutorSubmissionAssignment(models.Model):
 
     assignment_id = models.UUIDField(primary_key=True,
@@ -623,6 +627,9 @@ class TutorSubmissionAssignment(models.Model):
 
     @transaction.atomic
     def set_done(self):
+        if not self.is_done:
+            self.submission.counter.done_assignments += 1
+            self.submission.counter.save()
         self.is_done = True
         self.save()
 

core/signals.py

+from core.models import Submission, SubmissionDoneAssignmentsCount
+
+from django.db.models.signals import post_save
+from django.dispatch import receiver
+
+
+@receiver(post_save, sender=Submission)
+def create_counter_after_submission_create(sender,
+                                           instance,
+                                           created,
+                                           **kwargs):
+    if created:
+        SubmissionDoneAssignmentsCount.objects.create(submission=instance)

core/tests/test_subscription_assignment_service.py

@@ -109,12 +109,6 @@ class TestApiEndpoints(APITestCase):
                     'full_score': 35,
                     'description': 'Very complicated',
                     'solution': 'Trivial!'
-                },
-                {
-                    'name': '03. This one exists for the sole purpose to test',
-                    'full_score': 30,
-                    'description': 'Very complicated',
-                    'solution': 'Trivial!'
                 }
             ],
             'students': [
@@ -160,12 +154,12 @@ class TestApiEndpoints(APITestCase):
                             '   on multi lines\n'
                             '       asasxasx\n'
                             '           lorem ipsum und so\n',
-                    'type': '03. This one exists for the sole purpose to test',
-                    'user': 'student01'
+                    'type': '01. Sort this or that',
+                    'user': 'student02'
                 },
                 {
                     'text': 'function lorem ipsum etc\n',
-                    'type': '03. This one exists for the sole purpose to test',
+                    'type': '02. Merge this or that or maybe even this',
                     'user': 'student02'
                 },
             ]}
@@ -242,17 +236,17 @@ class TestApiEndpoints(APITestCase):
         client = APIClient()
         client.force_authenticate(user=self.data['reviewers'][0])
 
-        student01 = self.data['students'][0]
+        student = self.data['students'][0]
 
         response = client.post(
             '/api/subscription/', {
                 'query_type': 'student',
-                'query_key': student01.student.student_id,
+                'query_key': student.student.student_id,
                 'stage': 'feedback-creation'
             })
 
         assignments = response.data['assignments']
-        self.assertEqual(2, len(assignments))
+        self.assertEqual(1, len(assignments))
 
     def test_two_tutors_cant_have_assignments_for_same_submission(self):
         client = APIClient()

util/factories.py

@@ -53,7 +53,7 @@ class GradyUserFactory:
         }[role]
 
     def _make_base_user(self, username, role, password=None,
-                        store_pw=False, name='', **kwargs):
+                        store_pw=False, fullname='', **kwargs):
         """ This is a specific wrapper for the django update_or_create method of
         objects.
             * If now username is passed, a generic one will be generated
@@ -72,7 +72,7 @@ class GradyUserFactory:
 
         user, created = UserAccount.objects.update_or_create(
             username=username,
-            fullname=name,
+            fullname=fullname,
             role=role,
             defaults=kwargs)
 

util/importer.py

@@ -110,7 +110,7 @@ def add_tests(submission_obj, tests):
 
     for name, test_data in ((name, tests[name]) for name in TEST_ORDER):
         test_obj, created = Test.objects.update_or_create(
-            name=test_data['fullname'],
+            name=test_data['name'],
             submission=submission_obj,
             defaults={
                 'label': test_data['label'],