diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 798959b..956715b 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -39,7 +39,7 @@ jobs: run: uv sync --all-extras - name: Run tests with coverage - run: uv run pytest --cov=bloomy --cov-report=term-missing --cov-report=xml + run: uv run pytest -m "not integration" --cov=bloomy --cov-report=term-missing --cov-report=xml - name: Upload coverage report if: matrix.python-version == '3.12' diff --git a/pyproject.toml b/pyproject.toml index 6f6cb90..df3c535 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "bloomy-python" -version = "0.21.0" +version = "0.21.1" description = "Python SDK for Bloom Growth API" readme = "README.md" authors = [{ name = "Franccesco Orozco", email = "franccesco@codingdose.info" }] @@ -79,6 +79,9 @@ reportMissingImports = true testpaths = ["tests"] pythonpath = ["src"] addopts = "-ra --strict-markers --cov=bloomy --cov-report=term-missing" +markers = [ + "integration: marks tests that hit the real Bloom Growth API (deselect with '-m \"not integration\"')", +] [dependency-groups] dev = [ diff --git a/src/bloomy/configuration.py b/src/bloomy/configuration.py index 179943c..6312210 100644 --- a/src/bloomy/configuration.py +++ b/src/bloomy/configuration.py @@ -32,7 +32,10 @@ def __init__(self, api_key: str | None = None) -> None: ``` """ - self.api_key = api_key or os.environ.get("BG_API_KEY") or self._load_api_key() + stripped_key = api_key.strip() if api_key else api_key + self.api_key = ( + stripped_key or os.environ.get("BG_API_KEY") or self._load_api_key() + ) def configure_api_key( self, username: str, password: str, store_key: bool = False diff --git a/src/bloomy/models.py b/src/bloomy/models.py index 0c4fc7b..061fe51 100644 --- a/src/bloomy/models.py +++ b/src/bloomy/models.py @@ -88,10 +88,10 @@ class UserSearchResult(BloomyBaseModel): id: int name: str - description: str + description: str | None = None email: str organization_id: int - image_url: str + image_url: str | None = None class UserListItem(BloomyBaseModel): @@ -100,8 +100,8 @@ class UserListItem(BloomyBaseModel): id: int name: str email: str - position: str - image_url: str + position: str | None = None + image_url: str | None = None class MeetingAttendee(BloomyBaseModel): @@ -265,7 +265,7 @@ class CreatedGoalInfo(BloomyBaseModel): user_name: str title: str meeting_id: int - meeting_title: str + meeting_title: str | None = None status: str created_at: str diff --git a/src/bloomy/operations/async_/goals.py b/src/bloomy/operations/async_/goals.py index 6b4a770..212fd82 100644 --- a/src/bloomy/operations/async_/goals.py +++ b/src/bloomy/operations/async_/goals.py @@ -103,17 +103,14 @@ async def update( Args: goal_id: The ID of the goal to update title: The new title of the goal - accountable_user: The ID of the user responsible for the goal - (default: initialized user ID) + accountable_user: The ID of the user responsible for the goal. + If not provided, the existing owner is preserved. status: The status value. Can be a GoalStatus enum member or string ('on', 'off', or 'complete'). Use GoalStatus.ON_TRACK, GoalStatus.AT_RISK, or GoalStatus.COMPLETE for type safety. Invalid values will raise ValueError via the update payload builder. """ - if accountable_user is None: - accountable_user = await self.get_user_id() - payload = self._build_goal_update_payload(accountable_user, title, status) response = await self._client.put(f"rocks/{goal_id}", json=payload) diff --git a/src/bloomy/operations/async_/headlines.py b/src/bloomy/operations/async_/headlines.py index cfaef2e..0ddaec2 100644 --- a/src/bloomy/operations/async_/headlines.py +++ b/src/bloomy/operations/async_/headlines.py @@ -56,7 +56,7 @@ async def create( id=data["Id"], title=data["Name"], owner_details=OwnerDetails(id=owner_id, name=None), - notes_url=data.get("DetailsUrl", ""), + notes_url=data.get("DetailsUrl") or "", ) async def update(self, headline_id: int, title: str) -> None: @@ -106,10 +106,10 @@ async def list( ValueError: If both user_id and meeting_id are provided """ - if user_id and meeting_id: + if user_id is not None and meeting_id is not None: raise ValueError("Please provide either user_id or meeting_id, not both.") - if meeting_id: + if meeting_id is not None: response = await self._client.get(f"l10/{meeting_id}/headlines") else: if user_id is None: diff --git a/src/bloomy/operations/async_/issues.py b/src/bloomy/operations/async_/issues.py index 90c2117..915a110 100644 --- a/src/bloomy/operations/async_/issues.py +++ b/src/bloomy/operations/async_/issues.py @@ -54,12 +54,12 @@ async def list( ValueError: When both user_id and meeting_id are provided """ - if user_id and meeting_id: + if user_id is not None and meeting_id is not None: raise ValueError( "Please provide either `user_id` or `meeting_id`, not both." ) - if meeting_id: + if meeting_id is not None: response = await self._client.get(f"l10/{meeting_id}/issues") else: if user_id is None: diff --git a/src/bloomy/operations/async_/meetings.py b/src/bloomy/operations/async_/meetings.py index a7591fb..8a63bb5 100644 --- a/src/bloomy/operations/async_/meetings.py +++ b/src/bloomy/operations/async_/meetings.py @@ -4,9 +4,8 @@ import asyncio import builtins -from typing import TYPE_CHECKING, Any +from typing import Any -from ...exceptions import APIError from ...models import ( BulkCreateError, BulkCreateResult, @@ -20,9 +19,6 @@ from ...utils.async_base_operations import AsyncBaseOperations from ..mixins.meetings_transform import MeetingOperationsMixin -if TYPE_CHECKING: - pass - class AsyncMeetingOperations(AsyncBaseOperations, MeetingOperationsMixin): """Async class to handle all operations related to meetings. @@ -176,9 +172,6 @@ async def details( Returns: A MeetingDetails model instance with comprehensive meeting information - Raises: - APIError: If the meeting with the given ID is not found - Example: ```python await client.meeting.details(1) @@ -187,11 +180,9 @@ async def details( ``` """ - meetings = await self.list() - meeting = next((m for m in meetings if m.id == meeting_id), None) - - if not meeting: - raise APIError(f"Meeting with ID {meeting_id} not found", status_code=404) + response = await self._client.get(f"L10/{meeting_id}") + response.raise_for_status() + data: Any = response.json() # Fetch all sub-resources in parallel for better performance attendees_task = asyncio.create_task(self.attendees(meeting_id)) @@ -209,11 +200,11 @@ async def details( ) return MeetingDetails( - id=meeting.id, - name=meeting.name, - start_date_utc=getattr(meeting, "start_date_utc", None), - created_date=getattr(meeting, "created_date", None), - organization_id=getattr(meeting, "organization_id", None), + id=data["Id"], + name=data.get("Basics", {}).get("Name", ""), + start_date_utc=data.get("StartDateUtc"), + created_date=data.get("CreateTime"), + organization_id=data.get("OrganizationId"), attendees=attendees, issues=issues, todos=todos, diff --git a/src/bloomy/operations/async_/scorecard.py b/src/bloomy/operations/async_/scorecard.py index 71b8259..28b40bc 100644 --- a/src/bloomy/operations/async_/scorecard.py +++ b/src/bloomy/operations/async_/scorecard.py @@ -51,12 +51,12 @@ async def list( ValueError: If both user_id and meeting_id are provided """ - if user_id and meeting_id: + if user_id is not None and meeting_id is not None: raise ValueError( "Please provide either `user_id` or `meeting_id`, not both." ) - if meeting_id: + if meeting_id is not None: response = await self._client.get(f"scorecard/meeting/{meeting_id}") else: if user_id is None: diff --git a/src/bloomy/operations/async_/todos.py b/src/bloomy/operations/async_/todos.py index e350e2b..ddd7f40 100644 --- a/src/bloomy/operations/async_/todos.py +++ b/src/bloomy/operations/async_/todos.py @@ -3,7 +3,7 @@ from __future__ import annotations import builtins -from datetime import datetime +from datetime import UTC, datetime from typing import TYPE_CHECKING from ...models import BulkCreateResult, Todo @@ -120,7 +120,7 @@ async def create( "DetailsUrl": data.get("DetailsUrl"), "DueDate": data.get("DueDate"), "CompleteTime": None, - "CreateTime": data.get("CreateTime", datetime.now().isoformat()), + "CreateTime": data.get("CreateTime", datetime.now(UTC).isoformat()), "OriginId": meeting_id, "Origin": None, "Complete": False, diff --git a/src/bloomy/operations/goals.py b/src/bloomy/operations/goals.py index be73c7c..5f62fe0 100644 --- a/src/bloomy/operations/goals.py +++ b/src/bloomy/operations/goals.py @@ -136,8 +136,8 @@ def update( Args: goal_id: The ID of the goal to update title: The new title of the goal - accountable_user: The ID of the user responsible for the goal - (default: initialized user ID) + accountable_user: The ID of the user responsible for the goal. + If not provided, the existing owner is preserved. status: The status value. Can be a GoalStatus enum member or string ('on', 'off', or 'complete'). Use GoalStatus.ON_TRACK, GoalStatus.AT_RISK, or GoalStatus.COMPLETE for type safety. @@ -155,9 +155,6 @@ def update( ``` """ - if accountable_user is None: - accountable_user = self.user_id - payload = self._build_goal_update_payload(accountable_user, title, status) response = self._client.put(f"rocks/{goal_id}", json=payload) diff --git a/src/bloomy/operations/headlines.py b/src/bloomy/operations/headlines.py index 77f8bad..a19a83f 100644 --- a/src/bloomy/operations/headlines.py +++ b/src/bloomy/operations/headlines.py @@ -52,7 +52,7 @@ def create( id=data["Id"], title=data["Name"], owner_details=OwnerDetails(id=owner_id, name=None), - notes_url=data.get("DetailsUrl", ""), + notes_url=data.get("DetailsUrl") or "", ) def update(self, headline_id: int, title: str) -> None: @@ -125,10 +125,10 @@ def list( ``` """ - if user_id and meeting_id: + if user_id is not None and meeting_id is not None: raise ValueError("Please provide either user_id or meeting_id, not both.") - if meeting_id: + if meeting_id is not None: response = self._client.get(f"l10/{meeting_id}/headlines") else: if user_id is None: diff --git a/src/bloomy/operations/issues.py b/src/bloomy/operations/issues.py index a5f75c9..d540838 100644 --- a/src/bloomy/operations/issues.py +++ b/src/bloomy/operations/issues.py @@ -73,12 +73,12 @@ def list( ``` """ - if user_id and meeting_id: + if user_id is not None and meeting_id is not None: raise ValueError( "Please provide either `user_id` or `meeting_id`, not both." ) - if meeting_id: + if meeting_id is not None: response = self._client.get(f"l10/{meeting_id}/issues") else: if user_id is None: diff --git a/src/bloomy/operations/meetings.py b/src/bloomy/operations/meetings.py index fd93efc..15e7850 100644 --- a/src/bloomy/operations/meetings.py +++ b/src/bloomy/operations/meetings.py @@ -5,7 +5,6 @@ import builtins from typing import Any -from ..exceptions import APIError from ..models import ( BulkCreateError, BulkCreateResult, @@ -167,9 +166,6 @@ def details(self, meeting_id: int, include_closed: bool = False) -> MeetingDetai Returns: A MeetingDetails model instance with comprehensive meeting information - Raises: - APIError: If the meeting with the specified ID is not found - Example: ```python client.meeting.details(1) @@ -178,18 +174,16 @@ def details(self, meeting_id: int, include_closed: bool = False) -> MeetingDetai ``` """ - meetings = self.list() - meeting = next((m for m in meetings if m.id == meeting_id), None) - - if not meeting: - raise APIError(f"Meeting with ID {meeting_id} not found", status_code=404) + response = self._client.get(f"L10/{meeting_id}") + response.raise_for_status() + data: Any = response.json() return MeetingDetails( - id=meeting.id, - name=meeting.name, - start_date_utc=getattr(meeting, "start_date_utc", None), - created_date=getattr(meeting, "created_date", None), - organization_id=getattr(meeting, "organization_id", None), + id=data["Id"], + name=data.get("Basics", {}).get("Name", ""), + start_date_utc=data.get("StartDateUtc"), + created_date=data.get("CreateTime"), + organization_id=data.get("OrganizationId"), attendees=self.attendees(meeting_id), issues=self.issues(meeting_id, include_closed=include_closed), todos=self.todos(meeting_id, include_closed=include_closed), diff --git a/src/bloomy/operations/mixins/goals_transform.py b/src/bloomy/operations/mixins/goals_transform.py index 3720441..a0edc5c 100644 --- a/src/bloomy/operations/mixins/goals_transform.py +++ b/src/bloomy/operations/mixins/goals_transform.py @@ -37,9 +37,15 @@ def _transform_goal_list(self, data: Sequence[dict[str, Any]]) -> list[GoalInfo] created_at=goal["CreateTime"], due_date=goal["DueDate"], status="Completed" if goal.get("Complete") else "Incomplete", - meeting_id=goal["Origins"][0]["Id"] if goal.get("Origins") else None, + meeting_id=( + goal["Origins"][0]["Id"] + if goal.get("Origins") and goal["Origins"][0] + else None + ), meeting_title=( - goal["Origins"][0]["Name"] if goal.get("Origins") else None + goal["Origins"][0]["Name"] + if goal.get("Origins") and goal["Origins"][0] + else None ), ) for goal in data @@ -93,14 +99,18 @@ def _transform_created_goal( user_name=data["Owner"]["Name"], title=title, meeting_id=meeting_id, - meeting_title=data["Origins"][0]["Name"], + meeting_title=( + data["Origins"][0]["Name"] + if data.get("Origins") and data["Origins"][0] + else None + ), status=status, created_at=data["CreateTime"], ) def _build_goal_update_payload( self, - accountable_user: int, + accountable_user: int | None = None, title: str | None = None, status: GoalStatus | str | None = None, ) -> dict[str, Any]: @@ -108,6 +118,7 @@ def _build_goal_update_payload( Args: accountable_user: The ID of the user responsible for the goal. + Only included in the payload when explicitly provided. title: The new title of the goal. status: The status value (GoalStatus enum or string). @@ -118,7 +129,10 @@ def _build_goal_update_payload( ValueError: If an invalid status value is provided. """ - payload: dict[str, Any] = {"accountableUserId": accountable_user} + payload: dict[str, Any] = {} + + if accountable_user is not None: + payload["accountableUserId"] = accountable_user if title is not None: payload["title"] = title diff --git a/src/bloomy/operations/mixins/meetings_transform.py b/src/bloomy/operations/mixins/meetings_transform.py index c457879..db5c727 100644 --- a/src/bloomy/operations/mixins/meetings_transform.py +++ b/src/bloomy/operations/mixins/meetings_transform.py @@ -63,7 +63,7 @@ def _transform_metrics(self, raw_data: Any) -> list[ScorecardMetric]: measurable_id = item_dict.get("Id") measurable_name = item_dict.get("Name") - if not measurable_id or not measurable_name: + if measurable_id is None or not measurable_name: continue owner_data = item_dict.get("Owner", {}) @@ -101,19 +101,22 @@ def _transform_meeting_issues( A list of Issue models. """ - return [ - Issue( - Id=issue["Id"], - Name=issue["Name"], - DetailsUrl=issue["DetailsUrl"], - CreateDate=issue["CreateTime"], - ClosedDate=issue["CloseTime"], - CompletionDate=issue.get("CompleteTime"), - OwnerId=issue.get("Owner", {}).get("Id", 0), - OwnerName=issue.get("Owner", {}).get("Name", ""), - OwnerImageUrl=issue.get("Owner", {}).get("ImageUrl", ""), - MeetingId=meeting_id, - MeetingName=issue["Origin"], + issues: list[Issue] = [] + for issue in data: + owner: dict[str, Any] = issue.get("Owner") or {} + issues.append( + Issue( + Id=issue["Id"], + Name=issue["Name"], + DetailsUrl=issue["DetailsUrl"], + CreateDate=issue["CreateTime"], + ClosedDate=issue["CloseTime"], + CompletionDate=issue.get("CompleteTime"), + OwnerId=owner.get("Id", 0), + OwnerName=owner.get("Name", ""), + OwnerImageUrl=owner.get("ImageUrl", ""), + MeetingId=meeting_id, + MeetingName=issue["Origin"], + ) ) - for issue in data - ] + return issues diff --git a/src/bloomy/operations/mixins/users_transform.py b/src/bloomy/operations/mixins/users_transform.py index e18abbc..e7d1bc4 100644 --- a/src/bloomy/operations/mixins/users_transform.py +++ b/src/bloomy/operations/mixins/users_transform.py @@ -130,7 +130,7 @@ def _transform_user_list( user for user in users if user["ResultType"] == "User" - and (include_placeholders or user["ImageUrl"] != "/i/userplaceholder") + and (include_placeholders or user.get("Email", "") != "") ] return [ diff --git a/src/bloomy/operations/scorecard.py b/src/bloomy/operations/scorecard.py index a052c20..1b7ad88 100644 --- a/src/bloomy/operations/scorecard.py +++ b/src/bloomy/operations/scorecard.py @@ -84,12 +84,12 @@ def list( positive value. """ - if user_id and meeting_id: + if user_id is not None and meeting_id is not None: raise ValueError( "Please provide either `user_id` or `meeting_id`, not both." ) - if meeting_id: + if meeting_id is not None: response = self._client.get(f"scorecard/meeting/{meeting_id}") else: if user_id is None: diff --git a/src/bloomy/operations/todos.py b/src/bloomy/operations/todos.py index bcaaf71..d7f1c97 100644 --- a/src/bloomy/operations/todos.py +++ b/src/bloomy/operations/todos.py @@ -3,7 +3,7 @@ from __future__ import annotations import builtins -from datetime import datetime +from datetime import UTC, datetime from typing import TYPE_CHECKING from ..models import BulkCreateResult, Todo @@ -120,7 +120,7 @@ def create( "DetailsUrl": data.get("DetailsUrl"), "DueDate": data.get("DueDate"), "CompleteTime": None, - "CreateTime": data.get("CreateTime", datetime.now().isoformat()), + "CreateTime": data.get("CreateTime", datetime.now(UTC).isoformat()), "OriginId": meeting_id, "Origin": None, "Complete": False, diff --git a/tests/test_adversarial_issues_headlines.py b/tests/test_adversarial_issues_headlines.py new file mode 100644 index 0000000..ef31ccf --- /dev/null +++ b/tests/test_adversarial_issues_headlines.py @@ -0,0 +1,701 @@ +"""Adversarial tests for Issues and Headlines operations. + +These tests target known bugs and edge cases: +1. Truthiness bug: `if user_id and meeting_id` should be + `if user_id is not None and meeting_id is not None` +2. Transform functions assuming all keys exist in API responses +3. Edge cases with None/missing Owner dicts, CloseTime, empty strings +""" + +from __future__ import annotations + +from typing import Any +from unittest.mock import AsyncMock, Mock, PropertyMock, patch + +import pytest + +from bloomy.operations.async_.headlines import AsyncHeadlineOperations +from bloomy.operations.async_.issues import AsyncIssueOperations +from bloomy.operations.headlines import HeadlineOperations +from bloomy.operations.issues import IssueOperations +from bloomy.operations.mixins.headlines_transform import HeadlineOperationsMixin +from bloomy.operations.mixins.issues_transform import IssueOperationsMixin + +# --------------------------------------------------------------------------- +# Fixtures +# --------------------------------------------------------------------------- + + +@pytest.fixture +def mock_http_client() -> Mock: + """Create a mock sync HTTP client. + + Returns: + A mock HTTP client. + + """ + client = Mock() + client.get = Mock() + client.post = Mock() + client.put = Mock() + client.delete = Mock() + return client + + +@pytest.fixture +def mock_async_http_client() -> AsyncMock: + """Create a mock async HTTP client. + + Returns: + A mock async HTTP client. + + """ + client = AsyncMock() + client.get = AsyncMock() + client.post = AsyncMock() + client.put = AsyncMock() + client.delete = AsyncMock() + return client + + +@pytest.fixture +def issue_ops(mock_http_client: Mock) -> IssueOperations: + """Create IssueOperations with a mock client. + + Returns: + An IssueOperations instance. + + """ + return IssueOperations(mock_http_client) + + +@pytest.fixture +def headline_ops(mock_http_client: Mock) -> HeadlineOperations: + """Create HeadlineOperations with a mock client. + + Returns: + A HeadlineOperations instance. + + """ + return HeadlineOperations(mock_http_client) + + +@pytest.fixture +def async_issue_ops(mock_async_http_client: AsyncMock) -> AsyncIssueOperations: + """Create AsyncIssueOperations with a mock client. + + Returns: + An AsyncIssueOperations instance. + + """ + return AsyncIssueOperations(mock_async_http_client) + + +@pytest.fixture +def async_headline_ops(mock_async_http_client: AsyncMock) -> AsyncHeadlineOperations: + """Create AsyncHeadlineOperations with a mock client. + + Returns: + An AsyncHeadlineOperations instance. + + """ + return AsyncHeadlineOperations(mock_async_http_client) + + +def _make_mock_response(data: Any) -> Mock: + """Create a mock response with given JSON data. + + Returns: + A mock response object. + + """ + resp = Mock() + resp.json.return_value = data + resp.raise_for_status = Mock() + return resp + + +def _make_async_mock_response(data: Any) -> Mock: + """Create a mock response for async client. + + Returns: + A mock response object. + + """ + resp = Mock() + resp.json.return_value = data + resp.raise_for_status = Mock() + return resp + + +# =========================================================================== +# 1. TRUTHINESS BUG: `if user_id and meeting_id` vs `if ... is not None` +# +# When user_id=0 (a valid int) and meeting_id is also provided, the truthiness +# check `if user_id and meeting_id` evaluates to `if 0 and meeting_id` which +# is False, so the ValueError is never raised. The correct check is: +# `if user_id is not None and meeting_id is not None` +# =========================================================================== + + +class TestIssueListTruthinessBug: + """Test the truthiness bug in IssueOperations.list().""" + + def test_user_id_zero_and_meeting_id_should_raise( + self, issue_ops: IssueOperations + ) -> None: + """list(user_id=0, meeting_id=123) must raise ValueError. + + BUG: `if user_id and meeting_id` treats user_id=0 as falsy, + so it silently falls through to the meeting_id branch instead of + raising the expected ValueError. + """ + with pytest.raises(ValueError, match="Please provide either"): + issue_ops.list(user_id=0, meeting_id=123) + + def test_meeting_id_zero_and_user_id_should_raise( + self, issue_ops: IssueOperations + ) -> None: + """list(user_id=123, meeting_id=0) must raise ValueError. + + BUG: `if user_id and meeting_id` treats meeting_id=0 as falsy. + """ + with pytest.raises(ValueError, match="Please provide either"): + issue_ops.list(user_id=123, meeting_id=0) + + def test_both_zero_should_raise(self, issue_ops: IssueOperations) -> None: + """list(user_id=0, meeting_id=0) must raise ValueError.""" + with pytest.raises(ValueError, match="Please provide either"): + issue_ops.list(user_id=0, meeting_id=0) + + def test_empty_string_ids_should_still_raise( + self, issue_ops: IssueOperations + ) -> None: + """list(user_id='', meeting_id='') — empty strings are also falsy. + + Even though the type hints say int|None, Python doesn't enforce at + runtime. This confirms the guard is robust against falsy non-None values. + """ + with pytest.raises(ValueError, match="Please provide either"): + issue_ops.list(user_id="", meeting_id="") # type: ignore[arg-type] + + +class TestHeadlineListTruthinessBug: + """Test the truthiness bug in HeadlineOperations.list().""" + + def test_user_id_zero_and_meeting_id_should_raise( + self, headline_ops: HeadlineOperations + ) -> None: + """list(user_id=0, meeting_id=123) must raise ValueError.""" + with pytest.raises(ValueError, match="Please provide either"): + headline_ops.list(user_id=0, meeting_id=123) + + def test_meeting_id_zero_and_user_id_should_raise( + self, headline_ops: HeadlineOperations + ) -> None: + """list(user_id=123, meeting_id=0) must raise ValueError.""" + with pytest.raises(ValueError, match="Please provide either"): + headline_ops.list(user_id=123, meeting_id=0) + + def test_both_zero_should_raise(self, headline_ops: HeadlineOperations) -> None: + """list(user_id=0, meeting_id=0) must raise ValueError.""" + with pytest.raises(ValueError, match="Please provide either"): + headline_ops.list(user_id=0, meeting_id=0) + + +class TestAsyncIssueListTruthinessBug: + """Test the truthiness bug in AsyncIssueOperations.list().""" + + @pytest.mark.asyncio + async def test_user_id_zero_and_meeting_id_should_raise( + self, async_issue_ops: AsyncIssueOperations + ) -> None: + """Async list(user_id=0, meeting_id=123) must raise ValueError.""" + with pytest.raises(ValueError, match="Please provide either"): + await async_issue_ops.list(user_id=0, meeting_id=123) + + @pytest.mark.asyncio + async def test_meeting_id_zero_and_user_id_should_raise( + self, async_issue_ops: AsyncIssueOperations + ) -> None: + """Async list(user_id=123, meeting_id=0) must raise ValueError.""" + with pytest.raises(ValueError, match="Please provide either"): + await async_issue_ops.list(user_id=123, meeting_id=0) + + @pytest.mark.asyncio + async def test_both_zero_should_raise( + self, async_issue_ops: AsyncIssueOperations + ) -> None: + """Async list(user_id=0, meeting_id=0) must raise ValueError.""" + with pytest.raises(ValueError, match="Please provide either"): + await async_issue_ops.list(user_id=0, meeting_id=0) + + +class TestAsyncHeadlineListTruthinessBug: + """Test the truthiness bug in AsyncHeadlineOperations.list().""" + + @pytest.mark.asyncio + async def test_user_id_zero_and_meeting_id_should_raise( + self, async_headline_ops: AsyncHeadlineOperations + ) -> None: + """Async list(user_id=0, meeting_id=123) must raise ValueError.""" + with pytest.raises(ValueError, match="Please provide either"): + await async_headline_ops.list(user_id=0, meeting_id=123) + + @pytest.mark.asyncio + async def test_meeting_id_zero_and_user_id_should_raise( + self, async_headline_ops: AsyncHeadlineOperations + ) -> None: + """Async list(user_id=123, meeting_id=0) must raise ValueError.""" + with pytest.raises(ValueError, match="Please provide either"): + await async_headline_ops.list(user_id=123, meeting_id=0) + + @pytest.mark.asyncio + async def test_both_zero_should_raise( + self, async_headline_ops: AsyncHeadlineOperations + ) -> None: + """Async list(user_id=0, meeting_id=0) must raise ValueError.""" + with pytest.raises(ValueError, match="Please provide either"): + await async_headline_ops.list(user_id=0, meeting_id=0) + + +# =========================================================================== +# 2. SECONDARY TRUTHINESS BUG: `if meeting_id:` branch routing +# +# After the ValueError guard, `if meeting_id:` is used to choose the API +# endpoint. If meeting_id=0 (valid but falsy), it falls through to the +# user_id branch instead of calling the meeting endpoint. +# =========================================================================== + + +class TestMeetingIdZeroRoutingBug: + """When meeting_id=0 is provided alone, it should hit the meeting endpoint.""" + + def test_issue_list_meeting_id_zero_routes_to_meeting_endpoint( + self, mock_http_client: Mock + ) -> None: + """list(meeting_id=0) should call l10/0/issues, not issues/users/...""" + mock_http_client.get.return_value = _make_mock_response([]) + issue_ops = IssueOperations(mock_http_client) + + with patch( + "bloomy.utils.base_operations.BaseOperations.user_id", + new_callable=PropertyMock, + return_value=999, + ): + issue_ops.list(meeting_id=0) + + mock_http_client.get.assert_called_once_with("l10/0/issues") + + def test_headline_list_meeting_id_zero_routes_to_meeting_endpoint( + self, mock_http_client: Mock + ) -> None: + """list(meeting_id=0) should call l10/0/headlines, not headline/users/...""" + mock_http_client.get.return_value = _make_mock_response([]) + headline_ops = HeadlineOperations(mock_http_client) + + with patch( + "bloomy.utils.base_operations.BaseOperations.user_id", + new_callable=PropertyMock, + return_value=999, + ): + headline_ops.list(meeting_id=0) + + mock_http_client.get.assert_called_once_with("l10/0/headlines") + + +# =========================================================================== +# 3. TRANSFORM BUGS: Missing keys in API response dicts +# =========================================================================== + + +class TestIssueTransformMissingKeys: + """Test _transform_issue_details with incomplete API responses.""" + + def test_missing_owner_key_raises_key_error(self) -> None: + """_transform_issue_details crashes if 'Owner' key is missing.""" + mixin = IssueOperationsMixin() + data: dict[str, Any] = { + "Id": 1, + "Name": "Test", + "DetailsUrl": "http://example.com", + "CreateTime": "2024-01-01", + "CloseTime": None, + "OriginId": 10, + "Origin": "Meeting", + # "Owner" key is missing entirely + } + with pytest.raises(KeyError): + mixin._transform_issue_details(data) + + def test_owner_missing_id_raises_key_error(self) -> None: + """_transform_issue_details crashes if Owner dict lacks 'Id'.""" + mixin = IssueOperationsMixin() + data: dict[str, Any] = { + "Id": 1, + "Name": "Test", + "DetailsUrl": "http://example.com", + "CreateTime": "2024-01-01", + "CloseTime": None, + "OriginId": 10, + "Origin": "Meeting", + "Owner": {"Name": "John"}, # Missing 'Id' + } + with pytest.raises(KeyError): + mixin._transform_issue_details(data) + + def test_owner_missing_name_raises_key_error(self) -> None: + """_transform_issue_details crashes if Owner dict lacks 'Name'.""" + mixin = IssueOperationsMixin() + data: dict[str, Any] = { + "Id": 1, + "Name": "Test", + "DetailsUrl": "http://example.com", + "CreateTime": "2024-01-01", + "CloseTime": None, + "OriginId": 10, + "Origin": "Meeting", + "Owner": {"Id": 1}, # Missing 'Name' + } + with pytest.raises(KeyError): + mixin._transform_issue_details(data) + + def test_missing_close_time_key_raises_key_error(self) -> None: + """_transform_issue_details crashes if 'CloseTime' key is absent. + + The transform accesses data['CloseTime'] directly without .get(), + so a completely missing key will raise KeyError. + """ + mixin = IssueOperationsMixin() + data: dict[str, Any] = { + "Id": 1, + "Name": "Test", + "DetailsUrl": "http://example.com", + "CreateTime": "2024-01-01", + # "CloseTime" missing + "OriginId": 10, + "Origin": "Meeting", + "Owner": {"Id": 1, "Name": "John"}, + } + with pytest.raises(KeyError): + mixin._transform_issue_details(data) + + def test_owner_is_none_raises_type_error(self) -> None: + """_transform_issue_details crashes if Owner is None (not a dict).""" + mixin = IssueOperationsMixin() + data: dict[str, Any] = { + "Id": 1, + "Name": "Test", + "DetailsUrl": "http://example.com", + "CreateTime": "2024-01-01", + "CloseTime": None, + "OriginId": 10, + "Origin": "Meeting", + "Owner": None, + } + with pytest.raises(TypeError): + mixin._transform_issue_details(data) + + def test_close_time_none_succeeds(self) -> None: + """CloseTime=None is a valid value and should work fine.""" + mixin = IssueOperationsMixin() + data: dict[str, Any] = { + "Id": 1, + "Name": "Test", + "DetailsUrl": "http://example.com", + "CreateTime": "2024-01-01", + "CloseTime": None, + "OriginId": 10, + "Origin": "Meeting", + "Owner": {"Id": 1, "Name": "John"}, + } + result = mixin._transform_issue_details(data) + assert result.completed_at is None + + def test_close_time_empty_string(self) -> None: + """CloseTime='' should work (the model field is str|None).""" + mixin = IssueOperationsMixin() + data: dict[str, Any] = { + "Id": 1, + "Name": "Test", + "DetailsUrl": "http://example.com", + "CreateTime": "2024-01-01", + "CloseTime": "", + "OriginId": 10, + "Origin": "Meeting", + "Owner": {"Id": 1, "Name": "John"}, + } + result = mixin._transform_issue_details(data) + # Empty string should be accepted (model field is str|None) + assert result.completed_at is not None or result.completed_at is None + + +class TestCreatedIssueTransformMissingKeys: + """Test _transform_created_issue with incomplete API responses.""" + + def test_missing_owner_key(self) -> None: + """_transform_created_issue crashes if Owner is missing.""" + mixin = IssueOperationsMixin() + data: dict[str, Any] = { + "Id": 1, + "OriginId": 10, + "Origin": "Meeting", + "Name": "Test", + "DetailsUrl": "http://example.com", + # "Owner" missing + } + with pytest.raises(KeyError): + mixin._transform_created_issue(data) + + def test_owner_none(self) -> None: + """_transform_created_issue crashes if Owner is None.""" + mixin = IssueOperationsMixin() + data: dict[str, Any] = { + "Id": 1, + "OriginId": 10, + "Origin": "Meeting", + "Name": "Test", + "DetailsUrl": "http://example.com", + "Owner": None, + } + with pytest.raises(TypeError): + mixin._transform_created_issue(data) + + +class TestHeadlineTransformMissingKeys: + """Test headline transforms with missing/incomplete data.""" + + def test_details_missing_owner_key(self) -> None: + """_transform_headline_details crashes if Owner is missing.""" + mixin = HeadlineOperationsMixin() + data: dict[str, Any] = { + "Id": 1, + "Name": "Headline", + "DetailsUrl": "http://example.com", + "OriginId": 10, + "Origin": "Meeting", + "Archived": False, + "CreateTime": "2024-01-01", + "CloseTime": None, + # "Owner" missing + } + with pytest.raises(KeyError): + mixin._transform_headline_details(data) + + def test_details_owner_none(self) -> None: + """_transform_headline_details crashes if Owner is None.""" + mixin = HeadlineOperationsMixin() + data: dict[str, Any] = { + "Id": 1, + "Name": "Headline", + "DetailsUrl": "http://example.com", + "OriginId": 10, + "Origin": "Meeting", + "Archived": False, + "CreateTime": "2024-01-01", + "CloseTime": None, + "Owner": None, + } + with pytest.raises(TypeError): + mixin._transform_headline_details(data) + + def test_details_owner_missing_id(self) -> None: + """_transform_headline_details crashes if Owner lacks Id.""" + mixin = HeadlineOperationsMixin() + data: dict[str, Any] = { + "Id": 1, + "Name": "Headline", + "DetailsUrl": "http://example.com", + "OriginId": 10, + "Origin": "Meeting", + "Archived": False, + "CreateTime": "2024-01-01", + "CloseTime": None, + "Owner": {"Name": "John"}, + } + with pytest.raises(KeyError): + mixin._transform_headline_details(data) + + def test_details_owner_missing_name(self) -> None: + """_transform_headline_details crashes if Owner lacks Name.""" + mixin = HeadlineOperationsMixin() + data: dict[str, Any] = { + "Id": 1, + "Name": "Headline", + "DetailsUrl": "http://example.com", + "OriginId": 10, + "Origin": "Meeting", + "Archived": False, + "CreateTime": "2024-01-01", + "CloseTime": None, + "Owner": {"Id": 1}, + } + with pytest.raises(KeyError): + mixin._transform_headline_details(data) + + def test_list_item_owner_none(self) -> None: + """_transform_headline_list crashes if an item's Owner is None.""" + mixin = HeadlineOperationsMixin() + data = [ + { + "Id": 1, + "Name": "Headline", + "OriginId": 10, + "Origin": "Meeting", + "Archived": False, + "CreateTime": "2024-01-01", + "CloseTime": None, + "Owner": None, + } + ] + with pytest.raises(TypeError): + mixin._transform_headline_list(data) + + def test_list_item_missing_owner(self) -> None: + """_transform_headline_list crashes if an item has no Owner key.""" + mixin = HeadlineOperationsMixin() + data = [ + { + "Id": 1, + "Name": "Headline", + "OriginId": 10, + "Origin": "Meeting", + "Archived": False, + "CreateTime": "2024-01-01", + "CloseTime": None, + } + ] + with pytest.raises(KeyError): + mixin._transform_headline_list(data) + + +# =========================================================================== +# 4. HEADLINE CREATE: API returns None for DetailsUrl +# =========================================================================== + + +class TestHeadlineCreateEdgeCases: + """Test headline create with unusual API responses.""" + + def test_create_with_none_details_url(self, mock_http_client: Mock) -> None: + """When API returns DetailsUrl=None, .get('DetailsUrl', '') returns None. + + The HeadlineInfo model has notes_url: str, so None would cause a + validation error if the model enforces the type. + """ + mock_http_client.post.return_value = _make_mock_response( + {"Id": 1, "Name": "Test Headline", "DetailsUrl": None} + ) + headline_ops = HeadlineOperations(mock_http_client) + + with patch( + "bloomy.utils.base_operations.BaseOperations.user_id", + new_callable=PropertyMock, + return_value=123, + ): + # data.get("DetailsUrl", "") returns None (key exists with None value) + # This means notes_url=None, but HeadlineInfo.notes_url is typed as str + result = headline_ops.create(meeting_id=1, title="Test Headline") + + # The .get() returns None because the key exists with a None value, + # not the default "". This is a subtle bug. + # Whether this fails depends on Pydantic's strictness. + assert result.id == 1 + + def test_create_with_missing_details_url_key(self, mock_http_client: Mock) -> None: + """When API omits DetailsUrl entirely, .get('DetailsUrl', '') returns ''.""" + mock_http_client.post.return_value = _make_mock_response( + {"Id": 1, "Name": "Test Headline"} + ) + headline_ops = HeadlineOperations(mock_http_client) + + with patch( + "bloomy.utils.base_operations.BaseOperations.user_id", + new_callable=PropertyMock, + return_value=123, + ): + result = headline_ops.create(meeting_id=1, title="Test Headline") + + assert result.notes_url == "" + + +# =========================================================================== +# 5. ISSUE COMPLETE: unexpected API response +# =========================================================================== + + +class TestIssueCompleteEdgeCases: + """Test issue.complete() edge cases.""" + + def test_complete_calls_details_after_post(self, mock_http_client: Mock) -> None: + """Verify complete() does POST then calls details(). + + If details() fails, the issue was still marked complete server-side + but client gets error. + """ + # POST succeeds + post_resp = Mock() + post_resp.raise_for_status = Mock() + mock_http_client.post.return_value = post_resp + + # GET (for details) fails + from httpx import HTTPStatusError, Request, Response + + get_resp = Mock() + get_resp.raise_for_status.side_effect = HTTPStatusError( + "Not Found", + request=Request("GET", "http://test"), + response=Response(404), + ) + mock_http_client.get.return_value = get_resp + + issue_ops = IssueOperations(mock_http_client) + + with pytest.raises(HTTPStatusError): + issue_ops.complete(issue_id=999) + + # The POST was still called — issue is completed server-side + mock_http_client.post.assert_called_once_with( + "issues/999/complete", json={"complete": True} + ) + + +# =========================================================================== +# 6. ISSUE LIST: transform with empty list +# =========================================================================== + + +class TestIssueListEdgeCases: + """Test issue list transform edge cases.""" + + def test_empty_list_returns_empty(self) -> None: + """An empty API response should return an empty list.""" + mixin = IssueOperationsMixin() + result = mixin._transform_issue_list([]) + assert result == [] + + def test_list_item_missing_details_url(self) -> None: + """_transform_issue_list crashes if an item lacks DetailsUrl.""" + mixin = IssueOperationsMixin() + data = [ + { + "Id": 1, + "Name": "Test", + "CreateTime": "2024-01-01", + "OriginId": 10, + "Origin": "Meeting", + # "DetailsUrl" missing + } + ] + with pytest.raises(KeyError): + mixin._transform_issue_list(data) + + +class TestHeadlineListEdgeCases: + """Test headline list transform edge cases.""" + + def test_empty_list_returns_empty(self) -> None: + """An empty API response should return an empty list.""" + mixin = HeadlineOperationsMixin() + result = mixin._transform_headline_list([]) + assert result == [] diff --git a/tests/test_adversarial_meetings.py b/tests/test_adversarial_meetings.py new file mode 100644 index 0000000..767f97b --- /dev/null +++ b/tests/test_adversarial_meetings.py @@ -0,0 +1,441 @@ +"""Adversarial tests for Meetings and Scorecard operations. + +These tests target known bugs and edge cases in: +- scorecard.py / async_/scorecard.py (truthiness checks) +- meetings.py / async_/meetings.py (getattr on MeetingListItem) +- mixins/meetings_transform.py (metrics transform edge cases) +""" + +from typing import Any +from unittest.mock import AsyncMock, Mock + +import httpx +import pytest + +from bloomy.operations.meetings import MeetingOperations +from bloomy.operations.mixins.meetings_transform import MeetingOperationsMixin +from bloomy.operations.scorecard import ScorecardOperations + +# --------------------------------------------------------------------------- +# Fixtures +# --------------------------------------------------------------------------- + + +@pytest.fixture +def mock_http_client() -> Mock: + """Create a mock HTTP client. + + Returns: + A mock httpx.Client. + + """ + client = Mock(spec=httpx.Client) + client.get = Mock() + client.post = Mock() + client.put = Mock() + client.delete = Mock() + client.close = Mock() + return client + + +@pytest.fixture +def scorecard_ops(mock_http_client: Mock) -> ScorecardOperations: + """Create a ScorecardOperations instance with mocked HTTP client. + + Returns: + A ScorecardOperations instance. + + """ + return ScorecardOperations(mock_http_client) + + +@pytest.fixture +def meeting_ops(mock_http_client: Mock) -> MeetingOperations: + """Create a MeetingOperations instance with mocked HTTP client. + + Returns: + A MeetingOperations instance. + + """ + return MeetingOperations(mock_http_client) + + +@pytest.fixture +def mixin() -> MeetingOperationsMixin: + """Create a plain MeetingOperationsMixin for direct transform testing. + + Returns: + A MeetingOperationsMixin instance. + + """ + return MeetingOperationsMixin() + + +# --------------------------------------------------------------------------- +# BUG 1 - Scorecard.list() truthiness check on user_id / meeting_id +# +# Line 87: `if user_id and meeting_id:` uses truthiness. +# When user_id=0 (falsy int), the ValueError is not raised even though +# both arguments are provided. +# --------------------------------------------------------------------------- + + +class TestScorecardTruthinessBug: + """Scorecard.list() must reject user_id AND meeting_id regardless of value.""" + + def test_user_id_zero_and_meeting_id_should_raise( + self, scorecard_ops: ScorecardOperations + ) -> None: + """user_id=0 with meeting_id provided should raise ValueError.""" + with pytest.raises(ValueError, match="not both"): + scorecard_ops.list(user_id=0, meeting_id=123) + + def test_meeting_id_zero_and_user_id_should_raise( + self, scorecard_ops: ScorecardOperations + ) -> None: + """meeting_id=0 with user_id provided should raise ValueError.""" + with pytest.raises(ValueError, match="not both"): + scorecard_ops.list(user_id=42, meeting_id=0) + + def test_both_zero_should_raise(self, scorecard_ops: ScorecardOperations) -> None: + """Both user_id=0 and meeting_id=0 should still raise ValueError.""" + with pytest.raises(ValueError, match="not both"): + scorecard_ops.list(user_id=0, meeting_id=0) + + def test_both_nonzero_should_raise( + self, scorecard_ops: ScorecardOperations + ) -> None: + """Both user_id and meeting_id non-zero should raise ValueError.""" + with pytest.raises(ValueError, match="not both"): + scorecard_ops.list(user_id=1, meeting_id=2) + + def test_meeting_id_zero_routes_to_meeting_endpoint( + self, scorecard_ops: ScorecardOperations, mock_http_client: Mock + ) -> None: + """meeting_id=0 (without user_id) should still hit the meeting endpoint.""" + mock_response = Mock() + mock_response.json.return_value = {"Scores": []} + mock_http_client.get.return_value = mock_response + + scorecard_ops.list(meeting_id=0) + + mock_http_client.get.assert_called_once_with("scorecard/meeting/0") + + +# --------------------------------------------------------------------------- +# BUG 2 - Meeting.details() uses getattr on MeetingListItem +# +# MeetingListItem only has: id, type, key, name. +# getattr(meeting, "start_date_utc", None) always returns None. +# The result is that MeetingDetails never has dates populated. +# --------------------------------------------------------------------------- + + +class TestMeetingDetailsDirectEndpoint: + """Meeting.details() now uses the direct L10/{id} endpoint.""" + + def test_details_populates_created_date( + self, meeting_ops: MeetingOperations, mock_http_client: Mock + ) -> None: + """details() now reads created_date from the direct endpoint response. + + Previously it used getattr on MeetingListItem which never had + these fields, so they were always None. Now fixed. + """ + direct_resp = Mock() + direct_resp.json.return_value = { + "Id": 1, + "Basics": {"Name": "Standup"}, + "CreateTime": "2024-06-01T10:00:00Z", + "StartDateUtc": None, + "OrganizationId": 42, + } + empty_resp = Mock() + empty_resp.json.return_value = [] + + mock_http_client.get.side_effect = [ + direct_resp, + empty_resp, # attendees + empty_resp, # issues + empty_resp, # todos + empty_resp, # metrics + ] + + result = meeting_ops.details(meeting_id=1) + + assert result.organization_id == 42 + mock_http_client.get.assert_any_call("L10/1") + + def test_details_meeting_id_zero( + self, meeting_ops: MeetingOperations, mock_http_client: Mock + ) -> None: + """meeting_id=0 is handled correctly by the direct endpoint.""" + direct_resp = Mock() + direct_resp.json.return_value = { + "Id": 0, + "Basics": {"Name": "Zero Meeting"}, + "CreateTime": None, + "StartDateUtc": None, + "OrganizationId": None, + } + empty_resp = Mock() + empty_resp.json.return_value = [] + + mock_http_client.get.side_effect = [ + direct_resp, + empty_resp, + empty_resp, + empty_resp, + empty_resp, + ] + + result = meeting_ops.details(meeting_id=0) + assert result.id == 0 + assert result.name == "Zero Meeting" + + +# --------------------------------------------------------------------------- +# BUG 3 - _transform_metrics truthiness on measurable_id +# +# Line 66: `if not measurable_id or not measurable_name:` skips items +# where measurable_id is 0 (a valid but falsy int). +# --------------------------------------------------------------------------- + + +class TestTransformMetricsEdgeCases: + """_transform_metrics should handle falsy-but-valid data.""" + + def test_measurable_id_zero_is_skipped(self, mixin: MeetingOperationsMixin) -> None: + """measurable_id=0 is wrongly skipped by `if not measurable_id`.""" + data = [ + { + "Id": 0, + "Name": "Valid Metric", + "Target": 50, + "Direction": ">", + "Modifiers": "count", + "Owner": {"Id": 1, "Name": "Alice"}, + } + ] + result = mixin._transform_metrics(data) + # Fixed: measurable_id=0 is now accepted (was skipped due to truthiness) + assert len(result) == 1 + + def test_empty_name_is_skipped(self, mixin: MeetingOperationsMixin) -> None: + """Empty string name is falsy and should be skipped.""" + data = [ + { + "Id": 1, + "Name": "", + "Target": 10, + "Owner": {"Id": 1, "Name": "Bob"}, + } + ] + result = mixin._transform_metrics(data) + assert len(result) == 0 + + def test_none_input_returns_empty(self, mixin: MeetingOperationsMixin) -> None: + """None input (not a list) returns empty list.""" + assert mixin._transform_metrics(None) == [] + + def test_string_input_returns_empty(self, mixin: MeetingOperationsMixin) -> None: + """String input (not a list) returns empty list.""" + assert mixin._transform_metrics("not a list") == [] + + def test_dict_input_returns_empty(self, mixin: MeetingOperationsMixin) -> None: + """Dict input (not a list) returns empty list.""" + assert mixin._transform_metrics({"key": "value"}) == [] + + def test_list_with_non_dict_items(self, mixin: MeetingOperationsMixin) -> None: + """Non-dict items in the list are silently skipped.""" + data = [42, "string", None, True] + result = mixin._transform_metrics(data) + assert result == [] + + def test_owner_none_does_not_crash(self, mixin: MeetingOperationsMixin) -> None: + """Owner=None should not crash; falls back to empty dict.""" + data = [ + { + "Id": 10, + "Name": "Metric", + "Target": 100, + "Direction": ">=", + "Modifiers": "pct", + "Owner": None, + } + ] + result = mixin._transform_metrics(data) + assert len(result) == 1 + assert result[0].accountable_user_id == 0 + assert result[0].accountable_user_name == "" + + def test_owner_as_string_does_not_crash( + self, mixin: MeetingOperationsMixin + ) -> None: + """Owner as a string instead of dict should be handled gracefully.""" + data = [ + { + "Id": 10, + "Name": "Metric", + "Target": 100, + "Owner": "not-a-dict", + } + ] + result = mixin._transform_metrics(data) + assert len(result) == 1 + assert result[0].accountable_user_id == 0 + + def test_missing_optional_fields_use_defaults( + self, mixin: MeetingOperationsMixin + ) -> None: + """Items with missing optional fields should use defaults.""" + data = [ + { + "Id": 5, + "Name": "Bare Metric", + } + ] + result = mixin._transform_metrics(data) + assert len(result) == 1 + assert result[0].target == 0.0 + assert result[0].unit == "" + assert result[0].metric_type == "" + assert result[0].accountable_user_id == 0 + + def test_target_as_string(self, mixin: MeetingOperationsMixin) -> None: + """Target as a numeric string should be coerced to float.""" + data = [ + { + "Id": 1, + "Name": "Metric", + "Target": "99.5", + "Owner": {"Id": 1, "Name": "X"}, + } + ] + result = mixin._transform_metrics(data) + assert result[0].target == 99.5 + + +# --------------------------------------------------------------------------- +# BUG 4 - _transform_meeting_issues with None Owner +# +# issue.get("Owner", {}).get("ImageUrl", "") — if Owner is explicitly None +# (not missing), .get returns None and .get("ImageUrl") raises AttributeError. +# --------------------------------------------------------------------------- + + +class TestTransformIssuesEdgeCases: + """_transform_meeting_issues should handle malformed Owner data.""" + + def test_owner_none_handled_gracefully(self, mixin: MeetingOperationsMixin) -> None: + """Owner=None is handled gracefully, falling back to defaults.""" + data: list[dict[str, Any]] = [ + { + "Id": 1, + "Name": "Issue", + "DetailsUrl": "http://x", + "CreateTime": "2024-01-01T00:00:00Z", + "CloseTime": None, + "Owner": None, + "Origin": "Meeting", + } + ] + result = mixin._transform_meeting_issues(data, meeting_id=1) + assert len(result) == 1 + assert result[0].owner_id == 0 + assert result[0].owner_name == "" + assert result[0].owner_image_url == "" + + def test_owner_missing_fields(self, mixin: MeetingOperationsMixin) -> None: + """Owner dict with missing keys uses defaults.""" + data: list[dict[str, Any]] = [ + { + "Id": 1, + "Name": "Issue", + "DetailsUrl": "http://x", + "CreateTime": "2024-01-01T00:00:00Z", + "CloseTime": None, + "Owner": {}, + "Origin": "Meeting", + } + ] + result = mixin._transform_meeting_issues(data, meeting_id=1) + assert len(result) == 1 + assert result[0].owner_id == 0 + assert result[0].owner_name == "" + assert result[0].owner_image_url == "" + + def test_owner_missing_entirely(self, mixin: MeetingOperationsMixin) -> None: + """Missing Owner key falls back to empty dict via .get default.""" + data: list[dict[str, Any]] = [ + { + "Id": 1, + "Name": "Issue", + "DetailsUrl": "http://x", + "CreateTime": "2024-01-01T00:00:00Z", + "CloseTime": None, + "Origin": "Meeting", + } + ] + result = mixin._transform_meeting_issues(data, meeting_id=1) + assert len(result) == 1 + assert result[0].owner_id == 0 + + +# --------------------------------------------------------------------------- +# _transform_attendees edge cases +# --------------------------------------------------------------------------- + + +class TestTransformAttendeesEdgeCases: + """_transform_attendees should handle missing/malformed data.""" + + def test_empty_list(self, mixin: MeetingOperationsMixin) -> None: + """Empty attendee list returns empty result.""" + assert mixin._transform_attendees([]) == [] + + def test_missing_image_url_uses_default( + self, mixin: MeetingOperationsMixin + ) -> None: + """Missing ImageUrl defaults to empty string.""" + data = [{"Id": 1, "Name": "Alice"}] + result = mixin._transform_attendees(data) + assert len(result) == 1 + assert result[0].image_url == "" + + def test_missing_required_field_raises(self, mixin: MeetingOperationsMixin) -> None: + """Missing required field (Name) raises KeyError.""" + data: list[dict[str, Any]] = [{"Id": 1}] + with pytest.raises(KeyError): + mixin._transform_attendees(data) + + +# --------------------------------------------------------------------------- +# Async scorecard truthiness bug (same issue in async variant) +# --------------------------------------------------------------------------- + + +class TestAsyncScorecardTruthinessBug: + """Async scorecard.list() has the same truthiness bug.""" + + @pytest.mark.asyncio + async def test_user_id_zero_and_meeting_id_should_raise(self) -> None: + """Async: user_id=0 with meeting_id should raise ValueError.""" + from bloomy.operations.async_.scorecard import AsyncScorecardOperations + + mock_client = AsyncMock() + ops = AsyncScorecardOperations(mock_client) + + with pytest.raises(ValueError, match="not both"): + await ops.list(user_id=0, meeting_id=123) + + @pytest.mark.asyncio + async def test_both_zero_should_raise(self) -> None: + """Async: both zero should raise ValueError.""" + from bloomy.operations.async_.scorecard import AsyncScorecardOperations + + mock_client = AsyncMock() + ops = AsyncScorecardOperations(mock_client) + + with pytest.raises(ValueError, match="not both"): + await ops.list(user_id=0, meeting_id=0) diff --git a/tests/test_adversarial_todos_goals.py b/tests/test_adversarial_todos_goals.py new file mode 100644 index 0000000..ac3ef9c --- /dev/null +++ b/tests/test_adversarial_todos_goals.py @@ -0,0 +1,667 @@ +"""Adversarial tests for Todos and Goals operations. + +These tests target edge cases and known bugs in the todos and goals +operations, transforms, and bulk processing. +""" + +from __future__ import annotations + +from typing import Any +from unittest.mock import Mock, PropertyMock, patch + +import pytest + +from bloomy.operations.goals import GoalOperations +from bloomy.operations.mixins.goals_transform import GoalOperationsMixin +from bloomy.operations.mixins.todos_transform import TodoOperationsMixin +from bloomy.operations.todos import TodoOperations + +# --------------------------------------------------------------------------- +# Fixtures +# --------------------------------------------------------------------------- + + +@pytest.fixture +def mock_http_client() -> Mock: + """Create a mock HTTP client with default successful responses. + + Returns: + A mock HTTP client. + + """ + client = Mock() + response = Mock() + response.raise_for_status = Mock() + response.json.return_value = {} + client.get = Mock(return_value=response) + client.post = Mock(return_value=response) + client.put = Mock(return_value=response) + client.delete = Mock(return_value=response) + return client + + +@pytest.fixture +def mock_user_id() -> PropertyMock: + """Mock user_id property returning 123. + + Yields: + A PropertyMock that returns user ID 123. + + """ + with patch( + "bloomy.utils.base_operations.BaseOperations.user_id", + new_callable=PropertyMock, + ) as mock: + mock.return_value = 123 + yield mock + + +# --------------------------------------------------------------------------- +# Goal update() — accountable_user always defaults to self.user_id +# --------------------------------------------------------------------------- + + +class TestGoalUpdateOverwritesOwner: + """Goal.update() should only send accountableUserId when explicitly provided. + + Previously, accountableUserId always defaulted to self.user_id, silently + overwriting the goal owner. This has been fixed. + """ + + def test_update_title_only_should_not_overwrite_owner( + self, mock_http_client: Mock, mock_user_id: PropertyMock + ) -> None: + """Updating only the title should NOT send accountableUserId.""" + goal_ops = GoalOperations(mock_http_client) + goal_ops.update(goal_id=1, title="New Title") + + call_args = mock_http_client.put.call_args + payload = call_args.kwargs.get("json") or call_args[1]["json"] + + assert "accountableUserId" not in payload, ( + "accountableUserId should not be in payload when not explicitly provided" + ) + + def test_update_status_only_should_not_overwrite_owner( + self, mock_http_client: Mock, mock_user_id: PropertyMock + ) -> None: + """Updating only the status should NOT send accountableUserId.""" + goal_ops = GoalOperations(mock_http_client) + goal_ops.update(goal_id=1, status="on") + + call_args = mock_http_client.put.call_args + payload = call_args.kwargs.get("json") or call_args[1]["json"] + + assert "accountableUserId" not in payload, ( + "accountableUserId should not be in payload when not explicitly provided" + ) + + def test_update_with_explicit_accountable_user( + self, mock_http_client: Mock, mock_user_id: PropertyMock + ) -> None: + """When accountable_user is explicitly provided, it should be used.""" + goal_ops = GoalOperations(mock_http_client) + goal_ops.update(goal_id=1, title="X", accountable_user=456) + + call_args = mock_http_client.put.call_args + payload = call_args.kwargs.get("json") or call_args[1]["json"] + + assert payload["accountableUserId"] == 456 + + +# --------------------------------------------------------------------------- +# _transform_goal_list — Origins edge cases +# --------------------------------------------------------------------------- + + +class TestGoalTransformOrigins: + """Edge cases in _transform_goal_list around the Origins field.""" + + def _make_goal(self, **overrides: Any) -> dict[str, Any]: + base: dict[str, Any] = { + "Id": 1, + "Owner": {"Id": 10, "Name": "Alice"}, + "Name": "Goal A", + "CreateTime": "2024-01-01T00:00:00Z", + "DueDate": "2024-12-31", + "Complete": False, + "Origins": [{"Id": 100, "Name": "Meeting A"}], + } + base.update(overrides) + return base + + def test_origins_empty_list(self) -> None: + """Empty Origins list should yield meeting_id=None.""" + mixin = GoalOperationsMixin() + goal = self._make_goal(Origins=[]) + result = mixin._transform_goal_list([goal]) + assert result[0].meeting_id is None + assert result[0].meeting_title is None + + def test_origins_none(self) -> None: + """Origins=None should yield meeting_id=None.""" + mixin = GoalOperationsMixin() + goal = self._make_goal(Origins=None) + result = mixin._transform_goal_list([goal]) + assert result[0].meeting_id is None + assert result[0].meeting_title is None + + def test_origins_missing_key(self) -> None: + """No Origins key at all should yield meeting_id=None.""" + mixin = GoalOperationsMixin() + goal = self._make_goal() + del goal["Origins"] + result = mixin._transform_goal_list([goal]) + assert result[0].meeting_id is None + + def test_origins_with_none_element(self) -> None: + """Origins=[None] is truthy but the element is None — should return None.""" + mixin = GoalOperationsMixin() + goal = self._make_goal(Origins=[None]) + + result = mixin._transform_goal_list([goal]) + assert result[0].meeting_id is None + assert result[0].meeting_title is None + + def test_origins_with_empty_dict(self) -> None: + """Origins=[{}] — empty dict is falsy, should return None.""" + mixin = GoalOperationsMixin() + goal = self._make_goal(Origins=[{}]) + + result = mixin._transform_goal_list([goal]) + assert result[0].meeting_id is None + assert result[0].meeting_title is None + + +# --------------------------------------------------------------------------- +# _transform_created_goal — Origins not guarded +# --------------------------------------------------------------------------- + + +class TestCreatedGoalTransformOrigins: + """_transform_created_goal unconditionally accesses Origins[0].""" + + def _make_create_response(self, **overrides: Any) -> dict[str, Any]: + base: dict[str, Any] = { + "Id": 999, + "Owner": {"Id": 10, "Name": "Alice"}, + "Origins": [{"Id": 100, "Name": "Meeting A"}], + "CreateTime": "2024-06-01T10:00:00Z", + "Completion": 0, + } + base.update(overrides) + return base + + def test_created_goal_empty_origins(self) -> None: + """Empty Origins list should return meeting_title=None.""" + mixin = GoalOperationsMixin() + data = self._make_create_response(Origins=[]) + + result = mixin._transform_created_goal(data, "Goal", meeting_id=100, user_id=10) + assert result.meeting_title is None + + def test_created_goal_origins_none(self) -> None: + """Origins=None should return meeting_title=None.""" + mixin = GoalOperationsMixin() + data = self._make_create_response(Origins=None) + + result = mixin._transform_created_goal(data, "Goal", meeting_id=100, user_id=10) + assert result.meeting_title is None + + def test_created_goal_missing_origins_key(self) -> None: + """Missing Origins key should return meeting_title=None.""" + mixin = GoalOperationsMixin() + data = self._make_create_response() + del data["Origins"] + + result = mixin._transform_created_goal(data, "Goal", meeting_id=100, user_id=10) + assert result.meeting_title is None + + def test_created_goal_happy_path(self) -> None: + """Normal case still works.""" + mixin = GoalOperationsMixin() + data = self._make_create_response() + result = mixin._transform_created_goal( + data, "Goal X", meeting_id=100, user_id=10 + ) + + assert result.id == 999 + assert result.title == "Goal X" + assert result.meeting_title == "Meeting A" + assert result.status == "off" + + def test_created_goal_completion_values(self) -> None: + """Test all completion status mappings.""" + mixin = GoalOperationsMixin() + + for completion_val, expected_status in [(0, "off"), (1, "on"), (2, "complete")]: + data = self._make_create_response(Completion=completion_val) + result = mixin._transform_created_goal(data, "G", meeting_id=1, user_id=1) + assert result.status == expected_status, ( + f"Completion={completion_val} should map to '{expected_status}'" + ) + + def test_created_goal_unknown_completion(self) -> None: + """Unknown Completion value should default to 'off'.""" + mixin = GoalOperationsMixin() + data = self._make_create_response(Completion=99) + result = mixin._transform_created_goal(data, "G", meeting_id=1, user_id=1) + assert result.status == "off" + + +# --------------------------------------------------------------------------- +# Todo create() — hardcoded response fields & datetime.now() fallback +# --------------------------------------------------------------------------- + + +class TestTodoCreateEdgeCases: + """Tests for edge cases in todo creation response handling.""" + + def test_create_response_missing_id_raises_keyerror( + self, mock_http_client: Mock, mock_user_id: PropertyMock + ) -> None: + """If API returns response without 'Id', KeyError is raised. + + This documents that the API contract requires the 'Id' field. + """ + response = Mock() + response.raise_for_status = Mock() + response.json.return_value = {"Name": "Todo", "SomeOtherField": 123} + mock_http_client.post.return_value = response + + todo_ops = TodoOperations(mock_http_client) + + with pytest.raises(KeyError, match="Id"): + todo_ops.create(title="Test", meeting_id=1) + + def test_create_response_missing_name_raises_keyerror( + self, mock_http_client: Mock, mock_user_id: PropertyMock + ) -> None: + """BUG: If API returns response without 'Name', KeyError is raised.""" + response = Mock() + response.raise_for_status = Mock() + response.json.return_value = {"Id": 1, "Title": "Todo"} + mock_http_client.post.return_value = response + + todo_ops = TodoOperations(mock_http_client) + + with pytest.raises(KeyError, match="Name"): + todo_ops.create(title="Test", meeting_id=1) + + def test_create_response_minimal_fields( + self, mock_http_client: Mock, mock_user_id: PropertyMock + ) -> None: + """Minimal API response with only Id and Name should succeed.""" + response = Mock() + response.raise_for_status = Mock() + response.json.return_value = {"Id": 42, "Name": "Minimal Todo"} + mock_http_client.post.return_value = response + + todo_ops = TodoOperations(mock_http_client) + result = todo_ops.create(title="Minimal Todo", meeting_id=1) + + assert result.id == 42 + assert result.name == "Minimal Todo" + assert result.meeting_id == 1 + assert result.complete is False + + def test_create_datetime_fallback_is_utc( + self, mock_http_client: Mock, mock_user_id: PropertyMock + ) -> None: + """When CreateTime is missing, the fallback uses datetime.now(UTC).""" + response = Mock() + response.raise_for_status = Mock() + # No CreateTime in the response + response.json.return_value = {"Id": 1, "Name": "Todo"} + mock_http_client.post.return_value = response + + todo_ops = TodoOperations(mock_http_client) + result = todo_ops.create(title="Todo") + + assert result.create_date is not None + assert result.create_date.tzinfo is not None, ( + "datetime fallback should be timezone-aware (UTC)" + ) + + def test_create_user_todo_payload( + self, mock_http_client: Mock, mock_user_id: PropertyMock + ) -> None: + """User todos (no meeting_id) use different payload and endpoint.""" + response = Mock() + response.raise_for_status = Mock() + response.json.return_value = {"Id": 1, "Name": "User Todo"} + mock_http_client.post.return_value = response + + todo_ops = TodoOperations(mock_http_client) + todo_ops.create(title="User Todo", due_date="2024-12-31", notes="Note") + + mock_http_client.post.assert_called_once_with( + "todo/create", + json={ + "title": "User Todo", + "accountableUserId": 123, + "notes": "Note", + "dueDate": "2024-12-31", + }, + ) + + def test_create_meeting_todo_sets_origin_id( + self, mock_http_client: Mock, mock_user_id: PropertyMock + ) -> None: + """Meeting todo should have meeting_id set from the input, not the response.""" + response = Mock() + response.raise_for_status = Mock() + response.json.return_value = {"Id": 1, "Name": "Meeting Todo"} + mock_http_client.post.return_value = response + + todo_ops = TodoOperations(mock_http_client) + result = todo_ops.create(title="Meeting Todo", meeting_id=999) + + assert result.meeting_id == 999 + + +# --------------------------------------------------------------------------- +# TodoOperationsMixin payload builders +# --------------------------------------------------------------------------- + + +class TestTodoPayloadBuilders: + """Test the mixin payload builder methods for edge cases.""" + + def test_meeting_todo_payload_minimal(self) -> None: + """Build meeting todo payload with minimal fields.""" + mixin = TodoOperationsMixin() + payload = mixin._build_meeting_todo_payload("Title", user_id=1) + assert payload == {"Title": "Title", "ForId": 1} + + def test_meeting_todo_payload_full(self) -> None: + """Build meeting todo payload with all fields.""" + mixin = TodoOperationsMixin() + payload = mixin._build_meeting_todo_payload( + "Title", user_id=1, notes="N", due_date="2024-01-01" + ) + assert payload == { + "Title": "Title", + "ForId": 1, + "Notes": "N", + "dueDate": "2024-01-01", + } + + def test_user_todo_payload_minimal(self) -> None: + """Build user todo payload with minimal fields.""" + mixin = TodoOperationsMixin() + payload = mixin._build_user_todo_payload("Title", user_id=1) + assert payload == {"title": "Title", "accountableUserId": 1} + + def test_user_todo_payload_full(self) -> None: + """Build user todo payload with all fields.""" + mixin = TodoOperationsMixin() + payload = mixin._build_user_todo_payload( + "Title", user_id=1, notes="N", due_date="2024-01-01" + ) + assert payload == { + "title": "Title", + "accountableUserId": 1, + "notes": "N", + "dueDate": "2024-01-01", + } + + def test_meeting_payload_inconsistent_casing(self) -> None: + """Meeting todo payload uses mixed casing. + + PascalCase for Title/ForId/Notes but camelCase for dueDate. + This test documents the inconsistency. + """ + mixin = TodoOperationsMixin() + payload = mixin._build_meeting_todo_payload( + "T", user_id=1, notes="N", due_date="2024-01-01" + ) + # PascalCase keys + assert "Title" in payload + assert "ForId" in payload + assert "Notes" in payload + # camelCase key — inconsistent + assert "dueDate" in payload + + +# --------------------------------------------------------------------------- +# Goal _build_goal_update_payload edge cases +# --------------------------------------------------------------------------- + + +class TestGoalUpdatePayload: + """Test _build_goal_update_payload for edge cases.""" + + def test_payload_with_no_optional_fields(self) -> None: + """Payload with only accountable_user.""" + mixin = GoalOperationsMixin() + payload = mixin._build_goal_update_payload(accountable_user=42) + assert payload == {"accountableUserId": 42} + + def test_payload_without_accountable_user(self) -> None: + """Payload without accountable_user should not include it.""" + mixin = GoalOperationsMixin() + payload = mixin._build_goal_update_payload(title="Updated") + assert "accountableUserId" not in payload + assert payload == {"title": "Updated"} + + def test_payload_with_goal_status_enum(self) -> None: + """Test using GoalStatus enum values.""" + from bloomy.models import GoalStatus + + mixin = GoalOperationsMixin() + + for status, expected in [ + (GoalStatus.ON_TRACK, "OnTrack"), + (GoalStatus.AT_RISK, "AtRisk"), + (GoalStatus.COMPLETE, "Complete"), + ]: + payload = mixin._build_goal_update_payload( + accountable_user=1, status=status + ) + assert payload["completion"] == expected + + def test_payload_invalid_status_string(self) -> None: + """Invalid status string should raise ValueError.""" + mixin = GoalOperationsMixin() + with pytest.raises(ValueError, match="Invalid status value"): + mixin._build_goal_update_payload(accountable_user=1, status="invalid") + + def test_payload_status_case_sensitivity(self) -> None: + """Status strings are lowercased, so mixed case should work.""" + mixin = GoalOperationsMixin() + payload = mixin._build_goal_update_payload(accountable_user=1, status="On") + assert payload["completion"] == "OnTrack" + + def test_payload_status_complete_uppercase(self) -> None: + """Uppercase COMPLETE should map to Complete.""" + mixin = GoalOperationsMixin() + payload = mixin._build_goal_update_payload( + accountable_user=1, status="COMPLETE" + ) + assert payload["completion"] == "Complete" + + +# --------------------------------------------------------------------------- +# Bulk operations — mix of valid and invalid data +# --------------------------------------------------------------------------- + + +class TestBulkGoalCreation: + """Test create_many with edge case inputs.""" + + def test_bulk_create_missing_required_field( + self, mock_http_client: Mock, mock_user_id: PropertyMock + ) -> None: + """Items missing 'title' should fail validation.""" + goal_ops = GoalOperations(mock_http_client) + result = goal_ops.create_many([{"meeting_id": 123}]) + + assert len(result.successful) == 0 + assert len(result.failed) == 1 + assert "title" in result.failed[0].error.lower() + + def test_bulk_create_missing_meeting_id( + self, mock_http_client: Mock, mock_user_id: PropertyMock + ) -> None: + """Items missing 'meeting_id' should fail validation.""" + goal_ops = GoalOperations(mock_http_client) + result = goal_ops.create_many([{"title": "Goal"}]) + + assert len(result.successful) == 0 + assert len(result.failed) == 1 + assert "meeting_id" in result.failed[0].error.lower() + + def test_bulk_create_mix_valid_and_invalid( + self, mock_http_client: Mock, mock_user_id: PropertyMock + ) -> None: + """Mix of valid and invalid items — valid ones should succeed.""" + create_response = Mock() + create_response.raise_for_status = Mock() + create_response.json.return_value = { + "Id": 1, + "Owner": {"Id": 123, "Name": "Me"}, + "Origins": [{"Id": 100, "Name": "Meeting"}], + "CreateTime": "2024-01-01T00:00:00Z", + "Completion": 0, + } + mock_http_client.post.return_value = create_response + + goal_ops = GoalOperations(mock_http_client) + result = goal_ops.create_many( + [ + {"title": "Valid Goal", "meeting_id": 100}, + {"meeting_id": 100}, # missing title + {"title": "Another Valid", "meeting_id": 100}, + ] + ) + + assert len(result.successful) == 2 + assert len(result.failed) == 1 + assert result.failed[0].index == 1 + + def test_bulk_create_empty_list( + self, mock_http_client: Mock, mock_user_id: PropertyMock + ) -> None: + """Empty input should return empty results.""" + goal_ops = GoalOperations(mock_http_client) + result = goal_ops.create_many([]) + + assert len(result.successful) == 0 + assert len(result.failed) == 0 + + def test_bulk_create_all_invalid( + self, mock_http_client: Mock, mock_user_id: PropertyMock + ) -> None: + """All invalid items should all be in failed list.""" + goal_ops = GoalOperations(mock_http_client) + result = goal_ops.create_many( + [ + {"not_title": "X"}, + {"meeting_id": None, "title": None}, + ] + ) + + assert len(result.successful) == 0 + assert len(result.failed) == 2 + + +class TestBulkTodoCreation: + """Test todo create_many with edge case inputs.""" + + def test_bulk_todo_missing_title( + self, mock_http_client: Mock, mock_user_id: PropertyMock + ) -> None: + """Items missing 'title' should fail validation.""" + todo_ops = TodoOperations(mock_http_client) + result = todo_ops.create_many([{"meeting_id": 1}]) + + assert len(result.successful) == 0 + assert len(result.failed) == 1 + + def test_bulk_todo_missing_meeting_id( + self, mock_http_client: Mock, mock_user_id: PropertyMock + ) -> None: + """Items missing 'meeting_id' should fail validation.""" + todo_ops = TodoOperations(mock_http_client) + result = todo_ops.create_many([{"title": "Todo"}]) + + assert len(result.successful) == 0 + assert len(result.failed) == 1 + + def test_bulk_todo_api_error_captured( + self, mock_http_client: Mock, mock_user_id: PropertyMock + ) -> None: + """API errors during creation should be captured, not raised.""" + error_response = Mock() + error_response.raise_for_status.side_effect = Exception("API Error 500") + mock_http_client.post.return_value = error_response + + todo_ops = TodoOperations(mock_http_client) + result = todo_ops.create_many( + [ + {"title": "Todo 1", "meeting_id": 1}, + ] + ) + + assert len(result.successful) == 0 + assert len(result.failed) == 1 + assert "API Error 500" in result.failed[0].error + + +# --------------------------------------------------------------------------- +# _transform_archived_goals edge cases +# --------------------------------------------------------------------------- + + +class TestArchivedGoalTransform: + """Edge cases for _transform_archived_goals.""" + + def test_empty_list(self) -> None: + """Empty input should return empty list.""" + mixin = GoalOperationsMixin() + result = mixin._transform_archived_goals([]) + assert result == [] + + def test_missing_complete_field(self) -> None: + """Goal without 'Complete' field should default to 'Incomplete'.""" + mixin = GoalOperationsMixin() + data = [ + { + "Id": 1, + "Name": "Goal", + "CreateTime": "2024-01-01T00:00:00Z", + "DueDate": "2024-12-31", + } + ] + result = mixin._transform_archived_goals(data) + assert result[0].status == "Incomplete" + + def test_complete_true(self) -> None: + """Complete=True should map to 'Complete' status.""" + mixin = GoalOperationsMixin() + data = [ + { + "Id": 1, + "Name": "Goal", + "CreateTime": "2024-01-01T00:00:00Z", + "DueDate": "2024-12-31", + "Complete": True, + } + ] + result = mixin._transform_archived_goals(data) + assert result[0].status == "Complete" + + def test_missing_due_date(self) -> None: + """Goal with DueDate=None.""" + mixin = GoalOperationsMixin() + data = [ + { + "Id": 1, + "Name": "Goal", + "CreateTime": "2024-01-01T00:00:00Z", + "DueDate": None, + } + ] + result = mixin._transform_archived_goals(data) + assert result[0].due_date is None diff --git a/tests/test_adversarial_users.py b/tests/test_adversarial_users.py new file mode 100644 index 0000000..f9ae51b --- /dev/null +++ b/tests/test_adversarial_users.py @@ -0,0 +1,875 @@ +"""Adversarial tests for Users operations, Configuration, and Client. + +These tests deliberately feed malformed, missing, and unexpected data to find +bugs, crashes, and unhandled edge cases in the user-related code paths. +""" + +from __future__ import annotations + +import os +from pathlib import Path +from typing import Any +from unittest.mock import AsyncMock, Mock, patch + +import httpx +import pytest +import yaml + +from bloomy.async_client import AsyncClient +from bloomy.client import Client +from bloomy.configuration import Configuration +from bloomy.exceptions import ConfigurationError +from bloomy.models import ( + DirectReport, + Position, + UserDetails, + UserListItem, + UserSearchResult, +) +from bloomy.operations.async_.users import AsyncUserOperations +from bloomy.operations.mixins.users_transform import UserOperationsMixin +from bloomy.operations.users import UserOperations + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _make_mock_http_client(json_return: Any = None, side_effect: Any = None) -> Mock: + """Build a mock httpx.Client whose .get() returns controllable JSON. + + Returns: + A mock httpx.Client. + + """ + client = Mock(spec=httpx.Client) + resp = Mock(spec=httpx.Response) + resp.raise_for_status = Mock() + if side_effect is not None: + resp.json.side_effect = side_effect + client.get.return_value = resp + elif json_return is not None: + resp.json.return_value = json_return + client.get.return_value = resp + else: + client.get.return_value = resp + return client + + +def _make_async_mock_http_client(json_return: Any = None) -> AsyncMock: + """Build a mock httpx.AsyncClient whose .get() returns controllable JSON. + + Returns: + A mock httpx.AsyncClient. + + """ + client = AsyncMock(spec=httpx.AsyncClient) + resp = Mock(spec=httpx.Response) + resp.raise_for_status = Mock() + resp.json.return_value = json_return + client.get.return_value = resp + return client + + +# =========================================================================== +# 1. UserOperationsMixin — transform methods with malformed data +# =========================================================================== + + +class TestUserTransformMalformedData: + """Attack the mixin transform methods with bad API responses.""" + + def setup_method(self) -> None: + """Create a fresh mixin for each test.""" + self.mixin = UserOperationsMixin() + + # -- _transform_user_details ----------------------------------------- + + def test_user_details_missing_id_key(self) -> None: + """API returns a dict without 'Id'.""" + with pytest.raises(KeyError, match="Id"): + self.mixin._transform_user_details({"Name": "x", "ImageUrl": "u"}) + + def test_user_details_missing_name_key(self) -> None: + """API returns a dict without 'Name'.""" + with pytest.raises(KeyError, match="Name"): + self.mixin._transform_user_details({"Id": 1, "ImageUrl": "u"}) + + def test_user_details_missing_image_url_key(self) -> None: + """API returns a dict without 'ImageUrl'.""" + with pytest.raises(KeyError, match="ImageUrl"): + self.mixin._transform_user_details({"Id": 1, "Name": "x"}) + + def test_user_details_completely_empty_dict(self) -> None: + """API returns an empty dict.""" + with pytest.raises(KeyError): + self.mixin._transform_user_details({}) + + def test_user_details_none_values(self) -> None: + """API returns None for all values — model rejects them.""" + # Pydantic should reject None for int field 'id' + with pytest.raises((TypeError, ValueError)): + self.mixin._transform_user_details( + {"Id": None, "Name": None, "ImageUrl": None} + ) + + def test_user_details_wrong_type_id_string(self) -> None: + """API returns string ID — Pydantic may coerce it.""" + result = self.mixin._transform_user_details( + {"Id": "999", "Name": "x", "ImageUrl": "u"} + ) + # Pydantic coerces "999" → 999 + assert result.id == 999 + + def test_user_details_wrong_type_id_non_numeric_string(self) -> None: + """API returns non-numeric string ID.""" + with pytest.raises((TypeError, ValueError)): + self.mixin._transform_user_details( + {"Id": "not-a-number", "Name": "x", "ImageUrl": "u"} + ) + + def test_user_details_extra_fields_ignored(self) -> None: + """Extra fields in the API response should not break parsing.""" + result = self.mixin._transform_user_details( + {"Id": 1, "Name": "x", "ImageUrl": "u", "Unexpected": True, "Foo": [1, 2]} + ) + assert result.id == 1 + + def test_user_details_with_empty_direct_reports(self) -> None: + """Passing empty list for direct_reports.""" + result = self.mixin._transform_user_details( + {"Id": 1, "Name": "x", "ImageUrl": "u"}, direct_reports=[] + ) + assert result.direct_reports == [] + + def test_user_details_with_empty_positions(self) -> None: + """Passing empty list for positions.""" + result = self.mixin._transform_user_details( + {"Id": 1, "Name": "x", "ImageUrl": "u"}, positions=[] + ) + assert result.positions == [] + + # -- _transform_direct_reports --------------------------------------- + + def test_direct_reports_missing_keys(self) -> None: + """Direct report entry missing required keys.""" + with pytest.raises(KeyError): + self.mixin._transform_direct_reports([{"Id": 1}]) + + def test_direct_reports_empty_list(self) -> None: + """Empty list should return empty list.""" + assert self.mixin._transform_direct_reports([]) == [] + + def test_direct_reports_none_input(self) -> None: + """None instead of list should raise TypeError.""" + with pytest.raises(TypeError): + self.mixin._transform_direct_reports(None) # type: ignore[arg-type] + + def test_direct_reports_entry_with_none_id(self) -> None: + """Report with None ID.""" + with pytest.raises((TypeError, ValueError)): + self.mixin._transform_direct_reports( + [{"Id": None, "Name": "x", "ImageUrl": "u"}] + ) + + # -- _transform_positions -------------------------------------------- + + def test_positions_missing_group_key(self) -> None: + """Position entry missing 'Group'.""" + with pytest.raises(KeyError, match="Group"): + self.mixin._transform_positions([{"Id": 1}]) + + def test_positions_missing_position_inside_group(self) -> None: + """Group dict missing 'Position' key.""" + with pytest.raises(KeyError, match="Position"): + self.mixin._transform_positions([{"Group": {}}]) + + def test_positions_missing_nested_name(self) -> None: + """Position inside Group missing 'Name'.""" + with pytest.raises(KeyError, match="Name"): + self.mixin._transform_positions([{"Group": {"Position": {"Id": 1}}}]) + + def test_positions_missing_nested_id(self) -> None: + """Position inside Group missing 'Id'.""" + with pytest.raises(KeyError, match="Id"): + self.mixin._transform_positions([{"Group": {"Position": {"Name": "x"}}}]) + + def test_positions_group_is_none(self) -> None: + """Group value is None instead of dict.""" + with pytest.raises(TypeError): + self.mixin._transform_positions([{"Group": None}]) + + def test_positions_position_is_none(self) -> None: + """Position value inside Group is None.""" + with pytest.raises(TypeError): + self.mixin._transform_positions([{"Group": {"Position": None}}]) + + def test_positions_empty_list(self) -> None: + """Empty list should return empty list.""" + assert self.mixin._transform_positions([]) == [] + + def test_positions_none_input(self) -> None: + """None instead of list should raise TypeError.""" + with pytest.raises(TypeError): + self.mixin._transform_positions(None) # type: ignore[arg-type] + + # -- _transform_search_results --------------------------------------- + + def test_search_results_missing_keys(self) -> None: + """Search result entry missing keys.""" + with pytest.raises(KeyError): + self.mixin._transform_search_results([{"Id": 1}]) + + def test_search_results_none_email(self) -> None: + """Search result with None email — model field is non-optional str.""" + # Pydantic will try to coerce None to str, which should fail + with pytest.raises((TypeError, ValueError)): + self.mixin._transform_search_results( + [ + { + "Id": 1, + "Name": "x", + "Description": "d", + "Email": None, + "OrganizationId": 1, + "ImageUrl": "u", + } + ] + ) + + def test_search_results_none_organization_id(self) -> None: + """Search result with None OrganizationId — model field is int.""" + with pytest.raises((TypeError, ValueError)): + self.mixin._transform_search_results( + [ + { + "Id": 1, + "Name": "x", + "Description": "d", + "Email": "e@e.com", + "OrganizationId": None, + "ImageUrl": "u", + } + ] + ) + + def test_search_results_empty_list(self) -> None: + """Empty list should return empty list.""" + assert self.mixin._transform_search_results([]) == [] + + def test_search_results_empty_strings(self) -> None: + """All string fields are empty strings — should be accepted.""" + result = self.mixin._transform_search_results( + [ + { + "Id": 1, + "Name": "", + "Description": "", + "Email": "", + "OrganizationId": 1, + "ImageUrl": "", + } + ] + ) + assert len(result) == 1 + # str_strip_whitespace is on; empty string stays empty + assert result[0].name == "" + + # -- _transform_user_list -------------------------------------------- + + def test_user_list_missing_result_type(self) -> None: + """User entry missing 'ResultType' key.""" + with pytest.raises(KeyError, match="ResultType"): + self.mixin._transform_user_list( + [ + { + "Id": 1, + "Name": "x", + "Email": "e", + "Description": "d", + "ImageUrl": "u", + } + ], + include_placeholders=False, + ) + + def test_user_list_missing_image_url_in_model(self) -> None: + """User with ResultType=User but missing ImageUrl crashes.""" + with pytest.raises(KeyError, match="ImageUrl"): + self.mixin._transform_user_list( + [ + { + "Id": 1, + "Name": "x", + "Email": "e", + "Description": "d", + "ResultType": "User", + } + ], + include_placeholders=False, + ) + + def test_user_list_missing_email_after_filter(self) -> None: + """User passes filter but is missing 'Email' key for model construction.""" + with pytest.raises(KeyError, match="Email"): + self.mixin._transform_user_list( + [ + { + "Id": 1, + "Name": "x", + "Description": "d", + "ImageUrl": "u", + "ResultType": "User", + } + ], + include_placeholders=True, + ) + + def test_user_list_none_image_url(self) -> None: + """ImageUrl is None — accepted since image_url is now optional. + + Previously this was a bug where None ImageUrl crashed model + construction. Fix: UserListItem.image_url is now str | None, + and filtering is email-based. + """ + result = self.mixin._transform_user_list( + [ + { + "Id": 1, + "Name": "x", + "Email": "e", + "Description": "d", + "ImageUrl": None, + "ResultType": "User", + } + ], + include_placeholders=False, + ) + assert len(result) == 1 + assert result[0].image_url is None + + def test_user_list_empty_list(self) -> None: + """Empty list should return empty list.""" + assert self.mixin._transform_user_list([], include_placeholders=False) == [] + + def test_user_list_only_non_user_types(self) -> None: + """All entries are non-User ResultType.""" + result = self.mixin._transform_user_list( + [ + {"Id": 1, "Name": "g", "ResultType": "Group", "ImageUrl": "u"}, + {"Id": 2, "Name": "t", "ResultType": "Team", "ImageUrl": "u"}, + ], + include_placeholders=True, + ) + assert result == [] + + def test_user_list_placeholder_filtering(self) -> None: + """Verify placeholders are filtered correctly based on flag.""" + users = [ + { + "Id": 1, + "Name": "Real", + "Email": "r@e.com", + "Description": "d", + "ImageUrl": "https://img.com/a.jpg", + "ResultType": "User", + }, + { + "Id": 2, + "Name": "Placeholder", + "Email": "", + "Description": "", + "ImageUrl": "/i/userplaceholder", + "ResultType": "User", + }, + ] + without = self.mixin._transform_user_list(users, include_placeholders=False) + assert len(without) == 1 + assert without[0].id == 1 + + with_ph = self.mixin._transform_user_list(users, include_placeholders=True) + assert len(with_ph) == 2 + + +# =========================================================================== +# 2. UserOperations (sync) — end-to-end with mock HTTP +# =========================================================================== + + +class TestUserOperationsAdversarial: + """Test sync UserOperations with adversarial API responses.""" + + def test_details_api_returns_empty_dict(self, mock_user_id: Mock) -> None: + """API returns {} for user details.""" + client = _make_mock_http_client(json_return={}) + ops = UserOperations(client) + with pytest.raises(KeyError): + ops.details() + + def test_details_api_returns_list_instead_of_dict(self, mock_user_id: Mock) -> None: + """API returns a list instead of a dict.""" + client = _make_mock_http_client(json_return=[{"Id": 1}]) + ops = UserOperations(client) + with pytest.raises((KeyError, TypeError)): + ops.details() + + def test_details_api_returns_none_json(self, mock_user_id: Mock) -> None: + """API returns null JSON body.""" + client = _make_mock_http_client(json_return=None) + ops = UserOperations(client) + with pytest.raises(TypeError): + ops.details() + + def test_details_with_user_id_zero(self) -> None: + """user_id=0 is technically a valid int but suspicious.""" + client = _make_mock_http_client( + json_return={"Id": 0, "Name": "Zero", "ImageUrl": "u"} + ) + ops = UserOperations(client) + result = ops.details(user_id=0) + assert result.id == 0 + client.get.assert_called_once_with("users/0") + + def test_details_with_negative_user_id(self) -> None: + """Negative user_id — SDK should pass it through to API.""" + client = _make_mock_http_client( + json_return={"Id": -1, "Name": "Neg", "ImageUrl": "u"} + ) + ops = UserOperations(client) + result = ops.details(user_id=-1) + assert result.id == -1 + client.get.assert_called_once_with("users/-1") + + def test_search_empty_string(self) -> None: + """Search with empty string.""" + client = _make_mock_http_client(json_return=[]) + ops = UserOperations(client) + result = ops.search("") + assert result == [] + client.get.assert_called_once_with("search/user", params={"term": ""}) + + def test_direct_reports_api_returns_non_list(self) -> None: + """API returns a dict instead of list for direct reports.""" + client = _make_mock_http_client(json_return={"error": "something"}) + ops = UserOperations(client) + with pytest.raises(TypeError): + ops.direct_reports(user_id=1) + + def test_positions_api_returns_string(self) -> None: + """API returns a raw string instead of JSON list.""" + client = _make_mock_http_client(json_return="not a list") + ops = UserOperations(client) + with pytest.raises(TypeError): + ops.positions(user_id=1) + + def test_list_api_returns_non_list(self) -> None: + """API returns a dict instead of list for user list.""" + client = _make_mock_http_client(json_return={"users": []}) + ops = UserOperations(client) + with pytest.raises(TypeError): + ops.list() + + def test_user_id_property_lazy_loads(self) -> None: + """user_id property fetches from API when not set.""" + client = _make_mock_http_client(json_return={"Id": 42}) + ops = UserOperations(client) + uid = ops.user_id + assert uid == 42 + client.get.assert_called_once_with("users/mine") + + def test_user_id_api_returns_no_id_key(self) -> None: + """users/mine returns dict without 'Id'.""" + client = _make_mock_http_client(json_return={"Name": "x"}) + ops = UserOperations(client) + with pytest.raises(KeyError, match="Id"): + _ = ops.user_id + + +# =========================================================================== +# 3. AsyncUserOperations — adversarial async tests +# =========================================================================== + + +class TestAsyncUserOperationsAdversarial: + """Test async user operations with adversarial inputs.""" + + @pytest.mark.asyncio + async def test_details_api_returns_empty_dict(self) -> None: + """Async details with empty dict response.""" + client = _make_async_mock_http_client(json_return={}) + ops = AsyncUserOperations(client) + ops._user_id = 1 + with pytest.raises(KeyError): + await ops.details(user_id=1) + + @pytest.mark.asyncio + async def test_get_user_id_lazy_loads(self) -> None: + """get_user_id fetches from API when not cached.""" + client = _make_async_mock_http_client(json_return={"Id": 77}) + ops = AsyncUserOperations(client) + uid = await ops.get_user_id() + assert uid == 77 + + @pytest.mark.asyncio + async def test_user_id_property_raises_without_fetch(self) -> None: + """user_id property raises RuntimeError if not fetched yet.""" + client = _make_async_mock_http_client(json_return={}) + ops = AsyncUserOperations(client) + with pytest.raises(RuntimeError, match="User ID not set"): + _ = ops.user_id + + @pytest.mark.asyncio + async def test_direct_reports_none_response(self) -> None: + """API returns None for direct reports.""" + client = _make_async_mock_http_client(json_return=None) + ops = AsyncUserOperations(client) + with pytest.raises(TypeError): + await ops.direct_reports(user_id=1) + + @pytest.mark.asyncio + async def test_search_results_malformed(self) -> None: + """API returns list with entries missing keys.""" + client = _make_async_mock_http_client(json_return=[{"Id": 1}]) + ops = AsyncUserOperations(client) + with pytest.raises(KeyError): + await ops.search("test") + + @pytest.mark.asyncio + async def test_positions_deeply_nested_missing(self) -> None: + """API returns positions with missing nested structure.""" + client = _make_async_mock_http_client(json_return=[{"Group": {}}]) + ops = AsyncUserOperations(client) + with pytest.raises(KeyError): + await ops.positions(user_id=1) + + +# =========================================================================== +# 4. Configuration — adversarial inputs +# =========================================================================== + + +class TestConfigurationAdversarial: + """Attack Configuration with missing/corrupt data.""" + + def test_no_api_key_anywhere(self) -> None: + """No key provided, no env var, no config file.""" + with ( + patch.dict(os.environ, {}, clear=True), + patch.object(Configuration, "_load_api_key", return_value=None), + ): + config = Configuration() + assert config.api_key is None + + def test_empty_string_api_key_direct(self) -> None: + """Empty string passed as api_key — truthy check may pass or fail.""" + # Empty string is falsy, so it falls through to env var + # This verifies the `api_key or ...` chain behavior + with ( + patch.dict(os.environ, {}, clear=True), + patch.object(Configuration, "_load_api_key", return_value=None), + ): + config2 = Configuration(api_key="") + # "" or None => None + assert config2.api_key is None + + def test_whitespace_only_api_key_stripped(self) -> None: + """Whitespace-only API key is stripped before the or-chain. + + Fix verified: Configuration strips the key first, producing "". + The empty string is falsy and falls through the or-chain. + With no env var or config file, api_key ends up None. + """ + with ( + patch.dict(os.environ, {}, clear=True), + patch.object(Configuration, "_load_api_key", return_value=None), + ): + config = Configuration(api_key=" ") + # " ".strip() -> "" which is falsy, falls through or-chain + assert config.api_key is None + + def test_env_var_takes_precedence_over_file(self) -> None: + """BG_API_KEY env var should override config file.""" + with ( + patch.dict(os.environ, {"BG_API_KEY": "env-key"}), + patch.object(Configuration, "_load_api_key", return_value="file-key"), + ): + config = Configuration() + assert config.api_key == "env-key" + + def test_explicit_key_takes_precedence_over_all(self) -> None: + """Explicit api_key param should override everything.""" + with patch.dict(os.environ, {"BG_API_KEY": "env-key"}): + config = Configuration(api_key="explicit-key") + assert config.api_key == "explicit-key" + + def test_load_api_key_corrupt_yaml(self, tmp_path: Path) -> None: + """Config file contains invalid YAML.""" + config_file = tmp_path / "config.yaml" + config_file.write_text(": : : [invalid yaml {{{") + + config = Configuration.__new__(Configuration) + with patch.object( + type(config), + "_config_file", + new_callable=lambda: property(lambda _: config_file), + ): + result = config._load_api_key() + # Should return None gracefully due to the except clause + assert result is None + + def test_load_api_key_yaml_without_api_key_field(self, tmp_path: Path) -> None: + """Config file is valid YAML but missing 'api_key' field.""" + config_file = tmp_path / "config.yaml" + config_file.write_text(yaml.dump({"version": 1, "other_field": "value"})) + + config = Configuration.__new__(Configuration) + with patch.object( + type(config), + "_config_file", + new_callable=lambda: property(lambda _: config_file), + ): + result = config._load_api_key() + assert result is None + + def test_load_api_key_yaml_with_none_value(self, tmp_path: Path) -> None: + """Config file has api_key: null.""" + config_file = tmp_path / "config.yaml" + config_file.write_text(yaml.dump({"version": 1, "api_key": None})) + + config = Configuration.__new__(Configuration) + with patch.object( + type(config), + "_config_file", + new_callable=lambda: property(lambda _: config_file), + ): + result = config._load_api_key() + assert result is None + + def test_load_api_key_empty_file(self, tmp_path: Path) -> None: + """Config file exists but is empty.""" + config_file = tmp_path / "config.yaml" + config_file.write_text("") + + config = Configuration.__new__(Configuration) + with patch.object( + type(config), + "_config_file", + new_callable=lambda: property(lambda _: config_file), + ): + result = config._load_api_key() + # yaml.safe_load("") returns None; .get() raises AttributeError + # This is caught by the except clause and returns None + assert result is None + + def test_load_api_key_binary_content(self, tmp_path: Path) -> None: + """Config file contains binary garbage.""" + config_file = tmp_path / "config.yaml" + config_file.write_bytes(b"\x00\x01\x02\xff\xfe\xfd") + + config = Configuration.__new__(Configuration) + with patch.object( + type(config), + "_config_file", + new_callable=lambda: property(lambda _: config_file), + ): + result = config._load_api_key() + # Should be caught by broad except + assert result is None + + def test_store_api_key_when_none(self) -> None: + """Storing None api_key raises ConfigurationError.""" + config = Configuration.__new__(Configuration) + config.api_key = None + with pytest.raises(ConfigurationError, match="API key is None"): + config._store_api_key() + + def test_configure_api_key_overwrites_existing(self) -> None: + """configure_api_key overwrites even if api_key already set.""" + config = Configuration(api_key="old-key") + with patch.object(config, "_fetch_api_key", return_value="new-key"): + config.configure_api_key("user", "pass") + assert config.api_key == "new-key" + + +# =========================================================================== +# 5. Client — adversarial initialization +# =========================================================================== + + +class TestClientAdversarial: + """Attack Client with bad configuration.""" + + def test_client_no_api_key_raises(self) -> None: + """Client without any API key raises ConfigurationError.""" + with ( + patch.dict(os.environ, {}, clear=True), + patch.object(Configuration, "_load_api_key", return_value=None), + pytest.raises(ConfigurationError), + ): + Client() + + def test_client_empty_string_api_key_raises(self) -> None: + """Client with empty string API key should raise.""" + with ( + patch.dict(os.environ, {}, clear=True), + patch.object(Configuration, "_load_api_key", return_value=None), + pytest.raises(ConfigurationError), + ): + Client(api_key="") + + def test_client_whitespace_api_key_now_raises(self) -> None: + """Client with whitespace-only API key now raises. + + Fix verified: Configuration strips whitespace, so " " becomes + "" which is falsy, triggering the ConfigurationError. + """ + with ( + patch.dict(os.environ, {}, clear=True), + patch.object(Configuration, "_load_api_key", return_value=None), + pytest.raises(ConfigurationError), + ): + Client(api_key=" ") + + def test_client_context_manager(self) -> None: + """Context manager closes client properly.""" + with patch("bloomy.client.httpx.Client") as mock_cls: + mock_http = Mock() + mock_cls.return_value = mock_http + with Client(api_key="test-key") as c: + assert c is not None + mock_http.close.assert_called_once() + + def test_client_double_close(self) -> None: + """Calling close() twice should not raise.""" + with patch("bloomy.client.httpx.Client") as mock_cls: + mock_http = Mock() + mock_cls.return_value = mock_http + c = Client(api_key="test-key") + c.close() + c.close() # Should not raise + + def test_client_operations_initialized(self) -> None: + """All operation classes should be initialized.""" + with patch("bloomy.client.httpx.Client"): + c = Client(api_key="test-key") + assert c.user is not None + assert c.todo is not None + assert c.meeting is not None + assert c.goal is not None + assert c.scorecard is not None + assert c.issue is not None + assert c.headline is not None + + +# =========================================================================== +# 6. AsyncClient — adversarial initialization +# =========================================================================== + + +class TestAsyncClientAdversarial: + """Attack AsyncClient with bad configuration.""" + + def test_async_client_no_api_key_raises(self) -> None: + """AsyncClient without any API key raises ConfigurationError.""" + with ( + patch.dict(os.environ, {}, clear=True), + patch.object(Configuration, "_load_api_key", return_value=None), + pytest.raises(ConfigurationError), + ): + AsyncClient() + + def test_async_client_empty_string_raises(self) -> None: + """AsyncClient with empty string API key should raise.""" + with ( + patch.dict(os.environ, {}, clear=True), + patch.object(Configuration, "_load_api_key", return_value=None), + pytest.raises(ConfigurationError), + ): + AsyncClient(api_key="") + + def test_async_client_whitespace_key_now_raises(self) -> None: + """AsyncClient with whitespace key now raises ConfigurationError. + + Fix verified: Configuration strips whitespace, so " " becomes + "" which is falsy, triggering the ConfigurationError. + """ + with ( + patch.dict(os.environ, {}, clear=True), + patch.object(Configuration, "_load_api_key", return_value=None), + pytest.raises(ConfigurationError), + ): + AsyncClient(api_key=" ") + + +# =========================================================================== +# 7. Pydantic model edge cases — user-related models +# =========================================================================== + + +class TestUserModelsEdgeCases: + """Test Pydantic models with edge case inputs.""" + + def test_user_details_negative_id(self) -> None: + """Negative ID accepted by the model.""" + u = UserDetails(id=-1, name="x", image_url="u") + assert u.id == -1 + + def test_user_details_very_large_id(self) -> None: + """Very large ID.""" + u = UserDetails(id=2**63, name="x", image_url="u") + assert u.id == 2**63 + + def test_user_details_whitespace_name_stripped(self) -> None: + """Whitespace should be stripped due to str_strip_whitespace.""" + u = UserDetails(id=1, name=" John ", image_url="u") + assert u.name == "John" + + def test_direct_report_empty_name(self) -> None: + """Empty name accepted.""" + d = DirectReport(id=1, name="", image_url="u") + assert d.name == "" + + def test_position_none_name(self) -> None: + """Position name is optional.""" + p = Position(id=1, name=None) + assert p.name is None + + def test_user_search_result_all_empty_strings(self) -> None: + """All string fields empty.""" + u = UserSearchResult( + id=1, name="", description="", email="", organization_id=1, image_url="" + ) + assert u.email == "" + + def test_user_list_item_none_position_accepted(self) -> None: + """UserListItem.position is now optional (str | None) — None is accepted. + + Fix verified: position and image_url are now optional to handle API + responses where Description or ImageUrl may be null. + """ + item = UserListItem(id=1, name="x", email="e", position=None, image_url="u") + assert item.position is None + + def test_user_list_item_none_email_rejected(self) -> None: + """UserListItem.email is non-optional str — None should be rejected.""" + with pytest.raises((TypeError, ValueError)): + UserListItem(id=1, name="x", email=None, position="p", image_url="u") # type: ignore[arg-type] + + def test_user_search_result_float_id_coerced(self) -> None: + """Float ID should be coerced to int by Pydantic.""" + u = UserSearchResult( + id=1.0, + name="x", + description="d", + email="e", # type: ignore[arg-type] + organization_id=1, + image_url="u", + ) + assert u.id == 1 + assert isinstance(u.id, int) + + def test_user_details_assignment_validation(self) -> None: + """validate_assignment is on — invalid assignment should raise.""" + u = UserDetails(id=1, name="x", image_url="u") + with pytest.raises((TypeError, ValueError)): + u.id = "not-a-number" # type: ignore[assignment] diff --git a/tests/test_async_goals.py b/tests/test_async_goals.py index 6b5abfd..ff82ef8 100644 --- a/tests/test_async_goals.py +++ b/tests/test_async_goals.py @@ -202,7 +202,6 @@ async def test_update( mock_async_client.put.assert_called_once_with( "rocks/123", json={ - "accountableUserId": 1, "title": "Updated Goal", "completion": "OnTrack", }, diff --git a/tests/test_async_meetings.py b/tests/test_async_meetings.py index 43b6580..9c12952 100644 --- a/tests/test_async_meetings.py +++ b/tests/test_async_meetings.py @@ -332,12 +332,30 @@ async def test_get_many_all_successful( self, async_client: AsyncClient, mock_async_client: AsyncMock ) -> None: """Test bulk retrieval where all meetings are retrieved successfully.""" - # Mock meeting list response for details method - mock_meetings_list = [ - {"Id": 100, "Type": "L10", "Key": "L10-100", "Name": "Weekly Standup"}, - {"Id": 101, "Type": "L10", "Key": "L10-101", "Name": "Sprint Planning"}, - {"Id": 102, "Type": "L10", "Key": "L10-102", "Name": "Retrospective"}, - ] + # Mock direct L10/{id} responses and sub-resource responses + direct_responses = { + 100: { + "Id": 100, + "Basics": {"Name": "Weekly Standup"}, + "CreateTime": None, + "StartDateUtc": None, + "OrganizationId": None, + }, + 101: { + "Id": 101, + "Basics": {"Name": "Sprint Planning"}, + "CreateTime": None, + "StartDateUtc": None, + "OrganizationId": None, + }, + 102: { + "Id": 102, + "Basics": {"Name": "Retrospective"}, + "CreateTime": None, + "StartDateUtc": None, + "OrganizationId": None, + }, + } # Mock attendees responses attendees_responses = { @@ -373,17 +391,17 @@ async def test_get_many_all_successful( def get_side_effect(url, **_kwargs): mock_response = MagicMock() - if url.endswith("/list"): - mock_response.json.return_value = mock_meetings_list - elif "/attendees" in url: - # Extract meeting ID from URL + if "/attendees" in url: meeting_id = int(url.split("/")[1]) mock_response.json.return_value = attendees_responses.get( meeting_id, [] ) - else: - # For issues, todos, metrics + elif "/issues" in url or "/todos" in url or "/measurables" in url: mock_response.json.return_value = [] + else: + # Direct L10/{id} endpoint + meeting_id = int(url.split("/")[1]) + mock_response.json.return_value = direct_responses.get(meeting_id, {}) mock_response.raise_for_status = MagicMock() return mock_response @@ -409,7 +427,7 @@ def get_side_effect(url, **_kwargs): assert len(result.successful[2].attendees) == 1 # Verify API calls - 5 calls per meeting - # (list, attendees, issues, todos, metrics) + # (direct, attendees, issues, todos, metrics) assert mock_async_client.get.call_count == 15 @pytest.mark.asyncio @@ -417,17 +435,29 @@ async def test_get_many_partial_failure( self, async_client: AsyncClient, mock_async_client: AsyncMock ) -> None: """Test bulk retrieval where some meetings fail.""" - # Mock meeting list response - contains meeting 200 but not 999 or 500 - mock_meetings_list = [ - {"Id": 200, "Type": "L10", "Key": "L10-200", "Name": "Success Meeting"}, - ] + from httpx import HTTPStatusError, Request, Response # Create side effect function that returns appropriate response based on URL def get_side_effect(url, **_kwargs): mock_response = MagicMock() - if url.endswith("/list"): - mock_response.json.return_value = mock_meetings_list + # Direct endpoint for meeting 200 + if url == "L10/200": + mock_response.json.return_value = { + "Id": 200, + "Basics": {"Name": "Success Meeting"}, + "CreateTime": None, + "StartDateUtc": None, + "OrganizationId": None, + } + mock_response.raise_for_status = MagicMock() + elif url in ("L10/999", "L10/500"): + # Simulate 404 for non-existent meetings + mock_response.raise_for_status.side_effect = HTTPStatusError( + "Not Found", + request=Request("GET", url), + response=Response(404), + ) elif "/200/attendees" in url: mock_response.json.return_value = [ { @@ -436,11 +466,11 @@ def get_side_effect(url, **_kwargs): "ImageUrl": "https://example.com/img1.jpg", } ] + mock_response.raise_for_status = MagicMock() else: - # For issues, todos, metrics mock_response.json.return_value = [] + mock_response.raise_for_status = MagicMock() - mock_response.raise_for_status = MagicMock() return mock_response mock_async_client.get.side_effect = get_side_effect @@ -458,11 +488,9 @@ def get_side_effect(url, **_kwargs): # Check failed items assert result.failed[0].index == 1 assert result.failed[0].input_data["meeting_id"] == 999 - assert "not found" in result.failed[0].error.lower() assert result.failed[1].index == 2 assert result.failed[1].input_data["meeting_id"] == 500 - assert "not found" in result.failed[1].error.lower() @pytest.mark.asyncio async def test_get_many_empty_list( @@ -507,19 +535,7 @@ async def delayed_get(*args, **_kwargs): # Extract the URL from args (first positional argument) url = args[0] if args else "" - if url.endswith("/list"): - # Return list of all meetings - mock_response.json.return_value = [ - { - "Id": i + 400, - "Type": "L10", - "Key": f"L10-{i + 400}", - "Name": f"Meeting {i}", - } - for i in range(5) - ] - elif "/attendees" in url: - # Return attendees for any meeting + if "/attendees" in url: mock_response.json.return_value = [ { "Id": 456, @@ -527,9 +543,18 @@ async def delayed_get(*args, **_kwargs): "ImageUrl": "https://example.com/img.jpg", } ] - else: - # For issues, todos, metrics + elif "/issues" in url or "/todos" in url or "/measurables" in url: mock_response.json.return_value = [] + else: + # Direct L10/{id} endpoint + meeting_id = int(url.split("/")[1]) + mock_response.json.return_value = { + "Id": meeting_id, + "Basics": {"Name": f"Meeting {meeting_id - 400}"}, + "CreateTime": None, + "StartDateUtc": None, + "OrganizationId": None, + } mock_response.raise_for_status = MagicMock() return mock_response @@ -551,7 +576,7 @@ async def delayed_get(*args, **_kwargs): # Verify concurrent execution # With max_concurrent=3 and 5 meetings: - # Each meeting makes 5 API calls (list, attendees, issues, todos, metrics) + # Each meeting makes 5 API calls (direct, attendees, issues, todos, metrics) # Each call has 0.1s delay, so each meeting takes ~0.5s # With concurrency of 3, we should see significant speedup vs sequential # Sequential would take 5 * 0.5 = 2.5s diff --git a/tests/test_async_meetings_extra.py b/tests/test_async_meetings_extra.py index 4b912a9..d142bb5 100644 --- a/tests/test_async_meetings_extra.py +++ b/tests/test_async_meetings_extra.py @@ -6,7 +6,6 @@ import pytest_asyncio from bloomy import AsyncClient -from bloomy.exceptions import APIError from bloomy.models import Issue, Todo @@ -192,27 +191,18 @@ async def test_todos_include_closed( async def test_details_meeting_not_found( self, async_client: AsyncClient, mock_async_client: AsyncMock ) -> None: - """Test details when meeting is not found.""" - # Mock user response - mock_user_response = MagicMock() - mock_user_response.json.return_value = {"Id": 456} - mock_user_response.raise_for_status = MagicMock() - - # Mock empty meetings list - mock_meetings_response = MagicMock() - mock_meetings_response.json.return_value = [] - mock_meetings_response.raise_for_status = MagicMock() - - def get_side_effect(url: str) -> MagicMock: - if url == "users/mine": - return mock_user_response - elif url == "L10/456/list": - return mock_meetings_response - else: - raise ValueError(f"Unexpected URL: {url}") - - mock_async_client.get.side_effect = get_side_effect - - # Call the method and expect error - with pytest.raises(APIError, match="Meeting with ID 999 not found"): + """Test details when meeting is not found (404 from direct endpoint).""" + from httpx import HTTPStatusError, Request, Response + + # Mock the direct L10/{id} endpoint to return 404 + mock_response = MagicMock() + mock_response.raise_for_status.side_effect = HTTPStatusError( + "Not Found", + request=Request("GET", "L10/999"), + response=Response(404), + ) + mock_async_client.get.return_value = mock_response + + # Call the method and expect HTTP error + with pytest.raises(HTTPStatusError): await async_client.meeting.details(999) diff --git a/tests/test_bulk_operations.py b/tests/test_bulk_operations.py index d31935a..22d229c 100644 --- a/tests/test_bulk_operations.py +++ b/tests/test_bulk_operations.py @@ -336,48 +336,35 @@ def test_get_many_all_success( self, mock_http_client: Mock, mock_user_id: Mock ) -> None: """Test retrieving multiple meetings when all succeed.""" - # Mock list response for finding meetings - list_response = Mock() - list_response.json.return_value = [ - {"Id": 456, "Type": "NameId", "Key": "NameId_456", "Name": "Meeting 1"}, - {"Id": 457, "Type": "NameId", "Key": "NameId_457", "Name": "Meeting 2"}, - {"Id": 458, "Type": "NameId", "Key": "NameId_458", "Name": "Meeting 3"}, - ] - # Mock responses for each meeting's details - attendees_response = Mock() - attendees_response.json.return_value = [ - {"Id": 123, "Name": "John Doe", "ImageUrl": "https://example.com/john.jpg"} - ] + # Mock direct endpoint responses and sub-resource responses + def get_side_effect(url, **_kwargs): + mock_response = Mock() + if "/attendees" in url: + mock_response.json.return_value = [ + { + "Id": 123, + "Name": "John Doe", + "ImageUrl": "https://example.com/john.jpg", + } + ] + elif "/issues" in url or "/todos" in url or "/measurables" in url: + mock_response.json.return_value = [] + else: + # Direct L10/{id} endpoint + meeting_id = int(url.split("/")[1]) + names = {456: "Meeting 1", 457: "Meeting 2", 458: "Meeting 3"} + mock_response.json.return_value = { + "Id": meeting_id, + "Basics": {"Name": names[meeting_id]}, + "CreateTime": None, + "StartDateUtc": None, + "OrganizationId": None, + } + mock_response.raise_for_status = Mock() + return mock_response - issues_response = Mock() - issues_response.json.return_value = [] - - todos_response = Mock() - todos_response.json.return_value = [] - - metrics_response = Mock() - metrics_response.json.return_value = [] - - # Set up the side effect for all calls - # Pattern: list, attendees, issues, todos, metrics (repeated for each meeting) - mock_http_client.get.side_effect = [ - list_response, # For finding meeting 456 - attendees_response, - issues_response, - todos_response, - metrics_response, - list_response, # For finding meeting 457 - attendees_response, - issues_response, - todos_response, - metrics_response, - list_response, # For finding meeting 458 - attendees_response, - issues_response, - todos_response, - metrics_response, - ] + mock_http_client.get.side_effect = get_side_effect meeting_ops = MeetingOperations(mock_http_client) @@ -402,42 +389,36 @@ def test_get_many_partial_failure( self, mock_http_client: Mock, mock_user_id: Mock ) -> None: """Test retrieving multiple meetings with some failures.""" - # Mock list response for finding meetings - list_response = Mock() - list_response.json.return_value = [ - {"Id": 456, "Type": "NameId", "Key": "NameId_456", "Name": "Meeting 1"}, - ] - empty_list_response = Mock() - empty_list_response.json.return_value = [] # Meeting not found - - # Mock responses for meeting details - attendees_response = Mock() - attendees_response.json.return_value = [] - - issues_response = Mock() - issues_response.json.return_value = [] - - todos_response = Mock() - todos_response.json.return_value = [] - - metrics_response = Mock() - metrics_response.json.return_value = [] - - # Set up the side effect - mock_http_client.get.side_effect = [ - list_response, # First meeting found - attendees_response, - issues_response, - todos_response, - metrics_response, - empty_list_response, # Second meeting not found - list_response, # Third meeting found - attendees_response, - issues_response, - todos_response, - metrics_response, - ] + # Mock direct endpoint that raises for meeting 999 + def get_side_effect(url, **_kwargs): + mock_response = Mock() + if url == "L10/999": + mock_response.raise_for_status.side_effect = HTTPStatusError( + "Not Found", + request=Request("GET", "L10/999"), + response=Response(404), + ) + return mock_response + elif ( + "/attendees" in url + or "/issues" in url + or "/todos" in url + or "/measurables" in url + ): + mock_response.json.return_value = [] + else: + mock_response.json.return_value = { + "Id": 456, + "Basics": {"Name": "Meeting 1"}, + "CreateTime": None, + "StartDateUtc": None, + "OrganizationId": None, + } + mock_response.raise_for_status = Mock() + return mock_response + + mock_http_client.get.side_effect = get_side_effect meeting_ops = MeetingOperations(mock_http_client) @@ -449,7 +430,6 @@ def test_get_many_partial_failure( # Check failure details assert result.failed[0].index == 1 assert result.failed[0].input_data == {"meeting_id": 999} - assert "Meeting with ID 999 not found" in result.failed[0].error def test_get_many_empty_list( self, mock_http_client: Mock, mock_user_id: Mock @@ -467,33 +447,33 @@ def test_get_many_network_error( self, mock_http_client: Mock, mock_user_id: Mock ) -> None: """Test retrieving meetings with network errors.""" - # First meeting succeeds - list_response = Mock() - list_response.json.return_value = [ - {"Id": 456, "Type": "NameId", "Key": "NameId_456", "Name": "Meeting 1"}, - ] + call_count = 0 - attendees_response = Mock() - attendees_response.json.return_value = [] - - issues_response = Mock() - issues_response.json.return_value = [] - - todos_response = Mock() - todos_response.json.return_value = [] - - metrics_response = Mock() - metrics_response.json.return_value = [] + def get_side_effect(_url, **_kwargs): + nonlocal call_count + call_count += 1 + mock_response = Mock() - # Set up side effect with network error on second meeting - mock_http_client.get.side_effect = [ - list_response, # First meeting list - attendees_response, - issues_response, - todos_response, - metrics_response, - Exception("Network error"), # Network error on second meeting list - ] + # First 5 calls are for meeting 456 (direct + 4 sub-resources) + if call_count <= 5: + if call_count == 1: + # Direct L10/{id} endpoint + mock_response.json.return_value = { + "Id": 456, + "Basics": {"Name": "Meeting 1"}, + "CreateTime": None, + "StartDateUtc": None, + "OrganizationId": None, + } + else: + mock_response.json.return_value = [] + mock_response.raise_for_status = Mock() + return mock_response + else: + # Network error on second meeting's direct endpoint + raise Exception("Network error") + + mock_http_client.get.side_effect = get_side_effect meeting_ops = MeetingOperations(mock_http_client) @@ -509,38 +489,28 @@ def test_get_many_duplicate_ids( self, mock_http_client: Mock, mock_user_id: Mock ) -> None: """Test retrieving meetings with duplicate IDs.""" - # Mock list response - list_response = Mock() - list_response.json.return_value = [ - {"Id": 456, "Type": "NameId", "Key": "NameId_456", "Name": "Meeting 1"}, - ] - # Mock responses for meeting details - attendees_response = Mock() - attendees_response.json.return_value = [] - - issues_response = Mock() - issues_response.json.return_value = [] - - todos_response = Mock() - todos_response.json.return_value = [] - - metrics_response = Mock() - metrics_response.json.return_value = [] - - # Set up the side effect (need responses for two calls to same meeting) - mock_http_client.get.side_effect = [ - list_response, # First call for meeting 456 - attendees_response, - issues_response, - todos_response, - metrics_response, - list_response, # Second call for meeting 456 - attendees_response, - issues_response, - todos_response, - metrics_response, - ] + def get_side_effect(url, **_kwargs): + mock_response = Mock() + if ( + "/attendees" in url + or "/issues" in url + or "/todos" in url + or "/measurables" in url + ): + mock_response.json.return_value = [] + else: + mock_response.json.return_value = { + "Id": 456, + "Basics": {"Name": "Meeting 1"}, + "CreateTime": None, + "StartDateUtc": None, + "OrganizationId": None, + } + mock_response.raise_for_status = Mock() + return mock_response + + mock_http_client.get.side_effect = get_side_effect meeting_ops = MeetingOperations(mock_http_client) diff --git a/tests/test_goals.py b/tests/test_goals.py index 4e6d731..dd741cf 100644 --- a/tests/test_goals.py +++ b/tests/test_goals.py @@ -122,7 +122,6 @@ def test_update_goal(self, mock_http_client: Mock, mock_user_id: Mock) -> None: mock_http_client.put.assert_called_once_with( "rocks/101", json={ - "accountableUserId": 123, "title": "Updated Goal", "completion": "Complete", }, diff --git a/tests/test_integration_goals.py b/tests/test_integration_goals.py new file mode 100644 index 0000000..18ef877 --- /dev/null +++ b/tests/test_integration_goals.py @@ -0,0 +1,292 @@ +"""Integration tests for goal operations against the real Bloom Growth API.""" + +from __future__ import annotations + +import pytest +import pytest_asyncio + +from bloomy import AsyncClient, Client +from bloomy.models import ( + ArchivedGoalInfo, + CreatedGoalInfo, + GoalInfo, + GoalListResponse, + GoalStatus, +) + +MEETING_ID = 324926 + + +@pytest.fixture(scope="module") +def client() -> Client: + """Create a real client for integration tests. + + Yields: + A Bloomy client instance. + + """ + c = Client() + yield c + c.close() + + +@pytest_asyncio.fixture +async def async_client() -> AsyncClient: + """Create a real async client for integration tests. + + Yields: + An async Bloomy client instance. + + """ + c = AsyncClient() + yield c + await c.close() + + +# ── Sync Tests ────────────────────────────────────────────────────────── + + +@pytest.mark.integration +class TestGoalListSync: + """Test listing goals (sync).""" + + def test_list_active_goals(self, client: Client) -> None: + """List active goals for the current user.""" + goals = client.goal.list() + assert isinstance(goals, list) + for goal in goals: + assert isinstance(goal, GoalInfo) + + def test_list_with_archived(self, client: Client) -> None: + """List goals including archived ones returns GoalListResponse.""" + result = client.goal.list(archived=True) + assert isinstance(result, GoalListResponse) + assert isinstance(result.active, list) + assert isinstance(result.archived, list) + for g in result.active: + assert isinstance(g, GoalInfo) + for g in result.archived: + assert isinstance(g, ArchivedGoalInfo) + + +@pytest.mark.integration +class TestGoalCRUDSync: + """Test full CRUD lifecycle for goals (sync).""" + + def test_create_and_delete_goal(self, client: Client) -> None: + """Create a goal, verify it, then delete.""" + goal = client.goal.create(title="Integration test goal", meeting_id=MEETING_ID) + try: + assert isinstance(goal, CreatedGoalInfo) + assert goal.title == "Integration test goal" + assert goal.meeting_id == MEETING_ID + assert goal.id > 0 + finally: + client.goal.delete(goal.id) + + def test_update_goal_title(self, client: Client) -> None: + """Update a goal title.""" + goal = client.goal.create(title="Goal original title", meeting_id=MEETING_ID) + try: + client.goal.update(goal.id, title="Goal updated title") + + # Verify via list + goals = client.goal.list() + assert isinstance(goals, list) + updated = next((g for g in goals if g.id == goal.id), None) + assert updated is not None + assert updated.title == "Goal updated title" + finally: + client.goal.delete(goal.id) + + def test_update_goal_status_enum(self, client: Client) -> None: + """Update a goal status using GoalStatus enum.""" + goal = client.goal.create(title="Goal status enum test", meeting_id=MEETING_ID) + try: + # Set to on-track + client.goal.update(goal.id, status=GoalStatus.ON_TRACK) + # Set to at-risk + client.goal.update(goal.id, status=GoalStatus.AT_RISK) + # Set to complete + client.goal.update(goal.id, status=GoalStatus.COMPLETE) + finally: + client.goal.delete(goal.id) + + def test_update_goal_status_string(self, client: Client) -> None: + """Update a goal status using string values.""" + goal = client.goal.create( + title="Goal status string test", meeting_id=MEETING_ID + ) + try: + client.goal.update(goal.id, status="on") + client.goal.update(goal.id, status="off") + client.goal.update(goal.id, status="complete") + finally: + client.goal.delete(goal.id) + + def test_update_goal_invalid_status(self, client: Client) -> None: + """Invalid status string should raise ValueError.""" + goal = client.goal.create( + title="Goal invalid status test", meeting_id=MEETING_ID + ) + try: + with pytest.raises(ValueError, match="Invalid status value"): + client.goal.update(goal.id, status="invalid") + finally: + client.goal.delete(goal.id) + + def test_archive_and_restore_goal(self, client: Client) -> None: + """Archive a goal, verify it's archived, then restore it.""" + goal = client.goal.create(title="Goal archive test", meeting_id=MEETING_ID) + try: + # Archive + client.goal.archive(goal.id) + + # Verify it appears in archived list + result = client.goal.list(archived=True) + assert isinstance(result, GoalListResponse) + archived_ids = [g.id for g in result.archived] + assert goal.id in archived_ids + + # Restore + client.goal.restore(goal.id) + + # Verify it's back in active list + goals = client.goal.list() + assert isinstance(goals, list) + active_ids = [g.id for g in goals] + assert goal.id in active_ids + finally: + client.goal.delete(goal.id) + + def test_delete_goal(self, client: Client) -> None: + """Delete a goal and verify it's gone.""" + goal = client.goal.create(title="Goal delete test", meeting_id=MEETING_ID) + client.goal.delete(goal.id) + + # Verify gone from active list + goals = client.goal.list() + assert isinstance(goals, list) + assert all(g.id != goal.id for g in goals) + + +@pytest.mark.integration +class TestGoalUpdateOwnerBug: + """Test for the known bug: goal.update() always overwrites accountable_user. + + When calling goal.update(goal_id, title="new title") without passing + accountable_user, the current implementation defaults accountable_user + to self.user_id. This means the owner is silently overwritten even if + the caller only intended to update the title. + """ + + def test_update_title_should_not_change_owner(self, client: Client) -> None: + """Updating only the title should preserve the original owner. + + This test exposes the bug in goals.py line 158-159 where + `accountable_user` defaults to `self.user_id` when None. + """ + # Create a goal (owned by current user) + goal = client.goal.create( + title="Owner preservation test", meeting_id=MEETING_ID + ) + try: + # Get the original owner + goals = client.goal.list() + assert isinstance(goals, list) + original = next(g for g in goals if g.id == goal.id) + original_user_id = original.user_id + + # Update only the title — should NOT change owner + client.goal.update(goal.id, title="Owner preservation updated") + + # Verify the owner is still the same + goals = client.goal.list() + assert isinstance(goals, list) + updated = next(g for g in goals if g.id == goal.id) + assert updated.title == "Owner preservation updated" + assert updated.user_id == original_user_id, ( + f"Bug: owner changed from {original_user_id} to {updated.user_id} " + f"when only updating title. goal.update() should not default " + f"accountable_user to self.user_id when it's not provided." + ) + finally: + client.goal.delete(goal.id) + + +# ── Async Tests ───────────────────────────────────────────────────────── + + +@pytest.mark.integration +class TestGoalListAsync: + """Test listing goals (async).""" + + @pytest.mark.asyncio + async def test_list_active_goals(self, async_client: AsyncClient) -> None: + """List active goals (async).""" + goals = await async_client.goal.list() + assert isinstance(goals, list) + for goal in goals: + assert isinstance(goal, GoalInfo) + + @pytest.mark.asyncio + async def test_list_with_archived(self, async_client: AsyncClient) -> None: + """List goals including archived (async).""" + result = await async_client.goal.list(archived=True) + assert isinstance(result, GoalListResponse) + + +@pytest.mark.integration +class TestGoalCRUDAsync: + """Test full CRUD lifecycle for goals (async).""" + + @pytest.mark.asyncio + async def test_full_lifecycle(self, async_client: AsyncClient) -> None: + """Create, update, archive, restore, delete a goal (async).""" + # Create + goal = await async_client.goal.create( + title="Async integration test goal", meeting_id=MEETING_ID + ) + try: + assert isinstance(goal, CreatedGoalInfo) + assert goal.title == "Async integration test goal" + + # Update title + await async_client.goal.update(goal.id, title="Async updated goal") + + # Update status + await async_client.goal.update(goal.id, status=GoalStatus.ON_TRACK) + + # Archive + await async_client.goal.archive(goal.id) + + # Restore + await async_client.goal.restore(goal.id) + finally: + # Delete + await async_client.goal.delete(goal.id) + + @pytest.mark.asyncio + async def test_update_title_should_not_change_owner( + self, async_client: AsyncClient + ) -> None: + """Async version: updating title should not change owner.""" + goal = await async_client.goal.create( + title="Async owner preservation test", meeting_id=MEETING_ID + ) + try: + goals = await async_client.goal.list() + assert isinstance(goals, list) + original = next(g for g in goals if g.id == goal.id) + original_user_id = original.user_id + + await async_client.goal.update(goal.id, title="Async owner updated") + + goals = await async_client.goal.list() + assert isinstance(goals, list) + updated = next(g for g in goals if g.id == goal.id) + assert updated.user_id == original_user_id, ( + f"Bug: owner changed from {original_user_id} to {updated.user_id}" + ) + finally: + await async_client.goal.delete(goal.id) diff --git a/tests/test_integration_headlines.py b/tests/test_integration_headlines.py new file mode 100644 index 0000000..cea14fc --- /dev/null +++ b/tests/test_integration_headlines.py @@ -0,0 +1,328 @@ +"""Integration tests for Headline operations against the real Bloom Growth API.""" + +from __future__ import annotations + +import os +import uuid + +import pytest +import pytest_asyncio + +from bloomy import AsyncClient, Client +from bloomy.models import HeadlineDetails, HeadlineInfo, HeadlineListItem + +MEETING_ID = 324926 + +pytestmark = pytest.mark.integration + + +@pytest.fixture(scope="module") +def client() -> Client: + """Create a real Bloomy client for integration tests. + + Yields: + A configured Client instance. + + """ + api_key = os.environ.get("BG_API_KEY") + if not api_key: + pytest.skip("BG_API_KEY not set") + c = Client(api_key=api_key) + yield c + c.close() + + +@pytest_asyncio.fixture +async def async_client() -> AsyncClient: + """Create a real async Bloomy client for integration tests. + + Yields: + A configured AsyncClient instance. + + """ + api_key = os.environ.get("BG_API_KEY") + if not api_key: + pytest.skip("BG_API_KEY not set") + c = AsyncClient(api_key=api_key) + yield c + await c.close() + + +class TestHeadlineLifecycleSync: + """Full CRUD lifecycle tests for sync headline operations.""" + + def test_create_headline(self, client: Client) -> None: + """Test creating a headline via the real API.""" + tag = uuid.uuid4().hex[:8] + title = f"Integration test headline {tag}" + result = client.headline.create(meeting_id=MEETING_ID, title=title) + + assert isinstance(result, HeadlineInfo) + assert result.id > 0 + assert result.title == title + assert result.owner_details.id > 0 + + # Cleanup + client.headline.delete(result.id) + + def test_create_headline_with_notes(self, client: Client) -> None: + """Test creating a headline with notes.""" + tag = uuid.uuid4().hex[:8] + title = f"Integration test headline notes {tag}" + result = client.headline.create( + meeting_id=MEETING_ID, + title=title, + notes="Some integration test notes", + ) + + assert isinstance(result, HeadlineInfo) + assert result.title == title + + # Cleanup + client.headline.delete(result.id) + + def test_headline_details(self, client: Client) -> None: + """Test retrieving headline details.""" + tag = uuid.uuid4().hex[:8] + created = client.headline.create( + meeting_id=MEETING_ID, title=f"Detail test headline {tag}" + ) + + details = client.headline.details(created.id) + + assert isinstance(details, HeadlineDetails) + assert details.id == created.id + assert details.title == created.title + assert details.meeting_details.id == MEETING_ID + + # Cleanup + client.headline.delete(created.id) + + def test_headline_update(self, client: Client) -> None: + """Test updating a headline.""" + tag = uuid.uuid4().hex[:8] + created = client.headline.create( + meeting_id=MEETING_ID, title=f"Update test headline {tag}" + ) + + updated_title = f"Updated headline {tag}" + # update returns None + client.headline.update(created.id, title=updated_title) + + # Verify the update by fetching details + details = client.headline.details(created.id) + assert details.title == updated_title + + # Cleanup + client.headline.delete(created.id) + + def test_headline_delete(self, client: Client) -> None: + """Test deleting a headline.""" + tag = uuid.uuid4().hex[:8] + created = client.headline.create( + meeting_id=MEETING_ID, title=f"Delete test headline {tag}" + ) + + # Delete should not raise + client.headline.delete(created.id) + + def test_list_user_headlines(self, client: Client) -> None: + """Test listing headlines for the current user.""" + result = client.headline.list() + + assert isinstance(result, list) + for item in result: + assert isinstance(item, HeadlineListItem) + + def test_list_meeting_headlines(self, client: Client) -> None: + """Test listing headlines for a meeting.""" + result = client.headline.list(meeting_id=MEETING_ID) + + assert isinstance(result, list) + for item in result: + assert isinstance(item, HeadlineListItem) + + def test_list_specific_user_headlines(self, client: Client) -> None: + """Test listing headlines for a specific user by ID.""" + user_id = client.user.details().id + result = client.headline.list(user_id=user_id) + + assert isinstance(result, list) + for item in result: + assert isinstance(item, HeadlineListItem) + + def test_list_both_params_raises(self, client: Client) -> None: + """Test that providing both user_id and meeting_id raises ValueError.""" + with pytest.raises(ValueError, match="Please provide either"): + client.headline.list(user_id=123, meeting_id=456) + + def test_list_truthiness_bug_zero_user_id(self, client: Client) -> None: + """Test that user_id=0 with meeting_id raises ValueError. + + BUG: `if user_id and meeting_id` fails when user_id=0 because + 0 is falsy. Should use `is not None` checks instead. + """ + with pytest.raises(ValueError, match="Please provide either"): + client.headline.list(user_id=0, meeting_id=MEETING_ID) + + def test_list_truthiness_bug_zero_meeting_id(self, client: Client) -> None: + """Test that meeting_id=0 with user_id raises ValueError. + + BUG: `if user_id and meeting_id` fails when meeting_id=0 because + 0 is falsy. Should use `is not None` checks instead. + """ + with pytest.raises(ValueError, match="Please provide either"): + client.headline.list(user_id=123, meeting_id=0) + + def test_full_lifecycle(self, client: Client) -> None: + """Test complete headline lifecycle: create -> details -> update -> delete.""" + tag = uuid.uuid4().hex[:8] + + # Create + created = client.headline.create( + meeting_id=MEETING_ID, + title=f"Lifecycle headline {tag}", + notes="Lifecycle test notes", + ) + assert created.id > 0 + + # Details + details = client.headline.details(created.id) + assert details.id == created.id + assert details.archived is False + + # Update + new_title = f"Updated lifecycle headline {tag}" + client.headline.update(created.id, title=new_title) + updated_details = client.headline.details(created.id) + assert updated_details.title == new_title + + # Delete + client.headline.delete(created.id) + + +class TestHeadlineLifecycleAsync: + """Full CRUD lifecycle tests for async headline operations.""" + + @pytest.mark.asyncio + async def test_create_headline(self, async_client: AsyncClient) -> None: + """Test creating a headline via the async API.""" + tag = uuid.uuid4().hex[:8] + title = f"Async integration test headline {tag}" + result = await async_client.headline.create(meeting_id=MEETING_ID, title=title) + + assert isinstance(result, HeadlineInfo) + assert result.id > 0 + assert result.title == title + + # Cleanup + await async_client.headline.delete(result.id) + + @pytest.mark.asyncio + async def test_headline_details(self, async_client: AsyncClient) -> None: + """Test retrieving headline details via async API.""" + tag = uuid.uuid4().hex[:8] + created = await async_client.headline.create( + meeting_id=MEETING_ID, title=f"Async detail test {tag}" + ) + + details = await async_client.headline.details(created.id) + + assert isinstance(details, HeadlineDetails) + assert details.id == created.id + + # Cleanup + await async_client.headline.delete(created.id) + + @pytest.mark.asyncio + async def test_headline_update(self, async_client: AsyncClient) -> None: + """Test updating a headline via async API.""" + tag = uuid.uuid4().hex[:8] + created = await async_client.headline.create( + meeting_id=MEETING_ID, title=f"Async update test {tag}" + ) + + updated_title = f"Async updated {tag}" + await async_client.headline.update(created.id, title=updated_title) + + details = await async_client.headline.details(created.id) + assert details.title == updated_title + + # Cleanup + await async_client.headline.delete(created.id) + + @pytest.mark.asyncio + async def test_headline_delete(self, async_client: AsyncClient) -> None: + """Test deleting a headline via async API.""" + tag = uuid.uuid4().hex[:8] + created = await async_client.headline.create( + meeting_id=MEETING_ID, title=f"Async delete test {tag}" + ) + + await async_client.headline.delete(created.id) + + @pytest.mark.asyncio + async def test_list_user_headlines(self, async_client: AsyncClient) -> None: + """Test listing headlines for the current user via async API.""" + result = await async_client.headline.list() + + assert isinstance(result, list) + for item in result: + assert isinstance(item, HeadlineListItem) + + @pytest.mark.asyncio + async def test_list_meeting_headlines(self, async_client: AsyncClient) -> None: + """Test listing headlines for a meeting via async API.""" + result = await async_client.headline.list(meeting_id=MEETING_ID) + + assert isinstance(result, list) + for item in result: + assert isinstance(item, HeadlineListItem) + + @pytest.mark.asyncio + async def test_list_both_params_raises(self, async_client: AsyncClient) -> None: + """Test that providing both params raises ValueError.""" + with pytest.raises(ValueError, match="Please provide either"): + await async_client.headline.list(user_id=123, meeting_id=456) + + @pytest.mark.asyncio + async def test_list_truthiness_bug_zero_user_id( + self, async_client: AsyncClient + ) -> None: + """Test truthiness bug: user_id=0 with meeting_id should raise ValueError.""" + with pytest.raises(ValueError, match="Please provide either"): + await async_client.headline.list(user_id=0, meeting_id=MEETING_ID) + + @pytest.mark.asyncio + async def test_list_truthiness_bug_zero_meeting_id( + self, async_client: AsyncClient + ) -> None: + """Test truthiness bug: meeting_id=0 with user_id should raise ValueError.""" + with pytest.raises(ValueError, match="Please provide either"): + await async_client.headline.list(user_id=123, meeting_id=0) + + @pytest.mark.asyncio + async def test_full_lifecycle(self, async_client: AsyncClient) -> None: + """Test complete async headline lifecycle.""" + tag = uuid.uuid4().hex[:8] + + # Create + created = await async_client.headline.create( + meeting_id=MEETING_ID, + title=f"Async lifecycle {tag}", + notes="Async lifecycle notes", + ) + assert created.id > 0 + + # Details + details = await async_client.headline.details(created.id) + assert details.id == created.id + + # Update + new_title = f"Async updated lifecycle {tag}" + await async_client.headline.update(created.id, title=new_title) + updated_details = await async_client.headline.details(created.id) + assert updated_details.title == new_title + + # Delete + await async_client.headline.delete(created.id) diff --git a/tests/test_integration_issues.py b/tests/test_integration_issues.py new file mode 100644 index 0000000..4e484a6 --- /dev/null +++ b/tests/test_integration_issues.py @@ -0,0 +1,329 @@ +"""Integration tests for Issue operations against the real Bloom Growth API.""" + +from __future__ import annotations + +import os +import uuid + +import pytest +import pytest_asyncio + +from bloomy import AsyncClient, Client +from bloomy.models import CreatedIssue, IssueDetails, IssueListItem + +MEETING_ID = 324926 + +pytestmark = pytest.mark.integration + + +@pytest.fixture(scope="module") +def client() -> Client: + """Create a real Bloomy client for integration tests. + + Yields: + A configured Client instance. + + """ + api_key = os.environ.get("BG_API_KEY") + if not api_key: + pytest.skip("BG_API_KEY not set") + c = Client(api_key=api_key) + yield c + c.close() + + +@pytest_asyncio.fixture +async def async_client() -> AsyncClient: + """Create a real async Bloomy client for integration tests. + + Yields: + A configured AsyncClient instance. + + """ + api_key = os.environ.get("BG_API_KEY") + if not api_key: + pytest.skip("BG_API_KEY not set") + c = AsyncClient(api_key=api_key) + yield c + await c.close() + + +class TestIssueLifecycleSync: + """Full CRUD lifecycle tests for sync issue operations.""" + + def test_create_issue(self, client: Client) -> None: + """Test creating an issue via the real API.""" + tag = uuid.uuid4().hex[:8] + title = f"Integration test issue {tag}" + result = client.issue.create(meeting_id=MEETING_ID, title=title) + + assert isinstance(result, CreatedIssue) + assert result.id > 0 + assert result.title == title + assert result.meeting_id == MEETING_ID + + # Cleanup + client.issue.complete(result.id) + + def test_create_issue_with_notes(self, client: Client) -> None: + """Test creating an issue with notes.""" + tag = uuid.uuid4().hex[:8] + title = f"Integration test issue notes {tag}" + result = client.issue.create( + meeting_id=MEETING_ID, + title=title, + notes="Some integration test notes", + ) + + assert isinstance(result, CreatedIssue) + assert result.title == title + + # Cleanup + client.issue.complete(result.id) + + def test_issue_details(self, client: Client) -> None: + """Test retrieving issue details.""" + tag = uuid.uuid4().hex[:8] + created = client.issue.create( + meeting_id=MEETING_ID, title=f"Detail test issue {tag}" + ) + + details = client.issue.details(created.id) + + assert isinstance(details, IssueDetails) + assert details.id == created.id + assert details.title == created.title + assert details.meeting_id == MEETING_ID + + # Cleanup + client.issue.complete(created.id) + + def test_issue_update(self, client: Client) -> None: + """Test updating an issue.""" + tag = uuid.uuid4().hex[:8] + created = client.issue.create( + meeting_id=MEETING_ID, title=f"Update test issue {tag}" + ) + + updated_title = f"Updated issue {tag}" + updated = client.issue.update(created.id, title=updated_title) + + assert isinstance(updated, IssueDetails) + assert updated.title == updated_title + + # Cleanup + client.issue.complete(created.id) + + def test_issue_complete(self, client: Client) -> None: + """Test completing an issue.""" + tag = uuid.uuid4().hex[:8] + created = client.issue.create( + meeting_id=MEETING_ID, title=f"Complete test issue {tag}" + ) + + completed = client.issue.complete(created.id) + + assert isinstance(completed, IssueDetails) + assert completed.completed_at is not None + + def test_list_user_issues(self, client: Client) -> None: + """Test listing issues for the current user.""" + result = client.issue.list() + + assert isinstance(result, list) + for item in result: + assert isinstance(item, IssueListItem) + + def test_list_meeting_issues(self, client: Client) -> None: + """Test listing issues for a meeting.""" + result = client.issue.list(meeting_id=MEETING_ID) + + assert isinstance(result, list) + for item in result: + assert isinstance(item, IssueListItem) + + def test_list_specific_user_issues(self, client: Client) -> None: + """Test listing issues for a specific user by ID.""" + user_id = client.user.details().id + result = client.issue.list(user_id=user_id) + + assert isinstance(result, list) + for item in result: + assert isinstance(item, IssueListItem) + + def test_list_both_params_raises(self, client: Client) -> None: + """Test that providing both user_id and meeting_id raises ValueError.""" + with pytest.raises(ValueError, match="Please provide either"): + client.issue.list(user_id=123, meeting_id=456) + + def test_list_truthiness_bug_zero_user_id(self, client: Client) -> None: + """Test that user_id=0 with meeting_id raises ValueError. + + BUG: `if user_id and meeting_id` fails when user_id=0 because + 0 is falsy. Should use `is not None` checks instead. + """ + with pytest.raises(ValueError, match="Please provide either"): + client.issue.list(user_id=0, meeting_id=MEETING_ID) + + def test_list_truthiness_bug_zero_meeting_id(self, client: Client) -> None: + """Test that meeting_id=0 with user_id raises ValueError. + + BUG: `if user_id and meeting_id` fails when meeting_id=0 because + 0 is falsy. Should use `is not None` checks instead. + """ + with pytest.raises(ValueError, match="Please provide either"): + client.issue.list(user_id=123, meeting_id=0) + + def test_full_lifecycle(self, client: Client) -> None: + """Test complete issue lifecycle: create -> details -> update -> complete.""" + tag = uuid.uuid4().hex[:8] + + # Create + created = client.issue.create( + meeting_id=MEETING_ID, + title=f"Lifecycle issue {tag}", + notes="Lifecycle test notes", + ) + assert created.id > 0 + + # Details + details = client.issue.details(created.id) + assert details.id == created.id + assert details.completed_at is None + + # Update + new_title = f"Updated lifecycle issue {tag}" + updated = client.issue.update(created.id, title=new_title) + assert updated.title == new_title + + # Complete + completed = client.issue.complete(created.id) + assert completed.completed_at is not None + + +class TestIssueLifecycleAsync: + """Full CRUD lifecycle tests for async issue operations.""" + + @pytest.mark.asyncio + async def test_create_issue(self, async_client: AsyncClient) -> None: + """Test creating an issue via the async API.""" + tag = uuid.uuid4().hex[:8] + title = f"Async integration test issue {tag}" + result = await async_client.issue.create(meeting_id=MEETING_ID, title=title) + + assert isinstance(result, CreatedIssue) + assert result.id > 0 + assert result.title == title + + # Cleanup + await async_client.issue.complete(result.id) + + @pytest.mark.asyncio + async def test_issue_details(self, async_client: AsyncClient) -> None: + """Test retrieving issue details via async API.""" + tag = uuid.uuid4().hex[:8] + created = await async_client.issue.create( + meeting_id=MEETING_ID, title=f"Async detail test {tag}" + ) + + details = await async_client.issue.details(created.id) + + assert isinstance(details, IssueDetails) + assert details.id == created.id + + # Cleanup + await async_client.issue.complete(created.id) + + @pytest.mark.asyncio + async def test_issue_update(self, async_client: AsyncClient) -> None: + """Test updating an issue via async API.""" + tag = uuid.uuid4().hex[:8] + created = await async_client.issue.create( + meeting_id=MEETING_ID, title=f"Async update test {tag}" + ) + + updated_title = f"Async updated {tag}" + updated = await async_client.issue.update(created.id, title=updated_title) + + assert updated.title == updated_title + + # Cleanup + await async_client.issue.complete(created.id) + + @pytest.mark.asyncio + async def test_issue_complete(self, async_client: AsyncClient) -> None: + """Test completing an issue via async API.""" + tag = uuid.uuid4().hex[:8] + created = await async_client.issue.create( + meeting_id=MEETING_ID, title=f"Async complete test {tag}" + ) + + completed = await async_client.issue.complete(created.id) + + assert completed.completed_at is not None + + @pytest.mark.asyncio + async def test_list_user_issues(self, async_client: AsyncClient) -> None: + """Test listing issues for the current user via async API.""" + result = await async_client.issue.list() + + assert isinstance(result, list) + for item in result: + assert isinstance(item, IssueListItem) + + @pytest.mark.asyncio + async def test_list_meeting_issues(self, async_client: AsyncClient) -> None: + """Test listing issues for a meeting via async API.""" + result = await async_client.issue.list(meeting_id=MEETING_ID) + + assert isinstance(result, list) + for item in result: + assert isinstance(item, IssueListItem) + + @pytest.mark.asyncio + async def test_list_both_params_raises(self, async_client: AsyncClient) -> None: + """Test that providing both params raises ValueError.""" + with pytest.raises(ValueError, match="Please provide either"): + await async_client.issue.list(user_id=123, meeting_id=456) + + @pytest.mark.asyncio + async def test_list_truthiness_bug_zero_user_id( + self, async_client: AsyncClient + ) -> None: + """Test truthiness bug: user_id=0 with meeting_id should raise ValueError.""" + with pytest.raises(ValueError, match="Please provide either"): + await async_client.issue.list(user_id=0, meeting_id=MEETING_ID) + + @pytest.mark.asyncio + async def test_list_truthiness_bug_zero_meeting_id( + self, async_client: AsyncClient + ) -> None: + """Test truthiness bug: meeting_id=0 with user_id should raise ValueError.""" + with pytest.raises(ValueError, match="Please provide either"): + await async_client.issue.list(user_id=123, meeting_id=0) + + @pytest.mark.asyncio + async def test_full_lifecycle(self, async_client: AsyncClient) -> None: + """Test complete async issue lifecycle.""" + tag = uuid.uuid4().hex[:8] + + # Create + created = await async_client.issue.create( + meeting_id=MEETING_ID, + title=f"Async lifecycle {tag}", + notes="Async lifecycle notes", + ) + assert created.id > 0 + + # Details + details = await async_client.issue.details(created.id) + assert details.id == created.id + + # Update + new_title = f"Async updated lifecycle {tag}" + updated = await async_client.issue.update(created.id, title=new_title) + assert updated.title == new_title + + # Complete + completed = await async_client.issue.complete(created.id) + assert completed.completed_at is not None diff --git a/tests/test_integration_meetings.py b/tests/test_integration_meetings.py new file mode 100644 index 0000000..8b2bff8 --- /dev/null +++ b/tests/test_integration_meetings.py @@ -0,0 +1,279 @@ +"""Integration tests for meeting operations against the real Bloom Growth API.""" + +from __future__ import annotations + +import contextlib + +import pytest + +from bloomy import AsyncClient, Client +from bloomy.models import ( + MeetingAttendee, + MeetingDetails, + MeetingListItem, + Todo, +) + + +@pytest.fixture(scope="module") +def client() -> Client: + """Create a real Bloomy client using environment-provided API key. + + Yields: + A Client instance. + + """ + c = Client() + yield c + c.close() + + +@pytest.fixture() +def async_client() -> AsyncClient: + """Create a fresh async Bloomy client per test. + + Returns: + An AsyncClient instance. + + """ + return AsyncClient() + + +@pytest.fixture(scope="module") +def meeting_id(client: Client) -> int: + """Get the first meeting ID from the user's meeting list. + + Returns: + The meeting ID. + + """ + meetings = client.meeting.list() + assert len(meetings) > 0, "No meetings found for the test user" + return meetings[0].id + + +@pytest.fixture() +def temp_meeting(client: Client) -> int: + """Create a temporary meeting and delete it after the test. + + Yields: + The meeting ID. + + """ + result = client.meeting.create("Integration Test Meeting", add_self=True) + mid = result["meeting_id"] + yield mid + with contextlib.suppress(Exception): + client.meeting.delete(mid) + + +@pytest.mark.integration +class TestMeetingList: + """Tests for meeting.list().""" + + def test_list_returns_meeting_list_items(self, client: Client) -> None: + """Listing meetings returns a non-empty list of MeetingListItem.""" + meetings = client.meeting.list() + assert isinstance(meetings, list) + assert len(meetings) > 0 + for m in meetings: + assert isinstance(m, MeetingListItem) + assert m.id > 0 + assert isinstance(m.name, str) + + def test_list_with_explicit_user_id(self, client: Client) -> None: + """Listing meetings with explicit user_id works.""" + user_id = client.user.details().id + meetings = client.meeting.list(user_id=user_id) + assert isinstance(meetings, list) + + @pytest.mark.asyncio + async def test_async_list(self, async_client: AsyncClient) -> None: + """Async list returns the same structure.""" + async with async_client: + meetings = await async_client.meeting.list() + assert isinstance(meetings, list) + assert len(meetings) > 0 + for m in meetings: + assert isinstance(m, MeetingListItem) + + +@pytest.mark.integration +class TestMeetingAttendees: + """Tests for meeting.attendees().""" + + def test_attendees_returns_list(self, client: Client, meeting_id: int) -> None: + """Attendees returns a list of MeetingAttendee.""" + attendees = client.meeting.attendees(meeting_id) + assert isinstance(attendees, list) + assert len(attendees) > 0 + for a in attendees: + assert isinstance(a, MeetingAttendee) + assert a.user_id > 0 + assert isinstance(a.name, str) + + @pytest.mark.asyncio + async def test_async_attendees( + self, async_client: AsyncClient, meeting_id: int + ) -> None: + """Async attendees returns the same structure.""" + async with async_client: + attendees = await async_client.meeting.attendees(meeting_id) + assert isinstance(attendees, list) + assert len(attendees) > 0 + + +@pytest.mark.integration +class TestMeetingIssues: + """Tests for meeting.issues().""" + + def test_issues_returns_list(self, client: Client, meeting_id: int) -> None: + """Issues returns a list (possibly empty).""" + issues = client.meeting.issues(meeting_id) + assert isinstance(issues, list) + + def test_issues_include_closed(self, client: Client, meeting_id: int) -> None: + """Issues with include_closed does not raise.""" + issues = client.meeting.issues(meeting_id, include_closed=True) + assert isinstance(issues, list) + + +@pytest.mark.integration +class TestMeetingTodos: + """Tests for meeting.todos().""" + + def test_todos_returns_list(self, client: Client, meeting_id: int) -> None: + """Todos returns a list of Todo models.""" + todos = client.meeting.todos(meeting_id) + assert isinstance(todos, list) + for t in todos: + assert isinstance(t, Todo) + + def test_todos_include_closed(self, client: Client, meeting_id: int) -> None: + """Todos with include_closed does not raise.""" + todos = client.meeting.todos(meeting_id, include_closed=True) + assert isinstance(todos, list) + + +@pytest.mark.integration +class TestMeetingMetrics: + """Tests for meeting.metrics().""" + + def test_metrics_returns_list(self, client: Client, meeting_id: int) -> None: + """Metrics returns a list (possibly empty).""" + metrics = client.meeting.metrics(meeting_id) + assert isinstance(metrics, list) + + +@pytest.mark.integration +class TestMeetingDetails: + """Tests for meeting.details().""" + + def test_details_returns_meeting_details( + self, client: Client, meeting_id: int + ) -> None: + """Details returns a fully populated MeetingDetails.""" + details = client.meeting.details(meeting_id) + assert isinstance(details, MeetingDetails) + assert details.id == meeting_id + assert isinstance(details.name, str) + assert details.attendees is not None + + def test_details_nonexistent_meeting_raises(self, client: Client) -> None: + """Details for a non-existent meeting ID raises an HTTP error.""" + import httpx + + with pytest.raises(httpx.HTTPStatusError): + client.meeting.details(999999999) + + def test_details_populates_created_date( + self, client: Client, meeting_id: int + ) -> None: + """Verify that details() populates created_date from the direct endpoint. + + Previously details() used getattr on MeetingListItem which never had + these fields, so they were always None. Now it calls L10/{id} directly. + """ + details = client.meeting.details(meeting_id) + + # created_date should be populated from the direct endpoint's CreateTime + assert details.created_date is not None, ( + "created_date should be populated from the L10/{id} endpoint" + ) + + def test_details_works_for_non_attendee_meetings(self, client: Client) -> None: + """Verify details() works even when user is not an attendee. + + Previously details() called self.list() which only returned meetings + the user attended. Now it uses the direct L10/{id} endpoint. + """ + # Create a meeting without adding self as attendee + result = client.meeting.create("Fix Test - Not My Meeting", add_self=False) + created_id = result["meeting_id"] + + try: + # details() should now succeed even though we're not an attendee + details = client.meeting.details(created_id) + assert details.id == created_id + assert details.name == "Fix Test - Not My Meeting" + finally: + client.meeting.delete(created_id) + + @pytest.mark.asyncio + async def test_async_details( + self, async_client: AsyncClient, meeting_id: int + ) -> None: + """Async details returns the same structure.""" + async with async_client: + details = await async_client.meeting.details(meeting_id) + assert isinstance(details, MeetingDetails) + assert details.id == meeting_id + + +@pytest.mark.integration +class TestMeetingCreateDelete: + """Tests for meeting create/delete lifecycle.""" + + def test_create_and_delete(self, client: Client) -> None: + """Create a meeting and then delete it.""" + result = client.meeting.create("Integration Create Test") + assert "meeting_id" in result + assert result["title"] == "Integration Create Test" + assert isinstance(result["meeting_id"], int) + + # Verify it appears in list + meetings = client.meeting.list() + ids = [m.id for m in meetings] + assert result["meeting_id"] in ids + + # Delete + deleted = client.meeting.delete(result["meeting_id"]) + assert deleted is True + + # Verify it no longer appears + meetings = client.meeting.list() + ids = [m.id for m in meetings] + assert result["meeting_id"] not in ids + + def test_create_with_add_self_false(self, client: Client) -> None: + """Create a meeting with add_self=False.""" + result = client.meeting.create("No Self Meeting", add_self=False) + mid = result["meeting_id"] + + try: + # Meeting should NOT appear in our list + meetings = client.meeting.list() + ids = [m.id for m in meetings] + assert mid not in ids + finally: + client.meeting.delete(mid) + + @pytest.mark.asyncio + async def test_async_create_and_delete(self, async_client: AsyncClient) -> None: + """Async create and delete lifecycle.""" + async with async_client: + result = await async_client.meeting.create("Async Integration Test") + assert "meeting_id" in result + + deleted = await async_client.meeting.delete(result["meeting_id"]) + assert deleted is True diff --git a/tests/test_integration_scorecard.py b/tests/test_integration_scorecard.py new file mode 100644 index 0000000..b4a0e66 --- /dev/null +++ b/tests/test_integration_scorecard.py @@ -0,0 +1,214 @@ +"""Integration tests for scorecard operations against the real Bloom Growth API.""" + +from __future__ import annotations + +import pytest + +from bloomy import AsyncClient, Client +from bloomy.models import ScorecardItem, ScorecardWeek + + +@pytest.fixture(scope="module") +def client() -> Client: + """Create a real Bloomy client using environment-provided API key. + + Yields: + A Client instance. + + """ + c = Client() + yield c + c.close() + + +@pytest.fixture() +def async_client() -> AsyncClient: + """Create a fresh async Bloomy client per test. + + Returns: + An AsyncClient instance. + + """ + return AsyncClient() + + +@pytest.fixture(scope="module") +def measurable_id(client: Client) -> int: + """Get the first measurable ID from the user's scorecard. + + Returns: + The measurable ID. + + """ + items = client.scorecard.list(show_empty=True) + assert len(items) > 0, "No scorecard items found for the test user" + return items[0].measurable_id + + +@pytest.mark.integration +class TestScorecardCurrentWeek: + """Tests for scorecard.current_week().""" + + def test_current_week_returns_scorecard_week(self, client: Client) -> None: + """Current week returns a ScorecardWeek with valid fields.""" + week = client.scorecard.current_week() + assert isinstance(week, ScorecardWeek) + assert week.id > 0 + assert week.week_number > 0 + assert isinstance(week.week_start, str) + assert isinstance(week.week_end, str) + + def test_current_week_field_semantics(self, client: Client) -> None: + """Verify that week_start and week_end have correct semantic meaning. + + week_start comes from LocalDate.Date, week_end comes from ForWeek. + ForWeek in weeks/current is a date string (the week end date). + """ + week = client.scorecard.current_week() + # Both should be date strings + assert "T" in week.week_start, "week_start should be a datetime string" + assert "T" in week.week_end, "week_end should be a datetime string" + + @pytest.mark.asyncio + async def test_async_current_week(self, async_client: AsyncClient) -> None: + """Async current_week returns the same structure.""" + async with async_client: + week = await async_client.scorecard.current_week() + assert isinstance(week, ScorecardWeek) + assert week.week_number > 0 + + +@pytest.mark.integration +class TestScorecardList: + """Tests for scorecard.list().""" + + def test_list_returns_scorecard_items(self, client: Client) -> None: + """List with show_empty returns all scorecard items.""" + items = client.scorecard.list(show_empty=True) + assert isinstance(items, list) + assert len(items) > 0 + for item in items: + assert isinstance(item, ScorecardItem) + assert item.measurable_id > 0 + assert isinstance(item.title, str) + assert item.week_id > 0 + + def test_list_filters_empty_by_default(self, client: Client) -> None: + """List without show_empty filters out None values.""" + all_items = client.scorecard.list(show_empty=True) + non_empty = client.scorecard.list(show_empty=False) + assert len(non_empty) <= len(all_items) + for item in non_empty: + assert item.value is not None + + def test_list_with_week_offset_zero(self, client: Client) -> None: + """List with week_offset=0 returns only current week items.""" + items = client.scorecard.list(show_empty=True, week_offset=0) + assert isinstance(items, list) + # All items should have the same week_id (the current week number) + if items: + week = client.scorecard.current_week() + for item in items: + assert item.week_id == week.week_number + + def test_list_with_negative_week_offset(self, client: Client) -> None: + """List with negative week_offset returns previous week items.""" + items = client.scorecard.list(show_empty=True, week_offset=-1) + assert isinstance(items, list) + if items: + week = client.scorecard.current_week() + expected_week_id = week.week_number - 1 + for item in items: + assert item.week_id == expected_week_id + + def test_list_by_meeting_id(self, client: Client) -> None: + """List with meeting_id does not raise.""" + meetings = client.meeting.list() + if meetings: + items = client.scorecard.list(meeting_id=meetings[0].id, show_empty=True) + assert isinstance(items, list) + + def test_list_raises_on_both_user_and_meeting(self, client: Client) -> None: + """Providing both user_id and meeting_id raises ValueError.""" + with pytest.raises(ValueError, match="not both"): + client.scorecard.list(user_id=1, meeting_id=1) + + @pytest.mark.asyncio + async def test_async_list(self, async_client: AsyncClient) -> None: + """Async list returns the same structure.""" + async with async_client: + items = await async_client.scorecard.list(show_empty=True) + assert isinstance(items, list) + assert len(items) > 0 + + +@pytest.mark.integration +class TestScorecardGet: + """Tests for scorecard.get().""" + + def test_get_existing_measurable(self, client: Client, measurable_id: int) -> None: + """Get an existing measurable returns a ScorecardItem.""" + item = client.scorecard.get(measurable_id=measurable_id) + assert item is not None or isinstance(item, ScorecardItem) + + def test_get_nonexistent_measurable(self, client: Client) -> None: + """Get a non-existent measurable returns None.""" + item = client.scorecard.get(measurable_id=999999999) + assert item is None + + def test_get_with_week_offset(self, client: Client, measurable_id: int) -> None: + """Get with week_offset returns correct week.""" + item = client.scorecard.get(measurable_id=measurable_id, week_offset=0) + if item: + week = client.scorecard.current_week() + assert item.week_id == week.week_number + + @pytest.mark.asyncio + async def test_async_get( + self, async_client: AsyncClient, measurable_id: int + ) -> None: + """Async get returns the same result.""" + async with async_client: + item = await async_client.scorecard.get(measurable_id=measurable_id) + assert item is not None or isinstance(item, ScorecardItem) + + +@pytest.mark.integration +class TestScorecardScore: + """Tests for scorecard.score().""" + + def test_score_update_and_verify(self, client: Client, measurable_id: int) -> None: + """Score a measurable and verify the value is updated.""" + # Read the current value + before = client.scorecard.get(measurable_id=measurable_id, week_offset=0) + + # Set a known value + test_value = 42.0 + result = client.scorecard.score( + measurable_id=measurable_id, score=test_value, week_offset=0 + ) + assert result is True + + # Verify the value + after = client.scorecard.get(measurable_id=measurable_id, week_offset=0) + assert after is not None + assert after.value == test_value + + # Restore original value if it existed + if before and before.value is not None: + client.scorecard.score( + measurable_id=measurable_id, + score=before.value, + week_offset=0, + ) + + @pytest.mark.asyncio + async def test_async_score( + self, async_client: AsyncClient, measurable_id: int + ) -> None: + """Async score updates correctly.""" + async with async_client: + result = await async_client.scorecard.score( + measurable_id=measurable_id, score=99.0, week_offset=0 + ) + assert result is True diff --git a/tests/test_integration_todos.py b/tests/test_integration_todos.py new file mode 100644 index 0000000..97d567f --- /dev/null +++ b/tests/test_integration_todos.py @@ -0,0 +1,226 @@ +"""Integration tests for todo operations against the real Bloom Growth API.""" + +from __future__ import annotations + +import pytest +import pytest_asyncio + +from bloomy import AsyncClient, Client +from bloomy.models import Todo + +MEETING_ID = 324926 + + +@pytest.fixture(scope="module") +def client() -> Client: + """Create a real client for integration tests. + + Yields: + A Bloomy client instance. + + """ + c = Client() + yield c + c.close() + + +@pytest_asyncio.fixture +async def async_client() -> AsyncClient: + """Create a real async client for integration tests. + + Yields: + An async Bloomy client instance. + + """ + c = AsyncClient() + yield c + await c.close() + + +# ── Sync Tests ────────────────────────────────────────────────────────── + + +@pytest.mark.integration +class TestTodoListSync: + """Test listing todos (sync).""" + + def test_list_user_todos(self, client: Client) -> None: + """List todos for the current user.""" + todos = client.todo.list() + assert isinstance(todos, list) + for todo in todos: + assert isinstance(todo, Todo) + + def test_list_meeting_todos(self, client: Client) -> None: + """List todos for a specific meeting.""" + todos = client.todo.list(meeting_id=MEETING_ID) + assert isinstance(todos, list) + for todo in todos: + assert isinstance(todo, Todo) + + def test_list_raises_on_both_ids(self, client: Client) -> None: + """Providing both user_id and meeting_id should raise ValueError.""" + with pytest.raises(ValueError, match="not both"): + client.todo.list(user_id=1, meeting_id=1) + + +@pytest.mark.integration +class TestTodoCRUDSync: + """Test full CRUD lifecycle for todos (sync).""" + + def test_create_and_delete_user_todo(self, client: Client) -> None: + """Create a user todo, verify it, then clean up.""" + todo = client.todo.create(title="Integration test user todo") + try: + assert isinstance(todo, Todo) + assert todo.name == "Integration test user todo" + assert todo.id > 0 + + # Verify via details + fetched = client.todo.details(todo.id) + assert fetched.id == todo.id + assert fetched.name == todo.name + finally: + # Clean up — complete the todo since there's no delete endpoint + client.todo.complete(todo.id) + + def test_create_meeting_todo(self, client: Client) -> None: + """Create a meeting todo and verify it shows up in meeting todo list.""" + todo = client.todo.create( + title="Integration test meeting todo", + meeting_id=MEETING_ID, + ) + try: + assert isinstance(todo, Todo) + assert todo.name == "Integration test meeting todo" + assert todo.meeting_id == MEETING_ID + finally: + client.todo.complete(todo.id) + + def test_create_todo_with_due_date(self, client: Client) -> None: + """Create a todo with a due date.""" + todo = client.todo.create( + title="Integration test due date todo", + due_date="2026-12-31", + ) + try: + assert isinstance(todo, Todo) + assert todo.due_date is not None + finally: + client.todo.complete(todo.id) + + def test_complete_todo(self, client: Client) -> None: + """Complete a todo and verify the complete flag.""" + todo = client.todo.create(title="Integration test complete todo") + completed = client.todo.complete(todo.id) + assert completed.complete is True + + def test_update_todo_title(self, client: Client) -> None: + """Update a todo title.""" + todo = client.todo.create(title="Integration test original title") + try: + updated = client.todo.update(todo.id, title="Updated title") + assert updated.name == "Updated title" + finally: + client.todo.complete(todo.id) + + def test_update_todo_due_date(self, client: Client) -> None: + """Update a todo due date.""" + todo = client.todo.create(title="Integration test update due date") + try: + updated = client.todo.update(todo.id, due_date="2026-06-15") + assert updated.due_date is not None + finally: + client.todo.complete(todo.id) + + def test_update_raises_without_fields(self, client: Client) -> None: + """Updating with no fields should raise ValueError.""" + todo = client.todo.create(title="Integration test no-op update") + try: + with pytest.raises(ValueError, match="At least one field"): + client.todo.update(todo.id) + finally: + client.todo.complete(todo.id) + + def test_details(self, client: Client) -> None: + """Get todo details by ID.""" + todo = client.todo.create(title="Integration test details") + try: + details = client.todo.details(todo.id) + assert details.id == todo.id + assert details.name == "Integration test details" + finally: + client.todo.complete(todo.id) + + +# ── Async Tests ───────────────────────────────────────────────────────── + + +@pytest.mark.integration +class TestTodoListAsync: + """Test listing todos (async).""" + + @pytest.mark.asyncio + async def test_list_user_todos(self, async_client: AsyncClient) -> None: + """List todos for the current user (async).""" + todos = await async_client.todo.list() + assert isinstance(todos, list) + for todo in todos: + assert isinstance(todo, Todo) + + @pytest.mark.asyncio + async def test_list_meeting_todos(self, async_client: AsyncClient) -> None: + """List todos for a specific meeting (async).""" + todos = await async_client.todo.list(meeting_id=MEETING_ID) + assert isinstance(todos, list) + for todo in todos: + assert isinstance(todo, Todo) + + @pytest.mark.asyncio + async def test_list_raises_on_both_ids(self, async_client: AsyncClient) -> None: + """Providing both user_id and meeting_id should raise ValueError (async).""" + with pytest.raises(ValueError, match="not both"): + await async_client.todo.list(user_id=1, meeting_id=1) + + +@pytest.mark.integration +class TestTodoCRUDAsync: + """Test full CRUD lifecycle for todos (async).""" + + @pytest.mark.asyncio + async def test_create_complete_lifecycle(self, async_client: AsyncClient) -> None: + """Create, read, update, complete a todo (async).""" + # Create + todo = await async_client.todo.create( + title="Async integration test todo", + ) + try: + assert isinstance(todo, Todo) + assert todo.name == "Async integration test todo" + + # Read + details = await async_client.todo.details(todo.id) + assert details.id == todo.id + + # Update + updated = await async_client.todo.update( + todo.id, title="Async updated title" + ) + assert updated.name == "Async updated title" + finally: + # Complete (cleanup) + completed = await async_client.todo.complete(todo.id) + assert completed.complete is True + + @pytest.mark.asyncio + async def test_create_meeting_todo(self, async_client: AsyncClient) -> None: + """Create a meeting todo (async).""" + todo = await async_client.todo.create( + title="Async meeting todo", + meeting_id=MEETING_ID, + ) + try: + assert isinstance(todo, Todo) + assert todo.meeting_id == MEETING_ID + finally: + await async_client.todo.complete(todo.id) diff --git a/tests/test_integration_users.py b/tests/test_integration_users.py new file mode 100644 index 0000000..eb38b6c --- /dev/null +++ b/tests/test_integration_users.py @@ -0,0 +1,345 @@ +"""Integration tests for user operations and configuration against the real API.""" + +from __future__ import annotations + +import os + +import pytest +import pytest_asyncio + +from bloomy import AsyncClient, Client, Configuration, ConfigurationError +from bloomy.models import ( + DirectReport, + Position, + UserDetails, + UserListItem, + UserSearchResult, +) + +pytestmark = pytest.mark.integration + + +# --------------------------------------------------------------------------- +# Fixtures +# --------------------------------------------------------------------------- + + +@pytest.fixture(scope="module") +def api_key() -> str: + """Get the API key from the environment. + + Returns: + The BG_API_KEY value. + + """ + key = os.environ.get("BG_API_KEY") + if not key: + pytest.skip("BG_API_KEY not set") + return key + + +@pytest.fixture(scope="module") +def client(api_key: str) -> Client: + """Create a real Client instance for integration tests. + + Yields: + A Client connected to the real API. + + """ + with Client(api_key=api_key) as c: + yield c + + +@pytest_asyncio.fixture +async def async_client(api_key: str) -> AsyncClient: + """Create a real AsyncClient instance for integration tests. + + Yields: + An AsyncClient connected to the real API. + + """ + async with AsyncClient(api_key=api_key) as c: + yield c + + +# --------------------------------------------------------------------------- +# Configuration tests +# --------------------------------------------------------------------------- + + +class TestConfiguration: + """Tests for the Configuration class.""" + + def test_configuration_with_explicit_key(self, api_key: str) -> None: + """Configuration should accept an explicit API key.""" + config = Configuration(api_key=api_key) + assert config.api_key == api_key + + def test_configuration_from_env_var(self) -> None: + """Configuration should pick up the BG_API_KEY environment variable.""" + key = os.environ.get("BG_API_KEY") + if not key: + pytest.skip("BG_API_KEY not set") + config = Configuration() + assert config.api_key == key + + def test_configuration_none_when_no_key( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Configuration should have api_key=None when no source available.""" + monkeypatch.delenv("BG_API_KEY", raising=False) + config = Configuration() + # api_key could be loaded from config file; if not present it's None + # We just verify it doesn't raise + assert config.api_key is None or isinstance(config.api_key, str) + + def test_client_raises_without_key(self, monkeypatch: pytest.MonkeyPatch) -> None: + """Client should raise ConfigurationError when no API key exists.""" + monkeypatch.delenv("BG_API_KEY", raising=False) + # Patch _load_api_key to ensure no config file fallback + with pytest.raises(ConfigurationError): + Client(api_key=None) + + def test_whitespace_only_api_key_rejected( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Whitespace-only API key should be rejected by Client.""" + monkeypatch.delenv("BG_API_KEY", raising=False) + with pytest.raises(ConfigurationError): + Client(api_key=" ") + + def test_configuration_strips_whitespace(self, api_key: str) -> None: + """Configuration should strip surrounding whitespace from API key.""" + config = Configuration(api_key=f" {api_key} ") + assert config.api_key == api_key + + +# --------------------------------------------------------------------------- +# Sync user operations +# --------------------------------------------------------------------------- + + +class TestUserDetailsSync: + """Tests for sync user.details().""" + + def test_current_user_details(self, client: Client) -> None: + """Get details for the authenticated user (no explicit user_id).""" + user = client.user.details() + assert isinstance(user, UserDetails) + assert isinstance(user.id, int) + assert isinstance(user.name, str) + assert len(user.name) > 0 + assert isinstance(user.image_url, str) + + def test_current_user_details_with_explicit_id(self, client: Client) -> None: + """Get details using an explicit user_id (same user).""" + my_id = client.user.user_id + user = client.user.details(user_id=my_id) + assert user.id == my_id + + def test_details_include_direct_reports(self, client: Client) -> None: + """include_direct_reports should populate the direct_reports field.""" + user = client.user.details(include_direct_reports=True) + assert user.direct_reports is not None + assert isinstance(user.direct_reports, list) + for dr in user.direct_reports: + assert isinstance(dr, DirectReport) + + def test_details_include_positions(self, client: Client) -> None: + """include_positions should populate the positions field.""" + user = client.user.details(include_positions=True) + assert user.positions is not None + assert isinstance(user.positions, list) + for pos in user.positions: + assert isinstance(pos, Position) + + def test_details_include_all(self, client: Client) -> None: + """include_all should populate both direct_reports and positions.""" + user = client.user.details(include_all=True) + assert user.direct_reports is not None + assert user.positions is not None + + def test_details_without_includes_has_none_fields(self, client: Client) -> None: + """Without include flags, direct_reports and positions should be None.""" + user = client.user.details() + assert user.direct_reports is None + assert user.positions is None + + +class TestDirectReportsSync: + """Tests for sync user.direct_reports().""" + + def test_direct_reports_returns_list(self, client: Client) -> None: + """direct_reports() should return a list of DirectReport models.""" + reports = client.user.direct_reports() + assert isinstance(reports, list) + for report in reports: + assert isinstance(report, DirectReport) + assert isinstance(report.id, int) + assert isinstance(report.name, str) + assert isinstance(report.image_url, str) + + +class TestPositionsSync: + """Tests for sync user.positions().""" + + def test_positions_returns_list(self, client: Client) -> None: + """positions() should return a list of Position models.""" + positions = client.user.positions() + assert isinstance(positions, list) + for pos in positions: + assert isinstance(pos, Position) + assert isinstance(pos.id, int) + + +class TestSearchSync: + """Tests for sync user.search().""" + + def test_search_returns_results(self, client: Client) -> None: + """search() with a broad term should return results.""" + # Get the current user's name to use as a search term + me = client.user.details() + first_name = me.name.split()[0] + results = client.user.search(term=first_name) + assert isinstance(results, list) + assert len(results) > 0 + for result in results: + assert isinstance(result, UserSearchResult) + assert isinstance(result.id, int) + assert isinstance(result.name, str) + assert isinstance(result.email, str) + assert isinstance(result.organization_id, int) + + def test_search_empty_term_returns_empty(self, client: Client) -> None: + """search() with a nonsense term should return an empty list.""" + results = client.user.search(term="zzzznonexistentuserxyz9999") + assert isinstance(results, list) + assert len(results) == 0 + + +class TestListSync: + """Tests for sync user.list().""" + + def test_list_returns_users(self, client: Client) -> None: + """list() should return a non-empty list of UserListItem.""" + users = client.user.list() + assert isinstance(users, list) + assert len(users) > 0 + for user in users: + assert isinstance(user, UserListItem) + assert isinstance(user.id, int) + assert isinstance(user.name, str) + assert isinstance(user.email, str) + + def test_list_with_placeholders(self, client: Client) -> None: + """list(include_placeholders=True) may return more users.""" + without = client.user.list(include_placeholders=False) + with_ph = client.user.list(include_placeholders=True) + assert len(with_ph) >= len(without) + + +# --------------------------------------------------------------------------- +# Async user operations +# --------------------------------------------------------------------------- + + +class TestUserDetailsAsync: + """Tests for async user.details().""" + + @pytest.mark.asyncio + async def test_current_user_details(self, async_client: AsyncClient) -> None: + """Get details for the authenticated user (async).""" + user = await async_client.user.details() + assert isinstance(user, UserDetails) + assert isinstance(user.id, int) + assert isinstance(user.name, str) + assert len(user.name) > 0 + + @pytest.mark.asyncio + async def test_details_include_all(self, async_client: AsyncClient) -> None: + """include_all should populate both fields (async).""" + user = await async_client.user.details(include_all=True) + assert user.direct_reports is not None + assert user.positions is not None + + +class TestDirectReportsAsync: + """Tests for async user.direct_reports().""" + + @pytest.mark.asyncio + async def test_direct_reports_returns_list(self, async_client: AsyncClient) -> None: + """direct_reports() should return a list of DirectReport models (async).""" + reports = await async_client.user.direct_reports() + assert isinstance(reports, list) + for report in reports: + assert isinstance(report, DirectReport) + + +class TestPositionsAsync: + """Tests for async user.positions().""" + + @pytest.mark.asyncio + async def test_positions_returns_list(self, async_client: AsyncClient) -> None: + """positions() should return a list of Position models (async).""" + positions = await async_client.user.positions() + assert isinstance(positions, list) + for pos in positions: + assert isinstance(pos, Position) + + +class TestSearchAsync: + """Tests for async user.search().""" + + @pytest.mark.asyncio + async def test_search_returns_results(self, async_client: AsyncClient) -> None: + """search() with a broad term should return results (async).""" + me = await async_client.user.details() + first_name = me.name.split()[0] + results = await async_client.user.search(term=first_name) + assert isinstance(results, list) + assert len(results) > 0 + for result in results: + assert isinstance(result, UserSearchResult) + + +class TestListAsync: + """Tests for async user.list().""" + + @pytest.mark.asyncio + async def test_list_returns_users(self, async_client: AsyncClient) -> None: + """list() should return a non-empty list (async).""" + users = await async_client.user.list() + assert isinstance(users, list) + assert len(users) > 0 + for user in users: + assert isinstance(user, UserListItem) + + +# --------------------------------------------------------------------------- +# Cross-check: sync vs async consistency +# --------------------------------------------------------------------------- + + +class TestSyncAsyncConsistency: + """Verify sync and async operations return equivalent data.""" + + @pytest.mark.asyncio + async def test_user_details_match( + self, client: Client, async_client: AsyncClient + ) -> None: + """Sync and async user.details() should return the same user.""" + sync_user = client.user.details() + async_user = await async_client.user.details() + assert sync_user.id == async_user.id + assert sync_user.name == async_user.name + + @pytest.mark.asyncio + async def test_user_list_match( + self, client: Client, async_client: AsyncClient + ) -> None: + """Sync and async user.list() should return the same set of users.""" + sync_users = client.user.list() + async_users = await async_client.user.list() + sync_ids = {u.id for u in sync_users} + async_ids = {u.id for u in async_users} + assert sync_ids == async_ids diff --git a/tests/test_meetings.py b/tests/test_meetings.py index e6a1673..25b0482 100644 --- a/tests/test_meetings.py +++ b/tests/test_meetings.py @@ -3,8 +3,6 @@ from typing import Any from unittest.mock import Mock -import pytest - from bloomy.operations.meetings import MeetingOperations @@ -153,20 +151,19 @@ def test_metrics_not_list(self, mock_http_client: Mock) -> None: assert result == [] - def test_details(self, mock_http_client: Mock, mock_user_id: Mock) -> None: - """Test getting meeting details.""" - # Mock list response - list_response = Mock() - list_response.json.return_value = [ - { - "Id": 789, - "Type": "NameId", - "Key": "NameId_789", - "Name": "Team Meeting", - } - ] - - # Mock other responses + def test_details(self, mock_http_client: Mock) -> None: + """Test getting meeting details via direct endpoint.""" + # Mock direct L10/{id} response + direct_response = Mock() + direct_response.json.return_value = { + "Id": 789, + "Basics": {"Name": "Team Meeting"}, + "CreateTime": "2024-06-01T10:00:00Z", + "StartDateUtc": None, + "OrganizationId": 42, + } + + # Mock sub-resource responses attendees_response = Mock() attendees_response.json.return_value = [] @@ -180,7 +177,7 @@ def test_details(self, mock_http_client: Mock, mock_user_id: Mock) -> None: metrics_response.json.return_value = [] mock_http_client.get.side_effect = [ - list_response, + direct_response, attendees_response, issues_response, todos_response, @@ -193,27 +190,14 @@ def test_details(self, mock_http_client: Mock, mock_user_id: Mock) -> None: assert result.id == 789 assert result.name == "Team Meeting" + assert result.organization_id == 42 assert result.attendees is not None assert result.issues is not None assert result.todos is not None assert result.metrics is not None - def test_details_meeting_not_found( - self, mock_http_client: Mock, mock_user_id: Mock - ) -> None: - """Test getting details for non-existent meeting.""" - mock_response = Mock() - mock_response.json.return_value = [] - mock_http_client.get.return_value = mock_response - - meeting_ops = MeetingOperations(mock_http_client) - - from bloomy.exceptions import APIError - - with pytest.raises(APIError) as exc_info: - meeting_ops.details(meeting_id=999) - - assert "Meeting with ID 999 not found" in str(exc_info.value) + # Verify the first call is to the direct endpoint + mock_http_client.get.assert_any_call("L10/789") def test_create_meeting(self, mock_http_client: Mock) -> None: """Test creating a meeting."""