diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index f0ffd63b..517c2f9c 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -9,7 +9,7 @@ jobs: docker-push: runs-on: ubuntu-22.04 steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v4 - name: Build run: | docker build -t onsdigital/eq-questionnaire-validator:latest . diff --git a/.github/workflows/pull_request.yml b/.github/workflows/pull_request.yml index ce70e4f0..1a97926a 100644 --- a/.github/workflows/pull_request.yml +++ b/.github/workflows/pull_request.yml @@ -9,13 +9,13 @@ jobs: lint: runs-on: ubuntu-22.04 steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - run: | echo "PYTHON_VERSION=$(cat .python-version)" >> $GITHUB_ENV - - uses: actions/setup-python@v4 + - uses: actions/setup-python@v5 with: python-version: ${{ env.PYTHON_VERSION }} - - uses: actions/setup-node@v3 + - uses: actions/setup-node@v4 with: node-version-file: ".nvmrc" cache: "npm" @@ -33,7 +33,7 @@ jobs: virtualenvs-path: ~/.virtualenvs - name: Cache Poetry virtualenv - uses: actions/cache@v3 + uses: actions/cache@v4 id: cache-virtualenv with: path: ~/.virtualenvs @@ -47,13 +47,13 @@ jobs: test-unit: runs-on: ubuntu-22.04 steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - run: echo "PYTHON_VERSION=$(cat .python-version)" >> $GITHUB_ENV - - uses: actions/setup-python@v4 + - uses: actions/setup-python@v5 with: python-version: ${{ env.PYTHON_VERSION }} - - uses: actions/setup-node@v3 + - uses: actions/setup-node@v4 with: node-version-file: ".nvmrc" cache: "npm" @@ -73,7 +73,7 @@ jobs: virtualenvs-path: ~/.virtualenvs - name: Cache Poetry virtualenv - uses: actions/cache@v3 + uses: actions/cache@v4 id: cache-virtualenv with: path: ~/.virtualenvs @@ -87,7 +87,7 @@ jobs: docker-push: runs-on: ubuntu-22.04 steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - name: Tag run: | CLEAN_TAG=$(echo "${{ github.event.pull_request.head.ref }}" | tr / -) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index c4cc48f0..92646e48 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -10,7 +10,7 @@ jobs: env: TAG: ${{ github.event.release.tag_name }} steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v4 - name: Build run: | docker build -t onsdigital/eq-questionnaire-validator:$TAG . diff --git a/app/error_messages.py b/app/error_messages.py index 89b57283..7ef737e2 100644 --- a/app/error_messages.py +++ b/app/error_messages.py @@ -40,4 +40,7 @@ "Variants have more than one question type for block." ) PREVIEW_WITHOUT_INTRODUCTION_BLOCK = "No introduction block found. Introduction block is mandatory when using the preview questions feature." +ANSWER_REFERENCED_BEFORE_EXISTS = ( + "Answer '{answer_id}' referenced as source before it has been added." +) LIST_REFERENCED_BEFORE_CREATED = "List referenced as source before it has been created." diff --git a/app/validators/questionnaire_schema.py b/app/validators/questionnaire_schema.py index 16b87618..186b113c 100644 --- a/app/validators/questionnaire_schema.py +++ b/app/validators/questionnaire_schema.py @@ -620,6 +620,27 @@ def get_parent_section_for_block(self, block_id) -> dict | None: if block_id == block["id"]: return self.sections_by_id[section_id] + def get_parent_list_collector_for_add_block(self, block_id) -> dict | None: + for blocks in self.blocks_by_section_id.values(): + for block in blocks: + if ( + block["type"] == "ListCollector" + and block["add_block"]["id"] == block_id + ): + return block["id"] + + def get_parent_list_collector_for_repeating_block(self, block_id) -> dict | None: + for blocks in self.blocks_by_section_id.values(): + for block in blocks: + if block["type"] in [ + "ListCollector", + "ListCollectorContent", + ] and block.get("repeating_blocks"): + for repeating_block in block["repeating_blocks"]: + if repeating_block["id"] == block_id: + return block["id"] + return None + def is_block_in_repeating_section(self, block_id: str) -> bool: parent_section = self.get_parent_section_for_block(block_id) return parent_section and self.is_repeating_section(parent_section["id"]) @@ -653,3 +674,8 @@ def get_section_id_for_block(self, block: Mapping) -> str | None: def get_section_id_for_block_id(self, block_id: str) -> str | None: if block := self.get_block(block_id): return self.get_section_id_for_block(block) + + def get_section_index_for_section_id(self, section_id: str) -> int: + for index, section in enumerate(self.sections): + if section["id"] == section_id: + return index diff --git a/app/validators/questionnaire_validator.py b/app/validators/questionnaire_validator.py index ee754a50..a4d4bec6 100644 --- a/app/validators/questionnaire_validator.py +++ b/app/validators/questionnaire_validator.py @@ -13,6 +13,7 @@ ) from app.validators.sections.section_validator import SectionValidator from app.validators.validator import Validator +from app.validators.value_source_validator import ValueSourceValidator class QuestionnaireValidator(Validator): @@ -34,6 +35,7 @@ def validate(self): self.validate_duplicates() self.validate_smart_quotes() self.validate_white_spaces() + self.validate_answer_references() self.validate_list_references() for section in self.questionnaire_schema.sections: @@ -140,6 +142,118 @@ def validate_introduction_block(self): if not has_introduction_blocks: self.add_error(error_messages.PREVIEW_WITHOUT_INTRODUCTION_BLOCK) + def validate_answer_references(self): + + # Handling blocks in group + for group in self.questionnaire_schema.groups: + self.validate_answer_source_group(group) + + # Handling section level "enabled" rule + for index, section in enumerate(self.questionnaire_schema.sections): + self.validate_answer_source_section(section, index) + + def validate_answer_source_group(self, group): + identifier_references = get_object_containing_key(group, "source") + for path, identifier_reference, parent_block in identifier_references: + # set up default parent_block_id for later check (group or block level) + parent_block_id = None + if ( + "source" in identifier_reference + and identifier_reference["source"] == "answers" + ): + + source_block = self.questionnaire_schema.get_block_by_answer_id( + identifier_reference["identifier"] + ) + # Handling non-existing blocks used as source + if not source_block: + self.add_error( + ValueSourceValidator.ANSWER_SOURCE_REFERENCE_INVALID, + identifier=identifier_reference["identifier"], + ) + return False + # Handling block level answer sources (skipping group level) + if parent_block and "blocks" in path: + parent_block_id = parent_block["id"] + parent_block_index = self.questionnaire_schema.block_ids.index( + parent_block_id + ) + else: + # Handling group level skip conditions + first_block_id_in_group = group["blocks"][0]["id"] + parent_block_index = self.questionnaire_schema.block_ids.index( + first_block_id_in_group + ) + + source_block_id = self.resolve_source_block_id(source_block) + + source_block_index = self.questionnaire_schema.block_ids.index( + source_block_id + ) + if source_block_index > parent_block_index: + if parent_block_id: + self.add_error( + error_messages.ANSWER_REFERENCED_BEFORE_EXISTS.format( + answer_id=identifier_reference["identifier"] + ), + block_id=parent_block_id, + ) + else: + self.add_error( + error_messages.ANSWER_REFERENCED_BEFORE_EXISTS.format( + answer_id=identifier_reference["identifier"] + ), + group_id=group["id"], + ) + + def validate_answer_source_section(self, section, section_index): + identifier_references = get_object_containing_key(section, "source") + for path, identifier_reference, _ in identifier_references: + if ( + "source" in identifier_reference + and identifier_reference["source"] == "answers" + and "enabled" in path + ): + source_block = self.questionnaire_schema.get_block_by_answer_id( + identifier_reference["identifier"] + ) + source_block_id = self.resolve_source_block_id(source_block) + + source_block_section_id = ( + self.questionnaire_schema.get_section_id_for_block_id( + source_block_id + ) + ) + source_block_section_index = ( + self.questionnaire_schema.get_section_index_for_section_id( + source_block_section_id + ) + ) + if section_index < source_block_section_index: + self.add_error( + error_messages.ANSWER_REFERENCED_BEFORE_EXISTS.format( + answer_id=identifier_reference["identifier"] + ), + section_id=section["id"], + ) + + def resolve_source_block_id(self, source_block): + # Handling of source block nested (list collector's add-block) + if source_block["type"] == "ListAddQuestion": + return self.questionnaire_schema.get_parent_list_collector_for_add_block( + source_block["id"] + ) + + # Handling of source block nested (list collector's repeating block) + if source_block["type"] == "ListRepeatingQuestion": + return ( + self.questionnaire_schema.get_parent_list_collector_for_repeating_block( + source_block["id"] + ) + ) + # Handling of standard source block + return source_block["id"] + def validate_list_references(self): lists_with_context = self.questionnaire_schema.lists_with_context diff --git a/tests/schemas/invalid/test_invalid_answer_source_reference.json b/tests/schemas/invalid/test_invalid_answer_source_reference.json new file mode 100644 index 00000000..77ad2ee8 --- /dev/null +++ b/tests/schemas/invalid/test_invalid_answer_source_reference.json @@ -0,0 +1,1086 @@ +{ + "mime_type": "application/json/ons/eq", + "language": "en", + "schema_version": "0.0.1", + "survey_id": "139", + "theme": "default", + "title": "Confirmation Question Test", + "data_version": "0.0.3", + "description": "A questionnaire to test answers referenced as source before it has been added.", + "navigation": { + "visible": true + }, + "metadata": [ + { + "name": "user_id", + "type": "string" + }, + { + "name": "period_id", + "type": "string" + }, + { + "name": "ru_name", + "type": "string" + }, + { + "name": "trad_as", + "type": "string", + "optional": true + } + ], + "questionnaire_flow": { + "type": "Linear", + "options": { + "summary": { + "collapsible": false + } + } + }, + "sections": [ + { + "summary": { + "show_on_completion": true, + "items": [ + { + "type": "List", + "for_list": "people", + "title": "Household members", + "add_link_text": "Add someone to this household", + "empty_list_text": "There are no householders" + } + ] + }, + "id": "household-section", + "title": "Household", + "groups": [ + { + "id": "household-group", + "title": "List", + "blocks": [ + { + "id": "list-collector", + "type": "ListCollector", + "for_list": "people", + "question": { + "id": "confirmation-question", + "type": "General", + "title": "Does anyone else live here?", + "answers": [ + { + "id": "anyone-else", + "mandatory": true, + "type": "Radio", + "options": [ + { + "label": "Yes", + "value": "Yes", + "action": { + "type": "RedirectToListAddBlock" + } + }, + { + "label": "No", + "value": "No" + } + ] + } + ] + }, + "add_block": { + "id": "add-person", + "type": "ListAddQuestion", + "question": { + "id": "add-question", + "type": "General", + "title": "What is the name of the person?", + "answers": [ + { + "id": "first-name", + "label": "First name", + "mandatory": true, + "type": "TextField" + }, + { + "id": "last-name", + "label": "Last name", + "mandatory": true, + "type": "TextField" + } + ] + } + }, + "edit_block": { + "id": "edit-person", + "type": "ListEditQuestion", + "question": { + "id": "edit-question", + "type": "General", + "title": "What is the name of the person?", + "answers": [ + { + "id": "first-name", + "label": "First name", + "mandatory": true, + "type": "TextField" + }, + { + "id": "last-name", + "label": "Last name", + "mandatory": true, + "type": "TextField" + } + ] + } + }, + "remove_block": { + "id": "remove-person", + "type": "ListRemoveQuestion", + "question": { + "id": "remove-question", + "type": "General", + "title": "Are you sure you want to remove this person?", + "answers": [ + { + "id": "remove-confirmation", + "mandatory": true, + "type": "Radio", + "options": [ + { + "label": "Yes", + "value": "Yes", + "action": { + "type": "RemoveListItemAndAnswers" + } + }, + { + "label": "No", + "value": "No" + } + ] + } + ] + } + }, + "summary": { + "title": "Household members", + "item_title": { + "text": "{person_name}", + "placeholders": [ + { + "placeholder": "person_name", + "transforms": [ + { + "arguments": { + "delimiter": " ", + "list_to_concatenate": [ + { + "source": "answers", + "identifier": "first-name" + }, + { + "source": "answers", + "identifier": "last-name" + } + ] + }, + "transform": "concatenate_list" + } + ] + } + ] + } + } + } + ] + } + ] + }, + { + "id": "personal-details-section", + "title": "Personal Details", + "summary": { + "show_on_completion": true + }, + "repeat": { + "for_list": "people", + "title": { + "text": "{person_name}", + "placeholders": [ + { + "placeholder": "person_name", + "transforms": [ + { + "transform": "concatenate_list", + "arguments": { + "list_to_concatenate": [ + { + "source": "answers", + "identifier": "first-name" + }, + { + "source": "answers", + "identifier": "last-name" + } + ], + "delimiter": " " + } + } + ] + } + ] + } + }, + "groups": [ + { + "id": "personal-details-group", + "title": "Personal Details", + "blocks": [ + { + "id": "proxy", + "question": { + "answers": [ + { + "default": "Yes", + "id": "proxy-answer", + "mandatory": false, + "options": [ + { + "label": "No, I’m answering for myself", + "value": "No, I’m answering for myself" + }, + { + "label": "Yes", + "value": "Yes" + } + ], + "type": "Radio" + } + ], + "id": "proxy-question", + "title": "Are you answering the questions on behalf of someone else?", + "type": "General" + }, + "type": "Question" + }, + { + "id": "confirm-dob", + "question": { + "answers": [ + { + "id": "confirm-date-of-birth-answer", + "mandatory": true, + "options": [ + { + "label": "Yes, it is my age", + "value": "Yes, it is my age" + }, + { + "label": "No, I need to change my date of birth", + "value": "No, I need to change my date of birth" + } + ], + "type": "Radio" + } + ], + "id": "confirm-date-of-birth", + "title": { + "placeholders": [ + { + "placeholder": "person_name", + "transforms": [ + { + "arguments": { + "delimiter": " ", + "list_to_concatenate": [ + { + "source": "answers", + "identifier": "first-name" + }, + { + "source": "answers", + "identifier": "last-name" + } + ] + }, + "transform": "concatenate_list" + } + ] + }, + { + "placeholder": "age", + "transforms": [ + { + "arguments": { + "first_date": { + "identifier": "date-of-birth-answer", + "source": "answers" + }, + "second_date": { + "value": "now" + } + }, + "transform": "calculate_date_difference" + } + ] + } + ], + "text": "{person_name} is {age} old. Is this correct?" + }, + "type": "General" + }, + "routing_rules": [ + { + "block": "date-of-birth", + "when": { + "==": [ + { + "source": "answers", + "identifier": "confirm-date-of-birth-answer" + }, + "No, I need to change my date of birth" + ] + } + }, + { + "block": "date-of-birth", + "when": { + "==": [ + { + "source": "answers", + "identifier": "confirm-date-of-birth-answer" + }, + "No, I need to change my date of birth" + ] + } + }, + { + "block": "sex" + } + ], + "type": "ConfirmationQuestion" + }, + { + "id": "date-of-birth", + "question": { + "answers": [ + { + "id": "date-of-birth-answer", + "mandatory": true, + "maximum": { + "value": "now" + }, + "minimum": { + "offset_by": { + "years": -115 + }, + "value": "2019-10-13" + }, + "type": "Date" + } + ], + "guidance": { + "contents": [ + { + "description": "For example 31 12 1970" + } + ] + }, + "id": "date-of-birth-question", + "title": { + "placeholders": [ + { + "placeholder": "person_name_possessive", + "transforms": [ + { + "arguments": { + "delimiter": " ", + "list_to_concatenate": [ + { + "source": "answers", + "identifier": "first-name" + }, + { + "source": "answers", + "identifier": "last-name" + } + ] + }, + "transform": "concatenate_list" + }, + { + "arguments": { + "string_to_format": { + "source": "previous_transform" + } + }, + "transform": "format_possessive" + } + ] + } + ], + "text": "What is {person_name_possessive} date of birth?" + }, + "type": "General" + }, + "type": "Question" + }, + { + "id": "confirm-sex", + "question": { + "answers": [ + { + "id": "confirm-sex-answer", + "mandatory": true, + "options": [ + { + "label": "Yes, that is correct", + "value": "Yes, that is correct" + }, + { + "label": "No, that is not my sex", + "value": "No, that is not my sex" + } + ], + "type": "Radio" + } + ], + "id": "confirm-sex-question", + "title": { + "placeholders": [ + { + "placeholder": "person_name_possessive", + "transforms": [ + { + "arguments": { + "delimiter": " ", + "list_to_concatenate": [ + { + "source": "answers", + "identifier": "first-name" + }, + { + "source": "answers", + "identifier": "last-name" + } + ] + }, + "transform": "concatenate_list" + }, + { + "arguments": { + "string_to_format": { + "source": "previous_transform" + } + }, + "transform": "format_possessive" + } + ] + }, + { + "placeholder": "sex-answer", + "value": { + "source": "answers", + "identifier": "sex-answer" + } + } + ], + "text": "Is {person_name_possessive} sex - {sex-answer}?" + }, + "type": "General" + }, + "routing_rules": [ + { + "block": "sex", + "when": { + "==": [ + { + "source": "answers", + "identifier": "confirm-sex-answer" + }, + "No, that is not my sex" + ] + } + }, + { + "section": "End" + } + ], + "type": "ConfirmationQuestion" + }, + { + "id": "sex", + "question": { + "answers": [ + { + "id": "sex-answer", + "mandatory": false, + "options": [ + { + "label": "Female", + "value": "Female" + }, + { + "label": "Male", + "value": "Male" + } + ], + "type": "Radio" + } + ], + "guidance": { + "contents": [ + { + "description": "A question about gender will follow" + } + ] + }, + "id": "sex-question", + "title": { + "placeholders": [ + { + "placeholder": "person_name_possessive", + "transforms": [ + { + "arguments": { + "delimiter": " ", + "list_to_concatenate": [ + { + "source": "answers", + "identifier": "first-name" + }, + { + "source": "answers", + "identifier": "last-name" + } + ] + }, + "transform": "concatenate_list" + }, + { + "arguments": { + "string_to_format": { + "source": "previous_transform" + } + }, + "transform": "format_possessive" + } + ] + } + ], + "text": "What is {person_name_possessive} sex?" + }, + "type": "General" + }, + "type": "Question" + } + ] + } + ] + }, + { + "id": "confirmation-section", + "title": "Questions", + "groups": [ + { + "id": "confirmation-group", + "title": "Confirmation Question Group", + "skip_conditions": { + "when": { + ">": [ + { + "source": "answers", + "identifier": "number-of-employees-total" + }, + 3 + ] + } + }, + "blocks": [ + { + "id": "number-of-employees-split-block", + "type": "Question", + "question": { + "answers": [ + { + "id": "number-of-employees-more-30-hours", + "label": "Number of employees working more than 30 hours per week", + "mandatory": false, + "type": "Number", + "maximum": { + "value": 1000 + } + } + ], + "id": "number-of-employees-split-question", + "title": { + "text": "Of the {number_of_employees_total} total employees employed, how many male and female employees worked the following hours?", + "placeholders": [ + { + "placeholder": "number_of_employees_total", + "value": { + "source": "answers", + "identifier": "number-of-employees-total" + } + } + ] + }, + "type": "General" + }, + "routing_rules": [ + { + "when": { + "==": [ + { + "source": "answers", + "identifier": "number-of-employees-total" + }, + 2 + ] + }, + "section": "End" + }, + { + "block": "confirm-zero-employees-block" + } + ] + }, + { + "type": "ConfirmationQuestion", + "id": "confirm-zero-employees-block", + "skip_conditions": { + "when": { + ">": [ + { + "source": "answers", + "identifier": "number-of-employees-total" + }, + 0 + ] + } + }, + "question": { + "type": "General", + "answers": [ + { + "type": "Radio", + "id": "confirm-zero-employees-answer", + "options": [ + { + "label": "Yes this is correct", + "value": "Yes this is correct" + }, + { + "label": "No I need to correct this", + "value": "No I need to correct this" + } + ], + "mandatory": true + } + ], + "id": "confirm-zero-employees-question", + "title": { + "text": "The current number of employees for {company_name} is 0, is this correct?", + "placeholders": [ + { + "placeholder": "company_name", + "transforms": [ + { + "transform": "first_non_empty_item", + "arguments": { + "items": [ + { + "source": "metadata", + "identifier": "trad_as" + }, + { + "source": "metadata", + "identifier": "ru_name" + } + ] + } + } + ] + } + ] + } + } + } + ] + } + ], + "enabled": { + "when": { + "==": [ + { + "identifier": "number-of-employees-total", + "source": "answers" + }, + 1 + ] + } + } + }, + { + "id": "employees-section", + "title": "Questions", + "groups": [ + { + "id": "employees-block", + "title": "Employees Question Group", + "blocks": [ + { + "id": "number-of-employees-total-block", + "question": { + "answers": [ + { + "id": "number-of-employees-total", + "label": "Total number of employees", + "mandatory": false, + "type": "Number", + "default": 0 + } + ], + "id": "number-of-employees-total-question", + "title": { + "text": "How many employees work at {company_name}?", + "placeholders": [ + { + "placeholder": "company_name", + "transforms": [ + { + "transform": "first_non_empty_item", + "arguments": { + "items": [ + { + "source": "metadata", + "identifier": "trad_as" + }, + { + "source": "metadata", + "identifier": "ru_name" + } + ] + } + } + ] + } + ] + }, + "type": "General" + }, + "type": "Question" + } + ] + } + ] + }, + { + "id": "companies-section", + "title": "General insurance business", + "summary": { + "show_on_completion": true, + "items": [ + { + "type": "List", + "for_list": "companies", + "title": "Companies or UK branches", + "item_anchor_answer_id": "company-or-branch-name", + "item_label": "Name of UK company or branch", + "add_link_text": "Add another UK company or branch", + "empty_list_text": "No UK company or branch added" + } + ], + "show_non_item_answers": true + }, + "groups": [ + { + "id": "group-companies", + "blocks": [ + { + "id": "any-other-companies-or-branches", + "type": "ListCollector", + "for_list": "companies", + "question": { + "id": "any-other-companies-or-branches-question", + "type": "General", + "title": "Do you need to add any other UK companies or branches that undertake general insurance business?", + "answers": [ + { + "id": "any-other-companies-or-branches-answer", + "mandatory": true, + "type": "Radio", + "options": [ + { + "label": "Yes", + "value": "Yes", + "action": { + "type": "RedirectToListAddBlock" + } + }, + { + "label": "No", + "value": "No" + } + ] + } + ] + }, + "add_block": { + "id": "add-company", + "type": "ListAddQuestion", + "question": { + "id": "add-question-companies", + "type": "General", + "title": "What is the name and registration number of the company?", + "answers": [ + { + "id": "company-or-branch-name", + "label": "Name of UK company or branch (Mandatory)", + "mandatory": true, + "type": "TextField" + } + ] + } + }, + "repeating_blocks": [ + { + "id": "companies-repeating-block-1", + "type": "ListRepeatingQuestion", + "question": { + "id": "companies-repeating-block-1-question", + "type": "General", + "title": { + "text": "You answered {any_other} about adding companies. Give details about {company_name}", + "placeholders": [ + { + "placeholder": "any_other", + "value": { + "source": "answers", + "identifier": "any-companies-or-branches-answer" + } + }, + { + "placeholder": "company_name", + "value": { + "source": "answers", + "identifier": "company-or-branch-name" + } + } + ] + }, + "answers": [ + { + "id": "registration-number", + "label": "Registration number (Mandatory)", + "mandatory": true, + "type": "Number", + "maximum": { + "value": 999, + "exclusive": false + }, + "decimal_places": 0 + }, + { + "id": "registration-date", + "label": "Date of Registration (Mandatory)", + "mandatory": true, + "type": "Date", + "maximum": { + "value": "now" + } + } + ] + } + }, + { + "id": "companies-repeating-block-2", + "type": "ListRepeatingQuestion", + "question": { + "id": "companies-repeating-block-2-question", + "type": "General", + "title": { + "text": "Give details about how {company_name} has been trading over the past {date_difference}.", + "placeholders": [ + { + "placeholder": "company_name", + "value": { + "source": "answers", + "identifier": "company-or-branch-name" + } + }, + { + "placeholder": "date_difference", + "transforms": [ + { + "transform": "calculate_date_difference", + "arguments": { + "first_date": { + "source": "answers", + "identifier": "registration-date" + }, + "second_date": { + "value": "now" + } + } + } + ] + } + ] + }, + "answers": [ + { + "type": "Radio", + "label": "Has this company been trading in the UK? (Mandatory)", + "id": "authorised-trader-uk-radio", + "mandatory": true, + "options": [ + { + "label": "Yes", + "value": "Yes" + }, + { + "label": "No", + "value": "No" + } + ] + }, + { + "type": "Radio", + "label": "Has this company been trading in the EU? (Not mandatory)", + "id": "authorised-trader-eu-radio", + "mandatory": false, + "options": [ + { + "label": "Yes", + "value": "Yes" + }, + { + "label": "No", + "value": "No" + } + ] + } + ] + } + } + ], + "edit_block": { + "id": "edit-company", + "type": "ListEditQuestion", + "question": { + "id": "edit-question-companies", + "type": "General", + "title": "What is the name and registration number of the company?", + "answers": [ + { + "id": "company-or-branch-name", + "label": "Name of UK company or branch (Mandatory)", + "mandatory": true, + "type": "TextField" + } + ] + } + }, + "remove_block": { + "id": "remove-company", + "type": "ListRemoveQuestion", + "question": { + "id": "remove-question-companies", + "type": "General", + "title": "Are you sure you want to remove this company or UK branch?", + "answers": [ + { + "id": "remove-company-confirmation", + "mandatory": true, + "type": "Radio", + "options": [ + { + "label": "Yes", + "value": "Yes", + "action": { + "type": "RemoveListItemAndAnswers" + } + }, + { + "label": "No", + "value": "No" + } + ] + } + ] + } + }, + "summary": { + "title": "Companies or UK branches", + "item_title": { + "text": "{company_name}", + "placeholders": [ + { + "placeholder": "company_name", + "value": { + "source": "answers", + "identifier": "company-or-branch-name" + } + } + ] + } + } + }, + { + "type": "ListCollectorDrivingQuestion", + "id": "any-companies-or-branches", + "for_list": "companies", + "question": { + "type": "General", + "id": "any-companies-or-branches-question", + "title": "Do any companies or branches within your United Kingdom group undertake general insurance business?", + "answers": [ + { + "type": "Radio", + "id": "any-companies-or-branches-answer", + "mandatory": true, + "options": [ + { + "label": "Yes", + "value": "Yes", + "action": { + "type": "RedirectToListAddBlock", + "params": { + "block_id": "add-company", + "list_name": "companies" + } + } + }, + { + "label": "No", + "value": "No" + } + ] + } + ] + }, + "routing_rules": [ + { + "when": { + "==": [ + { + "source": "answers", + "identifier": "any-companies-or-branches-answer" + }, + "Yes" + ] + }, + "block": "any-other-companies-or-branches" + }, + { + "section": "End" + } + ] + } + ] + } + ] + } + ] +} diff --git a/tests/schemas/invalid/test_invalid_list_source_reference.json b/tests/schemas/invalid/test_invalid_list_source_reference.json index 67662b99..f6d8ab6e 100644 --- a/tests/schemas/invalid/test_invalid_list_source_reference.json +++ b/tests/schemas/invalid/test_invalid_list_source_reference.json @@ -39,65 +39,12 @@ { "content": { "contents": [ - { - "description": { - "placeholders": [ - { - "placeholder": "person_name", - "transforms": [ - { - "arguments": { - "delimiter": " ", - "list_to_concatenate": [ - { - "source": "answers", - "identifier": "first-name" - }, - { - "source": "answers", - "identifier": "last-name" - } - ] - }, - "transform": "concatenate_list" - } - ] - } - ], - "text": "In this section, we are going to ask you questions about {person_name}." - } - }, { "list": ["date of birth"], "title": "You will need to know personal details such as" } ], - "title": { - "placeholders": [ - { - "placeholder": "person_name", - "transforms": [ - { - "arguments": { - "delimiter": " ", - "list_to_concatenate": [ - { - "source": "answers", - "identifier": "first-name" - }, - { - "source": "answers", - "identifier": "last-name" - } - ] - }, - "transform": "concatenate_list" - } - ] - } - ], - "text": "{person_name}" - } + "title": "A person" }, "page_title": "Individual interstitial", "id": "individual-interstitial", @@ -156,32 +103,8 @@ } ], "id": "proxy-question", - "title": { - "placeholders": [ - { - "placeholder": "person_name", - "transforms": [ - { - "arguments": { - "delimiter": " ", - "list_to_concatenate": [ - { - "source": "answers", - "identifier": "first-name" - }, - { - "source": "answers", - "identifier": "last-name" - } - ] - }, - "transform": "concatenate_list" - } - ] - } - ], - "text": "Are you {person_name}?" - }, + "title": "Are you the person answering?", + "type": "General" }, "type": "Question" @@ -581,18 +504,7 @@ }, "answers": [ { - "label": { - "text": "Monthly expenditure on {utility_bill} bills", - "placeholders": [ - { - "placeholder": "utility_bill", - "value": { - "source": "answers", - "identifier": "utility-bill-name" - } - } - ] - }, + "label": "Monthly expenditure on bills", "id": "utility-bill-monthly-cost", "type": "Currency", "mandatory": true, diff --git a/tests/test_questionnaire_validator.py b/tests/test_questionnaire_validator.py index 0cccede6..d60a2410 100644 --- a/tests/test_questionnaire_validator.py +++ b/tests/test_questionnaire_validator.py @@ -96,6 +96,10 @@ def test_invalid_placeholder_answer_ids(): validator.validate() expected_errors = [ + { + "identifier": "invalid-answer0", + "message": ValueSourceValidator.ANSWER_SOURCE_REFERENCE_INVALID, + }, { "message": ValueSourceValidator.ANSWER_SOURCE_REFERENCE_INVALID, "identifier": "invalid-answer0", @@ -408,6 +412,60 @@ def test_invalid_calculated_or_grand_calculated_summary_id_in_value_source(): assert validator.errors == expected_errors +def test_answer_as_source_referenced_before_created(): + """ + The schema being validated contains blocks where an invalid answer source being referenced multiple times within that block, + resulting in duplicated expected errors. + """ + filename = "schemas/invalid/test_invalid_answer_source_reference.json" + validator = QuestionnaireValidator(_open_and_load_schema_file(filename)) + + expected_errors = [ + { + "block_id": "confirm-dob", + "message": "Answer 'date-of-birth-answer' referenced as source before it has " + "been added.", + }, + { + "block_id": "confirm-sex", + "message": "Answer 'sex-answer' referenced as source before it has been " + "added.", + }, + { + "group_id": "confirmation-group", + "message": "Answer 'number-of-employees-total' referenced as source before " + "it has been added.", + }, + { + "block_id": "number-of-employees-split-block", + "message": "Answer 'number-of-employees-total' referenced as source before " + "it has been added.", + }, + { + "block_id": "number-of-employees-split-block", + "message": "Answer 'number-of-employees-total' referenced as source before " + "it has been added.", + }, + { + "block_id": "confirm-zero-employees-block", + "message": "Answer 'number-of-employees-total' referenced as source before " + "it has been added.", + }, + { + "block_id": "any-other-companies-or-branches", + "message": "Answer 'any-companies-or-branches-answer' referenced as source " + "before it has been added.", + }, + { + "message": "Answer 'number-of-employees-total' referenced as source before " + "it has been added.", + "section_id": "confirmation-section", + }, + ] + validator.validate() + assert validator.errors == expected_errors + + def test_list_as_source_referenced_before_created(): filename = "schemas/invalid/test_invalid_list_source_reference.json" validator = QuestionnaireValidator(_open_and_load_schema_file(filename)) @@ -463,11 +521,10 @@ def test_list_as_source_referenced_before_created(): ] validator.validate() - assert validator.errors == expected_errors -def test_list_as_source_referenced_before_created_repeating_blocks(): +def test_list_and_answer_source_referenced_before_created_repeating_blocks(): filename = ( "schemas/invalid/test_invalid_list_source_reference_repeating_blocks.json" ) @@ -475,13 +532,17 @@ def test_list_as_source_referenced_before_created_repeating_blocks(): expected_errors = [ { + "block_id": "any-other-companies-or-branches", + "message": "Answer 'company-or-branch-name' referenced as source before it " + "has been added.", + }, + { + "block_id": "any-other-companies-or-branches", "list_id": "companies", "message": error_messages.LIST_REFERENCED_BEFORE_CREATED, - "block_id": "any-other-companies-or-branches", "section_id": "section-companies", - } + }, ] validator.validate() - assert validator.errors == expected_errors