From de52907eec8ee8984306eb4d162de959c6234351 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Fromme?= Date: Sat, 6 Dec 2025 15:01:31 +0100 Subject: [PATCH] fix: remove form options cache causing issues --- src/Form/BookingParticipantType.php | 11 +- .../ParticipantFieldHandlerRegistry.php | 164 +++++++++++++++++- .../ParticipantFieldOptionsProvider.php | 46 +---- 3 files changed, 166 insertions(+), 55 deletions(-) diff --git a/src/Form/BookingParticipantType.php b/src/Form/BookingParticipantType.php index 7d3f9b7..b5de297 100644 --- a/src/Form/BookingParticipantType.php +++ b/src/Form/BookingParticipantType.php @@ -63,6 +63,10 @@ class BookingParticipantType extends AbstractType * Field handlers are executed in PRE_SUBMIT to clean and transform data * before Symfony binds it to the form. This matches the pattern used in * the old BookingCreateStep2Type parent form. + * + * The method also synchronizes the submitted data with the DTO state to + * ensure that handler modifications (like discount replacement) are reflected + * in the form binding. */ private function processFieldHandlers(FormEvent $event, BookingDto $bookingContext): void { @@ -80,12 +84,15 @@ class BookingParticipantType extends AbstractType return; } - // Process all field handlers for this participant in dependency order - $this->fieldHandlerRegistry->processFieldsForParticipant( + // Process all field handlers for this participant and sync submitted data + $syncedData = $this->fieldHandlerRegistry->processFieldsForParticipantAndSync( $submittedData, $bookingContext, $data->participant->index ); + + // Update the event data with synchronized values + $event->setData($syncedData); } /** diff --git a/src/Form/Service/ParticipantFieldHandlerRegistry.php b/src/Form/Service/ParticipantFieldHandlerRegistry.php index ce775d2..485f54a 100644 --- a/src/Form/Service/ParticipantFieldHandlerRegistry.php +++ b/src/Form/Service/ParticipantFieldHandlerRegistry.php @@ -167,6 +167,150 @@ class ParticipantFieldHandlerRegistry } } + /** + * Processes field handlers for a single participant and synchronizes submitted data. + * + * This method combines field processing with data synchronization to ensure that + * the submitted form data reflects any changes made by field handlers. This is + * essential for handlers that modify fields (like discount replacement) to have + * their changes reflected in the form binding. + * + * Only fields that are actually modified by handlers (detected by comparing + * submitted ID with DTO value after processing) are synchronized. This prevents + * overwriting partial user input (like incomplete dates). + * + * @param array $participantData Submitted data for one participant + * @param BookingDto $bookingDto The booking DTO to update + * @param int $participantIndex Index of participant to process + * + * @return array The synchronized participant data reflecting DTO changes + */ + public function processFieldsForParticipantAndSync(array $participantData, BookingDto $bookingDto, int $participantIndex): array + { + // Process all field handlers for this participant + $this->processFieldsForParticipant($participantData, $bookingDto, $participantIndex); + + // Synchronize only modified service fields with the cleaned DTO state + $participant = $bookingDto->getParticipant($participantIndex); + if (null === $participant) { + return $participantData; + } + + return $this->syncModifiedServiceFields($participantData, $participant); + } + + /** + * Synchronizes only service fields that were modified by handlers. + * + * This method compares the submitted service ID with the DTO value after processing. + * Only fields where the handler assigned a different service are synchronized back. + * This prevents overwriting user input for fields like dateOfBirth where the handler + * just parses the value without changing it. + * + * @param array $participantData The submitted participant data + * @param ParticipantDto $participant The processed participant DTO + * + * @return array Updated participant data with modified fields synchronized + */ + private function syncModifiedServiceFields(array $participantData, ParticipantDto $participant): array + { + // Define fields that may be reassigned by handlers and need syncing + $serviceFields = [ + 'transportationOutbound', + 'transportationInbound', + 'insurance', + 'skiPass', + 'rentals', + 'courses', + 'additionalServices', + 'board', + 'pickup', + ]; + + foreach ($serviceFields as $fieldName) { + if (false === array_key_exists($fieldName, $participantData)) { + continue; + } + + if (false === property_exists($participant, $fieldName)) { + continue; + } + + $dtoValue = $participant->{$fieldName}; + $submittedValue = $participantData[$fieldName]; + + // Check if the DTO value differs from submitted value + if ($this->hasFieldValueChanged($submittedValue, $dtoValue)) { + $participantData[$fieldName] = $this->convertDtoValueToSubmittedFormat($dtoValue); + } + } + + return $participantData; + } + + /** + * Checks if a field value was changed by a handler. + * + * Compares the submitted value (usually an ID string) with the DTO value + * (usually a Service/Insurance object) to detect if a handler reassigned it. + * + * @param mixed $submittedValue The original submitted value (ID or array of IDs) + * @param mixed $dtoValue The DTO value after handler processing + * + * @return bool True if the values differ (handler modified the field) + */ + private function hasFieldValueChanged(mixed $submittedValue, mixed $dtoValue): bool + { + // Handle empty string as null (form submits empty string for unselected) + if ('' === $submittedValue) { + $submittedValue = null; + } + + // Handle null cases + if (null === $dtoValue && null === $submittedValue) { + return false; + } + if (null === $dtoValue || null === $submittedValue) { + return true; + } + + // Handle arrays (multiple selections like rentals, courses) + if (is_array($submittedValue) && is_array($dtoValue)) { + $submittedIds = array_map('strval', $submittedValue); + $dtoIds = array_map(fn ($item) => $this->extractId($item), $dtoValue); + sort($submittedIds); + sort($dtoIds); + + return $submittedIds !== $dtoIds; + } + + // Handle single service/insurance objects + $dtoId = $this->extractId($dtoValue); + + // Compare as integers for reliable comparison (form may submit string, DTO has int) + return (int) $submittedValue !== (int) $dtoId; + } + + /** + * Extracts the ID from a service, insurance, or pickup object. + * + * @param mixed $value The value to extract ID from + * + * @return string|null The extracted ID or null + */ + private function extractId(mixed $value): ?string + { + if ($value instanceof Service || $value instanceof Insurance || $value instanceof Pickup) { + return (string) $value->id; + } + + if (is_scalar($value)) { + return (string) $value; + } + + return null; + } + /** * Returns handler names sorted by dependency order using cached results when possible. * @@ -385,19 +529,19 @@ class ParticipantFieldHandlerRegistry */ private function convertSingleValueToSubmittedFormat(mixed $value): mixed { - // Handle Service objects -> convert to ID + // Handle Service objects -> convert to ID (as string for form compatibility) if ($value instanceof Service) { - return $value->id; + return (string) $value->id; } - // Handle Pickup objects -> convert to ID + // Handle Pickup objects -> convert to ID (as string for form compatibility) if ($value instanceof Pickup) { - return $value->id; + return (string) $value->id; } - // Handle Insurance objects -> convert to ID + // Handle Insurance objects -> convert to ID (as string for form compatibility) if ($value instanceof Insurance) { - return $value->id; + return (string) $value->id; } // Handle Address objects -> convert to array of properties @@ -411,9 +555,13 @@ class ParticipantFieldHandlerRegistry ]; } - // Handle DateTimeInterface -> convert to string format + // Handle DateTimeInterface -> convert to array format for BirthdayType widget if ($value instanceof \DateTimeInterface) { - return $value->format('Y-m-d'); + return [ + 'year' => $value->format('Y'), + 'month' => $value->format('n'), + 'day' => $value->format('j'), + ]; } // Handle numeric values diff --git a/src/Form/Service/ParticipantFieldOptionsProvider.php b/src/Form/Service/ParticipantFieldOptionsProvider.php index d43d017..597702c 100644 --- a/src/Form/Service/ParticipantFieldOptionsProvider.php +++ b/src/Form/Service/ParticipantFieldOptionsProvider.php @@ -36,9 +36,6 @@ use App\Service\ServiceAvailabilityCalculator; */ class ParticipantFieldOptionsProvider extends AbstractFieldOptionsProvider { - /** @var array> Request-scoped cache for field options */ - private array $optionsCache = []; - /** * Initializes the provider with required dependencies. * @@ -61,39 +58,6 @@ class ParticipantFieldOptionsProvider extends AbstractFieldOptionsProvider parent::__construct(); } - /** - * Retrieves form field options with request-scoped caching. - * - * Overrides parent to add instance-level caching, preventing redundant - * field option calculations when the same field is accessed multiple times - * during form building (happens ~16 times per participant × 50 participants = 800 calls). - * - * Cache key includes: field name, participant index, mode, and date of birth. - * The cache is automatically cleared between requests since the service is request-scoped. - * - * @param string $fieldName The name of the field to configure - * @param BookingDto $bookingDto The current booking data for context - * @param int $participantIndex The index of the participant being configured - * @param array $options Additional options to customize field behavior - * - * @return array Symfony form field options, or empty array if field not supported - */ - public function getFieldOptions(string $fieldName, BookingDto $bookingDto, int $participantIndex, array $options = []): array - { - // Generate cache key based on factors that affect field options - $participant = $bookingDto->getParticipant($participantIndex); - $cacheKey = sprintf( - '%s_%d_%s_%s', - $fieldName, - $participantIndex, - $bookingDto->getMode(), - $participant?->dateOfBirth?->format('Y-m-d') ?? 'no_dob' - ); - - // Return cached result if available, otherwise compute and cache - return $this->optionsCache[$cacheKey] ??= parent::getFieldOptions($fieldName, $bookingDto, $participantIndex, $options); - } - /** * Registers all field option providers during service initialization. * @@ -494,15 +458,7 @@ class ParticipantFieldOptionsProvider extends AbstractFieldOptionsProvider 'placeholder' => false, // Disable default placeholder - synthetic "keine Versicherung gewünscht" option injected instead 'choices' => $this->getEligibleInsurances($bookingDto, $participantIndex), 'choice_value' => 'id', - 'choice_label' => function (?Insurance $insurance) { - $label = $insurance->label; - - if (null !== $insurance->price && $insurance->price > 0) { - $label .= sprintf(' (€%s)', number_format($insurance->price, 2, ',', '.')); - } - - return $label; - }, + 'choice_label' => fn (?Insurance $insurance) => $insurance?->label, ]; // Purchase voucher field provider - redemption code for vouchers that apply to complete booking