diff --git a/src/BusProNet/XmlParser/AbstractParser.php b/src/BusProNet/XmlParser/AbstractParser.php index 56a6e9d..56f52c4 100644 --- a/src/BusProNet/XmlParser/AbstractParser.php +++ b/src/BusProNet/XmlParser/AbstractParser.php @@ -9,15 +9,6 @@ abstract class AbstractParser { use TypeConversionTrait; - protected function getIntOrNullAttribute(?string $value): ?int - { - if (null === $value || '' === $value) { - return null; - } - - return (int) $value; - } - protected function getStringOrNullValue(Crawler $node): ?string { return 0 < $node->count() ? $node->text() : null; @@ -64,7 +55,27 @@ abstract class AbstractParser return null; } - return $node->attr($attribute); + $value = $node->attr($attribute); + if (null !== $value) { + return $value; + } + + // BusPro mixes casing (MinPax in the travel export, minpax in booking + // responses) and addXmlContent() makes attr() case-sensitive. + foreach ($node->getNode(0)->attributes ?? [] as $candidate) { + if (0 === strcasecmp($candidate->name, $attribute)) { + return $candidate->value; + } + } + + return null; + } + + protected function getIntAttrOrNullValue(Crawler $node, string $attribute): ?int + { + $value = $this->getAttrOrNullValue($node, $attribute); + + return null !== $value && '' !== $value ? (int) $value : null; } protected function getRequiredAttrValue(Crawler $node, string $attribute, string $context): string diff --git a/src/BusProNet/XmlParser/TravelParser.php b/src/BusProNet/XmlParser/TravelParser.php index 8ef3657..8c1eec3 100644 --- a/src/BusProNet/XmlParser/TravelParser.php +++ b/src/BusProNet/XmlParser/TravelParser.php @@ -381,8 +381,8 @@ class TravelParser extends AbstractParser $room->category = $this->getAttrOrNullValue($roomNode, 'kat'); $room->boardId = (int) $this->getAttrOrNullValue($roomNode, 'idbuspro_vp'); $room->label = $this->getAttrOrNullValue($roomNode, 'zimmertext'); - $room->minPax = (int) $this->getAttrOrNullValue($roomNode, 'minpax'); - $room->maxPax = (int) $this->getAttrOrNullValue($roomNode, 'maxpax'); + $room->minPax = $this->getIntAttrOrNullValue($roomNode, 'minpax'); + $room->maxPax = $this->getIntAttrOrNullValue($roomNode, 'maxpax'); $room->nights = (int) $this->getAttrOrNullValue($roomNode, 'naechte'); $roomPrice = $this->getAttrOrNullValue($roomNode, 'preis'); $room->price = null !== $roomPrice && '' !== $roomPrice diff --git a/src/Controller/Booking/Create/Step2Controller.php b/src/Controller/Booking/Create/Step2Controller.php index e8bd42d..ab75508 100644 --- a/src/Controller/Booking/Create/Step2Controller.php +++ b/src/Controller/Booking/Create/Step2Controller.php @@ -19,6 +19,9 @@ use App\Service\BookingSessionManager; use App\Service\ParticipantDataPrefiller; use App\Service\RoomAssigner; use App\Service\TravelDataProvider; +use Psr\Log\LoggerInterface; +use Symfony\Component\DependencyInjection\Attribute\Autowire; +use Symfony\Component\HttpFoundation\RedirectResponse; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\Routing\Attribute\Route; @@ -38,6 +41,8 @@ class Step2Controller extends AbstractBookingCreateController private readonly TravelDataProvider $travelDataService, private readonly RoomAssigner $roomAssignmentService, private readonly ParticipantDataPrefiller $prepopulationService, + #[Autowire(service: 'monolog.logger.bpn')] + private readonly LoggerInterface $logger, ) { } @@ -62,6 +67,10 @@ class Step2Controller extends AbstractBookingCreateController $this->travelDataService->enrichWithFreshAvailabilities($bookingCreateDto->travel); // Ensure correct number of participants first, then prepopulate the applicant if needed. + if (null !== $redirect = $this->guardAgainstEmptyParticipantList($bookingCreateDto)) { + return $redirect; + } + $this->ensureCorrectNumberOfParticipants($bookingCreateDto); $user = $this->getUser(); @@ -111,6 +120,49 @@ class Step2Controller extends AbstractBookingCreateController return $this->render('booking/create/step_2.html.twig', $templateData); } + /** + * Rejects a room selection that resolves to no participants at all. + * + * A selected room with a quantity always yields at least one participant, so a zero count means the + * travel data carries no room occupancy. Rendering step 2 anyway would show an empty participant list + * that still validates and lets the booking continue to step 3. + */ + private function guardAgainstEmptyParticipantList(BookingDto $bookingCreateDto): ?RedirectResponse + { + $selectedRooms = $bookingCreateDto->getSelectedRooms(); + + if ([] === $selectedRooms) { + return null; + } + + if (0 < $this->calculateParticipantsCount($bookingCreateDto->roomSelections, $bookingCreateDto->travel)) { + return null; + } + + $rooms = []; + foreach ($selectedRooms as $roomSelection) { + $rooms[] = [ + 'roomId' => $roomSelection->id, + 'quantity' => $roomSelection->quantity, + 'capacity' => $roomSelection->capacity, + ]; + } + + $this->handleApiError( + $this->logger, + 'Room selection resolves to zero participants; travel data carries no room occupancy.', + [ + 'travelId' => $bookingCreateDto->travel->id, + 'travelCode' => $bookingCreateDto->travel->code, + 'hotelId' => $bookingCreateDto->hotelId, + 'rooms' => $rooms, + ], + 'Die Zimmerdaten dieser Reise sind unvollständig. Bitte wähle dein Zimmer erneut oder kontaktiere uns.' + ); + + return $this->redirectToRoute('app_booking_create_step_1'); + } + /** * Ensures the booking DTO has the expected number of participant objects based on room selections. */ @@ -139,7 +191,7 @@ class Step2Controller extends AbstractBookingCreateController foreach ($roomSelections as $roomSelection) { $room = $rooms[$roomSelection->id]; - $participantsCount += $room->minPax * $roomSelection->quantity; + $participantsCount += ($room->minPax ?? 1) * $roomSelection->quantity; } return $participantsCount; diff --git a/src/Form/RoomSelectType.php b/src/Form/RoomSelectType.php index 0bbfcc5..ac1df7e 100644 --- a/src/Form/RoomSelectType.php +++ b/src/Form/RoomSelectType.php @@ -24,12 +24,17 @@ class RoomSelectType extends AbstractType $data = $event->getData(); $form = $event->getForm(); + $choices = ['-' => 0]; + if (0 < $data->maxQuantity) { + $choices += array_combine(range(1, $data->maxQuantity), range(1, $data->maxQuantity)); + } + $form->add('quantity', StepSelectChoiceType::class, [ 'label' => false, 'required' => true, 'min_value' => 0, 'max_value' => $data->maxQuantity, - 'choices' => ['-' => 0] + array_combine(range(1, $data->maxQuantity), range(1, $data->maxQuantity)), + 'choices' => $choices, ]); }); } diff --git a/src/Service/BookingConfigurator.php b/src/Service/BookingConfigurator.php index 8480df1..af3db09 100644 --- a/src/Service/BookingConfigurator.php +++ b/src/Service/BookingConfigurator.php @@ -110,7 +110,7 @@ class BookingConfigurator $selection->price = $room->price; $selection->status = $room->status; $selection->maxQuantity = $room->available; - $selection->capacity = $room->minPax; + $selection->capacity = $room->minPax ?? 1; $selection->quantity = $roomsIdsAndQuantities[$room->id] ?? 0; return $selection; diff --git a/tests/BusProNet/XmlParser/TravelParserTest.php b/tests/BusProNet/XmlParser/TravelParserTest.php index 71fb892..b3b734b 100644 --- a/tests/BusProNet/XmlParser/TravelParserTest.php +++ b/tests/BusProNet/XmlParser/TravelParserTest.php @@ -6,6 +6,7 @@ namespace App\Tests\BusProNet\XmlParser; use App\BusProNet\XmlCrawlerFactory; use App\BusProNet\XmlParser\TravelParser; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; class TravelParserTest extends TestCase @@ -426,4 +427,35 @@ class TravelParserTest extends TestCase $this->assertSame('2029-12-30 18:00', $pickups[1]->time->format('Y-m-d H:i')); $this->assertSame('2029-12-30 01:00', $pickups[2]->time->format('Y-m-d H:i')); } + + /** + * The travel export writes MinPax/MaxPax capitalized while booking responses write them lowercase. + * XmlCrawlerFactory uses addXmlContent(), which makes attribute lookups case-sensitive, so both + * spellings have to resolve. + */ + #[DataProvider('roomOccupancyAttributeProvider')] + public function testGetRoomsReadsOccupancyRegardlessOfAttributeCasing(string $attributes, ?int $expectedPax): void + { + $xml = sprintf(' + + +', $attributes); + + $crawler = XmlCrawlerFactory::create($xml); + $rooms = $this->parser->getRooms($crawler->filterXPath('//zimmer')->first()); + + $this->assertArrayHasKey(95, $rooms); + $this->assertSame($expectedPax, $rooms[95]->minPax); + $this->assertSame($expectedPax, $rooms[95]->maxPax); + } + + /** + * @return iterable + */ + public static function roomOccupancyAttributeProvider(): iterable + { + yield 'export casing' => ['MinPax="4" MaxPax="4"', 4]; + yield 'lowercase casing' => ['minpax="4" maxpax="4"', 4]; + yield 'missing stays null' => ['', null]; + } } diff --git a/tests/Form/RoomSelectTypeTest.php b/tests/Form/RoomSelectTypeTest.php new file mode 100644 index 0000000..76c6ccf --- /dev/null +++ b/tests/Form/RoomSelectTypeTest.php @@ -0,0 +1,39 @@ + $expectedChoices + */ + #[DataProvider('quantityChoiceProvider')] + public function testQuantityChoicesFollowAvailability(int $maxQuantity, array $expectedChoices): void + { + $data = new RoomSelectionDto(); + $data->id = 95; + $data->maxQuantity = $maxQuantity; + + $form = Forms::createFormFactory()->create(RoomSelectType::class, $data); + + $this->assertSame($expectedChoices, $form->get('quantity')->getConfig()->getOption('choices')); + } + + /** + * @return iterable}> + */ + public static function quantityChoiceProvider(): iterable + { + yield 'available rooms are selectable' => [2, ['-' => 0, '1' => 1, '2' => 2]]; + // range(1, 0) would return [1, 0], offering a bookable "1" for a room with no availability. + yield 'no availability offers only the placeholder' => [0, ['-' => 0]]; + } +} diff --git a/tests/Service/BookingSummaryAssemblerTest.php b/tests/Service/BookingSummaryAssemblerTest.php index b7ff669..efce9c4 100644 --- a/tests/Service/BookingSummaryAssemblerTest.php +++ b/tests/Service/BookingSummaryAssemblerTest.php @@ -70,6 +70,23 @@ class BookingSummaryAssemblerTest extends TestCase $this->assertSame(2, $summary->participantCount); } + public function testCreateStep1CountsRoomsWithUnknownOccupancyAsZero(): void + { + $service = $this->createService(); + $bookingDto = $this->createBookingDto(); + $bookingDto->travel->rooms[12] = $this->createRoom(12, null); + $bookingDto->currentStep = 1; + $bookingDto->roomSelections = [ + $this->createRoomSelection(10, 1), + $this->createRoomSelection(12, 2), + ]; + + $summary = $service->getSummaryData($bookingDto); + + // 1×2 for the known room; the room without occupancy contributes nothing. + $this->assertSame(2, $summary->participantCount); + } + private function createService(): BookingSummaryAssembler { $priceCalculator = $this->createStub(BookingPriceCalculator::class); @@ -107,7 +124,7 @@ class BookingSummaryAssemblerTest extends TestCase return new BookingDto($travel, 157047); } - private function createRoom(int $id, int $maxPax): Room + private function createRoom(int $id, ?int $maxPax): Room { $room = new Room(); $room->id = $id; diff --git a/tests/Service/RoomAssignerTest.php b/tests/Service/RoomAssignerTest.php index 3ef99a5..12572cf 100644 --- a/tests/Service/RoomAssignerTest.php +++ b/tests/Service/RoomAssignerTest.php @@ -272,6 +272,32 @@ class RoomAssignerTest extends TestCase self::assertNull($bookingDto->participants[1]->assignedRoomId); } + #[Test] + public function assignParticipantsToRoomsFallsBackToOneBedWhenOccupancyIsUnknown(): void + { + $bookingDto = $this->createBookingDtoWithRoomSelections([ + ['roomId' => 100, 'quantity' => 2, 'capacity' => 1], + ]); + + $bookingDto->participants = [ + new ParticipantDto(), + new ParticipantDto(), + ]; + + $room = new Room(); + $room->id = 100; + $room->minPax = null; + $room->available = 10; + $room->status = Constants::STATUS_AVAILABLE; + + $bookingDto->travel->rooms = [$room]; + + $this->service->assignParticipantsToRooms($bookingDto); + + self::assertSame(100, $bookingDto->participants[0]->assignedRoomId); + self::assertSame(100, $bookingDto->participants[1]->assignedRoomId); + } + private function createBookingDtoWithRoomSelections(array $selections): BookingDto { $travel = new Travel(); diff --git a/tests/Service/RoomPricingCalculatorTest.php b/tests/Service/RoomPricingCalculatorTest.php index 03c5e7f..866b47c 100644 --- a/tests/Service/RoomPricingCalculatorTest.php +++ b/tests/Service/RoomPricingCalculatorTest.php @@ -247,6 +247,31 @@ class RoomPricingCalculatorTest extends TestCase $this->assertEquals(458.0, $result[0]['totalPrice']); } + public function testCalculateRoomPricingFallsBackToOneParticipantWhenOccupancyIsUnknown(): void + { + $room = new Room(); + $room->id = 4; + $room->price = 419.0; + $room->minPax = null; + $room->label = 'Bett im Mehrbettzimmer'; + + $travel = new Travel(); + $travel->rooms = [$room]; + + $roomSelection = new RoomSelectionDto(); + $roomSelection->id = 4; + $roomSelection->quantity = 2; + + $bookingDto = new BookingDto($travel, 1); + $bookingDto->roomSelections = [$roomSelection]; + + $result = $this->calculator->calculateRoomPricing($bookingDto, RoomPricingCalculator::PRICING_MODE_SELECTION); + + $this->assertCount(1, $result); + $this->assertEquals(2, $result[0]['participantCount']); + $this->assertEquals(838.0, $result[0]['totalPrice']); + } + public function testCalculateRoomPricingInEditModeUsesStoredIndividualPrices(): void { $travel = new Travel();