fix: correct send_message and update_event return contracts (closes #238, closes #239)

spond/spond.py

@@ -395,35 +395,31 @@ async def send_message(
         Returns
         -------
         JSONDict
-            On success: the Spond API response for the send operation.
-
-        Notes
-        -----
-        Has two known bugs tracked in #238:
-
-        - When `chat_id` is provided, the call to `_continue_chat()` is
-          missing `await`, so the returned value is a coroutine rather than
-          the API response.
-        - When `user` and `group_uid` are inconsistent, the method returns a
-          sentinel dict `{"error": "..."}` rather than raising, and an
-          unreachable branch returns `False` despite the `JSONDict`
-          annotation.
+            The Spond API response for the send operation.
+
+        Raises
+        ------
+        ValueError
+            Neither `chat_id` nor both of `user`/`group_uid` were supplied —
+            the call has no way to identify the target chat.
+        KeyError
+            `user` was given but doesn't match any member or guardian in any
+            of the authenticated user's groups (propagated from
+            `get_person`).
         """
         if self._auth is None:
             await self._login_chat()
 
         if chat_id is not None:
-            return self._continue_chat(chat_id, text)
+            return await self._continue_chat(chat_id, text)
         if group_uid is None or user is None:
-            return {
-                "error": "wrong usage, group_id and user_id needed or continue chat with chat_id"
-            }
+            raise ValueError(
+                "send_message requires either chat_id (to continue an existing "
+                "chat) or both user and group_uid (to start a new one)."
+            )
 
         user_obj = await self.get_person(user)
-        if user_obj:
-            user_uid = user_obj["profile"]["id"]
-        else:
-            return False
+        user_uid = user_obj["profile"]["id"]
         url = f"{self._chat_url}/messages"
         data = {
             "text": text,
@@ -571,7 +567,7 @@ async def get_event(self, uid: str) -> JSONDict:
         return await self._get_entity(self._EVENT, uid)
 
     @_SpondBase.require_authentication
-    async def update_event(self, uid: str, updates: JSONDict) -> list[JSONDict] | None:
+    async def update_event(self, uid: str, updates: JSONDict) -> JSONDict:
         """Update an existing event by merging changes into the current state.
 
         The implementation fetches the event via `_get_entity()`, copies the
@@ -593,22 +589,9 @@ async def update_event(self, uid: str, updates: JSONDict) -> list[JSONDict] | No
 
         Returns
         -------
-        list[JSONDict] or None
-            Currently returns `self.events` (the cached events list, or
-            `None` if no `get_events()` call has populated it). This is a
-            bug — see Notes.
-
-        Notes
-        -----
-        Known bug #239: the return value should be the Spond API response
-        from the POST, not the cached events list. The API response *is*
-        captured on `self.events_update`, so the data is still accessible
-        as a workaround until #239 is fixed:
-
-        ```python
-        await s.update_event(uid, {"description": "..."})
-        result = s.events_update  # the actual API response
-        ```
+        JSONDict
+            The Spond API response from the POST — the updated event as
+            persisted server-side.
         """
         event = await self._get_entity(self._EVENT, uid)
         url = f"{self.api_url}sponds/{uid}"
@@ -623,8 +606,7 @@ async def update_event(self, uid: str, updates: JSONDict) -> list[JSONDict] | No
         async with self.clientsession.post(
             url, json=base_event, headers=self.auth_headers
         ) as r:
-            self.events_update = await r.json()
-            return self.events
+            return await r.json()
 
     @_SpondBase.require_authentication
     async def get_event_attendance_xlsx(self, uid: str) -> bytes:

tests/test_spond.py

@@ -113,6 +113,32 @@ async def test_get_event__no_events_available_raises_keyerror(
         with pytest.raises(KeyError):
             await s.get_event("ID1")
 
+    @pytest.mark.asyncio
+    @patch("aiohttp.ClientSession.post")
+    async def test_update_event__returns_api_response(
+        self, mock_post, mock_token
+    ) -> None:
+        """`update_event()` should return the POST response, not the cached
+        events list (regression test for #239)."""
+        s = Spond(MOCK_USERNAME, MOCK_PASSWORD)
+        s.token = mock_token
+        s.events = [{"id": "ID1", "heading": "Old"}]  # cached event for _get_entity
+
+        api_response = {
+            "id": "ID1",
+            "heading": "New",
+            "updated": "2026-05-14T13:00:00Z",
+        }
+        mock_post.return_value.__aenter__.return_value.json = AsyncMock(
+            return_value=api_response
+        )
+
+        result = await s.update_event(uid="ID1", updates={"heading": "New"})
+
+        assert result == api_response
+        # The cached events list should NOT be what we returned.
+        assert result is not s.events
+
     @pytest.mark.asyncio
     @patch("aiohttp.ClientSession.put")
     async def test_change_response(self, mock_put, mock_payload, mock_token) -> None:
@@ -219,6 +245,54 @@ async def test_get_group__no_groups_available_raises_keyerror(
             await s.get_group("ID1")
 
 
+class TestSendMessage:
+    """Tests for `Spond.send_message()` — covers the fixes in #238."""
+
+    @pytest.mark.asyncio
+    @patch("aiohttp.ClientSession.post", new_callable=AsyncMock)
+    async def test_send_message__continues_chat_when_chat_id_given(
+        self, mock_post, mock_token
+    ) -> None:
+        """With `chat_id`, the call should route through `_continue_chat()`
+        and properly await it (regression: the await was missing)."""
+        s = Spond(MOCK_USERNAME, MOCK_PASSWORD)
+        s.token = mock_token
+        s._auth = "MOCK_CHAT_AUTH"
+        s._chat_url = "https://chat.example.invalid"
+
+        api_response = {"ok": True, "messageId": "MID1"}
+        # _continue_chat does `r = await session.post(...)` (no `async with`),
+        # so the post mock must be AsyncMock and r.json must be AsyncMock too.
+        mock_post.return_value.json = AsyncMock(return_value=api_response)
+
+        result = await s.send_message(text="hello", chat_id="CHAT1")
+
+        assert result == api_response  # was a coroutine before the fix
+        mock_post.assert_called_once()
+        _, kwargs = mock_post.call_args
+        assert kwargs["json"] == {"chatId": "CHAT1", "text": "hello", "type": "TEXT"}
+
+    @pytest.mark.asyncio
+    async def test_send_message__missing_args_raises_valueerror(
+        self, mock_token
+    ) -> None:
+        """Without `chat_id` and without both `user` and `group_uid`, the
+        call should raise rather than silently return a sentinel dict."""
+        s = Spond(MOCK_USERNAME, MOCK_PASSWORD)
+        s.token = mock_token
+        s._auth = "MOCK_CHAT_AUTH"
+        s._chat_url = "https://chat.example.invalid"
+
+        with pytest.raises(ValueError, match="chat_id"):
+            await s.send_message(text="hello")
+
+        with pytest.raises(ValueError, match="user and group_uid"):
+            await s.send_message(text="hello", user="USER1")
+
+        with pytest.raises(ValueError, match="user and group_uid"):
+            await s.send_message(text="hello", group_uid="GROUP1")
+
+
 class TestExportMethod:
     @pytest.mark.asyncio
     @patch("aiohttp.ClientSession.get")