Commander: {{commander}} Status: {{status}}
" + + +def build_configuration(hosting_type: str, **overrides): + """A ConfluenceConfiguration as the storage plugin really receives it.""" + from dispatch.plugins.dispatch_atlassian_confluence.plugin import ConfluenceConfiguration + + values = { + "api_url": CLOUD_URL if hosting_type == "cloud" else SERVER_URL, + "hosting_type": hosting_type, + "username": USERNAME, + "password": PASSWORD, + "template_id": TEMPLATE_ID, + "root_id": SPACE_KEY, + "parent_id": PARENT_ID, + } + values.update(overrides) + return ConfluenceConfiguration(**values) + + +def build_document_configuration(hosting_type: str, **overrides): + """A ConfluenceConfigurationBase, as the document plugin really receives it.""" + from dispatch.plugins.dispatch_atlassian_confluence.config import ( + ConfluenceConfigurationBase, + ) + + values = { + "api_url": CLOUD_URL if hosting_type == "cloud" else SERVER_URL, + "hosting_type": hosting_type, + "username": USERNAME, + "password": PASSWORD, + } + values.update(overrides) + return ConfluenceConfigurationBase(**values) + + +class Request: + """One HTTP request as Confluence would have received it.""" + + def __init__(self, prepared): + self.method = prepared.method + self.url = prepared.url + self.path = requests.utils.urlparse(prepared.url).path + self.query = requests.utils.urlparse(prepared.url).query + self.headers = prepared.headers + self.body = json.loads(prepared.body) if prepared.body else None + + def __repr__(self): + return f"Commander: {{commander}} Status: {{status}}
`; + the plugin wraps each kwarg name in braces before replacing.""" + plugin = document_plugin(hosting_type) + + plugin.update(ROOT_PAGE_ID, commander="Ada Lovelace", status="Stable") + + body = confluence.last("PUT").body["body"]["storage"]["value"] + assert body == "Commander: Ada Lovelace Status: Stable
" + + +def test_update_leaves_placeholders_alone_when_a_value_is_empty(confluence, hosting_type): + plugin = document_plugin(hosting_type) + + plugin.update(ROOT_PAGE_ID, commander="Ada Lovelace", status="") + + body = confluence.last("PUT").body["body"]["storage"]["value"] + assert body == "Commander: Ada Lovelace Status: {{status}}
" + + +def test_update_ignores_keys_the_template_does_not_mention(confluence, hosting_type): + plugin = document_plugin(hosting_type) + + plugin.update(ROOT_PAGE_ID, commander="Ada Lovelace", unrelated="ignored") + + body = confluence.last("PUT").body["body"]["storage"]["value"] + assert "ignored" not in body + + +def test_update_keeps_the_existing_page_title(confluence, hosting_type): + """Both APIs require a title on update; sending the wrong one renames the + incident document.""" + plugin = document_plugin(hosting_type) + + plugin.update(ROOT_PAGE_ID, commander="Ada Lovelace") + + assert confluence.last("PUT").body["title"] == "Incident Template" + + +def test_update_targets_the_document_on_the_platforms_own_endpoint(confluence, hosting_type): + plugin = document_plugin(hosting_type) + + plugin.update(ROOT_PAGE_ID, commander="Ada Lovelace") + + path = confluence.last("PUT").path + if hosting_type == "cloud": + assert path == f"/wiki/api/v2/pages/{ROOT_PAGE_ID}" + else: + assert path == f"/rest/api/content/{ROOT_PAGE_ID}" + + +def test_update_advances_the_page_version(confluence, hosting_type): + """Both APIs reject an update that does not move the version forward; v2 + makes the caller supply it, v1 reads it from the page history.""" + plugin = document_plugin(hosting_type) + + plugin.update(ROOT_PAGE_ID, commander="Ada Lovelace") + + assert confluence.last("PUT").body["version"]["number"] == 2 + + +def test_update_returns_the_updated_page(confluence, hosting_type): + plugin = document_plugin(hosting_type) + + result = plugin.update(ROOT_PAGE_ID, commander="Ada Lovelace") + + assert result["id"] == ROOT_PAGE_ID + assert result["body"]["storage"]["value"] == "Commander: Ada Lovelace Status: {{status}}
" + + +def test_update_propagates_confluence_failures(confluence, hosting_type): + """`DocumentPlugin.update` has no except of its own, and its callers treat a + raised error as the document flow failing. Swallowing it here would report + an unsubstituted document as successfully written.""" + plugin = document_plugin(hosting_type) + confluence.fail_with(500) + + with pytest.raises(HTTPError): + plugin.update(ROOT_PAGE_ID, commander="Ada Lovelace") + + assert all(request.method != "PUT" for request in confluence.requests) diff --git a/tests/plugins/dispatch_atlassian_confluence/test_confluence_storage_plugin.py b/tests/plugins/dispatch_atlassian_confluence/test_confluence_storage_plugin.py new file mode 100644 index 000000000..624e478d5 --- /dev/null +++ b/tests/plugins/dispatch_atlassian_confluence/test_confluence_storage_plugin.py @@ -0,0 +1,174 @@ +"""What ConfluencePagePlugin actually sends and returns, per platform (#214).""" + +import pytest + +from tests.plugins.dispatch_atlassian_confluence.conftest import storage_plugin +from tests.plugins.dispatch_atlassian_confluence.fake_confluence import ( + PARENT_ID, + SPACE_ID, + SPACE_KEY, + TEMPLATE_BODY, + TEMPLATE_ID, +) + + +def test_create_file_creates_a_page_in_the_configured_space(confluence, hosting_type): + plugin = storage_plugin(hosting_type) + + plugin.create_file(parent_id=SPACE_KEY, name="Dispatch Incident") + + request = confluence.last("POST") + assert request.body["title"] == "Dispatch Incident" + assert "Commander: {{commander}} Status: {{status}}
" +# Straight out of Atlassian's published v2 OpenAPI spec +# (developer.atlassian.com/cloud/confluence/openapi-v2.v3.json): the client +# sends only what it is told to, so a wrapper that omits one of these builds a +# request the API rejects and no amount of method-level mocking would notice. +V2_CREATE_REQUIRED = ("spaceId",) +V2_UPDATE_REQUIRED = ("id", "status", "title", "body", "version") + def build_configuration(hosting_type: str, **overrides): """A ConfluenceConfiguration as the storage plugin really receives it.""" @@ -77,8 +85,10 @@ class Request: def __init__(self, prepared): self.method = prepared.method self.url = prepared.url - self.path = requests.utils.urlparse(prepared.url).path - self.query = requests.utils.urlparse(prepared.url).query + parsed = urlparse(prepared.url) + self.path = parsed.path + self.query = parsed.query + self.params = dict(parse_qsl(parsed.query)) self.headers = prepared.headers self.body = json.loads(prepared.body) if prepared.body else None @@ -114,19 +124,32 @@ def handle(self, request: Request): path, method = request.path, request.method # v2 (Cloud) - if path == "/wiki/api/v2/spaces" and method == "GET": - return 200, {"results": [{"id": SPACE_ID, "key": SPACE_KEY, "name": "Incidents"}]} - if path == "/wiki/api/v2/pages" and method == "POST": + if path.endswith("/wiki/api/v2/spaces") and method == "GET": + # Honours the keys filter, so a page id sent where a space key + # belongs comes back empty, as it would from real Confluence. + keys = request.params.get("keys", "").split(",") + found = [{"id": SPACE_ID, "key": SPACE_KEY, "name": "Incidents"}] + return 200, {"results": found if SPACE_KEY in keys else []} + if path.endswith("/wiki/api/v2/pages") and method == "POST": + # PageCreateRequest requires spaceId, and the space must exist. + missing = self._missing(request.body, V2_CREATE_REQUIRED) + if missing: + return 400, {"errors": [{"title": f"missing required field(s): {missing}"}]} + if request.body["spaceId"] != str(SPACE_ID): + return 404, {"errors": [{"title": "space not found"}]} return 200, self._v2_page( page_id="900001", title=request.body["title"], body=request.body["body"]["storage"]["value"], ) - if path.startswith("/wiki/api/v2/pages/") and method == "GET": + if "/wiki/api/v2/pages/" in path and method == "GET": return 200, self._v2_page( page_id=path.rsplit("/", 1)[1], title="Incident Template", body=TEMPLATE_BODY ) - if path.startswith("/wiki/api/v2/pages/") and method == "PUT": + if "/wiki/api/v2/pages/" in path and method == "PUT": + missing = self._missing(request.body, V2_UPDATE_REQUIRED) + if missing: + return 400, {"errors": [{"title": f"missing required field(s): {missing}"}]} return 200, self._v2_page( page_id=path.rsplit("/", 1)[1], title=request.body["title"], @@ -143,17 +166,19 @@ def handle(self, request: Request): if path.endswith("/history") and method == "GET": # update_page reads the current version from here before its PUT. return 200, {"lastUpdated": {"number": 1}} - if path.rstrip("/") == "/rest/api/content" and method == "POST": + if path.rstrip("/").endswith("/rest/api/content") and method == "POST": + if request.body["space"]["key"] != SPACE_KEY: + return 400, {"message": f"unknown space key {request.body['space']['key']}"} return 200, self._v1_page( page_id="900001", title=request.body["title"], body=request.body["body"]["storage"]["value"], ) - if path.startswith("/rest/api/content/") and method == "GET": + if "/rest/api/content/" in path and method == "GET": return 200, self._v1_page( page_id=path.rsplit("/", 1)[1], title="Incident Template", body=TEMPLATE_BODY ) - if path.startswith("/rest/api/content/") and method == "PUT": + if "/rest/api/content/" in path and method == "PUT": return 200, self._v1_page( page_id=path.rsplit("/", 1)[1], title=request.body["title"], @@ -163,6 +188,10 @@ def handle(self, request: Request): return 404, {"message": f"no route for {method} {path}"} + @staticmethod + def _missing(body, required): + return sorted(field for field in required if not body.get(field)) + @staticmethod def _v2_page(*, page_id, title, body, version=1): return { diff --git a/tests/plugins/dispatch_atlassian_confluence/test_confluence_document_plugin.py b/tests/plugins/dispatch_atlassian_confluence/test_confluence_document_plugin.py index b71c33732..e5b337a5e 100644 --- a/tests/plugins/dispatch_atlassian_confluence/test_confluence_document_plugin.py +++ b/tests/plugins/dispatch_atlassian_confluence/test_confluence_document_plugin.py @@ -1,6 +1,7 @@ """What ConfluencePageDocPlugin actually substitutes and sends (#214).""" import pytest +from atlassian.errors import ApiError from requests import HTTPError from tests.plugins.dispatch_atlassian_confluence.conftest import document_plugin @@ -77,14 +78,40 @@ def test_update_returns_the_updated_page(confluence, hosting_type): assert result["body"]["storage"]["value"] == "Commander: Ada Lovelace Status: {{status}}
" -def test_update_propagates_confluence_failures(confluence, hosting_type): +@pytest.mark.parametrize("status", [403, 404, 500]) +def test_update_propagates_confluence_failures(confluence, hosting_type, status): """`DocumentPlugin.update` has no except of its own, and its callers treat a raised error as the document flow failing. Swallowing it here would report - an unsubstituted document as successfully written.""" + an unsubstituted document as successfully written. + + Both exception families are accepted because the two clients differ: the + Server client translates 403/404 into atlassian's own ApiError subclasses, + while the Cloud client lets requests' HTTPError through. + """ plugin = document_plugin(hosting_type) - confluence.fail_with(500) + confluence.fail_with(status) - with pytest.raises(HTTPError): + with pytest.raises((HTTPError, ApiError)): plugin.update(ROOT_PAGE_ID, commander="Ada Lovelace") assert all(request.method != "PUT" for request in confluence.requests) + + +def test_server_skips_the_write_when_nothing_changed(confluence): + """A documented platform difference: the v1 client compares the stored body + first and returns without a PUT when it matches, where v2 always writes.""" + plugin = document_plugin("server") + + plugin.update(ROOT_PAGE_ID, commander="") + + assert all(request.method != "PUT" for request in confluence.requests) + + +def test_cloud_writes_even_when_nothing_changed(confluence): + plugin = document_plugin("cloud") + + plugin.update(ROOT_PAGE_ID, commander="") + + assert confluence.last("PUT").body["body"]["storage"]["value"] == ( + "Commander: {{commander}} Status: {{status}}
" + ) diff --git a/tests/plugins/dispatch_atlassian_confluence/test_confluence_storage_plugin.py b/tests/plugins/dispatch_atlassian_confluence/test_confluence_storage_plugin.py index 624e478d5..545e3e6ef 100644 --- a/tests/plugins/dispatch_atlassian_confluence/test_confluence_storage_plugin.py +++ b/tests/plugins/dispatch_atlassian_confluence/test_confluence_storage_plugin.py @@ -172,3 +172,75 @@ def test_move_file_is_a_no_op_that_keeps_the_storage_interface(confluence, hosti assert plugin.move_file(new_folder_id="1", file_id="2") == {} assert confluence.requests == [] + + +def test_create_file_cannot_make_sub_pages_from_a_page_id(confluence, hosting_type, caplog): + """Core's storage flow calls create_file three times: once with the project + storage root (a space key) and twice more with the page id it just got back, + for "Logs" and "Screengrabs". Confluence has no folders and this plugin uses + the argument as a space key, so those two calls cannot succeed. + + What matters is that they fail the way callers expect -- a logged error and + a falsy return, which `dispatch.storage.flows` checks for -- rather than + raising into the incident flow or silently filing pages in the wrong space. + """ + plugin = storage_plugin(hosting_type) + + assert plugin.create_file(parent_id="900001", name="Logs") is None + assert "Exception happened while creating page" in caplog.text + + +def test_copy_file_weblink_names_the_space_the_page_was_created_in(confluence, hosting_type): + """copy_file creates the page in the configured space, so the weblink has to + name that space and not the parent page it was filed under.""" + plugin = storage_plugin(hosting_type) + + result = plugin.copy_file(folder_id="444444", file_id="x", name="Incident Review") + + if hosting_type == "cloud": + assert result["weblink"] == ( + f"https://dispatch-tests.atlassian.net/wiki/spaces/{SPACE_KEY}" + "/pages/900001/Incident Review" + ) + else: + assert result["weblink"] == ( + "https://confluence.internal.example.com/pages/viewpage.action?pageId=900001" + ) + + +def test_server_weblink_survives_an_instance_under_a_context_path(confluence): + """Confluence Server usually sits under one, and AnyHttpUrl only appends a + trailing slash when the path is empty.""" + plugin = storage_plugin("server", api_url="https://confluence.example.com/confluence") + + result = plugin.create_file(parent_id=SPACE_KEY, name="Dispatch Incident") + + assert result["weblink"] == ( + "https://confluence.example.com/confluence/pages/viewpage.action?pageId=900001" + ) + + +def test_cloud_weblink_is_not_doubled_when_the_url_already_has_the_wiki_path(confluence): + """The client appends /wiki itself when the configured URL lacks it, so the + weblink must not assume it is absent.""" + plugin = storage_plugin("cloud", api_url="https://dispatch-tests.atlassian.net/wiki") + + result = plugin.create_file(parent_id=SPACE_KEY, name="Dispatch Incident") + + assert result["weblink"] == ( + f"https://dispatch-tests.atlassian.net/wiki/spaces/{SPACE_KEY}" + "/pages/900001/Dispatch Incident" + ) + + +def test_cloud_failures_are_logged_with_something_actionable(confluence, caplog): + """The Cloud client raises HTTPError with an empty message, because its + raise_for_status does not parse v2's error envelope. Logging the exception + alone leaves an operator with a bare "Exception happened while creating + page:" and no status to act on.""" + plugin = storage_plugin("cloud") + confluence.fail_with(403, {"errors": [{"status": 403, "title": "Not permitted"}]}) + + assert plugin.create_file(parent_id=SPACE_KEY, name="Dispatch Incident") is None + + assert "403" in caplog.text From 3df55e6aafc9a9bfab568a32360b60580e131bcf Mon Sep 17 00:00:00 2001 From: Jamyn Shanley <696574+Jamyn@users.noreply.github.com> Date: Thu, 20 Aug 2026 04:41:37 +0000 Subject: [PATCH 4/4] docs(confluence): name the space key field for what the Cloud API requires --- .../settings/plugins/configuring-atlassian-confluence.mdx | 4 ++-- src/dispatch/plugins/dispatch_atlassian_confluence/plugin.py | 3 ++- .../test_confluence_storage_plugin.py | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/docs/docs/administration/settings/plugins/configuring-atlassian-confluence.mdx b/docs/docs/administration/settings/plugins/configuring-atlassian-confluence.mdx index e2ea6686f..aa0c84a54 100644 --- a/docs/docs/administration/settings/plugins/configuring-atlassian-confluence.mdx +++ b/docs/docs/administration/settings/plugins/configuring-atlassian-confluence.mdx @@ -30,9 +30,9 @@ Dispatch ships with Atlassian Confluence storage and document plugins. This page > 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. -### `Default Space ID` \[Required\] +### `Default Space Key` \[Required\] -> This is the default space [key](https://confluence.atlassian.com/doc/space-keys-829076188.html) where all the pages will be created in confluence. +> This is the default space [key](https://confluence.atlassian.com/doc/space-keys-829076188.html) where all the pages will be created in confluence. The space key, not its numeric id: on Cloud, Dispatch resolves the key to the id the v2 API requires. ### `Parent ID of the pages` \[Required\] diff --git a/src/dispatch/plugins/dispatch_atlassian_confluence/plugin.py b/src/dispatch/plugins/dispatch_atlassian_confluence/plugin.py index d8915ceac..23f9c20bc 100644 --- a/src/dispatch/plugins/dispatch_atlassian_confluence/plugin.py +++ b/src/dispatch/plugins/dispatch_atlassian_confluence/plugin.py @@ -34,7 +34,8 @@ class ConfluenceConfiguration(ConfluenceConfigurationBase): title="Incident template ID", description="This is the page id of the template." ) root_id: str = Field( - title="Default Space ID", description="Defines the default Confluence Space to use." + title="Default Space Key", + description="Key of the default Confluence Space to use, not its numeric id or display name.", ) parent_id: str = Field( title="Parent ID for the pages", diff --git a/tests/plugins/dispatch_atlassian_confluence/test_confluence_storage_plugin.py b/tests/plugins/dispatch_atlassian_confluence/test_confluence_storage_plugin.py index 545e3e6ef..a45d22ae9 100644 --- a/tests/plugins/dispatch_atlassian_confluence/test_confluence_storage_plugin.py +++ b/tests/plugins/dispatch_atlassian_confluence/test_confluence_storage_plugin.py @@ -124,7 +124,7 @@ def test_copy_file_reads_the_configured_template(confluence, hosting_type): plugin.copy_file(folder_id="444444", file_id="ignored", name="Incident Review") - assert TEMPLATE_ID in confluence.requests[0].path + assert any(TEMPLATE_ID in r.path for r in confluence.requests if r.method == "GET") def test_copy_file_creates_the_page_under_the_given_folder(confluence, hosting_type):