diff --git a/src/Controller/Booking/Edit/ParticipantController.php b/src/Controller/Booking/Edit/ParticipantController.php index 8b690a5..82cf967 100644 --- a/src/Controller/Booking/Edit/ParticipantController.php +++ b/src/Controller/Booking/Edit/ParticipantController.php @@ -10,6 +10,7 @@ use App\Entity\User; use App\Form\BookingParticipantType; use App\Form\Model\BookingDto; use App\Htmx\HxTrait; +use App\Service\BookingChangeTracker; use App\Service\BookingConfigurator; use App\Service\BookingEditContextFactory; use App\Service\BookingEditDataLoader; @@ -37,6 +38,7 @@ class ParticipantController extends AbstractController private readonly BookingEditDraftManager $draftService, private readonly BookingEditContextFactory $editContextFactory, private readonly BookingConfigurator $bookingConfigurator, + private readonly BookingChangeTracker $changeTracker, private readonly BookingSessionManager $bookingSessionService, private readonly ParticipantDataPrefiller $prepopulationService, private readonly ParticipantFormSupport $participantFormSupportService, @@ -99,7 +101,7 @@ class ParticipantController extends AbstractController $this->prepopulationService->fillDummyParticipant($bookingDto->participants[$index], $index); $this->bookingSessionService->saveBookingDto($request, $bookingDto, BookingDto::MODE_EDIT); - $this->draftService->saveDraft($user, $bookingId, $bookingDto); + $this->persistDraftState($user, $bookingId, $bookingDto); $form = $this->createParticipantForm($bookingDto, $index); @@ -110,7 +112,7 @@ class ParticipantController extends AbstractController if ($form->isSubmitted() && $form->isValid()) { $this->bookingSessionService->saveBookingDto($request, $bookingDto, BookingDto::MODE_EDIT); - $this->draftService->saveDraft($user, $bookingId, $bookingDto); + $this->persistDraftState($user, $bookingId, $bookingDto); $this->addNotificationsAsFlashMessages($notifications); return $this->redirectToRoute('app_booking_edit', ['bookingId' => $bookingId]); @@ -196,6 +198,24 @@ class ParticipantController extends AbstractController return $response; } + /** + * Persists the draft only while there is something to draft. + * + * Saving a draft that reproduces the API state exactly would make the next + * overview load report a restored draft for edits the user never made. + * Such a draft is obsolete, so it is removed instead. + */ + private function persistDraftState(User $user, int $bookingId, BookingDto $bookingDto): void + { + if (true === $this->changeTracker->isDirty($bookingDto)) { + $this->draftService->saveDraft($user, $bookingId, $bookingDto); + + return; + } + + $this->draftService->deleteDraft($user, $bookingId); + } + /** * @param array $notifications */ diff --git a/src/Service/BookingEditDataLoader.php b/src/Service/BookingEditDataLoader.php index 41a3452..587f771 100644 --- a/src/Service/BookingEditDataLoader.php +++ b/src/Service/BookingEditDataLoader.php @@ -172,9 +172,11 @@ class BookingEditDataLoader $draft = $this->draftService->findDraft($user, $bookingId); if (null !== $draft) { $applied = $this->draftService->applyDraftToDto($draft, $formData, $travelData); - if (true === $applied) { - $this->draftRestored = true; - } + + // A draft that reproduces the API state exactly is not a restore. Reporting + // it would tell the user their draft was restored for edits they never made. + $this->draftRestored = true === $applied + && $formData->originalFingerprint !== $this->fingerprintService->generateFingerprint($formData); } $this->bookingSessionService->saveBookingDto($request, $formData, BookingDto::MODE_EDIT); diff --git a/tests/Controller/Booking/Edit/ParticipantControllerTest.php b/tests/Controller/Booking/Edit/ParticipantControllerTest.php index 89d9511..7ae29a5 100644 --- a/tests/Controller/Booking/Edit/ParticipantControllerTest.php +++ b/tests/Controller/Booking/Edit/ParticipantControllerTest.php @@ -52,6 +52,7 @@ class ParticipantControllerTest extends TestCase $bookingDto = $this->createBookingDto(); $bookingData = $this->createBooking(); $participant = $bookingDto->participants[0]; + $bookingDto->originalFingerprint = (new BookingChangeTracker())->generateFingerprint($bookingDto); $participant->lastName = ParticipantDataPrefiller::TOKEN; $form = $this->createMock(FormInterface::class); @@ -254,6 +255,96 @@ class ParticipantControllerTest extends TestCase $this->assertSame(['participant_form', 'booking_summary'], $controller->renderedBlocks); } + /** + * Regression: saving the participant form without touching anything used to write a + * draft identical to the API state. The overview then reloaded from the API, found + * that draft, applied it and told the user their draft had been restored — for edits + * they never made. + */ + public function testSubmitWithoutChangesDeletesDraftInsteadOfSavingIt(): void + { + $draftService = $this->createMock(BookingEditDraftManager::class); + $draftService->expects($this->never())->method('saveDraft'); + $draftService->expects($this->once())->method('deleteDraft')->with($this->isInstanceOf(User::class), 42); + + $this->runValidSubmit($draftService, dirty: false); + } + + public function testSubmitWithChangesPersistsDraft(): void + { + $draftService = $this->createMock(BookingEditDraftManager::class); + $draftService->expects($this->once())->method('saveDraft'); + $draftService->expects($this->never())->method('deleteDraft'); + + $this->runValidSubmit($draftService, dirty: true); + } + + /** + * Drives editParticipant() through a valid, non-dummy submit. + */ + private function runValidSubmit(BookingEditDraftManager $draftService, bool $dirty): void + { + $request = Request::create('/bookings/42/edit/participants/0', 'POST'); + $user = $this->createUser(); + $bookingDto = $this->createBookingDto(); + $bookingData = $this->createBooking(); + $participant = $bookingDto->participants[0]; + $participant->firstName = 'Anna'; + + $bookingDto->originalFingerprint = (new BookingChangeTracker())->generateFingerprint($bookingDto); + + if (true === $dirty) { + $participant->firstName = 'Berta'; + } + + $form = $this->createStub(FormInterface::class); + $form->method('handleRequest')->willReturnSelf(); + $form->method('isSubmitted')->willReturn(true); + $form->method('isValid')->willReturn(true); + + $dataLoader = $this->createStub(BookingEditDataLoader::class); + $dataLoader->method('fetchBookingData')->willReturn($bookingData); + + $bookingSessionService = $this->createMock(BookingSessionManager::class); + $bookingSessionService->method('getBookingDto')->willReturn($bookingDto); + $bookingSessionService->expects($this->once()) + ->method('saveBookingDto') + ->with($request, $bookingDto, BookingDto::MODE_EDIT); + + $participantFormSupportService = $this->createStub(ParticipantFormSupport::class); + $participantFormSupportService->method('ensureParticipantExists')->willReturn($participant); + $participantFormSupportService->method('createParticipantEditDto') + ->willReturn(new ParticipantEditDto($participant, $bookingDto)); + $participantFormSupportService->method('getParticipantFormOptions') + ->willReturn(['booking_context' => $bookingDto]); + $participantFormSupportService->method('collectAndClearNotifications')->willReturn([]); + + $contextFactory = $this->createStub(BookingEditContextFactory::class); + $contextFactory->method('createParticipantContext')->willReturn( + new BookingEditContext($bookingDto, $bookingData, null, $this->createStub(BookingSummaryDto::class)) + ); + + $controller = new TestableParticipantController( + $dataLoader, + $draftService, + $contextFactory, + $this->createStub(BookingConfigurator::class), + $bookingSessionService, + new ParticipantDataPrefiller( + $this->createStub(ApiClient::class), + $this->createStub(Crypt::class), + $this->createStub(LoggerInterface::class), + ), + $participantFormSupportService, + $user, + $form, + ); + + $response = $controller->editParticipant(42, 0, $request); + + $this->assertInstanceOf(RedirectResponse::class, $response); + } + private function createUser(): User { return new User('tester@example.com'); @@ -445,6 +536,7 @@ final class TestableParticipantController extends ParticipantController $draftService, $editContextFactory, $bookingService, + new BookingChangeTracker(), $bookingSessionService, $prepopulationService, $participantFormSupportService, diff --git a/tests/Service/BookingEditDataLoaderDraftRestoreTest.php b/tests/Service/BookingEditDataLoaderDraftRestoreTest.php new file mode 100644 index 0000000..dc5e111 --- /dev/null +++ b/tests/Service/BookingEditDataLoaderDraftRestoreTest.php @@ -0,0 +1,125 @@ +createLoader(static function (BookingDto $dto): void { + // Draft reproduces the API state: nothing changes. + }); + + $loader->initializeFromApi(new Request(), 42, $this->createUser()); + + $this->assertFalse($loader->isDraftRestored()); + } + + public function testDraftThatChangesDataIsReportedAsRestored(): void + { + $loader = $this->createLoader(static function (BookingDto $dto): void { + $dto->participants[0]->firstName = 'Berta'; + }); + + $loader->initializeFromApi(new Request(), 42, $this->createUser()); + + $this->assertTrue($loader->isDraftRestored()); + } + + private function createUser(): User + { + $user = new User('tester@example.com'); + $user->setPassword('encrypted'); + + return $user; + } + + /** + * @param callable(BookingDto): void $applyDraft what the stored draft does to the DTO + */ + private function createLoader(callable $applyDraft): BookingEditDataLoader + { + $booking = new Booking(); + $booking->id = 42; + $booking->dateId = 1234; + $booking->hotelId = 7; + + $travel = new Travel(); + $travel->id = 1234; + + $bookingDto = new BookingDto($travel, 77); + $bookingDto->booking = $booking; + $participant = new ParticipantDto(); + $participant->index = 0; + $participant->firstName = 'Anna'; + $bookingDto->participants = [$participant]; + + $cache = $this->createStub(TagAwareCacheInterface::class); + $cache->method('get')->willReturn($booking); + + $travelDataService = $this->createStub(TravelDataProvider::class); + $travelDataService->method('getTravelData')->willReturn($travel); + $travelDataService->method('getMutabilityData')->willReturn($this->createStub(BaseData::class)); + $travelDataService->method('getAvailabilityData') + ->willReturn($this->createStub(ServiceAvailabilityResponse::class)); + + $bookingDataProcessor = $this->createStub(BookingDataProcessor::class); + $bookingDataProcessor->method('createBookingDtoFromBooking')->willReturn($bookingDto); + + $draftService = $this->createStub(BookingEditDraftManager::class); + $draftService->method('findDraft')->willReturn($this->createStub(BookingEditDraft::class)); + $draftService->method('applyDraftToDto')->willReturnCallback( + static function (BookingEditDraft $draft, BookingDto $dto, Travel $travel) use ($applyDraft): bool { + $applyDraft($dto); + + return true; + } + ); + + $crypt = $this->createStub(Crypt::class); + $crypt->method('decrypt')->willReturn('secret'); + + return new BookingEditDataLoader( + $this->createStub(ApiClient::class), + $bookingDataProcessor, + $this->createStub(BookingSessionManager::class), + new BookingChangeTracker(), + $travelDataService, + $draftService, + $this->createStub(AgencyLoader::class), + $crypt, + $cache, + ); + } +}