diff --git a/docs/service-simplification-plan.md b/docs/service-simplification-plan.md index 115c7e0..fd087c5 100644 --- a/docs/service-simplification-plan.md +++ b/docs/service-simplification-plan.md @@ -1,7 +1,7 @@ # Service Simplification Plan Status: draft -Last updated: 2026-04-05 +Last updated: 2026-04-13 ## Purpose @@ -71,6 +71,62 @@ Concrete next steps: Decision rule: - if a registry owns actual workflow behavior, treat it as a boundary rather than a smell +## Implementation Backlog + +### 1. Keep the summary count contract explicit + +Goal: make the booking summary read clearly without duplicating equivalent count fields. + +Tasks: +- use a single summary-facing count field for the sidebar and step summary views +- keep the room-capacity-derived meaning explicit in the field name and docblock +- keep the participant-shaping count logic separate if the code still needs it internally +- remove template branching that compares two equivalent summary counts + +Acceptance criteria: +- the summary template reads one count field, not two equivalent ones +- the field name makes the room-capacity meaning obvious to a new developer +- participant-shaping logic can still use its own internal count without leaking that distinction into the view layer + +### 2. Reduce `BookingService` + +Primary goal: make the booking create/edit flow easier to read by splitting unrelated concerns. + +Concrete next steps: +- keep booking session lifecycle in one place +- extract baseline room snapshot handling into a narrower helper or dedicated service +- separate return URL handling if it stays conceptually unrelated +- keep `startFreshBooking()` focused on booking bootstrap rather than general session utilities +- keep hydration behavior obvious and local to the booking session path + +Decision rule: +- if a method only forwards to DTO/session behavior, prefer removing the wrapper +- if a method is a genuine workflow owner, keep it and narrow the surrounding API instead of splitting it into generic helpers + +### 3. Keep pricing calculation focused + +Primary goal: keep pricing code about pricing, not rendering. + +Concrete next steps: +- keep `BookingPriceCalculatorService` as the pricing boundary +- continue removing display formatting from pricing code paths +- keep any remaining view-specific formatting in the presentation layer or a dedicated UI helper +- avoid introducing another service that only formats values already known to the view + +Decision rule: +- if a value is only needed for display, prefer exposing the raw numeric/domain value and formatting it as close to the UI as possible + +### 4. Leave the field-handler registry in place + +Primary goal: avoid unnecessary churn in a class that is already a meaningful orchestration layer. + +Concrete next steps: +- do not refactor `ParticipantFieldHandlerRegistry` in this pass +- revisit only if a later change can split ordering, mutability, and synchronization into clear collaborators without making the flow harder to trace + +Decision rule: +- if a registry owns actual workflow behavior, treat it as a boundary rather than a smell + ## Follow-Up Queue After the booking service pass, the next likely candidates are: diff --git a/src/Service/BookingSummaryDataService.php b/src/Service/BookingSummaryDataService.php index 9133cf8..9c63ce8 100644 --- a/src/Service/BookingSummaryDataService.php +++ b/src/Service/BookingSummaryDataService.php @@ -27,6 +27,7 @@ class BookingSummaryDataService { public function __construct( private readonly BookingPriceCalculatorService $priceCalculator, + private readonly BookingSummaryParticipantCountService $participantCountService, private readonly CmsDataService $cmsDataService, private readonly HotelLoader $hotelLoader, private readonly CountryDataProvider $countryDataProvider, @@ -74,8 +75,7 @@ class BookingSummaryDataService $cmsData = $this->getCmsDataForProduct($productCode, $hotelCode); } - // Calculate participant count from room capacity (source of truth) - $participantCount = $this->calculateParticipantCountFromRooms($bookingDto); + $participantCount = $this->participantCountService->calculate($bookingDto); $acceptedVouchers = $bookingDto->getAcceptedVouchers($participantPrices); @@ -118,33 +118,6 @@ class BookingSummaryDataService return max(0.0, $grandTotal - $acceptedVouchers->getTotalDiscount()); } - /** - * Calculates participant count. - * - * In edit mode, counts actual participants. In create mode, calculates - * from room selections by multiplying quantity by maximum capacity (maxPax). - */ - private function calculateParticipantCountFromRooms(BookingDto $bookingDto): int - { - // In edit mode, use actual participant count - if (BookingDto::MODE_EDIT === $bookingDto->getMode()) { - return count($bookingDto->participants); - } - - // In create mode, calculate from room selections - $totalCapacity = 0; - $availableRooms = $bookingDto->travel->getAvailableRooms(); - - foreach ($bookingDto->roomSelections as $selection) { - if ($selection->quantity > 0 && isset($availableRooms[$selection->id])) { - $room = $availableRooms[$selection->id]; - $totalCapacity += $selection->quantity * ($room->maxPax ?? 0); - } - } - - return $totalCapacity; - } - /** * Fetches hotel display data combining local base hotel data with CMS images. * diff --git a/src/Service/BookingSummaryParticipantCountService.php b/src/Service/BookingSummaryParticipantCountService.php new file mode 100644 index 0000000..e07c218 --- /dev/null +++ b/src/Service/BookingSummaryParticipantCountService.php @@ -0,0 +1,40 @@ +getMode() && 1 === $bookingDto->currentStep) { + return $this->calculateExpectedParticipantCountFromRooms($bookingDto); + } + + return count($bookingDto->participants); + } + + private function calculateExpectedParticipantCountFromRooms(BookingDto $bookingDto): int + { + $totalCapacity = 0; + $availableRooms = $bookingDto->travel->getAvailableRooms(); + + foreach ($bookingDto->roomSelections as $selection) { + if ($selection->quantity > 0 && isset($availableRooms[$selection->id])) { + $room = $availableRooms[$selection->id]; + $totalCapacity += $selection->quantity * ($room->maxPax ?? 0); + } + } + + return $totalCapacity; + } +} diff --git a/tests/Service/BookingSummaryDataServiceTest.php b/tests/Service/BookingSummaryDataServiceTest.php new file mode 100644 index 0000000..7e2cf7a --- /dev/null +++ b/tests/Service/BookingSummaryDataServiceTest.php @@ -0,0 +1,87 @@ +createService(8); + $bookingDto = $this->createBookingDto(); + + $summary = $service->getSummaryData($bookingDto); + + $this->assertInstanceOf(BookingSummaryDto::class, $summary); + $this->assertSame(8, $summary->participantCount); + } + + public function testLaterCreateStepsUseActualParticipantCount(): void + { + $service = $this->createService(3); + $bookingDto = $this->createBookingDto(); + $bookingDto->currentStep = 2; + + $summary = $service->getSummaryData($bookingDto); + + $this->assertSame(3, $summary->participantCount); + } + + public function testEditModeUsesActualParticipantCount(): void + { + $service = $this->createService(2); + $bookingDto = $this->createBookingDto(); + $bookingDto->booking = new Booking(); + + $summary = $service->getSummaryData($bookingDto); + + $this->assertSame(2, $summary->participantCount); + } + + private function createService(int $participantCount): BookingSummaryDataService + { + $priceCalculator = $this->createMock(BookingPriceCalculatorService::class); + $priceCalculator->method('calculateAllParticipantIndividualPrices')->willReturn([]); + $priceCalculator->method('getPricingBreakdown')->willReturn([ + 'rooms' => [], + 'services' => [], + 'surcharges' => null, + 'grandTotal' => 0.0, + ]); + + $participantCountService = $this->createMock(BookingSummaryParticipantCountService::class); + $participantCountService->method('calculate') + ->willReturn($participantCount); + + return new BookingSummaryDataService( + $priceCalculator, + $participantCountService, + $this->createMock(CmsDataService::class), + $this->createMock(HotelLoader::class), + $this->createMock(CountryDataProvider::class), + $this->createMock(CacheInterface::class), + new NullLogger(), + ); + } + + private function createBookingDto(): BookingDto + { + $bookingDto = new BookingDto(new \App\BusProNet\Model\Travel(), 157047); + + return $bookingDto; + } +} diff --git a/tests/Service/BookingSummaryParticipantCountServiceTest.php b/tests/Service/BookingSummaryParticipantCountServiceTest.php new file mode 100644 index 0000000..9f2663b --- /dev/null +++ b/tests/Service/BookingSummaryParticipantCountServiceTest.php @@ -0,0 +1,90 @@ +createBookingDto(); + $bookingDto->currentStep = 1; + $bookingDto->roomSelections = [ + $this->createRoomSelection(10, 1), + $this->createRoomSelection(11, 2), + ]; + + self::assertSame(8, $service->calculate($bookingDto)); + } + + public function testLaterCreateStepsUseActualParticipantCount(): void + { + $service = new BookingSummaryParticipantCountService(); + $bookingDto = $this->createBookingDto(); + $bookingDto->currentStep = 2; + $bookingDto->participants = [ + new ParticipantDto(), + new ParticipantDto(), + new ParticipantDto(), + ]; + + self::assertSame(3, $service->calculate($bookingDto)); + } + + public function testEditModeUsesActualParticipantCount(): void + { + $service = new BookingSummaryParticipantCountService(); + $bookingDto = $this->createBookingDto(); + $bookingDto->booking = new \App\BusProNet\Model\Booking(); + $bookingDto->participants = [ + new ParticipantDto(), + new ParticipantDto(), + ]; + + self::assertSame(2, $service->calculate($bookingDto)); + } + + private function createBookingDto(): BookingDto + { + $travel = new Travel(); + $travel->dateFrom = new \DateTimeImmutable('2030-01-01'); + $travel->dateTo = new \DateTimeImmutable('2030-01-06'); + $travel->rooms = [ + 10 => $this->createRoom(10, 2), + 11 => $this->createRoom(11, 3), + ]; + + return new BookingDto($travel, 157047); + } + + private function createRoom(int $id, int $maxPax): Room + { + $room = new Room(); + $room->id = $id; + $room->label = 'Room '.$id; + $room->available = 4; + $room->status = 'Frei'; + $room->maxPax = $maxPax; + + return $room; + } + + private function createRoomSelection(int $id, int $quantity): RoomSelectionDto + { + $selection = new RoomSelectionDto(); + $selection->id = $id; + $selection->quantity = $quantity; + + return $selection; + } +}