update

Backend/src/bookings/routes/booking_routes.py

@@ -1,6 +1,7 @@
 from fastapi import APIRouter, Depends, HTTPException, status, Query
 from sqlalchemy.orm import Session, joinedload, selectinload, load_only
 from sqlalchemy import and_, or_, func
+from sqlalchemy.exc import IntegrityError
 from typing import Optional
 from datetime import datetime
 import random
@@ -25,6 +26,7 @@ from ...loyalty.services.loyalty_service import LoyaltyService
 from ...shared.utils.currency_helpers import get_currency_symbol
 from ...shared.utils.response_helpers import success_response
 from ..schemas.booking import CreateBookingRequest, UpdateBookingRequest
+from ..schemas.admin_booking import AdminCreateBookingRequest
 router = APIRouter(prefix='/bookings', tags=['bookings'])
 
 def _generate_invoice_email_html(invoice: dict, is_proforma: bool=False) -> str:
@@ -176,9 +178,13 @@ async def create_booking(booking_data: CreateBookingRequest, current_user: User=
     role = db.query(Role).filter(Role.id == current_user.role_id).first()
     if role and role.name in ['admin', 'staff', 'accountant']:
         raise HTTPException(status_code=403, detail='Admin, staff, and accountant users cannot create bookings')
+    
+    import logging
+    logger = logging.getLogger(__name__)
+    
+    # Start transaction
+    transaction = db.begin()
     try:
-        import logging
-        logger = logging.getLogger(__name__)
         logger.info(f'Received booking request from user {current_user.id}: {booking_data.dict()}')
         
         # Extract validated data from Pydantic model
@@ -194,8 +200,10 @@ async def create_booking(booking_data: CreateBookingRequest, current_user: User=
         services = booking_data.services or []
         invoice_info = booking_data.invoice_info.dict() if booking_data.invoice_info else {}
         
-        room = db.query(Room).filter(Room.id == room_id).first()
+        # Lock room row to prevent race conditions (SELECT FOR UPDATE)
+        room = db.query(Room).filter(Room.id == room_id).with_for_update().first()
         if not room:
+            transaction.rollback()
             raise HTTPException(status_code=404, detail='Room not found')
         
         # Parse dates (schema validation already ensures format is valid)
@@ -210,9 +218,20 @@ async def create_booking(booking_data: CreateBookingRequest, current_user: User=
         
         # Date validation already done in schema, but keeping as safety check
         if check_in >= check_out:
+            transaction.rollback()
             raise HTTPException(status_code=400, detail='Check-out date must be after check-in date')
-        overlapping = db.query(Booking).filter(and_(Booking.room_id == room_id, Booking.status != BookingStatus.cancelled, Booking.check_in_date < check_out, Booking.check_out_date > check_in)).first()
+        
+        # Check for overlapping bookings with row-level locking to prevent race conditions
+        overlapping = db.query(Booking).filter(
+            and_(
+                Booking.room_id == room_id,
+                Booking.status != BookingStatus.cancelled,
+                Booking.check_in_date < check_out,
+                Booking.check_out_date > check_in
+            )
+        ).with_for_update().first()
         if overlapping:
+            transaction.rollback()
             raise HTTPException(status_code=409, detail='Room already booked for the selected dates')
         
         # Check for maintenance blocks
@@ -370,19 +389,20 @@ async def create_booking(booking_data: CreateBookingRequest, current_user: User=
                 total_price = unit_price * quantity
                 service_usage = ServiceUsage(booking_id=booking.id, service_id=service_id, quantity=quantity, unit_price=unit_price, total_price=total_price)
                 db.add(service_usage)
-        db.commit()
+        
+        # Commit transaction - all database operations are atomic
+        transaction.commit()
         db.refresh(booking)
         
-        # Send booking confirmation notification
+        # Send booking confirmation notification (outside transaction)
         try:
             from ...notifications.services.notification_service import NotificationService
             if booking.status == BookingStatus.confirmed:
                 NotificationService.send_booking_confirmation(db, booking)
         except Exception as e:
-            import logging
-            logger = logging.getLogger(__name__)
             logger.warning(f'Failed to send booking confirmation notification: {e}')
         
+        # Create invoice (outside transaction to avoid nested transaction issues)
         try:
             from ...payments.services.invoice_service import InvoiceService
             from ...shared.utils.mailer import send_email
@@ -486,8 +506,17 @@ async def create_booking(booking_data: CreateBookingRequest, current_user: User=
         message = f'Booking created. Please pay {deposit_percentage}% deposit to confirm.' if requires_deposit else 'Booking created successfully'
         return success_response(data={'booking': booking_dict}, message=message)
     except HTTPException:
+        if 'transaction' in locals():
+            transaction.rollback()
         raise
+    except IntegrityError as e:
+        transaction.rollback()
+        logger.error(f'Database integrity error during booking creation: {str(e)}')
+        raise HTTPException(status_code=409, detail='Booking conflict detected. Please try again.')
     except Exception as e:
+        if 'transaction' in locals():
+            transaction.rollback()
+        logger.error(f'Error creating booking: {str(e)}', exc_info=True)
         import logging
         import traceback
         logger = logging.getLogger(__name__)
@@ -745,7 +774,14 @@ async def update_booking(id: int, booking_data: UpdateBookingRequest, current_us
         if booking_data.notes is not None:
             booking.special_requests = booking_data.notes
         
+        # Restrict staff from modifying booking prices (only admin can)
         if booking_data.total_price is not None:
+            from ...shared.utils.role_helpers import is_admin
+            if not is_admin(current_user, db):
+                raise HTTPException(
+                    status_code=403,
+                    detail='Staff members cannot modify booking prices. Please contact an administrator.'
+                )
             booking.total_price = booking_data.total_price
         
         db.commit()
@@ -908,60 +944,44 @@ async def check_booking_by_number(booking_number: str, db: Session=Depends(get_d
         raise HTTPException(status_code=500, detail=str(e))
 
 @router.post('/admin-create', dependencies=[Depends(authorize_roles('admin', 'staff'))])
-async def admin_create_booking(booking_data: dict, current_user: User=Depends(authorize_roles('admin', 'staff')), db: Session=Depends(get_db)):
+async def admin_create_booking(booking_data: AdminCreateBookingRequest, current_user: User=Depends(authorize_roles('admin', 'staff')), db: Session=Depends(get_db)):
     """Create a booking on behalf of a user (admin/staff only)"""
+    import logging
+    logger = logging.getLogger(__name__)
+    
+    # Start transaction
+    transaction = db.begin()
     try:
-        import logging
-        logger = logging.getLogger(__name__)
-        if not isinstance(booking_data, dict):
-            logger.error(f'Invalid booking_data type: {type(booking_data)}, value: {booking_data}')
-            raise HTTPException(status_code=400, detail='Invalid request body. Expected JSON object.')
-        
-        # Get user_id from booking_data (required for admin/staff bookings)
-        user_id = booking_data.get('user_id')
-        if not user_id:
-            raise HTTPException(status_code=400, detail='user_id is required for admin/staff bookings')
+        # Extract validated data from Pydantic model
+        user_id = booking_data.user_id
+        room_id = booking_data.room_id
+        check_in_date = booking_data.check_in_date
+        check_out_date = booking_data.check_out_date
+        total_price = booking_data.total_price
+        guest_count = booking_data.guest_count
+        notes = booking_data.notes
+        payment_method = booking_data.payment_method
+        payment_status = booking_data.payment_status
+        promotion_code = booking_data.promotion_code
+        status = booking_data.status
+        services = booking_data.services or []
+        invoice_info = booking_data.invoice_info.dict() if booking_data.invoice_info else {}
         
         # Verify user exists
         target_user = db.query(User).filter(User.id == user_id).first()
         if not target_user:
+            transaction.rollback()
             raise HTTPException(status_code=404, detail='User not found')
         
-        logger.info(f'Admin/Staff {current_user.id} creating booking for user {user_id}: {booking_data}')
+        logger.info(f'Admin/Staff {current_user.id} creating booking for user {user_id}')
         
-        room_id = booking_data.get('room_id')
-        check_in_date = booking_data.get('check_in_date')
-        check_out_date = booking_data.get('check_out_date')
-        total_price = booking_data.get('total_price')
-        guest_count = booking_data.get('guest_count', 1)
-        if guest_count < 1 or guest_count > 20:
-            raise HTTPException(status_code=400, detail='Guest count must be between 1 and 20')
-        notes = booking_data.get('notes')
-        payment_method = booking_data.get('payment_method', 'cash')
-        payment_status = booking_data.get('payment_status', 'unpaid')  # 'full', 'deposit', or 'unpaid'
-        promotion_code = booking_data.get('promotion_code')
-        status = booking_data.get('status', 'confirmed')  # Default to confirmed for admin bookings
-        invoice_info = booking_data.get('invoice_info', {})
-        
-        missing_fields = []
-        if not room_id:
-            missing_fields.append('room_id')
-        if not check_in_date:
-            missing_fields.append('check_in_date')
-        if not check_out_date:
-            missing_fields.append('check_out_date')
-        if total_price is None:
-            missing_fields.append('total_price')
-        if missing_fields:
-            error_msg = f'Missing required booking fields: {', '.join(missing_fields)}'
-            logger.error(error_msg)
-            raise HTTPException(status_code=400, detail=error_msg)
-        
-        room = db.query(Room).filter(Room.id == room_id).first()
+        # Lock room row to prevent race conditions
+        room = db.query(Room).filter(Room.id == room_id).with_for_update().first()
         if not room:
+            transaction.rollback()
             raise HTTPException(status_code=404, detail='Room not found')
         
-        # Parse dates
+        # Parse dates (already validated by Pydantic)
         if 'T' in check_in_date or 'Z' in check_in_date or '+' in check_in_date:
             check_in = datetime.fromisoformat(check_in_date.replace('Z', '+00:00'))
         else:
@@ -971,11 +991,7 @@ async def admin_create_booking(booking_data: dict, current_user: User=Depends(au
         else:
             check_out = datetime.strptime(check_out_date, '%Y-%m-%d')
         
-        # Validate dates
-        if check_in >= check_out:
-            raise HTTPException(status_code=400, detail='Check-out date must be after check-in date')
-        
-        # Check for overlapping bookings
+        # Check for overlapping bookings with row-level locking
         overlapping = db.query(Booking).filter(
             and_(
                 Booking.room_id == room_id,
@@ -983,8 +999,9 @@ async def admin_create_booking(booking_data: dict, current_user: User=Depends(au
                 Booking.check_in_date < check_out,
                 Booking.check_out_date > check_in
             )
-        ).first()
+        ).with_for_update().first()
         if overlapping:
+            transaction.rollback()
             raise HTTPException(status_code=409, detail='Room already booked for the selected dates')
         
         booking_number = generate_booking_number()
@@ -996,17 +1013,15 @@ async def admin_create_booking(booking_data: dict, current_user: User=Depends(au
         room_total = room_price * number_of_nights
         
         # Calculate services total if any
-        services = booking_data.get('services', [])
         services_total = 0.0
         if services:
             from ...hotel_services.models.service import Service
             for service_item in services:
-                service_id = service_item.get('service_id')
-                quantity = service_item.get('quantity', 1)
-                if service_id:
-                    service = db.query(Service).filter(Service.id == service_id).first()
-                    if service and service.is_active:
-                        services_total += float(service.price) * quantity
+                service_id = service_item.service_id
+                quantity = service_item.quantity
+                service = db.query(Service).filter(Service.id == service_id).first()
+                if service and service.is_active:
+                    services_total += float(service.price) * quantity
         
         original_price = room_total + services_total
         
@@ -1181,10 +1196,8 @@ async def admin_create_booking(booking_data: dict, current_user: User=Depends(au
         if services:
             from ...hotel_services.models.service import Service
             for service_item in services:
-                service_id = service_item.get('service_id')
-                quantity = service_item.get('quantity', 1)
-                if not service_id:
-                    continue
+                service_id = service_item.service_id
+                quantity = service_item.quantity
                 service = db.query(Service).filter(Service.id == service_id).first()
                 if not service or not service.is_active:
                     continue
@@ -1199,7 +1212,8 @@ async def admin_create_booking(booking_data: dict, current_user: User=Depends(au
                 )
                 db.add(service_usage)
         
-        db.commit()
+        # Commit transaction
+        transaction.commit()
         db.refresh(booking)
         
         # Load booking with relationships
@@ -1305,12 +1319,16 @@ async def admin_create_booking(booking_data: dict, current_user: User=Depends(au
             message=f'Booking created successfully by {current_user.full_name}'
         )
     except HTTPException:
+        if 'transaction' in locals():
+            transaction.rollback()
         raise
+    except IntegrityError as e:
+        if 'transaction' in locals():
+            transaction.rollback()
+        logger.error(f'Database integrity error during admin booking creation: {str(e)}')
+        raise HTTPException(status_code=409, detail='Booking conflict detected. Please try again.')
     except Exception as e:
-        import logging
-        import traceback
-        logger = logging.getLogger(__name__)
-        logger.error(f'Error creating booking (admin/staff): {str(e)}')
-        logger.error(f'Traceback: {traceback.format_exc()}')
-        db.rollback()
-        raise HTTPException(status_code=500, detail=str(e))
+        if 'transaction' in locals():
+            transaction.rollback()
+        logger.error(f'Error creating booking (admin/staff): {str(e)}', exc_info=True)
+        raise HTTPException(status_code=500, detail='An error occurred while creating the booking')