Skip to content

Commit 5431d1e

Browse files
committed
Cospend: reject empty payed_for; add update_member activated round-trip test
1 parent c56ec86 commit 5431d1e

2 files changed

Lines changed: 74 additions & 3 deletions

File tree

src/nc_mcp_server/tools/cospend.py

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -422,8 +422,10 @@ async def create_cospend_bill(
422422
what: Short description of the expense (e.g. "Pizza").
423423
amount: Total amount paid.
424424
payer: Member id of the person who paid.
425-
payed_for: List of member ids who share the cost. Pass [] only for
426-
a personal expense (rare).
425+
payed_for: Non-empty list of member ids who share the cost.
426+
Cospend requires at least one ower per bill — passing [] raises
427+
ValueError (the server would reject it on create and silently
428+
no-op on update, so we reject it up front for both).
427429
date: Date string "YYYY-MM-DD". Defaults to today (UTC) if both
428430
date and timestamp are omitted; the underlying API requires one.
429431
timestamp: Alternative to date — Unix seconds. If both are set, the
@@ -445,6 +447,8 @@ async def create_cospend_bill(
445447
Returns:
446448
JSON {"bill_id": <int>} — the integer id of the new bill.
447449
"""
450+
if not payed_for:
451+
raise ValueError("payed_for must be a non-empty list of member ids")
448452
client = get_client()
449453
if timestamp is None and date is None:
450454
date = datetime.now(UTC).date().isoformat()
@@ -497,7 +501,10 @@ async def update_cospend_bill(
497501
what: New description.
498502
amount: New amount.
499503
payer: New payer member id.
500-
payed_for: New list of ower member ids (replaces, doesn't merge).
504+
payed_for: New non-empty list of ower member ids (replaces,
505+
doesn't merge). Passing [] raises ValueError — the server
506+
no-ops silently on empty payedFor, which would look like a
507+
successful update but leave owers unchanged.
501508
date: New "YYYY-MM-DD" date.
502509
timestamp: Alternative to date.
503510
comment: New comment.
@@ -513,6 +520,8 @@ async def update_cospend_bill(
513520
Returns:
514521
JSON {"bill_id": <int>} confirming the updated bill id.
515522
"""
523+
if payed_for is not None and not payed_for:
524+
raise ValueError("payed_for must be a non-empty list of member ids")
516525
client = get_client()
517526
body = _body(
518527
what=what,

tests/integration/test_cospend.py

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,40 @@ async def test_update_weight_persists(self, nc_mcp: McpTestHelper) -> None:
188188
)
189189
assert updated["weight"] == 3.5
190190

191+
@pytest.mark.asyncio
192+
async def test_update_activated_false_then_true_round_trip(self, nc_mcp: McpTestHelper) -> None:
193+
"""Soft-disable via update path, then re-enable. Catches the create/update field-name asymmetry
194+
(create uses `active` int, update uses `activated` bool)."""
195+
pid = "mcp-test-mem-toggle"
196+
await _make_project(nc_mcp, pid)
197+
member = await _make_member(nc_mcp, pid, "Toggle")
198+
bob = await _make_member(nc_mcp, pid, "Bob")
199+
# Member must own a bill so update(activated=False) soft-disables instead of deleting
200+
await _make_bill(nc_mcp, pid, "Lunch", 10.0, payer=member["id"], payed_for=[member["id"], bob["id"]])
201+
202+
disabled = json.loads(
203+
await nc_mcp.call(
204+
"update_cospend_member",
205+
project_id=pid,
206+
member_id=member["id"],
207+
activated=False,
208+
)
209+
)
210+
assert disabled is not None, "member with bills should be returned as soft-disabled, not deleted"
211+
assert disabled["activated"] is False
212+
members = json.loads(await nc_mcp.call("list_cospend_members", project_id=pid))
213+
assert next(m for m in members if m["id"] == member["id"])["activated"] is False
214+
215+
re_enabled = json.loads(
216+
await nc_mcp.call(
217+
"update_cospend_member",
218+
project_id=pid,
219+
member_id=member["id"],
220+
activated=True,
221+
)
222+
)
223+
assert re_enabled["activated"] is True
224+
191225
@pytest.mark.asyncio
192226
async def test_delete_removes_member_with_no_bills(self, nc_mcp: McpTestHelper) -> None:
193227
await _make_project(nc_mcp, "mcp-test-mem-del")
@@ -465,6 +499,34 @@ async def test_get_bill_with_bad_id_raises(self, nc_mcp: McpTestHelper) -> None:
465499
with pytest.raises((ToolError, NextcloudError)):
466500
await nc_mcp.call("get_cospend_bill", project_id="mcp-test-bad-bill", bill_id=999_999_999)
467501

502+
@pytest.mark.asyncio
503+
async def test_create_bill_with_empty_payed_for_rejected(self, nc_mcp: McpTestHelper) -> None:
504+
"""payed_for=[] is rejected client-side — server would 400, we fail fast with a clearer message."""
505+
pid = "mcp-test-bill-empty-create"
506+
await _make_project(nc_mcp, pid)
507+
alice = await _make_member(nc_mcp, pid, "Alice")
508+
with pytest.raises(ToolError, match="non-empty"):
509+
await nc_mcp.call(
510+
"create_cospend_bill",
511+
project_id=pid,
512+
what="X",
513+
amount=1.0,
514+
payer=alice["id"],
515+
payed_for=[],
516+
)
517+
518+
@pytest.mark.asyncio
519+
async def test_update_bill_with_empty_payed_for_rejected(self, nc_mcp: McpTestHelper) -> None:
520+
"""payed_for=[] on update is rejected — server would silently no-op (200 OK with owers unchanged),
521+
which would look like a successful update. We reject up front instead."""
522+
pid = "mcp-test-bill-empty-update"
523+
await _make_project(nc_mcp, pid)
524+
alice = await _make_member(nc_mcp, pid, "Alice")
525+
bob = await _make_member(nc_mcp, pid, "Bob")
526+
bill_id = await _make_bill(nc_mcp, pid, "X", 1.0, alice["id"], [alice["id"], bob["id"]])
527+
with pytest.raises(ToolError, match="non-empty"):
528+
await nc_mcp.call("update_cospend_bill", project_id=pid, bill_id=bill_id, payed_for=[])
529+
468530

469531
class TestToolRegistration:
470532
"""Confirm all expected tools are registered (catches accidental drops)."""

0 commit comments

Comments
 (0)