diff --git a/app/questionnaire/placeholder_renderer.py b/app/questionnaire/placeholder_renderer.py index 1309c52a56..70a8315bb0 100644 --- a/app/questionnaire/placeholder_renderer.py +++ b/app/questionnaire/placeholder_renderer.py @@ -1,5 +1,5 @@ from copy import deepcopy -from typing import Any, Mapping, MutableMapping +from typing import Mapping, MutableMapping from jsonpointer import resolve_pointer, set_pointer @@ -124,9 +124,9 @@ def render( """ Transform the current schema json to a fully rendered dictionary """ - data_to_render_mutable: dict[ - str, Any - ] = QuestionnaireSchema.get_mutable_deepcopy(data_to_render) + data_to_render_mutable: dict = QuestionnaireSchema.get_mutable_deepcopy( + data_to_render + ) self._handle_and_resolve_dynamic_answers(data_to_render_mutable) @@ -156,9 +156,7 @@ def render( set_pointer(data_to_render_mutable, pointer, rendered_text) return data_to_render_mutable - def _handle_and_resolve_dynamic_answers( - self, data_to_render_mutable: dict[str, Any] - ) -> None: + def _handle_and_resolve_dynamic_answers(self, data_to_render_mutable: dict) -> None: pointers = find_pointers_containing(data_to_render_mutable, "dynamic_answers") for pointer in pointers: diff --git a/app/questionnaire/questionnaire_schema.py b/app/questionnaire/questionnaire_schema.py index e8488f3c44..cfff180db3 100644 --- a/app/questionnaire/questionnaire_schema.py +++ b/app/questionnaire/questionnaire_schema.py @@ -82,7 +82,8 @@ def __init__( ] = defaultdict(set) self._language_code = language_code self._questionnaire_json = questionnaire_json - self._repeating_block_ids: list[str] = [] + self._list_names_by_list_repeating_block_id: dict[str, str] = {} + self._repeating_block_answer_ids: set[str] = set() self.dynamic_answers_parent_block_ids: set[str] = set() # The ordering here is required as they depend on each other. @@ -92,7 +93,7 @@ def __init__( self._questions_by_id = self._get_questions_by_id() self._answers_by_id = self._get_answers_by_id() self._dynamic_answer_ids: set[str] = set() - self._list_collector_dynamic_answer_dependencies: dict[str, str] = {} + self._list_dependent_block_additional_dependencies: dict[str, set[str]] = {} # Post schema parsing. self._populate_answer_dependencies() @@ -236,6 +237,14 @@ def is_view_submitted_response_enabled(self) -> bool: is_enabled: bool = schema.get("view_response", False) return is_enabled + @cached_property + def list_names_by_list_repeating_block_id(self) -> ImmutableDict[str, str]: + return ImmutableDict(self._list_names_by_list_repeating_block_id) + + @cached_property + def list_collector_repeating_block_ids(self) -> list[str]: + return list(self._list_names_by_list_repeating_block_id.keys()) + def get_all_when_rules_section_dependencies_for_section( self, section_id: str ) -> set[str]: @@ -298,7 +307,9 @@ def _get_blocks_by_id(self) -> dict[str, ImmutableDict]: repeating_block_id = repeating_block["id"] blocks[repeating_block_id] = repeating_block self._parent_id_map[repeating_block_id] = block_id - self._repeating_block_ids.append(repeating_block_id) + self._list_names_by_list_repeating_block_id[ + repeating_block_id + ] = block["for_list"] return blocks @@ -319,10 +330,16 @@ def _get_answers_by_id(self) -> dict[str, list[ImmutableDict]]: for question in self._get_flattened_questions(): question_id = question["id"] + is_for_repeating_block = ( + self._parent_id_map[question_id] + in self.list_collector_repeating_block_ids + ) for answer in get_answers_from_question(question): answer_id = answer["id"] self._parent_id_map[answer_id] = question_id + if is_for_repeating_block: + self._repeating_block_answer_ids.add(answer_id) answers_by_id[answer_id].append(answer) for option in answer.get("options", []): @@ -340,6 +357,9 @@ def _populate_answer_dependencies(self) -> None: self._update_answer_dependencies_for_summary(block) continue + if block["type"] == "ListCollector" and block.get("repeating_blocks"): + self._update_dependencies_for_list_repeating_blocks(block) + for question in self.get_all_questions_for_block(block): self.update_dependencies_for_dynamic_answers( question=question, block_id=block["id"] @@ -361,6 +381,23 @@ def _populate_answer_dependencies(self) -> None: option["detail_answer"], block_id=block["id"] ) + def _update_dependencies_for_list_repeating_blocks( + self, list_collector_block: ImmutableDict + ) -> None: + """Blocks depending on repeating questions may need to depend on adding/removing items from the parent list collector, so update the map""" + list_block_dependencies: set[str] = set() + for child in ("add_block", "remove_block"): + if child_block := list_collector_block.get(child): + list_block_dependencies.update( + self.get_answer_ids_for_block(child_block["id"]) + ) + + if list_block_dependencies: + for repeating_block in list_collector_block["repeating_blocks"]: + self._list_dependent_block_additional_dependencies[ + repeating_block["id"] + ] = list_block_dependencies + def _update_answer_dependencies_for_summary(self, block: ImmutableDict) -> None: if block["type"] == "CalculatedSummary": self._update_answer_dependencies_for_calculated_summary_dependency( @@ -375,32 +412,31 @@ def _update_answer_dependencies_for_calculated_summary_dependency( """ update all calculated summary answers to be dependencies of the dependent block - in the case that one of the calculated summary answers is dynamic, so has multiple answers for a particular list - the calculated summary block needs to depend on the `remove_block` for the list - so that removing items forces user to reconfirm the calculated summary - - but not the add/edit block, as those don't update the total unless the dynamic answers change which it already depends on + in the case that one of the calculated summary answers is dynamic/repeating, so has multiple answers for a particular list + the calculated summary block needs to depend on the `remove_block` and `add_block` for the list + so that adding/removing items requires re-confirming the calculated summary """ calculated_summary_answer_ids = get_calculated_summary_answer_ids( calculated_summary_block ) + answer_dependent = self._get_answer_dependent_for_block_id( + block_id=dependent_block["id"] + ) for answer_id in calculated_summary_answer_ids: - if answer_id in self._dynamic_answer_ids: + if answer_id in [ + *self._dynamic_answer_ids, + *self._repeating_block_answer_ids, + ]: # Type ignore: answer_id is valid so block must exist block_id: str = self.get_block_for_answer_id(answer_id)["id"] # type: ignore - if block_id in self._list_collector_dynamic_answer_dependencies: - remove_block_id = self._list_collector_dynamic_answer_dependencies[ - block_id - ] - self._answer_dependencies_map[remove_block_id] |= { - # note the omission of for_list here is intentional, as the calculated summary is not repeating - self._get_answer_dependent_for_block_id( - block_id=dependent_block["id"] + if block_id in self._list_dependent_block_additional_dependencies: + for ( + list_block_id + ) in self._list_dependent_block_additional_dependencies[block_id]: + self._answer_dependencies_map[list_block_id].add( + answer_dependent ) - } - self._answer_dependencies_map[answer_id] |= { - self._get_answer_dependent_for_block_id(block_id=dependent_block["id"]) - } + self._answer_dependencies_map[answer_id].add(answer_dependent) def _update_answer_dependencies_for_grand_calculated_summary( self, grand_calculated_summary_block: ImmutableDict @@ -502,8 +538,8 @@ def _update_answer_dependencies_for_list_source( ) -> None: """Updates dependencies for a block depending on a list collector - This method also stores a map of { block_depending_on_list_source -> remove_block_for_that_list }, because: - blocks like dynamic_answers, don't directly need to depend on the remove_block, + This method also stores a map of { block_depending_on_list_source -> {add_block, remove_block} }, because: + blocks like dynamic_answers, don't directly need to depend on the add_block/remove_block, but a block depending on the dynamic answers might (such as a calculated summary) """ # Type ignore: section will always exist at this point, same with optional returns below @@ -528,10 +564,14 @@ def _update_answer_dependencies_for_list_source( # non-repeating blocks such as dynamic-answers could depend on the list else self._get_answer_dependent_for_block_id(block_id=block_id) } + self._list_dependent_block_additional_dependencies[block_id] = set( + answer_ids_for_block + ) # removing an item from a list will require any dependent calculated summaries to be re-confirmed, so cache dependencies - if remove_block_question := self.get_remove_block_id_for_list(list_name): - remove_block_id = self.get_first_answer_id_for_block(remove_block_question) - self._list_collector_dynamic_answer_dependencies[block_id] = remove_block_id + if remove_block_id := self.get_remove_block_id_for_list(list_name): + self._list_dependent_block_additional_dependencies[block_id].update( + self.get_answer_ids_for_block(remove_block_id) + ) def _get_answer_dependent_for_block_id( self, @@ -751,9 +791,6 @@ def get_first_block_id_for_section(self, section_id: str) -> str | None: def get_blocks(self) -> Iterable[ImmutableDict]: return self._blocks_by_id.values() - def get_repeating_block_ids(self) -> list[str]: - return self._repeating_block_ids - def get_block(self, block_id: str) -> ImmutableDict | None: return self._blocks_by_id.get(block_id) @@ -778,6 +815,9 @@ def is_answer_in_repeating_section(self, answer_id: str) -> bool | None: def is_answer_dynamic(self, answer_id: str) -> bool: return answer_id in self._dynamic_answer_ids + def is_answer_in_list_collector_repeating_block(self, answer_id: str) -> bool: + return answer_id in self._repeating_block_answer_ids + def get_list_name_for_dynamic_answer(self, block_id: str) -> str: # type ignore block always exists at this point return self.get_block(block_id)["question"]["dynamic_answers"]["values"]["identifier"] # type: ignore @@ -821,6 +861,12 @@ def get_add_block_for_list_collector( ] return add_block + def get_edit_block_for_list_collector( + self, list_collector_id: str + ) -> ImmutableDict | None: + # Type ignore: for any valid list collector id, list collector block will always exist + return self.get_block(list_collector_id).get("edit_block") # type: ignore + def get_repeating_blocks_for_list_collector( self, list_collector_id: str ) -> list[ImmutableDict] | None: @@ -997,11 +1043,12 @@ def is_list_block_type(block_type: str) -> bool: @staticmethod def is_question_block_type(block_type: str) -> bool: - return block_type in [ + return block_type in { "Question", "ListCollectorDrivingQuestion", "ConfirmationQuestion", - ] + "ListRepeatingQuestion", + } @staticmethod def has_address_lookup_answer(question: Mapping) -> bool: @@ -1049,7 +1096,7 @@ def _block_for_answer(self, answer_id: str) -> ImmutableDict | None: if ( parent_block and parent_block["type"] == "ListCollector" - and block_id not in self._repeating_block_ids + and block_id not in self.list_collector_repeating_block_ids ): return parent_block diff --git a/app/questionnaire/questionnaire_store_updater.py b/app/questionnaire/questionnaire_store_updater.py index ea8b2ede3f..ca702943eb 100644 --- a/app/questionnaire/questionnaire_store_updater.py +++ b/app/questionnaire/questionnaire_store_updater.py @@ -165,25 +165,25 @@ def remove_list_item_data(self, list_name: str, list_item_id: str) -> None: def _capture_dependencies_for_list_item_removal(self, list_name: str) -> None: """ - If an item is removed from a list, dependencies of both the add block and remove block need to be captured - and their progress updated. - (E.g. dynamic-answers depending on the add block could have validation, so need revisiting if an item is removed) + If an item is removed from a list, dependencies of any child blocks need to be captured and their progress updated. + (E.g. dynamic-answers depending on a child block could have validation, so need revisiting if an item is removed) """ - if remove_block_id := self._schema.get_remove_block_id_for_list(list_name): - answer_ids = self._schema.get_answer_ids_for_block(remove_block_id) - for answer_id in answer_ids: - self._capture_block_dependencies_for_answer(answer_id) - for list_collector in self._schema.get_list_collectors_for_list( for_list=list_name, # Type ignore: section must exist at this point section=self._schema.get_section(self._current_location.section_id), # type: ignore ): - if add_block := self._schema.get_add_block_for_list_collector( - list_collector["id"] - ): - for answer_id in self._schema.get_answer_ids_for_block(add_block["id"]): - self._capture_block_dependencies_for_answer(answer_id) + child_blocks = ( + list_collector.get("add_block"), + list_collector.get("remove_block"), + *list_collector.get("repeating_blocks", []), + ) + for child_block in child_blocks: + if child_block: + for answer_id in self._schema.get_answer_ids_for_block( + child_block["id"] + ): + self._capture_block_dependencies_for_answer(answer_id) def _get_relationship_answers_for_list_name( self, list_name: str diff --git a/app/questionnaire/schema_utils.py b/app/questionnaire/schema_utils.py index 28d7830ae2..f5da936495 100644 --- a/app/questionnaire/schema_utils.py +++ b/app/questionnaire/schema_utils.py @@ -18,9 +18,7 @@ def find_pointers_containing( if search_key in input_data: yield pointer or "" for k, v in input_data.items(): - if (isinstance(v, dict)) and search_key in v: - yield pointer + "/" + k if pointer else "/" + k - elif isinstance(v, (list, tuple, dict)): + if isinstance(v, (list, tuple, dict)): yield from find_pointers_containing( v, search_key, pointer + "/" + k if pointer else "/" + k ) diff --git a/app/questionnaire/value_source_resolver.py b/app/questionnaire/value_source_resolver.py index 70a7951711..264ce16ee3 100644 --- a/app/questionnaire/value_source_resolver.py +++ b/app/questionnaire/value_source_resolver.py @@ -1,12 +1,16 @@ from dataclasses import dataclass from decimal import Decimal -from typing import Callable, Iterable, Mapping, MutableMapping +from typing import Callable, Iterable, Mapping, MutableMapping, TypeAlias from markupsafe import Markup from werkzeug.datastructures import ImmutableDict from app.data_models import ProgressStore -from app.data_models.answer import AnswerValueTypes, escape_answer_value +from app.data_models.answer import ( + AnswerValueEscapedTypes, + AnswerValueTypes, + escape_answer_value, +) from app.data_models.answer_store import AnswerStore from app.data_models.list_store import ListModel, ListStore from app.data_models.metadata_proxy import MetadataProxy, NoMetadataException @@ -15,9 +19,10 @@ from app.questionnaire.relationship_location import RelationshipLocation from app.questionnaire.rules import rule_evaluator -ValueSourceTypes = None | str | int | Decimal | list -ValueSourceEscapedTypes = Markup | list[Markup] -IntOrDecimal = int | Decimal +ValueSourceTypes: TypeAlias = None | str | int | Decimal | list +ValueSourceEscapedTypes: TypeAlias = Markup | list[Markup] +IntOrDecimal: TypeAlias = int | Decimal +ResolvedAnswerList: TypeAlias = list[AnswerValueTypes | AnswerValueEscapedTypes | None] @dataclass @@ -42,10 +47,19 @@ def _is_answer_on_path(self, answer_id: str) -> bool: return True def _is_block_on_path(self, block_id: str) -> bool: - return ( - self.routing_path_block_ids is not None - and block_id in self.routing_path_block_ids - ) + # other usages of this function than _is_answer_on_path don't have this check so require it here + if not self.routing_path_block_ids: + return True + + if block_id in self.schema.list_collector_repeating_block_ids: + # repeating blocks aren't on the path, so check the parent list collector + list_name = self.schema.list_names_by_list_repeating_block_id[block_id] + # Type ignore: section and list collector will both exist if the block is repeating + section: ImmutableDict = self.schema.get_section_for_block_id(block_id) # type: ignore + list_collector_block: ImmutableDict = self.schema.get_list_collector_for_list(section, list_name) # type: ignore + return list_collector_block["id"] in self.routing_path_block_ids + + return block_id in self.routing_path_block_ids def _get_answer_value( self, @@ -100,36 +114,63 @@ def _resolve_list_item_id_for_value_source( else None ) + def _resolve_repeating_answers_for_list( + self, *, answer_id: str, list_name: str + ) -> ResolvedAnswerList: + """Return the list of answers in answer store that correspond to the given list name and dynamic/repeating answer_id""" + answer_values: ResolvedAnswerList = [] + for list_item_id in self.list_store[list_name]: + answer_value = self._get_answer_value( + answer_id=answer_id, list_item_id=list_item_id + ) + if answer_value is not None: + answer_values.append( + escape_answer_value(answer_value) + if self.escape_answer_values + else answer_value + ) + return answer_values + def _resolve_dynamic_answers( self, answer_id: str, - ) -> list[AnswerValueTypes]: - # Type ignore: the answer block will exist at this stage + ) -> ResolvedAnswerList | None: + # Type ignore: block must exist for this function to be called question = self.schema.get_block_for_answer_id(answer_id).get("question", {}) # type: ignore - answer_values: list[AnswerValueTypes] = [] - - if dynamic_answers := question.get("dynamic_answers"): - values = dynamic_answers["values"] - if values["source"] == "list": - for list_item_id in self.list_store[values["identifier"]]: - if answer_value := self._get_answer_value( - answer_id=answer_id, list_item_id=list_item_id - ): - answer_values.append(answer_value) - return answer_values + dynamic_answers = question["dynamic_answers"] + values = dynamic_answers["values"] + if values["source"] == "list": + return self._resolve_repeating_answers_for_list( + answer_id=answer_id, list_name=values["identifier"] + ) + + def _resolve_list_repeating_block_answers( + self, answer_id: str + ) -> ResolvedAnswerList: + # Type ignore: block must exist for this function to be called + repeating_block: ImmutableDict = self.schema.get_block_for_answer_id(answer_id) # type: ignore + list_name = self.schema.list_names_by_list_repeating_block_id[ + repeating_block["id"] + ] + return self._resolve_repeating_answers_for_list( + answer_id=answer_id, list_name=list_name + ) def _resolve_answer_value_source( self, value_source: Mapping ) -> ValueSourceEscapedTypes | ValueSourceTypes: """resolves answer value by first checking if the answer is dynamic whilst not in a repeating section, - which indicates that it is a dynamic answer resolving to a list. Otherwise, retrieve answer value as normal. + which indicates that it is a repeating answer resolving to a list. Otherwise, retrieve answer value as normal. """ list_item_id = self._resolve_list_item_id_for_value_source(value_source) answer_id = value_source["identifier"] - # if not in a repeating section and the id is for a list of dynamic answers, then return the list of values - if not list_item_id and self.schema.is_answer_dynamic(answer_id): - return self._resolve_dynamic_answers(answer_id) + # if not in a repeating section and the id is for a list of dynamic/repeating block answers, then return the list of values + if not list_item_id: + if self.schema.is_answer_dynamic(answer_id): + return self._resolve_dynamic_answers(answer_id) + if self.schema.is_answer_in_list_collector_repeating_block(answer_id): + return self._resolve_list_repeating_block_answers(answer_id) answer_value = self._get_answer_value( answer_id=answer_id, list_item_id=list_item_id @@ -197,9 +238,7 @@ def _resolve_calculated_summary_value_source( """ calculated_summary_block: ImmutableDict = self.schema.get_block(value_source["identifier"]) # type: ignore - if self.routing_path_block_ids and not self._is_block_on_path( - calculated_summary_block["id"] - ): + if not self._is_block_on_path(calculated_summary_block["id"]): return None calculation = calculated_summary_block["calculation"] diff --git a/app/views/contexts/list_context.py b/app/views/contexts/list_context.py index 9a5f12ba0b..937d577f39 100644 --- a/app/views/contexts/list_context.py +++ b/app/views/contexts/list_context.py @@ -92,14 +92,17 @@ def _build_list_items_context( } if edit_block_id: - if is_primary and primary_person_edit_block_id: - list_item_context["edit_link"] = partial_url_for( - block_id=primary_person_edit_block_id - ) - else: - list_item_context["edit_link"] = partial_url_for( - block_id=edit_block_id - ) + block_id = ( + primary_person_edit_block_id + if is_primary and primary_person_edit_block_id + else edit_block_id + ) + # return to answer id is used to snap back to the appropriate list item when editing from a summary page + # unlike other repeating answers that use answer_id-list_item_id, the edit-block is linked to item-label and anchored by list item id + return_to_answer_id = list_item_id if return_to else None + list_item_context["edit_link"] = partial_url_for( + block_id=block_id, return_to_answer_id=return_to_answer_id + ) if remove_block_id: list_item_context["remove_link"] = partial_url_for( diff --git a/app/views/contexts/section_summary_context.py b/app/views/contexts/section_summary_context.py index 06c4fc392e..a6b8014078 100644 --- a/app/views/contexts/section_summary_context.py +++ b/app/views/contexts/section_summary_context.py @@ -42,7 +42,7 @@ def __init__( def __call__( self, - return_to: Optional[str] = "section-summary", + return_to: str | None = "section-summary", view_submitted_response: bool = False, ) -> Mapping[str, Any]: summary = self.build_summary(return_to, view_submitted_response) @@ -181,6 +181,7 @@ def _custom_summary_elements( schema=self._schema, location=self.current_location, language=self._language, + return_to="section-summary", ) yield list_collector_block.list_summary_element(summary_element) diff --git a/app/views/contexts/summary/answer.py b/app/views/contexts/summary/answer.py index 2c275f3434..54f3bd7769 100644 --- a/app/views/contexts/summary/answer.py +++ b/app/views/contexts/summary/answer.py @@ -1,15 +1,15 @@ -from typing import Any, Mapping, Optional, Union +from typing import Mapping from flask import url_for from app.data_models.answer import AnswerValueEscapedTypes -RadioCheckboxTypes = dict[str, Union[str, AnswerValueEscapedTypes, None]] -DateRangeTypes = dict[str, Optional[AnswerValueEscapedTypes]] +RadioCheckboxTypes = dict[str, str | AnswerValueEscapedTypes | None] +DateRangeTypes = dict[str, AnswerValueEscapedTypes | None] -InferredAnswerValueTypes = Union[ - None, DateRangeTypes, str, AnswerValueEscapedTypes, RadioCheckboxTypes -] +InferredAnswerValueTypes = ( + None | DateRangeTypes | str | AnswerValueEscapedTypes | RadioCheckboxTypes +) class Answer: @@ -19,10 +19,11 @@ def __init__( answer_schema: Mapping[str, str], answer_value: InferredAnswerValueTypes, block_id: str, - list_name: Optional[str], - list_item_id: Optional[str], - return_to: Optional[str], - return_to_block_id: Optional[str], + list_name: str | None, + list_item_id: str | None, + return_to: str | None, + return_to_block_id: str | None, + is_in_repeating_section: bool, ) -> None: self.id = answer_schema["id"] self.label = answer_schema.get("label") @@ -31,15 +32,17 @@ def __init__( self.unit = answer_schema.get("unit") self.unit_length = answer_schema.get("unit_length") self.currency = answer_schema.get("currency") + self._original_answer_id = answer_schema.get("original_answer_id") self.link = self._build_link( block_id=block_id, list_name=list_name, list_item_id=list_item_id, return_to=return_to, return_to_block_id=return_to_block_id, + is_in_repeating_section=is_in_repeating_section, ) - def serialize(self) -> dict[str, Any]: + def serialize(self) -> dict: return { "id": self.id, "label": self.label, @@ -55,10 +58,11 @@ def _build_link( self, *, block_id: str, - list_name: Optional[str], - list_item_id: Optional[str], - return_to: Optional[str], - return_to_block_id: Optional[str], + list_name: str | None, + list_item_id: str | None, + return_to: str | None, + return_to_block_id: str | None, + is_in_repeating_section: bool, ) -> str: return url_for( endpoint="questionnaire.block", @@ -66,7 +70,31 @@ def _build_link( block_id=block_id, list_item_id=list_item_id, return_to=return_to, - return_to_answer_id=self.id if return_to else None, + return_to_answer_id=self._return_to_answer_id( + return_to=return_to, + list_item_id=list_item_id, + is_in_repeating_section=is_in_repeating_section, + ), return_to_block_id=return_to_block_id, _anchor=self.id, ) + + def _return_to_answer_id( + self, + *, + return_to: str | None, + list_item_id: str | None, + is_in_repeating_section: bool, + ) -> 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 + """ + 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 diff --git a/app/views/contexts/summary/block.py b/app/views/contexts/summary/block.py index 13f271a0c4..3d6c27daab 100644 --- a/app/views/contexts/summary/block.py +++ b/app/views/contexts/summary/block.py @@ -1,9 +1,12 @@ -from typing import Any, Mapping, MutableMapping, Optional +from typing import Mapping, MutableMapping + +from jsonpointer import resolve_pointer from app.data_models import AnswerStore, ListStore, ProgressStore from app.data_models.metadata_proxy import MetadataProxy from app.questionnaire import Location, QuestionnaireSchema from app.questionnaire.rules.rule_evaluator import RuleEvaluator +from app.questionnaire.schema_utils import find_pointers_containing from app.questionnaire.value_source_resolver import ValueSourceResolver from app.questionnaire.variants import choose_variant from app.views.contexts.summary.question import Question @@ -12,30 +15,32 @@ class Block: def __init__( self, - block_schema: Mapping[str, Any], + block_schema: Mapping, *, answer_store: AnswerStore, list_store: ListStore, - metadata: Optional[MetadataProxy], + metadata: MetadataProxy | None, response_metadata: MutableMapping, schema: QuestionnaireSchema, location: Location, - return_to: Optional[str], - return_to_block_id: Optional[str] = None, + return_to: str | None, + return_to_block_id: str | None = None, progress_store: ProgressStore, language: str, ) -> None: self.id = block_schema["id"] self.title = block_schema.get("title") self.number = block_schema.get("number") + self.location = location + self.schema = schema self._rule_evaluator = RuleEvaluator( - schema=schema, + schema=self.schema, answer_store=answer_store, list_store=list_store, metadata=metadata, response_metadata=response_metadata, - location=location, + location=self.location, progress_store=progress_store, ) @@ -44,9 +49,9 @@ def __init__( list_store=list_store, metadata=metadata, response_metadata=response_metadata, - schema=schema, - location=location, - list_item_id=location.list_item_id if location else None, + schema=self.schema, + location=self.location, + list_item_id=self.location.list_item_id if self.location else None, use_default_answer=True, progress_store=progress_store, ) @@ -57,8 +62,6 @@ def __init__( list_store=list_store, metadata=metadata, response_metadata=response_metadata, - schema=schema, - location=location, return_to=return_to, return_to_block_id=return_to_block_id, progress_store=progress_store, @@ -68,15 +71,13 @@ def __init__( def get_question( self, *, - block_schema: Mapping[str, Any], + block_schema: Mapping, answer_store: AnswerStore, list_store: ListStore, - metadata: Optional[MetadataProxy], + metadata: MetadataProxy | None, response_metadata: MutableMapping, - schema: QuestionnaireSchema, - location: Location, - return_to: Optional[str], - return_to_block_id: Optional[str], + return_to: str | None, + return_to_block_id: str | None, progress_store: ProgressStore, language: str, ) -> dict[str, Question]: @@ -84,14 +85,14 @@ def get_question( variant = choose_variant( block_schema, - schema, + self.schema, metadata, response_metadata, answer_store, list_store, variants_key="question_variants", single_key="question", - current_location=location, + current_location=self.location, progress_store=progress_store, ) return Question( @@ -99,10 +100,10 @@ def get_question( answer_store=answer_store, list_store=list_store, progress_store=progress_store, - schema=schema, + schema=self.schema, rule_evaluator=self._rule_evaluator, value_source_resolver=self._value_source_resolver, - location=location, + location=self.location, block_id=self.id, return_to=return_to, return_to_block_id=return_to_block_id, @@ -111,10 +112,27 @@ def get_question( language=language, ).serialize() - def serialize(self) -> dict[str, Any]: - return { - "id": self.id, - "title": self.title, - "number": self.number, - "question": self.question, - } + def _handle_id_suffixing(self, block: dict) -> dict: + """ + If the block is repeating but not within a repeating section, summary pages will render it multiple times, once per list item + so the block id, as well as any other ids (e.g. question, answer) need suffixing with list_item_id to ensure the HTML rendered is valid and doesn't + have duplicate div ids + """ + if ( + self.location.list_item_id + and not self.schema.is_block_in_repeating_section(self.id) + ): + for pointer in find_pointers_containing(block, "id"): + data = resolve_pointer(block, pointer) + data["id"] = f"{data['id']}-{self.location.list_item_id}" + return block + + def serialize(self) -> dict: + return self._handle_id_suffixing( + { + "id": self.id, + "title": self.title, + "number": self.number, + "question": self.question, + } + ) diff --git a/app/views/contexts/summary/group.py b/app/views/contexts/summary/group.py index 01be23b42c..b09ef8d422 100644 --- a/app/views/contexts/summary/group.py +++ b/app/views/contexts/summary/group.py @@ -88,6 +88,34 @@ def _build_blocks_and_links( blocks = [] for block in group_schema["blocks"]: + # the block type will only be ListRepeatingQuestion when in the context of a calculated summary or grand calculated summary + # any other summary like section-summary will use the parent list collector instead and render items as part of the ListCollector check further down + if block["type"] == "ListRepeatingQuestion": + # list repeating questions aren't themselves on the path, it's determined by the parent list collector + parent_list_collector_block_id = schema.parent_id_map[block["id"]] + if parent_list_collector_block_id not in routing_path_block_ids: + continue + + list_collector_block = ListCollectorBlock( + routing_path_block_ids=routing_path_block_ids, + answer_store=answer_store, + list_store=list_store, + progress_store=progress_store, + metadata=metadata, + response_metadata=response_metadata, + schema=schema, + location=location, + language=language, + return_to=return_to, + return_to_block_id=return_to_block_id, + ) + repeating_answer_blocks = ( + list_collector_block.get_repeating_block_related_answer_blocks( + block + ) + ) + blocks.extend(repeating_answer_blocks) + if block["id"] not in routing_path_block_ids: continue if block["type"] in [ @@ -151,6 +179,7 @@ def _build_blocks_and_links( schema=schema, location=location, language=language, + return_to=return_to, ) list_summary_element = list_collector_block.list_summary_element( diff --git a/app/views/contexts/summary/list_collector_block.py b/app/views/contexts/summary/list_collector_block.py index 029bddff25..1428e5c885 100644 --- a/app/views/contexts/summary/list_collector_block.py +++ b/app/views/contexts/summary/list_collector_block.py @@ -1,5 +1,5 @@ from collections import defaultdict -from typing import Any, Iterable, Mapping, MutableMapping, Sequence +from typing import Iterable, Mapping, MutableMapping, Sequence from flask import url_for from werkzeug.datastructures import ImmutableDict @@ -25,6 +25,8 @@ def __init__( schema: QuestionnaireSchema, location: Location, language: str, + return_to: str | None, + return_to_block_id: str | None = None, ) -> None: self._location = location self._placeholder_renderer = PlaceholderRenderer( @@ -48,9 +50,11 @@ def __init__( self._response_metadata = response_metadata self._routing_path_block_ids = routing_path_block_ids self._progress_store = progress_store + self._return_to = return_to + self._return_to_block_id = return_to_block_id # pylint: disable=too-many-locals - def list_summary_element(self, summary: Mapping[str, Any]) -> dict[str, Any]: + def list_summary_element(self, summary: Mapping) -> dict: list_collector_block = None ( edit_block_id, @@ -60,7 +64,7 @@ def list_summary_element(self, summary: Mapping[str, Any]) -> dict[str, Any]: item_label, item_anchor, ) = (None, None, None, None, None, None) - current_list = self._list_store[summary["for_list"]] + list_model = self._list_store[summary["for_list"]] list_collector_blocks = list( self._schema.get_list_collectors_for_list( @@ -92,11 +96,13 @@ def list_summary_element(self, summary: Mapping[str, Any]) -> dict[str, Any]: remove_block_id = list_collector_block["remove_block"]["id"] add_link = self._add_link(summary, list_collector_block) repeating_blocks = list_collector_block.get("repeating_blocks", []) - related_answers = self._get_related_answers(current_list, repeating_blocks) - item_anchor = self._schema.get_item_anchor(section_id, current_list.name) - item_label = self._schema.get_item_label(section_id, current_list.name) + related_answers = self._get_related_answer_blocks_by_list_item_id( + list_model=list_model, repeating_blocks=repeating_blocks + ) + item_anchor = self._schema.get_item_anchor(section_id, list_model.name) + item_label = self._schema.get_item_label(section_id, list_model.name) - if len(current_list) == 1 and current_list.primary_person: + if len(list_model) == 1 and list_model.primary_person: if primary_person_block := self._schema.get_list_collector_for_list( self._section, for_list=summary["for_list"], primary=True ): @@ -109,7 +115,7 @@ def list_summary_element(self, summary: Mapping[str, Any]) -> dict[str, Any]: for_list=list_collector_block["for_list"], section_id=self._location.section_id, has_repeating_blocks=bool(list_collector_block.get("repeating_blocks")), - return_to="section-summary", + return_to=self._return_to, edit_block_id=edit_block_id, remove_block_id=remove_block_id, primary_person_edit_block_id=primary_person_edit_block_id, @@ -128,6 +134,23 @@ def list_summary_element(self, summary: Mapping[str, Any]) -> dict[str, Any]: **list_summary_context, } + def get_repeating_block_related_answer_blocks( + self, block: ImmutableDict + ) -> list[dict]: + """ + Given a repeating block question to render, + return the list of rendered question blocks for each list item id + """ + list_name = self._schema.list_names_by_list_repeating_block_id[block["id"]] + list_model = self._list_store[list_name] + blocks: list[dict] = [] + if answer_blocks_by_list_item_id := self._get_related_answer_blocks_by_list_item_id( + list_model=list_model, repeating_blocks=[block] + ): + for answer_blocks in answer_blocks_by_list_item_id.values(): + blocks.extend(answer_blocks) + return blocks + @property def list_context(self) -> ListContext: return ListContext( @@ -142,15 +165,15 @@ def list_context(self) -> ListContext: def _add_link( self, - summary: Mapping[str, Any], - list_collector_block: Mapping[str, Any] | None, + summary: Mapping, + list_collector_block: Mapping | None, ) -> str | None: if list_collector_block: return url_for( "questionnaire.block", list_name=summary["for_list"], block_id=list_collector_block["add_block"]["id"], - return_to="section-summary", + return_to=self._return_to, ) if driving_question_block := self._schema.get_driving_question_for_list( @@ -159,11 +182,11 @@ def _add_link( return url_for( "questionnaire.block", block_id=driving_question_block["id"], - return_to="section-summary", + return_to=self._return_to, ) - def _get_related_answers( - self, list_model: ListModel, repeating_blocks: Sequence[ImmutableDict] + def _get_related_answer_blocks_by_list_item_id( + self, *, list_model: ListModel, repeating_blocks: Sequence[ImmutableDict] ) -> dict[str, list[dict]] | None: section_id = self._section["id"] @@ -186,23 +209,27 @@ def _get_related_answers( for list_id in list_model: serialized_blocks = [ - Block( - block, - answer_store=self._answer_store, - list_store=self._list_store, - metadata=self._metadata, - response_metadata=self._response_metadata, - schema=self._schema, - location=Location( - list_name=list_model.name, - list_item_id=list_id, - section_id=section_id, - ), - return_to="section-summary", - return_to_block_id=None, - progress_store=self._progress_store, - language=self._language, - ).serialize() + # related answers for repeating blocks may use placeholders, so each block needs rendering here + self._placeholder_renderer.render( + data_to_render=Block( + block, + answer_store=self._answer_store, + list_store=self._list_store, + metadata=self._metadata, + response_metadata=self._response_metadata, + schema=self._schema, + location=Location( + list_name=list_model.name, + list_item_id=list_id, + section_id=section_id, + ), + return_to=self._return_to, + return_to_block_id=self._return_to_block_id, + progress_store=self._progress_store, + language=self._language, + ).serialize(), + list_item_id=list_id, + ) for block in blocks ] diff --git a/app/views/contexts/summary/question.py b/app/views/contexts/summary/question.py index 0f16e95e0d..af0e9e29e0 100644 --- a/app/views/contexts/summary/question.py +++ b/app/views/contexts/summary/question.py @@ -55,6 +55,10 @@ def __init__( self.rule_evaluator = rule_evaluator self.value_source_resolver = value_source_resolver + # no need to call the method if no list item id + self._is_in_repeating_section = bool( + self.list_item_id and self.schema.is_block_in_repeating_section(block_id) + ) self.answers = self._build_answers( answer_store=answer_store, @@ -137,6 +141,7 @@ def _build_answers( list_item_id=list_item_id or self.list_item_id, return_to=return_to, return_to_block_id=return_to_block_id, + is_in_repeating_section=self._is_in_repeating_section, ).serialize() summary_answers.append(summary_answer) diff --git a/app/views/handlers/list_action.py b/app/views/handlers/list_action.py index e34add386d..00c0a80cec 100644 --- a/app/views/handlers/list_action.py +++ b/app/views/handlers/list_action.py @@ -18,6 +18,7 @@ def parent_location(self): ) def _get_routing_path(self): + """Only the section id is required, as list collectors won't be in a repeating section""" return self.router.routing_path(section_id=self.parent_location.section_id) def is_location_valid(self): @@ -34,8 +35,8 @@ def is_location_valid(self): return True def get_previous_location_url(self): - if self._is_returning_to_section_summary(): - return self.get_section_summary_url() + if url := self.get_section_or_final_summary_url(): + return url block_id = self._request_args.get("previous") return self._get_location_url( @@ -45,14 +46,26 @@ def get_previous_location_url(self): return_to_block_id=self._return_to_block_id, ) - def get_section_summary_url(self): - return url_for( - "questionnaire.get_section", section_id=self.parent_location.section_id - ) + def get_section_or_final_summary_url(self): + if ( + self._return_to == "section-summary" + and self.router.can_display_section_summary( + self.parent_location.section_id, self.parent_location.list_item_id + ) + ): + return url_for( + "questionnaire.get_section", + section_id=self.parent_location.section_id, + _anchor=self._return_to_answer_id, + ) + if self._return_to == "final-summary" and self.router.is_questionnaire_complete: + return url_for( + "questionnaire.submit_questionnaire", _anchor=self._return_to_answer_id + ) def get_next_location_url(self): - if self._is_returning_to_section_summary(): - return self.get_section_summary_url() + if url := self.get_section_or_final_summary_url(): + return url if self.router.is_block_complete( block_id=self.parent_location.block_id, @@ -80,9 +93,7 @@ def handle_post(self): ) if self.questionnaire_store_updater.is_dirty(): - self._routing_path = self.router.routing_path( - self.current_location.section_id, self.current_location.list_item_id - ) + self._routing_path = self._get_routing_path() self.questionnaire_store_updater.remove_dependent_blocks_and_capture_dependent_sections() self.questionnaire_store_updater.update_progress_for_dependent_sections() self.questionnaire_store_updater.save() @@ -94,6 +105,7 @@ def _get_location_url( return_to=None, return_to_answer_id=None, return_to_block_id=None, + anchor=None, ): if block_id and self._schema.is_block_valid(block_id): section_id = self._schema.get_section_id_for_block_id(block_id) @@ -101,18 +113,12 @@ def _get_location_url( return_to=return_to, return_to_answer_id=return_to_answer_id, return_to_block_id=return_to_block_id, + _anchor=anchor, ) return self.parent_location.url( return_to=return_to, return_to_answer_id=return_to_answer_id, return_to_block_id=return_to_block_id, - ) - - def _is_returning_to_section_summary(self) -> bool: - return ( - self._return_to == "section-summary" - and self.router.can_display_section_summary( - self.parent_location.section_id, self.parent_location.list_item_id - ) + _anchor=anchor, ) diff --git a/app/views/handlers/list_collector.py b/app/views/handlers/list_collector.py index b248dcc291..36bf4c9960 100644 --- a/app/views/handlers/list_collector.py +++ b/app/views/handlers/list_collector.py @@ -33,7 +33,7 @@ def get_next_location_url(self): ) return add_url - if incomplete_block := self.get_first_incomplete_repeating_block_location( + if incomplete_block := self.get_first_incomplete_list_repeating_block_location( repeating_block_ids=self.repeating_block_ids, section_id=self.current_location.section_id, list_name=self.list_name, @@ -89,7 +89,7 @@ def handle_post(self): return super().handle_post() def _is_list_collector_complete(self): - return not self.get_first_incomplete_repeating_block_location( + return not self.get_first_incomplete_list_repeating_block_location( repeating_block_ids=self.repeating_block_ids, section_id=self.current_location.section_id, list_name=self.list_name, diff --git a/app/views/handlers/list_edit_question.py b/app/views/handlers/list_edit_question.py index 1d640135b4..d1b2982429 100644 --- a/app/views/handlers/list_edit_question.py +++ b/app/views/handlers/list_edit_question.py @@ -1,3 +1,5 @@ +from flask import url_for + from app.views.handlers.list_action import ListAction @@ -13,6 +15,31 @@ def is_location_valid(self): return False return True + def get_next_location_url(self): + """ + Unless editing from the summary page, If there are repeating blocks and not all are complete, go to the next one + """ + if url := self.get_section_or_final_summary_url(): + return url + + if first_incomplete_block := self.get_first_incomplete_list_repeating_block_location_for_list_item( + repeating_block_ids=self._schema.list_collector_repeating_block_ids, + section_id=self.current_location.section_id, + list_item_id=self.current_location.list_item_id, + list_name=self.current_location.list_name, + ): + return url_for( + "questionnaire.block", + list_name=first_incomplete_block.list_name, + list_item_id=first_incomplete_block.list_item_id, + block_id=first_incomplete_block.block_id, + return_to=self._return_to, + return_to_answer_id=self._return_to_answer_id, + return_to_block_id=self._return_to_block_id, + ) + + return super().get_next_location_url() + def handle_post(self): # pylint: disable=no-member # wtforms Form parents are not discoverable in the 2.3.3 implementation diff --git a/app/views/handlers/list_repeating_question.py b/app/views/handlers/list_repeating_question.py index 42e2d618f2..c9a302da47 100644 --- a/app/views/handlers/list_repeating_question.py +++ b/app/views/handlers/list_repeating_question.py @@ -2,40 +2,68 @@ from flask import url_for -from app.views.handlers.list_action import ListAction +from app.questionnaire import Location +from app.views.handlers.list_edit_question import ListEditQuestion -class ListRepeatingQuestion(ListAction): +class ListRepeatingQuestion(ListEditQuestion): @cached_property def repeating_block_ids(self) -> list[str]: - return self._schema.get_repeating_block_ids() + return self._schema.list_collector_repeating_block_ids - def get_next_location_url(self) -> str: - if self._is_returning_to_section_summary(): - return self.get_section_summary_url() + def get_previous_location_url(self): + """ + return to previous location, or when return to is None, navigate to the previous repeating block + unless this is the first repeating block, in which case, route back to the edit block + """ + if url := self.get_section_or_final_summary_url(): + return url - if first_incomplete_block := self.get_first_incomplete_repeating_block_location_for_list_item( - repeating_block_ids=self.repeating_block_ids, - section_id=self.current_location.section_id, - list_item_id=self.current_location.list_item_id, - list_name=self.current_location.list_name, + if self.return_to and self.router.can_access_location( + Location( + section_id=self.current_location.section_id, + block_id=self.return_to_block_id, + ), + routing_path=self._routing_path, ): - repeating_block_url = url_for( + return self._get_location_url( + block_id=self._return_to_block_id, + anchor=self._return_to_answer_id, + ) + + repeating_block_index = self.repeating_block_ids.index( + self.current_location.block_id + ) + if repeating_block_index != 0: + previous_repeating_block_id = self.repeating_block_ids[ + repeating_block_index - 1 + ] + return url_for( "questionnaire.block", - list_name=first_incomplete_block.list_name, - list_item_id=first_incomplete_block.list_item_id, - block_id=first_incomplete_block.block_id, + list_name=self.current_location.list_name, + list_item_id=self.current_location.list_item_id, + block_id=previous_repeating_block_id, return_to=self._return_to, return_to_answer_id=self._return_to_answer_id, return_to_block_id=self._return_to_block_id, ) - return repeating_block_url - return super().get_next_location_url() + edit_block = self._schema.get_edit_block_for_list_collector( + self.parent_block["id"] + ) + return url_for( + "questionnaire.block", + list_name=self.current_location.list_name, + list_item_id=self.current_location.list_item_id, + block_id=edit_block["id"], + return_to=self._return_to, + return_to_answer_id=self._return_to_answer_id, + return_to_block_id=self._return_to_block_id, + ) def handle_post(self): self.questionnaire_store_updater.add_completed_location(self.current_location) - if not self.get_first_incomplete_repeating_block_location_for_list_item( + if not self.get_first_incomplete_list_repeating_block_location_for_list_item( repeating_block_ids=self.repeating_block_ids, section_id=self.current_location.section_id, list_item_id=self.current_location.list_item_id, @@ -47,5 +75,4 @@ def handle_post(self): list_item_id=self.current_location.list_item_id, ) - self.questionnaire_store_updater.update_answers(self.form.data) super().handle_post() diff --git a/app/views/handlers/question.py b/app/views/handlers/question.py index cfcfda64be..45339422d3 100644 --- a/app/views/handlers/question.py +++ b/app/views/handlers/question.py @@ -263,7 +263,7 @@ def clear_radio_answers(self): ) self.questionnaire_store_updater.save() - def get_first_incomplete_repeating_block_location( + def get_first_incomplete_list_repeating_block_location( self, *, repeating_block_ids: Sequence[str], section_id: str, list_name: str ) -> Location | None: if not repeating_block_ids: @@ -271,7 +271,7 @@ def get_first_incomplete_repeating_block_location( list_model = self._questionnaire_store.list_store.get(list_name) for list_item_id in list_model.items: - if incomplete_location := self.get_first_incomplete_repeating_block_location_for_list_item( + if incomplete_location := self.get_first_incomplete_list_repeating_block_location_for_list_item( repeating_block_ids=repeating_block_ids, section_id=section_id, list_item_id=list_item_id, @@ -279,7 +279,7 @@ def get_first_incomplete_repeating_block_location( ): return incomplete_location - def get_first_incomplete_repeating_block_location_for_list_item( + def get_first_incomplete_list_repeating_block_location_for_list_item( self, *, repeating_block_ids: Sequence[str], diff --git a/schemas/test/en/test_new_calculated_summary_repeating_blocks.json b/schemas/test/en/test_new_calculated_summary_repeating_blocks.json new file mode 100644 index 0000000000..83f0fef8e5 --- /dev/null +++ b/schemas/test/en/test_new_calculated_summary_repeating_blocks.json @@ -0,0 +1,518 @@ +{ + "mime_type": "application/json/ons/eq", + "language": "en", + "schema_version": "0.0.1", + "data_version": "0.0.3", + "survey_id": "0", + "title": "Calculated Summary of answers from Repeating blocks", + "theme": "default", + "description": "A demo of a calculated summary which uses a answers from the repeating blocks in a list collector.", + "metadata": [ + { + "name": "user_id", + "type": "string" + }, + { + "name": "period_id", + "type": "string" + }, + { + "name": "ru_name", + "type": "string" + } + ], + "questionnaire_flow": { + "type": "Hub", + "options": { + "required_completed_sections": ["section-1"] + } + }, + "sections": [ + { + "id": "section-1", + "title": "Transport", + "summary": { + "show_on_completion": true, + "items": [ + { + "type": "List", + "for_list": "transport", + "title": "transport", + "item_anchor_answer_id": "transport-name", + "item_label": "Name of transport", + "add_link_text": "Add another method of transport", + "empty_list_text": "No method of transport added" + } + ], + "show_non_item_answers": true + }, + "groups": [ + { + "id": "group-1", + "blocks": [ + { + "id": "block-car", + "type": "Question", + "question": { + "id": "question-car", + "type": "General", + "title": "How much do you spend per month travelling by Car?", + "answers": [ + { + "id": "answer-car", + "label": "Monthly expenditure travelling by car", + "mandatory": false, + "type": "Currency", + "currency": "GBP" + } + ] + } + }, + { + "id": "block-skip", + "type": "Question", + "question": { + "id": "question-skip", + "type": "General", + "title": "Would you like to skip the list collector that asks about other methods of transport?", + "guidance": { + "contents": [ + { + "description": "Use this to check the calculated summary shows the correct values when the list collector is not on the path." + } + ] + }, + "answers": [ + { + "id": "answer-skip", + "mandatory": true, + "type": "Radio", + "options": [ + { + "label": "Yes", + "value": "Yes" + }, + { + "label": "No", + "value": "No" + } + ] + } + ] + }, + "routing_rules": [ + { + "block": "list-collector", + "when": { + "==": [ + { + "identifier": "answer-skip", + "source": "answers" + }, + "No" + ] + } + }, + { + "block": "calculated-summary-spending" + } + ] + }, + { + "id": "list-collector", + "type": "ListCollector", + "for_list": "transport", + "question": { + "id": "confirmation-question", + "type": "General", + "title": "Do you use any other methods of transport?", + "answers": [ + { + "id": "list-collector-answer", + "mandatory": true, + "type": "Radio", + "options": [ + { + "label": "Yes", + "value": "Yes", + "action": { + "type": "RedirectToListAddBlock" + } + }, + { + "label": "No", + "value": "No" + } + ] + } + ] + }, + "add_block": { + "id": "add-transport", + "type": "ListAddQuestion", + "cancel_text": "Don’t need to add any other method of transport?", + "question": { + "id": "add-question", + "type": "General", + "title": "What other method of transport do you use?", + "answers": [ + { + "id": "transport-name", + "label": "Transport", + "mandatory": true, + "type": "Dropdown", + "options": [ + { + "label": "Tube", + "value": "Tube" + }, + { + "label": "Bus", + "value": "Bus" + }, + { + "label": "Train", + "value": "Train" + }, + { + "label": "Plane", + "value": "Plane" + } + ] + } + ] + } + }, + "edit_block": { + "id": "edit-transport", + "type": "ListEditQuestion", + "cancel_text": "Don’t need to add any other method of transport?", + "question": { + "id": "add-question", + "type": "General", + "title": "What other method of transport do you use?", + "answers": [ + { + "id": "transport-name", + "label": "Transport", + "mandatory": true, + "type": "Dropdown", + "options": [ + { + "label": "Tube", + "value": "Tube" + }, + { + "label": "Bus", + "value": "Bus" + }, + { + "label": "Train", + "value": "Train" + }, + { + "label": "Plane", + "value": "Plane" + } + ] + } + ] + } + }, + "remove_block": { + "id": "remove-transport", + "type": "ListRemoveQuestion", + "cancel_text": "Don’t need to remove this method of transport?", + "question": { + "id": "remove-question", + "type": "General", + "title": "Are you sure you want to remove this method of transport?", + "warning": "All of the information about this method of transport will be deleted", + "answers": [ + { + "id": "remove-confirmation", + "mandatory": true, + "type": "Radio", + "options": [ + { + "label": "Yes", + "value": "Yes", + "action": { + "type": "RemoveListItemAndAnswers" + } + }, + { + "label": "No", + "value": "No" + } + ] + } + ] + } + }, + "repeating_blocks": [ + { + "id": "transport-repeating-block-1", + "type": "ListRepeatingQuestion", + "question": { + "id": "transport-repeating-block-1-question", + "type": "General", + "title": { + "text": "Give details of your expenditure travelling by {transport_name}", + "placeholders": [ + { + "placeholder": "transport_name", + "value": { + "source": "answers", + "identifier": "transport-name" + } + } + ] + }, + "answers": [ + { + "id": "transport-company", + "label": { + "placeholders": [ + { + "placeholder": "transport_name", + "value": { + "source": "answers", + "identifier": "transport-name" + } + } + ], + "text": "Which company do primarily use for travelling by {transport_name}?" + }, + "mandatory": false, + "type": "TextField" + }, + { + "id": "transport-cost", + "label": { + "placeholders": [ + { + "placeholder": "transport_name", + "value": { + "source": "answers", + "identifier": "transport-name" + } + } + ], + "text": "Monthly season ticket expenditure for travel by {transport_name}" + }, + "mandatory": false, + "type": "Currency", + "currency": "GBP" + }, + { + "id": "transport-additional-cost", + "label": { + "placeholders": [ + { + "placeholder": "transport_name", + "value": { + "source": "answers", + "identifier": "transport-name" + } + } + ], + "text": "Additional monthly expenditure for travel by {transport_name}" + }, + "mandatory": false, + "type": "Currency", + "currency": "GBP" + } + ] + } + }, + { + "id": "transport-repeating-block-2", + "type": "ListRepeatingQuestion", + "question": { + "id": "transport-repeating-block-2-question-1", + "type": "General", + "title": { + "text": "How often do you travel by {transport_name}?", + "placeholders": [ + { + "placeholder": "transport_name", + "value": { + "source": "answers", + "identifier": "transport-name" + } + } + ] + }, + "answers": [ + { + "id": "transport-count", + "label": { + "placeholders": [ + { + "placeholder": "transport_name", + "value": { + "source": "answers", + "identifier": "transport-name" + } + } + ], + "text": "Monthly journeys by {transport_name}" + }, + "mandatory": false, + "type": "Number" + } + ] + } + } + ], + "summary": { + "title": "transport", + "item_title": { + "text": "{transport_name}", + "placeholders": [ + { + "placeholder": "transport_name", + "value": { + "source": "answers", + "identifier": "transport-name" + } + } + ] + } + } + }, + { + "type": "CalculatedSummary", + "id": "calculated-summary-spending", + "title": "We calculate the total monthly expenditure on transport to be %(total)s. Is this correct?", + "calculation": { + "title": "Monthly transport expenditure", + "operation": { + "+": [ + { + "source": "answers", + "identifier": "answer-car" + }, + { + "source": "answers", + "identifier": "transport-cost" + }, + { + "source": "answers", + "identifier": "transport-additional-cost" + } + ] + } + } + }, + { + "type": "CalculatedSummary", + "id": "calculated-summary-count", + "title": "We calculate the total journeys made per month to be %(total)s. Is this correct?", + "calculation": { + "title": "Monthly journeys", + "operation": { + "+": [ + { + "source": "answers", + "identifier": "transport-count" + } + ] + } + }, + "skip_conditions": { + "when": { + "or": [ + { + "==": [ + { + "count": [ + { + "source": "list", + "identifier": "transport" + } + ] + }, + 0 + ] + }, + { + "==": [ + { + "source": "answers", + "identifier": "answer-skip" + }, + "Yes" + ] + } + ] + } + } + } + ] + } + ] + }, + { + "enabled": { + "when": { + ">": [ + { + "source": "calculated_summary", + "identifier": "calculated-summary-count" + }, + 0 + ] + } + }, + "id": "section-2", + "summary": { + "show_on_completion": true + }, + "title": "Travel Details", + "groups": [ + { + "id": "group-2", + "blocks": [ + { + "type": "Question", + "id": "family-journeys", + "question": { + "id": "family-journeys-question", + "title": { + "placeholders": [ + { + "placeholder": "total_journeys", + "value": { + "identifier": "calculated-summary-count", + "source": "calculated_summary" + } + } + ], + "text": "How many of your {total_journeys} journeys are to visit family?" + }, + "type": "General", + "answers": [ + { + "id": "family-journeys-answer", + "label": "Number of trips to visit family", + "mandatory": true, + "description": "Cannot exceed the total journeys from section 1", + "type": "Number", + "maximum": { + "value": { + "source": "calculated_summary", + "identifier": "calculated-summary-count" + } + } + } + ] + } + } + ] + } + ] + } + ] +} diff --git a/tests/app/questionnaire/conftest.py b/tests/app/questionnaire/conftest.py index 8d726e64a5..14d6a7bc28 100644 --- a/tests/app/questionnaire/conftest.py +++ b/tests/app/questionnaire/conftest.py @@ -984,6 +984,9 @@ def mock_schema(mocker): ) ) schema.is_answer_dynamic = mocker.MagicMock(return_value=False) + schema.is_answer_in_list_collector_repeating_block = mocker.MagicMock( + return_value=False + ) return schema @@ -1365,6 +1368,40 @@ def placeholder_transform_question_dynamic_answers_json(): } +@pytest.fixture +@pytest.mark.usefixtures("app", "gb_locale") +def placeholder_transform_question_repeating_block(): + return { + "id": "repeating-block-1", + "type": "ListRepeatingQuestion", + "question": { + "id": "transport-repeating-block-1-question", + "type": "General", + "title": "title", + }, + "answers": [ + { + "id": "transport-cost", + "label": { + "placeholders": [ + { + "placeholder": "transport_name", + "value": { + "source": "answers", + "identifier": "transport-name", + }, + } + ], + "text": "What is your monthly expenditure travelling by {transport_name}?", + }, + "mandatory": True, + "type": "Currency", + "currency": "GBP", + } + ], + } + + @pytest.fixture @pytest.mark.usefixtures("app", "gb_locale") def placeholder_transform_question_dynamic_answers_pointer_json(): diff --git a/tests/app/questionnaire/rules/test_rule_evaluator.py b/tests/app/questionnaire/rules/test_rule_evaluator.py index 7cb23f7217..ba9ee6ceb7 100644 --- a/tests/app/questionnaire/rules/test_rule_evaluator.py +++ b/tests/app/questionnaire/rules/test_rule_evaluator.py @@ -3,7 +3,7 @@ import pytest from freezegun import freeze_time -from mock import Mock +from mock import MagicMock, Mock from app.data_models import AnswerStore, ListStore, ProgressStore from app.data_models.answer import Answer @@ -21,7 +21,7 @@ def get_mock_schema(): - schema = Mock( + schema = MagicMock( QuestionnaireSchema( { "questionnaire_flow": { @@ -32,6 +32,7 @@ def get_mock_schema(): ) ) schema.is_answer_dynamic = Mock(return_value=False) + schema.is_answer_in_list_collector_repeating_block = Mock(return_value=False) return schema @@ -54,6 +55,7 @@ def get_rule_evaluator( schema.is_repeating_answer = Mock(return_value=True) schema.get_default_answer = Mock(return_value=None) schema.is_answer_dynamic = Mock(return_value=False) + schema.is_answer_in_list_collector_repeating_block = Mock(return_value=False) return RuleEvaluator( language=language, diff --git a/tests/app/questionnaire/test_dynamic_answer_options.py b/tests/app/questionnaire/test_dynamic_answer_options.py index 19177c5650..a1b0212559 100644 --- a/tests/app/questionnaire/test_dynamic_answer_options.py +++ b/tests/app/questionnaire/test_dynamic_answer_options.py @@ -118,6 +118,9 @@ def test_dynamic_answer_options_answer_source( mock_schema.get_answers_by_answer_id = mocker.Mock(return_value=answer_schema) mock_schema.get_default_answer = mocker.Mock(return_value=None) mock_schema.is_answer_dynamic = mocker.Mock(return_value=False) + mock_schema.is_answer_in_list_collector_repeating_block = mocker.Mock( + return_value=False + ) if checkbox_answer: value_source_resolver.answer_store.add_or_update( diff --git a/tests/app/questionnaire/test_placeholder_renderer.py b/tests/app/questionnaire/test_placeholder_renderer.py index 08d0fb9927..df87dcc43f 100644 --- a/tests/app/questionnaire/test_placeholder_renderer.py +++ b/tests/app/questionnaire/test_placeholder_renderer.py @@ -358,6 +358,7 @@ def get_placeholder_render( ): schema = MagicMock() schema.is_answer_dynamic = MagicMock(return_value=False) + schema.is_answer_in_list_collector_repeating_block = MagicMock(return_value=False) renderer = PlaceholderRenderer( language=language, answer_store=answer_store, diff --git a/tests/app/questionnaire/test_value_source_resolver.py b/tests/app/questionnaire/test_value_source_resolver.py index 48b4fba7fd..148277310a 100644 --- a/tests/app/questionnaire/test_value_source_resolver.py +++ b/tests/app/questionnaire/test_value_source_resolver.py @@ -1,7 +1,7 @@ from typing import Mapping, Optional, Union import pytest -from mock import Mock +from mock import MagicMock, Mock from app.authentication.auth_payload_versions import AuthPayloadVersion from app.data_models import AnswerStore, ListStore, ProgressStore @@ -20,7 +20,7 @@ def get_list_items(num: int): def get_mock_schema(): - schema = Mock( + schema = MagicMock( QuestionnaireSchema( { "questionnaire_flow": { @@ -31,6 +31,7 @@ def get_mock_schema(): ) ) schema.is_answer_dynamic = Mock(return_value=False) + schema.is_answer_in_list_collector_repeating_block = Mock(return_value=False) return schema @@ -53,6 +54,7 @@ def get_value_source_resolver( schema = get_mock_schema() schema.is_repeating_answer = Mock(return_value=bool(list_item_id)) schema.is_answer_dynamic = Mock(return_value=False) + schema.is_answer_in_list_collector_repeating_block = Mock(return_value=False) if not use_default_answer: schema.get_default_answer = Mock(return_value=None) @@ -344,9 +346,15 @@ def test_answer_source_default_answer(use_default_answer): ) -@pytest.mark.parametrize("answer_values", ([], [10, 5], [100, 200, 300])) +@pytest.mark.parametrize( + "answer_values,escape_answer_values", + (([], False), ([10, 5], False), ([100, 200, 300], False), ([HTML_CONTENT], True)), +) def test_answer_source_dynamic_answer( - mocker, placeholder_transform_question_dynamic_answers_json, answer_values + mocker, + placeholder_transform_question_dynamic_answers_json, + answer_values, + escape_answer_values, ): """ Tests that a dynamic answer id as a value source resolves to the list of answers for that list and question @@ -370,11 +378,50 @@ def test_answer_source_dynamic_answer( ), list_store=ListStore([{"name": "supermarkets", "items": list_item_ids}]), schema=schema, + escape_answer_values=escape_answer_values, ) + expected_result = [ESCAPED_CONTENT] if escape_answer_values else answer_values assert ( value_source_resolver.resolve( {"source": "answers", "identifier": "percentage-of-shopping"} ) + == expected_result + ) + + +@pytest.mark.parametrize("answer_values", ([], [10, 5], [100, 200, 300])) +def test_answer_source_repeating_block_answers( + mocker, placeholder_transform_question_repeating_block, answer_values +): + """ + Tests that an answer id from a repeating block resolves to the list of answers for that list and repeating block question + """ + schema = mocker.MagicMock() + schema.list_names_by_list_repeating_block_id = {"repeating-block-1": "transport"} + schema.is_answer_dynamic = Mock(return_value=False) + schema.is_answer_in_list_collector_repeating_block = Mock(return_value=True) + schema.get_block_for_answer_id = Mock( + return_value=placeholder_transform_question_repeating_block + ) + list_item_ids = get_list_items(len(answer_values)) + value_source_resolver = get_value_source_resolver( + answer_store=AnswerStore( + [ + AnswerDict( + answer_id="transport-cost", + value=value, + list_item_id=list_item_id, + ) + for list_item_id, value in zip(list_item_ids, answer_values) + ] + ), + list_store=ListStore([{"name": "transport", "items": list_item_ids}]), + schema=schema, + ) + assert ( + value_source_resolver.resolve( + {"source": "answers", "identifier": "transport-cost"} + ) == answer_values ) @@ -591,6 +638,7 @@ def test_new_calculated_summary_value_source(mocker, list_item_id): }, ) schema.is_answer_dynamic = Mock(return_value=False) + schema.is_answer_in_list_collector_repeating_block = Mock(return_value=False) location = Location( section_id="test-section", block_id="test-block", list_item_id=list_item_id @@ -647,6 +695,7 @@ def test_new_calculated_summary_nested_value_source(mocker, list_item_id): }, ) schema.is_answer_dynamic = Mock(return_value=False) + schema.is_answer_in_list_collector_repeating_block = Mock(return_value=False) location = Location( section_id="test-section", block_id="test-block", list_item_id=list_item_id diff --git a/tests/app/views/contexts/conftest.py b/tests/app/views/contexts/conftest.py index 8010d48062..6df95e9334 100644 --- a/tests/app/views/contexts/conftest.py +++ b/tests/app/views/contexts/conftest.py @@ -404,6 +404,11 @@ def test_calculated_summary_repeating_and_static_answers_schema(): ) +@pytest.fixture +def test_calculated_summary_repeating_blocks(): + return load_schema_from_name("test_new_calculated_summary_repeating_blocks") + + @pytest.fixture def test_calculated_summary_answers(): answers = [ diff --git a/tests/app/views/contexts/summary/test_answer.py b/tests/app/views/contexts/summary/test_answer.py index b7896a8f95..ab36c64bc9 100644 --- a/tests/app/views/contexts/summary/test_answer.py +++ b/tests/app/views/contexts/summary/test_answer.py @@ -5,14 +5,17 @@ @pytest.mark.usefixtures("app") @pytest.mark.parametrize( - "return_to, return_to_block_id", + "return_to, return_to_block_id, is_in_repeating_section", [ - ("section-summary", None), - (None, None), - ("calculated-summary", "total"), + ("section-summary", None, False), + (None, None, False), + ("calculated-summary", "total", False), + ("section-summary", None, True), + (None, None, True), + ("calculated-summary", "total", True), ], ) -def test_create_answer(return_to, return_to_block_id): +def test_create_answer(return_to, return_to_block_id, is_in_repeating_section): answer = Answer( answer_schema={"id": "answer-id", "label": "Answer Label", "type": "date"}, answer_value="An answer", @@ -21,6 +24,7 @@ def test_create_answer(return_to, return_to_block_id): list_item_id="answer-item-id", return_to=return_to, return_to_block_id=return_to_block_id, + is_in_repeating_section=is_in_repeating_section, ) assert answer.id == "answer-id" @@ -28,10 +32,16 @@ def test_create_answer(return_to, return_to_block_id): 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={answer.id}&return_to_block_id={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={answer.id}" + query_string = ( + f"?return_to={return_to}&return_to_answer_id={return_to_answer_id}" + ) else: query_string = "" @@ -52,6 +62,7 @@ def test_date_answer_type(): list_item_id="answer-item-id", return_to="section-summary", return_to_block_id=None, + is_in_repeating_section=False, ) # Then diff --git a/tests/app/views/contexts/test_calculated_summary_context.py b/tests/app/views/contexts/test_calculated_summary_context.py index 820d079f52..1cab04f550 100644 --- a/tests/app/views/contexts/test_calculated_summary_context.py +++ b/tests/app/views/contexts/test_calculated_summary_context.py @@ -1,6 +1,6 @@ import pytest -from app.data_models import ListStore +from app.data_models import AnswerStore, ListStore from app.questionnaire import Location from app.questionnaire.routing_path import RoutingPath from app.views.contexts.calculated_summary_context import CalculatedSummaryContext @@ -309,3 +309,122 @@ def test_build_view_context_for_calculated_summary_with_dynamic_answers( answers_to_keep = calculation_blocks[0]["question"]["answers"] answer_ids = [answer["id"] for answer in answers_to_keep] assert answer_ids == expected_answer_ids + + # blocks with dynamic answers show each answer suffixed with the list item id, so the anchor needs to also include it + assert all( + answer["link"].endswith( + f"return_to=calculated-summary&return_to_answer_id={answer['id']}&return_to_block_id={block_id}#{answer['id']}" + ) + for answer in answers_to_keep + ) + + +@pytest.mark.usefixtures("app") +@pytest.mark.parametrize( + "block_id,expected_answer_ids,expected_answer_labels,expected_block_ids,anchors", + ( + ( + "calculated-summary-spending", + [ + "answer-car", + "transport-cost-CHKtQS", + "transport-additional-cost-CHKtQS", + "transport-cost-laFWcs", + "transport-additional-cost-laFWcs", + ], + [ + "Monthly expenditure travelling by car", + "Monthly season ticket expenditure for travel by Train", + "Additional monthly expenditure for travel by Train", + "Monthly season ticket expenditure for travel by Bus", + "Additional monthly expenditure for travel by Bus", + ], + [ + "block-car", + "transport-repeating-block-1-CHKtQS", + "transport-repeating-block-1-laFWcs", + ], + [ + "answer-car", + "transport-cost", + "transport-additional-cost", + "transport-cost", + "transport-additional-cost", + ], + ), + ( + "calculated-summary-count", + ["transport-count-CHKtQS", "transport-count-laFWcs"], + ["Monthly journeys by Train", "Monthly journeys by Bus"], + [ + "transport-repeating-block-2-CHKtQS", + "transport-repeating-block-2-laFWcs", + ], + ["transport-count", "transport-count"], + ), + ), +) +def test_build_view_context_for_calculated_summary_with_answers_from_repeating_blocks( + test_calculated_summary_repeating_blocks, + progress_store, + mocker, + block_id, + expected_answer_ids, + expected_answer_labels, + expected_block_ids, + anchors, +): + """ + Tests that when different dynamic answers for the same list are used in different calculated summaries + that the calculated summary context filters the answers to keep correctly. + """ + mocker.patch( + "app.jinja_filters.flask_babel.get_locale", + mocker.MagicMock(return_value="en_GB"), + ) + + block_ids = ["block-car", "list-collector"] + answer_store = AnswerStore( + [ + {"answer_id": "transport-name", "value": "Train", "list_item_id": "CHKtQS"}, + {"answer_id": "transport-name", "value": "Bus", "list_item_id": "laFWcs"}, + ] + ) + + calculated_summary_context = CalculatedSummaryContext( + language="en", + schema=test_calculated_summary_repeating_blocks, + answer_store=answer_store, + list_store=ListStore([{"items": ["CHKtQS", "laFWcs"], "name": "transport"}]), + progress_store=progress_store, + metadata=None, + response_metadata={}, + routing_path=RoutingPath(section_id="section-1", block_ids=block_ids), + current_location=Location(section_id="section-1", block_id=block_id), + ) + + context = calculated_summary_context.build_view_context() + assert "summary" in context + assert_summary_context(context) + context_summary = context["summary"] + calculation_blocks = context_summary["sections"][0]["groups"][0]["blocks"] + + block_ids = [block["id"] for block in calculation_blocks] + assert block_ids == expected_block_ids + + questions = [block["question"] for block in calculation_blocks] + answers = [answer for question in questions for answer in question["answers"]] + answer_ids = [answer["id"] for answer in answers] + assert answer_ids == expected_answer_ids + + answer_labels = [answer["label"] for answer in answers] + assert answer_labels == expected_answer_labels + + # on summary pages, repeating block answer ids are suffixed with list item ids, + # but the anchor on the change links needs to not have them, because the repeating block itself doesn't do that + assert all( + answer["link"].endswith( + f"return_to=calculated-summary&return_to_answer_id={answer['id']}&return_to_block_id={block_id}#{anchor}" + ) + for anchor, answer in zip(anchors, answers) + ) diff --git a/tests/app/views/contexts/test_section_summary_context.py b/tests/app/views/contexts/test_section_summary_context.py index 9ba480601d..8c64cfbc7d 100644 --- a/tests/app/views/contexts/test_section_summary_context.py +++ b/tests/app/views/contexts/test_section_summary_context.py @@ -167,7 +167,7 @@ def test_context_for_section_list_summary(people_answer_store, progress_store): "editable": True, "list_items": [ { - "edit_link": "/questionnaire/people/PlwgoG/edit-person/?return_to=section-summary", + "edit_link": "/questionnaire/people/PlwgoG/edit-person/?return_to=section-summary&return_to_answer_id=PlwgoG", "item_title": "Toni Morrison", "list_item_id": "PlwgoG", "primary_person": False, @@ -176,7 +176,7 @@ def test_context_for_section_list_summary(people_answer_store, progress_store): "repeating_blocks": False, }, { - "edit_link": "/questionnaire/people/UHPLbX/edit-person/?return_to=section-summary", + "edit_link": "/questionnaire/people/UHPLbX/edit-person/?return_to=section-summary&return_to_answer_id=UHPLbX", "item_title": "Barry Pheloung", "list_item_id": "UHPLbX", "primary_person": False, @@ -201,7 +201,7 @@ def test_context_for_section_list_summary(people_answer_store, progress_store): "editable": True, "list_items": [ { - "edit_link": "/questionnaire/visitors/gTrlio/edit-visitor-person/?return_to=section-summary", + "edit_link": "/questionnaire/visitors/gTrlio/edit-visitor-person/?return_to=section-summary&return_to_answer_id=gTrlio", "item_title": "", "list_item_id": "gTrlio", "primary_person": False, @@ -227,390 +227,48 @@ def test_context_for_section_list_summary(people_answer_store, progress_store): # pylint: disable=line-too-long -@pytest.mark.usefixtures("app") -def test_context_for_section_summary_with_list_summary( - companies_answer_store, progress_store -): - schema = load_schema_from_name("test_list_collector_section_summary") - - summary_context = SectionSummaryContext( - language=DEFAULT_LANGUAGE_CODE, - schema=schema, - answer_store=companies_answer_store, - list_store=ListStore( - [ - {"items": ["PlwgoG", "UHPLbX"], "name": "companies"}, - ] - ), - progress_store=progress_store, - metadata=None, - response_metadata={}, - current_location=Location(section_id="section-companies"), - routing_path=RoutingPath( - [ - "any-other-companies-or-branches", - ], - section_id="section-companies", +@pytest.mark.parametrize( + "test_schema, answer_store_fixture, item_label, answer_1_label, answer_2_label", + [ + ( + "test_list_collector_section_summary", + "companies_answer_store", + "Name of UK company or branch", + "Registration number", + "Is this UK company or branch an authorised insurer?", ), - ) - context = summary_context() - expected = { - "summary": { - "answers_are_editable": True, - "collapsible": False, - "sections": [ - { - "groups": [ - { - "blocks": [], - "id": "group-companies-0", - "links": {}, - "placeholder_text": None, - "title": None, - }, - { - "blocks": [ - { - "add_link": "/questionnaire/companies/add-company/?return_to=section-summary", - "add_link_text": "Add another UK company or branch", - "empty_list_text": "No UK company or branch added", - "item_anchor": "#company-or-branch-name", - "item_label": "Name of UK company or branch", - "list": { - "editable": True, - "list_items": [ - { - "edit_link": "/questionnaire/companies/PlwgoG/edit-company/?return_to=section-summary", - "item_title": "company a", - "list_item_id": "PlwgoG", - "primary_person": False, - "remove_link": "/questionnaire/companies/PlwgoG/remove-company/?return_to=section-summary", - "is_complete": False, - "repeating_blocks": False, - }, - { - "edit_link": "/questionnaire/companies/UHPLbX/edit-company/?return_to=section-summary", - "item_title": "company b", - "list_item_id": "UHPLbX", - "primary_person": False, - "remove_link": "/questionnaire/companies/UHPLbX/remove-company/?return_to=section-summary", - "is_complete": False, - "repeating_blocks": False, - }, - ], - }, - "list_name": "companies", - "related_answers": { - "PlwgoG": [ - { - "id": "edit-company", - "number": None, - "question": { - "answers": [ - { - "currency": None, - "id": "registration-number", - "label": "Registration number", - "link": "/questionnaire/companies/PlwgoG/edit-company/?return_to=section-summary&return_to_answer_id=registration-number#registration-number", - "type": "number", - "unit": None, - "unit_length": None, - "value": 123, - }, - { - "currency": None, - "id": "authorised-insurer-radio", - "label": "Is this UK company or branch an authorised insurer?", - "link": "/questionnaire/companies/PlwgoG/edit-company/?return_to=section-summary&return_to_answer_id=authorised-insurer-radio#authorised-insurer-radio", - "type": "radio", - "unit": None, - "unit_length": None, - "value": { - "detail_answer_value": None, - "label": "Yes", - }, - }, - ], - "id": "edit-question-companies", - "number": None, - "title": "What is the name of the company?", - "type": "General", - }, - "title": None, - } - ], - "UHPLbX": [ - { - "id": "edit-company", - "number": None, - "question": { - "answers": [ - { - "currency": None, - "id": "registration-number", - "label": "Registration number", - "link": "/questionnaire/companies/UHPLbX/edit-company/?return_to=section-summary&return_to_answer_id=registration-number#registration-number", - "type": "number", - "unit": None, - "unit_length": None, - "value": 456, - }, - { - "currency": None, - "id": "authorised-insurer-radio", - "label": "Is this UK company or branch an authorised insurer?", - "link": "/questionnaire/companies/UHPLbX/edit-company/?return_to=section-summary&return_to_answer_id=authorised-insurer-radio#authorised-insurer-radio", - "type": "radio", - "unit": None, - "unit_length": None, - "value": { - "detail_answer_value": None, - "label": "No", - }, - }, - ], - "id": "edit-question-companies", - "number": None, - "title": "What is the name of the company?", - "type": "General", - }, - "title": None, - } - ], - }, - "title": "Companies or UK branches", - "type": "List", - } - ], - "id": "group-companies-1", - "links": { - "add_link": Link( - text="Add another UK company or branch", - url="/questionnaire/companies/add-company/?return_to=section-summary", - target="_self", - attributes={"data-qa": "add-item-link"}, - ) - }, - "placeholder_text": "No UK company or branch added", - "title": None, - }, - { - "blocks": [], - "id": "group-companies-2", - "links": {}, - "placeholder_text": None, - "title": None, - }, - ], - "title": "General insurance business", - } - ], - "page_title": "General insurance business", - "summary_type": "SectionSummary", - "title": "General insurance business", - } - } - - assert context == expected - - -@pytest.mark.usefixtures("app") -def test_context_for_section_summary_with_list_summary_and_first_variant( - companies_variants_answer_store_first_variant, progress_store -): - schema = load_schema_from_name("test_list_collector_variants_section_summary") - - summary_context = SectionSummaryContext( - language=DEFAULT_LANGUAGE_CODE, - schema=schema, - answer_store=companies_variants_answer_store_first_variant, - list_store=ListStore( - [ - {"items": ["PlwgoG", "UHPLbX"], "name": "companies"}, - ] + ( + "test_list_collector_variants_section_summary", + "companies_variants_answer_store_first_variant", + "Name of UK or non-UK company or branch", + "UK Registration number", + "Is this UK company or branch an authorised insurer?", ), - progress_store=progress_store, - metadata=None, - response_metadata={}, - current_location=Location(section_id="section-companies"), - routing_path=RoutingPath( - [ - "any-other-companies-or-branches", - ], - section_id="section-companies", + ( + "test_list_collector_variants_section_summary", + "companies_variants_answer_store_second_variant", + "Name of UK or non-UK company or branch", + "Non-UK Registration number", + "Is this non-UK company or branch an authorised insurer?", ), - ) - context = summary_context() - expected = { - "summary": { - "answers_are_editable": True, - "collapsible": False, - "sections": [ - { - "groups": [ - { - "blocks": [], - "id": "group-companies-0", - "links": {}, - "placeholder_text": None, - "title": None, - }, - { - "blocks": [ - { - "add_link": "/questionnaire/companies/add-company/?return_to=section-summary", - "add_link_text": "Add another UK company or branch", - "empty_list_text": "No UK company or branch added", - "item_anchor": "#company-or-branch-name", - "item_label": "Name of UK or non-UK company or branch", - "list": { - "editable": True, - "list_items": [ - { - "edit_link": "/questionnaire/companies/PlwgoG/edit-company/?return_to=section-summary", - "item_title": "company a", - "list_item_id": "PlwgoG", - "primary_person": False, - "remove_link": "/questionnaire/companies/PlwgoG/remove-company/?return_to=section-summary", - "is_complete": False, - "repeating_blocks": False, - }, - { - "edit_link": "/questionnaire/companies/UHPLbX/edit-company/?return_to=section-summary", - "item_title": "company b", - "list_item_id": "UHPLbX", - "primary_person": False, - "remove_link": "/questionnaire/companies/UHPLbX/remove-company/?return_to=section-summary", - "is_complete": False, - "repeating_blocks": False, - }, - ], - }, - "list_name": "companies", - "related_answers": { - "PlwgoG": [ - { - "id": "edit-company", - "number": None, - "question": { - "answers": [ - { - "currency": None, - "id": "registration-number", - "label": "UK Registration number", - "link": "/questionnaire/companies/PlwgoG/edit-company/?return_to=section-summary&return_to_answer_id=registration-number#registration-number", - "type": "number", - "unit": None, - "unit_length": None, - "value": 123, - }, - { - "currency": None, - "id": "authorised-insurer-radio", - "label": "Is this UK company or branch an authorised insurer?", - "link": "/questionnaire/companies/PlwgoG/edit-company/?return_to=section-summary&return_to_answer_id=authorised-insurer-radio#authorised-insurer-radio", - "type": "radio", - "unit": None, - "unit_length": None, - "value": { - "detail_answer_value": None, - "label": "Yes", - }, - }, - ], - "id": "edit-question-companies", - "number": None, - "title": "What is the name of the company?", - "type": "General", - }, - "title": None, - } - ], - "UHPLbX": [ - { - "id": "edit-company", - "number": None, - "question": { - "answers": [ - { - "currency": None, - "id": "registration-number", - "label": "UK Registration number", - "link": "/questionnaire/companies/UHPLbX/edit-company/?return_to=section-summary&return_to_answer_id=registration-number#registration-number", - "type": "number", - "unit": None, - "unit_length": None, - "value": 456, - }, - { - "currency": None, - "id": "authorised-insurer-radio", - "label": "Is this UK company or branch an authorised insurer?", - "link": "/questionnaire/companies/UHPLbX/edit-company/?return_to=section-summary&return_to_answer_id=authorised-insurer-radio#authorised-insurer-radio", - "type": "radio", - "unit": None, - "unit_length": None, - "value": { - "detail_answer_value": None, - "label": "No", - }, - }, - ], - "id": "edit-question-companies", - "number": None, - "title": "What is the name of the company?", - "type": "General", - }, - "title": None, - } - ], - }, - "title": "Companies or UK branches", - "type": "List", - } - ], - "id": "group-companies-1", - "links": { - "add_link": Link( - text="Add another UK company or branch", - url="/questionnaire/companies/add-company/?return_to=section-summary", - target="_self", - attributes={"data-qa": "add-item-link"}, - ) - }, - "placeholder_text": "No UK company or branch added", - "title": None, - }, - { - "blocks": [], - "id": "group-companies-2", - "links": {}, - "placeholder_text": None, - "title": None, - }, - ], - "title": "General insurance business", - } - ], - "page_title": "General insurance business", - "summary_type": "SectionSummary", - "title": "General insurance business", - } - } - - assert context == expected - - + ], +) @pytest.mark.usefixtures("app") -def test_context_for_section_summary_with_list_summary_and_second_variant( - companies_variants_answer_store_second_variant, progress_store +def test_context_for_section_summary_with_list_summary_and_first_variant( + test_schema, + answer_store_fixture, + item_label, + answer_1_label, + answer_2_label, + progress_store, + request, ): - schema = load_schema_from_name("test_list_collector_variants_section_summary") - + schema = load_schema_from_name(test_schema) + answer_store = request.getfixturevalue(answer_store_fixture) summary_context = SectionSummaryContext( language=DEFAULT_LANGUAGE_CODE, schema=schema, - answer_store=companies_variants_answer_store_second_variant, + answer_store=answer_store, list_store=ListStore( [ {"items": ["PlwgoG", "UHPLbX"], "name": "companies"}, @@ -649,12 +307,12 @@ def test_context_for_section_summary_with_list_summary_and_second_variant( "add_link_text": "Add another UK company or branch", "empty_list_text": "No UK company or branch added", "item_anchor": "#company-or-branch-name", - "item_label": "Name of UK or non-UK company or branch", + "item_label": item_label, "list": { "editable": True, "list_items": [ { - "edit_link": "/questionnaire/companies/PlwgoG/edit-company/?return_to=section-summary", + "edit_link": "/questionnaire/companies/PlwgoG/edit-company/?return_to=section-summary&return_to_answer_id=PlwgoG", "item_title": "company a", "list_item_id": "PlwgoG", "primary_person": False, @@ -663,7 +321,7 @@ def test_context_for_section_summary_with_list_summary_and_second_variant( "repeating_blocks": False, }, { - "edit_link": "/questionnaire/companies/UHPLbX/edit-company/?return_to=section-summary", + "edit_link": "/questionnaire/companies/UHPLbX/edit-company/?return_to=section-summary&return_to_answer_id=UHPLbX", "item_title": "company b", "list_item_id": "UHPLbX", "primary_person": False, @@ -677,15 +335,15 @@ def test_context_for_section_summary_with_list_summary_and_second_variant( "related_answers": { "PlwgoG": [ { - "id": "edit-company", + "id": "edit-company-PlwgoG", "number": None, "question": { "answers": [ { "currency": None, - "id": "registration-number", - "label": "Non-UK Registration number", - "link": "/questionnaire/companies/PlwgoG/edit-company/?return_to=section-summary&return_to_answer_id=registration-number#registration-number", + "id": "registration-number-PlwgoG", + "label": answer_1_label, + "link": "/questionnaire/companies/PlwgoG/edit-company/?return_to=section-summary&return_to_answer_id=registration-number-PlwgoG#registration-number", "type": "number", "unit": None, "unit_length": None, @@ -693,9 +351,9 @@ def test_context_for_section_summary_with_list_summary_and_second_variant( }, { "currency": None, - "id": "authorised-insurer-radio", - "label": "Is this non-UK company or branch an authorised insurer?", - "link": "/questionnaire/companies/PlwgoG/edit-company/?return_to=section-summary&return_to_answer_id=authorised-insurer-radio#authorised-insurer-radio", + "id": "authorised-insurer-radio-PlwgoG", + "label": answer_2_label, + "link": "/questionnaire/companies/PlwgoG/edit-company/?return_to=section-summary&return_to_answer_id=authorised-insurer-radio-PlwgoG#authorised-insurer-radio", "type": "radio", "unit": None, "unit_length": None, @@ -705,7 +363,7 @@ def test_context_for_section_summary_with_list_summary_and_second_variant( }, }, ], - "id": "edit-question-companies", + "id": "edit-question-companies-PlwgoG", "number": None, "title": "What is the name of the company?", "type": "General", @@ -715,15 +373,15 @@ def test_context_for_section_summary_with_list_summary_and_second_variant( ], "UHPLbX": [ { - "id": "edit-company", + "id": "edit-company-UHPLbX", "number": None, "question": { "answers": [ { "currency": None, - "id": "registration-number", - "label": "Non-UK Registration number", - "link": "/questionnaire/companies/UHPLbX/edit-company/?return_to=section-summary&return_to_answer_id=registration-number#registration-number", + "id": "registration-number-UHPLbX", + "label": answer_1_label, + "link": "/questionnaire/companies/UHPLbX/edit-company/?return_to=section-summary&return_to_answer_id=registration-number-UHPLbX#registration-number", "type": "number", "unit": None, "unit_length": None, @@ -731,9 +389,9 @@ def test_context_for_section_summary_with_list_summary_and_second_variant( }, { "currency": None, - "id": "authorised-insurer-radio", - "label": "Is this non-UK company or branch an authorised insurer?", - "link": "/questionnaire/companies/UHPLbX/edit-company/?return_to=section-summary&return_to_answer_id=authorised-insurer-radio#authorised-insurer-radio", + "id": "authorised-insurer-radio-UHPLbX", + "label": answer_2_label, + "link": "/questionnaire/companies/UHPLbX/edit-company/?return_to=section-summary&return_to_answer_id=authorised-insurer-radio-UHPLbX#authorised-insurer-radio", "type": "radio", "unit": None, "unit_length": None, @@ -743,7 +401,7 @@ def test_context_for_section_summary_with_list_summary_and_second_variant( }, }, ], - "id": "edit-question-companies", + "id": "edit-question-companies-UHPLbX", "number": None, "title": "What is the name of the company?", "type": "General", @@ -876,7 +534,7 @@ def test_context_for_driving_question_summary(progress_store): "editable": True, "list_items": [ { - "edit_link": "/questionnaire/people/PlwgoG/edit-person/?return_to=section-summary", + "edit_link": "/questionnaire/people/PlwgoG/edit-person/?return_to=section-summary&return_to_answer_id=PlwgoG", "item_title": "Toni Morrison", "list_item_id": "PlwgoG", "primary_person": False, diff --git a/tests/functional/base_pages/calculated-summary.page.js b/tests/functional/base_pages/calculated-summary.page.js index e147911349..7ee638b426 100644 --- a/tests/functional/base_pages/calculated-summary.page.js +++ b/tests/functional/base_pages/calculated-summary.page.js @@ -12,6 +12,10 @@ class CalculatedSummaryPage extends BasePage { calculatedSummaryAnswer() { return "[data-qa=calculated-summary-answer]"; } + + summaryItems() { + return "div.ons-summary__items"; + } } export default CalculatedSummaryPage; diff --git a/tests/functional/helpers.js b/tests/functional/helpers.js index 9a4d1dd3b0..9261798965 100644 --- a/tests/functional/helpers.js +++ b/tests/functional/helpers.js @@ -12,3 +12,21 @@ export const checkListItemComplete = async (listItemLabel) => { export const checkListItemIncomplete = async (listItemLabel) => { await expect(await $(listItemLabel).$(`.ons-summary__item-title-icon.ons-summary__item-title-icon--check`).isExisting()).to.be.false; }; + +export const assertSummaryValues = async (values) => { + // check each summary value provided is present and that the number of them matches what is on the page + // needs to include both dynamic and static answers on any summary with both + const summaryValues = 'dd[class="ons-summary__values"]'; + await values.map(async (value, index) => { + await expect(await $$(summaryValues)[index].getText()).to.equal(value); + }); + await expect(await $$(summaryValues).length).to.equal(values.length); +}; + +export const repeatingAnswerChangeLink = (answerIndex) => { + return $$('dd[class="ons-summary__actions"]')[answerIndex].$("a"); +}; + +export const listItemIds = () => { + return $$("[data-list-item-id]").map((listItem) => listItem.getAttribute("data-list-item-id")); +}; diff --git a/tests/functional/spec/features/calculated_summary/new_calculated_summary_repeating_and_static_answers.spec.js b/tests/functional/spec/features/calculated_summary/new_calculated_summary_repeating_and_static_answers.spec.js index a0c9a2dfc1..6fa1c2ffec 100644 --- a/tests/functional/spec/features/calculated_summary/new_calculated_summary_repeating_and_static_answers.spec.js +++ b/tests/functional/spec/features/calculated_summary/new_calculated_summary_repeating_and_static_answers.spec.js @@ -12,21 +12,10 @@ import ListCollectorRemovePage from "../../../generated_pages/new_calculated_sum import SupermarketTransportPage from "../../../generated_pages/new_calculated_summary_repeating_and_static_answers/supermarket-transport.page"; import SupermarketTransportCostPage from "../../../generated_pages/new_calculated_summary_repeating_and_static_answers/supermarket-transport-cost.page"; import CalculatedSummaryPipingPage from "../../../generated_pages/new_calculated_summary_repeating_and_static_answers/calculated-summary-piping.page"; +import { assertSummaryValues } from "../../../helpers"; describe("Calculated summary with repeating answers", () => { - const group = 'div[id="group"]'; - const summaryValues = 'dd[class="ons-summary__values"]'; const summaryActions = 'dd[class="ons-summary__actions"]'; - - const assertSummaryValues = async (values) => { - // check each summary value provided is present and that the number of them matches what is on the page - // needs to include both dynamic and static answers on any summary with both - await values.map(async (value, index) => { - await expect(await $(group).$$(summaryValues)[index].getText()).to.equal(value); - }); - await expect(await $(group).$$(summaryValues).length).to.equal(values.length); - }; - const dynamicAnswerChangeLink = (answerIndex) => $$(summaryActions)[answerIndex].$("a"); before("Completing the list collector and dynamic answer", async () => { @@ -65,7 +54,6 @@ describe("Calculated summary with repeating answers", () => { "We calculate the total visits to the shop to be 6. Is this correct?" ); await assertSummaryValues(["4", "2"]); - await expect(await $(group).$$(summaryValues).length).to.equal(2); }); it("Given I click on a change link, when I use the previous button, I return to the calculated summary", async () => { diff --git a/tests/functional/spec/features/calculated_summary/new_calculated_summary_repeating_blocks.spec.js b/tests/functional/spec/features/calculated_summary/new_calculated_summary_repeating_blocks.spec.js new file mode 100644 index 0000000000..13abc8836b --- /dev/null +++ b/tests/functional/spec/features/calculated_summary/new_calculated_summary_repeating_blocks.spec.js @@ -0,0 +1,236 @@ +import SectionOnePage from "../../../generated_pages/new_calculated_summary_repeating_blocks/section-1-summary.page.js"; +import SectionTwoPage from "../../../generated_pages/new_calculated_summary_repeating_blocks/section-2-summary.page.js"; +import BlockCarPage from "../../../generated_pages/new_calculated_summary_repeating_blocks/block-car.page.js"; +import AddTransportPage from "../../../generated_pages/new_calculated_summary_repeating_blocks/list-collector-add.page.js"; +import RemoveTransportPage from "../../../generated_pages/new_calculated_summary_repeating_blocks/list-collector-remove.page.js"; +import TransportRepeatingBlock1Page from "../../../generated_pages/new_calculated_summary_repeating_blocks/transport-repeating-block-1-repeating-block.page.js"; +import TransportRepeatingBlock2Page from "../../../generated_pages/new_calculated_summary_repeating_blocks/transport-repeating-block-2-repeating-block.page.js"; +import ListCollectorPage from "../../../generated_pages/new_calculated_summary_repeating_blocks/list-collector.page.js"; +import CalculatedSummarySpendingPage from "../../../generated_pages/new_calculated_summary_repeating_blocks/calculated-summary-spending.page.js"; +import CalculatedSummaryCountPage from "../../../generated_pages/new_calculated_summary_repeating_blocks/calculated-summary-count.page.js"; +import HubPage from "../../../base_pages/hub.page"; +import FamilyJourneysPage from "../../../generated_pages/new_calculated_summary_repeating_blocks/family-journeys.page"; +import BlockSkipPage from "../../../generated_pages/new_calculated_summary_repeating_blocks/block-skip.page"; +import { assertSummaryValues, repeatingAnswerChangeLink } from "../../../helpers"; + +describe("Feature: Calculated Summary using Repeating Blocks", () => { + before("Reaching the first calculated summary", async () => { + await browser.openQuestionnaire("test_new_calculated_summary_repeating_blocks.json"); + await $(BlockCarPage.car()).setValue(100); + await $(BlockCarPage.submit()).click(); + await $(BlockSkipPage.no()).click(); + await $(BlockSkipPage.submit()).click(); + await $(ListCollectorPage.yes()).click(); + await $(ListCollectorPage.submit()).click(); + await $(AddTransportPage.transportName()).selectByAttribute("value", "Bus"); + await $(AddTransportPage.submit()).click(); + await $(TransportRepeatingBlock1Page.transportCompany()).setValue("First"); + await $(TransportRepeatingBlock1Page.transportCost()).setValue(30); + await $(TransportRepeatingBlock1Page.transportAdditionalCost()).setValue(5); + await $(TransportRepeatingBlock1Page.submit()).click(); + await $(TransportRepeatingBlock2Page.transportCount()).setValue(10); + await $(TransportRepeatingBlock2Page.submit()).click(); + await $(ListCollectorPage.yes()).click(); + await $(ListCollectorPage.submit()).click(); + await $(AddTransportPage.transportName()).selectByAttribute("value", "Plane"); + await $(AddTransportPage.submit()).click(); + await $(TransportRepeatingBlock1Page.transportCompany()).setValue("EasyJet"); + await $(TransportRepeatingBlock1Page.transportCost()).setValue(0); + await $(TransportRepeatingBlock1Page.transportAdditionalCost()).setValue(265); + await $(TransportRepeatingBlock1Page.submit()).click(); + await $(TransportRepeatingBlock2Page.transportCount()).setValue(2); + await $(TransportRepeatingBlock2Page.submit()).click(); + await $(ListCollectorPage.no()).click(); + await $(ListCollectorPage.submit()).click(); + }); + + it("Given I have a calculated summary using both list repeating block and static answers, When I reach the calculated summary page, Then I see the correct items and total.", async () => { + await expect(await $(CalculatedSummarySpendingPage.calculatedSummaryTitle()).getText()).to.contain( + "We calculate the total monthly expenditure on transport to be £400.00. Is this correct?" + ); + await assertSummaryValues(["£100.00", "£30.00", "£5.00", "£0.00", "£265.00"]); + await expect(await $(CalculatedSummarySpendingPage.summaryItems()).getText()).to.contain("Monthly expenditure travelling by car"); + await expect(await $(CalculatedSummarySpendingPage.summaryItems()).getText()).to.contain("Monthly season ticket expenditure for travel by Bus"); + await expect(await $(CalculatedSummarySpendingPage.summaryItems()).getText()).to.contain("Additional monthly expenditure for travel by Bus"); + await expect(await $(CalculatedSummarySpendingPage.summaryItems()).getText()).to.contain("Monthly season ticket expenditure for travel by Plane"); + await expect(await $(CalculatedSummarySpendingPage.summaryItems()).getText()).to.contain("Additional monthly expenditure for travel by Plane"); + await $(CalculatedSummarySpendingPage.submit()).click(); + }); + + it("Given I have a calculated summary using a single answer from a repeating block, When I reach the calculated summary page, Then I see the correct items and total", async () => { + await expect(await $(CalculatedSummaryCountPage.calculatedSummaryTitle()).getText()).to.contain( + "We calculate the total journeys made per month to be 12. Is this correct?" + ); + await assertSummaryValues(["10", "2"]); + await expect(await $(CalculatedSummaryCountPage.summaryItems()).getText()).to.contain("Monthly journeys by Bus"); + await expect(await $(CalculatedSummaryCountPage.summaryItems()).getText()).to.contain("Monthly journeys by Plane"); + await $(CalculatedSummaryCountPage.submit()).click(); + }); + + it("Given I add a new item to the list, When I complete the repeating blocks and press continue, Then I see the first calculated summary page which the updated total", async () => { + await $(SectionOnePage.transportListAddLink()).click(); + await $(AddTransportPage.transportName()).selectByAttribute("value", "Train"); + await $(AddTransportPage.submit()).click(); + await $(TransportRepeatingBlock1Page.transportCompany()).setValue("Great Western Railway"); + await $(TransportRepeatingBlock1Page.transportCost()).setValue(100); + await $(TransportRepeatingBlock1Page.transportAdditionalCost()).setValue(50); + await $(TransportRepeatingBlock1Page.submit()).click(); + await $(TransportRepeatingBlock2Page.transportCount()).setValue(6); + await $(TransportRepeatingBlock2Page.submit()).click(); + await $(ListCollectorPage.no()).click(); + await $(ListCollectorPage.submit()).click(); + await expect(await browser.getUrl()).to.contain(CalculatedSummarySpendingPage.pageName); + await expect(await $(CalculatedSummarySpendingPage.calculatedSummaryTitle()).getText()).to.contain( + "We calculate the total monthly expenditure on transport to be £550.00. Is this correct?" + ); + await assertSummaryValues(["£100.00", "£30.00", "£5.00", "£0.00", "£265.00", "£100.00", "£50.00"]); + }); + + it("Given I am on the first calculated summary, When I confirm the total, Then I see the second calculated summary with an updated total", async () => { + await $(CalculatedSummarySpendingPage.submit()).click(); + await expect(await browser.getUrl()).to.contain(CalculatedSummaryCountPage.pageName); + await expect(await $(CalculatedSummaryCountPage.calculatedSummaryTitle()).getText()).to.contain( + "We calculate the total journeys made per month to be 18. Is this correct?" + ); + await assertSummaryValues(["10", "2", "6"]); + await $(CalculatedSummaryCountPage.previous()).click(); + }); + + it("Given I am on the first calculated summary, When I use one of the change links, Then I see the correct repeating block", async () => { + await repeatingAnswerChangeLink(1).click(); + await expect(await browser.getUrl()).to.contain(TransportRepeatingBlock1Page.pageName); + }); + + it("Given I have used a change link on a calculated summary to go back to the first repeating block, When I press continue, Then I see the calculated summary I came from", async () => { + await $(TransportRepeatingBlock1Page.submit()).click(); + await expect(await browser.getUrl()).to.contain(CalculatedSummarySpendingPage.pageName); + }); + + it("Given I am on a calculated summary with change links for repeating blocks, When I use a change link and click previous, Then I see the calculated summary I came from", async () => { + await repeatingAnswerChangeLink(1).click(); + await $(TransportRepeatingBlock1Page.previous()).click(); + await expect(await browser.getUrl()).to.contain(CalculatedSummarySpendingPage.pageName); + }); + + it("Given I use a repeating block change link on the first calculated summary, When I edit my answer and press continue, Then I see the first calculated summary with a new correct total", async () => { + await repeatingAnswerChangeLink(1).click(); + await $(TransportRepeatingBlock1Page.transportCost()).setValue(60); + await $(TransportRepeatingBlock1Page.submit()).click(); + await expect(await browser.getUrl()).to.contain(CalculatedSummarySpendingPage.pageName); + await expect(await $(CalculatedSummarySpendingPage.calculatedSummaryTitle()).getText()).to.contain( + "We calculate the total monthly expenditure on transport to be £580.00. Is this correct?" + ); + await assertSummaryValues(["£100.00", "£60.00", "£5.00", "£0.00", "£265.00", "£100.00", "£50.00"]); + await $(CalculatedSummarySpendingPage.submit()).click(); + }); + + it("Given I use a repeating block change link on the second calculated summary, When I edit my answer and press continue, Then I see the second calculated summary with a new correct total", async () => { + await repeatingAnswerChangeLink(2).click(); + await $(TransportRepeatingBlock2Page.transportCount()).setValue(12); + await $(TransportRepeatingBlock2Page.submit()).click(); + await expect(await browser.getUrl()).to.contain(CalculatedSummaryCountPage.pageName); + await expect(await $(CalculatedSummaryCountPage.calculatedSummaryTitle()).getText()).to.contain( + "We calculate the total journeys made per month to be 24. Is this correct?" + ); + await assertSummaryValues(["10", "2", "12"]); + await $(CalculatedSummaryCountPage.submit()).click(); + }); + + it("Given I use a remove link for on the summary page, When I press yes to confirm deleting the item, Then I see see the first calculated summary where I'm asked to reconfirm the total", async () => { + await $(SectionOnePage.transportListRemoveLink(1)).click(); + await $(RemoveTransportPage.yes()).click(); + await $(RemoveTransportPage.submit()).click(); + await expect(await browser.getUrl()).to.contain(CalculatedSummarySpendingPage.pageName); + await expect(await $(CalculatedSummarySpendingPage.calculatedSummaryTitle()).getText()).to.contain( + "We calculate the total monthly expenditure on transport to be £515.00. Is this correct?" + ); + await assertSummaryValues(["£100.00", "£0.00", "£265.00", "£100.00", "£50.00"]); + }); + + it("Given I have confirmed the first updated total, When I press continue, Then I see the next calculated summary to confirm that total too", async () => { + await $(CalculatedSummarySpendingPage.submit()).click(); + await expect(await browser.getUrl()).to.contain(CalculatedSummaryCountPage.pageName); + await expect(await $(CalculatedSummaryCountPage.calculatedSummaryTitle()).getText()).to.contain( + "We calculate the total journeys made per month to be 14. Is this correct?" + ); + await assertSummaryValues(["2", "12"]); + }); + + it("Given I have a second section, When I begin and answer the first question with a total higher than the calculated summary, Then I see an error message preventing me from continuing", async () => { + await $(CalculatedSummaryCountPage.submit()).click(); + await $(SectionOnePage.submit()).click(); + await $(HubPage.submit()).click(); + await expect(await $(FamilyJourneysPage.questionTitle()).getText()).to.contain("How many of your 14 journeys are to visit family?"); + await $(FamilyJourneysPage.answer()).setValue(15); + await $(FamilyJourneysPage.submit()).click(); + await expect(await $(FamilyJourneysPage.singleErrorLink()).getText()).to.contain("Enter an answer less than or equal to 14"); + }); + + it("Given I enter a value below the calculated summary from section 1, When I press Continue, Then I see my answer displayed on the next page", async () => { + await $(FamilyJourneysPage.answer()).setValue(10); + await $(FamilyJourneysPage.submit()).click(); + await expect(await $(SectionTwoPage.familyJourneysQuestion()).getText()).to.contain("How many of your 14 journeys are to visit family?"); + await expect(await $(SectionTwoPage.familyJourneysAnswer()).getText()).to.contain("10"); + await $(SectionTwoPage.submit()).click(); + }); + + it("Given I use the add list item link, When I add a new item and return to the Hub, Then I see the progress of section 2 has reverted to Partially Complete", async () => { + await expect(await $(HubPage.summaryRowState("section-1")).getText()).to.equal("Completed"); + await expect(await $(HubPage.summaryRowState("section-2")).getText()).to.equal("Completed"); + await $(HubPage.summaryRowLink("section-1")).click(); + await $(SectionOnePage.transportListAddLink()).click(); + await $(AddTransportPage.transportName()).selectByAttribute("value", "Tube"); + await $(AddTransportPage.submit()).click(); + await $(TransportRepeatingBlock1Page.submit()).click(); + await $(TransportRepeatingBlock2Page.transportCount()).setValue(2); + await $(TransportRepeatingBlock2Page.submit()).click(); + await $(ListCollectorPage.no()).click(); + await $(ListCollectorPage.submit()).click(); + await $(CalculatedSummarySpendingPage.submit()).click(); + await $(CalculatedSummaryCountPage.submit()).click(); + await browser.url(HubPage.url()); + await expect(await $(HubPage.summaryRowState("section-1")).getText()).to.equal("Completed"); + await expect(await $(HubPage.summaryRowState("section-2")).getText()).to.equal("Partially completed"); + }); + + it("Given I complete section-2 again, When I remove a list item and return to the Hub, Then I see the progress of section 2 has reverted to Partially Complete", async () => { + await $(HubPage.submit()).click(); + await $(FamilyJourneysPage.answer()).setValue(16); + await $(FamilyJourneysPage.submit()).click(); + await $(SectionTwoPage.submit()).click(); + await expect(await $(HubPage.summaryRowState("section-1")).getText()).to.equal("Completed"); + await expect(await $(HubPage.summaryRowState("section-2")).getText()).to.equal("Completed"); + await $(HubPage.summaryRowLink("section-1")).click(); + await $(SectionOnePage.transportListRemoveLink(3)).click(); + await $(RemoveTransportPage.yes()).click(); + await $(RemoveTransportPage.submit()).click(); + await $(CalculatedSummarySpendingPage.submit()).click(); + await $(CalculatedSummaryCountPage.submit()).click(); + await $(SectionOnePage.submit()).click(); + await expect(await $(HubPage.summaryRowState("section-1")).getText()).to.equal("Completed"); + await expect(await $(HubPage.summaryRowState("section-2")).getText()).to.equal("Partially completed"); + }); + + it("Given I have a question which removes the list collector from the path, When I change my answer to the question removing the list collector and route backwards from the summary, Then I see the first calculated summary with an updated total", async () => { + await $(HubPage.summaryRowLink("section-1")).click(); + await $(SectionOnePage.answerSkipEdit()).click(); + await $(BlockSkipPage.yes()).click(); + await $(BlockSkipPage.submit()).click(); + // calculated summary progress is not altered by removing the list collector from the path so next location is summary page + await expect(await browser.getUrl()).to.contain(SectionOnePage.pageName); + await $(SectionOnePage.previous()).click(); + // other calculated summary should not be on the path, so go straight back to the spending one which now has none of the list items + await expect(await browser.getUrl()).to.contain(CalculatedSummarySpendingPage.pageName); + await expect(await $(CalculatedSummarySpendingPage.calculatedSummaryTitle()).getText()).to.contain( + "We calculate the total monthly expenditure on transport to be £100.00. Is this correct?" + ); + await assertSummaryValues(["£100.00"]); + }); + + it("Given I confirm the calculated summary and finish the section, When I return to the Hub, Then I see that section 2 is no longer available", async () => { + await $(CalculatedSummarySpendingPage.submit()).click(); + await $(SectionOnePage.submit()).click(); + // section 2 is now gone + await expect(await $$(HubPage.summaryItems()).length).to.equal(1); + }); +}); diff --git a/tests/functional/spec/features/repeating_blocks/list_collector_repeating_blocks.spec.js b/tests/functional/spec/features/repeating_blocks/list_collector_repeating_blocks.spec.js index 303d1ecfab..02488bd5db 100644 --- a/tests/functional/spec/features/repeating_blocks/list_collector_repeating_blocks.spec.js +++ b/tests/functional/spec/features/repeating_blocks/list_collector_repeating_blocks.spec.js @@ -9,9 +9,10 @@ import AnyOtherCompaniesOrBranchesPage from "../../../generated_pages/list_colle import SectionCompaniesPage from "../../../generated_pages/list_collector_repeating_blocks_section_summary/section-companies-summary.page"; import AnyOtherTradingDetailsPage from "../../../generated_pages/list_collector_repeating_blocks_section_summary/any-other-trading-details.page"; import SubmitPage from "../../../generated_pages/list_collector_repeating_blocks_section_summary/submit.page"; -import { checkItemsInList, checkListItemComplete, checkListItemIncomplete } from "../../../helpers"; +import { repeatingAnswerChangeLink, checkItemsInList, checkListItemComplete, checkListItemIncomplete } from "../../../helpers"; import HubPage from "../../../base_pages/hub.page"; +const summaryValues = 'dd[class="ons-summary__values"]'; async function proceedToListCollector() { await $(ResponsiblePartyPage.yes()).click(); await $(AnyCompaniesOrBranchesPage.submit()).click(); @@ -141,6 +142,7 @@ describe("List Collector Repeating Blocks", () => { await $(AddCompanyPage.companyOrBranchName()).setValue("GOV"); await $(AddCompanyPage.submit()).click(); await $(CompaniesRepeatingBlock1Page.cancelAndReturn()).click(); + await $(EditCompanyPage.cancelAndReturn()).click(); await $(AnyOtherCompaniesOrBranchesPage.yes()).click(); await $(AnyOtherCompaniesOrBranchesPage.submit()).click(); @@ -152,6 +154,8 @@ describe("List Collector Repeating Blocks", () => { await $(CompaniesRepeatingBlock1Page.registrationDateyear()).setValue(2023); await $(CompaniesRepeatingBlock1Page.submit()).click(); await $(CompaniesRepeatingBlock2Page.cancelAndReturn()).click(); + await $(CompaniesRepeatingBlock1Page.cancelAndReturn()).click(); + await $(EditCompanyPage.cancelAndReturn()).click(); await $(AnyOtherCompaniesOrBranchesPage.yes()).click(); await $(AnyOtherCompaniesOrBranchesPage.submit()).click(); @@ -220,30 +224,30 @@ describe("List Collector Repeating Blocks", () => { }); it("Edit each type of answer on different items from the section summary.", async () => { - await expect(await $$(`dd[data-qa="registration-number"]`)[1].getText()).to.have.string(456); - await $$(`a[data-qa="registration-number-edit"]`)[1].click(); + await expect(await $$(summaryValues)[8].getText()).to.have.string(456); + await repeatingAnswerChangeLink(8).click(); await $(CompaniesRepeatingBlock1Page.registrationNumber()).setValue(789); await $(CompaniesRepeatingBlock1Page.submit()).click(); - await expect(await $$(`dd[data-qa="registration-number"]`)[1].getText()).to.have.string(789); + await expect(await $$(summaryValues)[8].getText()).to.have.string(789); - await expect(await $$(`dd[data-qa="registration-date"]`)[0].getText()).to.have.string("1 January 2023"); - await $$(`a[data-qa="registration-date-edit"]`)[0].click(); + await expect(await $$(summaryValues)[4].getText()).to.have.string("1 January 2023"); + await repeatingAnswerChangeLink(4).click(); await $(CompaniesRepeatingBlock1Page.registrationDateday()).setValue(4); await $(CompaniesRepeatingBlock1Page.registrationDatemonth()).setValue(4); await $(CompaniesRepeatingBlock1Page.submit()).click(); - await expect(await $$(`dd[data-qa="registration-date"]`)[0].getText()).to.have.string("4 April 2023"); + await expect(await $$(summaryValues)[4].getText()).to.have.string("4 April 2023"); - await expect(await $$(`dd[data-qa="authorised-trader-uk-radio"]`)[0].getText()).to.have.string("Yes"); - await $$(`a[data-qa="authorised-trader-uk-radio-edit"]`)[0].click(); + await expect(await $$(summaryValues)[5].getText()).to.have.string("Yes"); + await repeatingAnswerChangeLink(5).click(); await $(CompaniesRepeatingBlock2Page.authorisedTraderUkRadioNo()).click(); await $(CompaniesRepeatingBlock2Page.submit()).click(); - await expect(await $$(`dd[data-qa="authorised-trader-uk-radio"]`)[0].getText()).to.have.string("No"); + await expect(await $$(summaryValues)[5].getText()).to.have.string("No"); - await expect(await $$(`dd[data-qa="authorised-trader-eu-radio"]`)[1].getText()).to.have.string("No answer provided"); - await $$(`a[data-qa="authorised-trader-eu-radio-edit"]`)[1].click(); + await expect(await $$(summaryValues)[11].getText()).to.have.string("No answer provided"); + await repeatingAnswerChangeLink(11).click(); await $(CompaniesRepeatingBlock2Page.authorisedTraderEuRadioYes()).click(); await $(CompaniesRepeatingBlock2Page.submit()).click(); - await expect(await $$(`dd[data-qa="authorised-trader-eu-radio"]`)[1].getText()).to.have.string("Yes"); + await expect(await $$(summaryValues)[11].getText()).to.have.string("Yes"); }); it("The list collector can then be submitted", async () => { diff --git a/tests/functional/spec/list_collector_section_summary.spec.js b/tests/functional/spec/list_collector_section_summary.spec.js index 0c9ff489ff..c3baa42f50 100644 --- a/tests/functional/spec/list_collector_section_summary.spec.js +++ b/tests/functional/spec/list_collector_section_summary.spec.js @@ -10,6 +10,7 @@ import HouseholderCheckboxPage from "../generated_pages/list_collector_section_s import SubmitPage from "../generated_pages/list_collector_section_summary/submit.page"; import ThankYouPage from "../base_pages/thank-you.page"; import ViewSubmittedResponsePage from "../generated_pages/list_collector_section_summary/view-submitted-response.page"; +import { listItemIds } from "../helpers"; describe("List Collector Section Summary and Summary Items", () => { describe("Given I launch the test list collector section summary items survey", () => { @@ -32,9 +33,14 @@ describe("List Collector Section Summary and Summary Items", () => { await expect(await $(companiesListRowItem(1, 1)).getText()).to.contain("Company A"); await expect(await $(companiesListRowItem(1, 2)).getText()).to.contain("123"); await expect(await $(companiesListRowItem(1, 3)).getText()).to.contain("Yes"); - await expect(await $(companiesListRowItemAnchor(1)).getHTML()).to.contain("return_to=section-summary#company-or-branch-name"); - await expect(await $(companiesListRowItemAnchor(2)).getHTML()).to.contain("return_to_answer_id=registration-number#registration-number"); - await expect(await $(companiesListRowItemAnchor(3)).getHTML()).to.contain("return_to_answer_id=authorised-insurer-radio#authorised-insurer-radio"); + const listItemId = (await listItemIds())[0]; + await expect(await $(companiesListRowItemAnchor(1)).getHTML()).to.contain( + `return_to=section-summary&return_to_answer_id=${listItemId}#company-or-branch-name` + ); + await expect(await $(companiesListRowItemAnchor(2)).getHTML()).to.contain(`return_to_answer_id=registration-number-${listItemId}#registration-number`); + await expect(await $(companiesListRowItemAnchor(3)).getHTML()).to.contain( + `return_to_answer_id=authorised-insurer-radio-${listItemId}#authorised-insurer-radio` + ); }); it("When I add multiple items, Then all the items should be visible on the section summary and have correct values", async () => { await drivingQuestionYes(); diff --git a/tests/integration/questionnaire/__init__.py b/tests/integration/questionnaire/__init__.py index f655dc2a3f..b5da4c7e6e 100644 --- a/tests/integration/questionnaire/__init__.py +++ b/tests/integration/questionnaire/__init__.py @@ -27,3 +27,9 @@ def get_link(self, action, position): selector = f"[data-qa='list-item-{action}-{position}-link']" selected = self.getHtmlSoup().select(selector) return selected[0].get("href") + + def get_list_item_change_link(self, answer_id, list_item_id): + """change link on calculated summary for list item""" + selector = f"[data-qa='{answer_id}-{list_item_id}-edit']" + selected = self.getHtmlSoup().select(selector) + return selected[0].get("href") diff --git a/tests/integration/questionnaire/test_questionnaire_calculated_summary.py b/tests/integration/questionnaire/test_questionnaire_calculated_summary.py index 74ee50f4d5..6bad6f9cc3 100644 --- a/tests/integration/questionnaire/test_questionnaire_calculated_summary.py +++ b/tests/integration/questionnaire/test_questionnaire_calculated_summary.py @@ -1,4 +1,4 @@ -from . import QuestionnaireTestCase +from tests.integration.questionnaire import QuestionnaireTestCase class TestQuestionnaireCalculatedSummary(QuestionnaireTestCase): @@ -212,3 +212,84 @@ def test_calculated_summary_repeating_answers_only(self): self.assertInBody( "We calculate the total monthly spending on public transport to be £300.00. Is this correct?" ) + + def test_new_calculated_summary_repeating_blocks(self): + """ + Tests a calculated summary with a repeating block answer id source resolving to a list of answers + """ + self.launchSurvey("test_new_calculated_summary_repeating_blocks") + self.post({"answer-car": "100"}) + self.post({"answer-skip": "No"}) + self.post({"list-collector-answer": "Yes"}) + self.post({"transport-name": "Bus"}) + self.post( + { + "transport-company": "First", + "transport-cost": "30", + "transport-additional-cost": "5", + } + ) + self.post({"transport-count": "10"}) + self.post({"list-collector-answer": "Yes"}) + self.post({"transport-name": "Plane"}) + self.post( + { + "transport-company": "EasyJet", + "transport-cost": "0", + "transport-additional-cost": "265", + } + ) + self.post({"transport-count": "2"}) + list_item_ids = self.get_list_item_ids() + self.post({"list-collector-answer": "No"}) + self.assertInBody( + "We calculate the total monthly expenditure on transport to be £400.00. Is this correct?" + ) + self.post() + self.assertInBody( + "We calculate the total journeys made per month to be 12. Is this correct?" + ) + + # check that using a change link and editing an answer takes you straight back to the relevant calculated summary + change_link = self.get_list_item_change_link( + "transport-count", list_item_ids[1] + ) + self.get(change_link) + self.post({"transport-count": "4"}) + self.assertInUrl("/calculated-summary-count/") + self.assertInBody( + "We calculate the total journeys made per month to be 14. Is this correct?" + ) + self.previous() + + # likewise for the other calculated summary + change_link = self.get_list_item_change_link("transport-cost", list_item_ids[0]) + self.get(change_link) + self.post( + { + "transport-company": "First", + "transport-cost": "300", + "transport-additional-cost": "50", + } + ) + self.assertInUrl("/calculated-summary-spending/") + self.assertInBody( + "We calculate the total monthly expenditure on transport to be £715.00. Is this correct?" + ) + + # check that removing the list collector from the path updates the calculated summary correctly + self.previous() + self.previous() + self.post({"answer-skip": "Yes"}) + + # calculated summary count should now not be on the path + self.assertInUrl("/calculated-summary-spending/") + self.assertInBody( + "We calculate the total monthly expenditure on transport to be £100.00. Is this correct?" + ) + # no list items should be there + self.assertNotInBody("Give details of your expenditure travelling by") + self.post() + # should be absent from section summary too + self.assertInUrl("/sections/section-1") + self.assertNotInBody("Name of transport") diff --git a/tests/integration/questionnaire/test_questionnaire_list_collector_repeating_blocks.py b/tests/integration/questionnaire/test_questionnaire_list_collector_repeating_blocks.py index 121d656cc5..4c8cebeccf 100644 --- a/tests/integration/questionnaire/test_questionnaire_list_collector_repeating_blocks.py +++ b/tests/integration/questionnaire/test_questionnaire_list_collector_repeating_blocks.py @@ -28,7 +28,6 @@ def add_company_from_list_collector( def post_repeating_block_1(self, registration_number: int, registration_date: date): self.assertInUrl("/companies/") self.assertInUrl("/companies-repeating-block-1/") - self.post( { "registration-number": registration_number, @@ -84,15 +83,29 @@ def get_list_item_link(self, action, position): return selected[0].get("href") def get_link(self, selector: str): - return self.get_links(selector)[0] + return self.getHtmlSoup().select(selector)[0]["href"] + + def get_list_item_ids(self): + result = self.getHtmlSoup().select("[data-list-item-id]") + return [list_item["data-list-item-id"] for list_item in result] - def get_links(self, selector: str): + def click_edit_link(self, answer_id: str, position: int): + list_item_ids = self.get_list_item_ids() + selector = f"[data-qa='{answer_id}-{list_item_ids[position]}-edit']" selected = self.getHtmlSoup().select(selector) - return [element["href"] for element in selected] + edit_link = selected[0]["href"] + self.get(edit_link) + + def click_add_link(self): + add_link = self.get_link("[data-qa='add-item-link']") + self.get(add_link) + + def click_cancel_link(self): + cancel_link = self.get_link("[id='cancel-and-return']") + self.get(cancel_link) def add_three_companies(self): # Add first company - self.add_company_and_repeating_blocks( company_name="Company1", registration_number=123, @@ -103,7 +116,6 @@ def add_three_companies(self): ) # Add second company - self.add_company_and_repeating_blocks( company_name="Company2", registration_number=456, @@ -126,11 +138,9 @@ def add_three_companies(self): def test_invalid_invalid_list_item_id(self): self.launch_repeating_blocks_test_survey() - self.get( "/questionnaire/companies/non-existing-list-item-id/companies-repeating-block-1/" ) - self.assertStatusNotFound() def test_happy_path(self): @@ -140,27 +150,22 @@ def test_happy_path(self): self.add_three_companies() # Remove item 2 - remove_link = self.get_list_item_link("remove", 2) self.get(remove_link) self.assertInBody("Are you sure you want to remove this company or UK branch?") # Cancel - self.post({"remove-confirmation": "No"}) self.assertEqualUrl("/questionnaire/any-other-companies-or-branches/") # Remove again - self.get(remove_link) self.post({"remove-confirmation": "Yes"}) # Check list item 3 has moved to second position - self.assert_company_completed(3, 2) # Test the previous link - edit_link_1 = self.get_list_item_link("change", 1) remove_link_1 = self.get_list_item_link("remove", 1) @@ -175,7 +180,6 @@ def test_happy_path(self): self.assertEqualUrl("/questionnaire/any-other-companies-or-branches/") # Submit survey - self.post({"any-other-companies-or-branches-answer": "No"}) self.post({"any-other-trading-details-answer": "No other details"}) self.post() @@ -188,15 +192,15 @@ def test_incomplete_repeating_blocks(self): self.launch_repeating_blocks_test_survey() # Add first company - only add block and first repeating block - self.add_company_from_list_collector(company_name="Company1", is_driving=True) self.post_repeating_block_1( registration_number=123, registration_date=date(2023, 1, 1) ) - self.previous() # Return to the list collector via previous button + self.previous() # return to previous repeating block + self.previous() # return to edit block + self.previous() # return to list collector # Add second company - complete - self.add_company_and_repeating_blocks( company_name="Company2", registration_number=456, @@ -206,11 +210,10 @@ def test_incomplete_repeating_blocks(self): # Add third company - only the add block self.add_company_from_list_collector(company_name="Company3") - cancel_link = self.get_link("[id='cancel-and-return']") - self.get(cancel_link) # Return to the list collector via cancel button + self.click_cancel_link() # Return to edit block + self.click_cancel_link() # Return to the list collector # Add fourth company - complete - self.add_company_and_repeating_blocks( company_name="Company4", registration_number=101, @@ -219,14 +222,12 @@ def test_incomplete_repeating_blocks(self): ) # Assert completeness - self.assert_company_incomplete(1, 1) self.assert_company_completed(2, 2) self.assert_company_incomplete(3, 3) self.assert_company_completed(4, 4) # Attempt to move along path after list collector - will route to first incomplete block of first incomplete item - self.post({"any-other-companies-or-branches-answer": "No"}) # Should be routed to incomplete block 2 of item 1 @@ -256,7 +257,6 @@ def test_adding_from_the_summary_page_adds_the_return_to_param_to_the_url( self.launch_repeating_blocks_test_survey() # Add first company - self.add_company_and_repeating_blocks( company_name="Company1", registration_number=123, @@ -267,7 +267,6 @@ def test_adding_from_the_summary_page_adds_the_return_to_param_to_the_url( ) # Assert list items complete - self.assert_company_completed(1, 1) # Navigate to section summary @@ -276,9 +275,7 @@ def test_adding_from_the_summary_page_adds_the_return_to_param_to_the_url( self.assertInUrl("/sections/section-companies/") # Add second company - from section summary - - add_link = self.get_link("[data-qa='add-item-link']") - self.get(add_link) + self.click_add_link() self.assertInUrl("?return_to=section-summary") self.add_company(company_name="Company2") self.post_repeating_block_1( @@ -291,12 +288,10 @@ def test_adding_from_the_summary_page_adds_the_return_to_param_to_the_url( self.post({"any-other-companies-or-branches-answer": "No"}) self.post() self.assertInUrl("/submit/") - add_link = self.get_link("[data-qa='add-item-link']") - self.get(add_link) - self.assertInUrl("?return_to=section-summary") + self.click_add_link() + self.assertInUrl("?return_to=final-summary") # Add third company - from submit summary - self.add_company(company_name="Company3") self.post_repeating_block_1( registration_number=789, registration_date=date(2023, 3, 3) @@ -326,7 +321,6 @@ def test_removing_from_the_summary_page_adds_the_return_to_param_to_the_url( self.post({"any-other-trading-details-answer": "No other details"}) # Remove item 3 - remove_link = self.get_list_item_link("remove", 3) self.get(remove_link) self.assertInUrl("?return_to=section-summary") @@ -338,10 +332,9 @@ def test_removing_from_the_summary_page_adds_the_return_to_param_to_the_url( self.post() # Remove item 2 - remove_link = self.get_list_item_link("remove", 2) self.get(remove_link) - self.assertInUrl("?return_to=section-summary") + self.assertInUrl("?return_to=final-summary") self.assertInBody("Are you sure you want to remove this company or UK branch?") self.post({"remove-confirmation": "Yes"}) @@ -365,10 +358,8 @@ def test_edit_repeating_block_from_the_summary_page_adds_the_return_to_param_to_ self.post({"any-other-trading-details-answer": "No other details"}) self.assertInUrl("/sections/section-companies/") - # Edit item 3 - - edit_links = self.get_links("[data-qa='registration-number-edit']") - self.get(edit_links[2]) + # Edit item 3 + self.click_edit_link("registration-number", 2) self.assertInUrl("?return_to=section-summary") self.post_repeating_block_1( registration_number=000, registration_date=date(2023, 5, 5) @@ -376,9 +367,7 @@ def test_edit_repeating_block_from_the_summary_page_adds_the_return_to_param_to_ self.assertInUrl("/sections/section-companies/") # Remove item 2 - - edit_links = self.get_links("[data-qa='authorised-trader-uk-radio-edit']") - self.get(edit_links[2]) + self.click_edit_link("authorised-trader-uk-radio", 2) self.assertInUrl("?return_to=section-summary") self.post_repeating_block_2(trader_uk="No", trader_eu="No") self.assertInUrl("/sections/section-companies/") @@ -390,6 +379,26 @@ def test_edit_repeating_block_from_the_summary_page_adds_the_return_to_param_to_ "Thank you for completing the Test a List Collector with Repeating Blocks and Section Summary Items" ) + def test_edit_incomplete_repeating_block_from_summary_page_routes_to_next_repeating_block( + self, + ): + self.launch_repeating_blocks_test_survey() + + # add incomplete item with answers for first repeating block + self.add_company_from_list_collector(company_name="Company4", is_driving=True) + self.post_repeating_block_1( + registration_number=123, registration_date=date(2023, 1, 1) + ) + + # go back to edit block and change the name + self.click_cancel_link() + self.click_cancel_link() + self.assertInUrl("/edit-company/") + self.post({"company-or-branch-name": "Company5"}) + + # should jump straight to repeating block 2 as this is the first incomplete one + self.assertInUrl("/companies-repeating-block-2/") + def test_adding_incomplete_list_item_from_summary_returns_to_list_collector_not_summary( self, ): @@ -404,11 +413,10 @@ def test_adding_incomplete_list_item_from_summary_returns_to_list_collector_not_ self.assertInUrl("/sections/section-companies/") # Add incomplete item from section summary add link - add_link = self.get_link("[data-qa='add-item-link']") - self.get(add_link) + self.click_add_link() self.add_company(company_name="Company4") - cancel_link = self.get_link("[id='cancel-and-return']") - self.get(cancel_link) + self.click_cancel_link() # cancel and go back to edit block + self.click_cancel_link() # cancel and go back to list collector self.assertInUrl("/any-other-companies-or-branches/") self.assert_company_incomplete(4, 4) @@ -425,11 +433,10 @@ def test_adding_incomplete_list_item_from_summary_returns_to_list_collector_not_ self.post() # Add incomplete item from submit summary add link - add_link = self.get_link("[data-qa='add-item-link']") - self.get(add_link) + self.click_add_link() self.add_company(company_name="Company5") - cancel_link = self.get_link("[id='cancel-and-return']") - self.get(cancel_link) + self.click_cancel_link() + self.click_cancel_link() self.assertInUrl("/any-other-companies-or-branches/") self.assert_company_incomplete(5, 5) @@ -443,9 +450,27 @@ def test_adding_incomplete_list_item_from_summary_returns_to_list_collector_not_ # Submit self.post({"any-other-companies-or-branches-answer": "No"}) - self.post({"any-other-trading-details-answer": "No other details"}) self.post() self.post() self.assertInBody( "Thank you for completing the Test a List Collector with Repeating Blocks and Section Summary Items" ) + + def test_edit_repeating_block_from_submit_page_returns_to_submit_page( + self, + ): + self.launch_repeating_blocks_test_survey() + + # Add some items and progress to submit page + self.add_three_companies() + self.post({"any-other-companies-or-branches-answer": "No"}) + self.post({"any-other-trading-details-answer": "No other details"}) + self.post() + self.assertInUrl("/submit/") + + # click an edit link + self.click_edit_link("registration-number", 2) + + # previous link should return to submit page + self.previous() + self.assertInUrl("/submit/")