Add group settings functionality and enhance group details screen (#128)

* feat: Add group settings functionality and enhance group details screen

* Refactor screens for improved readability and consistency

- Updated import statements to use consistent formatting across GroupDetailsScreen, GroupSettingsScreen, HomeScreen, and JoinGroupScreen.
- Changed string literals to use double quotes for consistency.
- Simplified API calls by removing unnecessary token parameters in group-related API functions.
- Enhanced error handling and user feedback with clearer alert messages.
- Improved layout and styling for better user experience in various components.
- Refactored settlement status calculations and rendering logic for clarity.

* feat: Implement balance checks for group member removal and leaving group

* feat: Enhance group settings with image upload functionality and update group icon handling

* feat: Improve balance verification on group leave and member removal, update error handling

* updates the package_lock.json

* feat: Enhance image handling in group settings and home screen

* feat: Enhance user profile management with image upload and display in account and friends screens

* feat: Normalize user ID shape in AuthContext and FriendsScreen for consistent handling

* feat: Enhance FriendsScreen with user images and loading skeletons for improved UI experience

* feat: Implement currency formatting utility and update screens to use formatted currency

* feat: Enhance API client with retry logic for transient errors and improve image MIME type handling in profile updates

* feat: Add tests for group leave and member removal with unsettled balances and exception handling

* feat: Simplify empty expenses message display in GroupDetailsScreen

---------

Co-authored-by: patel.devasy.23 <[email protected]>

Author: Devasy

backend/app/expenses/service.py

@@ -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": [],

backend/app/groups/service.py

@@ -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}}}
@@ -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}}}

backend/tests/groups/test_groups_service.py

@@ -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"""
@@ -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"),
@@ -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
@@ -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