chore: Additional changes from review

capella2polarion/connectors/polarion_repo.py

@@ -6,6 +6,7 @@
 import collections.abc as cabc
 
 import bidict
+import polarion_rest_api_client as polarion_api
 
 from capella2polarion import data_models
 
@@ -114,3 +115,15 @@ def remove_work_items_by_capella_uuid(self, uuids: cabc.Iterable[str]):
         for uuid in uuids:
             del self._work_items[uuid]
             del self._id_mapping[uuid]
+
+
+DocumentRepository = dict[
+    tuple[str | None, str, str],
+    tuple[polarion_api.Document | None, list[polarion_api.WorkItem]],
+]
+"""A dict providing a mapping for documents and their text workitems.
+
+It has (project, space, name) of the document as key and (document,
+workitems) as value. The project can be None and the None value means
+that the document is in the same project as the model sync work items.
+"""

capella2polarion/connectors/polarion_worker.py

@@ -33,6 +33,7 @@
     "EXISTS (SELECT rel1.* FROM POLARION.REL_MODULE_WORKITEM rel1 WHERE "
     "rel1.FK_URI_MODULE = doc.C_URI AND rel1.FK_URI_WORKITEM = item.C_URI))"
 )
+"""An SQL query to get work items which are inserted in a given document."""
 
 
 class PolarionWorkerParams:
@@ -142,7 +143,7 @@ def delete_orphaned_work_items(
             if work_item.status != "deleted"
         }
         uuids: set[str] = existing_work_items - set(converter_session)
-        work_items: list[data_models.CapellaWorkitem] = []
+        work_items: list[data_models.CapellaWorkItem] = []
         for uuid in uuids:
             if wi := self.polarion_data_repo.get_work_item_by_capella_uuid(
                 uuid
@@ -527,7 +528,7 @@ def get_document(
         self, space: str, name: str, document_project: str | None = None
     ) -> polarion_api.Document | None:
         """Get a document from polarion and return None if not found.
-        
+
         Notes
         -----
         If the ``document_project`` is ``None`` the default client is
@@ -546,10 +547,7 @@ def get_document(
     def load_polarion_documents(
         self,
         document_infos: t.Iterable[data_models.DocumentInfo],
-    ) -> dict[
-        tuple[str | None, str, str],
-        tuple[polarion_api.Document | None, list[polarion_api.WorkItem]],
-    ]:
+    ) -> polarion_repo.DocumentRepository:
         """Load the documents referenced and text work items from Polarion."""
         return {
             (di.project_id, di.module_folder, di.module_name): (

capella2polarion/converters/document_renderer.py

@@ -70,10 +70,7 @@ def __init__(
         self.overwrite_heading_numbering = overwrite_heading_numbering
         self.overwrite_layouts = overwrite_layouts
         self.projects: dict[str | None, ProjectData] = {}
-        self.existing_documents: dict[
-            tuple[str | None, str, str],
-            tuple[polarion_api.Document | None, list[polarion_api.WorkItem]],
-        ] = {}
+        self.existing_documents: polarion_repo.DocumentRepository = {}
 
     def setup_env(self, env: jinja2.Environment):
         """Add globals and filters to the environment."""
@@ -354,10 +351,7 @@ def _get_and_customize_doc(
     def render_documents(
         self,
         configs: document_config.DocumentConfigs,
-        existing_documents: dict[
-            tuple[str | None, str, str],
-            tuple[polarion_api.Document | None, list[polarion_api.WorkItem]],
-        ],
+        existing_documents: polarion_repo.DocumentRepository,
     ) -> dict[str | None, ProjectData]:
         """Render all documents defined in the given config.
 

capella2polarion/converters/polarion_html_helper.py

@@ -10,7 +10,7 @@
 import jinja2
 import polarion_rest_api_client as polarion_api
 from capellambse import helpers as chelpers
-from lxml import etree, html
+from lxml import html
 
 wi_id_prefix = "polarion_wiki macro name=module-workitem;params=id="
 h_regex = re.compile("h[0-9]")
@@ -114,8 +114,8 @@ def setup_env(self, env: jinja2.Environment):
 
 
 def remove_table_ids(
-    html_content: str | list[html.HtmlElement],
-) -> list[etree._Element]:
+    html_content: str | list[html.HtmlElement | str],
+) -> list[html.HtmlElement | str]:
     """Remove the ID field from all tables.
 
     This is necessary due to a bug in Polarion where Polarion does not
@@ -126,43 +126,48 @@ def remove_table_ids(
     html_fragments = ensure_fragments(html_content)
 
     for element in html_fragments:
+        if not isinstance(element, html.HtmlElement):
+            continue
+
         if element.tag == "table":
             element.attrib.pop("id", None)
 
     return html_fragments
 
 
 def ensure_fragments(
-    html_content: str | list[html.HtmlElement],
-) -> list[html.HtmlElement]:
+    html_content: str | list[html.HtmlElement | str],
+) -> list[html.HtmlElement | str]:
     """Convert string to html elements."""
     if isinstance(html_content, str):
         return html.fragments_fromstring(html_content)
     return html_content
 
 
-def extract_headings(html_content: str | list[html.HtmlElement]) -> list[str]:
+def extract_headings(
+    html_content: str | list[html.HtmlElement | str],
+) -> list[str]:
     """Return a list of work item IDs for all headings in the given content."""
     return extract_work_items(html_content, h_regex)
 
 
 def extract_work_items(
-    html_content: str | list[html.HtmlElement],
+    html_content: str | list[html.HtmlElement | str],
     tag_regex: re.Pattern | None = None,
 ) -> list[str]:
     """Return a list of work item IDs for work items in the given content."""
-    work_items = []
+    work_item_ids: list[str] = []
     html_fragments = ensure_fragments(html_content)
     for element in html_fragments:
-        if isinstance(element, html.HtmlComment):
+        if not isinstance(element, html.HtmlElement):
             continue
 
         if (tag_regex is not None and tag_regex.fullmatch(element.tag)) or (
             tag_regex is None and element.tag == "div"
         ):
             if matches := wi_id_regex.match(element.get("id")):
-                work_items.append(matches.group(1))
-    return work_items
+                work_item_ids.append(matches.group(1))
+    return work_item_ids
 
 
 def get_layout_index(

capella2polarion/converters/text_work_item_provider.py

@@ -100,7 +100,7 @@ def insert_text_work_items(
         new_content = []
         last_match = -1
         for index, element in enumerate(html_fragments):
-            if isinstance(element, html.HtmlComment):
+            if not isinstance(element, html.HtmlElement):
                 continue
 
             if element.tag == "workitem":

tests/test_documents.py

@@ -6,7 +6,7 @@
 from lxml import etree, html
 
 from capella2polarion import data_models as dm
-from capella2polarion.connectors import polarion_worker
+from capella2polarion.connectors import polarion_repo, polarion_worker
 from capella2polarion.converters import (
     document_config,
     document_renderer,
@@ -28,10 +28,7 @@
 MIXED_AUTHORITY_DOCUMENT = TEST_DOCUMENT_ROOT / "mixed_authority_doc.html"
 
 
-def existing_documents() -> dict[
-    tuple[str | None, str, str],
-    tuple[polarion_api.Document, list[polarion_api.WorkItem]],
-]:
+def existing_documents() -> polarion_repo.DocumentRepository:
     return {
         (None, "_default", "id123"): (
             polarion_api.Document(

tests/test_polarion_worker_documents.py

@@ -44,9 +44,8 @@ def test_update_document(
     document_data.text_work_item_provider.generate_text_work_items(
         document.home_page_content.value
     )
-    empty_polarion_worker.project_client.work_items.create.side_effect = (
-        _set_work_item_id
-    )
+    client = empty_polarion_worker.project_client
+    client.work_items.create.side_effect = _set_work_item_id
 
     empty_polarion_worker.update_documents([document_data])
 
@@ -56,47 +55,13 @@ def test_update_document(
         '<div id="polarion_wiki macro name=module-workitem;'
         'params=id=id0|layout=0|external=true"></div>'
     )
-    assert (
-        empty_polarion_worker.project_client.documents.update.call_count == 1
-    )
-    assert (
-        empty_polarion_worker.project_client.documents.update.call_args.args[0]
-        == [document]
-    )
-    assert (
-        empty_polarion_worker.project_client.work_items.create.call_count == 1
-    )
-    assert (
-        len(
-            empty_polarion_worker.project_client.work_items.create.call_args.args[
-                0
-            ]
-        )
-        == 1
-    )
-    assert (
-        empty_polarion_worker.project_client.work_items.update.call_count == 2
-    )
-    assert (
-        len(
-            empty_polarion_worker.project_client.work_items.update.call_args_list[
-                0
-            ].args[
-                0
-            ]
-        )
-        == 1
-    )
-    assert (
-        len(
-            empty_polarion_worker.project_client.work_items.update.call_args_list[
-                1
-            ].args[
-                0
-            ]
-        )
-        == 0
-    )
+    assert client.documents.update.call_count == 1
+    assert client.documents.update.call_args.args[0] == [document]
+    assert client.work_items.create.call_count == 1
+    assert len(client.work_items.create.call_args.args[0]) == 1
+    assert client.work_items.update.call_count == 2
+    assert len(client.work_items.update.call_args_list[0].args[0]) == 1
+    assert len(client.work_items.update.call_args_list[1].args[0]) == 0
 
 
 def test_create_document(
@@ -123,9 +88,8 @@ def test_create_document(
     document_data.text_work_item_provider.generate_text_work_items(
         document.home_page_content.value
     )
-    empty_polarion_worker.project_client.work_items.create.side_effect = (
-        _set_work_item_id
-    )
+    client = empty_polarion_worker.project_client
+    client.work_items.create.side_effect = _set_work_item_id
 
     empty_polarion_worker.update_documents([document_data])
 
@@ -135,44 +99,10 @@ def test_create_document(
         '<div id="polarion_wiki macro name=module-workitem;'
         'params=id=id1|layout=0|external=true"></div>'
     )
-    assert (
-        empty_polarion_worker.project_client.documents.update.call_count == 1
-    )
-    assert (
-        empty_polarion_worker.project_client.documents.update.call_args.args[0]
-        == [document]
-    )
-    assert (
-        empty_polarion_worker.project_client.work_items.create.call_count == 1
-    )
-    assert (
-        len(
-            empty_polarion_worker.project_client.work_items.create.call_args.args[
-                0
-            ]
-        )
-        == 2
-    )
-    assert (
-        empty_polarion_worker.project_client.work_items.update.call_count == 2
-    )
-    assert (
-        len(
-            empty_polarion_worker.project_client.work_items.update.call_args_list[
-                0
-            ].args[
-                0
-            ]
-        )
-        == 0
-    )
-    assert (
-        len(
-            empty_polarion_worker.project_client.work_items.update.call_args_list[
-                1
-            ].args[
-                0
-            ]
-        )
-        == 0
-    )
+    assert client.documents.update.call_count == 1
+    assert client.documents.update.call_args.args[0] == [document]
+    assert client.work_items.create.call_count == 1
+    assert len(client.work_items.create.call_args.args[0]) == 2
+    assert client.work_items.update.call_count == 2
+    assert len(client.work_items.update.call_args_list[0].args[0]) == 0
+    assert len(client.work_items.update.call_args_list[1].args[0]) == 0