fix: false draft-restored message
This commit is contained in:
@@ -10,6 +10,7 @@ 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\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;
|
||||||
@@ -37,6 +38,7 @@ class ParticipantController extends AbstractController
|
|||||||
private readonly BookingEditDraftManager $draftService,
|
private readonly BookingEditDraftManager $draftService,
|
||||||
private readonly BookingEditContextFactory $editContextFactory,
|
private readonly BookingEditContextFactory $editContextFactory,
|
||||||
private readonly BookingConfigurator $bookingConfigurator,
|
private readonly BookingConfigurator $bookingConfigurator,
|
||||||
|
private readonly BookingChangeTracker $changeTracker,
|
||||||
private readonly BookingSessionManager $bookingSessionService,
|
private readonly BookingSessionManager $bookingSessionService,
|
||||||
private readonly ParticipantDataPrefiller $prepopulationService,
|
private readonly ParticipantDataPrefiller $prepopulationService,
|
||||||
private readonly ParticipantFormSupport $participantFormSupportService,
|
private readonly ParticipantFormSupport $participantFormSupportService,
|
||||||
@@ -99,7 +101,7 @@ class ParticipantController extends AbstractController
|
|||||||
$this->prepopulationService->fillDummyParticipant($bookingDto->participants[$index], $index);
|
$this->prepopulationService->fillDummyParticipant($bookingDto->participants[$index], $index);
|
||||||
|
|
||||||
$this->bookingSessionService->saveBookingDto($request, $bookingDto, BookingDto::MODE_EDIT);
|
$this->bookingSessionService->saveBookingDto($request, $bookingDto, BookingDto::MODE_EDIT);
|
||||||
$this->draftService->saveDraft($user, $bookingId, $bookingDto);
|
$this->persistDraftState($user, $bookingId, $bookingDto);
|
||||||
|
|
||||||
$form = $this->createParticipantForm($bookingDto, $index);
|
$form = $this->createParticipantForm($bookingDto, $index);
|
||||||
|
|
||||||
@@ -110,7 +112,7 @@ class ParticipantController extends AbstractController
|
|||||||
|
|
||||||
if ($form->isSubmitted() && $form->isValid()) {
|
if ($form->isSubmitted() && $form->isValid()) {
|
||||||
$this->bookingSessionService->saveBookingDto($request, $bookingDto, BookingDto::MODE_EDIT);
|
$this->bookingSessionService->saveBookingDto($request, $bookingDto, BookingDto::MODE_EDIT);
|
||||||
$this->draftService->saveDraft($user, $bookingId, $bookingDto);
|
$this->persistDraftState($user, $bookingId, $bookingDto);
|
||||||
$this->addNotificationsAsFlashMessages($notifications);
|
$this->addNotificationsAsFlashMessages($notifications);
|
||||||
|
|
||||||
return $this->redirectToRoute('app_booking_edit', ['bookingId' => $bookingId]);
|
return $this->redirectToRoute('app_booking_edit', ['bookingId' => $bookingId]);
|
||||||
@@ -196,6 +198,24 @@ class ParticipantController extends AbstractController
|
|||||||
return $response;
|
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<array{type: string, message: string}> $notifications
|
* @param array<array{type: string, message: string}> $notifications
|
||||||
*/
|
*/
|
||||||
|
|||||||
@@ -172,9 +172,11 @@ class BookingEditDataLoader
|
|||||||
$draft = $this->draftService->findDraft($user, $bookingId);
|
$draft = $this->draftService->findDraft($user, $bookingId);
|
||||||
if (null !== $draft) {
|
if (null !== $draft) {
|
||||||
$applied = $this->draftService->applyDraftToDto($draft, $formData, $travelData);
|
$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);
|
$this->bookingSessionService->saveBookingDto($request, $formData, BookingDto::MODE_EDIT);
|
||||||
|
|||||||
@@ -52,6 +52,7 @@ class ParticipantControllerTest extends TestCase
|
|||||||
$bookingDto = $this->createBookingDto();
|
$bookingDto = $this->createBookingDto();
|
||||||
$bookingData = $this->createBooking();
|
$bookingData = $this->createBooking();
|
||||||
$participant = $bookingDto->participants[0];
|
$participant = $bookingDto->participants[0];
|
||||||
|
$bookingDto->originalFingerprint = (new BookingChangeTracker())->generateFingerprint($bookingDto);
|
||||||
$participant->lastName = ParticipantDataPrefiller::TOKEN;
|
$participant->lastName = ParticipantDataPrefiller::TOKEN;
|
||||||
|
|
||||||
$form = $this->createMock(FormInterface::class);
|
$form = $this->createMock(FormInterface::class);
|
||||||
@@ -254,6 +255,96 @@ class ParticipantControllerTest extends TestCase
|
|||||||
$this->assertSame(['participant_form', 'booking_summary'], $controller->renderedBlocks);
|
$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
|
private function createUser(): User
|
||||||
{
|
{
|
||||||
return new User('[email protected]');
|
return new User('[email protected]');
|
||||||
@@ -445,6 +536,7 @@ final class TestableParticipantController extends ParticipantController
|
|||||||
$draftService,
|
$draftService,
|
||||||
$editContextFactory,
|
$editContextFactory,
|
||||||
$bookingService,
|
$bookingService,
|
||||||
|
new BookingChangeTracker(),
|
||||||
$bookingSessionService,
|
$bookingSessionService,
|
||||||
$prepopulationService,
|
$prepopulationService,
|
||||||
$participantFormSupportService,
|
$participantFormSupportService,
|
||||||
|
|||||||
@@ -0,0 +1,125 @@
|
|||||||
|
<?php
|
||||||
|
|
||||||
|
declare(strict_types=1);
|
||||||
|
|
||||||
|
namespace App\Tests\Service;
|
||||||
|
|
||||||
|
use App\BusProNet\ApiClient;
|
||||||
|
use App\BusProNet\DataProcessor\BookingDataProcessor;
|
||||||
|
use App\BusProNet\Model\BaseData;
|
||||||
|
use App\BusProNet\Model\Booking;
|
||||||
|
use App\BusProNet\Model\ServiceAvailabilityResponse;
|
||||||
|
use App\BusProNet\Model\Travel;
|
||||||
|
use App\BusProNet\XmlLoader\AgencyLoader;
|
||||||
|
use App\Entity\BookingEditDraft;
|
||||||
|
use App\Entity\User;
|
||||||
|
use App\Form\Model\BookingDto;
|
||||||
|
use App\Form\Model\ParticipantDto;
|
||||||
|
use App\Security\Crypt;
|
||||||
|
use App\Service\BookingChangeTracker;
|
||||||
|
use App\Service\BookingEditDataLoader;
|
||||||
|
use App\Service\BookingEditDraftManager;
|
||||||
|
use App\Service\BookingSessionManager;
|
||||||
|
use App\Service\TravelDataProvider;
|
||||||
|
use PHPUnit\Framework\TestCase;
|
||||||
|
use Symfony\Component\HttpFoundation\Request;
|
||||||
|
use Symfony\Contracts\Cache\TagAwareCacheInterface;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Covers the "draft restored" signal that drives the overview flash message.
|
||||||
|
*
|
||||||
|
* A draft is written on every participant save, including saves that changed nothing.
|
||||||
|
* Such a draft reproduces the API state exactly, so applying it is a no-op and must not
|
||||||
|
* be reported as a restore — otherwise the user is told their draft was restored for
|
||||||
|
* edits they never made.
|
||||||
|
*/
|
||||||
|
class BookingEditDataLoaderDraftRestoreTest extends TestCase
|
||||||
|
{
|
||||||
|
public function testNoOpDraftIsNotReportedAsRestored(): void
|
||||||
|
{
|
||||||
|
$loader = $this->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('[email protected]');
|
||||||
|
$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,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user