fix: stop mutating booking state on edit-mode page views

This commit is contained in:
Björn Fromme
2026-08-24 17:13:19 +02:00
parent c9c28792e5
commit 578a21d327
2 changed files with 118 additions and 9 deletions
@@ -10,7 +10,6 @@ use App\Entity\User;
use App\Form\BookingParticipantType; use App\Form\BookingParticipantType;
use App\Form\Model\BookingDto; use App\Form\Model\BookingDto;
use App\Htmx\HxTrait; use App\Htmx\HxTrait;
use App\Service\ApplicantInsuranceCascade;
use App\Service\BookingConfigurator; use App\Service\BookingConfigurator;
use App\Service\BookingEditContextFactory; use App\Service\BookingEditContextFactory;
use App\Service\BookingEditDataLoader; use App\Service\BookingEditDataLoader;
@@ -41,7 +40,6 @@ class ParticipantController extends AbstractController
private readonly BookingSessionManager $bookingSessionService, private readonly BookingSessionManager $bookingSessionService,
private readonly ParticipantDataPrefiller $prepopulationService, private readonly ParticipantDataPrefiller $prepopulationService,
private readonly ParticipantFormSupport $participantFormSupportService, private readonly ParticipantFormSupport $participantFormSupportService,
private readonly ApplicantInsuranceCascade $applicantInsuranceCascade,
private readonly LoggerInterface $logger, private readonly LoggerInterface $logger,
) { ) {
} }
@@ -87,7 +85,6 @@ class ParticipantController extends AbstractController
} }
$this->editContextFactory->prepareBookingDto($bookingDto); $this->editContextFactory->prepareBookingDto($bookingDto);
$this->applicantInsuranceCascade->apply($bookingDto);
$form = $this->createParticipantForm($bookingDto, $index); $form = $this->createParticipantForm($bookingDto, $index);
@@ -159,7 +156,6 @@ class ParticipantController extends AbstractController
$form->handleRequest($request); $form->handleRequest($request);
$this->bookingConfigurator->preselectDefaultServices($bookingDto); $this->bookingConfigurator->preselectDefaultServices($bookingDto);
$this->applicantInsuranceCascade->apply($bookingDto);
$this->bookingSessionService->saveBookingDto($request, $bookingDto, BookingDto::MODE_EDIT); $this->bookingSessionService->saveBookingDto($request, $bookingDto, BookingDto::MODE_EDIT);
$form = $this->createForm( $form = $this->createForm(
@@ -6,6 +6,7 @@ namespace App\Tests\Controller\Booking\Edit;
use App\BusProNet\ApiClient; use App\BusProNet\ApiClient;
use App\BusProNet\Model\Booking; use App\BusProNet\Model\Booking;
use App\BusProNet\Model\Insurance;
use App\BusProNet\Model\Travel; use App\BusProNet\Model\Travel;
use App\Controller\Booking\Edit\ParticipantController; use App\Controller\Booking\Edit\ParticipantController;
use App\Entity\User; use App\Entity\User;
@@ -16,7 +17,7 @@ use App\Form\Model\BookingSummaryDto;
use App\Form\Model\ParticipantDto; use App\Form\Model\ParticipantDto;
use App\Form\Model\ParticipantEditDto; use App\Form\Model\ParticipantEditDto;
use App\Security\Crypt; use App\Security\Crypt;
use App\Service\ApplicantInsuranceCascade; use App\Service\BookingChangeTracker;
use App\Service\BookingConfigurator; use App\Service\BookingConfigurator;
use App\Service\BookingEditContextFactory; use App\Service\BookingEditContextFactory;
use App\Service\BookingEditDataLoader; use App\Service\BookingEditDataLoader;
@@ -134,7 +135,6 @@ class ParticipantControllerTest extends TestCase
$bookingSessionService, $bookingSessionService,
$prepopulationService, $prepopulationService,
$participantFormSupportService, $participantFormSupportService,
$this->createMock(ApplicantInsuranceCascade::class),
$user, $user,
$form, $form,
); );
@@ -243,7 +243,6 @@ class ParticipantControllerTest extends TestCase
$bookingSessionService, $bookingSessionService,
$prepopulationService, $prepopulationService,
$participantFormSupportService, $participantFormSupportService,
$this->createMock(ApplicantInsuranceCascade::class),
$user, $user,
$form, $form,
); );
@@ -260,6 +259,92 @@ class ParticipantControllerTest extends TestCase
return new User('[email protected]'); return new User('[email protected]');
} }
/**
* 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 private function createBookingDto(): BookingDto
{ {
$travel = new Travel(); $travel = new Travel();
@@ -274,6 +359,36 @@ class ParticipantControllerTest extends TestCase
return $bookingDto; 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 <zuordnung>, 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 private function createBooking(): Booking
{ {
$booking = new Booking(); $booking = new Booking();
@@ -320,7 +435,6 @@ final class TestableParticipantController extends ParticipantController
BookingSessionManager $bookingSessionService, BookingSessionManager $bookingSessionService,
ParticipantDataPrefiller $prepopulationService, ParticipantDataPrefiller $prepopulationService,
ParticipantFormSupport $participantFormSupportService, ParticipantFormSupport $participantFormSupportService,
ApplicantInsuranceCascade $applicantInsuranceCascade,
private readonly User $user, private readonly User $user,
FormInterface $form, FormInterface $form,
) { ) {
@@ -334,7 +448,6 @@ final class TestableParticipantController extends ParticipantController
$bookingSessionService, $bookingSessionService,
$prepopulationService, $prepopulationService,
$participantFormSupportService, $participantFormSupportService,
$applicantInsuranceCascade,
new NullLogger(), new NullLogger(),
); );
} }