diff --git a/docs/service-simplification-plan.md b/docs/service-simplification-plan.md index aa1ac15..a897fe5 100644 --- a/docs/service-simplification-plan.md +++ b/docs/service-simplification-plan.md @@ -17,8 +17,8 @@ The emphasis is not on deleting services for its own sake. The emphasis is on: The codebase is already in a better place than it was at the start of the refactor, but a few services still carry more than one responsibility: -- `BookingService` no longer owns session lifecycle, baseline snapshot handling, or return URL management. That work now lives in `BookingSessionService`, which keeps the booking orchestration boundary narrower. -- `BookingService` still covers hydration, booking bootstrap, room grouping, participant counting, service preselection, and booking status rules. +- `BookingService` no longer owns session lifecycle, baseline snapshot handling, return URL management, or room grouping. That work now lives in `BookingSessionService` and `BookingRoomSelectionService`, which keeps the booking orchestration boundary narrower. +- `BookingService` still covers hydration, booking bootstrap, participant counting for participant auto-fill, service preselection, and booking status rules. - `BookingPriceCalculatorService` is focused on pricing, but it still sits close to display-oriented behavior in adjacent code paths. - `TravelDataService` remains broad and is likely the next larger boundary after booking orchestration is reduced. @@ -104,7 +104,7 @@ Likely directions, only if justified later: |------|--------|-------| | Participant card DTO cleanup | Done | Card data now uses typed DTOs instead of nested array payloads | | Room label formatting cleanup | Done | Pricing labels now have a dedicated presentation helper | -| Booking service split | In progress | Session lifecycle, baseline snapshot, and return URL handling moved to `BookingSessionService` | +| Booking service split | In progress | Session lifecycle, baseline snapshot, return URL handling, and room grouping moved out of `BookingService` | | Pricing service review | Pending | Keep focused on calculation, not rendering | | Travel data service review | Pending | Broad boundary, likely later pass | | Participant field registry review | Deferred | Real orchestration boundary, intentionally left alone for now | diff --git a/src/Controller/Booking/Create/Step1Controller.php b/src/Controller/Booking/Create/Step1Controller.php index a0d0ed7..2072a05 100644 --- a/src/Controller/Booking/Create/Step1Controller.php +++ b/src/Controller/Booking/Create/Step1Controller.php @@ -10,6 +10,7 @@ use App\Form\BookingCreateStep1Type; use App\Form\Model\BookingDto; use App\Htmx\HxTrait; use App\Service\BookingService; +use App\Service\BookingRoomSelectionService; use App\Service\BookingSessionService; use App\Service\BookingSummaryDataService; use App\Service\RoomPricingCalculator; @@ -32,6 +33,7 @@ class Step1Controller extends AbstractController public function __construct( private readonly BookingService $bookingService, + private readonly BookingRoomSelectionService $roomSelectionService, private readonly BookingSessionService $bookingSessionService, private readonly BookingSummaryDataService $summaryDataService, ) { @@ -84,7 +86,7 @@ class Step1Controller extends AbstractController $summaryData = $this->summaryDataService->getSummaryData($bookingCreateDto, RoomPricingCalculator::PRICING_MODE_SELECTION); $availableRooms = $bookingCreateDto->travel->getAvailableRooms(); - $groupedRooms = $this->bookingService->groupRoomsBySelectionType($availableRooms); + $groupedRooms = $this->roomSelectionService->groupRoomsBySelectionType($availableRooms); return $this->render('booking/create/step_1.html.twig', [ 'bookingCreateDto' => $bookingCreateDto, @@ -116,7 +118,7 @@ class Step1Controller extends AbstractController // Get complete summary data (pricing, rooms, CMS data) $summaryData = $this->summaryDataService->getSummaryData($bookingCreateDto, RoomPricingCalculator::PRICING_MODE_SELECTION); $availableRooms = $bookingCreateDto->travel->getAvailableRooms(); - $groupedRooms = $this->bookingService->groupRoomsBySelectionType($availableRooms); + $groupedRooms = $this->roomSelectionService->groupRoomsBySelectionType($availableRooms); // The DTO is now updated with the latest selection. // We can now render the blocks with the fresh data. diff --git a/src/Controller/Booking/Traits/BookingCreateTrait.php b/src/Controller/Booking/Traits/BookingCreateTrait.php index 9be709c..5c31058 100644 --- a/src/Controller/Booking/Traits/BookingCreateTrait.php +++ b/src/Controller/Booking/Traits/BookingCreateTrait.php @@ -48,16 +48,6 @@ trait BookingCreateTrait return $this->redirectToRoute($route); } - /** - * Returns the total number of participants based on room selections. - */ - private function getParticipantsCount(BookingDto $bookingCreateDto): int - { - return $this - ->bookingService - ->getParticipantsCount($bookingCreateDto->roomSelections, $bookingCreateDto->travel); - } - /** * Prepares template variables for the booking summary sidebar. * @@ -65,17 +55,13 @@ trait BookingCreateTrait */ private function getSummaryVariables(BookingDto $bookingCreateDto): array { - $participantsCount = $this->getParticipantsCount($bookingCreateDto); - $summary = $this->bookingService->getRoomSummaryAndParticipantCount($bookingCreateDto); - $roomAssignmentCounts = $bookingCreateDto->getRoomAssignmentCounts(); - $availableRooms = $bookingCreateDto->travel->getAvailableRooms(); - $groupedSelectedRooms = $this->bookingService->groupRoomSelectionsByType($bookingCreateDto->getSelectedRooms(), $availableRooms); + $summary = $this->summaryDataService->getSummaryData($bookingCreateDto); return [ - 'participantsCount' => $participantsCount, - 'pricingData' => $summary['pricing'], - 'assignmentCounts' => $roomAssignmentCounts, - 'groupedSelectedRooms' => $groupedSelectedRooms, + 'participantsCount' => $summary->participantCount, + 'pricingData' => $summary->pricingData, + 'assignmentCounts' => $summary->assignmentCounts, + 'groupedSelectedRooms' => $summary->groupedSelectedRooms, ]; } diff --git a/src/Service/BookingRoomSelectionService.php b/src/Service/BookingRoomSelectionService.php new file mode 100644 index 0000000..5e7e969 --- /dev/null +++ b/src/Service/BookingRoomSelectionService.php @@ -0,0 +1,40 @@ + $rooms + * + * @return array{by_pax: array, by_room: array} + */ + public function groupRoomsBySelectionType(array $rooms): array + { + $groups = [ + Room::SELECTION_TYPE_BY_PAX => [], + Room::SELECTION_TYPE_BY_ROOM => [], + ]; + + foreach ($rooms as $room) { + if (false !== stripos($room->label, 'bett')) { + $groups[Room::SELECTION_TYPE_BY_PAX][$room->id] = $room; + continue; + } + + $groups[Room::SELECTION_TYPE_BY_ROOM][$room->id] = $room; + } + + uasort($groups[Room::SELECTION_TYPE_BY_PAX], static fn (Room $a, Room $b): int => $a->maxPax <=> $b->maxPax); + uasort($groups[Room::SELECTION_TYPE_BY_ROOM], static fn (Room $a, Room $b): int => $a->maxPax <=> $b->maxPax); + + return $groups; + } +} diff --git a/src/Service/BookingService.php b/src/Service/BookingService.php index d1a42c2..18095e1 100644 --- a/src/Service/BookingService.php +++ b/src/Service/BookingService.php @@ -24,7 +24,6 @@ class BookingService public function __construct( private readonly BookingSessionService $bookingSessionService, private readonly TravelDataService $travelDataService, - private readonly BookingPriceCalculatorService $priceCalculator, private readonly ParticipantEligibilityService $participantEligibilityService, private readonly BookingStatusRuleRegistry $bookingStatusRuleRegistry, private readonly AgencyLoader $agencyLoader, @@ -115,108 +114,6 @@ class BookingService return $selection; } - /** - * Calculates the total number of participants based on room selections. - * - * Multiplies each room's minimum occupancy (minPax) by the selected quantity - * to determine the total number of participants required for the booking. - * - * @param array $roomSelections Array of RoomSelectionDto objects - * @param Travel $travelData Travel data containing room information - * - * @return int Total number of participants required - */ - public function getParticipantsCount(array $roomSelections, Travel $travelData): int - { - $participantsCount = 0; - $rooms = $travelData->getAvailableRooms(); - - foreach ($roomSelections as $roomSelection) { - $room = $rooms[$roomSelection->id]; - $participantsCount += $room->minPax * $roomSelection->quantity; - } - - return $participantsCount; - } - - /** - * Returns a summary of selected rooms, participant count, and pricing information for a booking. - * - * @return array{selectedRooms: array, participantCount: int, pricing: array} - */ - public function getRoomSummaryAndParticipantCount(BookingDto $bookingDto): array - { - $selectedRooms = $bookingDto->getSelectedRooms(); - - // In edit mode, count actual participants; in create mode, calculate from room selections - if (BookingDto::MODE_EDIT === $bookingDto->getMode()) { - $participantCount = count($bookingDto->getParticipants()); - } else { - $participantCount = $this->getParticipantsCount($selectedRooms, $bookingDto->travel); - } - - $pricing = $this->priceCalculator->getPricingBreakdown($bookingDto); - - return [ - 'selectedRooms' => $selectedRooms, - 'participantCount' => $participantCount, - 'pricing' => $pricing, - ]; - } - - /** - * Groups available rooms by selection type ('by_pax' or 'by_room') and sorts them by maxPax. - * - * @param array $rooms Rooms indexed by room ID - * - * @return array{by_pax: array, by_room: array} Rooms grouped and sorted by maxPax (ascending) - */ - public function groupRoomsBySelectionType(array $rooms): array - { - $groups = [ - Room::SELECTION_TYPE_BY_PAX => [], - Room::SELECTION_TYPE_BY_ROOM => [], - ]; - foreach ($rooms as $room) { - if (false !== stripos($room->label, 'bett')) { - $groups[Room::SELECTION_TYPE_BY_PAX][$room->id] = $room; - } else { - $groups[Room::SELECTION_TYPE_BY_ROOM][$room->id] = $room; - } - } - - // Sort each group by maxPax (ascending order - smallest capacity first) - uasort($groups[Room::SELECTION_TYPE_BY_PAX], fn (Room $a, Room $b): int => $a->maxPax <=> $b->maxPax); - uasort($groups[Room::SELECTION_TYPE_BY_ROOM], fn (Room $a, Room $b): int => $a->maxPax <=> $b->maxPax); - - return $groups; - } - - /** - * Groups roomSelections by selection type ('by_pax' or 'by_room'), using Room::getSelectionType(). - * - * @param array $roomSelections Array of selected RoomSelectionDto - * @param array $roomsById Rooms indexed by room ID - * - * @return array{by_pax: array, by_room: array} - */ - public function groupRoomSelectionsByType(array $roomSelections, array $roomsById): array - { - $groups = [ - Room::SELECTION_TYPE_BY_PAX => [], - Room::SELECTION_TYPE_BY_ROOM => [], - ]; - foreach ($roomSelections as $roomSelection) { - $room = $roomsById[$roomSelection->id] ?? null; - if ($room) { - $type = $room->getSelectionType(); - $groups[$type][] = $roomSelection; - } - } - - return $groups; - } - /** * Pre-selects default services for all participants. * @@ -768,7 +665,7 @@ class BookingService ?\Symfony\Component\Security\Core\User\UserInterface $user = null, ?callable $prepopulateCallback = null, ): void { - $participantsCount = $this->getParticipantsCount($bookingDto->roomSelections, $bookingDto->travel); + $participantsCount = $this->calculateParticipantsCount($bookingDto->roomSelections, $bookingDto->travel); $existingParticipants = $bookingDto->participants; $bookingDto->participants = []; @@ -795,4 +692,28 @@ class BookingService { return null === $participant->firstName || '' === $participant->firstName; } + + /** + * Calculates the total number of participants based on room selections. + * + * Multiplies each room's minimum occupancy (minPax) by the selected quantity + * to determine the total number of participants required for the booking. + * + * @param array $roomSelections Array of RoomSelectionDto objects + * @param Travel $travelData Travel data containing room information + * + * @return int Total number of participants required + */ + private function calculateParticipantsCount(array $roomSelections, Travel $travelData): int + { + $participantsCount = 0; + $rooms = $travelData->getAvailableRooms(); + + foreach ($roomSelections as $roomSelection) { + $room = $rooms[$roomSelection->id]; + $participantsCount += $room->minPax * $roomSelection->quantity; + } + + return $participantsCount; + } } diff --git a/tests/Service/BookingRoomSelectionServiceTest.php b/tests/Service/BookingRoomSelectionServiceTest.php new file mode 100644 index 0000000..99984cf --- /dev/null +++ b/tests/Service/BookingRoomSelectionServiceTest.php @@ -0,0 +1,43 @@ +service = new BookingRoomSelectionService(); + } + + public function testGroupRoomsBySelectionTypeSplitsAndSortsRooms(): void + { + $rooms = [ + 10 => $this->createRoom(10, '2 Bett Zimmer', 4), + 11 => $this->createRoom(11, 'Suite', 2), + 12 => $this->createRoom(12, '3 Bett Zimmer', 6), + ]; + + $groups = $this->service->groupRoomsBySelectionType($rooms); + + $this->assertSame([10, 12], array_keys($groups[Room::SELECTION_TYPE_BY_PAX])); + $this->assertSame([11], array_keys($groups[Room::SELECTION_TYPE_BY_ROOM])); + } + + private function createRoom(int $id, string $label, int $maxPax): Room + { + $room = new Room(); + $room->id = $id; + $room->label = $label; + $room->maxPax = $maxPax; + + return $room; + } +} diff --git a/tests/Service/BookingServiceBabyTest.php b/tests/Service/BookingServiceBabyTest.php index 5d4c1f3..72524cc 100644 --- a/tests/Service/BookingServiceBabyTest.php +++ b/tests/Service/BookingServiceBabyTest.php @@ -11,7 +11,6 @@ use App\BusProNet\Service\BookingStatusRuleRegistry; use App\BusProNet\XmlLoader\AgencyLoader; use App\Form\Model\BookingDto; use App\Form\Model\ParticipantDto; -use App\Service\BookingPriceCalculatorService; use App\Service\BookingService; use App\Service\BookingSessionService; use App\Service\ParticipantEligibilityService; @@ -26,7 +25,6 @@ class BookingServiceBabyTest extends TestCase protected function setUp(): void { $travelDataService = $this->createMock(TravelDataService::class); - $priceCalculator = $this->createMock(BookingPriceCalculatorService::class); $this->participantEligibilityService = $this->createMock(ParticipantEligibilityService::class); $bookingStatusRuleRegistry = $this->createMock(BookingStatusRuleRegistry::class); $bookingStatusRuleRegistry->method('evaluateStatus')->willReturn('F'); @@ -35,7 +33,6 @@ class BookingServiceBabyTest extends TestCase $this->bookingService = new BookingService( $this->createMock(BookingSessionService::class), $travelDataService, - $priceCalculator, $this->participantEligibilityService, $bookingStatusRuleRegistry, $agencyLoader, diff --git a/tests/Service/BookingServiceStatusTest.php b/tests/Service/BookingServiceStatusTest.php index 3d6a186..c06c11a 100644 --- a/tests/Service/BookingServiceStatusTest.php +++ b/tests/Service/BookingServiceStatusTest.php @@ -11,7 +11,6 @@ use App\BusProNet\Model\Travel; use App\BusProNet\Service\BookingStatusRuleRegistry; use App\BusProNet\XmlLoader\AgencyLoader; use App\Exception\NoRoomsAvailableException; -use App\Service\BookingPriceCalculatorService; use App\Service\BookingService; use App\Service\BookingSessionService; use App\Service\ParticipantEligibilityService; @@ -30,7 +29,6 @@ class BookingServiceStatusTest extends TestCase { $bookingSessionService = $this->createMock(BookingSessionService::class); $this->travelDataService = $this->createMock(TravelDataService::class); - $priceCalculator = $this->createMock(BookingPriceCalculatorService::class); $participantEligibility = $this->createMock(ParticipantEligibilityService::class); $bookingStatusRuleRegistry = $this->createMock(BookingStatusRuleRegistry::class); $bookingStatusRuleRegistry->method('evaluateStatus')->willReturn('F'); @@ -39,7 +37,6 @@ class BookingServiceStatusTest extends TestCase $this->bookingService = new BookingService( $bookingSessionService, $this->travelDataService, - $priceCalculator, $participantEligibility, $bookingStatusRuleRegistry, $agencyLoader, @@ -183,7 +180,6 @@ class BookingServiceStatusTest extends TestCase $bookingService = new BookingService( $this->createMock(BookingSessionService::class), $this->travelDataService, - $this->createMock(BookingPriceCalculatorService::class), $this->createMock(ParticipantEligibilityService::class), $bookingStatusRuleRegistry, $this->createMock(AgencyLoader::class), @@ -215,7 +211,6 @@ class BookingServiceStatusTest extends TestCase $bookingService = new BookingService( $this->createMock(BookingSessionService::class), $this->travelDataService, - $this->createMock(BookingPriceCalculatorService::class), $this->createMock(ParticipantEligibilityService::class), $bookingStatusRuleRegistry, $this->createMock(AgencyLoader::class),