fix: restore room occupancy lost to case-sensitive xml attribute lookup
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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,
|
||||
]);
|
||||
});
|
||||
}
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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('<?xml version="1.0" encoding="utf-8"?>
|
||||
<zimmer>
|
||||
<preis zimmercode="4erDW" idbuspro_zimmer="95" zimmertext="4er Zimmer Dusche/WC" %s naechte="5" idbuspro_vp="2" preis="689,00" status="Frei" verfuegbar="8" />
|
||||
</zimmer>', $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<string, array{string, int|null}>
|
||||
*/
|
||||
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];
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,39 @@
|
||||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
namespace App\Tests\Form;
|
||||
|
||||
use App\Form\Model\RoomSelectionDto;
|
||||
use App\Form\RoomSelectType;
|
||||
use PHPUnit\Framework\Attributes\DataProvider;
|
||||
use PHPUnit\Framework\TestCase;
|
||||
use Symfony\Component\Form\Forms;
|
||||
|
||||
class RoomSelectTypeTest extends TestCase
|
||||
{
|
||||
/**
|
||||
* @param array<string, int> $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<string, array{int, array<string, int>}>
|
||||
*/
|
||||
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]];
|
||||
}
|
||||
}
|
||||
@@ -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;
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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();
|
||||
|
||||
Reference in New Issue
Block a user