diff --git a/questionpy_sdk/webserver/question_ui/__init__.py b/questionpy_sdk/webserver/question_ui/__init__.py index b311f3ba..900d571e 100644 --- a/questionpy_sdk/webserver/question_ui/__init__.py +++ b/questionpy_sdk/webserver/question_ui/__init__.py @@ -15,10 +15,13 @@ from questionpy_sdk.webserver.question_ui.errors import ( ConversionError, + ExpectedAncestorError, InvalidAttributeValueError, InvalidCleanOptionError, + InvalidTextPlacementError, PlaceholderReferenceError, RenderErrorCollection, + UnknownAttributeError, UnknownElementError, XMLSyntaxError, ) @@ -582,9 +585,9 @@ def collect(self) -> RenderErrorCollection: self._validate_placeholders() self._validate_feedback() self._validate_if_role() - self._validate_shuffle_contents() + self._validate_shuffle_contents_and_shuffled_index() self._validate_format_floats() - self._look_for_unknown_qpy_elements() + self._look_for_unknown_qpy_elements_and_attributes() return self.errors @@ -639,11 +642,8 @@ def _validate_if_role(self) -> None: ) self.errors.insert(error) - def _validate_shuffle_contents(self) -> None: + def _validate_shuffle_contents_and_shuffled_index(self) -> None: """Validates elements marked with `qpy:shuffle-contents`.""" - # TODO: - check if shuffle-contents has children - # - check if shuffled-index elements have a parent element marked with qpy:shuffle-contests - for element in _assert_element_list(self._xpath("//*[@qpy:shuffle-contents]")): child_elements = [child for child in element if isinstance(child, etree._Element)] for child in child_elements: @@ -652,7 +652,24 @@ def _validate_shuffle_contents(self) -> None: ): format_style = index_element.get("format", "123") if format_style not in {"123", "abc", "ABC", "iii", "III"}: - self.errors.insert(InvalidAttributeValueError(index_element, "format", format_style)) + attribute_error = InvalidAttributeValueError( + element=index_element, attribute="format", value=format_style + ) + self.errors.insert(attribute_error) + + # Gather every qpy:shuffle-contents with direct text nodes. + for element in _assert_element_list( + self._xpath("//*[@qpy:shuffle-contents and text()[normalize-space()] != '']") + ): + placement_error = InvalidTextPlacementError(element=element, attribute="qpy:shuffle-contents") + self.errors.insert(placement_error) + + # Gather every qpy:shuffled-index without qpy:shuffle-contents ancestor. + for element in _assert_element_list( + self._xpath("//qpy:shuffled-index[not(ancestor::*[@qpy:shuffle-contents])]") + ): + ancestor_error = ExpectedAncestorError(element=element, expected_ancestor_attribute="qpy:shuffle-contents") + self.errors.insert(ancestor_error) def _validate_format_floats(self) -> None: """Validates the `qpy:format-float` element.""" @@ -692,13 +709,22 @@ def _validate_format_floats(self) -> None: ) self.errors.insert(thousands_sep_error) - def _look_for_unknown_qpy_elements(self) -> None: - """Checks if there are any unknown qpy-elements. - - TODO: also look for unknown qpy-attributes - """ + def _look_for_unknown_qpy_elements_and_attributes(self) -> None: + """Checks if there are any unknown qpy-elements or -attributes.""" known_elements = ["shuffled-index", "format-float"] - xpath = " and ".join([f"name() != 'qpy:{element}'" for element in known_elements]) - for element in _assert_element_list(self._xpath(f"//*[starts-with(name(), 'qpy:') and {xpath}]")): - error = UnknownElementError(element=element) - self.errors.insert(error) + xpath_elements = " and ".join(f"name() != 'qpy:{element}'" for element in known_elements) + for element in _assert_element_list(self._xpath(f"//*[starts-with(name(), 'qpy:') and {xpath_elements}]")): + unknown_element_error = UnknownElementError(element=element) + self.errors.insert(unknown_element_error) + + known_attrs = ["feedback", "if-role", "shuffle-contents", "correct-response"] + xpath_attrs = " and ".join(f"name() != 'qpy:{attr}'" for attr in known_attrs) + for element in _assert_element_list(self._xpath(f"//*[@*[starts-with(name(), 'qpy:') and {xpath_attrs}]]")): + unknown_attributes: list[str] = [ + attr.replace(f"{{{_QPY_NAMESPACE}}}", "qpy:") + for attr in map(str, element.attrib) + if attr.startswith(f"{{{_QPY_NAMESPACE}}}") and attr.split("}")[1] not in known_attrs + ] + if unknown_attributes: + unknown_attribute_error = UnknownAttributeError(element=element, attributes=unknown_attributes) + self.errors.insert(unknown_attribute_error) diff --git a/questionpy_sdk/webserver/question_ui/errors.py b/questionpy_sdk/webserver/question_ui/errors.py index 6e0f2968..7f4e0f6c 100644 --- a/questionpy_sdk/webserver/question_ui/errors.py +++ b/questionpy_sdk/webserver/question_ui/errors.py @@ -180,6 +180,31 @@ def __init__(self, element: etree._Element, option: str, expected: Collection[st ) +@dataclass(frozen=True) +class InvalidTextPlacementError(RenderElementError): + """Invalid text placement.""" + + def __init__(self, element: etree._Element, attribute: str): + super().__init__( + element=element, + template="Avoid placing text directly inside {element} with the {attribute} attribute. Use child elements " + "for text instead.", + template_kwargs={"attribute": attribute}, + ) + + +@dataclass(frozen=True) +class ExpectedAncestorError(RenderElementError): + """Invalid element placement.""" + + def __init__(self, element: etree._Element, expected_ancestor_attribute: str): + super().__init__( + element=element, + template="{element} must be placed inside an element with the {expected_ancestor_attribute} attribute.", + template_kwargs={"expected_ancestor_attribute": expected_ancestor_attribute}, + ) + + @dataclass(frozen=True) class UnknownElementError(RenderElementError): """Unknown element with qpy-namespace.""" @@ -191,6 +216,19 @@ def __init__(self, element: etree._Element): ) +@dataclass(frozen=True) +class UnknownAttributeError(RenderElementError): + """Unknown attribute with qpy-namespace.""" + + def __init__(self, element: etree._Element, attributes: Collection[str]): + s = "" if len(attributes) == 1 else "s" + super().__init__( + element=element, + template=f"Unknown attribute{s} {{attributes}} on element {{element}}.", + template_kwargs={"attributes": attributes}, + ) + + @dataclass(frozen=True) class XMLSyntaxError(RenderError): """Syntax error while parsing the XML.""" @@ -247,5 +285,5 @@ def log_render_errors(render_errors: RenderErrorCollections) -> None: line = f"Line {error.line}: " if error.line else "" errors_string += f"\n\t- {line}{error.type} - {error.message}" error_count = len(errors) - s = "s" if error_count > 1 else "" + s = "" if error_count == 1 else "s" _log.warning(f"{error_count} error{s} occurred while rendering {section}:{errors_string}") diff --git a/tests/questionpy_sdk/webserver/test_data/faulty.xhtml b/tests/questionpy_sdk/webserver/test_data/faulty.xhtml index 9c5e981e..c7a10617 100644 --- a/tests/questionpy_sdk/webserver/test_data/faulty.xhtml +++ b/tests/questionpy_sdk/webserver/test_data/faulty.xhtml @@ -8,8 +8,10 @@ Invalid shuffle format. . A + Invalid text placement.
Missing placeholder.
Empty placeholder.
+ Missing attribute value. diff --git a/tests/questionpy_sdk/webserver/test_question_ui.py b/tests/questionpy_sdk/webserver/test_question_ui.py index cea57767..49ea71a9 100644 --- a/tests/questionpy_sdk/webserver/test_question_ui.py +++ b/tests/questionpy_sdk/webserver/test_question_ui.py @@ -8,6 +8,7 @@ from questionpy_sdk.webserver.question_ui import ( InvalidAttributeValueError, + InvalidTextPlacementError, QuestionDisplayOptions, QuestionDisplayRole, QuestionFormulationUIRenderer, @@ -17,6 +18,7 @@ ) from questionpy_sdk.webserver.question_ui.errors import ( ConversionError, + ExpectedAncestorError, InvalidCleanOptionError, PlaceholderReferenceError, RenderError, @@ -336,7 +338,7 @@ def test_clean_up(renderer: QuestionUIRenderer) -> None: """ html, errors = renderer.render() - assert len(errors) == 0 + assert len(errors) == 1 # Undefined attribute. assert_html_is_equal(html, expected) @@ -363,7 +365,10 @@ def test_errors_should_be_collected(renderer: QuestionUIRenderer) -> None: expected = """
<qpy:format-float xmlns:qpy="http://questionpy.org/ns/question" xmlns="http://www.w3.org/1999/xhtml" thousands-separator="maybe" precision="invalid">Unknown value.</qpy:format-float> -
+
+ + Invalid text placement. +
Missing placeholder.
Empty placeholder.
Missing attribute value. @@ -373,17 +378,19 @@ def test_errors_should_be_collected(renderer: QuestionUIRenderer) -> None: expected_errors: list[tuple[type[RenderError], int]] = [ # Even though the syntax error occurs after all the other errors, it should be listed first. - (XMLSyntaxError, 14), + (XMLSyntaxError, 16), (InvalidAttributeValueError, 2), (UnknownElementError, 3), (InvalidAttributeValueError, 4), (ConversionError, 5), (ConversionError, 5), (InvalidAttributeValueError, 5), + (InvalidTextPlacementError, 6), (InvalidAttributeValueError, 9), - (InvalidCleanOptionError, 12), - (PlaceholderReferenceError, 12), + (InvalidCleanOptionError, 13), (PlaceholderReferenceError, 13), + (PlaceholderReferenceError, 14), + (ExpectedAncestorError, 15), ] assert len(errors) == len(expected_errors)