From 6dde8560fe057949aea1949e9d965264b412e557 Mon Sep 17 00:00:00 2001 From: Jason Wen Date: Thu, 12 Mar 2026 22:15:41 -0400 Subject: [PATCH] another --- .github/workflows/sync-docs-discourse.yml | 10 +- docs_sp/tools/discourse_client.py | 72 ++++++++------ docs_sp/tools/test_discourse_client.py | 112 +++++++++++----------- 3 files changed, 102 insertions(+), 92 deletions(-) diff --git a/.github/workflows/sync-docs-discourse.yml b/.github/workflows/sync-docs-discourse.yml index edbd53731c..57e5411c49 100644 --- a/.github/workflows/sync-docs-discourse.yml +++ b/.github/workflows/sync-docs-discourse.yml @@ -31,6 +31,7 @@ jobs: DISCOURSE_URL: ${{ secrets.DISCOURSE_URL }} DISCOURSE_API_KEY: ${{ secrets.DISCOURSE_API_KEY }} DISCOURSE_API_USER: ${{ secrets.DISCOURSE_API_USER }} + DISCOURSE_CATEGORY_MAP: '{"getting-started": 115}' run: | uv run --python 3.12 python -c " import sys @@ -66,11 +67,8 @@ jobs: config = DiscourseConfig.from_env() client = DiscourseClient(config) - - cat_id = client.get_category_id() - if cat_id is None: - print(f'ERROR: Category not found on {config.base_url}') - sys.exit(1) + category_id = category_id_for(DOC_PATH) + print(f'Category ID: {category_id}') existing = client.find_topic_by_sync_id(DOC_PATH) if existing is not None: @@ -85,7 +83,7 @@ jobs: sys.exit(1) print(f'Updated: {config.base_url}/t/{topic_id}') else: - result = client.create_topic(title=title, raw=body, category_id=cat_id, tags=['docs-auto-sync']) + result = client.create_topic(title=title, raw=body, category_id=category_id, tags=['docs-auto-sync']) if result is None: print('ERROR: Failed to create topic') sys.exit(1) diff --git a/docs_sp/tools/discourse_client.py b/docs_sp/tools/discourse_client.py index d04412d783..f9ed6ce2a3 100644 --- a/docs_sp/tools/discourse_client.py +++ b/docs_sp/tools/discourse_client.py @@ -1,17 +1,18 @@ """Minimal Discourse API client using only urllib (zero external deps). -Provides the 5 CRUD operations needed by the docs sync orchestrator: -1. get_category_id(slug) -2. find_topic_by_sync_id(sync_id) -3. create_topic(title, raw, category_id, tags) -4. update_post(post_id, raw, edit_reason) -5. first_post_id(topic_id) +Provides the 4 CRUD operations needed by the docs sync orchestrator: +1. find_topic_by_sync_id(sync_id) +2. create_topic(title, raw, category_id, tags) +3. update_post(post_id, raw, edit_reason) +4. first_post_id(topic_id) Configuration via environment variables: - DISCOURSE_URL - Base URL (e.g. https://community.sunnypilot.ai) - DISCOURSE_API_KEY - API key with topic create/update permissions - DISCOURSE_API_USER - Username for API requests (default: "system") - DISCOURSE_CATEGORY - Category slug for documentation (default: "documentation") + DISCOURSE_URL - Base URL (e.g. https://community.sunnypilot.ai) + DISCOURSE_API_KEY - API key with topic create/update permissions + DISCOURSE_API_USER - Username for API requests (default: "system") + DISCOURSE_CATEGORY_MAP - JSON mapping of doc section to Discourse category ID + e.g. '{"getting-started": 115, "features": 116}' + Falls back to parent category 114 for unmapped sections. """ from __future__ import annotations @@ -21,10 +22,15 @@ import os import urllib.error import urllib.parse import urllib.request -from dataclasses import dataclass +from dataclasses import dataclass, field from typing import Any +DEFAULT_PARENT_CATEGORY_ID = 114 + +DEFAULT_CATEGORY_MAP: dict[str, int] = {} + + @dataclass(frozen=True) class DiscourseConfig: """Immutable configuration for the Discourse API client.""" @@ -32,14 +38,24 @@ class DiscourseConfig: base_url: str api_key: str api_user: str = "system" - category_slug: str = "documentation" + category_mapping: dict[str, int] = field(default_factory=lambda: dict(DEFAULT_CATEGORY_MAP)) + + def category_id_for(self, doc_path: str) -> int: + """Return the Discourse category ID for a given doc path. + + Looks up the top-level folder (e.g. "getting-started" from + "getting-started/what-is-sunnypilot.md") in category_mapping. + Falls back to the parent Documentation category (114). + """ + section = doc_path.split("/")[0] if "/" in doc_path else doc_path + return self.category_mapping.get(section, DEFAULT_PARENT_CATEGORY_ID) @classmethod def from_env(cls) -> DiscourseConfig: """Build config from environment variables. Raises: - ValueError: If required env vars are missing. + ValueError: If required env vars are missing or map is invalid JSON. """ base_url = os.environ.get("DISCOURSE_URL", "") api_key = os.environ.get("DISCOURSE_API_KEY", "") @@ -49,11 +65,23 @@ class DiscourseConfig: if not api_key: raise ValueError("DISCOURSE_API_KEY environment variable is required") + category_map_str = os.environ.get("DISCOURSE_CATEGORY_MAP", "") + if category_map_str: + try: + raw_map = json.loads(category_map_str) + except json.JSONDecodeError as e: + raise ValueError(f"DISCOURSE_CATEGORY_MAP must be valid JSON: {e}") + if not isinstance(raw_map, dict): + raise ValueError("DISCOURSE_CATEGORY_MAP must be a JSON object") + category_mapping = {str(k): int(v) for k, v in raw_map.items()} + else: + category_mapping = dict(DEFAULT_CATEGORY_MAP) + return cls( base_url=base_url.rstrip("/"), api_key=api_key, api_user=os.environ.get("DISCOURSE_API_USER", "system"), - category_slug=os.environ.get("DISCOURSE_CATEGORY", "documentation"), + category_mapping=category_mapping, ) @@ -69,21 +97,6 @@ class DiscourseClient: # ----- Public API ----- - def get_category_id(self, slug: str | None = None) -> int | None: - """Look up a category ID by slug. - - Args: - slug: Category slug. Defaults to config.category_slug. - - Returns: - Category ID, or None if not found. - """ - slug = slug or self._config.category_slug - data = self._get(f"/c/{slug}/show.json") - if data is None: - return None - return data.get("category", {}).get("id") - def find_topic_by_sync_id(self, sync_id: str) -> dict[str, Any] | None: """Find an existing topic by its embedded sync ID comment. @@ -178,6 +191,7 @@ class DiscourseClient: "Content-Type": "application/json", "Api-Key": self._config.api_key, "Api-Username": self._config.api_user, + "User-Agent": "Mozilla/5.0 (compatible; sunnypilot-docs-sync/1.0)", } def _get(self, path: str) -> dict[str, Any] | None: diff --git a/docs_sp/tools/test_discourse_client.py b/docs_sp/tools/test_discourse_client.py index 6f1fae8e7b..89e4a6cfeb 100644 --- a/docs_sp/tools/test_discourse_client.py +++ b/docs_sp/tools/test_discourse_client.py @@ -20,7 +20,7 @@ TEST_CONFIG = DiscourseConfig( base_url="https://community.sunnypilot.ai", api_key="test-api-key-123", api_user="docs-bot", - category_slug="documentation", + category_mapping={"getting-started": 115, "features": 116}, ) @@ -61,7 +61,7 @@ def test_config_from_env(): "DISCOURSE_URL": "https://forum.example.com/", "DISCOURSE_API_KEY": "secret-key", "DISCOURSE_API_USER": "bot", - "DISCOURSE_CATEGORY": "docs", + "DISCOURSE_CATEGORY_MAP": '{"getting-started": 115, "features": 116}', } with patch.dict("os.environ", env, clear=False): config = DiscourseConfig.from_env() @@ -69,7 +69,7 @@ def test_config_from_env(): assert config.base_url == "https://forum.example.com" # trailing slash stripped assert config.api_key == "secret-key" assert config.api_user == "bot" - assert config.category_slug == "docs" + assert config.category_mapping == {"getting-started": 115, "features": 116} print(" PASS: config_from_env") @@ -78,16 +78,55 @@ def test_config_from_env_defaults(): "DISCOURSE_URL": "https://forum.example.com", "DISCOURSE_API_KEY": "key", } - with patch.dict("os.environ", env, clear=False): - # Remove optional vars if present - with patch.dict("os.environ", {"DISCOURSE_API_USER": "", "DISCOURSE_CATEGORY": ""}, clear=False): - pass + with patch.dict("os.environ", env, clear=True): config = DiscourseConfig.from_env() - assert config.api_user in ("system", os.environ.get("DISCOURSE_API_USER", "system")) + assert config.api_user == "system" + assert config.category_mapping == {} print(" PASS: config_from_env_defaults") +def test_config_invalid_category_map_json(): + env = { + "DISCOURSE_URL": "https://forum.example.com", + "DISCOURSE_API_KEY": "key", + "DISCOURSE_CATEGORY_MAP": "not-valid-json", + } + with patch.dict("os.environ", env, clear=True): + try: + DiscourseConfig.from_env() + assert False, "Should have raised ValueError" + except ValueError as e: + assert "DISCOURSE_CATEGORY_MAP" in str(e) + print(" PASS: config_invalid_category_map_json") + + +def test_config_invalid_category_map_type(): + env = { + "DISCOURSE_URL": "https://forum.example.com", + "DISCOURSE_API_KEY": "key", + "DISCOURSE_CATEGORY_MAP": "[1, 2, 3]", + } + with patch.dict("os.environ", env, clear=True): + try: + DiscourseConfig.from_env() + assert False, "Should have raised ValueError" + except ValueError as e: + assert "DISCOURSE_CATEGORY_MAP" in str(e) + print(" PASS: config_invalid_category_map_type") + + +def test_category_id_for_mapped(): + assert TEST_CONFIG.category_id_for("getting-started/what-is-sunnypilot.md") == 115 + assert TEST_CONFIG.category_id_for("features/icbm.md") == 116 + print(" PASS: category_id_for_mapped") + + +def test_category_id_for_unmapped(): + assert TEST_CONFIG.category_id_for("unknown-section/doc.md") == 114 + print(" PASS: category_id_for_unmapped") + + def test_config_missing_url(): env = {"DISCOURSE_API_KEY": "key"} with patch.dict("os.environ", env, clear=True): @@ -119,48 +158,6 @@ def test_config_immutable(): print(" PASS: config_immutable") -# --------------------------------------------------------------------------- -# get_category_id -# --------------------------------------------------------------------------- - - -@patch("urllib.request.urlopen") -def test_get_category_id_found(mock_urlopen: MagicMock): - mock_urlopen.return_value = mock_response({"category": {"id": 42, "slug": "documentation"}}) - client = DiscourseClient(TEST_CONFIG) - - result = client.get_category_id("documentation") - - assert result == 42 - call_args = mock_urlopen.call_args[0][0] - assert "/c/documentation/show.json" in call_args.full_url - assert call_args.get_header("Api-key") == "test-api-key-123" - assert call_args.get_header("Api-username") == "docs-bot" - print(" PASS: get_category_id_found") - - -@patch("urllib.request.urlopen") -def test_get_category_id_not_found(mock_urlopen: MagicMock): - mock_urlopen.side_effect = mock_http_error(404, "Not Found") - client = DiscourseClient(TEST_CONFIG) - - result = client.get_category_id("nonexistent") - assert result is None - print(" PASS: get_category_id_not_found") - - -@patch("urllib.request.urlopen") -def test_get_category_id_uses_default_slug(mock_urlopen: MagicMock): - mock_urlopen.return_value = mock_response({"category": {"id": 7}}) - client = DiscourseClient(TEST_CONFIG) - - client.get_category_id() # no arg — uses config.category_slug - - call_args = mock_urlopen.call_args[0][0] - assert "/c/documentation/show.json" in call_args.full_url - print(" PASS: get_category_id_uses_default_slug") - - # --------------------------------------------------------------------------- # find_topic_by_sync_id # --------------------------------------------------------------------------- @@ -355,15 +352,16 @@ def test_first_post_id_topic_not_found(mock_urlopen: MagicMock): @patch("urllib.request.urlopen") def test_headers_set_correctly(mock_urlopen: MagicMock): - mock_urlopen.return_value = mock_response({"category": {"id": 1}}) + mock_urlopen.return_value = mock_response({"topics": []}) client = DiscourseClient(TEST_CONFIG) - client.get_category_id() + client.find_topic_by_sync_id("test.md") req = mock_urlopen.call_args[0][0] assert req.get_header("Api-key") == "test-api-key-123" assert req.get_header("Api-username") == "docs-bot" assert req.get_header("Content-type") == "application/json" + assert "Mozilla" in req.get_header("User-agent") print(" PASS: headers_set_correctly") @@ -377,7 +375,7 @@ def test_connection_error_returns_none(mock_urlopen: MagicMock): mock_urlopen.side_effect = urllib.error.URLError("Connection refused") client = DiscourseClient(TEST_CONFIG) - result = client.get_category_id() + result = client.find_topic_by_sync_id("test.md") assert result is None print(" PASS: connection_error_returns_none") @@ -394,13 +392,13 @@ if __name__ == "__main__": # Config test_config_from_env, test_config_from_env_defaults, + test_config_invalid_category_map_json, + test_config_invalid_category_map_type, test_config_missing_url, test_config_missing_api_key, test_config_immutable, - # get_category_id - test_get_category_id_found, - test_get_category_id_not_found, - test_get_category_id_uses_default_slug, + test_category_id_for_mapped, + test_category_id_for_unmapped, # find_topic_by_sync_id test_find_topic_found, test_find_topic_not_found,