fix: address PR #574 review comments

Restore contact email/birthday/nickname data and per-event calendar
source that were silently dropped during response model wrapping.
Remove dead elif branches in OAuth deck tests, add regression tests.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

nextcloud_mcp_server/models/calendar.py

@@ -34,6 +34,12 @@ class CalendarEventSummary(BaseModel):
     status: Optional[str] = Field(
         None, description="Event status (CONFIRMED, TENTATIVE, CANCELLED)"
     )
+    calendar_name: Optional[str] = Field(
+        None, description="Calendar containing this event"
+    )
+    calendar_display_name: Optional[str] = Field(
+        None, description="Display name of calendar containing this event"
+    )
 
 
 class CalendarEvent(CalendarEventSummary):

nextcloud_mcp_server/server/calendar.py

@@ -39,6 +39,8 @@ def _event_dict_to_summary(event: dict) -> CalendarEventSummary:
         description=event.get("description") or None,
         categories=categories,
         status=event.get("status"),
+        calendar_name=event.get("calendar_name"),
+        calendar_display_name=event.get("calendar_display_name"),
     )
 
 
@@ -470,6 +472,9 @@ def configure_calendar_tools(mcp: FastMCP):
                         end_datetime=end_datetime,
                         limit=limit,
                     )
+                    for event in cal_events:
+                        event["calendar_name"] = calendar["name"]
+                        event["calendar_display_name"] = calendar["display_name"]
                     all_events.extend(cal_events)
                 except Exception as e:
                     logger.warning(

nextcloud_mcp_server/server/contacts.py

@@ -8,6 +8,7 @@ from nextcloud_mcp_server.context import get_client
 from nextcloud_mcp_server.models.contacts import (
     AddressBook,
     Contact,
+    ContactField,
     ListAddressBooksResponse,
     ListContactsResponse,
 )
@@ -52,14 +53,34 @@ def configure_contacts_tools(mcp: FastMCP):
         """List all contacts in the specified addressbook."""
         client = await get_client(ctx)
         contacts_data = await client.contacts.list_contacts(addressbook=addressbook)
-        contacts = [
+        contacts = []
-            Contact(
+        for c in contacts_data:
-                uid=c["vcard_id"],
+            contact_info = c.get("contact", {})
-                fn=c.get("contact", {}).get("fullname", ""),
+
-                etag=c.get("getetag"),
+            # Convert email field (str, list, or None) to list[ContactField]
+            raw_email = contact_info.get("email")
+            emails: list[ContactField] = []
+            if isinstance(raw_email, list):
+                emails = [ContactField(type="email", value=e) for e in raw_email if e]
+            elif isinstance(raw_email, str) and raw_email:
+                emails = [ContactField(type="email", value=raw_email)]
+
+            # Nickname goes into custom_fields (no dedicated model field)
+            custom_fields: dict[str, str] = {}
+            nickname = contact_info.get("nickname")
+            if nickname:
+                custom_fields["nickname"] = nickname
+
+            contacts.append(
+                Contact(
+                    uid=c["vcard_id"],
+                    fn=contact_info.get("fullname", ""),
+                    etag=c.get("getetag"),
+                    birthday=contact_info.get("birthday"),
+                    emails=emails,
+                    custom_fields=custom_fields,
+                )
             )
-            for c in contacts_data
-        ]
         return ListContactsResponse(
             contacts=contacts, addressbook=addressbook, total_count=len(contacts)
         )

tests/server/oauth/test_oauth_deck_permissions.py

@@ -79,12 +79,9 @@ async def test_deck_board_view_permissions(
         if not result.isError:
             response_data = json.loads(result.content[0].text)
             # Response is a ListBoardsResponse with a "boards" field
-            if isinstance(response_data, dict) and "boards" in response_data:
+            board_list = response_data.get("boards", [])
-                response_data = response_data["boards"]
+            board_ids = [b["id"] for b in board_list]
-            elif not isinstance(response_data, list):
+            logger.info(f"Bob can see {len(board_list)} boards: {board_ids}")
-                response_data = [response_data] if response_data else []
-            board_ids = [b["id"] for b in response_data]
-            logger.info(f"Bob can see {len(response_data)} boards: {board_ids}")
 
             # Bob should see the shared board
             if board_id in board_ids:
@@ -101,12 +98,9 @@ async def test_deck_board_view_permissions(
         if not result.isError:
             response_data = json.loads(result.content[0].text)
             # Response is a ListBoardsResponse with a "boards" field
-            if isinstance(response_data, dict) and "boards" in response_data:
+            board_list = response_data.get("boards", [])
-                response_data = response_data["boards"]
+            board_ids = [b["id"] for b in board_list]
-            elif not isinstance(response_data, list):
+            logger.info(f"Diana can see {len(board_list)} boards")
-                response_data = [response_data] if response_data else []
-            board_ids = [b["id"] for b in response_data]
-            logger.info(f"Diana can see {len(response_data)} boards")
 
             # Diana should NOT see the board
             assert board_id not in board_ids, "Diana should not see board without ACL"
@@ -318,11 +312,8 @@ async def test_deck_user_isolation(nc_client, alice_mcp_client, bob_mcp_client):
         if not result.isError:
             response_data = json.loads(result.content[0].text)
             # Response is a ListBoardsResponse with a "boards" field
-            if isinstance(response_data, dict) and "boards" in response_data:
+            board_list = response_data.get("boards", [])
-                response_data = response_data["boards"]
+            board_ids = [b["id"] for b in board_list]
-            elif not isinstance(response_data, list):
-                response_data = [response_data] if response_data else []
-            board_ids = [b["id"] for b in response_data]
             logger.info(f"Alice can see boards: {board_ids}")
 
             # Alice should NOT see Bob's board
@@ -339,11 +330,8 @@ async def test_deck_user_isolation(nc_client, alice_mcp_client, bob_mcp_client):
         if not result.isError:
             response_data = json.loads(result.content[0].text)
             # Response is a ListBoardsResponse with a "boards" field
-            if isinstance(response_data, dict) and "boards" in response_data:
+            board_list = response_data.get("boards", [])
-                response_data = response_data["boards"]
+            board_ids = [b["id"] for b in board_list]
-            elif not isinstance(response_data, list):
-                response_data = [response_data] if response_data else []
-            board_ids = [b["id"] for b in response_data]
             logger.info(f"Bob can see boards: {board_ids}")
 
             # Bob should NOT see Alice's board

tests/unit/test_response_models.py

@@ -2,6 +2,11 @@
 
 import pytest
 
+from nextcloud_mcp_server.models.contacts import (
+    Contact,
+    ContactField,
+    ListContactsResponse,
+)
 from nextcloud_mcp_server.models.notes import (
     CreateNoteResponse,
     Note,
@@ -267,3 +272,125 @@ def test_sampling_search_response_serialization():
     assert data["model_used"] == "claude-3-5-sonnet"
     assert data["stop_reason"] == "maxTokens"
     assert data["success"] is True
+
+
+def _map_contact(raw: dict) -> Contact:
+    """Replicate the mapping logic from server/contacts.py for testing."""
+    contact_info = raw.get("contact", {})
+
+    raw_email = contact_info.get("email")
+    emails: list[ContactField] = []
+    if isinstance(raw_email, list):
+        emails = [ContactField(type="email", value=e) for e in raw_email if e]
+    elif isinstance(raw_email, str) and raw_email:
+        emails = [ContactField(type="email", value=raw_email)]
+
+    custom_fields: dict[str, str] = {}
+    nickname = contact_info.get("nickname")
+    if nickname:
+        custom_fields["nickname"] = nickname
+
+    return Contact(
+        uid=raw["vcard_id"],
+        fn=contact_info.get("fullname", ""),
+        etag=raw.get("getetag"),
+        birthday=contact_info.get("birthday"),
+        emails=emails,
+        custom_fields=custom_fields,
+    )
+
+
+@pytest.mark.unit
+def test_contact_mapping_preserves_email_birthday_nickname():
+    """Test that list_contacts mapping preserves email, birthday, and nickname.
+
+    Regression test for PR #574: the original mapping only kept uid, fn, etag
+    and silently dropped email, birthday, and nickname.
+    """
+    raw_contact = {
+        "vcard_id": "abc-123",
+        "getetag": '"etag-val"',
+        "contact": {
+            "fullname": "Jane Doe",
+            "email": "jane@example.com",
+            "birthday": "1990-05-15",
+            "nickname": "JD",
+        },
+    }
+
+    contact = _map_contact(raw_contact)
+
+    assert contact.uid == "abc-123"
+    assert contact.fn == "Jane Doe"
+    assert contact.etag == '"etag-val"'
+    assert contact.birthday == "1990-05-15"
+    assert len(contact.emails) == 1
+    assert contact.emails[0].value == "jane@example.com"
+    assert contact.emails[0].type == "email"
+    assert contact.custom_fields["nickname"] == "JD"
+
+
+@pytest.mark.unit
+def test_contact_mapping_multiple_emails():
+    """Test that multiple emails are mapped correctly."""
+    raw_contact = {
+        "vcard_id": "def-456",
+        "contact": {
+            "fullname": "John Smith",
+            "email": ["john@work.com", "john@home.com"],
+        },
+    }
+
+    contact = _map_contact(raw_contact)
+
+    assert len(contact.emails) == 2
+    assert contact.emails[0].value == "john@work.com"
+    assert contact.emails[1].value == "john@home.com"
+
+
+@pytest.mark.unit
+def test_contact_mapping_missing_optional_fields():
+    """Test mapping when email, birthday, and nickname are absent."""
+    raw_contact = {
+        "vcard_id": "ghi-789",
+        "contact": {"fullname": "No Details"},
+    }
+
+    contact = _map_contact(raw_contact)
+
+    assert contact.uid == "ghi-789"
+    assert contact.fn == "No Details"
+    assert contact.birthday is None
+    assert contact.emails == []
+    assert contact.custom_fields == {}
+
+
+@pytest.mark.unit
+def test_list_contacts_response_wraps_contacts():
+    """Test ListContactsResponse wraps contacts correctly for MCP output."""
+    contacts = [
+        _map_contact(
+            {
+                "vcard_id": "a",
+                "getetag": '"e1"',
+                "contact": {
+                    "fullname": "Alice",
+                    "email": "alice@test.com",
+                    "birthday": "2000-01-01",
+                    "nickname": "Ali",
+                },
+            }
+        ),
+    ]
+
+    response = ListContactsResponse(
+        contacts=contacts, addressbook="personal", total_count=1
+    )
+
+    data = response.model_dump()
+    assert data["total_count"] == 1
+    assert len(data["contacts"]) == 1
+    c = data["contacts"][0]
+    assert c["birthday"] == "2000-01-01"
+    assert c["emails"][0]["value"] == "alice@test.com"
+    assert c["custom_fields"]["nickname"] == "Ali"