From 082aad000d49d2dfb7ee07ce9a41ca95d7f4ea09 Mon Sep 17 00:00:00 2001 From: Struan Donald Date: Wed, 18 Dec 2024 15:56:41 +0000 Subject: [PATCH 1/5] add config option to allow skipping questions on right of reply This is initially for WhoFundsThem so MPs only need to response to things where they have an entry in the register. If "right_of_reply_responses_to_ignore" is set as a session config then any response that matches the list, plus blank responses, will not be displayed on the right of reply question screen. The config should be a JSON array with the text descriptions. For multi option values it will skip only if all selected values are in the list. Also updates the section page questions counts to reflect this --- crowdsourcer/models.py | 1 + .../tests/test_right_of_reply_views.py | 52 +++++++++++++++++++ crowdsourcer/views/rightofreply.py | 49 +++++++++++++++-- 3 files changed, 97 insertions(+), 5 deletions(-) diff --git a/crowdsourcer/models.py b/crowdsourcer/models.py index a6a35a9..9761590 100644 --- a/crowdsourcer/models.py +++ b/crowdsourcer/models.py @@ -261,6 +261,7 @@ def response_counts( ).annotate( num_questions=Subquery( Question.objects.filter( + id__in=questions, questiongroup=OuterRef("questiongroup"), section__title=section, section__marking_session=marking_session, diff --git a/crowdsourcer/tests/test_right_of_reply_views.py b/crowdsourcer/tests/test_right_of_reply_views.py index e44bbbc..f29a498 100644 --- a/crowdsourcer/tests/test_right_of_reply_views.py +++ b/crowdsourcer/tests/test_right_of_reply_views.py @@ -12,6 +12,7 @@ PublicAuthority, Response, ResponseType, + SessionConfig, SessionProperties, SessionPropertyValues, ) @@ -110,6 +111,31 @@ def test_duplicate_answers_ignored(self): self.assertEqual(first.complete, 1) + def test_ignore_questions_config(self): + SessionConfig.objects.create( + name="right_of_reply_responses_to_ignore", + marking_session=MarkingSession.objects.get(label="Default"), + config_type="json", + json_value=["Car share"], + ) + + 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] + second = sections[1] + self.assertEqual(first.title, "Buildings & Heating") + self.assertEqual(second.title, "Transport") + + self.assertEqual(first.total, 2) + self.assertEqual(first.complete, 2) + self.assertEqual(second.total, 1) + self.assertEqual(second.complete, 0) + class TestTwoCouncilsAssignmentView(BaseTestCase): def setUp(self): @@ -400,6 +426,32 @@ def test_questions(self): self.assertRegex(response.content, rb"vehicle fleet") self.assertNotRegex(response.content, rb"Second Session") + def test_ignore_questions_config(self): + url = reverse("authority_ror", args=("Aberdeenshire Council", "Transport")) + response = self.client.get(url) + self.assertEqual(response.status_code, 200) + + self.assertEqual(len(response.context["form"].initial), 2) + self.assertEqual( + response.context["form"].initial[0]["question"].number_and_part, "1" + ) + self.assertEqual( + response.context["form"].initial[1]["question"].number_and_part, "2" + ) + + SessionConfig.objects.create( + name="right_of_reply_responses_to_ignore", + marking_session=MarkingSession.objects.get(label="Default"), + config_type="json", + json_value=["Car share"], + ) + + response = self.client.get(url) + self.assertEqual(len(response.context["form"].initial), 1) + self.assertEqual( + response.context["form"].initial[0]["question"].number_and_part, "1" + ) + def test_questions_alt_session(self): u = User.objects.get(username="other_marker") rt = ResponseType.objects.get(type="Right of Reply") diff --git a/crowdsourcer/views/rightofreply.py b/crowdsourcer/views/rightofreply.py index 77c4255..f9e753d 100644 --- a/crowdsourcer/views/rightofreply.py +++ b/crowdsourcer/views/rightofreply.py @@ -17,6 +17,7 @@ Response, ResponseType, Section, + SessionConfig, SessionProperties, SessionPropertyValues, ) @@ -109,14 +110,31 @@ def get_context_data(self, **kwargs): question_types = ["volunteer", "national_volunteer", "foi"] response_type = ResponseType.objects.get(type="Right of Reply") + authority = PublicAuthority.objects.get(name=context["authority_name"]) + for section in sections: - questions = Question.objects.filter( - section=section, - how_marked__in=question_types, + responses_to_ignore = SessionConfig.get_config( + self.request.current_session, "right_of_reply_responses_to_ignore" ) - question_list = list(questions.values_list("id", flat=True)) + if responses_to_ignore: + questions = ( + Response.objects.filter( + authority=authority, + question__section=section, + question__how_marked__in=question_types, + response_type=ResponseType.objects.get(type="First Mark"), + ) + .exclude(option__description__in=responses_to_ignore) + .exclude(multi_option__description__in=responses_to_ignore) + ) + question_list = list(questions.values_list("question_id", flat=True)) + else: + questions = Question.objects.filter( + section=section, + how_marked__in=question_types, + ) + question_list = list(questions.values_list("id", flat=True)) - authority = PublicAuthority.objects.get(name=context["authority_name"]) args = [ question_list, section.title, @@ -177,6 +195,27 @@ def get_initial_obj(self): initial[r.question.id] = data + responses_to_ignore = SessionConfig.get_config( + self.request.current_session, "right_of_reply_responses_to_ignore" + ) + if responses_to_ignore: + valid_questions = ( + Response.objects.filter( + authority=self.authority, + question__in=self.questions, + response_type=rt, + ) + .exclude(option__description__in=responses_to_ignore) + .exclude(multi_option__description__in=responses_to_ignore) + .values_list("question_id", flat=True) + ) + + new_initial = {} + for k in initial.keys(): + if k in valid_questions: + new_initial[k] = initial[k] + initial = new_initial + if self.has_previous(): ror_rt = ResponseType.objects.get(type="Right of Reply") initial = self.add_previous(initial, ror_rt) From 9bd652315c1b24812951e48bb4f5456e5d9a0c30 Mon Sep 17 00:00:00 2001 From: Struan Donald Date: Thu, 19 Dec 2024 09:40:23 +0000 Subject: [PATCH 2/5] fix spaces in f string --- crowdsourcer/views/progress.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crowdsourcer/views/progress.py b/crowdsourcer/views/progress.py index 9ce38ff..37ec224 100644 --- a/crowdsourcer/views/progress.py +++ b/crowdsourcer/views/progress.py @@ -129,10 +129,10 @@ def get_context_data(self, **kwargs): if self.request.current_session.entity_name is not None: self.page_title = ( - f"{self.request.current_session.entity_name}s {self.page_title }" + f"{self.request.current_session.entity_name}s {self.page_title}" ) else: - self.page_title = f"Councils {self.page_title }" + self.page_title = f"Councils {self.page_title}" context["councils"] = council_totals context["authorities"] = authorities From 863ecd33b0831cf21cb98a9739569d49a7493a32 Mon Sep 17 00:00:00 2001 From: Struan Donald Date: Thu, 19 Dec 2024 09:48:55 +0000 Subject: [PATCH 3/5] throw 404 if bad authority for ror question view --- crowdsourcer/views/rightofreply.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crowdsourcer/views/rightofreply.py b/crowdsourcer/views/rightofreply.py index f9e753d..8a9fdbb 100644 --- a/crowdsourcer/views/rightofreply.py +++ b/crowdsourcer/views/rightofreply.py @@ -4,7 +4,7 @@ from django.core.exceptions import PermissionDenied from django.http import HttpResponse -from django.shortcuts import redirect +from django.shortcuts import get_object_or_404, redirect from django.urls import reverse from django.views.generic import ListView @@ -224,7 +224,8 @@ def get_initial_obj(self): def check_permissions(self): denied = True - authority = PublicAuthority.objects.get(name=self.kwargs["name"]) + authority = get_object_or_404(PublicAuthority, name=self.kwargs["name"]) + user = self.request.user if user.is_anonymous: From d19b35e425ff664935fbc09845e4446e9c5e7eeb Mon Sep 17 00:00:00 2001 From: Struan Donald Date: Thu, 19 Dec 2024 09:52:39 +0000 Subject: [PATCH 4/5] raise a 404 if the marking session name is bad --- crowdsourcer/middleware.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crowdsourcer/middleware.py b/crowdsourcer/middleware.py index a66db83..4425ff6 100644 --- a/crowdsourcer/middleware.py +++ b/crowdsourcer/middleware.py @@ -1,4 +1,5 @@ from django.conf import settings +from django.http import Http404 from crowdsourcer.models import MarkingSession, ResponseType @@ -21,6 +22,8 @@ def process_view(self, request, view_func, view_args, view_kwargs): current_session = MarkingSession.objects.filter( label=session_name, active=True ).first() + if current_session is None: + raise Http404 else: current_session = ( MarkingSession.objects.filter(active=True).order_by("-default").first() From 699da05b04633d991249d1226ea8c06ce77bdcbb Mon Sep 17 00:00:00 2001 From: Struan Donald Date: Thu, 19 Dec 2024 10:10:13 +0000 Subject: [PATCH 5/5] make sure authority and section exist on marking page --- crowdsourcer/views/base.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/crowdsourcer/views/base.py b/crowdsourcer/views/base.py index eac44b6..2873856 100644 --- a/crowdsourcer/views/base.py +++ b/crowdsourcer/views/base.py @@ -10,6 +10,7 @@ from django.db.models.functions import Cast from django.dispatch import receiver from django.http import JsonResponse +from django.shortcuts import get_object_or_404 from django.views.generic import ListView, TemplateView from crowdsourcer.forms import ResponseForm, ResponseFormset @@ -97,10 +98,14 @@ def add_previous(self, initial, rt): return initial def get_initial_obj(self): - self.authority = PublicAuthority.objects.get(name=self.kwargs["name"]) + self.authority = get_object_or_404(PublicAuthority, name=self.kwargs["name"]) + section = get_object_or_404( + Section, + title=self.kwargs["section_title"], + marking_session=self.request.current_session, + ) self.questions = Question.objects.filter( - section__marking_session=self.request.current_session, - section__title=self.kwargs["section_title"], + section=section, questiongroup=self.authority.questiongroup, how_marked__in=self.how_marked_in, ).order_by("number", "number_part")