From 14b1cdcee430ef6418ed950fc05d45f7c9c5beb1 Mon Sep 17 00:00:00 2001 From: Andrew Maddaford Date: Tue, 13 Oct 2020 15:28:52 +0100 Subject: [PATCH] Filter list summaries on unrelated question blocks List summaries on unrelated question blocks now only include people after the current person. --- app/views/contexts/list_context.py | 9 +++++ app/views/handlers/question.py | 33 ++++++++++--------- .../relationships/unrelated_question.py | 22 +++++++++++-- tests/app/views/contexts/test_list_context.py | 30 +++++++++++++++++ ...t_questionnaire_relationships_unrelated.py | 22 +++++++++++-- 5 files changed, 96 insertions(+), 20 deletions(-) diff --git a/app/views/contexts/list_context.py b/app/views/contexts/list_context.py index ba23e10451..9b827c7985 100644 --- a/app/views/contexts/list_context.py +++ b/app/views/contexts/list_context.py @@ -15,6 +15,7 @@ def __call__( edit_block_id=None, remove_block_id=None, primary_person_edit_block_id=None, + for_list_item_ids=None, ): list_items = ( list( @@ -25,6 +26,7 @@ def __call__( edit_block_id, remove_block_id, primary_person_edit_block_id, + for_list_item_ids, ) ) if summary_definition @@ -46,8 +48,15 @@ def _build_list_items_context( edit_block_id, remove_block_id, primary_person_edit_block_id, + for_list_item_ids, ): list_item_ids = self._list_store[for_list] + if for_list_item_ids: + list_item_ids = [ + list_item_id + for list_item_id in list_item_ids + if list_item_id in for_list_item_ids + ] primary_person = self._list_store[for_list].primary_person for list_item_id in list_item_ids: diff --git a/app/views/handlers/question.py b/app/views/handlers/question.py index 2b2edbd07b..9258d87d9a 100644 --- a/app/views/handlers/question.py +++ b/app/views/handlers/question.py @@ -73,6 +73,17 @@ def rendered_block(self): **{"question": rendered_question}, } + @cached_property + def list_context(self): + return ListContext( + self._language, + self._schema, + self._questionnaire_store.answer_store, + self._questionnaire_store.list_store, + self._questionnaire_store.progress_store, + self._questionnaire_store.metadata, + ) + def get_next_location_url(self): answer_action = self._get_answer_action() if self._has_redirect_to_list_add_action(answer_action): @@ -132,21 +143,7 @@ def get_context(self): ] = self.get_last_viewed_question_guidance_context() if "list_summary" in self.rendered_block: - list_context = ListContext( - self._language, - self._schema, - self._questionnaire_store.answer_store, - self._questionnaire_store.list_store, - self._questionnaire_store.progress_store, - self._questionnaire_store.metadata, - ) - - context.update( - list_context( - self.rendered_block["list_summary"]["summary"], - self.rendered_block["list_summary"]["for_list"], - ) - ) + context.update(self.get_list_summary_context()) if self.form.errors or self.form.question_errors: self.page_title = gettext("Error: {page_title}").format( @@ -162,6 +159,12 @@ def get_last_viewed_question_guidance_context(self): ).url() return {"first_location_in_section_url": first_location_in_section_url} + def get_list_summary_context(self): + return self.list_context( + self.rendered_block["list_summary"]["summary"], + self.rendered_block["list_summary"]["for_list"], + ) + def handle_post(self): self.questionnaire_store_updater.update_answers(self.form.data) if self.questionnaire_store_updater.is_dirty(): diff --git a/app/views/handlers/relationships/unrelated_question.py b/app/views/handlers/relationships/unrelated_question.py index 870ac1eb29..f7e3cffa5b 100644 --- a/app/views/handlers/relationships/unrelated_question.py +++ b/app/views/handlers/relationships/unrelated_question.py @@ -1,19 +1,35 @@ +from functools import cached_property + from app.questionnaire.location import Location from app.views.handlers.question import Question class UnrelatedQuestion(Question): - def _get_parent_list_name(self): + @cached_property + def list_name(self): parent_block_id = self._schema.parent_id_map[self.block["id"]] return self._schema.get_block(parent_block_id)["for_list"] - @property + @cached_property def parent_location(self): parent_block_id = self._schema.parent_id_map[self.block["id"]] return Location( section_id=self._current_location.section_id, block_id=parent_block_id ) + @cached_property + def remaining_relationship_list_item_ids(self): + list_model = self._questionnaire_store.list_store[self.list_name] + start = list_model.index(self._current_location.list_item_id) + 1 + return list_model[start:] + + def get_list_summary_context(self): + return self.list_context( + self.rendered_block["list_summary"]["summary"], + self.rendered_block["list_summary"]["for_list"], + for_list_item_ids=self.remaining_relationship_list_item_ids, + ) + def _get_routing_path(self): return self.router.routing_path(section_id=self.parent_location.section_id) @@ -25,7 +41,7 @@ def is_location_valid(self): if not can_access_parent_location: return False - if self.current_location.list_name != self._get_parent_list_name() or ( + if self.current_location.list_name != self.list_name or ( self.current_location.list_item_id and not self.router.is_list_item_in_list_store( self.current_location.list_item_id, self.current_location.list_name diff --git a/tests/app/views/contexts/test_list_context.py b/tests/app/views/contexts/test_list_context.py index 3e25e2784a..213176c6c4 100644 --- a/tests/app/views/contexts/test_list_context.py +++ b/tests/app/views/contexts/test_list_context.py @@ -91,3 +91,33 @@ def test_assert_primary_person_string_appended( assert list_context["list"]["list_items"][0]["primary_person"] is True assert list_context["list"]["list_items"][0]["item_title"] == "Toni Morrison (You)" assert list_context["list"]["list_items"][1]["item_title"] == "Barry Pheloung" + + +@pytest.mark.usefixtures("app") +def test_for_list_item_ids( + list_collector_block, people_answer_store, people_list_store +): + schema = load_schema_from_name("test_list_collector_primary_person") + + list_context = ListContext( + language=DEFAULT_LANGUAGE_CODE, + progress_store=ProgressStore(), + list_store=people_list_store, + schema=schema, + answer_store=people_answer_store, + metadata=None, + ) + list_context = list_context( + list_collector_block["summary"], + list_collector_block["for_list"], + for_list_item_ids=["UHPLbX"], + ) + + expected = [ + { + "item_title": "Barry Pheloung", + "primary_person": False, + } + ] + + assert expected == list_context["list"]["list_items"] diff --git a/tests/integration/questionnaire/test_questionnaire_relationships_unrelated.py b/tests/integration/questionnaire/test_questionnaire_relationships_unrelated.py index 1d1edf523a..6842be3a33 100644 --- a/tests/integration/questionnaire/test_questionnaire_relationships_unrelated.py +++ b/tests/integration/questionnaire/test_questionnaire_relationships_unrelated.py @@ -6,6 +6,7 @@ def launch_survey_and_add_people(self): self.launchSurvey("test_relationships_unrelated", roles=["dumper"]) self.add_person("Marie", "Doe") self.add_person("John", "Doe") + self.add_person("Jane", "Doe") self.post({"anyone-else": "No"}) def test_is_accessible_when_list_name_and_list_item_valid( @@ -18,8 +19,6 @@ def test_is_accessible_when_list_name_and_list_item_valid( f"/questionnaire/relationships/people/{first_list_item}/related-to-anyone-else" ) self.assertInBody("Are any of these people related to you?") - self.assertInBody("Marie Doe") - self.assertInBody("John Doe") def test_is_not_accessible_when_invalid_list_item(self): self.launchSurvey("test_relationships_unrelated") @@ -45,3 +44,22 @@ def test_is_not_accessible_when_invalid_block_id(self): f"/questionnaire/relationships/people/{first_list_item}/invalid-block-id" ) self.assertStatusNotFound() + + def test_list_summary(self): + self.launch_survey_and_add_people() + + first_list_item = self.dump_debug()["LISTS"][0]["items"][0] + self.get( + f"/questionnaire/relationships/people/{first_list_item}/related-to-anyone-else" + ) + self.assertNotInBody("Marie Doe") + self.assertInBody("John Doe") + self.assertInBody("Jane Doe") + + second_list_item = self.dump_debug()["LISTS"][0]["items"][1] + self.get( + f"/questionnaire/relationships/people/{second_list_item}/related-to-anyone-else" + ) + self.assertNotInBody("Marie Doe") + self.assertNotInBody("John Doe") + self.assertInBody("Jane Doe")