From 578a21d327de3fe84fca7a4739afc2f802e2942a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Fromme?= Date: Mon, 24 Aug 2026 17:13:19 +0200 Subject: [PATCH] fix: stop mutating booking state on edit-mode page views --- .../Booking/Edit/ParticipantController.php | 4 - .../Edit/ParticipantControllerTest.php | 123 +++++++++++++++++- 2 files changed, 118 insertions(+), 9 deletions(-) diff --git a/src/Controller/Booking/Edit/ParticipantController.php b/src/Controller/Booking/Edit/ParticipantController.php index cba58db..8b690a5 100644 --- a/src/Controller/Booking/Edit/ParticipantController.php +++ b/src/Controller/Booking/Edit/ParticipantController.php @@ -10,7 +10,6 @@ use App\Entity\User; use App\Form\BookingParticipantType; use App\Form\Model\BookingDto; use App\Htmx\HxTrait; -use App\Service\ApplicantInsuranceCascade; use App\Service\BookingConfigurator; use App\Service\BookingEditContextFactory; use App\Service\BookingEditDataLoader; @@ -41,7 +40,6 @@ class ParticipantController extends AbstractController private readonly BookingSessionManager $bookingSessionService, private readonly ParticipantDataPrefiller $prepopulationService, private readonly ParticipantFormSupport $participantFormSupportService, - private readonly ApplicantInsuranceCascade $applicantInsuranceCascade, private readonly LoggerInterface $logger, ) { } @@ -87,7 +85,6 @@ class ParticipantController extends AbstractController } $this->editContextFactory->prepareBookingDto($bookingDto); - $this->applicantInsuranceCascade->apply($bookingDto); $form = $this->createParticipantForm($bookingDto, $index); @@ -159,7 +156,6 @@ class ParticipantController extends AbstractController $form->handleRequest($request); $this->bookingConfigurator->preselectDefaultServices($bookingDto); - $this->applicantInsuranceCascade->apply($bookingDto); $this->bookingSessionService->saveBookingDto($request, $bookingDto, BookingDto::MODE_EDIT); $form = $this->createForm( diff --git a/tests/Controller/Booking/Edit/ParticipantControllerTest.php b/tests/Controller/Booking/Edit/ParticipantControllerTest.php index 6b4a70d..1983d39 100644 --- a/tests/Controller/Booking/Edit/ParticipantControllerTest.php +++ b/tests/Controller/Booking/Edit/ParticipantControllerTest.php @@ -6,6 +6,7 @@ namespace App\Tests\Controller\Booking\Edit; use App\BusProNet\ApiClient; use App\BusProNet\Model\Booking; +use App\BusProNet\Model\Insurance; use App\BusProNet\Model\Travel; use App\Controller\Booking\Edit\ParticipantController; use App\Entity\User; @@ -16,7 +17,7 @@ use App\Form\Model\BookingSummaryDto; use App\Form\Model\ParticipantDto; use App\Form\Model\ParticipantEditDto; use App\Security\Crypt; -use App\Service\ApplicantInsuranceCascade; +use App\Service\BookingChangeTracker; use App\Service\BookingConfigurator; use App\Service\BookingEditContextFactory; use App\Service\BookingEditDataLoader; @@ -134,7 +135,6 @@ class ParticipantControllerTest extends TestCase $bookingSessionService, $prepopulationService, $participantFormSupportService, - $this->createMock(ApplicantInsuranceCascade::class), $user, $form, ); @@ -243,7 +243,6 @@ class ParticipantControllerTest extends TestCase $bookingSessionService, $prepopulationService, $participantFormSupportService, - $this->createMock(ApplicantInsuranceCascade::class), $user, $form, ); @@ -260,6 +259,92 @@ class ParticipantControllerTest extends TestCase return new User('tester@example.com'); } + /** + * Regression: the edit flow must not mutate the BookingDto on a page view. + * + * BookingChangeTracker hashes each participant's insurance into the fingerprint that + * drives unsaved-changes detection, and the baseline is taken from un-cascaded API data + * in BookingEditDataLoader before the user reaches this route. An earlier revision called + * ApplicantInsuranceCascade here, which cleared dependents' BPN-loaded insurance and + * re-tiered the applicant's. That made the booking permanently "dirty": the overview only + * re-baselines while still clean, and the reload-from-BusPro path is gated on the same + * flag, so BusPro-side changes stopped surfacing for the rest of the session. + */ + public function testRefreshDoesNotDirtyFingerprintForFamilyInsuranceBooking(): void + { + $request = Request::create('/bookings/42/edit/participants/0/refresh', 'POST'); + $user = $this->createUser(); + $bookingDto = $this->createFamilyInsuranceBookingDto(); + $bookingData = $this->createBooking(); + $bookingData->participantsStatus = ['F', 'F']; + $participant = $bookingDto->participants[0]; + + $applicantInsurance = $bookingDto->participants[0]->insurance; + $dependentInsurance = $bookingDto->participants[1]->insurance; + + $changeTracker = new BookingChangeTracker(); + $bookingDto->originalFingerprint = $changeTracker->generateFingerprint($bookingDto); + $this->assertFalse($changeTracker->isDirty($bookingDto), 'Baseline must start clean'); + + $form = $this->createMock(FormInterface::class); + $form->method('handleRequest')->willReturnSelf(); + $form->method('createView')->willReturn(new FormView()); + + $dataLoader = $this->createMock(BookingEditDataLoader::class); + $dataLoader->method('fetchBookingData')->willReturn($bookingData); + + $bookingSessionService = $this->createMock(BookingSessionManager::class); + $bookingSessionService->method('getBookingDto')->willReturn($bookingDto); + + $participantFormSupportService = $this->createMock(ParticipantFormSupport::class); + $participantFormSupportService->method('ensureParticipantExists')->willReturn($participant); + $participantFormSupportService->method('createParticipantEditDto') + ->willReturn(new ParticipantEditDto($participant, $bookingDto)); + $participantFormSupportService->method('getParticipantFormOptions')->willReturn([ + 'booking_context' => $bookingDto, + 'validation_groups' => false, + ]); + $participantFormSupportService->method('collectAndClearNotifications')->willReturn([]); + + $summaryData = $this->createMock(BookingSummaryDto::class); + $contextFactory = $this->createMock(BookingEditContextFactory::class); + $contextFactory->method('createParticipantContext') + ->willReturn(new BookingEditContext($bookingDto, $bookingData, null, $summaryData)); + + $controller = new TestableParticipantController( + $dataLoader, + $this->createMock(BookingEditDraftManager::class), + $contextFactory, + $this->createMock(BookingConfigurator::class), + $bookingSessionService, + new ParticipantDataPrefiller( + $this->createMock(ApiClient::class), + $this->createMock(Crypt::class), + $this->createMock(LoggerInterface::class), + ), + $participantFormSupportService, + $user, + $form, + ); + + $controller->refreshParticipantForm(42, 0, $request); + + $this->assertFalse( + $changeTracker->isDirty($bookingDto), + 'Viewing a participant must not mark an unchanged booking as dirty' + ); + $this->assertSame( + $applicantInsurance, + $bookingDto->participants[0]->insurance, + "Applicant's insurance must not be re-tiered on a page view" + ); + $this->assertSame( + $dependentInsurance, + $bookingDto->participants[1]->insurance, + "Dependent's BusPro-loaded insurance must survive a page view" + ); + } + private function createBookingDto(): BookingDto { $travel = new Travel(); @@ -274,6 +359,36 @@ class ParticipantControllerTest extends TestCase return $bookingDto; } + private function createFamilyInsuranceBookingDto(): BookingDto + { + $bookingDto = $this->createBookingDto(); + + $familyInsurance = new Insurance(); + $familyInsurance->id = 'family-1'; + $familyInsurance->label = 'Familienversicherung'; + $familyInsurance->price = 120.0; + $familyInsurance->subType = 'RRV'; + $familyInsurance->familyInsurance = true; + + // BusPro can map one policy to several participants via , so a dependent + // may legitimately load holding it too. + $dependentInsurance = new Insurance(); + $dependentInsurance->id = 'dependent-own'; + $dependentInsurance->label = 'Reise-Rucktritt'; + $dependentInsurance->price = 50.0; + $dependentInsurance->subType = 'RRV'; + $dependentInsurance->familyInsurance = false; + + $bookingDto->participants[0]->insurance = $familyInsurance; + + $dependent = new ParticipantDto(); + $dependent->index = 1; + $dependent->insurance = $dependentInsurance; + $bookingDto->participants[] = $dependent; + + return $bookingDto; + } + private function createBooking(): Booking { $booking = new Booking(); @@ -320,7 +435,6 @@ final class TestableParticipantController extends ParticipantController BookingSessionManager $bookingSessionService, ParticipantDataPrefiller $prepopulationService, ParticipantFormSupport $participantFormSupportService, - ApplicantInsuranceCascade $applicantInsuranceCascade, private readonly User $user, FormInterface $form, ) { @@ -334,7 +448,6 @@ final class TestableParticipantController extends ParticipantController $bookingSessionService, $prepopulationService, $participantFormSupportService, - $applicantInsuranceCascade, new NullLogger(), ); }