diff --git a/nextcloud_mcp_server/client/contacts.py b/nextcloud_mcp_server/client/contacts.py index 527af8df..764fd695 100644 --- a/nextcloud_mcp_server/client/contacts.py +++ b/nextcloud_mcp_server/client/contacts.py @@ -482,7 +482,29 @@ class ContactsClient(BaseNextcloudClient): def _merge_vcard_properties( self, raw_vcard: str, contact_data: dict[str, Any], uid: str ) -> str: - """Merge new contact data into existing raw vCard while preserving all properties.""" + """Merge new contact data into existing raw vCard while preserving all properties. + + Limitation: dict / list-form ``email`` and ``tel`` inputs are not applied + by this text-merge path. Existing EMAIL/TEL lines are preserved unchanged, + and no new lines are written for the dict/list inputs. Pass plain strings + to update EMAIL/TEL here, or recreate via ``create_contact`` for full + multi-entry support with TYPE annotations. + """ + # Surface dict/list email/tel up front rather than silently no-op in the + # add-new loop below (where the isinstance(value, str) guard skips them). + for _key in ("email", "tel"): + _value = contact_data.get(_key) + if _value is not None and not isinstance(_value, str): + logger.warning( + "update_contact: %s=%r (dict/list shape) is not applied via " + "the text-merge update path; existing %s lines are preserved " + "unchanged. Use a plain string to update %s here, or recreate " + "via create_contact for multi-entry support.", + _key, + _value, + _key.upper(), + _key.upper(), + ) try: # Instead of using pythonvCard4 which has formatting issues, # let's do a simple text-based merge to preserve exact formatting diff --git a/nextcloud_mcp_server/server/contacts.py b/nextcloud_mcp_server/server/contacts.py index 90935b05..587aa736 100644 --- a/nextcloud_mcp_server/server/contacts.py +++ b/nextcloud_mcp_server/server/contacts.py @@ -250,7 +250,30 @@ def configure_contacts_tools(mcp: FastMCP): 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"}. + contact_data: A dictionary with the contact's updated details. Supported + keys mirror nc_contacts_create_contact: + + - ``fn`` (str): Formatted full name. + - ``email`` (str): Email address. **Update path supports plain + strings only**; dict / list-form inputs are not applied — the + existing EMAIL line is preserved unchanged and a warning is + logged. Use create_contact for multi-entry support with TYPE + annotations. + - ``tel`` / ``phone`` (str): Phone number. Same single-string + limitation as ``email`` above. + - ``org`` / ``organization`` (str or list of str): Organization. + Lists become semicolon-separated ORG components per RFC 6350. + - ``title`` (str): Job title. + - ``note`` (str): Free-form note. + - ``nickname`` (str or list of str). + - ``bday`` (ISO date str ``"YYYY-MM-DD"`` or ``datetime.date``). + Non-ISO strings are rejected with a warning; the existing + BDAY line is preserved. + - ``categories`` (list of str, or comma-separated str). + - ``url`` (str or list of str). Only the first URL is written + on update; multi-URL contacts should use create_contact. + + Example: ``{"fn": "Jane Doe", "email": "jane.doe@example.com"}``. etag: Optional ETag for optimistic concurrency control. """ client = await get_client(ctx) diff --git a/tests/unit/client/test_contacts.py b/tests/unit/client/test_contacts.py index 5784139c..a5deebb0 100644 --- a/tests/unit/client/test_contacts.py +++ b/tests/unit/client/test_contacts.py @@ -368,3 +368,44 @@ class TestMergeVcardProperties: result = self._merge(existing, {"org": ["Acme", "Engineering"]}) assert "ORG:Acme;Engineering" in result assert "[" not in result + + def test_dict_email_on_no_existing_line_warns(self, caplog): + """No existing EMAIL + dict input is a known limitation of the text-merge + path. Surface it as a warning so the silent no-op is at least observable. + """ + existing = "BEGIN:VCARD\nVERSION:3.0\nUID:merge-test\nFN:Alice\nEND:VCARD\n" + with caplog.at_level("WARNING"): + result = self._merge( + existing, {"email": {"value": "alice@work.com", "type": ["WORK"]}} + ) + # The dict input is not applied; no EMAIL line is added. + assert "EMAIL" not in result + # A warning specifically calls out the dict/list shape and recommends + # plain str / create_contact as alternatives. + assert any( + "email" in r.message and "dict/list shape" in r.message + for r in caplog.records + ) + + def test_list_tel_on_no_existing_line_warns(self, caplog): + """Same warning behaviour for TEL — list input on a contact without an + existing TEL line must not silently disappear. + """ + existing = "BEGIN:VCARD\nVERSION:3.0\nUID:merge-test\nFN:Alice\nEND:VCARD\n" + with caplog.at_level("WARNING"): + result = self._merge( + existing, {"tel": [{"value": "555-9999", "type": ["WORK"]}]} + ) + assert "TEL" not in result + assert any( + "tel" in r.message and "dict/list shape" in r.message + for r in caplog.records + ) + + def test_str_email_does_not_warn(self, caplog): + """Plain string email is the supported shape — no warning should fire.""" + existing = "BEGIN:VCARD\nVERSION:3.0\nUID:merge-test\nFN:Alice\nEND:VCARD\n" + with caplog.at_level("WARNING"): + result = self._merge(existing, {"email": "alice@work.com"}) + assert "EMAIL:alice@work.com" in result + assert not any("dict/list shape" in r.message for r in caplog.records)