From efff024196f703f764ab2394ab6b9d1f4b0e355b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Fromme?= Date: Tue, 10 Mar 2026 17:40:01 +0100 Subject: [PATCH] fix: ensure mutability is accounted for when restoring drafts addresses #869cdzmkq --- .../Booking/Edit/IndexController.php | 4 + src/Service/BookingEditDataLoaderService.php | 29 +- src/Service/BookingEditDraftService.php | 253 +++++++++++++++++- src/Service/TravelDataService.php | 16 +- .../BookingEditDraftServiceMutabilityTest.php | 84 +++++- 5 files changed, 364 insertions(+), 22 deletions(-) diff --git a/src/Controller/Booking/Edit/IndexController.php b/src/Controller/Booking/Edit/IndexController.php index 31297b9..d1358dc 100644 --- a/src/Controller/Booking/Edit/IndexController.php +++ b/src/Controller/Booking/Edit/IndexController.php @@ -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.) diff --git a/src/Service/BookingEditDataLoaderService.php b/src/Service/BookingEditDataLoaderService.php index 23ce880..c4c7c57 100644 --- a/src/Service/BookingEditDataLoaderService.php +++ b/src/Service/BookingEditDataLoaderService.php @@ -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; } } diff --git a/src/Service/BookingEditDraftService.php b/src/Service/BookingEditDraftService.php index c55c026..7737f38 100644 --- a/src/Service/BookingEditDraftService.php +++ b/src/Service/BookingEditDraftService.php @@ -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. */ diff --git a/src/Service/TravelDataService.php b/src/Service/TravelDataService.php index 2edf2df..b06667a 100644 --- a/src/Service/TravelDataService.php +++ b/src/Service/TravelDataService.php @@ -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); }); diff --git a/tests/Service/BookingEditDraftServiceMutabilityTest.php b/tests/Service/BookingEditDraftServiceMutabilityTest.php index 5b2c0ec..d1e2ca3 100644 --- a/tests/Service/BookingEditDraftServiceMutabilityTest.php +++ b/tests/Service/BookingEditDraftServiceMutabilityTest.php @@ -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 ---