Skip to content
Open
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
2 changes: 1 addition & 1 deletion backend/app/auth/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ class UserResponse(BaseModel):
name: str
avatar: Optional[str] = None
currency: str = "USD"
created_at: datetime
created_at: datetime = Field(alias="createdAt")

model_config = ConfigDict(populate_by_name=True)

Expand Down
56 changes: 32 additions & 24 deletions backend/app/auth/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -289,38 +289,46 @@ async def refresh_access_token(self, refresh_token: str) -> str:

return new_refresh_token

async def verify_access_token(self, token: str) -> Dict[str, Any]:
"""
Verifies an access token and retrieves the associated user.

Args:
token: The JWT access token to verify.
async def verify_access_token(self, token: str) -> Dict[str, Any]:
"""
Verifies an access token and retrieves the associated user.

Returns:
The user document corresponding to the token's subject.
Args:
token: The JWT access token to verify.

Raises:
HTTPException: If the token is invalid or the user does not exist.
"""
from app.auth.security import verify_token
Returns:
The user document corresponding to the token's subject.

payload = verify_token(token)
user_id = payload.get("sub")
Raises:
HTTPException: If the token is invalid or the user does not exist.
"""
from app.auth.security import verify_token
from bson import ObjectId

if not user_id:
raise HTTPException(
status_code=status.HTTP_401_UNAUTHORIZED, detail="Invalid token"
)
payload = verify_token(token)
user_id = payload.get("sub")

db = self.get_db()
user = await db.users.find_one({"_id": user_id})
if not user_id:
raise HTTPException(
status_code=status.HTTP_401_UNAUTHORIZED, detail="Invalid token"
)

if not user:
raise HTTPException(
status_code=status.HTTP_401_UNAUTHORIZED, detail="User not found"
)
db = self.get_db()

try:
user = await db.users.find_one({"_id": ObjectId(user_id)})
except Exception:
raise HTTPException(
status_code=status.HTTP_401_UNAUTHORIZED, detail="Invalid user ID format"
)

if not user:
raise HTTPException(
status_code=status.HTTP_401_UNAUTHORIZED, detail="User not found"
)

return user
return user
Comment on lines +293 to +331
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Critical issues in function refactoring - function is broken

The refactoring from instance method to standalone function has introduced several critical issues:

  1. Function still references self: Line 317 calls self.get_db() but the function is no longer an instance method
  2. Redundant import: ObjectId is already imported at line 16, the import at line 307 is unnecessary
  3. Poor exception handling: The broad except Exception should use proper exception chaining

Apply this diff to fix the issues:

-async def verify_access_token(self, token: str) -> Dict[str, Any]:
+async def verify_access_token(token: str) -> Dict[str, Any]:
     """
     Verifies an access token and retrieves the associated user.

     Args:
         token: The JWT access token to verify.

     Returns:
         The user document corresponding to the token's subject.

     Raises:
         HTTPException: If the token is invalid or the user does not exist.
     """
     from app.auth.security import verify_token
-    from bson import ObjectId

     payload = verify_token(token)
     user_id = payload.get("sub")

     if not user_id:
         raise HTTPException(
             status_code=status.HTTP_401_UNAUTHORIZED, detail="Invalid token"
         )

-    db = self.get_db()
+    db = get_database()

     try:
         user = await db.users.find_one({"_id": ObjectId(user_id)})
-    except Exception:
+    except Exception as e:
         raise HTTPException(
             status_code=status.HTTP_401_UNAUTHORIZED, detail="Invalid user ID format"
-        )
+        ) from e

     if not user:
         raise HTTPException(
             status_code=status.HTTP_401_UNAUTHORIZED, detail="User not found"
         )

     return user
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
async def verify_access_token(self, token: str) -> Dict[str, Any]:
"""
Verifies an access token and retrieves the associated user.
Returns:
The user document corresponding to the token's subject.
Args:
token: The JWT access token to verify.
Raises:
HTTPException: If the token is invalid or the user does not exist.
"""
from app.auth.security import verify_token
Returns:
The user document corresponding to the token's subject.
payload = verify_token(token)
user_id = payload.get("sub")
Raises:
HTTPException: If the token is invalid or the user does not exist.
"""
from app.auth.security import verify_token
from bson import ObjectId
if not user_id:
raise HTTPException(
status_code=status.HTTP_401_UNAUTHORIZED, detail="Invalid token"
)
payload = verify_token(token)
user_id = payload.get("sub")
db = self.get_db()
user = await db.users.find_one({"_id": user_id})
if not user_id:
raise HTTPException(
status_code=status.HTTP_401_UNAUTHORIZED, detail="Invalid token"
)
if not user:
raise HTTPException(
status_code=status.HTTP_401_UNAUTHORIZED, detail="User not found"
)
db = self.get_db()
try:
user = await db.users.find_one({"_id": ObjectId(user_id)})
except Exception:
raise HTTPException(
status_code=status.HTTP_401_UNAUTHORIZED, detail="Invalid user ID format"
)
if not user:
raise HTTPException(
status_code=status.HTTP_401_UNAUTHORIZED, detail="User not found"
)
return user
return user
async def verify_access_token(token: str) -> Dict[str, Any]:
"""
Verifies an access token and retrieves the associated user.
Args:
token: The JWT access token to verify.
Returns:
The user document corresponding to the token's subject.
Raises:
HTTPException: If the token is invalid or the user does not exist.
"""
from app.auth.security import verify_token
payload = verify_token(token)
user_id = payload.get("sub")
if not user_id:
raise HTTPException(
status_code=status.HTTP_401_UNAUTHORIZED, detail="Invalid token"
)
db = get_database()
try:
user = await db.users.find_one({"_id": ObjectId(user_id)})
except Exception as e:
raise HTTPException(
status_code=status.HTTP_401_UNAUTHORIZED, detail="Invalid user ID format"
) from e
if not user:
raise HTTPException(
status_code=status.HTTP_401_UNAUTHORIZED, detail="User not found"
)
return user
🧰 Tools
🪛 Ruff (0.12.2)

307-307: Redefinition of unused ObjectId from line 16

Remove definition: ObjectId

(F811)


322-324: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling

(B904)

🤖 Prompt for AI Agents
In backend/app/auth/service.py between lines 293 and 331, the
verify_access_token function was refactored to a standalone function but still
references self.get_db(), which is invalid. Remove the self parameter and
replace self.get_db() with a proper database access method or pass the database
as a parameter. Also, remove the redundant import of ObjectId since it is
already imported at the top of the file. Finally, replace the broad except
Exception with a more specific exception for ObjectId conversion errors and use
exception chaining to preserve the original error context.


async def request_password_reset(self, email: str) -> bool:
"""
Expand Down
24 changes: 11 additions & 13 deletions backend/app/expenses/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,7 @@ class ExpenseCreateRequest(BaseModel):
def validate_splits_sum(cls, v, values):
if "amount" in values:
total_split = sum(split.amount for split in v)
if (
abs(total_split - values["amount"]) > 0.01
): # Allow small floating point differences
if abs(total_split - values["amount"]) > 0.01:
raise ValueError(
"Split amounts must sum to total expense amount")
return v
Expand All @@ -52,7 +50,6 @@ class ExpenseUpdateRequest(BaseModel):

@validator("splits")
def validate_splits_sum(cls, v, values):
# Only validate if both splits and amount are provided in the update
if v is not None and "amount" in values and values["amount"] is not None:
total_split = sum(split.amount for split in v)
if abs(total_split - values["amount"]) > 0.01:
Expand All @@ -61,7 +58,6 @@ def validate_splits_sum(cls, v, values):
return v

class Config:
# Allow validation to work with partial updates
validate_assignment = True


Expand All @@ -70,7 +66,7 @@ class ExpenseComment(BaseModel):
userId: str
userName: str
content: str
createdAt: datetime
createdAt: datetime = Field(alias="created_at")

model_config = ConfigDict(
populate_by_name=True, str_strip_whitespace=True, validate_assignment=True
Expand All @@ -82,7 +78,7 @@ class ExpenseHistoryEntry(BaseModel):
userId: str
userName: str
beforeData: Dict[str, Any]
editedAt: datetime
editedAt: datetime = Field(alias="edited_at")

model_config = ConfigDict(populate_by_name=True)

Expand All @@ -99,15 +95,15 @@ class ExpenseResponse(BaseModel):
receiptUrls: List[str] = []
comments: Optional[List[ExpenseComment]] = []
history: Optional[List[ExpenseHistoryEntry]] = []
createdAt: datetime
updatedAt: datetime
createdAt: datetime = Field(alias="created_at")
updatedAt: datetime = Field(alias="updated_at")

model_config = ConfigDict(populate_by_name=True)


class Settlement(BaseModel):
id: str = Field(alias="_id")
expenseId: Optional[str] = None # None for manual settlements
expenseId: Optional[str] = None
groupId: str
payerId: str
payeeId: str
Expand All @@ -116,8 +112,8 @@ class Settlement(BaseModel):
amount: float
status: SettlementStatus
description: Optional[str] = None
paidAt: Optional[datetime] = None
createdAt: datetime
paidAt: Optional[datetime] = Field(default=None, alias="paidAt")
createdAt: datetime = Field(alias="created_at")

model_config = ConfigDict(populate_by_name=True)

Expand Down Expand Up @@ -194,7 +190,9 @@ class FriendBalance(BaseModel):
netBalance: float
owesYou: bool
breakdown: List[FriendBalanceBreakdown]
lastActivity: datetime
lastActivity: datetime = Field(alias="last_activity")

model_config = ConfigDict(populate_by_name=True)


class FriendsBalanceResponse(BaseModel):
Expand Down
87 changes: 57 additions & 30 deletions backend/app/groups/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,68 +5,95 @@


class GroupMember(BaseModel):
userId: str
role: str = "member" # "admin" or "member"
joinedAt: datetime
userId: str = Field(..., alias="userId")
role: str = Field(default="member", alias="role") # "admin" or "member"
joinedAt: datetime = Field(..., alias="joinedAt")

model_config = ConfigDict(populate_by_name=True)


class GroupMemberWithDetails(BaseModel):
userId: str
role: str = "member" # "admin" or "member"
joinedAt: datetime
user: Optional[dict] = None # Contains user details like name, email
userId: str = Field(..., alias="userId")
role: str = Field(default="member", alias="role") # "admin" or "member"
joinedAt: datetime = Field(..., alias="joinedAt")
user: Optional[dict] = Field(
default=None, alias="user") # Contains user details

model_config = ConfigDict(populate_by_name=True)


class GroupCreateRequest(BaseModel):
name: str = Field(..., min_length=1, max_length=100)
currency: Optional[str] = "USD"
imageUrl: Optional[str] = None
name: str = Field(..., min_length=1, max_length=100, alias="name")
currency: Optional[str] = Field(default="USD", alias="currency")
imageUrl: Optional[str] = Field(default=None, alias="imageUrl")

model_config = ConfigDict(populate_by_name=True)


class GroupUpdateRequest(BaseModel):
name: Optional[str] = Field(None, min_length=1, max_length=100)
imageUrl: Optional[str] = None
name: Optional[str] = Field(
default=None, min_length=1, max_length=100, alias="name"
)
imageUrl: Optional[str] = Field(default=None, alias="imageUrl")

model_config = ConfigDict(populate_by_name=True)


class GroupResponse(BaseModel):
id: str = Field(alias="_id")
name: str
currency: str
joinCode: str
createdBy: str
createdAt: datetime
imageUrl: Optional[str] = None
members: Optional[List[GroupMemberWithDetails]] = []
id: str = Field(..., alias="_id")
name: str = Field(..., alias="name")
currency: str = Field(..., alias="currency")
joinCode: str = Field(..., alias="joinCode")
createdBy: str = Field(..., alias="createdBy")
createdAt: datetime = Field(..., alias="createdAt")
imageUrl: Optional[str] = Field(default=None, alias="imageUrl")
members: Optional[List[GroupMemberWithDetails]] = Field(
default_factory=list, alias="members"
)

model_config = ConfigDict(populate_by_name=True)


class GroupListResponse(BaseModel):
groups: List[GroupResponse]
groups: List[GroupResponse] = Field(..., alias="groups")

model_config = ConfigDict(populate_by_name=True)


class JoinGroupRequest(BaseModel):
joinCode: str = Field(..., min_length=1)
joinCode: str = Field(..., min_length=1, alias="joinCode")

model_config = ConfigDict(populate_by_name=True)


class JoinGroupResponse(BaseModel):
group: GroupResponse
group: GroupResponse = Field(..., alias="group")

model_config = ConfigDict(populate_by_name=True)


class MemberRoleUpdateRequest(BaseModel):
role: str = Field(..., pattern="^(admin|member)$")
role: str = Field(..., pattern="^(admin|member)$", alias="role")

model_config = ConfigDict(populate_by_name=True)


class LeaveGroupResponse(BaseModel):
success: bool
message: str
success: bool = Field(..., alias="success")
message: str = Field(..., alias="message")

model_config = ConfigDict(populate_by_name=True)


class DeleteGroupResponse(BaseModel):
success: bool
message: str
success: bool = Field(..., alias="success")
message: str = Field(..., alias="message")

model_config = ConfigDict(populate_by_name=True)


class RemoveMemberResponse(BaseModel):
success: bool
message: str
success: bool = Field(..., alias="success")
message: str = Field(..., alias="message")

model_config = ConfigDict(populate_by_name=True)
15 changes: 10 additions & 5 deletions backend/app/user/schemas.py
Original file line number Diff line number Diff line change
@@ -1,24 +1,29 @@
from datetime import datetime
from typing import Optional

from pydantic import BaseModel, EmailStr
from pydantic import BaseModel, EmailStr, Field
from pydantic.config import ConfigDict


class UserProfileResponse(BaseModel):
id: str
name: str
email: EmailStr
imageUrl: Optional[str] = None
image_url: Optional[str] = Field(default=None, alias="imageUrl")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Verify service layer field naming consistency.

The schema now uses image_url with alias "imageUrl", but the service layer may still be using the old field names.


🏁 Script executed:

#!/bin/bash
# Description: Check for field naming consistency between schemas and service layer
# Expected: Service layer should use snake_case field names or be updated accordingly

# Check user service for field usage
rg -A 3 -B 3 "imageUrl|image_url" backend/app/user/service.py

# Check for any remaining camelCase usage in service files that should be snake_case
rg -A 2 -B 2 "imageUrl.*=" backend/app/user/

Length of output: 829


Service layer should use snake_case internal field names

The service layer currently accesses and filters updates using "imageUrl" (camelCase), but the database still stores image_url. This mismatch can lead to missing data or failed updates. Update backend/app/user/service.py as follows:

• Change retrieval from user.get("imageUrl") to user.get("image_url").
• Replace "imageUrl" with "image_url" in the allowed set and update filters.

Example diff:

@@ backend/app/user/service.py
-    "imageUrl": user.get("imageUrl") or user.get("avatar"),
+    "imageUrl": user.get("image_url") or user.get("avatar"),

-    allowed = {"name", "imageUrl", "currency"}
+    allowed = {"name", "image_url", "currency"}

With model_config = ConfigDict(populate_by_name=True) in your Pydantic schemas, responses will continue to serialize image_url as imageUrl.

🤖 Prompt for AI Agents
In backend/app/user/schemas.py at line 12, the field uses alias "imageUrl" for
serialization, but in backend/app/user/service.py, update all internal
references from "imageUrl" to "image_url" to match the database field naming.
Specifically, change user.get("imageUrl") to user.get("image_url") and replace
"imageUrl" with "image_url" in any allowed field sets and filters. This ensures
consistent snake_case usage internally while preserving camelCase in API
responses via Pydantic's aliasing.

currency: str = "USD"
createdAt: datetime
updatedAt: datetime
created_at: datetime = Field(alias="createdAt")
updated_at: datetime = Field(alias="updatedAt")

model_config = ConfigDict(populate_by_name=True)


class UserProfileUpdateRequest(BaseModel):
name: Optional[str] = None
imageUrl: Optional[str] = None
image_url: Optional[str] = Field(default=None, alias="imageUrl")
currency: Optional[str] = None

model_config = ConfigDict(populate_by_name=True)


class DeleteUserResponse(BaseModel):
success: bool = True
Expand Down
6 changes: 3 additions & 3 deletions backend/tests/auth/test_auth_routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ async def test_signup_with_existing_email(mock_db):
"email": existing_email,
"hashed_password": "hashedpassword",
"name": "Existing User",
"created_at": "sometime", # Simplified for mock
"createdAt": "sometime", # Simplified for mock
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Verify database field naming convention.

The test data now uses "createdAt" in database documents, but typically databases use snake_case (created_at) internally while APIs expose camelCase through Pydantic aliases.

Also applies to: 174-174, 217-217


🏁 Script executed:

#!/bin/bash
# Description: Check if database operations use snake_case or camelCase field names
# Expected: Database operations should use snake_case, API responses use camelCase via aliases

# Check auth service for database field usage
rg -A 3 -B 3 "created_at|createdAt" backend/app/auth/service.py

# Check user service for database field usage  
rg -A 3 -B 3 "created_at|createdAt" backend/app/user/service.py

# Check database schemas/models
rg -A 3 -B 3 "created_at|createdAt" backend/app/models/

Length of output: 1603


Use snake_case for DB mocks; camelCase only in API responses

The auth service and user service both write/read created_at internally and expose createdAt via Pydantic aliases. Your tests are mocking database documents, so the mocks should use created_at. Only your assertions against the HTTP response should use createdAt.

Please update backend/tests/auth/test_auth_routes.py at the following locations:

  • Line 78:
  • "createdAt": "sometime", # Simplified for mock
  • "created_at": "sometime", # Simplified for mock
- Line 174:
```diff
-   "createdAt": some_timestamp,
+   "created_at": some_timestamp,
  • Line 217:
  • "createdAt": other_timestamp,
  • "created_at": other_timestamp,

Then, when verifying the endpoint’s JSON response, assert on `response.json()["createdAt"]` (the alias), not on the mock’s field name.

<!-- suggestion_start -->

<details>
<summary>📝 Committable suggestion</summary>

> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

```suggestion
--- a/backend/tests/auth/test_auth_routes.py
+++ b/backend/tests/auth/test_auth_routes.py
@@ -75,7 +75,7 @@
       # Simplified mock document for user creation
       {
-            "createdAt": "sometime",  # Simplified for mock
+            "created_at": "sometime",  # Simplified for mock
           "email": "[email protected]",
           "id": str(user_id),
       }
@@ -171,7 +171,7 @@
       # Mock document for fetching auth tokens
       {
           "token": str(token),
-            "createdAt": some_timestamp,
+            "created_at": some_timestamp,
           "userId": str(user_id),
       }
@@ -214,7 +214,7 @@
       # Mock document for refresh-token rotation
       {
           "refreshToken": other_refresh_token,
-            "createdAt": other_timestamp,
+            "created_at": other_timestamp,
           "userId": str(user_id),
       }
🤖 Prompt for AI Agents
In backend/tests/auth/test_auth_routes.py at lines 78, 174, and 217, change the
mock database fields from camelCase (createdAt) to snake_case (created_at) to
match internal DB conventions. Then, in the test assertions that check the HTTP
response JSON, verify the alias field createdAt instead of created_at. This
ensures mocks reflect DB structure while assertions validate API response
format.

}
)

Expand Down Expand Up @@ -171,7 +171,7 @@ async def test_login_with_email_success(mock_db):
"avatar": None,
"currency": "USD",
# Ensure datetime is used
"created_at": datetime.now(timezone.utc),
"createdAt": datetime.now(timezone.utc),
"auth_provider": "email",
"firebase_uid": None,
}
Expand Down Expand Up @@ -214,7 +214,7 @@ async def test_login_with_incorrect_password(mock_db):
"email": user_email,
"hashed_password": get_password_hash(correct_password),
"name": "Wrong Pass User",
"created_at": datetime.now(timezone.utc),
"createdAt": datetime.now(timezone.utc),
}
)

Expand Down
1 change: 1 addition & 0 deletions backend/tests/expenses/test_expense_routes.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from unittest.mock import AsyncMock, patch

import pytest
from app.auth.dependencies import get_current_user
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Remove unused import.

The get_current_user import is not used in this file since the tests mock it via @patch decorators.

-from app.auth.dependencies import get_current_user
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
from app.auth.dependencies import get_current_user
🧰 Tools
🪛 Ruff (0.12.2)

4-4: app.auth.dependencies.get_current_user imported but unused

Remove unused import: app.auth.dependencies.get_current_user

(F401)

🤖 Prompt for AI Agents
In backend/tests/expenses/test_expense_routes.py at line 4, the import of
get_current_user from app.auth.dependencies is unused because the tests mock
this dependency using @patch decorators. Remove this import statement to clean
up the code and avoid unnecessary imports.

from app.expenses.schemas import ExpenseCreateRequest, ExpenseSplit
from fastapi import status
from httpx import ASGITransport, AsyncClient
Expand Down
Loading