Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for multiple return to answer ids #1242

Merged
merged 28 commits into from
Nov 15, 2023
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
e496d5d
Add router changes
petechd Oct 27, 2023
5a83008
Add assertions in functional test
petechd Oct 30, 2023
b1e9f48
Fix existing test and add linting
petechd Oct 30, 2023
f628280
Fix calculated summary assertions
petechd Oct 30, 2023
2e5cb23
Merge branch 'main' into support-for-multiple-return-to-answer-ids
petechd Oct 31, 2023
3def80c
Fix router type ignores
petechd Oct 31, 2023
4023e9b
Fix return_to_answer_id in url_for
petechd Oct 31, 2023
9772f34
Address comment about return_answer_id on calculated summary
petechd Oct 31, 2023
ae25ff1
Fix issue on question page
petechd Oct 31, 2023
bd97678
Fix multiple answer_ids in url
petechd Nov 1, 2023
5fa0602
Fix mypy issue of nonexistent answer id
petechd Nov 1, 2023
049f036
Add review changes
petechd Nov 3, 2023
4b458e3
Move answer_id method
petechd Nov 3, 2023
4e6bd72
Remove extra check in return_to_answer_id
petechd Nov 6, 2023
f46e493
Add more review fixes
petechd Nov 7, 2023
b2c476f
Add tests
petechd Nov 7, 2023
cdcfec2
Format tests
petechd Nov 7, 2023
e7f82c6
Merge branch 'main' into support-for-multiple-return-to-answer-ids
petechd Nov 7, 2023
b74a31f
Merge branch 'main' into support-for-multiple-return-to-answer-ids
petechd Nov 10, 2023
76e7ea1
Fix existing test
petechd Nov 10, 2023
30b40a7
Merge branch 'support-for-multiple-return-to-answer-ids' of github.co…
petechd Nov 10, 2023
5244328
Add fixes to tests
petechd Nov 10, 2023
24370fb
Merge branch 'main' into support-for-multiple-return-to-answer-ids
petechd Nov 14, 2023
4574d57
Fixes post merge
petechd Nov 15, 2023
e42847f
Merge branch 'main' into support-for-multiple-return-to-answer-ids
petechd Nov 15, 2023
a3ecfd2
Reformat test
petechd Nov 15, 2023
deddc91
Merge branch 'support-for-multiple-return-to-answer-ids' of github.co…
petechd Nov 15, 2023
40c6a40
Fix test parameters
petechd Nov 15, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 9 additions & 4 deletions app/questionnaire/router.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
3 changes: 3 additions & 0 deletions app/views/contexts/calculated_summary_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Expand Down
24 changes: 17 additions & 7 deletions app/views/contexts/summary/answer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
katie-gardner marked this conversation as resolved.
Show resolved Hide resolved
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
143 changes: 135 additions & 8 deletions tests/app/questionnaire/test_router.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
)

Expand All @@ -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
katie-gardner marked this conversation as resolved.
Show resolved Hide resolved

@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.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.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
Expand Down Expand Up @@ -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
Expand All @@ -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",
)

Expand Down Expand Up @@ -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(
Expand All @@ -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
Expand Down Expand Up @@ -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(
Expand All @@ -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
Expand Down
44 changes: 25 additions & 19 deletions tests/app/views/contexts/summary/test_answer.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,24 @@

@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",
[
("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, None),
(None, None, False, None),
("calculated-summary", "total", False, None),
("section-summary", None, True, None),
(None, None, True, None),
("calculated-summary", "total", True, None),
("calculated-summary", "total", True, "calculated-summary-1"),
],
)
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
):
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(
Expand All @@ -37,18 +41,20 @@ 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}"
)
if return_to and return_to_block_id and return_to_answer_id:
berroar marked this conversation as resolved.
Show resolved Hide resolved
query_string = f"?return_to={return_to}&return_to_answer_id={answer.id},{return_to_answer_id}&return_to_block_id={return_to_block_id}"
else:
query_string = ""
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
berroar marked this conversation as resolved.
Show resolved Hide resolved
await $(HubPage.summaryRowLink("section-3")).click();
await $(GrandCalculatedSummaryShoppingPage.calculatedSummary2Edit()).click();
await $(CalculatedSummary2Page.q1A2Edit()).click();
Expand All @@ -49,13 +49,17 @@ 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
await expect(browser).toHaveUrlContaining(GrandCalculatedSummaryShoppingPage.pageName);
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 () => {
Expand Down
Loading