diff --git a/docs/user_guide.rst b/docs/user_guide.rst index 04ebbdd..77f8ba6 100644 --- a/docs/user_guide.rst +++ b/docs/user_guide.rst @@ -593,9 +593,9 @@ The knowledge base mixins map to ``/Knowledgebase``. Articles and categories expose the ``search_ / get_ / create_ / update_ / delete_`` shape; comments are nested under an article; revisions are read-only. Article ``content`` and ``description`` accept and return Markdown. An -article's ``categories`` association is read-only in the GLPI contract: -it is returned by ``get_kb_article`` but is not set through the article -create/update body. +article's ``categories`` association is read-only in the v2 GLPI contract, +so the client sets it through a legacy fallback — see +`Assigning categories`_. .. note:: @@ -641,6 +641,37 @@ create/update body. Example output:: ['Networking'] + +Assigning categories +^^^^^^^^^^^^^^^^^^^^^ + +On GLPI 11 the v2 API cannot write a KB article's categories (the nested +``categories[].id`` is ``readOnly`` and category writes are silently +dropped). GLPI 11 stores KB categories as a many-to-many relationship that +only the legacy ``apirest.php`` can write, so the client applies categories +through the legacy v1 session. Configure ``v1_base_url`` / ``v1_user_token`` +(pointing at the legacy ``apirest.php``) and either pass ``categories`` on +create/update or call the helper directly. The supplied ids replace the +article's full category set; passing an empty list clears every category. + +.. code-block:: python + + from glpi_python_client import IdNameRef + + # Categories set on create are applied via the legacy fallback. The + # create is atomic: if the assignment fails, the new article is rolled + # back and the error is re-raised. + article_id = client.create_kb_article( + PostKBArticle( + name="Reset a Wi-Fi controller", + content="Hold **reset** for 10s.", + categories=[IdNameRef(id=14)], + ) + ) + + # Or set them explicitly at any time. + client.set_kb_article_categories(article_id, [14]) # replace the full set + client.set_kb_article_categories(article_id, []) # clear all 42 Reset a Wi-Fi controller 42 Reset a Wi-Fi controller 1 revision(s) diff --git a/glpi_python_client/clients/api/knowledgebase/_article.py b/glpi_python_client/clients/api/knowledgebase/_article.py index 883ac73..13d5091 100644 --- a/glpi_python_client/clients/api/knowledgebase/_article.py +++ b/glpi_python_client/clients/api/knowledgebase/_article.py @@ -8,11 +8,14 @@ from __future__ import annotations +from collections.abc import Sequence + from glpi_python_client.clients.commons._constants import ( KB_ARTICLE_ENDPOINT, GlpiId, ) from glpi_python_client.clients.commons._transport import TransportMixin +from glpi_python_client.models.api_schema._common import IdNameRef from glpi_python_client.models.api_schema.knowledgebase._article import ( DeleteKBArticle, GetKBArticle, @@ -20,6 +23,8 @@ PostKBArticle, ) +_V1_CATEGORY_FEATURE_LABEL = "knowledge base category assignments" + class KBArticleMixin(TransportMixin): """Synchronous CRUD helpers for ``/Knowledgebase/Article``.""" @@ -80,18 +85,49 @@ def get_kb_article(self, article_id: GlpiId) -> GetKBArticle: ) def create_kb_article(self, article: PostKBArticle) -> int: - """Create one knowledge base article and return its new identifier.""" + """Create one knowledge base article and return its new identifier. + + When ``article.categories`` is a non-empty list, the categories are + applied through the legacy fallback (:meth:`set_kb_article_categories`) + because the v2 API cannot write them; this requires a configured v1 + session. ``None`` or an empty list is skipped — a freshly created + article has no categories to clear — so callers that omit categories + never need a v1 session. The v2 create is not undone if the category + assignment fails: the article already exists, so the failure raises a + ``RuntimeError`` naming the new article id (chaining the original + error) and leaves the article in place for you to re-assign categories. + """ - return self._resource_create( + new_id = self._resource_create( KB_ARTICLE_ENDPOINT, article, failure_message="Failed to create KB article", missing_message="GLPI KB article create response did not include an ID", log_message_factory=lambda new_id: f"GLPI API created KB article {new_id}", ) + # A new article has no categories to clear, so only a non-empty list + # triggers the legacy fallback; ``None``/``[]`` are no-ops here. + if article.categories: + try: + self._apply_category_fallback(new_id, article.categories) + except Exception as exc: + raise RuntimeError( + f"KB article {new_id} was created but assigning its " + f"categories failed: {exc}" + ) from exc + return new_id def update_kb_article(self, article_id: GlpiId, article: PatchKBArticle) -> None: - """Update one knowledge base article with a partial body.""" + """Update one knowledge base article with a partial body. + + When ``article.categories`` is provided — including an empty list to + clear every category — the categories are applied through the legacy + fallback (:meth:`set_kb_article_categories`) after the v2 patch, because + the v2 API cannot write them; this requires a configured v1 session. + ``None`` (the default) leaves categories untouched. Unlike create, + update is not rolled back on a category failure — the v2 field changes + are already applied. + """ self._resource_update( f"{KB_ARTICLE_ENDPOINT}/{article_id}", @@ -99,6 +135,7 @@ def update_kb_article(self, article_id: GlpiId, article: PatchKBArticle) -> None failure_message=f"Failed to update KB article {article_id}", log_message=f"GLPI API updated KB article {article_id}", ) + self._apply_category_fallback(article_id, article.categories) def delete_kb_article( self, article_id: GlpiId, *, force: bool | None = None @@ -113,5 +150,60 @@ def delete_kb_article( delete_model_cls=DeleteKBArticle, ) + def set_kb_article_categories( + self, article_id: GlpiId, category_ids: Sequence[int] + ) -> None: + """Set the categories of one knowledge base article. + + GLPI 11 stores KB categories as a many-to-many relationship that the + v2 API exposes as read-only (``KBArticle.categories[].id`` is + ``readOnly``), so it silently drops category writes. This helper sets + the underlying ``_categories`` field through the legacy v1 API, which + requires ``v1_base_url``/``v1_user_token`` to be configured (pointing + at the legacy ``apirest.php``). + + The supplied ids REPLACE the article's full category set; passing an + empty sequence clears every category. Category ids are not validated + against the server — an unknown id simply is not linked. + + Raises + ------ + RuntimeError + When no legacy v1 session is configured on the client. + ValueError + When the legacy API returns a non-success status. + """ + + v1 = self._require_v1_session(_V1_CATEGORY_FEATURE_LABEL) + v1.request_json( + "PUT", + f"KnowbaseItem/{article_id}", + json_body={"input": {"_categories": [int(c) for c in category_ids]}}, + failure_message=f"Failed to set categories on KB article {article_id}", + ) + + def _apply_category_fallback( + self, article_id: GlpiId, categories: list[IdNameRef] | None + ) -> None: + """Apply ``categories`` through the legacy fallback when provided. + + No-op when ``categories`` is ``None``. An empty list clears every + category (used by update); ``create_kb_article`` skips the empty case + before calling this helper. Raises ``ValueError`` when a category + reference lacks an ``id``. + """ + + if categories is None: + return + ids: list[int] = [] + for ref in categories: + if ref.id is None: + raise ValueError( + "KB article categories require an 'id' to be linked; got a " + "category reference without an id." + ) + ids.append(ref.id) + self.set_kb_article_categories(article_id, ids) + __all__ = ["KBArticleMixin"] diff --git a/glpi_python_client/clients/api/knowledgebase/tests/test_article_mixin.py b/glpi_python_client/clients/api/knowledgebase/tests/test_article_mixin.py index 1652680..8593c90 100644 --- a/glpi_python_client/clients/api/knowledgebase/tests/test_article_mixin.py +++ b/glpi_python_client/clients/api/knowledgebase/tests/test_article_mixin.py @@ -6,10 +6,40 @@ import pytest -from glpi_python_client import GlpiClient, PatchKBArticle, PostKBArticle +from glpi_python_client import GlpiClient, IdNameRef, PatchKBArticle, PostKBArticle from glpi_python_client.testing.utils import FakeResponse, make_client +class _FakeV1: + """Stand-in for GLPIV1Session recording ``request_json`` calls.""" + + def __init__(self, *, error: Exception | None = None) -> None: + self.calls: list[dict[str, Any]] = [] + self._error = error + + def request_json( + self, + method: str, + path: str, + *, + params: dict[str, Any] | None = None, + json_body: dict[str, Any] | None = None, + success_statuses: tuple[int, ...] = (200, 201, 204, 206), + failure_message: str | None = None, + ) -> object: + self.calls.append( + { + "method": method, + "path": path, + "json_body": json_body, + "failure_message": failure_message, + } + ) + if self._error is not None: + raise self._error + return [{"1": True, "message": ""}] + + class _Recorder: """Transport recorder returning canned FakeResponse objects.""" @@ -128,3 +158,118 @@ def test_delete_kb_article_with_force(client: GlpiClient) -> None: assert call["method"] == "DELETE" assert call["endpoint"] == "Knowledgebase/Article/5" assert call["json"] == {"force": True} + + +def test_set_kb_article_categories_writes_via_v1(client: GlpiClient) -> None: + fake = _FakeV1() + client._v1 = fake # type: ignore[assignment] + client.set_kb_article_categories(31, [14, 15]) + call = fake.calls[0] + assert call["method"] == "PUT" + assert call["path"] == "KnowbaseItem/31" + assert call["json_body"] == {"input": {"_categories": [14, 15]}} + + +def test_set_kb_article_categories_empty_clears(client: GlpiClient) -> None: + fake = _FakeV1() + client._v1 = fake # type: ignore[assignment] + client.set_kb_article_categories(31, []) + assert fake.calls[0]["json_body"] == {"input": {"_categories": []}} + + +def test_set_kb_article_categories_requires_v1(client: GlpiClient) -> None: + assert client._v1 is None + with pytest.raises(RuntimeError): + client.set_kb_article_categories(31, [14]) + + +def test_create_kb_article_applies_categories_via_v1(client: GlpiClient) -> None: + rec = _Recorder() + rec.install(client) + fake = _FakeV1() + client._v1 = fake # type: ignore[assignment] + new_id = client.create_kb_article( + PostKBArticle(name="P", content="c", categories=[IdNameRef(id=14)]) + ) + assert new_id == 88 + assert rec.calls[0]["method"] == "POST" + assert fake.calls[0]["path"] == "KnowbaseItem/88" + assert fake.calls[0]["json_body"] == {"input": {"_categories": [14]}} + + +def test_create_kb_article_without_categories_skips_v1(client: GlpiClient) -> None: + rec = _Recorder() + rec.install(client) + assert client._v1 is None # no v1 configured + new_id = client.create_kb_article(PostKBArticle(name="P", content="c")) + assert new_id == 88 # no RuntimeError despite missing v1 + + +def test_create_kb_article_category_failure_raises_without_rollback( + client: GlpiClient, +) -> None: + rec = _Recorder() + rec.install(client) + client._v1 = _FakeV1(error=ValueError("boom")) # type: ignore[assignment] + with pytest.raises(RuntimeError, match="88") as excinfo: + client.create_kb_article( + PostKBArticle(name="P", content="c", categories=[IdNameRef(id=14)]) + ) + # The article is NOT rolled back; the failure just raises, naming the id + # and chaining the original error so the partial state is recoverable. + assert not any(c["method"] == "DELETE" for c in rec.calls) + assert isinstance(excinfo.value.__cause__, ValueError) + assert "boom" in str(excinfo.value.__cause__) + + +def test_create_kb_article_ref_without_id_raises(client: GlpiClient) -> None: + rec = _Recorder() + rec.install(client) + client._v1 = _FakeV1() # type: ignore[assignment] + with pytest.raises(RuntimeError, match="require an 'id'"): + client.create_kb_article( + PostKBArticle(name="P", content="c", categories=[IdNameRef(name="Parrots")]) + ) + assert not any(c["method"] == "DELETE" for c in rec.calls) + + +def test_create_kb_article_empty_categories_skips_v1(client: GlpiClient) -> None: + rec = _Recorder() + rec.install(client) + assert client._v1 is None # no v1 configured + new_id = client.create_kb_article( + PostKBArticle(name="P", content="c", categories=[]) + ) + assert new_id == 88 # empty list is a no-op on create; no v1 needed + assert not any(c["method"] == "DELETE" for c in rec.calls) # no legacy call + + +def test_update_kb_article_applies_categories_via_v1(client: GlpiClient) -> None: + rec = _Recorder() + rec.install(client) + fake = _FakeV1() + client._v1 = fake # type: ignore[assignment] + client.update_kb_article(5, PatchKBArticle(categories=[IdNameRef(id=14)])) + assert rec.calls[0]["method"] == "PATCH" + assert fake.calls[0]["path"] == "KnowbaseItem/5" + assert fake.calls[0]["json_body"] == {"input": {"_categories": [14]}} + + +def test_update_kb_article_without_categories_skips_v1(client: GlpiClient) -> None: + rec = _Recorder() + rec.install(client) + assert client._v1 is None + client.update_kb_article(5, PatchKBArticle(is_pinned=True)) # no RuntimeError + assert rec.calls[0]["method"] == "PATCH" + + +def test_update_kb_article_category_failure_does_not_roll_back( + client: GlpiClient, +) -> None: + rec = _Recorder() + rec.install(client) + client._v1 = _FakeV1(error=ValueError("boom")) # type: ignore[assignment] + with pytest.raises(ValueError, match="boom"): + client.update_kb_article(5, PatchKBArticle(categories=[IdNameRef(id=14)])) + # Update is intentionally non-atomic: the v2 patch stays, no rollback delete. + assert not any(c["method"] == "DELETE" for c in rec.calls) diff --git a/glpi_python_client/models/api_schema/knowledgebase/_article.py b/glpi_python_client/models/api_schema/knowledgebase/_article.py index d320d96..cd5da3b 100644 --- a/glpi_python_client/models/api_schema/knowledgebase/_article.py +++ b/glpi_python_client/models/api_schema/knowledgebase/_article.py @@ -70,6 +70,12 @@ class PostKBArticle(GlpiModel): arrays. The contract marks ``user.id`` as writable (the article author), so ``user`` is exposed here even though the server defaults it to the current user when omitted. + + ``categories`` looks writable but the GLPI 2.3.0 v2 API drops it (the + nested ``id`` is ``readOnly``). ``GlpiClient.create_kb_article`` / + ``update_kb_article`` apply it through a legacy fallback + (``set_kb_article_categories``), which needs a configured v1 session + pointing at the legacy ``apirest.php``. """ name: str | None = None diff --git a/integration_tests/test_integration_kb.py b/integration_tests/test_integration_kb.py index 705fbed..7b0c519 100644 --- a/integration_tests/test_integration_kb.py +++ b/integration_tests/test_integration_kb.py @@ -53,6 +53,9 @@ def client(live_config: _LiveGlpiConfig) -> Iterator[GlpiClient]: glpi_profile=live_config.profile, entity_recursive=live_config.entity_recursive, verify_ssl=live_config.verify_ssl, + v1_base_url=live_config.v1_base_url, + v1_user_token=live_config.v1_user_token, + v1_app_token=live_config.v1_app_token, ) try: yield glpi_client @@ -154,6 +157,40 @@ def test_kb_article_full_crud_and_markdown(client: GlpiClient) -> None: client.get_kb_article(article_id) +@pytest.mark.usefixtures("kb_available") +def test_kb_article_set_categories( + client: GlpiClient, live_config: _LiveGlpiConfig +) -> None: + """Assign a category to an article via the legacy fallback and read it back. + + Requires ``v1_base_url`` to point at the legacy ``apirest.php`` (the same + session used by document upload / the Fields plugin); skipped otherwise. + """ + + if not live_config.v1_base_url or not live_config.v1_user_token: + pytest.skip("legacy v1 session not configured; category write needs it") + + suffix = _suffix() + category_id = client.create_kb_category(PostKBCategory(name=f"itest-cat-{suffix}")) + try: + article_id = client.create_kb_article( + PostKBArticle(name=f"itest-kb-cat-{suffix}", content="Body.") + ) + try: + client.set_kb_article_categories(article_id, [category_id]) + fetched = client.get_kb_article(article_id) + assert fetched.categories is not None + assert any(c.id == category_id for c in fetched.categories) + + # Clearing removes the link again. + client.set_kb_article_categories(article_id, []) + assert not (client.get_kb_article(article_id).categories or []) + finally: + client.delete_kb_article(article_id, force=True) + finally: + client.delete_kb_category(category_id, force=True) + + @pytest.mark.usefixtures("kb_available") def test_kb_article_comment_full_crud(client: GlpiClient) -> None: """Exercise every comment helper: create, list, get, update, delete."""