diff --git a/app/questionnaire/router.py b/app/questionnaire/router.py index 07b0d20522..c1902c597c 100644 --- a/app/questionnaire/router.py +++ b/app/questionnaire/router.py @@ -348,18 +348,23 @@ def _get_return_to_for_calculated_summary( return_to_block_id = ",".join(remaining) if remaining else None # remove first item and return the remaining ones - # Type ignore: return_location.return_to will always be populated at this point - return_to_remaining = ",".join(return_location.return_to.split(",")[1:]) or None # type: ignore + # Type ignore: return_location.return_to and return_location.return_to_answer_id will always be populated at this point + return_to = ",".join(return_location.return_to.split(",")[1:]) or None # type: ignore + anchor, *return_to_answer_ids = return_location.return_to_answer_id.split(",") # type: ignore + return_to_answer_id = ( + ",".join(return_to_answer_ids) if return_to_answer_ids else None + ) return url_for( "questionnaire.block", block_id=block_id, list_name=location.list_name, list_item_id=location.list_item_id, - return_to=return_to_remaining, + return_to=return_to, return_to_block_id=return_to_block_id, return_to_list_item_id=return_location.return_to_list_item_id, - _anchor=return_location.return_to_answer_id, + return_to_answer_id=return_to_answer_id, + _anchor=anchor, ) def _get_return_url_for_inaccessible_location( diff --git a/app/views/contexts/calculated_summary_context.py b/app/views/contexts/calculated_summary_context.py index 95e3375c1b..a6c0c61b9f 100644 --- a/app/views/contexts/calculated_summary_context.py +++ b/app/views/contexts/calculated_summary_context.py @@ -82,6 +82,9 @@ def build_groups_for_section( return_to=return_to, return_to_block_id=return_to_block_id, return_to_list_item_id=self.return_location.return_to_list_item_id, + return_to_answer_id=self.return_location.return_to_answer_id + if self.return_location.return_to == "grand-calculated-summary" + else None, ), ).serialize() for group in section["groups"] diff --git a/app/views/contexts/summary/answer.py b/app/views/contexts/summary/answer.py index ea11a130b2..226ac9c043 100644 --- a/app/views/contexts/summary/answer.py +++ b/app/views/contexts/summary/answer.py @@ -64,17 +64,19 @@ def _build_link( return_location: ReturnLocation, is_in_repeating_section: bool, ) -> str: + return_to_answer_id = self._return_to_answer_id( + return_to=return_location.return_to, + list_item_id=list_item_id, + is_in_repeating_section=is_in_repeating_section, + return_to_answer_id=return_location.return_to_answer_id, + ) return url_for( endpoint="questionnaire.block", list_name=list_name, block_id=block_id, list_item_id=list_item_id, return_to=return_location.return_to, - return_to_answer_id=self._return_to_answer_id( - return_to=return_location.return_to, - list_item_id=list_item_id, - is_in_repeating_section=is_in_repeating_section, - ), + return_to_answer_id=return_to_answer_id, return_to_block_id=return_location.return_to_block_id, return_to_list_item_id=return_location.return_to_list_item_id, _anchor=self.id, @@ -86,16 +88,24 @@ def _return_to_answer_id( return_to: str | None, list_item_id: str | None, is_in_repeating_section: bool, + return_to_answer_id: str | None, ) -> str | None: """ If the summary page using this answer has repeating answers, but it is not in a repeating section, then the answer ids will be suffixed with list item id, so the return to answer id link also needs this to work correctly """ + answer_id = None if return_to: if ( list_item_id and not is_in_repeating_section and not self._original_answer_id # original answer would mean id has already been suffixed ): - return f"{self.id}-{list_item_id}" - return self.id + answer_id = f"{self.id}-{list_item_id}" + else: + answer_id = self.id + + if return_to_answer_id: + answer_id += f",{return_to_answer_id}" + + return answer_id diff --git a/tests/app/questionnaire/test_router.py b/tests/app/questionnaire/test_router.py index d05d0b9302..f39658b53f 100644 --- a/tests/app/questionnaire/test_router.py +++ b/tests/app/questionnaire/test_router.py @@ -771,7 +771,7 @@ def test_return_to_grand_calculated_summary_from_answer( return_location = ReturnLocation( return_to="calculated-summary,grand-calculated-summary", - return_to_answer_id="distance-calculated-summary-1", + return_to_answer_id="q1-a1,distance-calculated-summary-1", return_to_block_id="distance-calculated-summary-1,distance-grand-calculated-summary", ) @@ -784,11 +784,137 @@ def test_return_to_grand_calculated_summary_from_answer( return_to="grand-calculated-summary", block_id="distance-calculated-summary-1", return_to_block_id="distance-grand-calculated-summary", - _anchor="distance-calculated-summary-1", + return_to_answer_id="distance-calculated-summary-1", + _anchor="q1-a1", ) assert expected_previous_url == next_location_url + @pytest.mark.usefixtures("app") + def test_return_to_calculated_summary_from_answer_when_multiple_answers(self): + """ + If going from GCS -> CS -> answer -> CS -> GCS this tests going from CS -> GCS having just come from an answer + """ + self.schema = load_schema_from_name( + "test_grand_calculated_summary_overlapping_answers" + ) + self.data_stores.progress_store = ProgressStore( + [ + ProgressDict( + section_id="introduction-section", + block_ids=[ + "introduction-block", + ], + status=CompletionStatus.COMPLETED, + ), + ProgressDict( + section_id="section-1", + block_ids=[ + "block-1", + "block-2", + "calculated-summary-1", + "calculated-summary-2", + "block-3", + "calculated-summary-3", + ], + status=CompletionStatus.COMPLETED, + ), + ] + ) + + current_location = Location(section_id="section-1", block_id="block-1") + + routing_path = RoutingPath( + block_ids=[ + "block-1", + "block-2", + "calculated-summary-1", + "calculated-summary-2", + "block-3", + "calculated-summary-3", + ], + section_id="section-1", + ) + + return_location = ReturnLocation( + return_to="calculated-summary,grand-calculated-summary", + return_to_answer_id="q1-a1,calculated-summary-1", + return_to_block_id="calculated-summary-1,grand-calculated-summary-shopping", + ) + + next_location_url = self.router.get_next_location_url( + current_location, routing_path, return_location + ) + + assert ( + next_location_url + == "/questionnaire/calculated-summary-1/?return_to=grand-calculated-summary&return_to_block_id=grand-calculated-summary-shopping&" + "return_to_answer_id=calculated-summary-1#q1-a1" + ) + + @pytest.mark.usefixtures("app") + def test_return_to_grand_calculated_summary_from_answer_when_multiple_answers(self): + """ + If going from GCS -> CS -> answer -> CS -> GCS this tests going from CS -> GCS having just come from an answer + """ + self.schema = load_schema_from_name( + "test_grand_calculated_summary_overlapping_answers" + ) + self.data_stores.progress_store = ProgressStore( + [ + ProgressDict( + section_id="introduction-section", + block_ids=[ + "introduction-block", + ], + status=CompletionStatus.COMPLETED, + ), + ProgressDict( + section_id="section-1", + block_ids=[ + "block-1", + "block-2", + "calculated-summary-1", + "calculated-summary-2", + "block-3", + "calculated-summary-3", + ], + status=CompletionStatus.COMPLETED, + ), + ] + ) + + current_location = Location( + section_id="section-1", block_id="calculated-summary-1" + ) + + routing_path = RoutingPath( + block_ids=[ + "block-1", + "block-2", + "calculated-summary-1", + "calculated-summary-2", + "block-3", + "calculated-summary-3", + ], + section_id="section-1", + ) + + return_location = ReturnLocation( + return_to="grand-calculated-summary", + return_to_answer_id="calculated-summary-1", + return_to_block_id="grand-calculated-summary-shopping", + ) + + next_location_url = self.router.get_next_location_url( + current_location, routing_path, return_location + ) + + assert ( + next_location_url + == "/questionnaire/grand-calculated-summary-shopping/#calculated-summary-1" + ) + @pytest.mark.usefixtures("app") def test_return_to_grand_calculated_summary_from_calculated_summary( self, grand_calculated_summary_progress_store, grand_calculated_summary_schema @@ -1030,7 +1156,7 @@ def test_return_to_calculated_summary_from_incomplete_section( ) return_location = ReturnLocation( return_to="calculated-summary,grand-calculated-summary", - return_to_answer_id="first-number-block", + return_to_answer_id="q1-a1,distance-calculated-summary-1", return_to_block_id="distance-calculated-summary-1,distance-grand-calculated-summary", ) # the test is being done as part of a two-step return to but its identical functionally @@ -1045,7 +1171,7 @@ def test_return_to_calculated_summary_from_incomplete_section( "questionnaire.block", return_to="calculated-summary,grand-calculated-summary", return_to_block_id="distance-calculated-summary-1,distance-grand-calculated-summary", - return_to_answer_id="first-number-block", + return_to_answer_id="q1-a1,distance-calculated-summary-1", block_id="second-number-block", ) @@ -1387,7 +1513,7 @@ def test_return_to_grand_calculated_summary_from_answer_incomplete_section( return_location = ReturnLocation( return_to="calculated-summary,grand-calculated-summary", - return_to_answer_id="second-number-block", + return_to_answer_id="q2-a1", return_to_block_id="number-calculated-summary-1,number-grand-calculated-summary", ) previous_location_url = self.router.get_previous_location_url( @@ -1401,7 +1527,7 @@ def test_return_to_grand_calculated_summary_from_answer_incomplete_section( return_to="calculated-summary,grand-calculated-summary", return_to_block_id="number-calculated-summary-1,number-grand-calculated-summary", block_id="first-number-block", - _anchor="second-number-block", + _anchor="q2-a1", ) assert expected_previous_url == previous_location_url @@ -1549,7 +1675,7 @@ def test_return_to_grand_calculated_summary_from_repeating_answer( return_location = ReturnLocation( return_to="calculated-summary,grand-calculated-summary", - return_to_answer_id="calculated-summary-6", + return_to_answer_id="streaming-service-monthly-cost-JSfZqh,calculated-summary-6", return_to_block_id="calculated-summary-6,grand-calculated-summary-3", ) next_location_url = self.router.get_previous_location_url( @@ -1563,7 +1689,8 @@ def test_return_to_grand_calculated_summary_from_repeating_answer( return_to="grand-calculated-summary", block_id="calculated-summary-6", return_to_block_id="grand-calculated-summary-3", - _anchor="calculated-summary-6", + return_to_answer_id="calculated-summary-6", + _anchor="streaming-service-monthly-cost-JSfZqh", ) assert expected_previous_url == next_location_url diff --git a/tests/app/views/contexts/summary/test_answer.py b/tests/app/views/contexts/summary/test_answer.py index 14e22ecef4..4eaa47078e 100644 --- a/tests/app/views/contexts/summary/test_answer.py +++ b/tests/app/views/contexts/summary/test_answer.py @@ -6,20 +6,58 @@ @pytest.mark.usefixtures("app") @pytest.mark.parametrize( - "return_to, return_to_block_id, is_in_repeating_section", + "return_to, return_to_block_id, is_in_repeating_section, return_to_answer_id, query_string", [ - ("section-summary", None, False), - (None, None, False), - ("calculated-summary", "total", False), - ("section-summary", None, True), - (None, None, True), - ("calculated-summary", "total", True), + ( + "section-summary", + None, + False, + "answer-id", + "?return_to=section-summary&return_to_answer_id=answer-id-answer-item-id,answer-id", + ), + (None, None, False, "answer-id", ""), + ( + "calculated-summary", + "total", + False, + "answer-id", + "?return_to=calculated-summary&return_to_answer_id=answer-id-answer-item-id,answer-id&return_to_block_id=total", + ), + ( + "section-summary", + None, + True, + "answer-id-answer-item-id", + "?return_to=section-summary&return_to_answer_id=answer-id,answer-id-answer-item-id", + ), + (None, None, True, "answer-id-answer-item-id", ""), + ( + "calculated-summary", + "total", + True, + "answer-id-answer-item-id", + "?return_to=calculated-summary&return_to_answer_id=answer-id,answer-id-answer-item-id&return_to_block_id=total", + ), + ( + "calculated-summary", + "total", + True, + "calculated-summary-1", + "?return_to=calculated-summary&return_to_answer_id=answer-id,calculated-summary-1&return_to_block_id=total", + ), ], ) -def test_create_answer(return_to, return_to_block_id, is_in_repeating_section): +def test_create_answer( + return_to, + return_to_block_id, + is_in_repeating_section, + return_to_answer_id, + query_string, +): return_location = ReturnLocation( return_to=return_to, return_to_block_id=return_to_block_id, + return_to_answer_id=return_to_answer_id, ) answer = Answer( @@ -37,19 +75,6 @@ def test_create_answer(return_to, return_to_block_id, is_in_repeating_section): assert answer.value == "An answer" assert answer.type == "date" - return_to_answer_id = answer.id - if not is_in_repeating_section: - return_to_answer_id += "-answer-item-id" - - if return_to and return_to_block_id: - query_string = f"?return_to={return_to}&return_to_answer_id={return_to_answer_id}&return_to_block_id={return_to_block_id}" - elif return_to: - query_string = ( - f"?return_to={return_to}&return_to_answer_id={return_to_answer_id}" - ) - else: - query_string = "" - assert ( answer.link == f"/questionnaire/answer-list/answer-item-id/house-type/{query_string}#{answer.id}" diff --git a/tests/functional/spec/features/grand_calculated_summary/grand_calculated_summary_overlapping_answers.spec.js b/tests/functional/spec/features/grand_calculated_summary/grand_calculated_summary_overlapping_answers.spec.js index 308918051f..2d4041779f 100644 --- a/tests/functional/spec/features/grand_calculated_summary/grand_calculated_summary_overlapping_answers.spec.js +++ b/tests/functional/spec/features/grand_calculated_summary/grand_calculated_summary_overlapping_answers.spec.js @@ -40,7 +40,7 @@ describe("Feature: Grand Calculated Summary", () => { await click(GrandCalculatedSummaryShoppingPage.submit()); }); - it("Given I edit an answer that is only used in a single calculated summary, I am routed back to the calculated summary and then the grand calculated summary", async () => { + it("Given I edit an answer that is only used in a single calculated summary, I am routed back to the calculated summary and then the grand calculated summary and the correct fields are focused", async () => { await $(HubPage.summaryRowLink("section-3")).click(); await $(GrandCalculatedSummaryShoppingPage.calculatedSummary2Edit()).click(); await $(CalculatedSummary2Page.q1A2Edit()).click(); @@ -49,6 +49,9 @@ describe("Feature: Grand Calculated Summary", () => { // taken back to calculated summary await expect(browser).toHaveUrlContaining(CalculatedSummary2Page.pageName); + await expect(await browser.getUrl()).toContain( + "/questionnaire/calculated-summary-2/?return_to=grand-calculated-summary&return_to_block_id=grand-calculated-summary-shopping&return_to_answer_id=calculated-summary-2#q1-a2", + ); await click(CalculatedSummary2Page.submit()); // then grand calculated summary @@ -56,6 +59,7 @@ describe("Feature: Grand Calculated Summary", () => { await expect(await $(GrandCalculatedSummaryShoppingPage.grandCalculatedSummaryTitle()).getText()).toBe( "Grand Calculated Summary of purchases this week comes to £460.00. Is this correct?.", ); + await expect(await browser.getUrl()).toContain("/questionnaire/grand-calculated-summary-shopping/#calculated-summary-2"); }); it("Given I edit an answer that is used in two calculated summaries, if I edit it from the first calculated summary change link, I taken through each block between the question and the second calculated summary before returning to the grand calculated summary", async () => {