chore: further cleanup and refactor

This commit is contained in:
Björn Fromme
2026-04-19 15:45:02 +02:00
parent 5a0bf11c81
commit 80969d1da0
21 changed files with 606 additions and 321 deletions
@@ -0,0 +1,257 @@
<?php
declare(strict_types=1);
namespace App\Tests\Controller\Booking\Edit;
use App\BusProNet\Model\Booking;
use App\BusProNet\Model\Travel;
use App\Controller\Booking\Edit\IndexController;
use App\Entity\User;
use App\Form\Model\BookingDto;
use App\Model\BookingEditSubmissionResult;
use App\Service\BookingChangeTracker;
use App\Service\BookingEditContextFactory;
use App\Service\BookingEditDataLoader;
use App\Service\BookingEditDraftManager;
use App\Service\BookingEditSubmitter;
use App\Service\BookingSessionManager;
use PHPUnit\Framework\TestCase;
use Symfony\Component\Form\FormInterface;
use Symfony\Component\HttpFoundation\RedirectResponse;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\Security\Core\User\UserInterface;
class IndexControllerTest extends TestCase
{
public function testIndexAppliesSubmissionResultFlashesAndRedirectsBackToEditPage(): void
{
$this->assertSubmissionResultFlashes(
new BookingEditSubmissionResult(
BookingEditSubmissionResult::STATUS_SUCCESS,
null,
true
),
[
['info', 'Einige Änderungen wurden verworfen, da sie aktuell nicht mehr änderbar sind.'],
['success', 'Buchung erfolgreich aktualisiert'],
],
);
}
/**
* @dataProvider submissionResultProvider
*
* @param array<int, array{0: string, 1: string}> $expectedFlashes
* @phpstan-param array<int, array{0: string, 1: string}> $expectedFlashes
*/
public function testIndexAppliesErrorSubmissionResultFlashesAndRedirectsBackToEditPage(
BookingEditSubmissionResult $submissionResult,
array $expectedFlashes,
): void {
$this->assertSubmissionResultFlashes($submissionResult, $expectedFlashes);
}
/**
* @return array<string, array{0: BookingEditSubmissionResult, 1: array<int, array{0: string, 1: string}>}>
*/
public static function submissionResultProvider(): array
{
return [
'notification error' => [
new BookingEditSubmissionResult(
BookingEditSubmissionResult::STATUS_NOTIFICATION_ERROR,
'BPN-FAIL',
false
),
[
['error', 'BPN-FAIL'],
],
],
'timeout' => [
new BookingEditSubmissionResult(
BookingEditSubmissionResult::STATUS_TIMEOUT,
null,
false
),
[
['error', 'Die Anfrage hat zu lange gedauert. Bitte versuche es erneut.'],
],
],
'reload failed' => [
new BookingEditSubmissionResult(
BookingEditSubmissionResult::STATUS_BOOKING_DATA_RELOAD_FAILED,
null,
false
),
[
['error', 'Buchungsdaten konnten vor dem Speichern nicht neu geladen werden'],
],
],
];
}
private function createUser(): User
{
return new User('[email protected]');
}
private function createBookingDto(): BookingDto
{
$travel = new Travel();
$travel->id = 1234;
$bookingDto = new BookingDto($travel, 77);
$bookingDto->booking = new Booking();
$bookingDto->booking->id = 42;
return $bookingDto;
}
private function createBooking(): Booking
{
$booking = new Booking();
$booking->id = 42;
$booking->dateId = 1234;
return $booking;
}
/**
* @param array<int, array{0: string, 1: string}> $expectedFlashes
*/
private function assertSubmissionResultFlashes(
BookingEditSubmissionResult $submissionResult,
array $expectedFlashes,
): void {
$request = Request::create('/bookings/42/edit', 'POST');
$user = $this->createUser();
$bookingDto = $this->createBookingDto();
$bookingData = $this->createBooking();
$form = $this->createMock(FormInterface::class);
$form->expects($this->once())
->method('handleRequest')
->with($request)
->willReturnSelf();
$form->expects($this->once())
->method('isSubmitted')
->willReturn(true);
$form->expects($this->once())
->method('isValid')
->willReturn(true);
$dataLoader = $this->createMock(BookingEditDataLoader::class);
$dataLoader->expects($this->once())
->method('loadFormData')
->with($request, 42, $user)
->willReturn($bookingDto);
$dataLoader->expects($this->once())
->method('fetchBookingData')
->with(42, $user)
->willReturn($bookingData);
$dataLoader->expects($this->once())
->method('isDraftRestored');
$submitter = $this->createMock(BookingEditSubmitter::class);
$submitter->expects($this->once())
->method('handleSubmission')
->with($request, $bookingDto, 42, $user)
->willReturn($submissionResult);
$controller = new TestableIndexController(
$dataLoader,
$this->createMock(BookingEditDraftManager::class),
$this->createMock(BookingSessionManager::class),
$this->createMock(BookingChangeTracker::class),
$this->createMock(BookingEditContextFactory::class),
$submitter,
$user,
$form,
);
$response = $controller->index(42, $request);
$this->assertInstanceOf(RedirectResponse::class, $response);
$this->assertSame('/bookings/42/edit', $response->headers->get('Location'));
$this->assertSame($expectedFlashes, $controller->flashes);
}
}
final class TestableIndexController extends IndexController
{
/**
* @var array<int, array{string, mixed}>
*/
public array $flashes = [];
/**
* @var FormInterface<mixed>
*/
private readonly FormInterface $form;
/**
* @param FormInterface<mixed> $form
*/
public function __construct(
BookingEditDataLoader $dataLoader,
BookingEditDraftManager $draftService,
BookingSessionManager $bookingSessionService,
BookingChangeTracker $fingerprintService,
BookingEditContextFactory $editContextFactory,
BookingEditSubmitter $formSubmitter,
private readonly User $user,
FormInterface $form,
) {
$this->form = $form;
parent::__construct(
$dataLoader,
$draftService,
$bookingSessionService,
$fingerprintService,
$editContextFactory,
$formSubmitter,
);
}
protected function getUser(): UserInterface
{
return $this->user;
}
/**
* @return FormInterface<mixed>
*/
protected function createForm(string $type, mixed $data = null, array $options = []): FormInterface
{
return $this->form;
}
protected function addFlash(string $type, mixed $message): void
{
$this->flashes[] = [$type, $message];
}
/**
* @param array<string, mixed> $parameters
*/
protected function redirectToRoute(string $route, array $parameters = [], int $status = 302): RedirectResponse
{
$location = match ($route) {
'app_booking_edit' => '/bookings/'.$parameters['bookingId'].'/edit',
default => '/'.$route,
};
return new RedirectResponse($location, $status);
}
/**
* @param array<string, mixed> $parameters
*/
protected function render(string $view, array $parameters = [], ?Response $response = null): Response
{
throw new \LogicException('Render should not be called in this test.');
}
}
+47 -49
View File
@@ -12,6 +12,7 @@ use App\BusProNet\Model\Notification;
use App\BusProNet\Model\Travel;
use App\Entity\User;
use App\Form\Model\BookingDto;
use App\Model\BookingEditSubmissionResult;
use App\Service\BookingEditDataLoader;
use App\Service\BookingEditDraftManager;
use App\Service\BookingEditSubmitGuard;
@@ -23,11 +24,10 @@ use Psr\Log\LoggerInterface;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Session\Session;
use Symfony\Component\HttpFoundation\Session\Storage\MockArraySessionStorage;
use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
class BookingEditSubmitterTest extends TestCase
{
public function testHandleSubmissionReturnsRedirectWhenFreshBookingDataCannotBeLoaded(): void
public function testHandleSubmissionReturnsResultWhenFreshBookingDataCannotBeLoaded(): void
{
$request = $this->createRequestWithSession();
$user = $this->createUser();
@@ -46,22 +46,18 @@ class BookingEditSubmitterTest extends TestCase
dataLoader: $dataLoader,
);
$response = $service->handleSubmission($request, $bookingDto, 42, $user);
$result = $service->handleSubmission($request, $bookingDto, 42, $user);
$this->assertSame('/bookings/42/edit', $response->headers->get('Location'));
$this->assertSame(
['Buchungsdaten konnten vor dem Speichern nicht neu geladen werden'],
$request->getSession()->getFlashBag()->get('error')
);
$this->assertSame(BookingEditSubmissionResult::STATUS_BOOKING_DATA_RELOAD_FAILED, $result->status);
$this->assertFalse($result->immutableChangesReverted);
}
/**
* @dataProvider updateFailureProvider
*/
public function testHandleSubmissionReturnsRedirectWhenUpdateThrows(
public function testHandleSubmissionReturnsResultWhenUpdateThrows(
\Throwable $exception,
string $expectedFlashType,
string $expectedFlashMessage,
string $expectedStatus,
): void {
$request = $this->createRequestWithSession();
$user = $this->createUser();
@@ -104,13 +100,14 @@ class BookingEditSubmitterTest extends TestCase
submitGuard: $submitGuard,
);
$response = $service->handleSubmission($request, $bookingDto, 42, $user);
$result = $service->handleSubmission($request, $bookingDto, 42, $user);
$this->assertSame('/bookings/42/edit', $response->headers->get('Location'));
$this->assertSame([$expectedFlashMessage], $request->getSession()->getFlashBag()->get($expectedFlashType));
$this->assertSame($expectedStatus, $result->status);
$this->assertNull($result->message);
$this->assertFalse($result->immutableChangesReverted);
}
public function testHandleSubmissionReturnsRedirectWhenUpdateIsUnsuccessful(): void
public function testHandleSubmissionReturnsResultWhenUpdateIsUnsuccessful(): void
{
$request = $this->createRequestWithSession();
$user = $this->createUser();
@@ -156,20 +153,21 @@ class BookingEditSubmitterTest extends TestCase
submitGuard: $submitGuard,
);
$response = $service->handleSubmission($request, $bookingDto, 42, $user);
$result = $service->handleSubmission($request, $bookingDto, 42, $user);
$this->assertSame('/bookings/42/edit', $response->headers->get('Location'));
$this->assertSame(['BPN-FAIL'], $request->getSession()->getFlashBag()->get('error'));
$this->assertSame(BookingEditSubmissionResult::STATUS_UNSUCCESSFUL, $result->status);
$this->assertSame('BPN-FAIL', $result->message);
$this->assertFalse($result->immutableChangesReverted);
}
public function testHandleSubmissionStoresInfoForNonErrorNotification(): void
{
$this->assertNotificationFlash(new Notification(650, 'Alles gut'), 'info');
$this->assertNotificationResult(new Notification(650, 'Alles gut'), 'info');
}
public function testHandleSubmissionStoresErrorForErrorNotification(): void
{
$this->assertNotificationFlash(new Notification(500, 'Kaputt'), 'error');
$this->assertNotificationResult(new Notification(500, 'Kaputt'), 'error');
}
public function testHandleSubmissionClearsSessionAndDraftOnSuccessfulUpdate(): void
@@ -231,10 +229,10 @@ class BookingEditSubmitterTest extends TestCase
bookingSessionService: $bookingSessionService,
);
$response = $service->handleSubmission($request, $bookingDto, 42, $user);
$result = $service->handleSubmission($request, $bookingDto, 42, $user);
$this->assertSame('/bookings/42/edit', $response->headers->get('Location'));
$this->assertSame(['Buchung erfolgreich aktualisiert'], $request->getSession()->getFlashBag()->get('success'));
$this->assertSame(BookingEditSubmissionResult::STATUS_SUCCESS, $result->status);
$this->assertFalse($result->immutableChangesReverted);
$this->assertSame($freshBookingData, $bookingDto->booking);
}
@@ -298,32 +296,31 @@ class BookingEditSubmitterTest extends TestCase
bookingSessionService: $bookingSessionService,
);
$response = $service->handleSubmission($request, $bookingDto, 42, $user);
$result = $service->handleSubmission($request, $bookingDto, 42, $user);
$this->assertSame('/bookings/42/edit', $response->headers->get('Location'));
$this->assertSame(['Einige Änderungen wurden verworfen, da sie aktuell nicht mehr änderbar sind.'], $request->getSession()->getFlashBag()->get('info'));
$this->assertSame(['Buchung erfolgreich aktualisiert'], $request->getSession()->getFlashBag()->get('success'));
$this->assertSame(BookingEditSubmissionResult::STATUS_SUCCESS, $result->status);
$this->assertTrue($result->immutableChangesReverted);
}
public function testHandleSubmissionReturnsRedirectOnTimeout(): void
public function testHandleSubmissionReturnsResultOnTimeout(): void
{
$this->assertExceptionFlash(new TimeoutException('slow'), 'Die Anfrage hat zu lange gedauert. Bitte versuchen Sie es erneut.');
$this->assertExceptionResult(new TimeoutException('slow'), BookingEditSubmissionResult::STATUS_TIMEOUT);
}
public function testHandleSubmissionReturnsRedirectOnApiClientException(): void
public function testHandleSubmissionReturnsResultOnApiClientException(): void
{
$this->assertExceptionFlash(new ApiClientException('boom'), 'Es ist ein Fehler in der Kommunikation mit dem Buchungssystem aufgetreten');
$this->assertExceptionResult(new ApiClientException('boom'), BookingEditSubmissionResult::STATUS_API_CLIENT_ERROR);
}
public static function updateFailureProvider(): array
{
return [
'timeout' => [new TimeoutException('slow'), 'error', 'Die Anfrage hat zu lange gedauert. Bitte versuchen Sie es erneut.'],
'api-client' => [new ApiClientException('boom'), 'error', 'Es ist ein Fehler in der Kommunikation mit dem Buchungssystem aufgetreten'],
'timeout' => [new TimeoutException('slow'), BookingEditSubmissionResult::STATUS_TIMEOUT],
'api-client' => [new ApiClientException('boom'), BookingEditSubmissionResult::STATUS_API_CLIENT_ERROR],
];
}
private function assertNotificationFlash(Notification $notification, string $expectedType): void
private function assertNotificationResult(Notification $notification, string $expectedType): void
{
$request = $this->createRequestWithSession();
$user = $this->createUser();
@@ -366,13 +363,14 @@ class BookingEditSubmitterTest extends TestCase
submitGuard: $submitGuard,
);
$response = $service->handleSubmission($request, $bookingDto, 42, $user);
$result = $service->handleSubmission($request, $bookingDto, 42, $user);
$this->assertSame('/bookings/42/edit', $response->headers->get('Location'));
$this->assertSame([$notification->message], $request->getSession()->getFlashBag()->get($expectedType));
$this->assertSame($this->resolveExpectedStatus($expectedType), $result->status);
$this->assertSame($notification->message, $result->message);
$this->assertFalse($result->immutableChangesReverted);
}
private function assertExceptionFlash(\Throwable $exception, string $expectedMessage): void
private function assertExceptionResult(\Throwable $exception, string $expectedStatus): void
{
$request = $this->createRequestWithSession();
$user = $this->createUser();
@@ -415,10 +413,11 @@ class BookingEditSubmitterTest extends TestCase
submitGuard: $submitGuard,
);
$response = $service->handleSubmission($request, $bookingDto, 42, $user);
$result = $service->handleSubmission($request, $bookingDto, 42, $user);
$this->assertSame('/bookings/42/edit', $response->headers->get('Location'));
$this->assertSame([$expectedMessage], $request->getSession()->getFlashBag()->get('error'));
$this->assertSame($expectedStatus, $result->status);
$this->assertNull($result->message);
$this->assertFalse($result->immutableChangesReverted);
}
private function createService(
@@ -436,7 +435,6 @@ class BookingEditSubmitterTest extends TestCase
$travelDataService ?? $this->createMock(TravelDataProvider::class),
$submitGuard ?? $this->createMock(BookingEditSubmitGuard::class),
$bookingSessionService ?? $this->createMock(BookingSessionManager::class),
$this->createUrlGenerator(),
$this->createMock(LoggerInterface::class),
);
}
@@ -470,13 +468,13 @@ class BookingEditSubmitterTest extends TestCase
return $request;
}
private function createUrlGenerator(): UrlGeneratorInterface
private function resolveExpectedStatus(string $expectedType): string
{
$urlGenerator = $this->createMock(UrlGeneratorInterface::class);
$urlGenerator->method('generate')
->with('app_booking_edit', ['id' => 42])
->willReturn('/bookings/42/edit');
return $urlGenerator;
return match ($expectedType) {
'error' => BookingEditSubmissionResult::STATUS_NOTIFICATION_ERROR,
'info' => BookingEditSubmissionResult::STATUS_NOTIFICATION_INFO,
default => $expectedType,
};
}
}