Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Add support in repeating GCS for static CS with repeating answers #1268

Merged
merged 11 commits into from
Dec 15, 2023
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions app/questionnaire/questionnaire_schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -835,11 +835,14 @@ def get_list_name_for_answer_id(self, answer_id: str) -> str | None:
if the answer is dynamic or in a repeating block or section, return the name of the list it repeats over, otherwise None.
"""
# Type ignore: safe to assume block exists, same for section below.
block_id: str = self.get_block_for_answer_id(answer_id)["id"] # type: ignore
block: ImmutableDict = self.get_block_for_answer_id(answer_id) # type: ignore
block_id: str = block["id"]
if self.is_answer_dynamic(answer_id):
return self.get_list_name_for_dynamic_answer(block_id)
if self.is_answer_in_list_collector_repeating_block(answer_id):
return self._list_names_by_list_repeating_block_id[block_id]
return self.list_names_by_list_repeating_block_id[block_id]
if self.is_answer_in_list_collector_block(answer_id):
return block["for_list"] # type: ignore
if self.is_answer_in_repeating_section(answer_id):
section_id: str = self.get_section_id_for_block_id(block_id) # type: ignore
return self.get_repeating_list_for_section(section_id)
Expand Down
28 changes: 20 additions & 8 deletions app/questionnaire/value_source_resolver.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,23 @@ def _get_answer_value(
):
return answer.value

def _resolve_list_item_id_for_answer_id(self, answer_id: str) -> str | None:
"""
If there's a list item id and the answer is repeating, return the list item id to resolve the instance of the answer
However if the answer is repeating for a different list, return None so that the repeating answer id resolves to a list
"""
if self.list_item_id and (
list_name_for_answer := self.schema.get_list_name_for_answer_id(answer_id)
):
# if there is a current list, and it differs to the repeating answer one, return None
if (
self.location
and self.location.list_name
and self.location.list_name != list_name_for_answer
):
return None
return self.list_item_id

def _resolve_list_item_id_for_value_source(
self, value_source: Mapping
) -> str | None:
Expand Down Expand Up @@ -102,12 +119,8 @@ def _resolve_list_item_id_for_value_source(
else None
)

return (
self.list_item_id
if self.list_item_id
and self.schema.is_repeating_answer(value_source["identifier"])
else None
)
if value_source["source"] == "answers":
return self._resolve_list_item_id_for_answer_id(value_source["identifier"])

def _resolve_repeating_answers_for_list(
self, *, answer_id: str, list_name: str
Expand Down Expand Up @@ -242,11 +255,10 @@ def _resolve_summary_with_calculation(
# the calculation object for the old type of calculated summary block may contain answers_to_calculate instead of operation
if calculation.get("answers_to_calculate"):
operator = self.get_calculation_operator(calculation["calculation_type"])
list_item_id = self._resolve_list_item_id_for_value_source(value_source)
values = [
self._get_answer_value(
answer_id=answer_id,
list_item_id=list_item_id,
list_item_id=self._resolve_list_item_id_for_answer_id(answer_id),
assess_routing_path=assess_routing_path,
)
for answer_id in calculation["answers_to_calculate"]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,17 @@
"id": "base-costs-section",
"title": "Vehicle Costs",
"summary": {
"show_on_completion": true
"show_on_completion": true,
"items": [
{
"type": "List",
"for_list": "costs",
"title": "Base Costs",
"add_link_text": "Add another base cost",
"empty_list_text": "There are no base costs"
}
],
"show_non_item_answers": true
},
"groups": [
{
Expand All @@ -44,19 +54,6 @@
"type": "General",
"id": "any-cost-question",
"title": "Do you have any outgoing costs for owning a vehicle?",
"guidance": {
"contents": [
{
"title": "Notes for testing"
},
{
"description": "For the first iteration, calculated summaries of repeating answers don’t work in a grand calculated summary in a separate repeating section"
},
{
"description": "<b>So you should answer no to this question</b> and remove this guidance once separate repeating answer calculated summaries are supported"
}
]
},
"answers": [
{
"type": "Radio",
Expand Down Expand Up @@ -216,6 +213,49 @@
]
}
},
"repeating_blocks": [
{
"id": "cost-repeating-block-1",
"type": "ListRepeatingQuestion",
"question": {
"id": "cost-repeating-block-1-question",
"type": "General",
"title": {
"text": "What is the base monthly rate for {cost_name} for a single vehicle?",
"placeholders": [
{
"placeholder": "cost_name",
"value": {
"source": "answers",
"identifier": "cost-name"
}
}
]
},
"answers": [
{
"id": "repeating-block-1-cost-base",
"label": {
"text": "Base {transformed_value} expenditure",
"placeholders": [
{
"placeholder": "transformed_value",
"value": {
"source": "answers",
"identifier": "cost-name"
}
}
]
},
"mandatory": true,
"type": "Currency",
"currency": "GBP",
"decimal_places": 2
}
]
}
}
],
"summary": {
"title": "cost",
"item_title": {
Expand Down Expand Up @@ -262,7 +302,7 @@
},
"question": {
"id": "dynamic-answer-question",
"title": "How much do you spend per month on the following for a single vehicle?",
"title": "How much extra do you normally spend per month for a single vehicle?",
"type": "General",
"dynamic_answers": {
"values": {
Expand All @@ -272,7 +312,7 @@
"answers": [
{
"label": {
"text": "Single vehicle {transformed_value}",
"text": "Extra {transformed_value} expenditure",
"placeholders": [
{
"placeholder": "transformed_value",
Expand All @@ -283,9 +323,9 @@
}
]
},
"id": "cost-of-cost",
"id": "dynamic-answer-cost-extra",
"type": "Currency",
"mandatory": true,
"mandatory": false,
"currency": "GBP",
"decimal_places": 2
}
Expand Down Expand Up @@ -322,7 +362,11 @@
"+": [
{
"source": "answers",
"identifier": "cost-of-cost"
"identifier": "repeating-block-1-cost-base"
},
{
"source": "answers",
"identifier": "dynamic-answer-cost-extra"
},
{
"source": "answers",
Expand Down Expand Up @@ -814,6 +858,13 @@
"id": "gcs-breakdown-block",
"question": {
"id": "gcs-breakdown-question",
"guidance": {
"contents": [
{
"description": "Currently this question is not revisited when the grand calculated summary changes. When grand calculated summary dependencies are implemented, this guidance should be removed, and this block should become incomplete upon the GCS changing."
}
]
},
"title": {
"text": "How do you pay for the monthly fees of {vehicle_cost}?",
"placeholders": [
Expand Down
20 changes: 10 additions & 10 deletions schemas/test/en/test_repeating_sections_with_hub_and_spoke.json
Original file line number Diff line number Diff line change
Expand Up @@ -436,13 +436,13 @@
"title": "What is the name of the person?",
"answers": [
{
"id": "first-name",
"id": "forename",
"label": "First name",
"mandatory": true,
"type": "TextField"
},
{
"id": "last-name",
"id": "surname",
"label": "Last name",
"mandatory": true,
"type": "TextField"
Expand All @@ -459,13 +459,13 @@
"title": "What is the name of the person?",
"answers": [
{
"id": "first-name",
"id": "forename",
"label": "First name",
"mandatory": true,
"type": "TextField"
},
{
"id": "last-name",
"id": "surname",
"label": "Last name",
"mandatory": true,
"type": "TextField"
Expand Down Expand Up @@ -516,11 +516,11 @@
"list_to_concatenate": [
{
"source": "answers",
"identifier": "first-name"
"identifier": "forename"
},
{
"source": "answers",
"identifier": "last-name"
"identifier": "surname"
}
]
},
Expand Down Expand Up @@ -1219,11 +1219,11 @@
"list_to_concatenate": [
{
"source": "answers",
"identifier": "first-name"
"identifier": "forename"
},
{
"source": "answers",
"identifier": "last-name"
"identifier": "surname"
}
],
"delimiter": " "
Expand Down Expand Up @@ -1278,11 +1278,11 @@
"list_to_concatenate": [
{
"source": "answers",
"identifier": "first-name"
"identifier": "forename"
},
{
"source": "answers",
"identifier": "last-name"
"identifier": "surname"
}
]
},
Expand Down
2 changes: 1 addition & 1 deletion scripts/run_validator.sh
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
#!/usr/bin/env bash
tag=latest
tag=gcs-in-repeat-with-static-cs-repeating-answers
katie-gardner marked this conversation as resolved.
Show resolved Hide resolved
TAG=${tag} docker-compose -f docker-compose-schema-validator.yml up -d
4 changes: 2 additions & 2 deletions tests/app/forms/field_handlers/test_date_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -146,8 +146,8 @@ def test_get_referenced_offset_value_for_answer_id(


@patch(
"app.questionnaire.questionnaire_schema.QuestionnaireSchema.is_repeating_answer",
return_value=True,
"app.questionnaire.questionnaire_schema.QuestionnaireSchema.get_list_name_for_answer_id",
return_value="list",
)
def test_get_referenced_offset_value_with_list_item_id(
app, value_source_resolver, rule_evaluator
Expand Down
8 changes: 4 additions & 4 deletions tests/app/questionnaire/rules/test_rule_evaluator.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ def get_rule_evaluator(
):
if not schema:
schema = get_mock_schema()
schema.is_repeating_answer = Mock(return_value=True)
schema.get_list_name_for_answer_id = Mock(return_value="mock-list")
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)
Expand Down Expand Up @@ -793,7 +793,7 @@ def test_date_value(rule, expected_result):
def test_answer_source_outside_of_repeating_section():
schema = get_mock_schema()

schema.is_repeating_answer = Mock(return_value=False)
schema.get_list_name_for_answer_id = Mock(return_value=None)
answer_store = AnswerStore([{"answer_id": "some-answer", "value": "Yes"}])

rule_evaluator = get_rule_evaluator(
Expand Down Expand Up @@ -857,7 +857,7 @@ def test_answer_source_not_on_path_non_repeating_section(is_answer_on_path):
@pytest.mark.parametrize("is_answer_on_path", [True, False])
def test_answer_source_not_on_path_repeating_section(is_answer_on_path):
schema = get_mock_schema()
schema.is_repeating_answer = Mock(return_value=True)
schema.get_list_name_for_answer_id = Mock(return_value="mock-list")
location = Location(
section_id="test-section", block_id="test-block", list_item_id="item-1"
)
Expand Down Expand Up @@ -1098,7 +1098,7 @@ def test_supplementary_data_source(
):
"""Tests rule evaluation of repeating and non-repeating supplementary data source inside a repeat"""
schema = get_mock_schema()
schema.is_repeating_answer = Mock(return_value=False)
schema.get_list_name_for_answer_id = Mock(return_value=None)
answer_store = AnswerStore([{"answer_id": "same-answer", "value": value}])

rule_evaluator = get_rule_evaluator(
Expand Down
4 changes: 2 additions & 2 deletions tests/app/questionnaire/test_placeholder_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -800,7 +800,7 @@ def test_placeholder_resolves_name_is_duplicate_chain(
}
]

mock_schema.is_repeating_answer = Mock(return_value=True)
mock_schema.get_list_name_for_answer_id = Mock(return_value="people")

parser = PlaceholderParser(
language="en",
Expand Down Expand Up @@ -911,7 +911,7 @@ def test_placeholder_resolves_list_has_items_chain(
}
]

mock_schema.is_repeating_answer = Mock(return_value=True)
mock_schema.get_list_name_for_answer_id = Mock(return_value="people")

parser = PlaceholderParser(
language="en",
Expand Down
Loading