From 962e51cc9ea5ecc0f9961475b28639d606d7f347 Mon Sep 17 00:00:00 2001 From: Franccesco Orozco Date: Fri, 20 Mar 2026 10:42:45 -0600 Subject: [PATCH 01/10] fix(operations): use proper None checks in mutual exclusion guards Replace `if user_id and meeting_id:` with `if user_id is not None and meeting_id is not None:` in issues, headlines, and scorecard operations (sync + async). The truthiness check failed silently when user_id=0, allowing both params to be used simultaneously instead of raising ValueError. Also fix downstream `if meeting_id:` to `if meeting_id is not None:` and headline create DetailsUrl null handling. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/bloomy/operations/async_/headlines.py | 6 +++--- src/bloomy/operations/async_/issues.py | 4 ++-- src/bloomy/operations/async_/scorecard.py | 4 ++-- src/bloomy/operations/headlines.py | 6 +++--- src/bloomy/operations/issues.py | 4 ++-- src/bloomy/operations/scorecard.py | 4 ++-- 6 files changed, 14 insertions(+), 14 deletions(-) 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_/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/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/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: From 3d4cf0cf9ca5ea8e054ef81f2acb9a039e385f25 Mon Sep 17 00:00:00 2001 From: Franccesco Orozco Date: Fri, 20 Mar 2026 10:42:54 -0600 Subject: [PATCH 02/10] fix(goals): make accountable_user truly optional and harden Origins access Goal update() previously always defaulted accountable_user to the current user, silently overwriting the existing owner when only updating the title or status. Now accountable_user is only included in the API payload when explicitly provided. Also add safety checks for empty or malformed Origins arrays in goal transform functions. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/bloomy/operations/async_/goals.py | 7 ++---- src/bloomy/operations/goals.py | 7 ++---- .../operations/mixins/goals_transform.py | 24 +++++++++++++++---- 3 files changed, 23 insertions(+), 15 deletions(-) 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/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/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 From 4473aa7f9b169f8cef0965588804b17673eee6f1 Mon Sep 17 00:00:00 2001 From: Franccesco Orozco Date: Fri, 20 Mar 2026 10:43:05 -0600 Subject: [PATCH 03/10] fix(meetings): rewrite details() to use direct API endpoint Meeting details() previously called self.list() which only returned the current user's meetings, then filtered by ID. This meant meetings the user was an attendee of (but didn't own) could not be found, and fields like start_date_utc, created_date, and organization_id were always None. Now calls GET L10/{meeting_id} directly, populating all fields from the API response. Also harden metrics transform measurable_id check and Owner null handling in meeting issues transform. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/bloomy/operations/async_/meetings.py | 27 +++++++------------ src/bloomy/operations/meetings.py | 22 ++++++--------- .../operations/mixins/meetings_transform.py | 8 +++--- 3 files changed, 21 insertions(+), 36 deletions(-) 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/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/meetings_transform.py b/src/bloomy/operations/mixins/meetings_transform.py index c457879..5873f69 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", {}) @@ -109,9 +109,9 @@ def _transform_meeting_issues( 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", ""), + OwnerId=(issue.get("Owner") or {}).get("Id", 0), + OwnerName=(issue.get("Owner") or {}).get("Name", ""), + OwnerImageUrl=(issue.get("Owner") or {}).get("ImageUrl", ""), MeetingId=meeting_id, MeetingName=issue["Origin"], ) From c35da5f9163de49eb904d1bc4e596686f4db4a95 Mon Sep 17 00:00:00 2001 From: Franccesco Orozco Date: Fri, 20 Mar 2026 10:43:15 -0600 Subject: [PATCH 04/10] fix(core): harden configuration, models, and todo edge cases Strip whitespace from API keys in Configuration to reject whitespace-only keys. Make UserSearchResult and UserListItem model fields optional where API may return None. Use timezone-aware UTC datetime in todo create fallback. Fix user list placeholder filtering to use email check. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/bloomy/configuration.py | 5 ++++- src/bloomy/models.py | 10 +++++----- src/bloomy/operations/async_/todos.py | 4 ++-- src/bloomy/operations/mixins/users_transform.py | 2 +- src/bloomy/operations/todos.py | 4 ++-- 5 files changed, 14 insertions(+), 11 deletions(-) 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_/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/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/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, From 13f11fdacdecd2a564e26360d035a06c7c49e5ad Mon Sep 17 00:00:00 2001 From: Franccesco Orozco Date: Fri, 20 Mar 2026 10:43:26 -0600 Subject: [PATCH 05/10] test: update existing tests for meeting details and goal update changes Update mock expectations in meeting tests to match the new direct API call pattern (GET L10/{id} instead of list-then-filter). Update goal tests to reflect that accountable_user is no longer auto-populated. Update bulk operation tests for new meeting details behavior. Co-Authored-By: Claude Opus 4.6 (1M context) --- tests/test_async_goals.py | 1 - tests/test_async_meetings.py | 103 ++++++++----- tests/test_async_meetings_extra.py | 38 ++--- tests/test_bulk_operations.py | 234 +++++++++++++---------------- tests/test_goals.py | 1 - tests/test_meetings.py | 50 +++--- 6 files changed, 197 insertions(+), 230 deletions(-) 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_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.""" From dea2edbfa9a379fae15d0b624372125f8ef073fc Mon Sep 17 00:00:00 2001 From: Franccesco Orozco Date: Fri, 20 Mar 2026 10:43:35 -0600 Subject: [PATCH 06/10] test: add integration tests for all SDK operations Add 134 integration tests that validate all operations against the real Bloom Growth API, covering users, meetings, scorecard, todos, goals, issues, and headlines (sync + async). Tests are marked with @pytest.mark.integration and can be deselected for CI with -m "not integration". Co-Authored-By: Claude Opus 4.6 (1M context) --- tests/test_integration_goals.py | 292 +++++++++++++++++++++++ tests/test_integration_headlines.py | 328 ++++++++++++++++++++++++++ tests/test_integration_issues.py | 329 ++++++++++++++++++++++++++ tests/test_integration_meetings.py | 279 ++++++++++++++++++++++ tests/test_integration_scorecard.py | 214 +++++++++++++++++ tests/test_integration_todos.py | 226 ++++++++++++++++++ tests/test_integration_users.py | 345 ++++++++++++++++++++++++++++ 7 files changed, 2013 insertions(+) create mode 100644 tests/test_integration_goals.py create mode 100644 tests/test_integration_headlines.py create mode 100644 tests/test_integration_issues.py create mode 100644 tests/test_integration_meetings.py create mode 100644 tests/test_integration_scorecard.py create mode 100644 tests/test_integration_todos.py create mode 100644 tests/test_integration_users.py 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 From cdf1114f86584d74476e82149e5d4f0d6ac34978 Mon Sep 17 00:00:00 2001 From: Franccesco Orozco Date: Fri, 20 Mar 2026 10:43:44 -0600 Subject: [PATCH 07/10] test: add adversarial tests for edge cases and malformed data Add 185 adversarial tests covering edge cases: zero IDs bypassing truthiness guards, malformed API responses, missing/None fields in transforms, empty Origins arrays, whitespace-only API keys, and Pydantic model coercion boundaries. Co-Authored-By: Claude Opus 4.6 (1M context) --- tests/test_adversarial_issues_headlines.py | 701 +++++++++++++++++ tests/test_adversarial_meetings.py | 441 +++++++++++ tests/test_adversarial_todos_goals.py | 667 ++++++++++++++++ tests/test_adversarial_users.py | 875 +++++++++++++++++++++ 4 files changed, 2684 insertions(+) create mode 100644 tests/test_adversarial_issues_headlines.py create mode 100644 tests/test_adversarial_meetings.py create mode 100644 tests/test_adversarial_todos_goals.py create mode 100644 tests/test_adversarial_users.py 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] From 24737a8082d07b1cc4e48c11c7764fdffd3f23d2 Mon Sep 17 00:00:00 2001 From: Franccesco Orozco Date: Fri, 20 Mar 2026 10:43:54 -0600 Subject: [PATCH 08/10] chore: register integration pytest marker in pyproject.toml Co-Authored-By: Claude Opus 4.6 (1M context) --- pyproject.toml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pyproject.toml b/pyproject.toml index 6f6cb90..91fb247 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -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 = [ From ec4fe5b50b23d5e9fce6d788b5586f92140c483f Mon Sep 17 00:00:00 2001 From: Franccesco Orozco Date: Fri, 20 Mar 2026 10:44:08 -0600 Subject: [PATCH 09/10] chore(release): bump version to 0.21.1 Co-Authored-By: Claude Opus 4.6 (1M context) --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 91fb247..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" }] From a5e35c24c1a4126a380359bf0712da3b8d226e90 Mon Sep 17 00:00:00 2001 From: Franccesco Orozco Date: Fri, 20 Mar 2026 11:14:33 -0600 Subject: [PATCH 10/10] fix(ci): skip integration tests in CI and fix pyright type error Exclude integration tests from CI runs with -m "not integration" since they require BG_API_KEY. Fix pyright type error in meetings transform by extracting Owner dict to a typed local variable. Co-Authored-By: Claude Opus 4.6 (1M context) --- .github/workflows/tests.yml | 2 +- .../operations/mixins/meetings_transform.py | 33 ++++++++++--------- 2 files changed, 19 insertions(+), 16 deletions(-) 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/src/bloomy/operations/mixins/meetings_transform.py b/src/bloomy/operations/mixins/meetings_transform.py index 5873f69..db5c727 100644 --- a/src/bloomy/operations/mixins/meetings_transform.py +++ b/src/bloomy/operations/mixins/meetings_transform.py @@ -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") or {}).get("Id", 0), - OwnerName=(issue.get("Owner") or {}).get("Name", ""), - OwnerImageUrl=(issue.get("Owner") or {}).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