Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions backend/app/expenses/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -970,17 +970,19 @@ async def get_friends_balance_summary(self, user_id: str) -> Dict[str, Any]:
if member["userId"] != user_id:
friend_ids.add(member["userId"])

# Get user names
# Get user names & images
users = await self.users_collection.find(
{"_id": {"$in": [ObjectId(uid) for uid in friend_ids]}}
).to_list(None)
user_names = {str(user["_id"]): user.get("name", "Unknown") for user in users}
user_images = {str(user["_id"]): user.get("imageUrl") for user in users}

for friend_id in friend_ids:
friend_balance_data = {
"userId": friend_id,
"userName": user_names.get(friend_id, "Unknown"),
"userImageUrl": None, # Would need to be fetched from user profile
# Populate image directly from users collection to avoid extra client round-trips
"userImageUrl": user_images.get(friend_id),
"netBalance": 0,
"owesYou": False,
"breakdown": [],
Expand Down
51 changes: 46 additions & 5 deletions backend/app/groups/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -312,9 +312,29 @@ async def leave_group(self, group_id: str, user_id: str) -> bool:
detail="Cannot leave group when you are the only admin. Delete the group or promote another member to admin first.",
)

# TODO: Check for outstanding balances with expense service
# For now, we'll allow leaving without balance check
# This should be implemented when expense service is ready
# Block leaving when there are unsettled balances involving this user
try:
pending = await db.settlements.find_one(
{
"groupId": group_id, # settlements store string groupId
"status": "pending",
"$or": [{"payerId": user_id}, {"payeeId": user_id}],
},
{"_id": 1},
)
except Exception as e:
logger.error(
f"Failed to verify unsettled balances for group {group_id}: {e}"
)
raise HTTPException(
status_code=503,
detail="Unable to verify unsettled balances right now. Please try again later.",
)
if pending:
raise HTTPException(
status_code=400,
detail="Cannot leave group with unsettled balances. Please settle up first.",
)

result = await db.groups.update_one(
{"_id": obj_id}, {"$pull": {"members": {"userId": user_id}}}
Expand Down Expand Up @@ -424,8 +444,29 @@ async def remove_member(self, group_id: str, member_id: str, user_id: str) -> bo
detail="Cannot remove yourself. Use leave group instead",
)

# TODO: Check for outstanding balances with expense service
# For now, we'll allow removal without balance check
# Block removal when there are unsettled balances involving the target member
try:
pending = await db.settlements.find_one(
{
"groupId": group_id, # settlements store string groupId
"status": "pending",
"$or": [{"payerId": member_id}, {"payeeId": member_id}],
},
{"_id": 1},
)
except Exception as e:
logger.error(
f"Failed to verify unsettled balances on removal for group {group_id}: {e}"
)
raise HTTPException(
status_code=503,
detail="Unable to verify unsettled balances right now. Please try again later.",
)
if pending:
raise HTTPException(
status_code=400,
detail="Cannot remove member with unsettled balances. Please settle up first.",
)

result = await db.groups.update_one(
{"_id": obj_id}, {"$pull": {"members": {"userId": member_id}}}
Expand Down
263 changes: 263 additions & 0 deletions backend/tests/groups/test_groups_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,125 @@ async def test_join_group_invalid_code(self):
assert exc_info.value.status_code == 404
assert "Invalid join code" in str(exc_info.value.detail)

@pytest.mark.asyncio
async def test_remove_member_blocked_when_unsettled(self):
"""Admin cannot remove member if pending settlements exist"""
mock_db = AsyncMock()
groups = AsyncMock()
settlements = AsyncMock()
mock_db.groups = groups
mock_db.settlements = settlements

group_id = str(ObjectId())
admin_id = "admin123"
member_id = "member456"

groups.find_one.return_value = {
"_id": ObjectId(group_id),
"members": [
{"userId": admin_id, "role": "admin"},
{"userId": member_id, "role": "member"},
],
}
settlements.find_one.return_value = {
"_id": ObjectId(),
"status": "pending",
} # Has pending settlements

with patch.object(self.service, "get_db", return_value=mock_db):
with pytest.raises(HTTPException) as exc:
await self.service.remove_member(group_id, member_id, admin_id)

assert exc.value.status_code == 400
assert "unsettled balances" in str(exc.value.detail)

@pytest.mark.asyncio
async def test_remove_member_allowed_when_settled(self):
"""Admin can remove member when no pending settlements"""
mock_db = AsyncMock()
groups = AsyncMock()
settlements = AsyncMock()
mock_db.groups = groups
mock_db.settlements = settlements

group_id = str(ObjectId())
admin_id = "admin123"
member_id = "member456"

groups.find_one.side_effect = [
{
"_id": ObjectId(group_id),
"members": [
{"userId": admin_id, "role": "admin"},
{"userId": member_id, "role": "member"},
],
}
]
settlements.find_one.return_value = None # No pending settlements
groups.update_one.return_value = MagicMock(modified_count=1)

with patch.object(self.service, "get_db", return_value=mock_db):
ok = await self.service.remove_member(group_id, member_id, admin_id)

assert ok is True

@pytest.mark.asyncio
async def test_leave_group_blocked_when_unsettled(self):
"""Member cannot leave when they have pending settlements"""
mock_db = AsyncMock()
groups = AsyncMock()
settlements = AsyncMock()
mock_db.groups = groups
mock_db.settlements = settlements

group_id = str(ObjectId())
user_id = "user123"

groups.find_one.return_value = {
"_id": ObjectId(group_id),
"members": [
{"userId": user_id, "role": "member"},
{"userId": "other", "role": "admin"},
],
}
settlements.find_one.return_value = {"_id": ObjectId(), "status": "pending"}

with patch.object(self.service, "get_db", return_value=mock_db):
with pytest.raises(HTTPException) as exc:
await self.service.leave_group(group_id, user_id)

assert exc.value.status_code == 400
assert "unsettled balances" in str(exc.value.detail)

@pytest.mark.asyncio
async def test_leave_group_allowed_when_settled(self):
"""Member can leave when no pending settlements and not sole admin"""
mock_db = AsyncMock()
groups = AsyncMock()
settlements = AsyncMock()
mock_db.groups = groups
mock_db.settlements = settlements

group_id = str(ObjectId())
user_id = "user123"

groups.find_one.side_effect = [
{
"_id": ObjectId(group_id),
"members": [
{"userId": user_id, "role": "member"},
{"userId": "admin2", "role": "admin"},
],
}
]
settlements.find_one.return_value = None # No pending settlements
groups.update_one.return_value = MagicMock(modified_count=1)

with patch.object(self.service, "get_db", return_value=mock_db):
ok = await self.service.leave_group(group_id, user_id)

assert ok is True

@pytest.mark.asyncio
async def test_join_group_already_member(self):
"""Test joining group when already a member"""
Expand Down Expand Up @@ -411,7 +530,9 @@ async def test_leave_group_allow_member_to_leave(self):
"""Test allowing regular members to leave"""
mock_db = AsyncMock()
mock_collection = AsyncMock()
mock_settlements = AsyncMock()
mock_db.groups = mock_collection
mock_db.settlements = mock_settlements

group = {
"_id": ObjectId("642f1e4a9b3c2d1f6a1b2c3d"),
Expand All @@ -431,6 +552,7 @@ async def test_leave_group_allow_member_to_leave(self):
}

mock_collection.find_one.return_value = group
mock_settlements.find_one.return_value = None # No pending settlements
mock_result = MagicMock()
mock_result.modified_count = 1
mock_collection.update_one.return_value = mock_result
Expand Down Expand Up @@ -477,3 +599,144 @@ def test_transform_group_document_partial_input(self):
assert result["name"] == "Partial Group"
assert result["currency"] == "USD" # default fallback
assert result["members"] == [] # default fallback

# --- New tests for unsettled balance checks & exception handling (coverage additions) ---
@pytest.mark.asyncio
async def test_leave_group_pending_settlement_blocks(self):
"""Member can't leave when a pending settlement exists (covers pending branch)."""
mock_db = AsyncMock()
groups = AsyncMock()
settlements = AsyncMock()
mock_db.groups = groups
mock_db.settlements = settlements

group = {
"_id": ObjectId("642f1e4a9b3c2d1f6a1b2c3d"),
"name": "Test Group",
"members": [
{
"userId": "admin1",
"role": "admin",
"joinedAt": "2023-01-01T00:00:00Z",
},
{
"userId": "member1",
"role": "member",
"joinedAt": "2023-01-01T00:00:00Z",
},
],
}
groups.find_one.return_value = group
settlements.find_one.return_value = {"_id": ObjectId()}

with patch.object(self.service, "get_db", return_value=mock_db):
with pytest.raises(HTTPException) as exc:
await self.service.leave_group(str(group["_id"]), "member1")

assert exc.value.status_code == 400
assert "Cannot leave group with unsettled balances" in exc.value.detail

@pytest.mark.asyncio
async def test_leave_group_settlement_lookup_failure(self):
"""Service returns 503 when settlement lookup errors (covers except block)."""
mock_db = AsyncMock()
groups = AsyncMock()
settlements = AsyncMock()
mock_db.groups = groups
mock_db.settlements = settlements

group = {
"_id": ObjectId("642f1e4a9b3c2d1f6a1b2c3d"),
"name": "Test Group",
"members": [
{
"userId": "admin1",
"role": "admin",
"joinedAt": "2023-01-01T00:00:00Z",
},
{
"userId": "member1",
"role": "member",
"joinedAt": "2023-01-01T00:00:00Z",
},
],
}
groups.find_one.return_value = group
settlements.find_one.side_effect = Exception("db down")

with patch.object(self.service, "get_db", return_value=mock_db):
with pytest.raises(HTTPException) as exc:
await self.service.leave_group(str(group["_id"]), "member1")

assert exc.value.status_code == 503
assert "Unable to verify unsettled balances" in exc.value.detail

@pytest.mark.asyncio
async def test_remove_member_pending_settlement_blocks(self):
"""Admin can't remove member with pending settlement (covers pending branch)."""
mock_db = AsyncMock()
groups = AsyncMock()
settlements = AsyncMock()
mock_db.groups = groups
mock_db.settlements = settlements

group = {
"_id": ObjectId("642f1e4a9b3c2d1f6a1b2c3d"),
"name": "Test Group",
"members": [
{
"userId": "admin1",
"role": "admin",
"joinedAt": "2023-01-01T00:00:00Z",
},
{
"userId": "member1",
"role": "member",
"joinedAt": "2023-01-01T00:00:00Z",
},
],
}
groups.find_one.return_value = group # Admin check passes
settlements.find_one.return_value = {"_id": ObjectId()}

with patch.object(self.service, "get_db", return_value=mock_db):
with pytest.raises(HTTPException) as exc:
await self.service.remove_member(str(group["_id"]), "member1", "admin1")

assert exc.value.status_code == 400
assert "Cannot remove member with unsettled balances" in exc.value.detail

@pytest.mark.asyncio
async def test_remove_member_settlement_lookup_failure(self):
"""Service returns 503 when settlement lookup fails during removal (covers except block)."""
mock_db = AsyncMock()
groups = AsyncMock()
settlements = AsyncMock()
mock_db.groups = groups
mock_db.settlements = settlements

group = {
"_id": ObjectId("642f1e4a9b3c2d1f6a1b2c3d"),
"name": "Test Group",
"members": [
{
"userId": "admin1",
"role": "admin",
"joinedAt": "2023-01-01T00:00:00Z",
},
{
"userId": "member1",
"role": "member",
"joinedAt": "2023-01-01T00:00:00Z",
},
],
}
groups.find_one.return_value = group # Admin check passes
settlements.find_one.side_effect = Exception("db error")

with patch.object(self.service, "get_db", return_value=mock_db):
with pytest.raises(HTTPException) as exc:
await self.service.remove_member(str(group["_id"]), "member1", "admin1")

assert exc.value.status_code == 503
assert "Unable to verify unsettled balances" in exc.value.detail
Loading
Loading