From fb914783c4808e6d9d8d87b73cac0790d5452361 Mon Sep 17 00:00:00 2001
From: Michael Harbarth <michael.harbarth@deutschebahn.com>
Date: Thu, 1 Aug 2024 17:44:17 +0200
Subject: [PATCH] chore: additional changes from code review

---
 .../converters/element_converter.py           |  2 +-
 .../converters/polarion_html_helper.py        |  5 +-
 tests/test_documents.py                       | 62 ++++++++-----------
 3 files changed, 30 insertions(+), 39 deletions(-)

diff --git a/capella2polarion/converters/element_converter.py b/capella2polarion/converters/element_converter.py
index dda15ff7..99c04000 100644
--- a/capella2polarion/converters/element_converter.py
+++ b/capella2polarion/converters/element_converter.py
@@ -239,8 +239,8 @@ def __insert_diagram(
         diagram: capellambse.model.diagram.AbstractDiagram,
         file_name: str,
         render_params: dict[str, t.Any] | None = None,
+        max_width: int = 800,
     ):
-        max_width = 800
         if attachment := next(
             (
                 att
diff --git a/capella2polarion/converters/polarion_html_helper.py b/capella2polarion/converters/polarion_html_helper.py
index 4a7d4cf5..5b29fcdc 100644
--- a/capella2polarion/converters/polarion_html_helper.py
+++ b/capella2polarion/converters/polarion_html_helper.py
@@ -115,7 +115,10 @@ def remove_table_ids(
 ) -> list[etree._Element]:
     """Remove the ID field from all tables.
 
-    This is necessary due to a bug in Polarion.
+    This is necessary due to a bug in Polarion where Polarion does not
+    ensure that the tables added in the UI have unique IDs. At the same
+    time the REST-API does not allow posting or patching a document with
+    multiple tables having the same ID.
     """
     html_fragments = _ensure_fragments(html_content)
 
diff --git a/tests/test_documents.py b/tests/test_documents.py
index 03b33a61..8023d830 100644
--- a/tests/test_documents.py
+++ b/tests/test_documents.py
@@ -48,7 +48,9 @@ def test_create_new_document(
         cls="c710f1c2-ede6-444e-9e2b-0ff30d7fd040",
     )
 
-    content = html.fromstring(new_doc.home_page_content.value)
+    content: list[etree._Element] = html.fromstring(
+        new_doc.home_page_content.value
+    )
     assert len(wis) == 0
     assert new_doc.rendering_layouts == [
         polarion_api.RenderingLayout(
@@ -56,26 +58,17 @@ def test_create_new_document(
         )
     ]
     assert len(content) == 4
-    assert (
-        etree.tostring(content[0])
-        .decode("utf-8")
-        .startswith("<h1>Class Document</h1>")
+    assert content[0].tag == "h1"
+    assert content[0].text == "Class Document"
+    assert content[2].tag == "div"
+    assert content[2].get("id") == (
+        "polarion_wiki macro name=module-workitem;"
+        "params=id=ATSY-1234|layout=0|external=true"
     )
-    assert (
-        etree.tostring(content[2])
-        .decode("utf-8")
-        .startswith(
-            '<div id="polarion_wiki macro name=module-workitem;'
-            'params=id=ATSY-1234|layout=0|external=true"/>'
-        )
-    )
-    assert (
-        etree.tostring(content[3])
-        .decode("utf-8")
-        .startswith(
-            '<div id="polarion_wiki macro name=module-workitem;'
-            'params=id=ATSY-4321|layout=0|external=true"/>'
-        )
+    assert content[3].tag == "div"
+    assert content[3].get("id") == (
+        "polarion_wiki macro name=module-workitem;"
+        "params=id=ATSY-4321|layout=0|external=true"
     )
 
 
@@ -125,26 +118,21 @@ def test_update_document(
         cls="c710f1c2-ede6-444e-9e2b-0ff30d7fd040",
     )
 
-    content = html.fromstring(new_doc.home_page_content.value)
+    content: list[etree._Element] = html.fromstring(
+        new_doc.home_page_content.value
+    )
     assert len(new_doc.rendering_layouts) == 1
     assert new_doc.rendering_layouts[
         0
     ].properties == polarion_api.data_models.RenderingProperties(
         fields_at_start=["ID"]
     )
-    assert (
-        etree.tostring(content[0])
-        .decode("utf-8")
-        .startswith(
-            '<h1 id="polarion_wiki macro name='
-            'module-workitem;params=id=ATSY-16062"'
-        )
-    )
-    assert (
-        etree.tostring(content[1])
-        .decode("utf-8")
-        .startswith("<h2>Data Classes</h2>")
+    assert content[0].get("id") == (
+        "polarion_wiki macro name=module-workitem;params=id=ATSY-16062"
     )
+    assert content[0].tag == "h1"
+    assert content[1].text == "Data Classes"
+    assert content[1].tag == "h2"
     assert len(wis) == 1
     assert wis[0].id == "ATSY-16062"
     assert wis[0].title == "Class Document"
@@ -188,14 +176,14 @@ def test_mixed_authority_document(
 
     assert len(content) == 15
     assert [c.tag for c in content[:3]] == ["h1", "p", "p"]
-    assert (c4 := content[4]).tag == "h3" and c4.text == "New Heading" 
+    assert (c4 := content[4]).tag == "h3" and c4.text == "New Heading"
     assert content[5].text == "Global Test"
     assert content[6].text == "Local Test section 1"
     assert content[8].text == "This will be kept."
-    assert content[10].id == (
-        "polarion_wiki macro name=module-workitem;'
-        'params=id=ATSY-18305"'
+    assert content[10].get("id") == (
+        "polarion_wiki macro name=module-workitem;params=id=ATSY-18305"
     )
+    assert content[10].tag == "h3"
     assert content[11].text == "Overwritten: Overwrite global param"
     assert content[12].text == "Local Test section 2"
     assert content[14].text == "Some postfix stuff"