diff --git a/backend/app/user/routes.py b/backend/app/user/routes.py index 439b76fe..8c4dca76 100644 --- a/backend/app/user/routes.py +++ b/backend/app/user/routes.py @@ -10,7 +10,7 @@ async def get_current_user_profile(current_user: Dict[str, Any] = Depends(get_current_user)): user = await user_service.get_user_by_id(current_user["_id"]) if not user: - raise HTTPException(status_code=404, detail="User not found") + raise HTTPException(status_code=404, detail={"error": "NotFound", "message": "User not found"}) return user @router.patch("/me", response_model=Dict[str, Any]) @@ -20,15 +20,15 @@ async def update_user_profile( ): update_data = updates.model_dump(exclude_unset=True) if not update_data: - raise HTTPException(status_code=400, detail="No update fields provided.") + raise HTTPException(status_code=400, detail={"error": "InvalidInput", "message": "No update fields provided."}) updated_user = await user_service.update_user_profile(current_user["_id"], update_data) if not updated_user: - raise HTTPException(status_code=404, detail="User not found") + raise HTTPException(status_code=404, detail={"error": "NotFound", "message": "User not found"}) return {"user": updated_user} @router.delete("/me", response_model=DeleteUserResponse) async def delete_user_account(current_user: Dict[str, Any] = Depends(get_current_user)): deleted = await user_service.delete_user(current_user["_id"]) if not deleted: - raise HTTPException(status_code=404, detail="User not found") + raise HTTPException(status_code=404, detail={"error": "NotFound", "message": "User not found"}) return DeleteUserResponse(success=True, message="User account scheduled for deletion.") diff --git a/backend/app/user/schemas.py b/backend/app/user/schemas.py index 9b1e8d2b..1c584089 100644 --- a/backend/app/user/schemas.py +++ b/backend/app/user/schemas.py @@ -1,18 +1,16 @@ -from pydantic import BaseModel, EmailStr, Field +from pydantic import BaseModel, EmailStr from typing import Optional from datetime import datetime class UserProfileResponse(BaseModel): - id: str = Field(alias="_id") + id: str name: str email: EmailStr - imageUrl: Optional[str] = Field(default=None, alias="avatar") + imageUrl: Optional[str] = None currency: str = "USD" createdAt: datetime updatedAt: datetime - model_config = {"populate_by_name": True} - class UserProfileUpdateRequest(BaseModel): name: Optional[str] = None imageUrl: Optional[str] = None diff --git a/backend/app/user/service.py b/backend/app/user/service.py index b773bc5a..2b81fa4d 100644 --- a/backend/app/user/service.py +++ b/backend/app/user/service.py @@ -14,18 +14,31 @@ def get_db(self): def transform_user_document(self, user: dict) -> dict: if not user: return None + + def iso(dt): + if not dt: + return None + if isinstance(dt, str): + return dt + # Normalize to UTC and append 'Z' + try: + dt_utc = dt.astimezone(timezone.utc) if getattr(dt, 'tzinfo', None) else dt.replace(tzinfo=timezone.utc) + return dt_utc.isoformat().replace("+00:00", "Z") + except AttributeError: + return str(dt) + try: user_id = str(user["_id"]) except Exception: return None # Handle invalid ObjectId gracefully return { - "_id": user_id, + "id": user_id, "name": user.get("name"), "email": user.get("email"), - "avatar": user.get("imageUrl") or user.get("avatar"), + "imageUrl": user.get("imageUrl") or user.get("avatar"), "currency": user.get("currency", "USD"), - "createdAt": user.get("created_at"), - "updatedAt": user.get("updated_at") or user.get("created_at"), + "createdAt": iso(user.get("created_at")), + "updatedAt": iso(user.get("updated_at") or user.get("created_at")), } async def get_user_by_id(self, user_id: str) -> Optional[dict]: @@ -43,6 +56,9 @@ async def update_user_profile(self, user_id: str, updates: dict) -> Optional[dic obj_id = ObjectId(user_id) except Exception: return None # Handle invalid ObjectId gracefully + # Only allow certain fields + allowed = {"name", "imageUrl", "currency"} + updates = {k: v for k, v in updates.items() if k in allowed} updates["updated_at"] = datetime.now(timezone.utc) result = await db.users.find_one_and_update( {"_id": obj_id}, @@ -58,6 +74,6 @@ async def delete_user(self, user_id: str) -> bool: except Exception: return False # Handle invalid ObjectId gracefully result = await db.users.delete_one({"_id": obj_id}) - return result.deleted_count == 1 + return result.deleted_count > 0 user_service = UserService() diff --git a/backend/tests/user/test_user_routes.py b/backend/tests/user/test_user_routes.py index 86640221..be2d59a7 100644 --- a/backend/tests/user/test_user_routes.py +++ b/backend/tests/user/test_user_routes.py @@ -1,17 +1,17 @@ import pytest -from fastapi.testclient import TestClient # Changed from httpx import AsyncClient +from fastapi.testclient import TestClient from fastapi import status -from main import app # Corrected: Assuming your FastAPI app instance is named 'app' in main.py -from app.auth.security import create_access_token # Helper to create tokens for testing -from datetime import datetime, timedelta # Added datetime +from main import app +from app.auth.security import create_access_token +from datetime import datetime, timedelta # Sample user data for testing -TEST_USER_ID = "60c72b2f9b1e8a3f9c8b4567" # Example ObjectId string +TEST_USER_ID = "60c72b2f9b1e8a3f9c8b4567" TEST_USER_EMAIL = "testuser@example.com" @pytest.fixture(scope="module") -def client(): # Changed to synchronous fixture - with TestClient(app) as c: # Changed to TestClient +def client(): + with TestClient(app) as c: yield c @pytest.fixture(scope="module") @@ -22,138 +22,121 @@ def auth_headers(): ) return {"Authorization": f"Bearer {token}"} -# Placeholder for database setup/teardown if needed, e.g., creating a test user @pytest.fixture(autouse=True, scope="function") async def setup_test_user(mocker): - # Mock the user service to avoid actual database calls initially - # This allows focusing on route logic. - # More specific mocks will be needed per test. + iso_date = "2023-01-01T00:00:00Z" + iso_date2 = "2023-01-02T00:00:00Z" + iso_date3 = "2023-01-03T00:00:00Z" mocker.patch("app.user.service.user_service.get_user_by_id", return_value={ - "_id": TEST_USER_ID, + "id": TEST_USER_ID, "name": "Test User", "email": TEST_USER_EMAIL, - "avatar": None, + "imageUrl": None, "currency": "USD", - "createdAt": datetime.fromisoformat("2023-01-01T00:00:00"), # Changed to camelCase - "updatedAt": datetime.fromisoformat("2023-01-01T00:00:00") # Changed to camelCase + "createdAt": iso_date, + "updatedAt": iso_date }) mocker.patch("app.user.service.user_service.update_user_profile", return_value={ - "_id": TEST_USER_ID, + "id": TEST_USER_ID, "name": "Updated Test User", "email": TEST_USER_EMAIL, - "avatar": "http://example.com/avatar.png", + "imageUrl": "http://example.com/avatar.png", "currency": "EUR", - "createdAt": datetime.fromisoformat("2023-01-01T00:00:00"), # Changed to camelCase - "updatedAt": datetime.fromisoformat("2023-01-02T00:00:00") # Changed to camelCase + "createdAt": iso_date, + "updatedAt": iso_date2 }) mocker.patch("app.user.service.user_service.delete_user", return_value=True) - - # If you have a real test database, you'd create a user here. - # For now, we rely on mocking the service layer. - # Example: - # from app.database import get_database - # db = get_database() - # await db.users.insert_one({ - # "_id": ObjectId(TEST_USER_ID), - # "email": TEST_USER_EMAIL, - # "name": "Test User", - # "hashed_password": "fakehashedpassword", # Add other necessary fields - # "created_at": datetime.utcnow(), - # "updated_at": datetime.utcnow() - # }) yield - # Teardown: remove the test user - # Example: - # await db.users.delete_one({"_id": ObjectId(TEST_USER_ID)}) # --- Tests for GET /users/me --- -def test_get_current_user_profile_success(client: TestClient, auth_headers: dict, mocker): # Changed AsyncClient, removed async +def test_get_current_user_profile_success(client: TestClient, auth_headers: dict, mocker): """Test successful retrieval of current user's profile.""" - # The setup_test_user fixture already mocks get_user_by_id - response = client.get("/users/me", headers=auth_headers) # removed await + response = client.get("/users/me", headers=auth_headers) assert response.status_code == status.HTTP_200_OK data = response.json() - assert data["_id"] == TEST_USER_ID + assert data["id"] == TEST_USER_ID assert data["email"] == TEST_USER_EMAIL assert "name" in data assert "currency" in data + assert "createdAt" in data and data["createdAt"].endswith("Z") + assert "updatedAt" in data and data["updatedAt"].endswith("Z") -def test_get_current_user_profile_not_found(client: TestClient, auth_headers: dict, mocker): # Changed AsyncClient, removed async +def test_get_current_user_profile_not_found(client: TestClient, auth_headers: dict, mocker): """Test retrieval when user is not found in service layer.""" mocker.patch("app.user.service.user_service.get_user_by_id", return_value=None) - - response = client.get("/users/me", headers=auth_headers) # removed await + response = client.get("/users/me", headers=auth_headers) assert response.status_code == status.HTTP_404_NOT_FOUND - assert response.json() == {"detail": "User not found"} + assert response.json() == {"detail": {"error": "NotFound", "message": "User not found"}} # --- Tests for PATCH /users/me --- -def test_update_user_profile_success(client: TestClient, auth_headers: dict, mocker): # Changed AsyncClient, removed async +def test_update_user_profile_success(client: TestClient, auth_headers: dict, mocker): """Test successful update of user profile.""" update_payload = { "name": "Updated Test User", "imageUrl": "http://example.com/avatar.png", "currency": "EUR" } - # The setup_test_user fixture already mocks update_user_profile - response = client.patch("/users/me", headers=auth_headers, json=update_payload) # removed await + response = client.patch("/users/me", headers=auth_headers, json=update_payload) assert response.status_code == status.HTTP_200_OK - data = response.json()["user"] # Response is {"user": updated_user_data} + data = response.json()["user"] assert data["name"] == "Updated Test User" - assert data["avatar"] == "http://example.com/avatar.png" # Note: schema uses imageUrl, service uses avatar + assert data["imageUrl"] == "http://example.com/avatar.png" assert data["currency"] == "EUR" - assert data["_id"] == TEST_USER_ID + assert data["id"] == TEST_USER_ID + assert "createdAt" in data and data["createdAt"].endswith("Z") + assert "updatedAt" in data and data["updatedAt"].endswith("Z") -def test_update_user_profile_partial_update(client: TestClient, auth_headers: dict, mocker): # Changed AsyncClient, removed async +def test_update_user_profile_partial_update(client: TestClient, auth_headers: dict, mocker): """Test updating only one field of the user profile.""" + iso_date = "2023-01-01T00:00:00Z" + iso_date3 = "2023-01-03T00:00:00Z" update_payload = {"name": "Only Name Updated"} - - # Specific mock for this test case if needed, or ensure global mock handles partials mocker.patch("app.user.service.user_service.update_user_profile", return_value={ - "_id": TEST_USER_ID, "name": "Only Name Updated", "email": TEST_USER_EMAIL, - "avatar": None, "currency": "USD", # Assuming other fields remain unchanged - "createdAt": datetime.fromisoformat("2023-01-01T00:00:00"), # Changed to camelCase and datetime - "updatedAt": datetime.fromisoformat("2023-01-03T00:00:00") # Changed to camelCase and datetime + "id": TEST_USER_ID, "name": "Only Name Updated", "email": TEST_USER_EMAIL, + "imageUrl": None, "currency": "USD", + "createdAt": iso_date, "updatedAt": iso_date3 }) - - response = client.patch("/users/me", headers=auth_headers, json=update_payload) # removed await + response = client.patch("/users/me", headers=auth_headers, json=update_payload) assert response.status_code == status.HTTP_200_OK data = response.json()["user"] assert data["name"] == "Only Name Updated" - assert data["currency"] == "USD" # Assuming currency wasn't updated + assert data["currency"] == "USD" + assert data["id"] == TEST_USER_ID + assert "createdAt" in data and data["createdAt"].endswith("Z") + assert "updatedAt" in data and data["updatedAt"].endswith("Z") -def test_update_user_profile_no_fields(client: TestClient, auth_headers: dict): # Changed AsyncClient, removed async +def test_update_user_profile_no_fields(client: TestClient, auth_headers: dict): """Test updating profile with no fields, expecting a 400 error.""" - response = client.patch("/users/me", headers=auth_headers, json={}) # removed await + response = client.patch("/users/me", headers=auth_headers, json={}) assert response.status_code == status.HTTP_400_BAD_REQUEST - assert response.json() == {"detail": "No update fields provided."} + assert response.json() == {"detail": {"error": "InvalidInput", "message": "No update fields provided."}} -def test_update_user_profile_user_not_found(client: TestClient, auth_headers: dict, mocker): # Changed AsyncClient, removed async +def test_update_user_profile_user_not_found(client: TestClient, auth_headers: dict, mocker): """Test updating profile when user is not found by the service.""" mocker.patch("app.user.service.user_service.update_user_profile", return_value=None) update_payload = {"name": "Attempted Update"} - response = client.patch("/users/me", headers=auth_headers, json=update_payload) # removed await + response = client.patch("/users/me", headers=auth_headers, json=update_payload) assert response.status_code == status.HTTP_404_NOT_FOUND - assert response.json() == {"detail": "User not found"} + assert response.json() == {"detail": {"error": "NotFound", "message": "User not found"}} # --- Tests for DELETE /users/me --- -def test_delete_user_account_success(client: TestClient, auth_headers: dict, mocker): # Changed AsyncClient, removed async +def test_delete_user_account_success(client: TestClient, auth_headers: dict, mocker): """Test successful deletion of a user account.""" - # The setup_test_user fixture already mocks delete_user to return True - response = client.delete("/users/me", headers=auth_headers) # removed await + response = client.delete("/users/me", headers=auth_headers) assert response.status_code == status.HTTP_200_OK data = response.json() assert data["success"] is True assert data["message"] == "User account scheduled for deletion." -def test_delete_user_account_not_found(client: TestClient, auth_headers: dict, mocker): # Changed AsyncClient, removed async +def test_delete_user_account_not_found(client: TestClient, auth_headers: dict, mocker): """Test deleting a user account when the user is not found by the service.""" mocker.patch("app.user.service.user_service.delete_user", return_value=False) - response = client.delete("/users/me", headers=auth_headers) # removed await + response = client.delete("/users/me", headers=auth_headers) assert response.status_code == status.HTTP_404_NOT_FOUND - assert response.json() == {"detail": "User not found"} + assert response.json() == {"detail": {"error": "NotFound", "message": "User not found"}} # All route tests are in place, removing the placeholder # def test_placeholder(): diff --git a/backend/tests/user/test_user_service.py b/backend/tests/user/test_user_service.py index 2d6e5b99..ab6ebfb7 100644 --- a/backend/tests/user/test_user_service.py +++ b/backend/tests/user/test_user_service.py @@ -1,9 +1,9 @@ import pytest from app.user.service import UserService -from app.database import get_database # To mock the database dependency +from app.database import get_database from bson import ObjectId -from datetime import datetime, timezone -from unittest.mock import AsyncMock, MagicMock # For mocking async methods and db client +from datetime import datetime, timezone, timedelta +from unittest.mock import AsyncMock, MagicMock # Initialize UserService instance for testing user_service = UserService() @@ -27,26 +27,31 @@ def mock_get_database(mocker, mock_db_client): TEST_OBJECT_ID_STR = "60c72b2f9b1e8a3f9c8b4567" TEST_OBJECT_ID = ObjectId(TEST_OBJECT_ID_STR) -NOW = datetime.now(timezone.utc) +NOW = datetime(2023, 1, 1, 0, 0, 0, tzinfo=timezone.utc) +LATER = datetime(2023, 1, 2, 0, 0, 0, tzinfo=timezone.utc) +LATER3 = datetime(2023, 1, 3, 0, 0, 0, tzinfo=timezone.utc) +ISO_NOW = NOW.isoformat().replace("+00:00", "Z") +ISO_LATER = LATER.isoformat().replace("+00:00", "Z") +ISO_LATER3 = LATER3.isoformat().replace("+00:00", "Z") RAW_USER_FROM_DB = { "_id": TEST_OBJECT_ID, "name": "Test User", "email": "test@example.com", - "avatar": "http://example.com/avatar.jpg", + "imageUrl": "http://example.com/avatar.jpg", "currency": "EUR", "created_at": NOW, "updated_at": NOW, } TRANSFORMED_USER_EXPECTED = { - "_id": TEST_OBJECT_ID_STR, + "id": TEST_OBJECT_ID_STR, "name": "Test User", "email": "test@example.com", - "avatar": "http://example.com/avatar.jpg", + "imageUrl": "http://example.com/avatar.jpg", "currency": "EUR", - "createdAt": NOW, - "updatedAt": NOW, + "createdAt": ISO_NOW, + "updatedAt": ISO_NOW, } # --- Tests for transform_user_document --- @@ -63,34 +68,33 @@ def test_transform_user_document_missing_optional_fields(): "created_at": NOW, } expected_transformed_minimal = { - "_id": TEST_OBJECT_ID_STR, + "id": TEST_OBJECT_ID_STR, "name": "Minimal User", "email": "minimal@example.com", - "avatar": None, # Expect None if not present - "currency": "USD", # Expect default if not present - "createdAt": NOW, - "updatedAt": NOW, # Expect createdAt if updatedAt not present + "imageUrl": None, + "currency": "USD", + "createdAt": ISO_NOW, + "updatedAt": ISO_NOW, } transformed = user_service.transform_user_document(raw_user_minimal) assert transformed == expected_transformed_minimal def test_transform_user_document_with_updated_at_different_from_created_at(): - later_time = datetime.now(timezone.utc) raw_user_updated = { "_id": TEST_OBJECT_ID, "name": "Updated User", "email": "updated@example.com", "created_at": NOW, - "updated_at": later_time + "updated_at": LATER } expected_transformed_updated = { - "_id": TEST_OBJECT_ID_STR, + "id": TEST_OBJECT_ID_STR, "name": "Updated User", "email": "updated@example.com", - "avatar": None, + "imageUrl": None, "currency": "USD", - "createdAt": NOW, - "updatedAt": later_time, + "createdAt": ISO_NOW, + "updatedAt": ISO_LATER, } transformed = user_service.transform_user_document(raw_user_updated) assert transformed == expected_transformed_updated @@ -98,6 +102,43 @@ def test_transform_user_document_with_updated_at_different_from_created_at(): def test_transform_user_document_none_input(): assert user_service.transform_user_document(None) is None +def test_transform_user_document_iso_none(): + user = {"_id": "x", "created_at": None} + result = user_service.transform_user_document(user) + assert result["createdAt"] is None + +def test_transform_user_document_iso_str(): + user = {"_id": "x", "created_at": "2025-06-28T12:00:00Z"} + result = user_service.transform_user_document(user) + assert result["createdAt"] == "2025-06-28T12:00:00Z" + +def test_transform_user_document_iso_naive_datetime(): + dt = datetime(2025, 6, 28, 12, 0, 0) + user = {"_id": "x", "created_at": dt} + result = user_service.transform_user_document(user) + assert result["createdAt"].endswith("Z") + +def test_transform_user_document_iso_aware_datetime_utc(): + dt = datetime(2025, 6, 28, 12, 0, 0, tzinfo=timezone.utc) + user = {"_id": "x", "created_at": dt} + result = user_service.transform_user_document(user) + assert result["createdAt"].endswith("Z") + assert result["createdAt"].startswith("2025-06-28T12:00:00") + +def test_transform_user_document_iso_aware_datetime_non_utc(): + dt = datetime(2025, 6, 28, 14, 0, 0, tzinfo=timezone(timedelta(hours=2))) + user = {"_id": "x", "created_at": dt} + result = user_service.transform_user_document(user) + assert result["createdAt"].endswith("Z") + assert result["createdAt"].startswith("2025-06-28T12:00:00") + +def test_transform_user_document_iso_unexpected_type(): + class Dummy: pass + dummy = Dummy() + user = {"_id": "x", "created_at": dummy} + result = user_service.transform_user_document(user) + assert result["createdAt"] == str(dummy) + # --- Tests for get_user_by_id --- @pytest.mark.asyncio @@ -127,10 +168,7 @@ async def test_update_user_profile_success(mock_db_client, mock_get_database): # The user document that find_one_and_update would return updated_user_doc_from_db = RAW_USER_FROM_DB.copy() updated_user_doc_from_db.update(update_data) - # updated_at will be set by the service method, so we don't put it in update_data - # but the mock return value should reflect it. - # For simplicity, we'll check its existence rather than exact value in this mock. - + updated_user_doc_from_db["updated_at"] = LATER mock_db_client.users.find_one_and_update.return_value = updated_user_doc_from_db # Expected transformed output @@ -149,9 +187,8 @@ async def test_update_user_profile_success(mock_db_client, mock_get_database): assert updated_user is not None assert updated_user["name"] == "New Name" assert updated_user["currency"] == "CAD" - assert updated_user["_id"] == TEST_OBJECT_ID_STR - # Ensure 'updated_at' in the result is more recent or equal to original if not updated by mock - assert updated_user["updatedAt"] >= TRANSFORMED_USER_EXPECTED["updatedAt"] + assert updated_user["id"] == TEST_OBJECT_ID_STR + assert updated_user["updatedAt"] == ISO_LATER @pytest.mark.asyncio diff --git a/docs/user-service.md b/docs/user-service.md index f82b495b..2aef81be 100644 --- a/docs/user-service.md +++ b/docs/user-service.md @@ -36,13 +36,16 @@ Retrieves the profile information for the currently authenticated user. "id": "usr_123abc", "name": "Jane Doe", "email": "jane.doe@example.com", - "image_url": "https://example.com/profile.jpg", + "imageUrl": "https://example.com/profile.jpg", "currency": "USD", - "created_at": "2024-01-15T10:00:00Z", - "updatedAt": "2024-01-16T12:30:00Z" + "createdAt": "2024-01-15T10:00:00Z", // ISO 8601 string, but may be parsed as datetime by backend frameworks + "updatedAt": "2024-01-16T12:30:00Z" // ISO 8601 string, but may be parsed as datetime by backend frameworks } ``` +> **Note:** +> - The backend stores and processes `createdAt` and `updatedAt` as `datetime` objects, but they are serialized to ISO 8601 strings in API responses for JSON compatibility. If you use Python or Pydantic, you may use `datetime` types in your models, but the API will always return them as strings. + **PlantUML Diagram:** ```plantuml