From 31822eab4841920776c1b3dc970966c2dc8cd714 Mon Sep 17 00:00:00 2001 From: isletspace Date: Tue, 8 Sep 2026 13:49:42 +0800 Subject: [PATCH] fix(documents): always send description_html, convert content to HTML, archive before delete (PLANECLI-7) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - doc create crashed constructing CreatePage: description_html is a required SDK field with no default, and --content bodies were wrapped in a single paragraph tag. CreatePage now always carries description_html (an empty paragraph when no content is given), and --content/--body text goes through the shared body_to_html converter (blank lines split paragraphs, single newlines become br tags, bare URLs become anchors, backticks become code tags, fenced blocks become pre-wrapped code) — the same conversion the comment commands use on integration-main. - doc update reuses the same converter and guards --content with is not None so an explicitly empty value still reaches the API. - doc delete now PATCHes archived_at (today, YYYY-MM-DD) and verifies the response actually carries it before DELETEing: the API only deletes archived pages and silently ignores unknown fields, so a bare DELETE left documents alive with no error. --- src/planecli/commands/documents.py | 59 +++++--- src/planecli/utils/body_html.py | 89 +++++++++++ tests/test_commands/test_documents.py | 210 ++++++++++++++++++++++++++ 3 files changed, 338 insertions(+), 20 deletions(-) create mode 100644 src/planecli/utils/body_html.py create mode 100644 tests/test_commands/test_documents.py diff --git a/src/planecli/commands/documents.py b/src/planecli/commands/documents.py index 1ceaa53..9f204fb 100644 --- a/src/planecli/commands/documents.py +++ b/src/planecli/commands/documents.py @@ -3,6 +3,7 @@ from __future__ import annotations import asyncio +from datetime import date from typing import Annotated import cyclopts @@ -12,6 +13,7 @@ from planecli.api.async_sdk import run_sdk from planecli.api.client import get_client, get_workspace, handle_api_error from planecli.formatters import output, output_single +from planecli.utils.body_html import body_to_html from planecli.utils.resolve import resolve_project_async doc_app = cyclopts.App( @@ -40,6 +42,7 @@ def _enrich_doc(data: dict) -> dict: desc_html = data.get("description_html") or "" if desc_html: import re + data["content_text"] = re.sub(r"<[^>]+>", "", desc_html).strip() else: data["content_text"] = "" @@ -75,12 +78,11 @@ async def list_( project_id = proj["id"] # Use direct API call since SDK doesn't have list_project_pages from planecli.api.client import get_config + config = get_config() url = f"{config.base_url}/api/v1/workspaces/{workspace}/projects/{project_id}/pages/" headers = {"X-Api-Key": config.api_key} - resp = await asyncio.to_thread( - requests.get, url, headers=headers, timeout=30 - ) + resp = await asyncio.to_thread(requests.get, url, headers=headers, timeout=30) resp.raise_for_status() resp_data = resp.json() # Handle paginated response @@ -129,9 +131,7 @@ async def show( client.pages.retrieve_project_page, workspace, project_id, document ) else: - page = await run_sdk( - client.pages.retrieve_workspace_page, workspace, document - ) + page = await run_sdk(client.pages.retrieve_workspace_page, workspace, document) data = _enrich_doc(page.model_dump()) except PlaneError as e: @@ -165,20 +165,19 @@ async def create( client = get_client() workspace = get_workspace() - page_data = CreatePage(name=title) - if content: - page_data.description_html = f"

{content}

" + # description_html is a required SDK field — omitting it crashes + # CreatePage construction even for an empty body. + page_data = CreatePage( + name=title, + description_html=body_to_html(content) if content is not None else "

", + ) if project: proj = await resolve_project_async(project, client, workspace) project_id = proj["id"] - page = await run_sdk( - client.pages.create_project_page, workspace, project_id, page_data - ) + page = await run_sdk(client.pages.create_project_page, workspace, project_id, page_data) else: - page = await run_sdk( - client.pages.create_workspace_page, workspace, page_data - ) + page = await run_sdk(client.pages.create_workspace_page, workspace, page_data) data = _enrich_doc(page.model_dump()) except PlaneError as e: @@ -217,13 +216,14 @@ async def update( client = get_client() workspace = get_workspace() from planecli.api.client import get_config + config = get_config() update_payload: dict = {} if title: update_payload["name"] = title - if content: - update_payload["description_html"] = f"

{content}

" + if content is not None: + update_payload["description_html"] = body_to_html(content) if project: proj = await resolve_project_async(project, client, workspace) @@ -260,16 +260,21 @@ async def delete( project Project name/ID (required for project-level pages). - Note: Uses direct API call since the SDK doesn't support page deletion. + Note: The API only deletes pages that are already archived, and silently + ignores unknown fields on the archive request — so this PATCHes archived_at + (today, YYYY-MM-DD), verifies the response actually carries it, and only + then DELETEs. """ import requests + from planecli.exceptions import APIError from planecli.formatters import console try: client = get_client() workspace = get_workspace() from planecli.api.client import get_config + config = get_config() if project: @@ -280,11 +285,25 @@ async def delete( else: url = f"{config.base_url}/api/v1/workspaces/{workspace}/pages/{document}/" - headers = {"X-Api-Key": config.api_key} + today = date.today().isoformat() + headers = {"X-Api-Key": config.api_key, "Content-Type": "application/json"} + # A 200 alone is not proof the archive happened — the API answers 200 + # and ignores unknown fields, so verify archived_at came back set. resp = await asyncio.to_thread( - requests.delete, url, headers=headers, timeout=30 + requests.patch, + url, + headers=headers, + json={"archived_at": today}, + timeout=30, ) resp.raise_for_status() + if resp.json().get("archived_at") != today: + raise APIError( + "the document was not archived, so it was not deleted. " + "Archiving may require a higher project role." + ) + resp = await asyncio.to_thread(requests.delete, url, headers=headers, timeout=30) + resp.raise_for_status() except PlaneError as e: raise handle_api_error(e) diff --git a/src/planecli/utils/body_html.py b/src/planecli/utils/body_html.py new file mode 100644 index 0000000..dd64ddf --- /dev/null +++ b/src/planecli/utils/body_html.py @@ -0,0 +1,89 @@ +"""Convert plain user-authored text to the HTML Plane's editor stores. + +The Plane API stores HTML verbatim and does not parse markdown or auto-link +URLs — only the web editor does — so text written via the CLI must arrive +already converted. Shared by comment and document write commands. +""" + +from __future__ import annotations + +import html +import re + +_URL_RE = re.compile(r"(?=])(https?://[A-Za-z0-9\-._~:/?#\[\]@!$&()*+,;=%]+)") +# Trailing punctuation that belongs to the sentence, not the URL. +_URL_TRAIL = ".,;:!?)]}。，；：！？）】、" + + +def _linkify(text: str) -> str: + """Turn bare http(s) URLs into anchors. + + URLs already inside an href attribute are left alone (the regex excludes + matches preceded by a quote or angle bracket from an attribute context). + """ + + def _sub(m: re.Match[str]) -> str: + url = m.group(1).rstrip(_URL_TRAIL) + trail = m.group(1)[len(url) :] + return f'{url}{trail}' + + return _URL_RE.sub(_sub, text) + + +def _inline_code(text: str) -> str: + """Convert `inline code` to a code tag with HTML-escaped content.""" + return re.sub( + r"`([^`\n]+)`", + lambda m: f"{html.escape(m.group(1))}", + text, + ) + + +def _extract_code_blocks(text: str) -> tuple[str, list[str]]: + """Pull fenced code blocks out of the text, replacing each with a token. + + Returns the text with \x00N\x00 placeholders and the list of HTML + fragments (pre-wrapped escaped code) to restore at the end. Extracting + first keeps their content out of linkify and the newline-to-br pass. + """ + blocks: list[str] = [] + + def _hold(match: re.Match[str]) -> str: + code = html.escape(match.group(1).strip("\n")) + blocks.append(f"
{code}
") + return f"\x00{len(blocks) - 1}\x00" + + text = re.sub( + r"```[ \t]*\w*[ \t]*\n(.*?)```", + _hold, + text, + flags=re.DOTALL, + ) + return text, blocks + + +def body_to_html(body: str) -> str: + """Convert plain text to HTML paragraphs. + + Blank lines separate paragraphs; a single newline becomes a br tag — + the editor collapses whitespace inside a paragraph, so unconverted + newlines would render as one long line. Backticks become code tags and + fenced blocks become pre-wrapped code (the editor stores HTML, it does + not parse markdown). + """ + text, blocks = _extract_code_blocks(body.strip()) + parts: list[str] = [] + for p in re.split(r"\n\s*\n", text): + p = p.strip() + if not p: + continue + converted = _linkify(_inline_code(p)).replace(chr(10), "
") + if re.fullmatch(r"(?:\x00\d+\x00[ \t]*)+", p): + # A paragraph that is only a code block keeps its pre wrapper. + parts.append(converted) + else: + parts.append(f"

{converted}

") + result = "".join(parts) + for i, fragment in enumerate(blocks): + result = result.replace(f"\x00{i}\x00", fragment) + return result diff --git a/tests/test_commands/test_documents.py b/tests/test_commands/test_documents.py new file mode 100644 index 0000000..ecca455 --- /dev/null +++ b/tests/test_commands/test_documents.py @@ -0,0 +1,210 @@ +"""Tests for document (page) commands.""" + +from __future__ import annotations + +from datetime import date +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest + +from planecli.commands.documents import _enrich_doc +from planecli.utils.body_html import body_to_html + + +def test_enrich_doc_strips_html_to_text(): + data = _enrich_doc({"id": "p1", "name": "Doc", "description_html": "

hi
there

"}) + assert data["content_text"] == "hithere" + + +def test_enrich_doc_empty_description(): + data = _enrich_doc({"id": "p1", "name": "Doc", "description_html": None}) + assert data["content_text"] == "" + + +def test_body_to_html_blank_lines_split_paragraphs(): + assert body_to_html("one\n\n\ntwo") == "

one

two

" + + +def test_body_to_html_single_newline_becomes_br(): + assert body_to_html("one\ntwo") == "

one
two

" + + +def test_body_to_html_linkifies_bare_urls(): + html = body_to_html("see https://example.com/a, ok") + assert html == '

see https://example.com/a, ok

' + + +def test_body_to_html_linkify_keeps_trailing_punctuation_outside_anchor(): + html = body_to_html("visit https://plane.so.") + assert html == '

visit https://plane.so.

' + + +def test_body_to_html_backticks_become_code(): + assert body_to_html("run `make test` now") == "

run make test now

" + + +def test_body_to_html_inline_code_is_escaped_and_not_linkified(): + html = body_to_html("use `https://x.io ` here") + assert html == "

use https://x.io <b> here

" + + +def test_body_to_html_fenced_block_becomes_pre_code(): + html = body_to_html("before\n\n```py\nprint('')\n```\n\nafter") + assert html == "

before

print('<x>')

after

" + + +@patch("planecli.commands.documents.output_single") +@patch("planecli.commands.documents.run_sdk", new_callable=AsyncMock) +@patch("planecli.commands.documents.resolve_project_async", new_callable=AsyncMock) +@patch("planecli.commands.documents.get_workspace", return_value="ws") +@patch("planecli.commands.documents.get_client") +async def test_doc_create_without_content_sends_empty_paragraph( + mock_client, mock_ws, mock_resolve, mock_run_sdk, mock_output +): + """description_html is a required CreatePage field; omitting it crashed + construction even when --content was not given.""" + from planecli.commands.documents import create + + mock_resolve.return_value = {"id": "proj-1"} + mock_run_sdk.return_value = MagicMock( + model_dump=lambda: {"id": "p1", "name": "Doc", "description_html": "

"} + ) + + await create(title="Doc", project="Frontend") + + page_data = mock_run_sdk.call_args[0][3] + assert page_data.description_html == "

" + assert page_data.name == "Doc" + + +@patch("planecli.commands.documents.output_single") +@patch("planecli.commands.documents.run_sdk", new_callable=AsyncMock) +@patch("planecli.commands.documents.resolve_project_async", new_callable=AsyncMock) +@patch("planecli.commands.documents.get_workspace", return_value="ws") +@patch("planecli.commands.documents.get_client") +async def test_doc_create_converts_content_to_html( + mock_client, mock_ws, mock_resolve, mock_run_sdk, mock_output +): + from planecli.commands.documents import create + + mock_resolve.return_value = {"id": "proj-1"} + mock_run_sdk.return_value = MagicMock( + model_dump=lambda: {"id": "p1", "name": "Doc", "description_html": ""} + ) + + await create( + title="Doc", + content="line one\nline two\n\nsee https://plane.so and `make check`", + project="Frontend", + ) + + page_data = mock_run_sdk.call_args[0][3] + assert page_data.description_html == ( + "

line one
line two

" + '

see https://plane.so' + " and make check

" + ) + + +def _mock_config(): + config = MagicMock() + config.base_url = "https://plane.example.com" + config.api_key = "key" + return config + + +@patch("planecli.commands.documents.output_single") +@patch("requests.patch") +@patch("planecli.api.client.get_config", return_value=_mock_config()) +@patch("planecli.commands.documents.resolve_project_async", new_callable=AsyncMock) +@patch("planecli.commands.documents.get_workspace", return_value="ws") +@patch("planecli.commands.documents.get_client") +async def test_doc_update_converts_content( + mock_client, mock_ws, mock_resolve, mock_config, mock_patch, mock_output +): + from planecli.commands.documents import update + + mock_resolve.return_value = {"id": "proj-1"} + resp = MagicMock() + resp.json.return_value = {"id": "p1", "name": "Doc", "description_html": ""} + mock_patch.return_value = resp + + await update("doc-uuid", title="Doc", content="a\nb", project="Frontend") + + url = mock_patch.call_args[0][0] + assert url == ("https://plane.example.com/api/v1/workspaces/ws/projects/proj-1/pages/doc-uuid/") + payload = mock_patch.call_args.kwargs["json"] + assert payload == {"name": "Doc", "description_html": "

a
b

"} + + +@patch("planecli.commands.documents.output_single") +@patch("requests.patch") +@patch("planecli.api.client.get_config", return_value=_mock_config()) +@patch("planecli.commands.documents.get_workspace", return_value="ws") +@patch("planecli.commands.documents.get_client") +async def test_doc_update_workspace_page_url( + mock_client, mock_ws, mock_config, mock_patch, mock_output +): + from planecli.commands.documents import update + + resp = MagicMock() + resp.json.return_value = {"id": "p1", "name": "Doc", "description_html": ""} + mock_patch.return_value = resp + + await update("doc-uuid", content="x") + + url = mock_patch.call_args[0][0] + assert url == "https://plane.example.com/api/v1/workspaces/ws/pages/doc-uuid/" + + +@patch("requests.delete") +@patch("requests.patch") +@patch("planecli.api.client.get_config", return_value=_mock_config()) +@patch("planecli.commands.documents.get_workspace", return_value="ws") +@patch("planecli.commands.documents.get_client") +async def test_doc_delete_archives_before_deleting( + mock_client, mock_ws, mock_config, mock_patch, mock_delete +): + """The API refuses to delete un-archived pages and silently ignores + unknown fields on the archive request, so delete must PATCH archived_at, + verify it came back set, and only then DELETE.""" + from planecli.commands.documents import delete + + today = date.today().isoformat() + patch_resp = MagicMock() + patch_resp.json.return_value = {"id": "p1", "archived_at": today} + mock_patch.return_value = patch_resp + mock_delete.return_value = MagicMock() + + await delete("doc-uuid") + + url = mock_patch.call_args[0][0] + assert url == "https://plane.example.com/api/v1/workspaces/ws/pages/doc-uuid/" + assert mock_patch.call_args.kwargs["json"] == {"archived_at": today} + # archive happened before delete, on the same URL + assert mock_delete.called + assert mock_delete.call_args[0][0] == url + + +@patch("requests.delete") +@patch("requests.patch") +@patch("planecli.api.client.get_config", return_value=_mock_config()) +@patch("planecli.commands.documents.get_workspace", return_value="ws") +@patch("planecli.commands.documents.get_client") +async def test_doc_delete_refuses_when_archive_not_applied( + mock_client, mock_ws, mock_config, mock_patch, mock_delete +): + """A 200 on the archive PATCH is not proof of archiving (unknown fields + are silently ignored) — if archived_at is missing from the response the + document must NOT be deleted.""" + from planecli.commands.documents import delete + from planecli.exceptions import APIError + + patch_resp = MagicMock() + patch_resp.json.return_value = {"id": "p1", "archived_at": None} + mock_patch.return_value = patch_resp + + with pytest.raises(APIError): + await delete("doc-uuid") + + mock_delete.assert_not_called()