diff --git a/plane/api/cycles.py b/plane/api/cycles.py index 5925f55..c39154c 100644 --- a/plane/api/cycles.py +++ b/plane/api/cycles.py @@ -241,24 +241,28 @@ def transfer_work_items( data.model_dump(exclude_none=True), ) - def archive(self, workspace_slug: str, project_id: str, cycle_id: str) -> bool: + def archive(self, workspace_slug: str, project_id: str, cycle_id: str) -> None: """Archive a cycle. Args: workspace_slug: The workspace slug identifier project_id: UUID of the project cycle_id: UUID of the cycle + + Returns: + None (HTTP 204 No Content) """ self._post(f"{workspace_slug}/projects/{project_id}/cycles/{cycle_id}/archive", {}) - return True - def unarchive(self, workspace_slug: str, project_id: str, cycle_id: str) -> bool: + def unarchive(self, workspace_slug: str, project_id: str, cycle_id: str) -> None: """Unarchive a cycle. Args: workspace_slug: The workspace slug identifier project_id: UUID of the project cycle_id: UUID of the cycle + + Returns: + None (HTTP 204 No Content) """ self._delete(f"{workspace_slug}/projects/{project_id}/archived-cycles/{cycle_id}/unarchive") - return True diff --git a/plane/errors/errors.py b/plane/errors/errors.py index e5f67db..03d1130 100644 --- a/plane/errors/errors.py +++ b/plane/errors/errors.py @@ -15,6 +15,13 @@ class ConfigurationError(PlaneError): def __init__(self, message: str) -> None: super().__init__(message, status_code=None) + def __reduce__(self) -> tuple: + # `ConfigurationError.__init__` only takes a `message`, so the + # default `PlaneError.__reduce__` (which passes `status_code`) breaks + # pickling with `TypeError: ConfigurationError.__init__() takes 2 + # positional arguments but 3 were given`. + return (type(self), (str(self),)) + def _extract_detail(payload: object) -> str | None: """Extract a human-readable detail string from an API error payload. diff --git a/tests/scripts/test_cycles.py b/tests/scripts/test_cycles.py index 5f2acd1..e47c047 100644 --- a/tests/scripts/test_cycles.py +++ b/tests/scripts/test_cycles.py @@ -250,10 +250,7 @@ def main() -> None: # Note: Only cycles with end_date in the past can be archived print_step(14, "Archiving cycle 1 (required for transfer)") archived = client.cycles.archive(workspace_slug, project.id, cycle_1.id) - if archived: - print_success(f"Archived cycle: {cycle_1.name} (ID: {cycle_1.id})") - else: - print_error("Failed to archive cycle") + print_success(f"Archived cycle: {cycle_1.name} (ID: {cycle_1.id})") # Transfer work items from archived cycle 1 to cycle 2 # (Transfer only works from completed/archived cycles) @@ -294,10 +291,7 @@ def main() -> None: unarchived = client.cycles.unarchive( workspace_slug, project.id, cycle_1.id ) - if unarchived: - print_success(f"Unarchived cycle: {cycle_1.name} (ID: {cycle_1.id})") - else: - print_error("Failed to unarchive cycle") + print_success(f"Unarchived cycle: {cycle_1.name} (ID: {cycle_1.id})") # Verify archived cycles list after unarchive archived_after_unarchive = client.cycles.list_archived(workspace_slug, project.id) diff --git a/tests/unit/test_cycles.py b/tests/unit/test_cycles.py index 82a9cbd..a6ec930 100644 --- a/tests/unit/test_cycles.py +++ b/tests/unit/test_cycles.py @@ -1,10 +1,12 @@ """Unit tests for Cycles API resource (smoke tests with real HTTP requests).""" from datetime import datetime +from unittest.mock import MagicMock import pytest from plane.client import PlaneClient +from plane.config import Configuration from plane.models.cycles import CreateCycle, Cycle, CycleLite, UpdateCycle from plane.models.projects import Project, ProjectFeature from plane.models.query_params import CycleLiteListQueryParams, CycleListQueryParams @@ -161,3 +163,79 @@ def test_list_work_items( assert hasattr(response, "count") assert isinstance(response.results, list) + + +# --------------------------------------------------------------------------- +# Pure unit tests for archive/unarchive return type +# --------------------------------------------------------------------------- +class TestCyclesArchiveReturnType: + """Regression tests: ``Cycles.archive`` and ``Cycles.unarchive`` historically + were annotated ``-> bool`` and hardcoded ``return True``, but their + ``_post`` / ``_delete`` calls return ``None`` on 2xx and raise on errors. + The signature now matches the docstring (``-> None``) and the rest of the + SDK. + """ + + @pytest.fixture + def cycles(self): + from plane.api.cycles import Cycles + + config = Configuration( + base_path="https://api.plane.so", + api_key="test-key", + ) + return Cycles(config) + + def test_archive_signature_returns_none(self, cycles) -> None: + """Annotation must be ``None``, matching the rest of the SDK.""" + import inspect + + sig = inspect.signature(cycles.archive) + assert sig.return_annotation is None, ( + f"archive should return None, got {sig.return_annotation!r}" + ) + + def test_unarchive_signature_returns_none(self, cycles) -> None: + """Annotation must be ``None``, matching the rest of the SDK.""" + import inspect + + sig = inspect.signature(cycles.unarchive) + assert sig.return_annotation is None, ( + f"unarchive should return None, got {sig.return_annotation!r}" + ) + + def test_archive_returns_none_on_success(self, cycles) -> None: + """On a 204, ``archive`` should return ``None`` (not a hardcoded ``True``).""" + cycles._post = MagicMock(return_value=None) + result = cycles.archive("ws", "proj-id", "cycle-id") + assert result is None + cycles._post.assert_called_once() + + def test_unarchive_returns_none_on_success(self, cycles) -> None: + """On a 204, ``unarchive`` should return ``None`` (not a hardcoded ``True``).""" + cycles._delete = MagicMock(return_value=None) + result = cycles.unarchive("ws", "proj-id", "cycle-id") + assert result is None + cycles._delete.assert_called_once() + + def test_archive_propagates_http_errors(self, cycles) -> None: + """An HTTP error from ``_post`` must propagate -- the old ``return True`` + silently swallowed 4xx/5xx into a misleading ``True``. + """ + from plane.errors.errors import HttpError + + cycles._post = MagicMock( + side_effect=HttpError("HTTP 400: Bad Request", 400, {"detail": "oops"}) + ) + with pytest.raises(HttpError): + cycles.archive("ws", "proj-id", "cycle-id") + + def test_unarchive_propagates_http_errors(self, cycles) -> None: + from plane.errors.errors import HttpError + + cycles._delete = MagicMock( + side_effect=HttpError("HTTP 404: Not Found", 404, {"detail": "not found"}) + ) + with pytest.raises(HttpError): + cycles.unarchive("ws", "proj-id", "cycle-id") + diff --git a/tests/unit/test_errors.py b/tests/unit/test_errors.py index ee8a3ab..4108f9f 100644 --- a/tests/unit/test_errors.py +++ b/tests/unit/test_errors.py @@ -164,6 +164,21 @@ def test_catchable_as_plane_error(self) -> None: with pytest.raises(PlaneError): raise ConfigurationError("test") + def test_error_can_be_pickled(self) -> None: + """Regression test: ``PlaneError.__reduce__`` passes ``status_code`` to + ``__init__``, but ``ConfigurationError.__init__`` only accepts a + ``message``. Without an override, ``pickle.dumps`` raises + ``TypeError: ConfigurationError.__init__() takes 2 positional arguments + but 3 were given``. + """ + import pickle + + err = ConfigurationError("missing api_key") + roundtripped = pickle.loads(pickle.dumps(err)) + assert isinstance(roundtripped, ConfigurationError) + assert str(roundtripped) == "missing api_key" + assert roundtripped.status_code is None + # --------------------------------------------------------------------------- # HttpError — __str__