diff --git a/be/app/api/v1/endpoints/items.py b/be/app/api/v1/endpoints/items.py index f44554d..38d130f 100644 --- a/be/app/api/v1/endpoints/items.py +++ b/be/app/api/v1/endpoints/items.py @@ -56,7 +56,8 @@ async def create_list_item( current_user: UserModel = Depends(get_current_user), ): """Adds a new item to a specific list. User must have access to the list.""" - logger.info(f"User {current_user.email} adding item to list {list_id}: {item_in.name}") + user_email = current_user.email # Access email attribute before async operations + logger.info(f"User {user_email} adding item to list {list_id}: {item_in.name}") # Verify user has access to the target list try: await crud_list.check_list_permission(db=db, list_id=list_id, user_id=current_user.id) @@ -67,7 +68,7 @@ async def create_list_item( created_item = await crud_item.create_item( db=db, item_in=item_in, list_id=list_id, user_id=current_user.id ) - logger.info(f"Item '{created_item.name}' (ID: {created_item.id}) added to list {list_id} by user {current_user.email}.") + logger.info(f"Item '{created_item.name}' (ID: {created_item.id}) added to list {list_id} by user {user_email}.") return created_item @@ -84,7 +85,8 @@ async def read_list_items( # Add sorting/filtering params later if needed: sort_by: str = 'created_at', order: str = 'asc' ): """Retrieves all items for a specific list if the user has access.""" - logger.info(f"User {current_user.email} listing items for list {list_id}") + user_email = current_user.email # Access email attribute before async operations + logger.info(f"User {user_email} listing items for list {list_id}") # Verify user has access to the list try: await crud_list.check_list_permission(db=db, list_id=list_id, user_id=current_user.id) @@ -119,20 +121,21 @@ async def update_item( If the version does not match, a 409 Conflict is returned. Sets/unsets `completed_by_id` based on `is_complete` flag. """ - logger.info(f"User {current_user.email} attempting to update item ID: {item_id} with version {item_in.version}") + user_email = current_user.email # Access email attribute before async operations + logger.info(f"User {user_email} attempting to update item ID: {item_id} with version {item_in.version}") # Permission check is handled by get_item_and_verify_access dependency try: updated_item = await crud_item.update_item( db=db, item_db=item_db, item_in=item_in, user_id=current_user.id ) - logger.info(f"Item {item_id} updated successfully by user {current_user.email} to version {updated_item.version}.") + logger.info(f"Item {item_id} updated successfully by user {user_email} to version {updated_item.version}.") return updated_item except ConflictError as e: - logger.warning(f"Conflict updating item {item_id} for user {current_user.email}: {str(e)}") + logger.warning(f"Conflict updating item {item_id} for user {user_email}: {str(e)}") raise HTTPException(status_code=status.HTTP_409_CONFLICT, detail=str(e)) except Exception as e: - logger.error(f"Error updating item {item_id} for user {current_user.email}: {str(e)}") + logger.error(f"Error updating item {item_id} for user {user_email}: {str(e)}") raise HTTPException(status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, detail="An unexpected error occurred while updating the item.") @@ -157,12 +160,13 @@ async def delete_item( If `expected_version` is provided and does not match the item's current version, a 409 Conflict is returned. """ - logger.info(f"User {current_user.email} attempting to delete item ID: {item_id}, expected version: {expected_version}") + user_email = current_user.email # Access email attribute before async operations + logger.info(f"User {user_email} attempting to delete item ID: {item_id}, expected version: {expected_version}") # Permission check is handled by get_item_and_verify_access dependency if expected_version is not None and item_db.version != expected_version: logger.warning( - f"Conflict deleting item {item_id} for user {current_user.email}. " + f"Conflict deleting item {item_id} for user {user_email}. " f"Expected version {expected_version}, actual version {item_db.version}." ) raise HTTPException( @@ -171,5 +175,5 @@ async def delete_item( ) await crud_item.delete_item(db=db, item_db=item_db) - logger.info(f"Item {item_id} (version {item_db.version}) deleted successfully by user {current_user.email}.") + logger.info(f"Item {item_id} (version {item_db.version}) deleted successfully by user {user_email}.") return Response(status_code=status.HTTP_204_NO_CONTENT) \ No newline at end of file diff --git a/be/app/crud/item.py b/be/app/crud/item.py index 9bd8726..6fb7456 100644 --- a/be/app/crud/item.py +++ b/be/app/crud/item.py @@ -73,32 +73,37 @@ async def get_item_by_id(db: AsyncSession, item_id: int) -> Optional[ItemModel]: async def update_item(db: AsyncSession, item_db: ItemModel, item_in: ItemUpdate, user_id: int) -> ItemModel: """Updates an existing item record, checking for version conflicts.""" try: - async with db.begin(): - if item_db.version != item_in.version: - raise ConflictError( - f"Item '{item_db.name}' (ID: {item_db.id}) has been modified. " - f"Your version is {item_in.version}, current version is {item_db.version}. Please refresh." - ) + # Check version conflict + if item_db.version != item_in.version: + raise ConflictError( + f"Item '{item_db.name}' (ID: {item_db.id}) has been modified. " + f"Your version is {item_in.version}, current version is {item_db.version}. Please refresh." + ) - update_data = item_in.model_dump(exclude_unset=True, exclude={'version'}) # Exclude version + update_data = item_in.model_dump(exclude_unset=True, exclude={'version'}) # Exclude version - # Special handling for is_complete - if 'is_complete' in update_data: - if update_data['is_complete'] is True: - if item_db.completed_by_id is None: # Only set if not already completed by someone - update_data['completed_by_id'] = user_id - else: - update_data['completed_by_id'] = None # Clear if marked incomplete - - for key, value in update_data.items(): - setattr(item_db, key, value) + # Special handling for is_complete + if 'is_complete' in update_data: + if update_data['is_complete'] is True: + if item_db.completed_by_id is None: # Only set if not already completed by someone + update_data['completed_by_id'] = user_id + else: + update_data['completed_by_id'] = None # Clear if marked incomplete + + # Apply updates + for key, value in update_data.items(): + setattr(item_db, key, value) - item_db.version += 1 # Increment version + item_db.version += 1 # Increment version - db.add(item_db) - await db.flush() - await db.refresh(item_db) - return item_db + db.add(item_db) + await db.flush() + await db.refresh(item_db) + + # Commit the transaction if not part of a larger transaction + await db.commit() + + return item_db except IntegrityError as e: await db.rollback() raise DatabaseIntegrityError(f"Failed to update item due to integrity constraint: {str(e)}") @@ -115,10 +120,9 @@ async def update_item(db: AsyncSession, item_db: ItemModel, item_in: ItemUpdate, async def delete_item(db: AsyncSession, item_db: ItemModel) -> None: """Deletes an item record. Version check should be done by the caller (API endpoint).""" try: - async with db.begin(): - await db.delete(item_db) - # await db.commit() # Not needed with async with db.begin() - return None + await db.delete(item_db) + await db.commit() + return None except OperationalError as e: await db.rollback() raise DatabaseConnectionError(f"Database connection error while deleting item: {str(e)}")