Skip to content

Commit ee1465c

Browse files
cbcoutinhoclaude
andcommitted
test(contacts): pin NICKNAME/BDAY/CATEGORIES update behaviour in _merge_vcard_properties
PR #719 review raised a claim that these three fields fall through to the "keep unchanged" catch-all in _merge_vcard_properties. The existing elif branches for NICKNAME/BDAY/CATEGORIES already prevent that, but the behaviour wasn't pinned by a test. Add a focused TestMergeVcardProperties class that calls the merge helper directly and asserts: - Existing NICKNAME/BDAY/CATEGORIES lines are overwritten by new values. - When the existing vCard has none of these lines, update adds them. - A URL update doesn't clobber unrelated ORG/NOTE/TEL properties. If the primary update path ever regresses for these fields, these tests will catch it immediately. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 0186b49 commit ee1465c

1 file changed

Lines changed: 63 additions & 0 deletions

File tree

tests/unit/client/test_contacts.py

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,3 +177,66 @@ def test_does_not_mutate_input(self):
177177

178178
def test_passthrough_for_unknown_keys(self):
179179
assert _normalize_contact_data({"foo": "bar"}) == {"foo": "bar"}
180+
181+
182+
class TestMergeVcardProperties:
183+
"""Direct tests for ``_merge_vcard_properties`` — the primary update path.
184+
185+
Written in response to PR #719 review claiming NICKNAME/BDAY/CATEGORIES are not
186+
updatable via this function. These tests pin the actual behaviour so future
187+
regressions (or claims) can be answered in one line.
188+
"""
189+
190+
@staticmethod
191+
def _merge(raw: str, data: dict) -> str:
192+
from nextcloud_mcp_server.client.contacts import ContactsClient
193+
194+
client = ContactsClient.__new__(ContactsClient) # no HTTP / no __init__
195+
return client._merge_vcard_properties(raw, data, uid="merge-test")
196+
197+
def test_nickname_overwrites_existing_line(self):
198+
"""Existing NICKNAME must be replaced with the new value, not preserved."""
199+
existing = "BEGIN:VCARD\nVERSION:3.0\nUID:merge-test\nFN:Alice\nNICKNAME:Bob\nEND:VCARD\n"
200+
result = self._merge(existing, {"nickname": "Robert"})
201+
assert "NICKNAME:Robert" in result
202+
assert "NICKNAME:Bob" not in result
203+
204+
def test_bday_overwrites_existing_line(self):
205+
existing = "BEGIN:VCARD\nVERSION:3.0\nUID:merge-test\nFN:Alice\nBDAY:1990-05-01\nEND:VCARD\n"
206+
result = self._merge(existing, {"bday": "1991-06-02"})
207+
assert "BDAY:1991-06-02" in result
208+
assert "BDAY:1990-05-01" not in result
209+
210+
def test_categories_overwrites_existing_line(self):
211+
existing = "BEGIN:VCARD\nVERSION:3.0\nUID:merge-test\nFN:Alice\nCATEGORIES:old,stale\nEND:VCARD\n"
212+
result = self._merge(existing, {"categories": ["vip", "new"]})
213+
assert "CATEGORIES:vip,new" in result
214+
assert "old,stale" not in result
215+
216+
def test_nickname_added_when_not_in_existing_vcard(self):
217+
"""If the existing vCard has no NICKNAME line, update must append one."""
218+
existing = "BEGIN:VCARD\nVERSION:3.0\nUID:merge-test\nFN:Alice\nEND:VCARD\n"
219+
result = self._merge(existing, {"nickname": "Bob"})
220+
assert "NICKNAME:Bob" in result
221+
222+
def test_bday_added_when_not_in_existing_vcard(self):
223+
existing = "BEGIN:VCARD\nVERSION:3.0\nUID:merge-test\nFN:Alice\nEND:VCARD\n"
224+
result = self._merge(existing, {"bday": "1990-05-01"})
225+
assert "BDAY:1990-05-01" in result
226+
227+
def test_categories_added_when_not_in_existing_vcard(self):
228+
existing = "BEGIN:VCARD\nVERSION:3.0\nUID:merge-test\nFN:Alice\nEND:VCARD\n"
229+
result = self._merge(existing, {"categories": "a,b,c"})
230+
assert "CATEGORIES:a,b,c" in result
231+
232+
def test_url_update_preserves_unrelated_properties(self):
233+
"""A URL update must not clobber ORG / NOTE / TEL from the existing vCard."""
234+
existing = (
235+
"BEGIN:VCARD\nVERSION:3.0\nUID:merge-test\nFN:Alice\n"
236+
"ORG:Acme\nTEL:555-1234\nNOTE:keep me\nEND:VCARD\n"
237+
)
238+
result = self._merge(existing, {"url": "https://example.com"})
239+
assert "URL:https://example.com" in result
240+
assert "ORG:Acme" in result
241+
assert "TEL:555-1234" in result
242+
assert "NOTE:keep me" in result

0 commit comments

Comments
 (0)