diff --git a/nextcloud_mcp_server/client/contacts.py b/nextcloud_mcp_server/client/contacts.py index 7ba0e93..8f959f6 100644 --- a/nextcloud_mcp_server/client/contacts.py +++ b/nextcloud_mcp_server/client/contacts.py @@ -274,6 +274,7 @@ class ContactsClient(BaseNextcloudClient): "nickname": contact.nickname, "birthday": contact.bday, "email": contact.email, + "tel": contact.tel, }, "addressdata": addressdata, } diff --git a/nextcloud_mcp_server/server/contacts.py b/nextcloud_mcp_server/server/contacts.py index 7618988..a5e91da 100644 --- a/nextcloud_mcp_server/server/contacts.py +++ b/nextcloud_mcp_server/server/contacts.py @@ -18,23 +18,60 @@ from nextcloud_mcp_server.observability.metrics import instrument_tool logger = logging.getLogger(__name__) +def _parse_vcard_fields( + raw_values: str | dict | list | None, field_type: str +) -> list[ContactField]: + """Parse polymorphic vCard field data into a list of ContactField. + + pythonvCard4 returns field values in several shapes: + - ``str`` – plain value, e.g. ``"alice@example.com"`` + - ``dict`` – ``{'value': '...', 'type': ['HOME', 'PREF']}`` + - ``list`` – a list whose items are any of the above + + The ``PREF`` type parameter is treated as a *preferred* flag rather than a + label. All other type values are lowercased and joined with ``", "``. + """ + if raw_values is None: + return [] + + items: list[str | dict] = ( + raw_values if isinstance(raw_values, list) else [raw_values] + ) + + fields: list[ContactField] = [] + for item in items: + if isinstance(item, dict): + value = str(item.get("value", "")) + if not value: + continue + raw_types: list[str] = item.get("type") or [] + preferred = any(t.upper() == "PREF" for t in raw_types) + labels = [t.lower() for t in raw_types if t.upper() != "PREF"] + fields.append( + ContactField( + type=field_type, + value=value, + label=", ".join(labels) if labels else None, + preferred=preferred, + ) + ) + elif isinstance(item, str) and item: + fields.append(ContactField(type=field_type, value=item)) + + return fields + + def _raw_contact_to_model(raw: dict) -> Contact: """Convert a raw contact dict from the contacts client to a Contact model. - Only maps fields the client's list_contacts() currently returns: - fullname, nickname, birthday, and email. Additional Contact model fields - (phones, addresses, organization, etc.) require expanding the client's - vCard parsing in ContactsClient.list_contacts(). + Maps fullname, nickname, birthday, email, and tel fields. + Email/tel values may be plain strings, dicts with ``value``/``type`` keys, + or lists of either – see :func:`_parse_vcard_fields`. """ contact_info = raw.get("contact", {}) - # 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)] + emails = _parse_vcard_fields(contact_info.get("email"), "email") + phones = _parse_vcard_fields(contact_info.get("tel"), "phone") # Nickname goes into custom_fields (no dedicated model field) custom_fields: dict[str, Any] = {} @@ -48,6 +85,7 @@ def _raw_contact_to_model(raw: dict) -> Contact: etag=raw.get("getetag"), birthday=contact_info.get("birthday"), emails=emails, + phones=phones, custom_fields=custom_fields, ) @@ -87,7 +125,13 @@ def configure_contacts_tools(mcp: FastMCP): async def nc_contacts_list_contacts( ctx: Context, *, addressbook: str ) -> ListContactsResponse: - """List all contacts in the specified addressbook.""" + """List all contacts in the specified addressbook. + + Args: + addressbook: The URI slug of the addressbook (e.g. "contacts"), + not the display name. Use nc_contacts_list_addressbooks to + find available URI slugs. + """ client = await get_client(ctx) contacts_data = await client.contacts.list_contacts(addressbook=addressbook) contacts = [_raw_contact_to_model(c) for c in contacts_data] @@ -140,7 +184,9 @@ def configure_contacts_tools(mcp: FastMCP): """Create a new contact. Args: - addressbook: The name of the addressbook to create the contact in. + addressbook: The URI slug of the addressbook (e.g. "contacts"), + not the display name. Use nc_contacts_list_addressbooks to + find available URI slugs. uid: The unique ID for the contact. contact_data: A dictionary with the contact's details, e.g. {"fn": "John Doe", "email": "john.doe@example.com"}. """ @@ -158,7 +204,14 @@ def configure_contacts_tools(mcp: FastMCP): @require_scopes("contacts:write") @instrument_tool async def nc_contacts_delete_contact(ctx: Context, *, addressbook: str, uid: str): - """Delete a contact.""" + """Delete a contact. + + Args: + addressbook: The URI slug of the addressbook (e.g. "contacts"), + not the display name. Use nc_contacts_list_addressbooks to + find available URI slugs. + uid: The unique ID of the contact to delete. + """ client = await get_client(ctx) return await client.contacts.delete_contact(addressbook=addressbook, uid=uid) @@ -174,7 +227,9 @@ def configure_contacts_tools(mcp: FastMCP): """Update an existing contact while preserving all existing properties. Args: - addressbook: The name of the addressbook containing the contact. + addressbook: The URI slug of the addressbook (e.g. "contacts"), + not the display name. Use nc_contacts_list_addressbooks to + find available URI slugs. uid: The unique ID of the contact to update. contact_data: A dictionary with the contact's updated details, e.g. {"fn": "Jane Doe", "email": "jane.doe@example.com"}. etag: Optional ETag for optimistic concurrency control. diff --git a/tests/unit/test_response_models.py b/tests/unit/test_response_models.py index 56c781e..da0fa38 100644 --- a/tests/unit/test_response_models.py +++ b/tests/unit/test_response_models.py @@ -376,6 +376,153 @@ def test_list_contacts_response_wraps_contacts(): assert c["custom_fields"]["nickname"] == "Ali" +@pytest.mark.unit +def test_contact_mapping_dict_format_emails(): + """Regression for #601: pythonvCard4 returns dicts, not plain strings.""" + raw_contact = { + "vcard_id": "dict-email-1", + "contact": { + "fullname": "Evrim Yilmaz", + "email": [ + {"value": "evrim@example.com", "type": ["HOME"]}, + {"value": "evrim@work.com", "type": ["WORK"]}, + ], + }, + } + + contact = _map_contact(raw_contact) + + assert len(contact.emails) == 2 + assert contact.emails[0].value == "evrim@example.com" + assert contact.emails[0].label == "home" + assert contact.emails[1].value == "evrim@work.com" + assert contact.emails[1].label == "work" + + +@pytest.mark.unit +def test_contact_mapping_dict_format_phones(): + """Phones from dict-format tel field are parsed into Contact.phones.""" + raw_contact = { + "vcard_id": "dict-tel-1", + "contact": { + "fullname": "Phone User", + "tel": [ + {"value": "+1-555-0100", "type": ["CELL"]}, + {"value": "+1-555-0200", "type": ["WORK", "VOICE"]}, + ], + }, + } + + contact = _map_contact(raw_contact) + + assert len(contact.phones) == 2 + assert contact.phones[0].value == "+1-555-0100" + assert contact.phones[0].type == "phone" + assert contact.phones[0].label == "cell" + assert contact.phones[1].value == "+1-555-0200" + assert contact.phones[1].label == "work, voice" + + +@pytest.mark.unit +def test_contact_mapping_pref_flag_extraction(): + """PREF type is extracted as preferred=True, not included in labels.""" + raw_contact = { + "vcard_id": "pref-1", + "contact": { + "fullname": "Pref User", + "email": [ + {"value": "pref@example.com", "type": ["HOME", "PREF"]}, + {"value": "other@example.com", "type": ["WORK"]}, + ], + "tel": [ + {"value": "+1-555-0001", "type": ["pref", "CELL"]}, + ], + }, + } + + contact = _map_contact(raw_contact) + + assert contact.emails[0].preferred is True + assert contact.emails[0].label == "home" # PREF stripped from label + assert contact.emails[1].preferred is False + assert contact.primary_email == "pref@example.com" + + assert contact.phones[0].preferred is True + assert contact.phones[0].label == "cell" + assert contact.primary_phone == "+1-555-0001" + + +@pytest.mark.unit +def test_contact_mapping_backward_compat_plain_strings(): + """Plain string emails/phones still work (backward compatibility).""" + raw_contact = { + "vcard_id": "compat-1", + "contact": { + "fullname": "Plain String", + "email": "plain@example.com", + "tel": "+1-555-9999", + }, + } + + contact = _map_contact(raw_contact) + + assert len(contact.emails) == 1 + assert contact.emails[0].value == "plain@example.com" + assert contact.emails[0].label is None + assert contact.emails[0].preferred is False + + assert len(contact.phones) == 1 + assert contact.phones[0].value == "+1-555-9999" + + +@pytest.mark.unit +def test_contact_mapping_empty_type_list(): + """Dict with empty or missing type list produces no label.""" + raw_contact = { + "vcard_id": "empty-type-1", + "contact": { + "fullname": "No Type", + "email": {"value": "notype@example.com", "type": []}, + }, + } + + contact = _map_contact(raw_contact) + + assert len(contact.emails) == 1 + assert contact.emails[0].value == "notype@example.com" + assert contact.emails[0].label is None + assert contact.emails[0].preferred is False + + +@pytest.mark.unit +def test_contact_mapping_multiple_dict_emails_with_labels(): + """Multiple dict-format emails preserve individual labels.""" + raw_contact = { + "vcard_id": "multi-label-1", + "contact": { + "fullname": "Multi Label", + "email": [ + {"value": "home@example.com", "type": ["HOME", "PREF"]}, + {"value": "work@example.com", "type": ["WORK"]}, + {"value": "other@example.com"}, + ], + }, + } + + contact = _map_contact(raw_contact) + + assert len(contact.emails) == 3 + assert contact.emails[0].value == "home@example.com" + assert contact.emails[0].label == "home" + assert contact.emails[0].preferred is True + assert contact.emails[1].value == "work@example.com" + assert contact.emails[1].label == "work" + assert contact.emails[1].preferred is False + assert contact.emails[2].value == "other@example.com" + assert contact.emails[2].label is None + assert contact.primary_email == "home@example.com" + + # ============= _event_dict_to_summary tests =============