From e7e725f2786077055bc1a9f1f14a5584ec282dc9 Mon Sep 17 00:00:00 2001
From: janmax <j.michal@stud.uni-goettingen.de>
Date: Thu, 15 Feb 2018 14:22:53 +0100
Subject: [PATCH] Fixes #94 thanks for the testcase. Added documentation and
 logging

---
 core/models.py                                | 39 +++++++++++++++----
 .../test_subscription_assignment_service.py   |  8 ++--
 2 files changed, 35 insertions(+), 12 deletions(-)

diff --git a/core/models.py b/core/models.py
index d10cc0d2..7acdf52b 100644
--- a/core/models.py
+++ b/core/models.py
@@ -501,17 +501,31 @@ class SubmissionSubscription(models.Model):
                            'feedback_stage')
 
     def _get_submission_base_query(self) -> QuerySet:
+        """ Get all submissions that are filtered by the query key and type,
+        e.g. all submissions of one student or submission type.
+        """
         if self.query_type == self.RANDOM:
             return Submission.objects.all()
 
         return Submission.objects.filter(
             **{self.type_query_mapper[self.query_type]: self.query_key})
 
-    def _get_submissions_that_do_not_have_final_feedback(self):
+    def _get_submissions_that_do_not_have_final_feedback(self) -> QuerySet:
+        """ There are a number of conditions to check for each submission
+
+        1. The submission does not have final feedback
+        2. The submission was not shown to this user before
+        3. The submission is not currently assigned to somebody else
+
+        Returns:
+            QuerySet -- a list of all submissions ready for consumption
+        """
         return self._get_submission_base_query().exclude(
             Q(feedback__isnull=False) & Q(feedback__is_final=True)
         ).exclude(
             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)),
@@ -519,7 +533,16 @@ class SubmissionSubscription(models.Model):
             )
         )
 
-    def _get_available_submissions_in_subscription_stage(self):
+    def _get_available_submissions_in_subscription_stage(self) -> QuerySet:
+        """ Another filter this time it returns all the submissions that
+        are valid in this stage. That means all previous stages have been
+        completed.
+
+        Raises:
+            SubscriptionEnded -- if the subscription will not yield
+                                 subscriptions in the future
+            SubscriptionTemporarilyEnded -- wait until new become available
+        """
         candidates = self._get_submissions_that_do_not_have_final_feedback()
 
         if candidates.count() == 0:
@@ -528,20 +551,20 @@ 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
+            done_assignments_count=done_assignments_count,
         )
 
         if stage_candiates.count() == 0:
             raise SubscriptionTemporarilyEnded(
-                'Currently unavailabe. Please check for more soon. '
+                'Currently unavailable. Please check for more soon. '
                 'Submissions remaining: %s' % stage_candiates.count())
 
         return stage_candiates
 
-    def get_remaining_not_final(self):
+    def get_remaining_not_final(self) -> int:
         return self._get_submissions_that_do_not_have_final_feedback().count()
 
-    def get_available_in_stage(self):
+    def get_available_in_stage(self) -> int:
         try:
             return self._get_available_submissions_in_subscription_stage().count()  # noqa
         except (SubscriptionTemporarilyEnded, SubscriptionEnded) as err:
@@ -554,6 +577,7 @@ class SubmissionSubscription(models.Model):
             raise NotMoreThanTwoOpenAssignmentsAllowed(
                 'Not more than 2 active assignments allowed.')
 
+        log.info(f'{self.owner} is assignment to {task}.')
         return TutorSubmissionAssignment.objects.get_or_create(
             subscription=self,
             submission=task)[0]
@@ -618,8 +642,7 @@ class TutorSubmissionAssignment(models.Model):
 
 
 class FeedbackComment(models.Model):
-
-    """This Class contains the Feedback for a specific line of a Submission"""
+    """ This Class contains the Feedback for a specific line of a Submission"""
     text = models.TextField()
     created = models.DateTimeField(auto_now_add=True)
     modified = models.DateTimeField(auto_now=True)
diff --git a/core/tests/test_subscription_assignment_service.py b/core/tests/test_subscription_assignment_service.py
index db9aa76e..ea2533b8 100644
--- a/core/tests/test_subscription_assignment_service.py
+++ b/core/tests/test_subscription_assignment_service.py
@@ -262,10 +262,10 @@ class TestApiEndpoints(APITestCase):
                                    {'query_type': 'random'}).data
 
         assignment_fst_tutor = client.post(
-                '/api/assignment/', {
-                    'subscription': subscription['pk']
-                }
-            ).data
+            '/api/assignment/', {
+                'subscription': subscription['pk']
+            }
+        ).data
 
         client.force_authenticate(user=self.data['tutors'][1])
 
-- 
GitLab