EP/myep
4
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: ensure mutability is accounted for when restoring drafts

Browse Source 
addresses #869cdzmkq
This commit is contained in:
Björn Fromme
2026-03-16 11:20:05 +01:00
parent 021671a45f
commit efff024196
5 changed files with 364 additions and 22 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/Controller/Booking/Edit/IndexController.php
+4
View File
@@ -103,6 +103,10 @@ class IndexController extends AbstractController
// Show flash message if draft was restored
if (true === $this->dataLoader->wasDraftRestored()) {
$this->addFlash('info', 'Dein zuvor gespeicherter Entwurf wurde wiederhergestellt.');
if (true === $this->dataLoader->draftHadImmutableSkips()) {
$this->addFlash('info', 'Einige Entwurfsänderungen wurden nicht übernommen, da diese aktuell nicht mehr änderbar sind.');
}
}
// Fetch booking data for display (surcharges, canceled status, etc.)
src/Service/BookingEditDataLoaderService.php
+25 -4
View File
@@ -25,7 +25,7 @@ use Symfony\Contracts\Cache\TagAwareCacheInterface;
*/
class BookingEditDataLoaderService
{
public const CACHE_TAG_USER_PREFIX = 'user_bookings_';
public const string CACHE_TAG_USER_PREFIX = 'user_bookings_';
private bool $draftWasRestored = false;
@@ -53,6 +53,14 @@ class BookingEditDataLoaderService
return $this->draftWasRestored;
}
/**
* Returns whether restored draft data was partially skipped due to immutability.
*/
public function draftHadImmutableSkips(): bool
{
return $this->draftWasRestored && $this->draftService->hadImmutableSkips();
}
/**
* Loads booking data from session or initializes from API.
*
@@ -85,8 +93,18 @@ class BookingEditDataLoaderService
return $this->initializeFromApi($request, $bookingId, $user);
}
// Refresh availability data
// Refresh availability and mutability data to ensure current state
// Availability changes frequently (real-time bookings), so always refresh
// Mutability uses 12h cache (changes daily at cutoff), no force refresh needed
$this->travelDataService->enrichWithFreshAvailabilities($formData->travel);
$bookingData = $this->fetchBookingData($bookingId, $user);
if (null !== $bookingData && !($bookingData instanceof Notification)) {
$mutableData = $this->travelDataService->getMutabilityData($bookingData->dateId, cached: true);
if (null !== $mutableData) {
$this->travelDataService->patchMutability($formData->travel, $mutableData);
}
}
return $formData;
}
@@ -117,7 +135,10 @@ class BookingEditDataLoaderService
return null;
}
$mutableData = $this->travelDataService->getMutabilityData($bookingData->dateId);
// Use cached mutability data (12h TTL ensures daily cutoffs are respected).
// Cache is shared across all users editing this travel date, reducing API load.
// The 12h TTL is shorter than typical mutability cutoff windows (days before travel).
$mutableData = $this->travelDataService->getMutabilityData($bookingData->dateId, cached: true);
// Force refresh ensures fresh data at edit start, then populates cache for subsequent loads
$availabilities = $this->travelDataService->getAvailabilityData($bookingData->dateId, cached: true, forceRefresh: true);
@@ -143,7 +164,7 @@ class BookingEditDataLoaderService
$draft = $this->draftService->findDraft($user, $bookingId);
if (null !== $draft) {
$applied = $this->draftService->applyDraftToDto($draft, $formData, $travelData);
if ($applied) {
if (true === $applied) {
$this->draftWasRestored = true;
}
}
src/Service/BookingEditDraftService.php
+245 -8
View File
@@ -25,6 +25,28 @@ use Psr\Log\LoggerInterface;
*/
class BookingEditDraftService
{
private const array ADDITIONAL_SERVICE_KEYS = [
'skiPass',
'courses',
'board',
'rentals',
'rentalInsurance',
'additionalServices',
];
private const array TRANSPORTATION_KEYS = [
'transportationOutbound',
'transportationInbound',
'parking',
];
private const array PICKUP_KEYS = [
'pickup',
'dropOff',
];
private bool $draftHadImmutableSkips = false;
public function __construct(
private readonly BookingEditDraftRepository $draftRepository,
private readonly EntityManagerInterface $entityManager,
@@ -112,6 +134,14 @@ class BookingEditDraftService
]);
}
/**
* Returns whether the last draft application skipped fields due to immutability.
*/
public function hadImmutableSkips(): bool
{
return $this->draftHadImmutableSkips;
}
/**
* Applies draft data to a fresh BookingDto loaded from the API.
*
@@ -128,6 +158,8 @@ class BookingEditDraftService
*/
public function applyDraftToDto(BookingEditDraft $draft, BookingDto $dto, Travel $travel): bool
{
$this->draftHadImmutableSkips = false;
try {
$formData = $draft->getFormData();
@@ -148,7 +180,7 @@ class BookingEditDraftService
continue;
}
$this->applyParticipantData($dto->participants[$index], $participantData, $travel);
$this->applyParticipantData($dto, $index, $participantData, $travel);
}
}
@@ -191,25 +223,32 @@ class BookingEditDraftService
/**
* Applies participant data from draft to a ParticipantDto.
*/
private function applyParticipantData(ParticipantDto $participant, array $data, Travel $travel): void
private function applyParticipantData(BookingDto $bookingDto, int $participantIndex, array $data, Travel $travel): void
{
$participant = $bookingDto->participants[$participantIndex];
$canApplyPersonalDataDraft = $this->canApplyPersonalDataDraft($bookingDto, $participantIndex, $participant);
if (false === $canApplyPersonalDataDraft && $this->hasBlockedPersonalOrAddressChanges($participant, $data)) {
$this->draftHadImmutableSkips = true;
}
// Personal data
if (isset($data['personalData']) && true === is_array($data['personalData'])) {
if (true === $canApplyPersonalDataDraft && isset($data['personalData']) && true === is_array($data['personalData'])) {
$this->applyPersonalData($participant, $data['personalData']);
}
// Address
if (isset($data['address']) && true === is_array($data['address'])) {
if (true === $canApplyPersonalDataDraft && isset($data['address']) && true === is_array($data['address'])) {
$this->applyAddressData($participant, $data['address']);
}
// Body dimensions
if (isset($data['bodyDimensions']) && true === is_array($data['bodyDimensions'])) {
if (true === isset($data['bodyDimensions']) && true === is_array($data['bodyDimensions'])) {
$this->applyBodyDimensions($participant, $data['bodyDimensions']);
}
// Room assignment
if (isset($data['roomAssignment']) && true === is_array($data['roomAssignment'])) {
if (true === isset($data['roomAssignment']) && true === is_array($data['roomAssignment'])) {
$this->applyRoomAssignment($participant, $data['roomAssignment']);
}
@@ -219,16 +258,214 @@ class BookingEditDraftService
}
// Services
if (isset($data['services']) && true === is_array($data['services'])) {
if (true === isset($data['services']) && true === is_array($data['services'])) {
$this->markImmutableServiceSkips($data['services'], $participant, $travel);
$this->applyServiceSelections($participant, $data['services'], $travel);
}
// Vouchers
if (isset($data['vouchers']) && true === is_array($data['vouchers'])) {
if (true === isset($data['vouchers']) && true === is_array($data['vouchers'])) {
$this->applyVoucherCodes($participant, $data['vouchers']);
}
}
/**
* Determines whether personal/address draft data may be applied.
*
* Edit rules mirror UI mutability behavior:
* - Internal agency bookings may always update participant personal data
* - Non-internal agency: first participant is always read-only in edit flow
* - Other participants require participant-level mutability from BPN
*/
private function canApplyPersonalDataDraft(BookingDto $bookingDto, int $participantIndex, ParticipantDto $participant): bool
{
if (true === $bookingDto->isInternalAgencyBooking()) {
return true;
}
if (0 === $participantIndex) {
return false;
}
return $participant->mutable;
}
/**
* Checks whether draft payload contains personal or address data sections.
*/
private function hasBlockedPersonalOrAddressChanges(ParticipantDto $participant, array $data): bool
{
$personalData = $data['personalData'] ?? null;
$addressData = $data['address'] ?? null;
if (true === is_array($personalData)) {
if ($this->hasDifferences($personalData, $this->normalizePersonalData($participant), [
'firstName',
'lastName',
'dateOfBirth',
'email',
'mobile',
'gender',
'nationality',
])) {
return true;
}
}
if (true === is_array($addressData)) {
if ($this->hasDifferences($addressData, $this->normalizeAddressData($participant), [
'street',
'postCode',
'city',
'country',
])) {
return true;
}
}
return false;
}
/**
* Marks draft as partially skipped when immutable service categories had draft changes.
*/
private function markImmutableServiceSkips(array $servicesData, ParticipantDto $participant, Travel $travel): void
{
$currentServices = $this->normalizeServiceState($participant);
$this->markImmutableSkipForCategory(
false === $travel->additionalServicesMutable,
$servicesData,
$currentServices,
self::ADDITIONAL_SERVICE_KEYS
);
$this->markImmutableSkipForCategory(
false === $travel->transportationServicesMutable,
$servicesData,
$currentServices,
self::TRANSPORTATION_KEYS
);
$this->markImmutableSkipForCategory(
false === $travel->pickupsMutable,
$servicesData,
$currentServices,
self::PICKUP_KEYS
);
}
/**
* Marks immutable skip flag when category is locked and draft contains related keys.
*/
private function markImmutableSkipForCategory(bool $isImmutable, array $servicesData, array $currentServices, array $categoryKeys): void
{
if ($isImmutable && $this->hasDifferences($servicesData, $currentServices, $categoryKeys)) {
$this->draftHadImmutableSkips = true;
}
}
/**
* Normalizes participant personal data to draft payload shape.
*/
private function normalizePersonalData(ParticipantDto $participant): array
{
return [
'firstName' => $participant->firstName,
'lastName' => $participant->lastName,
'dateOfBirth' => $participant->dateOfBirth?->format('Y-m-d'),
'email' => $participant->email,
'mobile' => $participant->mobile,
'gender' => $participant->gender,
'nationality' => $participant->nationality,
];
}
/**
* Normalizes participant address data to draft payload shape.
*/
private function normalizeAddressData(ParticipantDto $participant): array
{
return [
'street' => $participant->address?->street,
'postCode' => $participant->address?->postCode,
'city' => $participant->address?->city,
'country' => $participant->address?->country,
];
}
/**
* Normalizes participant service state to draft payload shape.
*/
private function normalizeServiceState(ParticipantDto $participant): array
{
return [
'skiPass' => $participant->skiPass?->id,
'courses' => $this->normalizeServiceIds($participant->courses),
'board' => $this->normalizeServiceIds($participant->board),
'rentals' => $this->normalizeServiceIds($participant->rentals),
'rentalInsurance' => $participant->rentalInsurance?->id,
'additionalServices' => $this->normalizeServiceIds($participant->additionalServices),
'transportationOutbound' => $participant->transportationOutbound?->id,
'transportationInbound' => $participant->transportationInbound?->id,
'pickup' => $participant->pickup?->id,
'dropOff' => $participant->dropOff?->id,
'parking' => (bool) $participant->parking,
'insurance' => $participant->insurance?->id,
'bulkInsuranceBooking' => (bool) $participant->bulkInsuranceBooking,
];
}
/**
* Returns sorted unique IDs for service arrays.
*/
private function normalizeServiceIds(array $services): array
{
$ids = array_map(static fn ($service) => $service->id, $services);
$ids = array_values(array_unique($ids));
sort($ids);
return $ids;
}
/**
* Returns true if any of the given keys differ between draft and current values.
*/
private function hasDifferences(array $draftData, array $currentData, array $keys): bool
{
foreach ($keys as $key) {
if (false === array_key_exists($key, $draftData)) {
continue;
}
$draftValue = $this->normalizeComparableValue($draftData[$key]);
$currentValue = $this->normalizeComparableValue($currentData[$key] ?? null);
if ($draftValue !== $currentValue) {
return true;
}
}
return false;
}
/**
* Normalizes scalar/list values for strict comparisons.
*/
private function normalizeComparableValue(mixed $value): mixed
{
if (is_array($value)) {
$normalized = array_map(fn ($item) => $this->normalizeComparableValue($item), array_values($value));
sort($normalized);
return $normalized;
}
if ('' === $value) {
return null;
}
return $value;
}
/**
* Applies personal data fields to participant.
*/
src/Service/TravelDataService.php
+11 -5
View File
@@ -34,6 +34,7 @@ class TravelDataService
public const SOURCE_LOCAL = 'local';
public const SOURCE_REMOTE = 'remote';
private const AVAILABILITY_CACHE_TTL = 600;
private const MUTABILITY_CACHE_TTL = 3600;
public function __construct(
private readonly TravelLoader $travelLoader,
@@ -438,20 +439,25 @@ class TravelDataService
/**
* Gets mutability data for a travel date.
*
* @param int $dateId The travel date ID for API call
* @param bool $cached Whether to use cached data (default: true, TTL: 12 hours)
* @param int $dateId The travel date ID for API call
* @param bool $cached Whether to use cached data (default: true, TTL: 1 hour)
* @param bool $forceRefresh Whether to invalidate cache before fetching (requires cached: true)
*
* @return BaseData|null The mutability data or null if not available or error occurred
*/
public function getMutabilityData(int $dateId, bool $cached = true): ?BaseData
public function getMutabilityData(int $dateId, bool $cached = true, bool $forceRefresh = false): ?BaseData
{
if ($cached) {
$cacheKey = sprintf('mutability_%d', $dateId);
try {
if (true === $forceRefresh) {
$this->cache->delete($cacheKey);
}
return $this->cache->get($cacheKey, function (ItemInterface $item) use ($dateId) {
// 12 hours TTL - mutability dates have date-only granularity
$item->expiresAfter(43200);
// 1 hour TTL - better safe than sorry with mutability changes
$item->expiresAfter(self::MUTABILITY_CACHE_TTL);
return $this->fetchMutabilityData($dateId);
});
tests/Service/BookingEditDraftServiceMutabilityTest.php
+79 -5
View File
@@ -84,6 +84,7 @@ class BookingEditDraftServiceMutabilityTest extends TestCase
$this->assertSame([], $participant->rentals);
$this->assertNull($participant->skiPass);
$this->assertNull($participant->rentalInsurance);
$this->assertTrue($this->service->hadImmutableSkips());
}
public function testDraftAppliesAdditionalServicesWhenMutable(): void
@@ -122,8 +123,11 @@ class BookingEditDraftServiceMutabilityTest extends TestCase
$this->assertSame(99, $participant->additionalServices[0]->id);
$this->assertCount(1, $participant->courses);
$this->assertSame(99, $participant->courses[0]->id);
$this->assertNotNull($participant->skiPass);
$this->assertSame(99, $participant->skiPass->id);
$this->assertInstanceOf(Service::class, $participant->skiPass);
$skiPass = $participant->skiPass;
$this->assertNotNull($skiPass);
$this->assertSame(99, $skiPass?->id);
$this->assertFalse($this->service->hadImmutableSkips());
}
// --- Transportation Mutability ---
@@ -167,6 +171,7 @@ class BookingEditDraftServiceMutabilityTest extends TestCase
$this->assertSame($originalOutbound, $participant->transportationOutbound);
$this->assertSame($originalInbound, $participant->transportationInbound);
$this->assertFalse($participant->parking);
$this->assertTrue($this->service->hadImmutableSkips());
}
public function testDraftAppliesTransportationWhenMutable(): void
@@ -237,6 +242,7 @@ class BookingEditDraftServiceMutabilityTest extends TestCase
$this->assertSame($originalPickup, $participant->pickup);
$this->assertNull($participant->dropOff);
$this->assertFalse($participant->differentDropOff);
$this->assertTrue($this->service->hadImmutableSkips());
}
public function testDraftAppliesPickupsWhenMutable(): void
@@ -347,11 +353,12 @@ class BookingEditDraftServiceMutabilityTest extends TestCase
// Pickups: mutable → draft applied
$this->assertSame($draftPickup, $participant->pickup);
$this->assertTrue($this->service->hadImmutableSkips());
}
// --- Personal data is always applied regardless of mutability ---
// --- Personal data draft restore respects edit mutability rules ---
public function testDraftAlwaysAppliesPersonalDataRegardlessOfMutability(): void
public function testDraftSkipsPersonalDataForFirstParticipantInNonInternalAgency(): void
{
$participant = new ParticipantDto();
$participant->firstName = 'Original';
@@ -376,7 +383,74 @@ class BookingEditDraftServiceMutabilityTest extends TestCase
$this->service->applyDraftToDto($draft, $dto, $travel);
$this->assertSame('Updated', $participant->firstName);
$this->assertSame('Original', $participant->firstName);
$this->assertTrue($this->service->hadImmutableSkips());
}
public function testDraftAppliesPersonalDataForMutableNonFirstParticipant(): void
{
$firstParticipant = new ParticipantDto();
$firstParticipant->firstName = 'Applicant';
$mutableParticipant = new ParticipantDto();
$mutableParticipant->firstName = 'Original';
$mutableParticipant->mutable = true;
$travel = $this->createTravel();
$dto = $this->createBookingDto($travel, [$firstParticipant, $mutableParticipant]);
$draft = $this->createDraft([
'participants' => [
1 => [
'personalData' => [
'firstName' => 'Updated',
],
],
],
]);
$this->service->applyDraftToDto($draft, $dto, $travel);
$this->assertSame('Updated', $mutableParticipant->firstName);
$this->assertFalse($this->service->hadImmutableSkips());
}
public function testDraftDoesNotFlagImmutableSkipWhenImmutableServiceStateMatchesDraft(): void
{
$originalService = $this->createService(10, 'Original');
$participant = new ParticipantDto();
$participant->additionalServices = [$originalService];
$participant->courses = [];
$participant->board = [];
$participant->rentals = [];
$participant->skiPass = null;
$participant->rentalInsurance = null;
$travel = $this->createTravel(
additionalServicesMutable: false,
additionalServices: [10 => $originalService],
);
$dto = $this->createBookingDto($travel, [$participant]);
$draft = $this->createDraft([
'participants' => [
0 => [
'services' => [
'additionalServices' => [10],
'courses' => [],
'board' => [],
'rentals' => [],
'skiPass' => null,
'rentalInsurance' => null,
],
],
],
]);
$this->service->applyDraftToDto($draft, $dto, $travel);
$this->assertFalse($this->service->hadImmutableSkips());
}
// --- Helpers ---