feat: clarify booking summary participant count
This commit is contained in:
@@ -1,7 +1,7 @@
|
|||||||
# Service Simplification Plan
|
# Service Simplification Plan
|
||||||
|
|
||||||
Status: draft
|
Status: draft
|
||||||
Last updated: 2026-04-05
|
Last updated: 2026-04-13
|
||||||
|
|
||||||
## Purpose
|
## Purpose
|
||||||
|
|
||||||
@@ -71,6 +71,62 @@ Concrete next steps:
|
|||||||
Decision rule:
|
Decision rule:
|
||||||
- if a registry owns actual workflow behavior, treat it as a boundary rather than a smell
|
- 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
|
## Follow-Up Queue
|
||||||
|
|
||||||
After the booking service pass, the next likely candidates are:
|
After the booking service pass, the next likely candidates are:
|
||||||
|
|||||||
@@ -27,6 +27,7 @@ class BookingSummaryDataService
|
|||||||
{
|
{
|
||||||
public function __construct(
|
public function __construct(
|
||||||
private readonly BookingPriceCalculatorService $priceCalculator,
|
private readonly BookingPriceCalculatorService $priceCalculator,
|
||||||
|
private readonly BookingSummaryParticipantCountService $participantCountService,
|
||||||
private readonly CmsDataService $cmsDataService,
|
private readonly CmsDataService $cmsDataService,
|
||||||
private readonly HotelLoader $hotelLoader,
|
private readonly HotelLoader $hotelLoader,
|
||||||
private readonly CountryDataProvider $countryDataProvider,
|
private readonly CountryDataProvider $countryDataProvider,
|
||||||
@@ -74,8 +75,7 @@ class BookingSummaryDataService
|
|||||||
$cmsData = $this->getCmsDataForProduct($productCode, $hotelCode);
|
$cmsData = $this->getCmsDataForProduct($productCode, $hotelCode);
|
||||||
}
|
}
|
||||||
|
|
||||||
// Calculate participant count from room capacity (source of truth)
|
$participantCount = $this->participantCountService->calculate($bookingDto);
|
||||||
$participantCount = $this->calculateParticipantCountFromRooms($bookingDto);
|
|
||||||
|
|
||||||
$acceptedVouchers = $bookingDto->getAcceptedVouchers($participantPrices);
|
$acceptedVouchers = $bookingDto->getAcceptedVouchers($participantPrices);
|
||||||
|
|
||||||
@@ -118,33 +118,6 @@ class BookingSummaryDataService
|
|||||||
return max(0.0, $grandTotal - $acceptedVouchers->getTotalDiscount());
|
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.
|
* Fetches hotel display data combining local base hotel data with CMS images.
|
||||||
*
|
*
|
||||||
|
|||||||
@@ -0,0 +1,40 @@
|
|||||||
|
<?php
|
||||||
|
|
||||||
|
declare(strict_types=1);
|
||||||
|
|
||||||
|
namespace App\Service;
|
||||||
|
|
||||||
|
use App\Form\Model\BookingDto;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Calculates the participant count shown in the summary sidebar.
|
||||||
|
*
|
||||||
|
* In create step 1, derive the expected participant count from room selections.
|
||||||
|
* In later create steps and edit mode, use the actual participant list.
|
||||||
|
*/
|
||||||
|
class BookingSummaryParticipantCountService
|
||||||
|
{
|
||||||
|
public function calculate(BookingDto $bookingDto): int
|
||||||
|
{
|
||||||
|
if (BookingDto::MODE_CREATE === $bookingDto->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;
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -0,0 +1,87 @@
|
|||||||
|
<?php
|
||||||
|
|
||||||
|
declare(strict_types=1);
|
||||||
|
|
||||||
|
namespace App\Tests\Service;
|
||||||
|
|
||||||
|
use App\BusProNet\Model\Booking;
|
||||||
|
use App\Form\Model\BookingDto;
|
||||||
|
use App\Form\Model\BookingSummaryDto;
|
||||||
|
use App\Service\BookingPriceCalculatorService;
|
||||||
|
use App\Service\BookingSummaryDataService;
|
||||||
|
use App\Service\BookingSummaryParticipantCountService;
|
||||||
|
use App\Service\CmsDataService;
|
||||||
|
use App\BusProNet\DataProvider\CountryDataProvider;
|
||||||
|
use App\BusProNet\XmlLoader\HotelLoader;
|
||||||
|
use PHPUnit\Framework\TestCase;
|
||||||
|
use Psr\Log\NullLogger;
|
||||||
|
use Symfony\Contracts\Cache\CacheInterface;
|
||||||
|
|
||||||
|
class BookingSummaryDataServiceTest extends TestCase
|
||||||
|
{
|
||||||
|
public function testCreateStep1UsesExpectedParticipantCountFromSelectedRooms(): void
|
||||||
|
{
|
||||||
|
$service = $this->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;
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -0,0 +1,90 @@
|
|||||||
|
<?php
|
||||||
|
|
||||||
|
declare(strict_types=1);
|
||||||
|
|
||||||
|
namespace App\Tests\Service;
|
||||||
|
|
||||||
|
use App\BusProNet\Model\Room;
|
||||||
|
use App\BusProNet\Model\Travel;
|
||||||
|
use App\Form\Model\BookingDto;
|
||||||
|
use App\Form\Model\ParticipantDto;
|
||||||
|
use App\Form\Model\RoomSelectionDto;
|
||||||
|
use App\Service\BookingSummaryParticipantCountService;
|
||||||
|
use PHPUnit\Framework\TestCase;
|
||||||
|
|
||||||
|
class BookingSummaryParticipantCountServiceTest extends TestCase
|
||||||
|
{
|
||||||
|
public function testCreateStep1UsesExpectedParticipantCountFromSelectedRooms(): void
|
||||||
|
{
|
||||||
|
$service = new BookingSummaryParticipantCountService();
|
||||||
|
$bookingDto = $this->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;
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user