diff --git a/questionpy_sdk/webserver/question_ui/__init__.py b/questionpy_sdk/webserver/question_ui/__init__.py index 5acd59a..db41c5c 100644 --- a/questionpy_sdk/webserver/question_ui/__init__.py +++ b/questionpy_sdk/webserver/question_ui/__init__.py @@ -15,6 +15,7 @@ from questionpy_common.api.attempt import DisplayRole from questionpy_sdk.webserver.question_ui.errors import ( ConversionError, + DuplicateNameError, ExpectedAncestorError, InvalidAttributeValueError, InvalidCleanOptionError, @@ -594,6 +595,7 @@ def collect(self) -> RenderErrorCollection: self._validate_shuffle_contents_and_shuffled_index() self._validate_format_floats() self._look_for_unknown_qpy_elements_and_attributes() + self._check_input_names() return self.errors @@ -740,3 +742,42 @@ def _look_for_unknown_qpy_elements_and_attributes(self) -> None: if unknown_attributes: unknown_attribute_error = UnknownAttributeError(element=element, attributes=unknown_attributes) self.errors.insert(unknown_attribute_error) + + def _check_input_names(self) -> None: + """Check if there are only valid input names. + + Duplicate input names are only allowed for `checkbox` and `radio` elements, but only when their types match and + they have different values. + """ + # Maps element name to the reference element and the values. + name_map: dict[str, tuple[etree._Element, set[str]]] = {} + + for current_element in _assert_element_list( + self._xpath("(//xhtml:button | //xhtml:input | //xhtml:select | //xhtml:textarea)[@name]") + ): + # Get name, type, and value of the current element. + name = str(current_element.attrib["name"]) + current_type = current_element.get("type", "text") + current_value = current_element.get("value", "on") + + if name not in name_map: + # This name has not been used yet by other elements. + name_map[name] = (current_element, {current_value}) + continue + + # Get type and values of the other element(s) with the same name. + other_element, values = name_map[name] + other_type = other_element.get("type", "text") + + if current_type not in {"checkbox", "radio"}: + # Duplicate names are not allowed for other elements. + error = DuplicateNameError(element=current_element, name=name, other_element=other_element) + self.errors.insert(error) + continue + + # Check that the types match and the value is unique. + if other_type != current_type or current_value in values: + error = DuplicateNameError(element=current_element, name=name, other_element=other_element) + self.errors.insert(error) + else: + values.add(current_value) diff --git a/questionpy_sdk/webserver/question_ui/errors.py b/questionpy_sdk/webserver/question_ui/errors.py index c615244..579ecff 100644 --- a/questionpy_sdk/webserver/question_ui/errors.py +++ b/questionpy_sdk/webserver/question_ui/errors.py @@ -27,6 +27,16 @@ def _format_human_readable_list(values: Collection[str], opening: str, closing: return opening + f"{closing}, {opening}".join(values) + f"{closing} and {last_value}" +def _element_representation(element: etree._Element) -> str: + # Return the whole element if it is a PI. + if isinstance(element, etree._ProcessingInstruction): + return str(element) + + # Create the prefix of an element. We do not want to keep 'html' as a prefix. + prefix = f"{element.prefix}:" if element.prefix and element.prefix != "html" else "" + return prefix + etree.QName(element).localname + + @dataclass(frozen=True) class RenderError(ABC): """Represents a generic error which occurred during rendering.""" @@ -76,7 +86,7 @@ class RenderElementError(RenderError, ABC): def _message(self, *, as_html: bool) -> str: (opening, closing) = ("", "") if as_html else ("'", "'") - template_kwargs = {"element": f"{opening}{self.element_representation}{closing}"} + template_kwargs = {"element": f"{opening}{_element_representation(self.element)}{closing}"} for key, values in self.template_kwargs.items(): collection = {values} if isinstance(values, str) else values @@ -92,16 +102,6 @@ def message(self) -> str: def html_message(self) -> str: return self._message(as_html=True) - @property - def element_representation(self) -> str: - # Return the whole element if it is a PI. - if isinstance(self.element, etree._ProcessingInstruction): - return str(self.element) - - # Create the prefix of an element. We do not want to keep 'html' as a prefix. - prefix = f"{self.element.prefix}:" if self.element.prefix and self.element.prefix != "html" else "" - return prefix + etree.QName(self.element).localname - @property def line(self) -> int | None: """Original line number as found by the parser or None if unknown.""" @@ -234,6 +234,22 @@ def __init__(self, element: etree._Element, attributes: Collection[str]): ) +@dataclass(frozen=True) +class DuplicateNameError(RenderElementError): + """Invalid duplicate input name.""" + + def __init__(self, element: etree._Element, name: str, other_element: etree._Element): + super().__init__( + element=element, + template="{element} should not have the same name ({name}) like {other_element} at line {line}.", + template_kwargs={ + "name": name, + "other_element": _element_representation(other_element), + "line": str(other_element.sourceline or "?"), + }, + ) + + @dataclass(frozen=True) class XMLSyntaxError(RenderError): """Syntax error while parsing the XML.""" diff --git a/tests/questionpy_sdk/webserver/test_data/faulty.xhtml b/tests/questionpy_sdk/webserver/test_data/faulty.xhtml index 99a7ebb..a94f237 100644 --- a/tests/questionpy_sdk/webserver/test_data/faulty.xhtml +++ b/tests/questionpy_sdk/webserver/test_data/faulty.xhtml @@ -14,5 +14,7 @@
Empty placeholder.
Unknown attribute.
+ + Missing attribute value. diff --git a/tests/questionpy_sdk/webserver/test_data/input-values.xhtml b/tests/questionpy_sdk/webserver/test_data/input-values.xhtml index c4909f3..59b0877 100644 --- a/tests/questionpy_sdk/webserver/test_data/input-values.xhtml +++ b/tests/questionpy_sdk/webserver/test_data/input-values.xhtml @@ -15,8 +15,8 @@ - - + + diff --git a/tests/questionpy_sdk/webserver/test_question_ui.py b/tests/questionpy_sdk/webserver/test_question_ui.py index 16aae47..4a615ae 100644 --- a/tests/questionpy_sdk/webserver/test_question_ui.py +++ b/tests/questionpy_sdk/webserver/test_question_ui.py @@ -15,6 +15,7 @@ ) from questionpy_sdk.webserver.question_ui.errors import ( ConversionError, + DuplicateNameError, ExpectedAncestorError, InvalidAttributeValueError, InvalidCleanOptionError, @@ -210,8 +211,8 @@ def test_should_set_input_values(renderer: QuestionUIRenderer) -> None: - - + + @@ -242,8 +243,8 @@ def test_should_disable_inputs(renderer: QuestionUIRenderer) -> None: - - + + @@ -389,6 +390,8 @@ def test_errors_should_be_collected(renderer: QuestionUIRenderer) -> None:
Missing placeholder.
Empty placeholder.
Unknown attribute.
+ + Missing attribute value. """ # noqa: E501 @@ -396,7 +399,7 @@ 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, 17), + (XMLSyntaxError, 19), (InvalidAttributeValueError, 2), (UnknownElementError, 3), (InvalidAttributeValueError, 4), @@ -410,6 +413,7 @@ def test_errors_should_be_collected(renderer: QuestionUIRenderer) -> None: (PlaceholderReferenceError, 14), (ExpectedAncestorError, 15), (UnknownAttributeError, 16), + (DuplicateNameError, 18), ] assert len(errors) == len(expected_errors)