From a9c29e9e6146409aa9437adfc638b127e7b5658d Mon Sep 17 00:00:00 2001 From: Mebin Abraham <35296336+MebinAbraham@users.noreply.github.com> Date: Thu, 25 Jan 2024 10:57:58 +0000 Subject: [PATCH] Bug fix: Backwards routing does not work when dependencies exist (#1306) Co-authored-by: Rhys Berrow <47635349+berroar@users.noreply.github.com> --- app/questionnaire/path_finder.py | 21 +++++++++--------- ...nfirmation_question_backwards_routing.json | 18 ++++++++++++--- tests/app/questionnaire/test_path_finder.py | 22 +++++++++++-------- 3 files changed, 38 insertions(+), 23 deletions(-) diff --git a/app/questionnaire/path_finder.py b/app/questionnaire/path_finder.py index 4b7e82d099..466c0aaf6c 100644 --- a/app/questionnaire/path_finder.py +++ b/app/questionnaire/path_finder.py @@ -105,7 +105,7 @@ def _build_routing_path_block_ids( when_rules_block_dependencies: list[str], ) -> list[str]: # Keep going unless we've hit the last block - + # routing_path_block_ids can be mutated by _evaluate_routing_rules routing_path_block_ids: list[str] = [] block_index = 0 repeating_list = self.schema.get_repeating_list_for_section( @@ -175,16 +175,16 @@ def _evaluate_routing_rules( routing_path_block_ids: list[str], when_rules_block_dependencies: list[str], ) -> int | None: - if when_rules_block_dependencies: - routing_path_block_ids = ( - when_rules_block_dependencies + routing_path_block_ids - ) + # Use `list` to create a shallow copy since routing_path_block_ids is mutated hence we don't want to update its memory reference + block_ids_for_dependencies = ( + list(routing_path_block_ids) + when_rules_block_dependencies + ) when_rule_evaluator = RuleEvaluator( self.schema, self.data_stores, location=this_location, - routing_path_block_ids=routing_path_block_ids, + routing_path_block_ids=block_ids_for_dependencies, ) for rule in routing_rules: rule_valid = ( @@ -224,16 +224,15 @@ def evaluate_skip_conditions( if not skip_conditions: return False - if when_rules_block_dependencies: - routing_path_block_ids = ( - when_rules_block_dependencies + routing_path_block_ids - ) + block_ids_for_dependencies = ( + list(routing_path_block_ids) + when_rules_block_dependencies + ) when_rule_evaluator = RuleEvaluator( schema=self.schema, data_stores=self.data_stores, location=current_location, - routing_path_block_ids=routing_path_block_ids, + routing_path_block_ids=block_ids_for_dependencies, ) return when_rule_evaluator.evaluate(skip_conditions["when"]) diff --git a/schemas/test/en/test_confirmation_question_backwards_routing.json b/schemas/test/en/test_confirmation_question_backwards_routing.json index 53555fb1cc..b8bc14b8fe 100644 --- a/schemas/test/en/test_confirmation_question_backwards_routing.json +++ b/schemas/test/en/test_confirmation_question_backwards_routing.json @@ -40,11 +40,11 @@ "sections": [ { "id": "default-section", - "title": "Questions", + "title": "Section 1", "groups": [ { "id": "confirmation", - "title": "Confirmation Question Test", + "title": "Confirmation Driver", "blocks": [ { "type": "Question", @@ -71,7 +71,19 @@ } ] } - }, + } + ] + } + ] + }, + { + "id": "section-2", + "title": "Section 2", + "groups": [ + { + "id": "group-2", + "title": "Confirmation Question", + "blocks": [ { "id": "number-of-employees-total-block", "question": { diff --git a/tests/app/questionnaire/test_path_finder.py b/tests/app/questionnaire/test_path_finder.py index 1a84a43d16..f47562f025 100644 --- a/tests/app/questionnaire/test_path_finder.py +++ b/tests/app/questionnaire/test_path_finder.py @@ -414,12 +414,17 @@ def test_remove_answer_and_block_if_routing_backwards(): "section_id": "default-section", "list_item_id": None, "status": CompletionStatus.COMPLETED, + "block_ids": ["route-backwards-block"], + }, + { + "section_id": "section-2", + "list_item_id": None, + "status": CompletionStatus.COMPLETED, "block_ids": [ - "route-backwards-block", "number-of-employees-total-block", "confirm-zero-employees-block", ], - } + }, ] ) @@ -443,10 +448,10 @@ def test_remove_answer_and_block_if_routing_backwards(): assert ( len( path_finder.data_stores.progress_store.get_completed_block_ids( - SectionKey("default-section") + SectionKey("section-2") ) ) - == 3 + == 2 ) assert len(path_finder.data_stores.answer_store) == 3 @@ -454,17 +459,16 @@ def test_remove_answer_and_block_if_routing_backwards(): expected_path = RoutingPath( block_ids=[ - "route-backwards-block", "number-of-employees-total-block", "confirm-zero-employees-block", "number-of-employees-total-block", ], - section_id="default-section", + section_id="section-2", ) assert routing_path == expected_path assert path_finder.data_stores.progress_store.get_completed_block_ids( - SectionKey("default-section") - ) == progress_store.get_completed_block_ids(SectionKey("default-section")) + SectionKey("section-2") + ) == progress_store.get_completed_block_ids(SectionKey("section-2")) assert len(path_finder.data_stores.answer_store) == 2 assert not path_finder.data_stores.answer_store.get_answer( @@ -472,7 +476,7 @@ def test_remove_answer_and_block_if_routing_backwards(): ) assert ( path_finder.data_stores.progress_store.get_section_status( - SectionKey("default-section") + SectionKey("section-2") ) == CompletionStatus.IN_PROGRESS )