From ede37d5d0f08ebe06b4142ae50aea0420c02051d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Fromme?= Date: Fri, 26 Sep 2025 09:00:05 +0200 Subject: [PATCH] feat: improved error handling in controllers, cleanup --- src/BusProNet/XmlLoader/TravelLoader.php | 4 +- src/Controller/Booking/BookingCreateTrait.php | 18 ++-- .../Booking/BookingExceptionHandlerTrait.php | 66 ++++++++++++ .../Booking/CreateInitController.php | 12 +++ .../Booking/CreateStep1Controller.php | 22 ++-- .../Booking/CreateStep2Controller.php | 46 +++++--- .../Booking/CreateStep3Controller.php | 16 +-- .../BookingSessionNotFoundException.php | 23 ++++ .../DataTransformer/XssCleanTransformer.php | 29 ----- src/Service/BookingService.php | 101 ++++++++++-------- templates/booking/create_error.html.twig | 43 ++++++++ 11 files changed, 261 insertions(+), 119 deletions(-) create mode 100644 src/Controller/Booking/BookingExceptionHandlerTrait.php create mode 100644 src/Exception/BookingSessionNotFoundException.php delete mode 100644 src/Form/DataTransformer/XssCleanTransformer.php create mode 100644 templates/booking/create_error.html.twig diff --git a/src/BusProNet/XmlLoader/TravelLoader.php b/src/BusProNet/XmlLoader/TravelLoader.php index cc62571..63972a9 100644 --- a/src/BusProNet/XmlLoader/TravelLoader.php +++ b/src/BusProNet/XmlLoader/TravelLoader.php @@ -314,7 +314,7 @@ class TravelLoader extends AbstractLoader $travelData = $this->loadById($travelId); $booking->travelData = $travelData; - if (null === $travelData || null === $travelData->dateFrom) { + if (null === $travelData->dateFrom) { continue; } @@ -322,7 +322,7 @@ class TravelLoader extends AbstractLoader $travelDate = $travelData->dateFrom; $travelCode = $travelData->code; - // generate url to travelinfo page five days before travel begins + // generate url to travel info page five days before travel begins if ($travelDate->modify('-5 days') < $now) { $booking->travelInfoUrl = sprintf( '%s/%s', diff --git a/src/Controller/Booking/BookingCreateTrait.php b/src/Controller/Booking/BookingCreateTrait.php index 5000401..abf9f44 100644 --- a/src/Controller/Booking/BookingCreateTrait.php +++ b/src/Controller/Booking/BookingCreateTrait.php @@ -17,16 +17,20 @@ use Symfony\Component\HttpFoundation\RedirectResponse; trait BookingCreateTrait { /** - * Validates step access and redirects if necessary. + * Validates step access and returns redirect response if necessary. + * + * @return RedirectResponse|null Returns redirect response if validation fails, null if access is allowed */ - private function validateStepAccess(BookingCreateDto $bookingCreateDto, int $expectedStep): void + private function validateStepAccess(BookingCreateDto $bookingCreateDto, int $expectedStep): ?RedirectResponse { // Allow access to current step or any previous step if ($expectedStep > $bookingCreateDto->currentStep) { $this->addFlash('error', 'Bitte erst die vorherigen Schritte abschließen.'); - $this->redirectToCurrentStep($bookingCreateDto); + return $this->redirectToCurrentStep($bookingCreateDto); } + + return null; } /** @@ -34,18 +38,12 @@ trait BookingCreateTrait */ private function redirectToCurrentStep(BookingCreateDto $bookingCreateDto): RedirectResponse { - $routeParams = [ - 'date_id' => $bookingCreateDto->travel->id, - 'hotel_id' => $bookingCreateDto->travel->hotelId, - ]; - $route = match ($bookingCreateDto->currentStep) { - 1 => 'app_booking_create_step_1', 2 => 'app_booking_create_step_2', 3 => 'app_booking_create_step_3', default => 'app_booking_create_step_1', }; - return $this->redirectToRoute($route, $routeParams); + return $this->redirectToRoute($route); } } diff --git a/src/Controller/Booking/BookingExceptionHandlerTrait.php b/src/Controller/Booking/BookingExceptionHandlerTrait.php new file mode 100644 index 0000000..64b2321 --- /dev/null +++ b/src/Controller/Booking/BookingExceptionHandlerTrait.php @@ -0,0 +1,66 @@ +getOrCreateBookingCreateDto($request); + } catch (BookingSessionNotFoundException $e) { + $this->addFlash('error', 'Ihre Buchungssitzung ist abgelaufen. Bitte starten Sie eine neue Buchung.'); + return $this->redirectToRoute('app_booking_create_error'); + } catch (TravelNotFoundException $e) { + $this->addFlash('error', 'Die angeforderte Reise wurde nicht gefunden.'); + return $this->redirectToRoute('app_booking_create_error'); + } catch (HotelNotFoundException $e) { + $this->addFlash('error', 'Das angeforderte Hotel wurde nicht gefunden.'); + return $this->redirectToRoute('app_booking_create_error'); + } catch (HotelNotInTravelException $e) { + $this->addFlash('error', 'Das Hotel ist für diese Reise nicht verfügbar.'); + return $this->redirectToRoute('app_booking_create_error'); + } catch (NoRoomsAvailableException $e) { + $this->addFlash('error', 'Für diese Reise sind aktuell keine Zimmer verfügbar.'); + return $this->redirectToRoute('app_booking_create_error'); + } + } + + /** + * Safely retrieves booking DTO for HTMX requests with lightweight error responses. + * + * Returns empty 400 responses for HTMX requests when exceptions occur, + * allowing the frontend to handle errors appropriately. + */ + protected function getOrCreateBookingCreateDtoForHtmx(BookingService $bookingService, Request $request): mixed + { + try { + return $bookingService->getOrCreateBookingCreateDto($request); + } catch (BookingSessionNotFoundException | TravelNotFoundException | HotelNotFoundException | HotelNotInTravelException | NoRoomsAvailableException $e) { + return new Response('', 400); + } + } +} \ No newline at end of file diff --git a/src/Controller/Booking/CreateInitController.php b/src/Controller/Booking/CreateInitController.php index 80f9a5c..1e18bd5 100644 --- a/src/Controller/Booking/CreateInitController.php +++ b/src/Controller/Booking/CreateInitController.php @@ -57,4 +57,16 @@ class CreateInitController extends AbstractController throw $this->createNotFoundException('No rooms available for this travel.'); } } + + /** + * Displays user-friendly error messages for booking initialization failures. + * + * This endpoint provides a centralized location for displaying booking errors + * with appropriate error messages and guidance for users. + */ + #[Route('/bookings/create/error', name: 'app_booking_create_error')] + public function error(Request $request): Response + { + return $this->render('booking/create_error.html.twig'); + } } diff --git a/src/Controller/Booking/CreateStep1Controller.php b/src/Controller/Booking/CreateStep1Controller.php index bc60751..b171f4f 100644 --- a/src/Controller/Booking/CreateStep1Controller.php +++ b/src/Controller/Booking/CreateStep1Controller.php @@ -4,7 +4,6 @@ declare(strict_types=1); namespace App\Controller\Booking; -use App\Exception\NoRoomsAvailableException; use App\Form\BookingCreateStep1Type; use App\Service\BookingService; use Symfony\Bundle\FrameworkBundle\Controller\AbstractController; @@ -21,6 +20,7 @@ use Symfony\Component\Routing\Attribute\Route; class CreateStep1Controller extends AbstractController { use BookingCreateTrait; + use BookingExceptionHandlerTrait; public function __construct( private readonly BookingService $bookingService, @@ -33,11 +33,11 @@ class CreateStep1Controller extends AbstractController #[Route('/bookings/create', name: 'app_booking_create_step_1')] public function index(Request $request): Response { - try { - $bookingCreateDto = $this->bookingService->getOrCreateBookingCreateDto($request); - } catch (NoRoomsAvailableException $e) { - throw $this->createNotFoundException('No rooms available for this travel.'); + $result = $this->getOrCreateBookingCreateDto($this->bookingService, $request); + if ($result instanceof Response) { + return $result; } + $bookingCreateDto = $result; // Get or create baseline snapshot for change detection $oldRoomSelectionSnapshot = $this->bookingService->getOrCreateBaselineSnapshot($request, $bookingCreateDto); @@ -45,7 +45,9 @@ class CreateStep1Controller extends AbstractController $summary = $this->bookingService->getRoomSummaryAndParticipantCount($bookingCreateDto); // Validate step access - allow step 1 or redirect to current step - $this->validateStepAccess($bookingCreateDto, 1); + if ($redirect = $this->validateStepAccess($bookingCreateDto, 1)) { + return $redirect; + } $form = $this->createForm(BookingCreateStep1Type::class, $bookingCreateDto, [ 'validation_groups' => ['booking_create_step_1'], @@ -88,11 +90,11 @@ class CreateStep1Controller extends AbstractController #[Route('/bookings/create/room-summary', name: 'app_booking_create_step_1_room_summary', methods: ['POST'])] public function roomSummary(Request $request): Response { - try { - $bookingCreateDto = $this->bookingService->getOrCreateBookingCreateDto($request); - } catch (NoRoomsAvailableException $e) { - throw $this->createNotFoundException('No rooms available for this travel.'); + $result = $this->getOrCreateBookingCreateDto($this->bookingService, $request); + if ($result instanceof Response) { + return $result; } + $bookingCreateDto = $result; // Process the form to update the DTO with the latest room selection $form = $this->createForm(BookingCreateStep1Type::class, $bookingCreateDto, [ diff --git a/src/Controller/Booking/CreateStep2Controller.php b/src/Controller/Booking/CreateStep2Controller.php index b095a70..8a05ac5 100644 --- a/src/Controller/Booking/CreateStep2Controller.php +++ b/src/Controller/Booking/CreateStep2Controller.php @@ -5,7 +5,6 @@ declare(strict_types=1); namespace App\Controller\Booking; use App\Controller\Traits\HtmxControllerTrait; -use App\Exception\NoRoomsAvailableException; use App\Form\BookingCreateStep2Type; use App\Form\Model\BookingCreateDto; use App\Form\Model\ParticipantDto; @@ -25,6 +24,7 @@ use Symfony\Component\Routing\Attribute\Route; class CreateStep2Controller extends AbstractController { use BookingCreateTrait; + use BookingExceptionHandlerTrait; use HtmxControllerTrait; public function __construct( @@ -39,21 +39,19 @@ class CreateStep2Controller extends AbstractController #[Route('/bookings/create/participants', name: 'app_booking_create_step_2')] public function participants(Request $request): Response { - try { - $bookingCreateDto = $this - ->bookingService - ->getOrCreateBookingCreateDto($request); - } catch (NoRoomsAvailableException $e) { - $this->addFlash('error', 'Leider sind für diese Reise aktuell keine Zimmer verfügbar.'); - // TODO: Redirect to travel listing or hotel details page - throw $this->createNotFoundException('No rooms available for this travel.'); + $result = $this->getOrCreateBookingCreateDto($this->bookingService, $request); + if ($result instanceof Response) { + return $result; } + $bookingCreateDto = $result; // Enrich with fresh availability data $this->enrichWithFreshAvailabilities($bookingCreateDto); // Validate step access - $this->validateStepAccess($bookingCreateDto, 2); + if ($redirect = $this->validateStepAccess($bookingCreateDto, 2)) { + return $redirect; + } // Ensure correct number of participants $this->ensureCorrectNumberOfParticipants($bookingCreateDto); @@ -100,12 +98,11 @@ class CreateStep2Controller extends AbstractController #[Route('/bookings/create/participants/refresh', name: 'app_booking_create_step_2_refresh', methods: ['POST'])] public function refreshParticipantForm(Request $request): Response { - try { - $bookingCreateDto = $this->bookingService->getOrCreateBookingCreateDto($request); - } catch (NoRoomsAvailableException $e) { - // For HTMX requests, return a simple error message - return new Response('
Keine Zimmer verfügbar
', 400); + $result = $this->getOrCreateBookingCreateDtoForHtmx($this->bookingService, $request); + if ($result instanceof Response) { + return $result; } + $bookingCreateDto = $result; // Enrich with fresh availability data $this->enrichWithFreshAvailabilities($bookingCreateDto); @@ -152,6 +149,13 @@ class CreateStep2Controller extends AbstractController ); } + /** + * Calculates the total number of participants based on room selections. + * + * @param BookingCreateDto $bookingCreateDto The booking DTO containing room selections + * + * @return int Total number of participants required + */ private function getParticipantsCount(BookingCreateDto $bookingCreateDto): int { return $this @@ -159,6 +163,15 @@ class CreateStep2Controller extends AbstractController ->getParticipantsCount($bookingCreateDto->roomSelections, $bookingCreateDto->travel); } + /** + * Ensures the booking DTO has the correct number of participant objects. + * + * Creates or reuses participant DTOs to match the required participant count + * based on room selections. Preserves existing participant data when possible + * and assigns proper index values. + * + * @param BookingCreateDto $bookingCreateDto The booking DTO to update + */ private function ensureCorrectNumberOfParticipants(BookingCreateDto $bookingCreateDto): void { $participantsCount = $this->getParticipantsCount($bookingCreateDto); @@ -177,6 +190,9 @@ class CreateStep2Controller extends AbstractController * * Fetches availability data with short-term caching and patches the travel object * to ensure service availability is reasonably up-to-date while reducing API calls. + * This is essential for accurate pricing and service selection during the booking process. + * + * @param BookingCreateDto $bookingCreateDto The booking DTO containing travel data to enrich */ private function enrichWithFreshAvailabilities(BookingCreateDto $bookingCreateDto): void { diff --git a/src/Controller/Booking/CreateStep3Controller.php b/src/Controller/Booking/CreateStep3Controller.php index 065e19f..6b31450 100644 --- a/src/Controller/Booking/CreateStep3Controller.php +++ b/src/Controller/Booking/CreateStep3Controller.php @@ -4,7 +4,6 @@ declare(strict_types=1); namespace App\Controller\Booking; -use App\Exception\NoRoomsAvailableException; use App\Service\BookingService; use Symfony\Bundle\FrameworkBundle\Controller\AbstractController; use Symfony\Component\HttpFoundation\Request; @@ -20,6 +19,7 @@ use Symfony\Component\Routing\Attribute\Route; class CreateStep3Controller extends AbstractController { use BookingCreateTrait; + use BookingExceptionHandlerTrait; public function __construct( private readonly BookingService $bookingCreateService, @@ -32,16 +32,16 @@ class CreateStep3Controller extends AbstractController #[Route('/bookings/create/confirm', name: 'app_booking_create_step_3')] public function confirm(Request $request): Response { - try { - $bookingCreateDto = $this->bookingCreateService->getOrCreateBookingCreateDto($request); - } catch (NoRoomsAvailableException $e) { - $this->addFlash('error', 'Leider sind für diese Reise aktuell keine Zimmer verfügbar.'); - // TODO: Redirect to travel listing or hotel details page - throw $this->createNotFoundException('No rooms available for this travel.'); + $result = $this->getOrCreateBookingCreateDto($this->bookingCreateService, $request); + if ($result instanceof Response) { + return $result; } + $bookingCreateDto = $result; // Validate step access - $this->validateStepAccess($bookingCreateDto, 3); + if ($redirect = $this->validateStepAccess($bookingCreateDto, 3)) { + return $redirect; + } return $this->render('booking/create_step_3.html.twig', [ 'bookingCreateDto' => $bookingCreateDto, diff --git a/src/Exception/BookingSessionNotFoundException.php b/src/Exception/BookingSessionNotFoundException.php new file mode 100644 index 0000000..e0ab5f6 --- /dev/null +++ b/src/Exception/BookingSessionNotFoundException.php @@ -0,0 +1,23 @@ +antiXss) { - $this->antiXss = new AntiXSS(); - } - - return $this->antiXss->xss_clean($value); - } -} diff --git a/src/Service/BookingService.php b/src/Service/BookingService.php index f2ef117..18bc9f7 100644 --- a/src/Service/BookingService.php +++ b/src/Service/BookingService.php @@ -4,6 +4,7 @@ namespace App\Service; use App\BusProNet\Model\Room; use App\BusProNet\Model\Travel; +use App\Exception\BookingSessionNotFoundException; use App\Exception\NoRoomsAvailableException; use App\Form\Model\BookingCreateDto; use App\Form\Model\RoomSelectionDto; @@ -53,6 +54,19 @@ class BookingService $request->getSession()->remove('booking_create_baseline_snapshot'); } + /** + * Retrieves the booking creation DTO from the session. + * + * This method enforces the secure booking flow by only returning existing + * session data. Users must go through the proper initialization flow via + * CreateInitController to create new booking sessions. + * + * @param Request $request The HTTP request containing session data + * + * @return BookingCreateDto The booking DTO from session + * + * @throws BookingSessionNotFoundException When no valid booking session exists + */ public function getOrCreateBookingCreateDto(Request $request): BookingCreateDto { $bookingCreateDto = $request->getSession()->get(self::BOOKING_CREATE_KEY); @@ -62,40 +76,19 @@ class BookingService return $bookingCreateDto; } - // Legacy support: Create a new DTO if date_id and hotel_id are provided - // This maintains backward compatibility for existing URLs with UID parameters - $dateId = $request->query->getInt('date_id'); - $hotelId = $request->query->getInt('hotel_id'); - - if (0 === $dateId || 0 === $hotelId) { - throw new NotFoundHttpException('No booking session found. Please start a new booking.'); - } - - $travelData = $this->travelDataService->getTravelData($dateId, $hotelId); - if (null === $travelData) { - throw new NotFoundHttpException(sprintf('Travel data not found for date ID %d and hotel ID %d', $dateId, $hotelId)); - } - $roomsIdsAndQuantities = $this->processRoomQuantities($request); - $availableRooms = $travelData->getAvailableRooms(); - - // Prevent booking flow entry when no rooms are available - if (empty($availableRooms)) { - throw new NoRoomsAvailableException($dateId, $hotelId); - } - - $roomSelections = array_map( - fn (Room $room) => $this->createRoomSelection($room, $roomsIdsAndQuantities), - $availableRooms - ); - - $bookingCreateDto = new BookingCreateDto($travelData, $hotelId); - $bookingCreateDto->roomSelections = $roomSelections; - - $this->saveBookingCreateDto($request, $bookingCreateDto); - - return $bookingCreateDto; + // No session found - user must go through proper init flow + throw new BookingSessionNotFoundException(); } + /** + * Saves the booking creation DTO to the session. + * + * Persists the current booking state to the session for retrieval + * across multiple HTTP requests during the booking flow. + * + * @param Request $request The HTTP request with session + * @param BookingCreateDto $bookingCreateDto The booking DTO to persist + */ public function saveBookingCreateDto(Request $request, BookingCreateDto $bookingCreateDto): void { $request->getSession()->set(self::BOOKING_CREATE_KEY, $bookingCreateDto); @@ -150,6 +143,17 @@ class BookingService return $bookingCreateDto; } + /** + * Creates a room selection DTO from room data and quantities. + * + * Converts a Room model into a RoomSelectionDto with the specified quantity + * selection. Used during booking initialization to create selectable room options. + * + * @param Room $room The room model to convert + * @param array $roomsIdsAndQuantities Array of room ID to quantity mappings + * + * @return RoomSelectionDto The room selection DTO + */ private function createRoomSelection(Room $room, array $roomsIdsAndQuantities): RoomSelectionDto { $selection = new RoomSelectionDto(); @@ -163,23 +167,16 @@ class BookingService } /** - * Extracts room IDs and quantities from the request data. + * Calculates the total number of participants based on room selections. * - * Processes the 'rooms' parameter from the request to extract room IDs - * as keys and their corresponding quantities as integer values. Filters - * out empty values and converts all quantities to integers. + * Multiplies each room's minimum occupancy (minPax) by the selected quantity + * to determine the total number of participants required for the booking. * - * @param Request $request The HTTP request containing room data + * @param array $roomSelections Array of RoomSelectionDto objects + * @param Travel $travelData Travel data containing room information * - * @return array Array with room IDs as keys and quantities as values + * @return int Total number of participants required */ - public function processRoomQuantities(Request $request): array - { - $rooms = $request->request->all('rooms'); - - return array_map('intval', array_filter($rooms, 'strlen')); - } - public function getParticipantsCount(array $roomSelections, Travel $travelData): int { $participantsCount = 0; @@ -279,6 +276,12 @@ class BookingService /** * Resets all participant room assignments in the DTO. + * + * Clears room assignments when room selections change to prevent + * invalid assignments. Called when users modify their room selections + * in step 1 to ensure participants are reassigned appropriately. + * + * @param BookingCreateDto $dto The booking DTO to reset assignments for */ public function resetParticipantAssignments(BookingCreateDto $dto): void { @@ -302,6 +305,14 @@ class BookingService /** * Checks if room selection has changed compared to a previous snapshot. + * + * Compares the current room selection state with a baseline snapshot + * to detect changes that would require participant reassignment. + * + * @param array $oldSnapshot The baseline room selection snapshot + * @param BookingCreateDto $newDto The current booking DTO + * + * @return bool True if room selections have changed, false otherwise */ public function hasRoomSelectionChanged(array $oldSnapshot, BookingCreateDto $newDto): bool { diff --git a/templates/booking/create_error.html.twig b/templates/booking/create_error.html.twig new file mode 100644 index 0000000..8c073e2 --- /dev/null +++ b/templates/booking/create_error.html.twig @@ -0,0 +1,43 @@ +{% extends 'layout.html.twig' %} + +{% block title %}Booking Error{% endblock %} + +{% block body %} +
+
+
+
+
+ + + +
+
+

+ Booking Error +

+
+ {% for flash_message in app.flashes('error') %} +

{{ flash_message }}

+ {% endfor %} + + {% if app.flashes('error') is empty %} +

Es ist ein Fehler beim Starten der Buchung aufgetreten. Bitte versuchen Sie es erneut.

+ {% endif %} +
+
+
+ + Zur Startseite + + +
+
+
+
+
+
+
+{% endblock %}