feat: more error handling

`ExpectedAncestorError`, `InvalidTextPlacementError`, and `UnknownAttributeError`

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)

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}")

tests/questionpy_sdk/webserver/test_data/faulty.xhtml

@@ -8,8 +8,10 @@
             Invalid shuffle format.
             <qpy:shuffled-index format="invalid"/>. A
         </label>
+        Invalid text placement.
     </fieldset>
     <div>Missing placeholder.<?p missing invalid-cleaning-option?></div>
     <div>Empty placeholder.<?p      ?></div>
+    <qpy:shuffled-index/> <!-- Not inside a qpy:shuffle-contents element. -->
     <span no-attribute-value>Missing attribute value.</span>
 </div>

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:
         </div>
     """
     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 = """
         <div>
             <span>&lt;qpy:format-float xmlns:qpy="http://questionpy.org/ns/question" xmlns="http://www.w3.org/1999/xhtml" thousands-separator="maybe" precision="invalid"&gt;Unknown value.&lt;/qpy:format-float&gt;</span>
-            <fieldset><label>Invalid shuffle format.<span>1</span>. A</label></fieldset>
+            <fieldset>
+                <label>Invalid shuffle format.<span>1</span>. A</label>
+                Invalid text placement.
+            </fieldset>
             <div>Missing placeholder.</div>
             <div>Empty placeholder.</div>
             <span>Missing attribute value.</span>
@@ -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)