fix: refuse implausible accommodation booking prices

This commit is contained in:
fromme committed 2026-09-26 15:39:04 +02:00
1 parent 773cd4f847
commit 56adf72d12
16 files changed
+628 -15

No files matched your search

@@ -8,6 +8,7 @@ use App\Controller\Admin\AccommodationBooking\ChangeAccommodationController;
use App\Entity\Groups\Accommodation;
use App\Entity\Groups\AccommodationBooking;
use App\Enum\Groups\AccommodationBookingStatus;
use App\Exception\ImplausibleBookingPriceException;
use App\Service\AccommodationBookingService;
use PHPUnit\Framework\Attributes\DataProvider;
use PHPUnit\Framework\TestCase;
@@ -87,6 +88,36 @@ class ChangeAccommodationControllerTest extends TestCase
self::assertSame('success', $controller->flashes[0]['type']);
}
public function testAHouseThatPricesImplausiblyIsReportedInTheModal(): void
{
// The service leaves the booking at its old house and rethrows; the office has to see why
// the change did not happen, on the modal it just submitted, not as an error page.
$booking = $this->draft();
$bookingService = $this->createStub(AccommodationBookingService::class);
$bookingService->method('changeAccommodation')
->willThrowException(new ImplausibleBookingPriceException(AccommodationBooking::MAX_TOTAL_PRICE + 1, 40, 2));
$field = $this->createStub(FormInterface::class);
$field->method('getData')->willReturn(new Accommodation());
$form = $this->createMock(FormInterface::class);
$form->method('handleRequest')->willReturn($form);
$form->method('isSubmitted')->willReturn(true);
$form->method('isValid')->willReturn(true);
$form->method('get')->with('accommodation')->willReturn($field);
$form->expects(self::once())->method('addError');
$controller = $this->buildController($bookingService, $form);
$response = $controller->index($booking, $this->request('POST'));
self::assertSame(Response::HTTP_OK, $response->getStatusCode());
self::assertFalse($response->headers->has('HX-Redirect'), 'the modal stays open on the error');
self::assertSame('admin/accommodation_booking/modal_change_accommodation.html.twig', $controller->renderedView);
self::assertSame([], $controller->flashes, 'nothing was changed, so nothing is reported as success');
}
public function testPickingTheSameHouseIsNotReportedAsAChange(): void
{
$booking = $this->draft();
@@ -7,6 +7,7 @@ namespace App\Tests\Controller\Admin\AccommodationBooking;
use App\Controller\Admin\AccommodationBooking\CreateController;
use App\Entity\Groups\AccommodationBooking;
use App\Entity\User;
use App\Exception\ImplausibleBookingPriceException;
use App\Service\AccommodationBookingService;
use Doctrine\ORM\EntityManagerInterface;
use PHPUnit\Framework\TestCase;
@@ -32,6 +33,37 @@ class CreateControllerTest extends TestCase
self::assertSame($user, $controller->capturedBooking?->getManagedBy());
}
public function testAnImplausiblePriceComesBackAsAFormErrorWithoutSaving(): void
{
// Same as on the edit form: the fix is in the house's price rows, so the office is told
// on the form it is standing in front of, and nothing is written.
$bookingService = $this->createStub(AccommodationBookingService::class);
$bookingService->method('refreshPriceSnapshot')
->willThrowException(new ImplausibleBookingPriceException(AccommodationBooking::MAX_TOTAL_PRICE + 1, 40, 2));
$form = $this->createMock(FormInterface::class);
$form->method('handleRequest')->willReturn($form);
$form->method('isSubmitted')->willReturn(true);
$form->method('isValid')->willReturn(true);
$form->expects(self::once())->method('addError');
$entityManager = $this->createMock(EntityManagerInterface::class);
$entityManager->expects(self::never())->method('persist');
$entityManager->expects(self::never())->method('flush');
$controller = new TestableCreateController(
$entityManager,
$this->createStub(LoggerInterface::class),
$bookingService,
$form,
new User('[email protected]'),
);
$response = $controller->index(Request::create('/admin/accommodation-booking/create', 'POST'));
self::assertSame(Response::HTTP_OK, $response->getStatusCode(), 'the modal is re-rendered, not redirected');
}
private function buildController(?UserInterface $user): TestableCreateController
{
$form = $this->createStub(FormInterface::class);
@@ -8,6 +8,7 @@ use App\Controller\Admin\AccommodationBooking\EditController;
use App\Entity\Groups\AccommodationBooking;
use App\Entity\User;
use App\Enum\Groups\AccommodationBookingStatus;
use App\Exception\ImplausibleBookingPriceException;
use App\Repository\Groups\AdditionalServiceRepository;
use App\Repository\Groups\BoardServiceRepository;
use App\Repository\UserRepository;
@@ -46,6 +47,39 @@ class EditControllerTest extends TestCase
self::assertSame($status, $booking->getStatus(), 'editing leaves the status where it was');
}
public function testAnImplausiblePriceComesBackAsAFormErrorWithoutSaving(): void
{
// The office can fix this — in the house's price rows — so it belongs on the form rather
// than on an error page, and nothing may be flushed in the meantime.
$bookingService = $this->createStub(AccommodationBookingService::class);
$bookingService->method('refreshPriceSnapshot')
->willThrowException(new ImplausibleBookingPriceException(AccommodationBooking::MAX_TOTAL_PRICE + 1, 40, 2));
$form = $this->createMock(FormInterface::class);
$form->method('handleRequest')->willReturn($form);
$form->method('isSubmitted')->willReturn(true);
$form->method('isValid')->willReturn(true);
$form->method('has')->willReturn(false);
$form->expects(self::once())->method('addError');
$entityManager = $this->createMock(EntityManagerInterface::class);
$entityManager->expects(self::never())->method('flush');
$controller = new TestableEditController(
$entityManager,
$this->createStub(LoggerInterface::class),
$this->createStub(BoardServiceRepository::class),
$this->createStub(AdditionalServiceRepository::class),
$this->createStub(UserRepository::class),
$bookingService,
$form,
);
$response = $controller->index(new AccommodationBooking(), Request::create('/admin/accommodation-booking/1/edit', 'POST'));
self::assertSame(Response::HTTP_OK, $response->getStatusCode(), 'the form is re-rendered, not redirected');
}
/**
* @return iterable<string, array{AccommodationBookingStatus}>
*/
@@ -8,11 +8,13 @@ use App\Controller\Groups\Booking\Step4Controller;
use App\Entity\Groups\Accommodation;
use App\Entity\Groups\AccommodationBooking;
use App\Enum\Groups\AccommodationBookingOrigin;
use App\Exception\ImplausibleBookingPriceException;
use App\Form\Model\AccommodationBookingDto;
use App\Service\AccommodationBookingService;
use App\Service\AccommodationSessionManager;
use App\Service\AccommodationTermsUrlProvider;
use PHPUnit\Framework\TestCase;
use Psr\Log\NullLogger;
use Symfony\Component\Form\FormInterface;
use Symfony\Component\HttpFoundation\RedirectResponse;
use Symfony\Component\HttpFoundation\Request;
@@ -302,6 +304,96 @@ class Step4ControllerTest extends TestCase
self::assertNull($sessionManager->getDto($request));
}
public function testConfirmPostEndsOnTheErrorPageWhenThePriceIsImplausible(): void
{
// A mistyped catalog price is the one way an implausible total still gets computed, and
// the customer must not meet it as a 500 — SQLSTATE 22003 on /admin is what started this.
$dto = new AccommodationBookingDto();
$dto->currentStep = 4;
$dto->accommodationId = 1;
$session = new Session(new MockArraySessionStorage());
$request = Request::create('/groups/booking/step-4/confirm', 'POST');
$request->setSession($session);
$request->headers->set('HX-Request', 'true');
$sessionManager = new AccommodationSessionManager();
$sessionManager->save($request, $dto);
$accommodation = (new Accommodation())
->setName('Hotel')
->setCalendarCode('HOTEL')
->setMaxAdolescentAge(17);
$bookingService = $this->createMock(AccommodationBookingService::class);
$bookingService->method('loadAccommodation')->with(1)->willReturn($accommodation);
$bookingService->method('loadAvailableServices')->willReturn([
'boardServices' => [],
'additionalServices' => [],
'groupedAdditionalServices' => [],
'ungroupedAdditionalServices' => [],
]);
$bookingService->method('loadPrices')->willReturn([]);
$bookingService->method('finalizeBooking')
->willThrowException(new ImplausibleBookingPriceException(AccommodationBooking::MAX_TOTAL_PRICE + 1, 40, 2));
$confirmationForm = $this->createStub(FormInterface::class);
$confirmationForm->method('handleRequest')->willReturnSelf();
$confirmationForm->method('isSubmitted')->willReturn(true);
$confirmationForm->method('isValid')->willReturn(true);
$controller = new TestableStep4Controller($bookingService, $sessionManager, $confirmationForm);
$response = $controller->confirm($request);
self::assertSame('/app_groups_booking_error', $response->headers->get('HX-Redirect'));
// The session survives: the desk still has the inquiry to pick up by phone.
self::assertNotNull($sessionManager->getDto($request));
}
public function testIndexPostEndsOnTheErrorPageWhenThePriceIsImplausible(): void
{
$dto = new AccommodationBookingDto();
$dto->currentStep = 4;
$dto->accommodationId = 1;
$session = new Session(new MockArraySessionStorage());
$request = Request::create('/groups/booking/step-4', 'POST');
$request->setSession($session);
$sessionManager = new AccommodationSessionManager();
$sessionManager->save($request, $dto);
$accommodation = (new Accommodation())
->setName('Hotel')
->setCalendarCode('HOTEL')
->setMaxAdolescentAge(17);
$bookingService = $this->createMock(AccommodationBookingService::class);
$bookingService->method('loadAccommodation')->with(1)->willReturn($accommodation);
$bookingService->method('loadAvailableServices')->willReturn([
'boardServices' => [],
'additionalServices' => [],
'groupedAdditionalServices' => [],
'ungroupedAdditionalServices' => [],
]);
$bookingService->method('loadPrices')->willReturn([]);
$bookingService->method('finalizeBooking')
->willThrowException(new ImplausibleBookingPriceException(AccommodationBooking::MAX_TOTAL_PRICE + 1, 40, 2));
$inquiryForm = $this->createStub(FormInterface::class);
$inquiryForm->method('handleRequest')->willReturnSelf();
$inquiryForm->method('isSubmitted')->willReturn(true);
$inquiryForm->method('isValid')->willReturn(true);
$controller = new TestableStep4Controller($bookingService, $sessionManager, $inquiryForm);
$response = $controller->index($request);
self::assertInstanceOf(RedirectResponse::class, $response);
self::assertSame('/app_groups_booking_error', $response->getTargetUrl());
}
public function testConfirmPostInvalidReRendersModalWithoutPersisting(): void
{
$dto = new AccommodationBookingDto();
@@ -358,6 +450,7 @@ final class TestableStep4Controller extends Step4Controller
$bookingService,
$sessionManager,
new AccommodationTermsUrlProvider(['AT' => '', 'CH' => '', 'IT' => ''], 'https://example.test/agb/'),
new NullLogger(),
);
}
@@ -7,6 +7,7 @@ namespace App\Tests\Entity\Groups;
use App\Entity\Groups\AccommodationBooking;
use App\Enum\Groups\AccommodationBookingOrigin;
use App\Enum\Groups\AccommodationBookingStatus;
use App\Exception\ImplausibleBookingPriceException;
use PHPUnit\Framework\Attributes\DataProvider;
use PHPUnit\Framework\TestCase;
@@ -87,6 +88,36 @@ class AccommodationBookingTest extends TestCase
}
}
public function testAnImplausibleTotalIsRefusedRatherThanStored(): void
{
// The pax count and the nights are capped, so what gets here is a catalog price with a
// digit too many. A stored total would end up on an offer; the INT column would answer
// it with SQLSTATE 22003 and say nothing about the cause.
$booking = new AccommodationBooking();
$booking->setDateFrom(new \DateTimeImmutable('2026-09-23'));
$booking->setDateTo(new \DateTimeImmutable('2026-09-25'));
$booking->setPaxCount(40);
$this->expectException(ImplausibleBookingPriceException::class);
$booking->setPriceSnapshot(['total' => AccommodationBooking::MAX_TOTAL_PRICE + 1], AccommodationBooking::MAX_TOTAL_PRICE + 1, 'EUR', 1);
}
public function testATotalAtTheCeilingIsStillStored(): void
{
$booking = new AccommodationBooking();
$booking->setPriceSnapshot(['total' => AccommodationBooking::MAX_TOTAL_PRICE], AccommodationBooking::MAX_TOTAL_PRICE, 'EUR', 1);
self::assertSame(AccommodationBooking::MAX_TOTAL_PRICE, $booking->getTotalPrice());
}
public function testTheCeilingLeavesTheIntColumnRoomToSpare(): void
{
// The ceiling exists to keep implausible prices out, not to use up the column: a total
// that needs more room than this is a mistake, never a wider column.
self::assertLessThan(2147483647, AccommodationBooking::MAX_TOTAL_PRICE);
}
public function testTheLabelIsIndependentOfWhereTheBookingCameFrom(): void
{
// Origin answers "how did this come about" and is deliberately not the same
@@ -109,6 +109,60 @@ class AccommodationBookingTypeTest extends TestCase
self::assertCount(0, $this->validate($booking, ['Default']));
}
/**
* The overflow this guards against reached production as an out-of-range write on
* total_price: every figure in the breakdown is multiplied by the pax count and the
* nights, so an implausible input for either produces an implausible total.
*/
public function testADepartureBeforeTheArrivalIsRejected(): void
{
$booking = $this->bookingFor('2026-09-23', '2026-09-20', 30);
$violations = $this->validate($booking, ['Default']);
self::assertCount(1, $violations);
self::assertSame('dateTo', $violations[0]->getPropertyPath());
}
public function testAMistypedYearIsRejectedRatherThanPriced(): void
{
// 2226 instead of 2026 — one wrong digit, 200 years of running costs.
$booking = $this->bookingFor('2026-09-23', '2226-09-25', 30);
$violations = $this->validate($booking, ['Default']);
self::assertCount(1, $violations);
self::assertSame('dateTo', $violations[0]->getPropertyPath());
}
public function testAnImplausiblePaxCountIsRejected(): void
{
$booking = $this->bookingFor('2026-09-23', '2026-09-25', AccommodationBooking::MAX_PAX + 1);
$violations = $this->validate($booking, ['Default']);
self::assertCount(1, $violations);
self::assertSame('paxCount', $violations[0]->getPropertyPath());
}
public function testTheLargestBookingOnRecordStillPasses(): void
{
// 150 people for 18 nights: the ceilings must not get in the way of real business.
$booking = $this->bookingFor('2026-09-23', '2026-10-11', 150);
self::assertCount(0, $this->validate($booking, ['Default']));
}
private function bookingFor(string $dateFrom, string $dateTo, int $paxCount): AccommodationBooking
{
$booking = new AccommodationBooking();
$booking->setDateFrom(new \DateTimeImmutable($dateFrom));
$booking->setDateTo(new \DateTimeImmutable($dateTo));
$booking->setPaxCount($paxCount);
return $booking;
}
/**
* @return string[]
*/
@@ -0,0 +1,69 @@
<?php
declare(strict_types=1);
namespace App\Tests\Form\Model;
use App\Entity\Groups\AccommodationBooking;
use App\Form\Model\AccommodationBookingDto;
use PHPUnit\Framework\Attributes\DataProvider;
use PHPUnit\Framework\TestCase;
use Symfony\Component\Validator\ConstraintViolationListInterface;
use Symfony\Component\Validator\Validation;
class AccommodationBookingDtoTest extends TestCase
{
/**
* Step 2 is the screen the customer types the head count on, and the one that prices it.
* Bounding it only in step_3 left the figure to surface two steps later, as a booking that
* could not be stored.
*/
#[DataProvider('implausibleHeadCounts')]
public function testAnImplausibleHeadCountIsRejectedWhereItIsEntered(string $property, int $value): void
{
$dto = new AccommodationBookingDto();
$dto->{$property} = $value;
$properties = array_map(
static fn ($violation) => $violation->getPropertyPath(),
iterator_to_array($this->validate($dto, ['step_2'])),
);
self::assertContains($property, $properties);
}
/**
* @return iterable<string, array{string, int}>
*/
public static function implausibleHeadCounts(): iterable
{
yield 'pax above the ceiling' => ['paxCount', AccommodationBooking::MAX_PAX + 1];
yield 'pax below one' => ['paxCount', 0];
yield 'negative pax' => ['paxCount', -5];
yield 'minors above the ceiling' => ['minorsCount', AccommodationBooking::MAX_PAX + 1];
yield 'negative minors' => ['minorsCount', -1];
yield 'children above the ceiling' => ['childrenCount', AccommodationBooking::MAX_PAX + 1];
}
public function testAFullHouseStillPasses(): void
{
$dto = new AccommodationBookingDto();
$dto->paxCount = AccommodationBooking::MAX_PAX;
$dto->minorsCount = 4;
$dto->childrenCount = 60;
$dto->selectedBoardServiceId = 1;
self::assertCount(0, $this->validate($dto, ['step_2']));
}
/**
* @param string[] $groups
*/
private function validate(AccommodationBookingDto $dto, array $groups): ConstraintViolationListInterface
{
return Validation::createValidatorBuilder()
->enableAttributeMapping()
->getValidator()
->validate($dto, null, $groups);
}
}
@@ -12,6 +12,7 @@ use App\Entity\Groups\AccommodationPrice;
use App\Enum\Groups\AccommodationBookingOrigin;
use App\Enum\Groups\AccommodationBookingStatus;
use App\Enum\Groups\AdditionalServiceType;
use App\Exception\ImplausibleBookingPriceException;
use App\Form\Model\AccommodationBookingDto;
use App\Model\AccommodationBookingQueryParams;
use App\Repository\Groups\AccommodationPriceRepository;
@@ -65,6 +66,18 @@ class AccommodationBookingServiceTest extends TestCase
$service->initFromParams(new AccommodationBookingQueryParams('HOTEL', '2026-02-31', '2026-03-05'));
}
public function testApplyDatesRejectsAnImplausiblyLongStay(): void
{
// The customer picks from a calendar, so this is a hand-built URL rather than a typo —
// the cap is the same one the backoffice form enforces on the entity.
$service = $this->createServiceWithAccommodation();
$this->expectException(\InvalidArgumentException::class);
$this->expectExceptionMessage(sprintf('Der Aufenthalt darf höchstens %d Nächte umfassen.', AccommodationBooking::MAX_NIGHTS));
$service->applyDates(new AccommodationBookingDto(), '2026-02-28', '2226-03-01');
}
public function testIssueAccessLinkForDirectBookingSetsTimestampForDirectBooking(): void
{
$entityManager = $this->createMock(EntityManagerInterface::class);
@@ -962,6 +975,33 @@ class AccommodationBookingServiceTest extends TestCase
self::assertNull($booking->getTotalPrice(), 'the frozen price belonged to the old house');
}
public function testChangeAccommodationLeavesNothingHalfAppliedWhenTheNewHousePricesImplausibly(): void
{
// The swap and the dropped services are already on the managed entity by the time the
// price is computed, so a refusal has to put the row back rather than leave a booking
// sitting at a new house with no price.
$breakdown = ['total' => AccommodationBooking::MAX_TOTAL_PRICE + 1, 'currency' => 'EUR'];
$breakdownCalculator = $this->createStub(AccommodationBookingBreakdownCalculator::class);
$breakdownCalculator->method('computeCurrent')->willReturn($breakdown);
$breakdownCalculator->method('withDiscounts')
->willReturn($breakdown + ['discounts' => [], 'discountedTotal' => AccommodationBooking::MAX_TOTAL_PRICE + 1]);
$entityManager = $this->createMock(EntityManagerInterface::class);
$entityManager->expects(self::once())->method('refresh');
$entityManager->expects(self::never())->method('flush');
$service = $this->createServiceWithAccommodation(
entityManager: $entityManager,
breakdownCalculator: $breakdownCalculator,
);
$booking = $this->draftWithServices();
$this->expectException(ImplausibleBookingPriceException::class);
$service->changeAccommodation($booking, (new Accommodation())->setName('Berghaus')->setCalendarCode('BERG'));
}
public function testChangeAccommodationNoOpsForANonDraft(): void
{
$entityManager = $this->createMock(EntityManagerInterface::class);