fixes to ignore responses with no values in counts

These should never happen but if they do we should not count them as
responses for progress counting purposes. Also do not count two
responses to the same question as two responses.

crowdsourcer/fixtures/ror_responses.json

@@ -4,9 +4,9 @@
   "pk": 6,
   "fields": {
     "authority": 2,
-    "question": 273,
+    "question": 272,
     "user": 3,
-    "option": null,
+    "option": 181,
     "response_type": 2,
     "public_notes": "",
     "page_number": "0",
@@ -27,7 +27,7 @@
     "authority": 2,
     "question": 273,
     "user": 3,
-    "option": null,
+    "option": 191,
     "response_type": 2,
     "public_notes": "",
     "page_number": "0",

crowdsourcer/models.py

@@ -134,10 +134,11 @@ def response_counts(
                     question__in=questions,
                     response_type=response_type,
                 )
+                .exclude(id__in=Response.null_responses())
                 .values("authority")
-                .annotate(response_count=Count("pk"))
+                .annotate(response_count=Count("question_id", distinct=True))
                 .values("response_count")
-            )
+            ),
         )
 
         if assigned is not None:
@@ -227,6 +228,10 @@ def get_absolute_url(self):
             },
         )
 
+    @classmethod
+    def null_responses(cls):
+        return cls.objects.filter(option__isnull=True, multi_option__isnull=True)
+
 
 class Assigned(models.Model):
     user = models.ForeignKey(User, on_delete=models.CASCADE)

crowdsourcer/tests/test_right_of_reply_views.py

@@ -50,6 +50,40 @@ def test_council_homepage(self):
         self.assertEqual(second.total, 2)
         self.assertEqual(second.complete, 0)
 
+    def test_null_answers_ignored(self):
+        Response.objects.filter(question_id=272, user=3, response_type=2).update(
+            option=None
+        )
+        url = reverse("authority_ror_sections", args=("Aberdeenshire Council",))
+        response = self.client.get(url)
+
+        context = response.context
+        sections = context["sections"]
+
+        self.assertEqual(len(sections), 7)
+
+        first = sections[0]
+        self.assertEqual(first.title, "Buildings & Heating")
+
+        self.assertEqual(first.complete, 1)
+
+    def test_duplicate_answers_ignored(self):
+        Response.objects.filter(question_id=272, user=3, response_type=2).update(
+            question_id=273
+        )
+        url = reverse("authority_ror_sections", args=("Aberdeenshire Council",))
+        response = self.client.get(url)
+
+        context = response.context
+        sections = context["sections"]
+
+        self.assertEqual(len(sections), 7)
+
+        first = sections[0]
+        self.assertEqual(first.title, "Buildings & Heating")
+
+        self.assertEqual(first.complete, 1)
+
 
 class TestTwoCouncilsAssignmentView(BaseTestCase):
     def setUp(self):

crowdsourcer/tests/test_views.py

@@ -106,6 +106,23 @@ def test_completion_stats(self):
         second = progress[1]
         self.assertEqual(second["complete"], 2)
 
+    def test_completion_stats_ignore_null_responses(self):
+        response = self.client.get("/")
+        context = response.context
+        progress = context["progress"]
+
+        second = progress[1]
+        self.assertEqual(second["complete"], 1)
+
+        Response.objects.filter(question_id=281, user=2).update(option=None)
+
+        response = self.client.get("/")
+        context = response.context
+        progress = context["progress"]
+
+        second = progress[1]
+        self.assertEqual(second["complete"], 0)
+
 
 class TestUserSectionProgressView(BaseTestCase):
     def test_view(self):
@@ -120,6 +137,16 @@ def test_view(self):
         self.assertEqual(context["authorities"][1].num_responses, None)
         self.assertEqual(context["authorities"][1].num_questions, 2)
 
+    def test_null_responses_ignored(self):
+        Response.objects.filter(question_id=281, user=2).update(option=None)
+
+        url = reverse("section_authorities", args=("Transport",))
+        response = self.client.get(url)
+        self.assertEqual(response.status_code, 200)
+
+        context = response.context
+        self.assertEqual(context["authorities"][0].num_responses, 1)
+
 
 class TestSaveView(BaseTestCase):
     fixtures = [
@@ -443,7 +470,7 @@ def test_view(self):
         self.assertEquals(context["councils"]["total"], 4)
 
 
-class TestSectionProgressView(BaseTestCase):
+class TestAllSectionProgressView(BaseTestCase):
     def test_non_admin_denied(self):
         response = self.client.get(reverse("all_section_progress"))
         self.assertEquals(response.status_code, 403)
@@ -465,6 +492,20 @@ def test_view(self):
         self.assertEquals(context["Buildings & Heating"]["assigned"], 1)
         self.assertEquals(context["Buildings & Heating"]["total"], 4)
 
+    def test_null_responses_ignored(self):
+        Response.objects.filter(question_id=281, user=2).update(option=None)
+
+        u = User.objects.get(username="admin")
+        self.client.force_login(u)
+        response = self.client.get(reverse("all_section_progress"))
+        self.assertEqual(response.status_code, 200)
+
+        context = response.context["progress"]
+        self.assertEquals(context["Transport"]["complete"], 0)
+        self.assertEquals(context["Transport"]["started"], 1)
+        self.assertEquals(context["Transport"]["assigned"], 2)
+        self.assertEquals(context["Transport"]["total"], 4)
+
 
 class TestAuthorityLoginView(BaseTestCase):
     def test_view(self):

crowdsourcer/views/progress.py

@@ -84,8 +84,9 @@ def get_queryset(self):
                         authority=OuterRef("pk"),
                         response_type=response_type,
                     )
+                    .exclude(id__in=Response.null_responses())
                     .values("authority")
-                    .annotate(response_count=Count("pk"))
+                    .annotate(response_count=Count("question_id", distinct=True))
                     .values("response_count")
                 )
             )
@@ -150,6 +151,7 @@ def get_context_data(self, **kwargs):
 
         stage = ResponseType.objects.get(type=self.stage)
         sections = context["sections"]
+
         for section in sections:
             questions = Question.objects.filter(
                 section=section, how_marked__in=self.types
@@ -176,8 +178,9 @@ def get_context_data(self, **kwargs):
                             question__in=question_list,
                             response_type=stage,
                         )
+                        .exclude(id__in=Response.null_responses())
                         .values("authority")
-                        .annotate(response_count=Count("pk"))
+                        .annotate(response_count=Count("question_id", distict=True))
                         .values("response_count")
                     )
                 )
@@ -260,8 +263,9 @@ def get_context_data(self, **kwargs):
                             authority=OuterRef("pk"),
                             response_type=self.current_stage,
                         )
+                        .exclude(id__in=Response.null_responses())
                         .values("authority")
-                        .annotate(response_count=Count("pk"))
+                        .annotate(response_count=Count("question_id", distinct=True))
                         .values("response_count")
                     )
                 )
@@ -428,7 +432,7 @@ def get_context_data(self, **kwargs):
                             agree_with_response=False,
                         )
                         .values("authority")
-                        .annotate(response_count=Count("pk"))
+                        .annotate(response_count=Count("question_id", distict=True))
                         .values("response_count")
                     )
                 )