From 792f91ce23e266b586107aa990cfb7d17b14b2f2 Mon Sep 17 00:00:00 2001 From: eduart77 Date: Wed, 3 Dec 2025 22:50:07 +0200 Subject: [PATCH 1/3] solved the split problem --- API/models/expense.py | 11 +++++++++++ API/repositories/expense_repository.py | 19 ++++++++++++------- API/schemas/expense.py | 13 +++++++++++++ 3 files changed, 36 insertions(+), 7 deletions(-) diff --git a/API/models/expense.py b/API/models/expense.py index d0f9268..307f705 100644 --- a/API/models/expense.py +++ b/API/models/expense.py @@ -41,3 +41,14 @@ class Expense(Base): name="chk_expenses_one_fk" ), ) + @property + def split_amount(self) -> float: + """ + Calculates amount per person if it is a group expense. + """ + print(f"DEBUG: Group {self.group_id} has members: {[u.id for u in self.group.users]}") + if self.group_id and self.group and self.group.users: + member_count = len(self.group.users) + if member_count > 0: + return round(self.amount / member_count, 2) + return self.amount diff --git a/API/repositories/expense_repository.py b/API/repositories/expense_repository.py index 292510d..c419960 100644 --- a/API/repositories/expense_repository.py +++ b/API/repositories/expense_repository.py @@ -5,8 +5,9 @@ from models.category import Category from models.expense import Expense from models.user_group import UserGroup +from models.group import Group from sqlalchemy import and_, asc, desc, or_, select -from sqlalchemy.orm import Session +from sqlalchemy.orm import Session, joinedload class IExpenseRepository(ABC): @@ -92,7 +93,7 @@ def get_by_id(self, expense_id: int) -> Optional[Expense]: """ statement = select(Expense).where(Expense.id == expense_id) - return self.db.scalars(statement).first() + return self.db.scalars(statement).unique().first() def get_all( self, @@ -127,12 +128,14 @@ def get_all( if category_ids: conditions.append(Expense.category_id.in_(category_ids)) - statement = select(Expense) + statement = select(Expense).options( + joinedload(Expense.group).joinedload(Group.users) + ) if conditions: statement = statement.where(and_(*conditions)) + statement = statement.order_by(sort_order).offset(offset).limit(limit) - - return list(self.db.scalars(statement)) + return list(self.db.scalars(statement).unique()) def get_by_user( self, @@ -187,13 +190,14 @@ def get_by_user( statement = ( select(Expense) + .options(joinedload(Expense.group).joinedload(Group.users)) .where(and_(*conditions)) .order_by(sort_order) .offset(offset) .limit(limit) ) - return list(self.db.scalars(statement)) + return list(self.db.scalars(statement).unique()) def get_by_group( self, @@ -231,13 +235,14 @@ def get_by_group( statement = ( select(Expense) + .options(joinedload(Expense.group).joinedload(Group.users)) .where(and_(*conditions)) .order_by(sort_order) .offset(offset) .limit(limit) ) - return list(self.db.scalars(statement)) + return list(self.db.scalars(statement).unique()) def update(self, expense_id: int, fields: dict) -> int: """ diff --git a/API/schemas/expense.py b/API/schemas/expense.py index fc9aa96..431bd5e 100644 --- a/API/schemas/expense.py +++ b/API/schemas/expense.py @@ -43,5 +43,18 @@ class ExpenseResponse(ExpenseBase): created_at: datetime description: Optional[str] = None + split_amount: float + + class Config: + from_attributes = True + +class Expense(ExpenseBase): + id: int + user_id: Optional[int] = None + group_id: Optional[int] = None + created_at: datetime + + split_amount: float + class Config: from_attributes = True From b50cfe951e2b317f01c75d599ba5911968ab6f68 Mon Sep 17 00:00:00 2001 From: eduart77 Date: Wed, 3 Dec 2025 23:00:48 +0200 Subject: [PATCH 2/3] added more tests for the user services --- API/services/user_service.py | 28 ++++++++++++---------- API/tests/test_user_service.py | 44 ++++++++++++++++++++++------------ 2 files changed, 44 insertions(+), 28 deletions(-) diff --git a/API/services/user_service.py b/API/services/user_service.py index df0d4ba..01d6aee 100644 --- a/API/services/user_service.py +++ b/API/services/user_service.py @@ -208,24 +208,26 @@ def delete_user(self, user_id: int) -> APIResponse: success=True ) - def change_password(self, user_id: int, old_password: str, new_password: str) -> APIResponse: + def change_password(self, user_id: int, old_password: str, new_password: str) -> dict: """ - Verifies the old password and updates to the new password. + Allows an authenticated user to change their password by providing the old one. """ - self.logger.info(f"Changing password for user_id={user_id}") - - user = self._validate_user(user_id=user_id) - if not PasswordUtil.verify_password(old_password, user.hashed_password): - self.logger.warning(f"Password change failed for user_id={user_id}: Invalid old password") - raise HTTPException(status_code=STATUS_BAD_REQUEST, detail="Invalid old password.") + self.logger.info(f"Password change requested for user_id={user_id}") + user = self.repository.get_by_id(user_id) + if not user: + raise HTTPException(status_code=404, detail="User not found.") - new_hashed_password = PasswordUtil.hash_password(new_password) + # Verify old password + if not PasswordUtil.verify_password(old_password, user.hashed_password): + self.logger.warning(f"Invalid old password provided for user_id={user_id}") + raise HTTPException(status_code=400, detail="Invalid old password.") - self.repository.update(user_id, {HASHED_PASSWORD_FIELD: new_hashed_password}) + # Update with new password + new_hashed = PasswordUtil.hash_password(new_password) + self.repository.update(user_id, {"hashed_password": new_hashed}) - return APIResponse( - success=True, - ) + self.logger.info(f"Password successfully changed for user_id={user_id}") + return {"message": "Password changed successfully."} def request_password_reset(self, email: str) -> APIResponse: """ diff --git a/API/tests/test_user_service.py b/API/tests/test_user_service.py index 9f819dc..ec4a93c 100644 --- a/API/tests/test_user_service.py +++ b/API/tests/test_user_service.py @@ -4,7 +4,6 @@ from schemas.user import UserCreate, UserLogin, UserUpdate from services.user_service import ALGORITHM, SECRET_KEY, UserService - # Mock UserRepository class MockUserRepository: """ @@ -61,10 +60,7 @@ def user_service(): mock_repo = MockUserRepository() service = UserService.__new__(UserService) # bypass __init__ service.repository = mock_repo - # Initialize logger mock if needed, or rely on the fact that Logger() is static/singleton - # For this test structure, assuming Logger calls are safe or mocked elsewhere isn't strictly necessary - # if the real Logger doesn't break tests. - # However, strictly speaking, we might want to silence the logger here. + # Mock the logger to prevent errors during tests service.logger = type('MockLogger', (), {'debug': lambda s: None, 'info': lambda s: None, 'warning': lambda s: None, 'error': lambda s: None}) return service @@ -78,8 +74,16 @@ def test_register_user_success(user_service): phone_number="123456" ) result = user_service.register_user(user_in) + assert "access_token" in result - assert result["user"].email == "test@example.com" + + # REPAIR: result["user"] is an ID (int), not an object + user_id = result["user"] + assert isinstance(user_id, int) + + # Fetch object to verify properties + created_user = user_service.repository.get_by_id(user_id) + assert created_user.email == "test@example.com" def test_register_duplicate_email(user_service): @@ -110,7 +114,9 @@ def test_login_success(user_service): login_in = UserLogin(email="login@example.com", password="Password123") result = user_service.login_user(login_in) + assert "access_token" in result + # Login service DOES return the User object (unlike register) assert result["user"].email == "login@example.com" @@ -140,7 +146,9 @@ def test_get_user_by_id_success(user_service): ) result = user_service.register_user(user_in) - user_id = result["user"].id + # REPAIR: Extract ID + user_id = result["user"] + user = user_service.get_user_by_id(user_id) assert user.id == user_id assert user.email == "getuser@example.com" @@ -174,7 +182,9 @@ def test_update_user_success(user_service): phone_number="123456" ) result = user_service.register_user(user_in) - user_id = result["user"].id + + # REPAIR: Extract ID + user_id = result["user"] update_in = UserUpdate(first_name="NewName") @@ -190,7 +200,8 @@ def test_update_user_email_conflict(user_service): last_name="A", phone_number="123", )) - u1_id = u1_result["user"].id + # REPAIR: Extract ID + u1_id = u1_result["user"] user_service.register_user(UserCreate( email="b@example.com", @@ -214,7 +225,8 @@ def test_delete_user(user_service): last_name="User", phone_number="000", )) - user_id = r["user"].id + # REPAIR: Extract ID + user_id = r["user"] user_service.delete_user(user_id) @@ -222,12 +234,12 @@ def test_delete_user(user_service): user_service.get_user_by_id(user_id) -def test_request_password_reset_nonexistent(user_service, capsys): +def test_request_password_reset_nonexistent(user_service): result = user_service.request_password_reset("none@example.com") assert "Check the API console" in result["message"] -def test_request_password_reset_success(user_service, capsys): +def test_request_password_reset_success(user_service): user_service.register_user(UserCreate( email="reset@example.com", password="Password123", @@ -248,7 +260,8 @@ def test_reset_password_success(user_service): last_name="B", phone_number="123", )) - user_id = reg["user"].id + # REPAIR: Extract ID + user_id = reg["user"] token = user_service._create_reset_token(user_id) @@ -331,7 +344,8 @@ def test_change_password_success(user_service): phone_number="111" ) res = user_service.register_user(reg_data) - user_id = res["user"].id + # NOTE: register_user now returns an ID in 'user', not the object. + user_id = res["user"] # 2. Change Password user_service.change_password(user_id, "OldPassword123", "NewPassword999") @@ -360,7 +374,7 @@ def test_change_password_invalid_old_password(user_service): phone_number="222" ) res = user_service.register_user(reg_data) - user_id = res["user"].id + user_id = res["user"] # 2. Attempt change with WRONG old password with pytest.raises(HTTPException) as exc: From 83084c7c3d09fd9398b1e0e8962a2a2d70cdd55a Mon Sep 17 00:00:00 2001 From: eduart77 Date: Wed, 3 Dec 2025 23:02:20 +0200 Subject: [PATCH 3/3] added more tests for the user services and ruff check --- API/models/expense.py | 1 + API/repositories/expense_repository.py | 2 +- API/tests/test_user_service.py | 1 + 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/API/models/expense.py b/API/models/expense.py index 307f705..1b41eed 100644 --- a/API/models/expense.py +++ b/API/models/expense.py @@ -41,6 +41,7 @@ class Expense(Base): name="chk_expenses_one_fk" ), ) + @property def split_amount(self) -> float: """ diff --git a/API/repositories/expense_repository.py b/API/repositories/expense_repository.py index c419960..1ecd782 100644 --- a/API/repositories/expense_repository.py +++ b/API/repositories/expense_repository.py @@ -4,8 +4,8 @@ from models.category import Category from models.expense import Expense -from models.user_group import UserGroup from models.group import Group +from models.user_group import UserGroup from sqlalchemy import and_, asc, desc, or_, select from sqlalchemy.orm import Session, joinedload diff --git a/API/tests/test_user_service.py b/API/tests/test_user_service.py index ec4a93c..28f5781 100644 --- a/API/tests/test_user_service.py +++ b/API/tests/test_user_service.py @@ -4,6 +4,7 @@ from schemas.user import UserCreate, UserLogin, UserUpdate from services.user_service import ALGORITHM, SECRET_KEY, UserService + # Mock UserRepository class MockUserRepository: """