Skip to content

Commit 070321b

Browse files
committed
feat: Improve balance verification on group leave and member removal, update error handling
1 parent a05c920 commit 070321b

File tree

7 files changed

+181
-141
lines changed

7 files changed

+181
-141
lines changed

backend/app/groups/service.py

Lines changed: 22 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -313,21 +313,24 @@ async def leave_group(self, group_id: str, user_id: str) -> bool:
313313
)
314314

315315
# Block leaving when there are unsettled balances involving this user
316-
pending_count = 0
317316
try:
318-
result = await db.settlements.count_documents(
317+
pending = await db.settlements.find_one(
319318
{
320-
"groupId": group_id,
319+
"groupId": group_id, # settlements store string groupId
321320
"status": "pending",
322321
"$or": [{"payerId": user_id}, {"payeeId": user_id}],
323-
}
322+
},
323+
{"_id": 1},
324324
)
325-
pending_count = result if isinstance(result, int) else 0
326325
except Exception as e:
327-
logger.warning(
328-
f"Skipping unsettled check on leave due to error: {e} (defaulting to 0)"
326+
logger.error(
327+
f"Failed to verify unsettled balances for group {group_id}: {e}"
329328
)
330-
if pending_count > 0:
329+
raise HTTPException(
330+
status_code=503,
331+
detail="Unable to verify unsettled balances right now. Please try again later.",
332+
)
333+
if pending:
331334
raise HTTPException(
332335
status_code=400,
333336
detail="Cannot leave group with unsettled balances. Please settle up first.",
@@ -442,21 +445,24 @@ async def remove_member(self, group_id: str, member_id: str, user_id: str) -> bo
442445
)
443446

444447
# Block removal when there are unsettled balances involving the target member
445-
pending_count = 0
446448
try:
447-
result = await db.settlements.count_documents(
449+
pending = await db.settlements.find_one(
448450
{
449-
"groupId": group_id,
451+
"groupId": group_id, # settlements store string groupId
450452
"status": "pending",
451453
"$or": [{"payerId": member_id}, {"payeeId": member_id}],
452-
}
454+
},
455+
{"_id": 1},
453456
)
454-
pending_count = result if isinstance(result, int) else 0
455457
except Exception as e:
456-
logger.warning(
457-
f"Skipping unsettled check on removal due to error: {e} (defaulting to 0)"
458+
logger.error(
459+
f"Failed to verify unsettled balances on removal for group {group_id}: {e}"
460+
)
461+
raise HTTPException(
462+
status_code=503,
463+
detail="Unable to verify unsettled balances right now. Please try again later.",
458464
)
459-
if pending_count > 0:
465+
if pending:
460466
raise HTTPException(
461467
status_code=400,
462468
detail="Cannot remove member with unsettled balances. Please settle up first.",

backend/tests/groups/test_groups_service.py

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,10 @@ async def test_remove_member_blocked_when_unsettled(self):
208208
{"userId": member_id, "role": "member"},
209209
],
210210
}
211-
settlements.count_documents.return_value = 2 # pending exists
211+
settlements.find_one.return_value = {
212+
"_id": ObjectId(),
213+
"status": "pending",
214+
} # Has pending settlements
212215

213216
with patch.object(self.service, "get_db", return_value=mock_db):
214217
with pytest.raises(HTTPException) as exc:
@@ -239,7 +242,7 @@ async def test_remove_member_allowed_when_settled(self):
239242
],
240243
}
241244
]
242-
settlements.count_documents.return_value = 0
245+
settlements.find_one.return_value = None # No pending settlements
243246
groups.update_one.return_value = MagicMock(modified_count=1)
244247

245248
with patch.object(self.service, "get_db", return_value=mock_db):
@@ -266,7 +269,7 @@ async def test_leave_group_blocked_when_unsettled(self):
266269
{"userId": "other", "role": "admin"},
267270
],
268271
}
269-
settlements.count_documents.return_value = 1
272+
settlements.find_one.return_value = {"_id": ObjectId(), "status": "pending"}
270273

271274
with patch.object(self.service, "get_db", return_value=mock_db):
272275
with pytest.raises(HTTPException) as exc:
@@ -296,7 +299,7 @@ async def test_leave_group_allowed_when_settled(self):
296299
],
297300
}
298301
]
299-
settlements.count_documents.return_value = 0
302+
settlements.find_one.return_value = None # No pending settlements
300303
groups.update_one.return_value = MagicMock(modified_count=1)
301304

302305
with patch.object(self.service, "get_db", return_value=mock_db):
@@ -527,7 +530,9 @@ async def test_leave_group_allow_member_to_leave(self):
527530
"""Test allowing regular members to leave"""
528531
mock_db = AsyncMock()
529532
mock_collection = AsyncMock()
533+
mock_settlements = AsyncMock()
530534
mock_db.groups = mock_collection
535+
mock_db.settlements = mock_settlements
531536

532537
group = {
533538
"_id": ObjectId("642f1e4a9b3c2d1f6a1b2c3d"),
@@ -547,6 +552,7 @@ async def test_leave_group_allow_member_to_leave(self):
547552
}
548553

549554
mock_collection.find_one.return_value = group
555+
mock_settlements.find_one.return_value = None # No pending settlements
550556
mock_result = MagicMock()
551557
mock_result.modified_count = 1
552558
mock_collection.update_one.return_value = mock_result

frontend/api/auth.js

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,7 @@ export const signup = (name, email, password) => {
88
return apiClient.post("/auth/signup/email", { name, email, password });
99
};
1010

11-
export const updateUser = (token, userData) => {
12-
return apiClient.patch("/user/", userData);
13-
};
11+
export const updateUser = (userData) => apiClient.patch("/user/", userData);
1412

1513
export const refresh = (refresh_token) => {
1614
return apiClient.post("/auth/refresh", { refresh_token });

frontend/api/client.js

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,8 @@ async function performRefresh() {
6161
return accessToken;
6262
} finally {
6363
isRefreshing = false;
64-
const p = refreshPromise; // preserve for awaiting callers
64+
// allow GC of the completed promise
6565
refreshPromise = null;
66-
return p; // not used
6766
}
6867
})();
6968
return refreshPromise;

frontend/screens/EditProfileScreen.js

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,28 @@
1-
import React, { useState, useContext } from 'react';
2-
import { View, StyleSheet, Alert } from 'react-native';
3-
import { Button, TextInput, Appbar, Title } from 'react-native-paper';
4-
import { AuthContext } from '../context/AuthContext';
5-
import { updateUser } from '../api/auth';
1+
import { useContext, useState } from "react";
2+
import { Alert, StyleSheet, View } from "react-native";
3+
import { Appbar, Button, TextInput, Title } from "react-native-paper";
4+
import { updateUser } from "../api/auth";
5+
import { AuthContext } from "../context/AuthContext";
66

77
const EditProfileScreen = ({ navigation }) => {
88
const { user, token, updateUserInContext } = useContext(AuthContext);
9-
const [name, setName] = useState(user?.name || '');
9+
const [name, setName] = useState(user?.name || "");
1010
const [isSubmitting, setIsSubmitting] = useState(false);
1111

1212
const handleUpdateProfile = async () => {
1313
if (!name) {
14-
Alert.alert('Error', 'Name cannot be empty.');
14+
Alert.alert("Error", "Name cannot be empty.");
1515
return;
1616
}
1717
setIsSubmitting(true);
1818
try {
19-
const response = await updateUser(token, { name });
19+
const response = await updateUser({ name });
2020
updateUserInContext(response.data);
21-
Alert.alert('Success', 'Profile updated successfully.');
21+
Alert.alert("Success", "Profile updated successfully.");
2222
navigation.goBack();
2323
} catch (error) {
24-
console.error('Failed to update profile:', error);
25-
Alert.alert('Error', 'Failed to update profile.');
24+
console.error("Failed to update profile:", error);
25+
Alert.alert("Error", "Failed to update profile.");
2626
} finally {
2727
setIsSubmitting(false);
2828
}

frontend/screens/GroupDetailsScreen.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -250,7 +250,7 @@ const styles = StyleSheet.create({
250250
},
251251
// Settlement Summary Styles
252252
settlementContainer: {
253-
gap: 16,
253+
marginBottom: 16,
254254
},
255255
settledContainer: {
256256
alignItems: "center",

0 commit comments

Comments
 (0)