diff --git a/src/Controller/Admin/AccommodationBooking/ChangeAccommodationController.php b/src/Controller/Admin/AccommodationBooking/ChangeAccommodationController.php index 73d3d021..205b44b7 100644 --- a/src/Controller/Admin/AccommodationBooking/ChangeAccommodationController.php +++ b/src/Controller/Admin/AccommodationBooking/ChangeAccommodationController.php @@ -6,11 +6,13 @@ namespace App\Controller\Admin\AccommodationBooking; use App\Entity\Groups\Accommodation; use App\Entity\Groups\AccommodationBooking; +use App\Exception\ImplausibleBookingPriceException; use App\Form\Admin\Groups\AccommodationBookingChangeAccommodationType; use App\Htmx\HxTrait; use App\Service\AccommodationBookingService; use Psr\Log\LoggerInterface; use Symfony\Bundle\FrameworkBundle\Controller\AbstractController; +use Symfony\Component\Form\FormError; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\Routing\Attribute\Route; @@ -49,7 +51,24 @@ class ChangeAccommodationController extends AbstractController $selected = $form->get('accommodation')->getData(); if ($selected instanceof Accommodation && $selected !== $booking->getAccommodation()) { - $this->bookingService->changeAccommodation($booking, $selected); + try { + $this->bookingService->changeAccommodation($booking, $selected); + } catch (ImplausibleBookingPriceException $e) { + // The house that was picked prices this stay implausibly high, so the change + // did not happen. Reported in the modal the office is standing in front of — + // the same treatment the create and edit forms give it. + $this->logger->error('Refused to change accommodation over an implausible booking total', [ + 'exception' => $e, + 'id' => $booking->getId(), + 'accommodation' => $selected->getCalendarCode(), + ]); + $form->addError(new FormError(ImplausibleBookingPriceException::FORM_ERROR_MESSAGE)); + + return $this->render('admin/accommodation_booking/modal_change_accommodation.html.twig', [ + 'booking' => $booking, + 'form' => $form, + ]); + } $this->addFlash('success', 'Das Gruppenhaus wurde geändert. Bitte wähle die Leistungen neu aus.'); diff --git a/src/Controller/Admin/AccommodationBooking/CreateController.php b/src/Controller/Admin/AccommodationBooking/CreateController.php index b38b9e0e..ee4483bf 100644 --- a/src/Controller/Admin/AccommodationBooking/CreateController.php +++ b/src/Controller/Admin/AccommodationBooking/CreateController.php @@ -6,12 +6,15 @@ namespace App\Controller\Admin\AccommodationBooking; use App\Entity\Groups\AccommodationBooking; use App\Entity\User; +use App\Exception\ImplausibleBookingPriceException; use App\Form\Admin\Groups\AccommodationBookingCreateType; use App\Htmx\HxRedirectResponse; use App\Service\AccommodationBookingService; use Doctrine\ORM\EntityManagerInterface; use Psr\Log\LoggerInterface; use Symfony\Bundle\FrameworkBundle\Controller\AbstractController; +use Symfony\Component\Form\FormError; +use Symfony\Component\Form\FormInterface; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\Routing\Attribute\Route; @@ -45,8 +48,7 @@ class CreateController extends AbstractController ]); $form->handleRequest($request); - if ($form->isSubmitted() && $form->isValid()) { - $this->bookingService->refreshPriceSnapshot($booking); + if ($form->isSubmitted() && $form->isValid() && $this->refreshPriceSnapshot($form, $booking)) { $this->entityManager->persist($booking); $this->entityManager->flush(); @@ -65,4 +67,31 @@ class CreateController extends AbstractController 'form' => $form, ]); } + + /** + * The price is computed after validation, so an implausible total cannot be a constraint on + * the form — it is reported as one anyway, because the office's next move is to go and fix + * the house's prices, not to read an error page. + * + * @param FormInterface $form + */ + private function refreshPriceSnapshot(FormInterface $form, AccommodationBooking $booking): bool + { + try { + $this->bookingService->refreshPriceSnapshot($booking); + + return true; + } catch (ImplausibleBookingPriceException $e) { + $this->logger->error('Refused to store an implausible accommodation booking total', [ + 'exception' => $e, + 'id' => $booking->getId(), + 'accommodation' => $booking->getAccommodation()?->getCalendarCode(), + 'paxCount' => $booking->getPaxCount(), + 'nights' => $booking->getNights(), + ]); + $form->addError(new FormError(ImplausibleBookingPriceException::FORM_ERROR_MESSAGE)); + + return false; + } + } } diff --git a/src/Controller/Admin/AccommodationBooking/EditController.php b/src/Controller/Admin/AccommodationBooking/EditController.php index 85a495a4..bb73e028 100644 --- a/src/Controller/Admin/AccommodationBooking/EditController.php +++ b/src/Controller/Admin/AccommodationBooking/EditController.php @@ -7,6 +7,7 @@ namespace App\Controller\Admin\AccommodationBooking; use App\Entity\Groups\AccommodationBooking; use App\Entity\Groups\AdditionalService; use App\Entity\Groups\BoardService; +use App\Exception\ImplausibleBookingPriceException; use App\Form\Admin\Groups\AccommodationBookingType; use App\Repository\Groups\AdditionalServiceRepository; use App\Repository\Groups\BoardServiceRepository; @@ -15,6 +16,8 @@ use App\Service\AccommodationBookingService; use Doctrine\ORM\EntityManagerInterface; use Psr\Log\LoggerInterface; use Symfony\Bundle\FrameworkBundle\Controller\AbstractController; +use Symfony\Component\Form\FormError; +use Symfony\Component\Form\FormInterface; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\Routing\Attribute\Route; @@ -121,7 +124,15 @@ class EditController extends AbstractController } } - $this->bookingService->refreshPriceSnapshot($booking); + if (!$this->refreshPriceSnapshot($form, $booking)) { + // The submitted changes stay on the entity but are never flushed, so the form + // comes back filled in with the error on it. Logging is safe here: the database + // handler writes through DBAL, not the EntityManager. + return $this->render('admin/accommodation_booking/edit.html.twig', [ + 'booking' => $booking, + 'form' => $form, + ]); + } // Editing never moves the booking on and never mails the customer: every status // change is its own named action (Angebot senden, bestätigen, absagen), so the @@ -154,4 +165,31 @@ class EditController extends AbstractController 'form' => $form, ]); } + + /** + * The price is computed after validation, so an implausible total cannot be a constraint on + * the form — it is reported as one anyway, because the office's next move is to go and fix + * the house's prices, not to read an error page. + * + * @param FormInterface $form + */ + private function refreshPriceSnapshot(FormInterface $form, AccommodationBooking $booking): bool + { + try { + $this->bookingService->refreshPriceSnapshot($booking); + + return true; + } catch (ImplausibleBookingPriceException $e) { + $this->logger->error('Refused to store an implausible accommodation booking total', [ + 'exception' => $e, + 'id' => $booking->getId(), + 'accommodation' => $booking->getAccommodation()?->getCalendarCode(), + 'paxCount' => $booking->getPaxCount(), + 'nights' => $booking->getNights(), + ]); + $form->addError(new FormError(ImplausibleBookingPriceException::FORM_ERROR_MESSAGE)); + + return false; + } + } } diff --git a/src/Controller/Groups/Booking/Step4Controller.php b/src/Controller/Groups/Booking/Step4Controller.php index e6134221..f2a9d0a5 100644 --- a/src/Controller/Groups/Booking/Step4Controller.php +++ b/src/Controller/Groups/Booking/Step4Controller.php @@ -8,6 +8,7 @@ use App\Entity\Groups\Accommodation; use App\Entity\Groups\AdditionalService; use App\Entity\Groups\BoardService; use App\Exception\AccommodationSessionNotFoundException; +use App\Exception\ImplausibleBookingPriceException; use App\Form\AccommodationBookingConfirmationType; use App\Form\AccommodationInquiryConfirmationType; use App\Form\Model\AccommodationBookingDto; @@ -16,6 +17,7 @@ use App\Model\AccommodationBookingContext; use App\Service\AccommodationBookingService; use App\Service\AccommodationSessionManager; use App\Service\AccommodationTermsUrlProvider; +use Psr\Log\LoggerInterface; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\Routing\Attribute\Route; @@ -28,6 +30,7 @@ class Step4Controller extends AbstractController private readonly AccommodationBookingService $bookingService, private readonly AccommodationSessionManager $sessionManager, private readonly AccommodationTermsUrlProvider $termsUrlProvider, + private readonly LoggerInterface $logger, ) { } @@ -51,7 +54,9 @@ class Step4Controller extends AbstractController } $dto->forceInquiry = true; - $this->persistBooking($request, $dto, $accommodation, $services); + if (null !== $failure = $this->persistBooking($request, $dto, $accommodation, $services)) { + return $failure; + } return $this->redirectToRoute('app_groups_booking_success'); } @@ -102,7 +107,9 @@ class Step4Controller extends AbstractController if ($confirmationForm->isSubmitted() && $confirmationForm->isValid()) { $services = $this->bookingService->loadAvailableServices($dto, $accommodation); - $this->persistBooking($request, $dto, $accommodation, $services); + if (null !== $failure = $this->persistBooking($request, $dto, $accommodation, $services)) { + return $failure; + } return $this->htmxRedirect($request, $this->generateUrl('app_groups_booking_success')); } @@ -148,12 +155,35 @@ class Step4Controller extends AbstractController * groupedAdditionalServices: array, * ungroupedAdditionalServices: AdditionalService[] * } $services + * + * @return Response|null a response to return instead of the success redirect, or null when + * the booking was stored */ - private function persistBooking(Request $request, AccommodationBookingDto $dto, Accommodation $accommodation, array $services): void + private function persistBooking(Request $request, AccommodationBookingDto $dto, Accommodation $accommodation, array $services): ?Response { $prices = $this->bookingService->loadPrices($dto, $accommodation); - $booking = $this->bookingService->finalizeBooking($dto, $accommodation, $prices, $services); + + try { + $booking = $this->bookingService->finalizeBooking($dto, $accommodation, $prices, $services); + } catch (ImplausibleBookingPriceException $e) { + // The bounded inputs are already rejected in step 2, so what is left is a mistyped + // catalog price for this house — nothing the customer can act on and nothing to show + // them an error page over. The session survives so the desk can pick the inquiry up + // by phone with the data still in hand. + $this->logger->error('Refused to store an implausible accommodation booking total', [ + 'exception' => $e, + 'accommodation' => $accommodation->getCalendarCode(), + 'paxCount' => $dto->paxCount, + 'nights' => $dto->getNights(), + ]); + $this->addFlash('error', 'Die Anfrage konnte nicht abgeschlossen werden. Bitte wende dich direkt an uns — wir kümmern uns darum.'); + + return $this->htmxRedirect($request, $this->generateUrl('app_groups_booking_error')); + } + $this->addFlash('groups_booking_result', $booking->getOrigin()->value); $this->sessionManager->clear($request); + + return null; } } diff --git a/src/Entity/Groups/AccommodationBooking.php b/src/Entity/Groups/AccommodationBooking.php index 87bd3401..c3f98a55 100644 --- a/src/Entity/Groups/AccommodationBooking.php +++ b/src/Entity/Groups/AccommodationBooking.php @@ -12,11 +12,13 @@ use App\Entity\User; use App\Enum\Groups\AccommodationBookingOrigin; use App\Enum\Groups\AccommodationBookingStatus; use App\Enum\Groups\AdditionalServiceType; +use App\Exception\ImplausibleBookingPriceException; use App\Repository\Groups\AccommodationBookingRepository; use Doctrine\DBAL\Types\Types; use Doctrine\ORM\Mapping as ORM; use Symfony\Component\Uid\Uuid; use Symfony\Component\Validator\Constraints as Assert; +use Symfony\Component\Validator\Context\ExecutionContextInterface; #[ORM\Entity(repositoryClass: AccommodationBookingRepository::class)] class AccommodationBooking implements BlameableEntityInterface, TimestampableEntityInterface @@ -24,6 +26,24 @@ class AccommodationBooking implements BlameableEntityInterface, TimestampableEnt use BlameableEntity; use TimestampableEntity; + /** + * Plausibility ceilings for the two figures every price in the breakdown is multiplied by. + * Both sit above anything the group houses actually see — the largest booking on record is + * 150 people for 18 nights — and exist so that a mistyped year or pax count is answered with + * a form error rather than a price nobody meant. + */ + public const int MAX_PAX = 250; + public const int MAX_NIGHTS = 30; + + /** + * And the ceiling on the product of the two, in minor units: a million euros against a + * largest-ever booking of some 42,000. The inputs above cannot reach it, so what it catches + * is a mistyped catalog price, the one factor of the total that is not bounded per booking. + * Deliberately well inside what the INT column holds — the column's range is not a price + * limit, and a total that needs more room is a mistake rather than a wider column. + */ + public const int MAX_TOTAL_PRICE = 100_000_000; + #[ORM\Id] #[ORM\GeneratedValue] #[ORM\Column] @@ -40,9 +60,11 @@ class AccommodationBooking implements BlameableEntityInterface, TimestampableEnt private ?\DateTimeImmutable $dateFrom = null; #[ORM\Column(type: Types::DATE_IMMUTABLE)] + #[Assert\GreaterThan(propertyPath: 'dateFrom', message: 'Das Abreisedatum muss nach dem Anreisedatum liegen.')] private ?\DateTimeImmutable $dateTo = null; #[ORM\Column] + #[Assert\Range(min: 0, max: self::MAX_PAX)] private int $paxCount = 0; #[ORM\Column] @@ -68,6 +90,7 @@ class AccommodationBooking implements BlameableEntityInterface, TimestampableEnt #[ORM\Column(type: Types::JSON, nullable: true)] private ?array $priceBreakdown = null; + /** In minor units, like every other price here, and never above self::MAX_TOTAL_PRICE. */ #[ORM\Column(nullable: true)] private ?int $totalPrice = null; @@ -232,6 +255,25 @@ class AccommodationBooking implements BlameableEntityInterface, TimestampableEnt return $this->dateFrom->diff($this->dateTo)->days; } + /** + * Reported on dateTo, the field a mistyped year is corrected in — a stay of 200 years is + * always the departure date being wrong, never the arrival. + */ + #[Assert\Callback] + public function validateStayLength(ExecutionContextInterface $context): void + { + if (null === $this->dateFrom || null === $this->dateTo) { + return; + } + + if ($this->getNights() > self::MAX_NIGHTS) { + $context->buildViolation('Der Aufenthalt darf höchstens {{ max }} Nächte umfassen — bitte Anreise und Abreise prüfen.') + ->setParameter('{{ max }}', (string) self::MAX_NIGHTS) + ->atPath('dateTo') + ->addViolation(); + } + } + public function getPaxCount(): int { return $this->paxCount; @@ -336,9 +378,17 @@ class AccommodationBooking implements BlameableEntityInterface, TimestampableEnt return $this->priceBreakdown; } - /** @param array $priceBreakdown */ + /** + * @param array $priceBreakdown + * + * @throws ImplausibleBookingPriceException if the total is too large to be a real price + */ public function setPriceSnapshot(array $priceBreakdown, int $totalPrice, string $currency, int $version): self { + if ($totalPrice > self::MAX_TOTAL_PRICE) { + throw new ImplausibleBookingPriceException($totalPrice, $this->paxCount, $this->getNights()); + } + $this->priceBreakdown = $priceBreakdown; $this->totalPrice = $totalPrice; $this->pricingCurrency = $currency; diff --git a/src/Exception/ImplausibleBookingPriceException.php b/src/Exception/ImplausibleBookingPriceException.php new file mode 100644 index 00000000..1ca45a39 --- /dev/null +++ b/src/Exception/ImplausibleBookingPriceException.php @@ -0,0 +1,40 @@ +diff($dateTo)->days > AccommodationBooking::MAX_NIGHTS) { + throw new \InvalidArgumentException(sprintf('Der Aufenthalt darf höchstens %d Nächte umfassen.', AccommodationBooking::MAX_NIGHTS)); + } + $dto->dateFrom = $dateFrom->setTime(0, 0, 0); $dto->dateTo = $dateTo->setTime(0, 0, 0); } @@ -566,6 +572,9 @@ class AccommodationBookingService * their access link, and swapping the house underneath them would rewrite what they were * offered. * Idempotent — a no-op for a booking that is not a draft or already sits at $accommodation. + * + * @throws ImplausibleBookingPriceException if the new house prices the stay implausibly high; + * the booking is left at its old house */ public function changeAccommodation(AccommodationBooking $booking, Accommodation $accommodation): void { @@ -580,8 +589,18 @@ class AccommodationBookingService $booking->setBoardServiceOriginalId(null); $booking->setAdditionalServices([]); - // Clears the snapshot by itself when the new house has no price for the booked range. - $this->refreshPriceSnapshot($booking); + try { + // Clears the snapshot by itself when the new house has no price for the booked range. + $this->refreshPriceSnapshot($booking); + } catch (ImplausibleBookingPriceException $e) { + // The swap and the dropped services are already on the managed entity, and the next + // flush in this request would write them without a price. Reloading the row puts the + // booking back at its old house so the caller can report the refusal on a record that + // still matches the database. + $this->entityManager->refresh($booking); + + throw $e; + } $this->entityManager->flush(); } diff --git a/tests/Controller/Admin/AccommodationBooking/ChangeAccommodationControllerTest.php b/tests/Controller/Admin/AccommodationBooking/ChangeAccommodationControllerTest.php index f53936d8..b2bbf132 100644 --- a/tests/Controller/Admin/AccommodationBooking/ChangeAccommodationControllerTest.php +++ b/tests/Controller/Admin/AccommodationBooking/ChangeAccommodationControllerTest.php @@ -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(); diff --git a/tests/Controller/Admin/AccommodationBooking/CreateControllerTest.php b/tests/Controller/Admin/AccommodationBooking/CreateControllerTest.php index 9f71d7b2..1498aceb 100644 --- a/tests/Controller/Admin/AccommodationBooking/CreateControllerTest.php +++ b/tests/Controller/Admin/AccommodationBooking/CreateControllerTest.php @@ -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('manager@example.test'), + ); + + $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); diff --git a/tests/Controller/Admin/AccommodationBooking/EditControllerTest.php b/tests/Controller/Admin/AccommodationBooking/EditControllerTest.php index 45d12bf8..e3d457a3 100644 --- a/tests/Controller/Admin/AccommodationBooking/EditControllerTest.php +++ b/tests/Controller/Admin/AccommodationBooking/EditControllerTest.php @@ -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 */ diff --git a/tests/Controller/Groups/Booking/Step4ControllerTest.php b/tests/Controller/Groups/Booking/Step4ControllerTest.php index a89e98af..54cb22d1 100644 --- a/tests/Controller/Groups/Booking/Step4ControllerTest.php +++ b/tests/Controller/Groups/Booking/Step4ControllerTest.php @@ -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(), ); } diff --git a/tests/Entity/Groups/AccommodationBookingTest.php b/tests/Entity/Groups/AccommodationBookingTest.php index 72b1bfe2..0b9d1139 100644 --- a/tests/Entity/Groups/AccommodationBookingTest.php +++ b/tests/Entity/Groups/AccommodationBookingTest.php @@ -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 diff --git a/tests/Form/Admin/Groups/AccommodationBookingTypeTest.php b/tests/Form/Admin/Groups/AccommodationBookingTypeTest.php index 72bf1df8..36ad8f05 100644 --- a/tests/Form/Admin/Groups/AccommodationBookingTypeTest.php +++ b/tests/Form/Admin/Groups/AccommodationBookingTypeTest.php @@ -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[] */ diff --git a/tests/Form/Model/AccommodationBookingDtoTest.php b/tests/Form/Model/AccommodationBookingDtoTest.php new file mode 100644 index 00000000..b76fd343 --- /dev/null +++ b/tests/Form/Model/AccommodationBookingDtoTest.php @@ -0,0 +1,69 @@ +{$property} = $value; + + $properties = array_map( + static fn ($violation) => $violation->getPropertyPath(), + iterator_to_array($this->validate($dto, ['step_2'])), + ); + + self::assertContains($property, $properties); + } + + /** + * @return iterable + */ + 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); + } +} diff --git a/tests/Service/AccommodationBookingServiceTest.php b/tests/Service/AccommodationBookingServiceTest.php index 1392f445..6f452138 100644 --- a/tests/Service/AccommodationBookingServiceTest.php +++ b/tests/Service/AccommodationBookingServiceTest.php @@ -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);