Bug fix: Backwards routing does not work when dependencies exist (#1306)

Co-authored-by: Rhys Berrow <47635349+berroar@users.noreply.github.com>
ONSdigital · Jan 25, 2024 · a9c29e9 · a9c29e9
1 parent 1bb38b6
commit a9c29e9
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 23 deletions.
diff --git 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
@@ -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
@@ -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,36 +448,35 @@ 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
 
     routing_path = path_finder.routing_path(SectionKey(section_id))
 
     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(
         "confirm-zero-employees-answer"
     )
     assert (
         path_finder.data_stores.progress_store.get_section_status(
-            SectionKey("default-section")
+            SectionKey("section-2")
         )
         == CompletionStatus.IN_PROGRESS
     )