feat(confluence): copy the requested template, and delete storage that is deleted

docs/docs/administration/settings/plugins/configuring-atlassian-confluence.mdx

@@ -26,10 +26,6 @@ Dispatch ships with Atlassian Confluence storage and document plugins. This page
 
 > API token to access the confluence instance. Please refer to the [link](https://support.atlassian.com/atlassian-account/docs/manage-api-tokens-for-your-atlassian-account/) on creating a new API token.
 
-### `Incident template ID` \[Required\]
-
-> This is the page id of the template that contains the post incident review document details. The plugin uses this template to create a new page and then replace the supported variables with the incident details.
-
 ### `Root Incident Storage Page ID` \[Required\]
 
 > Page id of the page each incident's own page is created under, found in the page URL: `/pages/<id>/<title>`.

src/dispatch/plugins/dispatch_atlassian_confluence/client.py

@@ -26,6 +26,10 @@
 )
 
 
+class ConfluenceError(Exception):
+    """A refusal the 5.x client reports without raising one of its own."""
+
+
 class ConfluenceApi:
     """The Confluence operations Dispatch performs, over one 5.x client."""
 
@@ -88,6 +92,10 @@ def _parent(self, page_id: str) -> dict:
     def get_page(self, page_id: str) -> dict:
         raise NotImplementedError
 
+    def delete_page(self, page_id: str) -> None:
+        """Removes a page and everything filed beneath it."""
+        raise NotImplementedError
+
     def update_page(self, *, page: dict, body: str) -> dict:
         """Rewrites the body of a page already read with ``get_page``.
 
@@ -144,6 +152,22 @@ def _parent(self, page_id: str) -> dict:
     def get_page(self, page_id: str) -> dict:
         return self.client.get_page_by_id(page_id, body_format="storage")
 
+    def delete_page(self, page_id: str) -> None:
+        # Cloud trashes only the page it is given and re-parents the children
+        # onto their grandparent, so an incident's sub-pages would resurface on
+        # the storage root. Depth-first, and each level is listed in full
+        # before any of it is removed.
+        for child_id in self._child_ids(page_id):
+            self.delete_page(child_id)
+        self.client.delete_page(page_id)
+
+    def _child_ids(self, page_id: str) -> list[str]:
+        # Not get_child_pages: it addresses `/children/page`, which is not an
+        # endpoint, and asks for the body-format the API rejects. Its paging
+        # helper is sound, though, and cursors are not worth restating.
+        children = self.client._get_paged(f"api/v2/pages/{page_id}/children", params={"limit": 250})
+        return [child["id"] for child in children]
+
     def update_page(self, *, page: dict, body: str) -> dict:
         # PageUpdateRequest requires id, status, title, body and version, and
         # the client sends `status` only when asked to. Supplying `version` is
@@ -194,6 +218,18 @@ def _parent(self, page_id: str) -> dict:
     def get_page(self, page_id: str) -> dict:
         return self.client.get_page_by_id(page_id, expand="body.storage", status=None, version=None)
 
+    def delete_page(self, page_id: str) -> None:
+        # Not remove_page(recursive=True): it issues the DELETE in advanced
+        # mode, which skips raise_for_status, so its own error handling never
+        # runs and a refusal comes back as a status code nobody reads. Each
+        # level is listed in full before any of it goes -- the collection
+        # shifts under a paged read otherwise.
+        for child in list(self.client.get_page_child_by_type(page_id)):
+            self.delete_page(child["id"])
+        status = self.client.remove_page(page_id)
+        if not 200 <= status < 300:
+            raise ConfluenceError(f"Confluence refused to delete page {page_id}: HTTP {status}")
+
     def update_page(self, *, page: dict, body: str) -> dict:
         # v1 reads the version from the page's history itself.
         return self.client.update_page(

src/dispatch/plugins/dispatch_atlassian_confluence/plugin.py

@@ -35,9 +35,6 @@ def describe(exception: Exception) -> str:
 class ConfluenceConfiguration(ConfluenceConfigurationBase):
     """Confluence configuration description."""
 
-    template_id: str = Field(
-        title="Incident template ID", description="This is the page id of the template."
-    )
     root_id: str = Field(
         title="Root Incident Storage Page ID",
         description=(
@@ -105,11 +102,16 @@ def create_file(
             )
 
     def copy_file(self, folder_id: str, file_id: str, name: str):
-        # TODO : This is the function that is responsible for making the incident documents.
+        """Copies the page it is given into the page it is given.
+
+        `file_id` is the template Dispatch resolved for this document -- the
+        incident type's own template, the executive report's, or the form
+        export's. They are different pages, and which one this is is the
+        caller's decision to make.
+        """
         try:
             api = confluence_api(self.configuration)
-            logger.info(f"Copy_file function with args {folder_id}, {file_id}, {name}")
-            template_content = api.get_page(self.configuration.template_id)
+            template_content = api.get_page(file_id)
             page_details = api.create_page(
                 parent_id=folder_id,
                 title=name,
@@ -123,6 +125,14 @@ def copy_file(self, folder_id: str, file_id: str, name: str):
         except Exception as e:
             logger.error(f"Exception happened while creating page {name!r}: {describe(e)}")
 
+    def delete_file(self, file_id: str, **kwargs):
+        """Removes a page and everything filed beneath it.
+
+        Failures are left to `delete_storage`, which logs them: swallowing one
+        here would report storage as cleaned up when it is still there.
+        """
+        confluence_api(self.configuration).delete_page(file_id)
+
     def move_file(self, new_folder_id: str, file_id: str, **kwargs):
         """Moves a file from one place to another. Not used in the plugin,
         keeping the body as the interface is needed to avoid exceptions.

tests/plugins/dispatch_atlassian_confluence/fake_confluence.py

@@ -71,7 +71,6 @@ def build_configuration(hosting_type: str, **overrides):
         "hosting_type": hosting_type,
         "username": USERNAME,
         "password": PASSWORD,
-        "template_id": TEMPLATE_ID,
         "root_id": ROOT_PAGE_ID,
     }
     values.update(overrides)
@@ -176,18 +175,25 @@ class FakeConfluence:
     def __init__(self):
         self.requests = []
         self.failure = None
+        self.failing_methods = ()
         self.omit_webui = False
         self.next_id = 900001
+        self.deleted = []
         self.pages = {
             ROOT_PAGE_ID: Page(ROOT_PAGE_ID, "Incidents", "<p>Incident home</p>"),
             TEMPLATE_ID: Page(TEMPLATE_ID, "Incident Template", TEMPLATE_BODY),
         }
 
     # -- what comes back ---------------------------------------------------
 
-    def fail_with(self, status: int, body: dict | None = None):
-        """Every subsequent request fails, in Confluence's error envelope."""
+    def fail_with(self, status: int, body: dict | None = None, methods: tuple = ()):
+        """Subsequent requests fail, in Confluence's error envelope.
+
+        ``methods`` narrows it to those verbs, which is the only way to reach
+        a refusal of the delete itself rather than of the read before it.
+        """
         self.failure = (status, body if body is not None else {"message": "boom"})
+        self.failing_methods = methods
 
     def page(self, page_id: str) -> Page:
         """The stored page, for asserting on where it ended up."""
@@ -220,11 +226,20 @@ def _render(self, page, **kwargs):
     # -- the routes --------------------------------------------------------
 
     def handle(self, request: Request):
-        if self.failure:
+        if self.failure and request.method in (self.failing_methods or (request.method,)):
             return self.failure
 
         path, method = request.path, request.method
 
+        if path.endswith("/children") and method == "GET":
+            return self._children(path.split("/")[-2])
+        if path.endswith("/child/page") and method == "GET":
+            return self._children(path.split("/")[-3])
+        if "/api/v2/pages/" in path and method == "DELETE":
+            return self._delete(path.rsplit("/", 1)[1], reparent=True)
+        if "/rest/api/content/" in path and method == "DELETE":
+            return self._delete(path.rsplit("/", 1)[1], reparent=False)
+
         # v2 (Cloud). Matched on the API path alone: the context path in front
         # of it is /wiki for a tenant URL and /ex/confluence/{cloudId} for a
         # gateway one, and getting that wrong is the bug under test.
@@ -251,6 +266,30 @@ def handle(self, request: Request):
 
         return 404, {"message": f"no route for {method} {path}"}
 
+    def _children(self, page_id):
+        if page_id not in self.pages:
+            return 404, {"message": "page not found"}
+        kids = [page for page in self.pages.values() if page.parent_id == page_id]
+        return 200, {"results": [{"id": page.id, "title": page.title} for page in kids]}
+
+    def _delete(self, page_id, *, reparent: bool):
+        """Removes one page, as Confluence removes one page.
+
+        Cloud re-parents the children onto the grandparent and leaves them
+        current -- observed against a real site -- so a delete that does not
+        walk the tree strands an incident's sub-pages on the storage root.
+        Server's client walks the tree itself.
+        """
+        page = self.pages.pop(page_id, None)
+        if page is None:
+            return 404, {"message": "page not found"}
+        if reparent:
+            for orphan in self.pages.values():
+                if orphan.parent_id == page_id:
+                    orphan.parent_id = page.parent_id
+        self.deleted.append(page_id)
+        return 204, {}
+
     # -- v2 ----------------------------------------------------------------
 
     def _v2_create(self, request):

tests/plugins/dispatch_atlassian_confluence/test_confluence_live.py

@@ -93,7 +93,6 @@ def __init__(self, platform, settings):
             username=settings["username"],
             password=settings["password"],
             root_id=settings["root_id"],
-            template_id=settings["template_id"],
         )
         self.storage = ConfluencePagePlugin()
         self.storage.configuration = self.configuration
@@ -214,6 +213,34 @@ def test_the_template_is_copied_under_the_incident_page(live):
     )
 
 
+def test_each_document_copies_the_template_it_was_given(live):
+    """Dispatch resolves a different template per document type. The plugin
+    used to ignore `file_id` and copy one configured page, so an executive
+    report was a copy of the incident template."""
+    incident = live.track(
+        live.storage.create_file(parent_id=live.root_id, name=live.title("incident"))
+    )
+    other_source = live.track(
+        live.storage.create_file(parent_id=live.root_id, name=live.title("other template"))
+    )
+
+    from_template = live.track(
+        live.storage.copy_file(
+            folder_id=incident["id"], file_id=live.settings["template_id"], name=live.title("a")
+        )
+    )
+    from_other = live.track(
+        live.storage.copy_file(
+            folder_id=incident["id"], file_id=other_source["id"], name=live.title("b")
+        )
+    )
+
+    body = lambda page_id: live.api.get_page(page_id)["body"]["storage"]["value"]  # noqa: E731
+    assert body(from_template["id"]) == body(live.settings["template_id"])
+    assert body(from_other["id"]) == body(other_source["id"])
+    assert body(from_template["id"]) != body(from_other["id"])
+
+
 def test_the_copied_document_can_be_substituted_into(live):
     """The template needs `{{commander}}` in it, per this module's docstring."""
     incident = live.track(
@@ -242,6 +269,35 @@ def test_a_page_cannot_be_created_under_a_page_that_does_not_exist(live):
     assert live.storage.create_file(parent_id="1", name=live.title("orphan")) is None
 
 
+def test_deleting_storage_takes_the_whole_incident_with_it(live):
+    """Cloud trashes only the page named and re-parents its children onto the
+    grandparent, leaving them current -- observed here, and the reason
+    `delete_page` walks the tree rather than calling the API once."""
+    incident = live.track(
+        live.storage.create_file(parent_id=live.root_id, name=live.title("incident"))
+    )
+    logs = live.track(live.storage.create_file(parent_id=incident["id"], name="Logs"))
+    document = live.track(
+        live.storage.copy_file(
+            folder_id=logs["id"], file_id=live.settings["template_id"], name=live.title("doc")
+        )
+    )
+
+    live.storage.delete_file(file_id=incident["id"])
+
+    for page_id in (document["id"], logs["id"], incident["id"]):
+        assert _is_gone(live, page_id), f"{page_id} survived the delete"
+    assert live.api.get_page(live.root_id)["id"] == live.root_id
+
+
+def _is_gone(live, page_id: str) -> bool:
+    """Trashed counts as gone: Cloud's delete is a soft delete."""
+    try:
+        return live.api.get_page(page_id).get("status") == "trashed"
+    except HTTPError as exception:
+        return exception.response is not None and exception.response.status_code in (404, 400)
+
+
 # -- inferences from the schema that only a real instance can settle ---------