From 3984a623bfd947de09e80731177458a9a1eab5f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Fromme?= Date: Thu, 11 Dec 2025 12:07:19 +0100 Subject: [PATCH] fix: properly assign readonly state to form fields for unavailable services --- src/BusProNet/Model/Travel.php | 48 +++++++---------- .../ParticipantFieldOptionsProvider.php | 37 ++++---------- .../ParticipantParkingFieldHandler.php | 2 +- ...ParticipantRentalInsuranceFieldHandler.php | 2 +- .../ParticipantRentalsFieldHandler.php | 2 +- .../ParticipantSkiPassFieldHandler.php | 4 +- ...rtationDiscountReplacementFieldHandler.php | 4 +- ...ipantTransportationInboundFieldHandler.php | 9 ++-- ...pantTransportationOutboundFieldHandler.php | 9 ++-- src/Service/ParticipantEligibilityService.php | 2 +- src/Service/ServiceAvailabilityCalculator.php | 28 +++++++--- src/Twig/AppRuntime.php | 6 +-- templates/booking/_participant_form.html.twig | 4 +- tests/BusProNet/Model/TravelTest.php | 51 ------------------- 14 files changed, 68 insertions(+), 140 deletions(-) diff --git a/src/BusProNet/Model/Travel.php b/src/BusProNet/Model/Travel.php index 1d23a2e..4691f8e 100644 --- a/src/BusProNet/Model/Travel.php +++ b/src/BusProNet/Model/Travel.php @@ -114,31 +114,28 @@ class Travel public array $insurances = []; /** - * Retrieves additional services filtered by subtype, availability, and optionally by travel date range. + * Retrieves additional services filtered by subtype and optionally by travel date range. * - * Filters additional services based on the provided subtype(s), availability, - * and optionally whether their date range overlaps with the travel dates. - * Services with null dates are considered always available when date filtering is enabled. + * Filters additional services based on the provided subtype(s) and optionally whether + * their date range overlaps with the travel dates. Services with null dates are + * considered always available when date filtering is enabled. * Services are sorted by price in ascending order. * + * Availability filtering is NOT applied here - it happens at the form/UI level where + * unavailable services are shown as readonly with appropriate tooltips. + * * @param mixed $subTypes The service subtype(s) to filter by - * @param bool $filterAvailable Whether to include only available services * @param bool $filterByTravelDateRange Whether to filter by travel date range overlap * * @return array The filtered and sorted services array */ - public function getAdditionalServicesBySubTypes(mixed $subTypes, bool $filterAvailable = true, bool $filterByTravelDateRange = false): array + public function getAdditionalServicesBySubTypes(mixed $subTypes, bool $filterByTravelDateRange = false): array { $subTypes = (array) $subTypes; - $services = array_filter($this->additionalServices, function (Service $service) use ($subTypes, $filterAvailable, $filterByTravelDateRange) { + $services = array_filter($this->additionalServices, function (Service $service) use ($subTypes, $filterByTravelDateRange) { // Check subtype - if (false === in_array($service->subType, $subTypes)) { - return false; - } - - // Check availability - if (true === $filterAvailable && null !== $service->available && 0 >= $service->available) { + if (false === in_array($service->subType, $subTypes, true)) { return false; } @@ -160,30 +157,21 @@ class Travel } /** - * Retrieves transportation services filtered by direction and availability. + * Retrieves transportation services filtered by direction. * - * Filters transportation services based on travel direction and optionally - * by availability. Services are sorted by subtype. + * Filters transportation services based on travel direction. Services are sorted by subtype. + * Availability filtering is NOT applied here - it happens at the form/UI level where + * unavailable services are shown as readonly with appropriate tooltips. * - * Note: PKW/CAR filtering based on booking context is NOT applied here. - * That filtering happens dynamically in ParticipantFieldOptionsProvider using - * per-booking availability calculations from ServiceAvailabilityCalculator. - * - * @param string $direction The travel direction to filter by - * @param bool $filterAvailable Whether to include only available services (API availability) + * @param string $direction The travel direction to filter by * * @return array The filtered and sorted transportation services */ - public function getTransportationServicesByDirection(string $direction, bool $filterAvailable = true): array + public function getTransportationServicesByDirection(string $direction): array { - $services = array_filter($this->transportationServices, function (Service $service) use ($direction, $filterAvailable) { - return $direction === $service->direction - && (false === $filterAvailable || $service->available > 0 || null === $service->available); - }); + $services = array_filter($this->transportationServices, fn (Service $service) => $direction === $service->direction); - usort($services, function (Service $a, Service $b) { - return $a->subType <=> $b->subType; - }); + usort($services, fn (Service $a, Service $b) => $a->subType <=> $b->subType); return $services; } diff --git a/src/Form/Service/ParticipantFieldOptionsProvider.php b/src/Form/Service/ParticipantFieldOptionsProvider.php index 969867f..8b5234d 100644 --- a/src/Form/Service/ParticipantFieldOptionsProvider.php +++ b/src/Form/Service/ParticipantFieldOptionsProvider.php @@ -104,10 +104,7 @@ class ParticipantFieldOptionsProvider extends AbstractFieldOptionsProvider 'expanded' => true, 'required' => false, 'choices' => $this->filterServicesByAgeConstraints( - $bookingDto->travel->getAdditionalServicesBySubTypes( - Constants::TOKEN_COURSES, - BookingDto::MODE_CREATE === $bookingDto->getMode() // Only filter by availability in create mode - ), + $bookingDto->travel->getAdditionalServicesBySubTypes(Constants::TOKEN_COURSES), $bookingDto, $participantIndex ), @@ -142,10 +139,7 @@ class ParticipantFieldOptionsProvider extends AbstractFieldOptionsProvider 'expanded' => true, 'required' => false, 'choices' => $this->filterServicesByAgeConstraints( - $bookingDto->travel->getAdditionalServicesBySubTypes( - Constants::TOKEN_ADDITIONAL, - BookingDto::MODE_EDIT !== $bookingDto->getMode() // Only filter by availability in create mode - ), + $bookingDto->travel->getAdditionalServicesBySubTypes(Constants::TOKEN_ADDITIONAL), $bookingDto, $participantIndex ), @@ -188,10 +182,7 @@ class ParticipantFieldOptionsProvider extends AbstractFieldOptionsProvider 'expanded' => true, 'required' => false, 'choices' => $this->filterServicesByAgeConstraints( - $bookingDto->travel->getAdditionalServicesBySubTypes( - Constants::TOKEN_BOARD, - BookingDto::MODE_CREATE === $bookingDto->getMode() // Only filter by availability in create mode - ), + $bookingDto->travel->getAdditionalServicesBySubTypes(Constants::TOKEN_BOARD), $bookingDto, $participantIndex ), @@ -227,11 +218,7 @@ class ParticipantFieldOptionsProvider extends AbstractFieldOptionsProvider 'required' => false, 'choices' => $this->filterServicesByAgeConstraints( $this->filterRentalsBySkiPassDuration( - $bookingDto->travel->getAdditionalServicesBySubTypes( - Constants::TOKEN_RENTALS, - BookingDto::MODE_CREATE === $bookingDto->getMode(), // Only filter by availability in create mode - true // Filter by travel date range - ), + $bookingDto->travel->getAdditionalServicesBySubTypes(Constants::TOKEN_RENTALS, true), $bookingDto, $participantIndex ), @@ -264,10 +251,10 @@ class ParticipantFieldOptionsProvider extends AbstractFieldOptionsProvider // Rental insurance field provider - provides rental insurance options when rental services are selected $this->fieldOptionProviders['rentalInsurance'] = fn (BookingDto $bookingDto, int $participantIndex, array $options = []) => [ - 'label' => $this->getRentalInsuranceCheckboxLabel($bookingDto->travel->getAdditionalServicesBySubTypes(Constants::TOKEN_RENTAL_INSURANCE, true, true)), + 'label' => $this->getRentalInsuranceCheckboxLabel($bookingDto->travel->getAdditionalServicesBySubTypes(Constants::TOKEN_RENTAL_INSURANCE, true)), 'required' => false, 'property_path' => 'rentalInsuranceSelected', - 'help' => $this->getRentalInsuranceDescription($bookingDto->travel->getAdditionalServicesBySubTypes(Constants::TOKEN_RENTAL_INSURANCE, true, true)), + 'help' => $this->getRentalInsuranceDescription($bookingDto->travel->getAdditionalServicesBySubTypes(Constants::TOKEN_RENTAL_INSURANCE, true)), ]; // License plate field provider - provides text input for vehicle license plate when parking is selected @@ -292,11 +279,7 @@ class ParticipantFieldOptionsProvider extends AbstractFieldOptionsProvider 'expanded' => true, 'required' => true, 'choices' => $this->filterServicesByAgeConstraints( - $bookingDto->travel->getAdditionalServicesBySubTypes( - Constants::TOKEN_SKI_PASS, - BookingDto::MODE_CREATE === $bookingDto->getMode(), // Only filter by availability in create mode - true // Filter by travel date range - ), + $bookingDto->travel->getAdditionalServicesBySubTypes(Constants::TOKEN_SKI_PASS, true), $bookingDto, $participantIndex ), @@ -340,7 +323,7 @@ class ParticipantFieldOptionsProvider extends AbstractFieldOptionsProvider $this->fieldOptionProviders['transportationOutbound'] = fn (BookingDto $bookingDto, int $participantIndex, array $options = []) => [ 'label' => 'Hinfahrt', 'choices' => $this->filterTransportationChoices( - $bookingDto->travel->getTransportationServicesByDirection(DirectionMapper::OUTBOUND_TRAVEL, false), + $bookingDto->travel->getTransportationServicesByDirection(DirectionMapper::OUTBOUND_TRAVEL), $bookingDto, $participantIndex ), @@ -374,7 +357,7 @@ class ParticipantFieldOptionsProvider extends AbstractFieldOptionsProvider // Inbound Transportation $this->fieldOptionProviders['transportationInbound'] = fn (BookingDto $bookingDto, int $participantIndex, array $options = []) => [ 'label' => 'Rückfahrt', - 'choices' => $bookingDto->travel->getTransportationServicesByDirection(DirectionMapper::INBOUND_TRAVEL, false), + 'choices' => $bookingDto->travel->getTransportationServicesByDirection(DirectionMapper::INBOUND_TRAVEL), 'choice_label' => fn (Service $service) => $service?->label, 'choice_value' => 'id', 'expanded' => true, @@ -417,7 +400,7 @@ class ParticipantFieldOptionsProvider extends AbstractFieldOptionsProvider // Parking (conditional - only shown when outbound transportation is PKW) // Simple checkbox since there's only ever one parking type $this->fieldOptionProviders['parking'] = fn (BookingDto $bookingDto, int $participantIndex, array $options = []) => [ - 'label' => $this->getParkingCheckboxLabel($bookingDto->travel->getAdditionalServicesBySubTypes(Constants::TOKEN_PARKING, true)), + 'label' => $this->getParkingCheckboxLabel($bookingDto->travel->getAdditionalServicesBySubTypes(Constants::TOKEN_PARKING)), 'required' => false, ]; diff --git a/src/Form/Service/ParticipantParkingFieldHandler.php b/src/Form/Service/ParticipantParkingFieldHandler.php index 84b3ce7..7121cb8 100644 --- a/src/Form/Service/ParticipantParkingFieldHandler.php +++ b/src/Form/Service/ParticipantParkingFieldHandler.php @@ -118,7 +118,7 @@ class ParticipantParkingFieldHandler extends AbstractParticipantFieldHandler */ private function findParkingService(BookingDto $bookingDto): ?Service { - $parkingServices = $bookingDto->travel->getAdditionalServicesBySubTypes(Constants::TOKEN_PARKING, true); + $parkingServices = $bookingDto->travel->getAdditionalServicesBySubTypes(Constants::TOKEN_PARKING); if (empty($parkingServices)) { return null; diff --git a/src/Form/Service/ParticipantRentalInsuranceFieldHandler.php b/src/Form/Service/ParticipantRentalInsuranceFieldHandler.php index 13a1501..9f65bcf 100644 --- a/src/Form/Service/ParticipantRentalInsuranceFieldHandler.php +++ b/src/Form/Service/ParticipantRentalInsuranceFieldHandler.php @@ -125,7 +125,7 @@ class ParticipantRentalInsuranceFieldHandler extends AbstractParticipantFieldHan */ private function findRentalInsuranceService(BookingDto $bookingDto): ?Service { - $rentalInsuranceServices = $bookingDto->travel->getAdditionalServicesBySubTypes(Constants::TOKEN_RENTAL_INSURANCE, true, true); + $rentalInsuranceServices = $bookingDto->travel->getAdditionalServicesBySubTypes(Constants::TOKEN_RENTAL_INSURANCE, true); if (empty($rentalInsuranceServices)) { return null; diff --git a/src/Form/Service/ParticipantRentalsFieldHandler.php b/src/Form/Service/ParticipantRentalsFieldHandler.php index f128398..ff0c58f 100644 --- a/src/Form/Service/ParticipantRentalsFieldHandler.php +++ b/src/Form/Service/ParticipantRentalsFieldHandler.php @@ -83,7 +83,7 @@ class ParticipantRentalsFieldHandler extends AbstractParticipantFieldHandler } $selectedRentals = $this->getFieldValue($submittedData, $this->getFieldName()) ?? []; - $availableRentals = $bookingDto->travel->getAdditionalServicesBySubTypes(Constants::TOKEN_RENTALS, true, true); + $availableRentals = $bookingDto->travel->getAdditionalServicesBySubTypes(Constants::TOKEN_RENTALS, true); // Filter rentals by skipass duration to ensure only matching rentals are available $durationFilteredRentals = $this->filterRentalsBySkiPassDuration($availableRentals, $participant); diff --git a/src/Form/Service/ParticipantSkiPassFieldHandler.php b/src/Form/Service/ParticipantSkiPassFieldHandler.php index a0b5c8c..56ced3a 100644 --- a/src/Form/Service/ParticipantSkiPassFieldHandler.php +++ b/src/Form/Service/ParticipantSkiPassFieldHandler.php @@ -94,8 +94,8 @@ class ParticipantSkiPassFieldHandler extends AbstractParticipantFieldHandler // Extract current skipass selection from submitted data $selectedSkiPass = $this->getFieldValue($submittedData, $this->getFieldName()); - // Get available skipasses from travel data (with date filtering) - $availableSkipasses = $bookingDto->travel->getAdditionalServicesBySubTypes(Constants::TOKEN_SKI_PASS, true, true); + // Get all skipasses from travel data (with date filtering) + $availableSkipasses = $bookingDto->travel->getAdditionalServicesBySubTypes(Constants::TOKEN_SKI_PASS, true); // For single selection, validate the selected skipass and convert ID to Service object $validSelection = null; diff --git a/src/Form/Service/ParticipantTransportationDiscountReplacementFieldHandler.php b/src/Form/Service/ParticipantTransportationDiscountReplacementFieldHandler.php index 049b82b..48f81c2 100644 --- a/src/Form/Service/ParticipantTransportationDiscountReplacementFieldHandler.php +++ b/src/Form/Service/ParticipantTransportationDiscountReplacementFieldHandler.php @@ -62,10 +62,8 @@ class ParticipantTransportationDiscountReplacementFieldHandler extends AbstractP && DirectionMapper::SUBTYPE_BUS_API === $participant->transportationInbound->subType; // Step 3: Get all outbound PKW services - // Get ALL outbound services (not filtered by availability or smart filtering) $allOutboundServices = $bookingDto->travel->getTransportationServicesByDirection( - DirectionMapper::OUTBOUND_TRAVEL, - false // Do NOT filter by per-booking availability + DirectionMapper::OUTBOUND_TRAVEL ); // Find discounted and regular PKW services diff --git a/src/Form/Service/ParticipantTransportationInboundFieldHandler.php b/src/Form/Service/ParticipantTransportationInboundFieldHandler.php index 51c88a6..96c0aed 100644 --- a/src/Form/Service/ParticipantTransportationInboundFieldHandler.php +++ b/src/Form/Service/ParticipantTransportationInboundFieldHandler.php @@ -50,13 +50,12 @@ class ParticipantTransportationInboundFieldHandler extends AbstractParticipantFi $selectedTransportation = $this->getFieldValue($submittedData, $this->getFieldName()); - // Get available inbound transportation services - $availableServices = $bookingDto->travel->getTransportationServicesByDirection( - DirectionMapper::INBOUND_TRAVEL, - true // filter available + // Get all inbound transportation services (availability filtering happens at form level) + $services = $bookingDto->travel->getTransportationServicesByDirection( + DirectionMapper::INBOUND_TRAVEL ); // Validate and update participant with selection - $participant->transportationInbound = $this->findItemById($selectedTransportation, $availableServices); + $participant->transportationInbound = $this->findItemById($selectedTransportation, $services); } } diff --git a/src/Form/Service/ParticipantTransportationOutboundFieldHandler.php b/src/Form/Service/ParticipantTransportationOutboundFieldHandler.php index 1c0b806..25cc1ce 100644 --- a/src/Form/Service/ParticipantTransportationOutboundFieldHandler.php +++ b/src/Form/Service/ParticipantTransportationOutboundFieldHandler.php @@ -50,13 +50,12 @@ class ParticipantTransportationOutboundFieldHandler extends AbstractParticipantF $selectedTransportation = $this->getFieldValue($submittedData, $this->getFieldName()); - // Get available outbound transportation services - $availableServices = $bookingDto->travel->getTransportationServicesByDirection( - DirectionMapper::OUTBOUND_TRAVEL, - true // filter available + // Get all outbound transportation services (availability filtering happens at form level) + $services = $bookingDto->travel->getTransportationServicesByDirection( + DirectionMapper::OUTBOUND_TRAVEL ); // Validate and update participant with selection - $participant->transportationOutbound = $this->findItemById($selectedTransportation, $availableServices); + $participant->transportationOutbound = $this->findItemById($selectedTransportation, $services); } } diff --git a/src/Service/ParticipantEligibilityService.php b/src/Service/ParticipantEligibilityService.php index e6a866b..2ea785c 100644 --- a/src/Service/ParticipantEligibilityService.php +++ b/src/Service/ParticipantEligibilityService.php @@ -64,7 +64,7 @@ class ParticipantEligibilityService return true; } - $allSkiPasses = $bookingDto->travel->getAdditionalServicesBySubTypes(Constants::TOKEN_SKI_PASS, true, true); + $allSkiPasses = $bookingDto->travel->getAdditionalServicesBySubTypes(Constants::TOKEN_SKI_PASS, true); $availableSkiPasses = array_filter( $allSkiPasses, fn (Service $service) => $this->isSkiPassAvailableForParticipant($service, $bookingDto, $participantIndex) diff --git a/src/Service/ServiceAvailabilityCalculator.php b/src/Service/ServiceAvailabilityCalculator.php index 9493629..c60a9db 100644 --- a/src/Service/ServiceAvailabilityCalculator.php +++ b/src/Service/ServiceAvailabilityCalculator.php @@ -63,12 +63,17 @@ class ServiceAvailabilityCalculator $remainingAvailability = $this->calculateRemainingAvailability($bookingDto, $participantIndex); return array_filter($services, function (Service $service) use ($remainingAvailability) { - // If service has no availability limit set, treat as unlimited - if (null === $service->available || $service->available <= 0) { + // If service has no availability limit set (null), treat as unlimited + if (null === $service->available) { return true; } - // For services with availability limits, check remaining availability + // If availability is 0, service is sold out at the API level + if (0 === $service->available) { + return false; + } + + // For services with positive availability, check remaining availability return ($remainingAvailability[$service->id] ?? $service->available) > 0; }); } @@ -94,11 +99,22 @@ class ServiceAvailabilityCalculator } } - // If service not found or has no availability limit, it's not unavailable - if (null === $service || null === $service->available || $service->available <= 0) { + // If service not found, treat as available (not unavailable) + if (null === $service) { return false; } + // If no availability tracking (null), service is unlimited and available + if (null === $service->available) { + return false; + } + + // If availability is 0, service is sold out at the API level + if (0 === $service->available) { + return true; + } + + // For services with positive availability, calculate remaining based on booking selections $remainingAvailability = $this->calculateRemainingAvailability($bookingDto, $participantIndex); return ($remainingAvailability[$serviceId] ?? $service->available) <= 0; @@ -195,7 +211,7 @@ class ServiceAvailabilityCalculator { $allServices = []; - // Get transportation services + // Get all transportation services $transportationServices = array_merge( $bookingDto->travel->getTransportationServicesByDirection(DirectionMapper::OUTBOUND_TRAVEL) ?? [], $bookingDto->travel->getTransportationServicesByDirection(DirectionMapper::INBOUND_TRAVEL) ?? [] diff --git a/src/Twig/AppRuntime.php b/src/Twig/AppRuntime.php index 9c4a001..bf33568 100644 --- a/src/Twig/AppRuntime.php +++ b/src/Twig/AppRuntime.php @@ -61,11 +61,7 @@ class AppRuntime implements RuntimeExtensionInterface */ public function formatServicePrice(float|int|null $price): string { - if (null === $price) { - return ''; - } - - if (0 === $price || 0.0 === $price) { + if (null === $price || 0 === $price || 0.0 === $price) { return 'inkl.'; } diff --git a/templates/booking/_participant_form.html.twig b/templates/booking/_participant_form.html.twig index 5bd70e6..fac3be8 100644 --- a/templates/booking/_participant_form.html.twig +++ b/templates/booking/_participant_form.html.twig @@ -347,7 +347,7 @@ {% endfor %} {% if form.rentalInsurance is defined %} - {% set rentalInsuranceService = bookingDto.travel.getAdditionalServicesBySubTypes(constant('App\\BusProNet\\Constants::TOKEN_RENTAL_INSURANCE'), true, true)|first %} + {% set rentalInsuranceService = bookingDto.travel.getAdditionalServicesBySubTypes(constant('App\\BusProNet\\Constants::TOKEN_RENTAL_INSURANCE'), true)|first %} {{ macros.checkbox_row( form.rentalInsurance, constant('App\\BusProNet\\Constants::SERVICE_LABELS')[constant('App\\BusProNet\\Constants::TOKEN_RENTAL_INSURANCE')], @@ -532,7 +532,7 @@ {% endfor %} {% if form.parking is defined %} - {% set parkingService = bookingDto.travel.getAdditionalServicesBySubTypes(constant('App\\BusProNet\\Constants::TOKEN_PARKING'), true)|first %} + {% set parkingService = bookingDto.travel.getAdditionalServicesBySubTypes(constant('App\\BusProNet\\Constants::TOKEN_PARKING'))|first %} {{ macros.checkbox_row( form.parking, 'Parkplatz', diff --git a/tests/BusProNet/Model/TravelTest.php b/tests/BusProNet/Model/TravelTest.php index abb69a8..6da2bfb 100644 --- a/tests/BusProNet/Model/TravelTest.php +++ b/tests/BusProNet/Model/TravelTest.php @@ -49,57 +49,6 @@ class TravelTest extends TestCase $this->assertSame($inboundPkw, $inboundResult[0]); } - /** - * Test that unavailable services are filtered out when filterAvailable is true. - */ - public function testUnavailableServicesFiltered(): void - { - $travel = new Travel(); - - $availablePkw = new Service(); - $availablePkw->id = 1; - $availablePkw->subType = 'PKW'; - $availablePkw->direction = 'HIN'; - $availablePkw->label = 'Eigene Anreise'; - $availablePkw->available = 10; - - $unavailablePkw = new Service(); - $unavailablePkw->id = 2; - $unavailablePkw->subType = 'PKW'; - $unavailablePkw->direction = 'HIN'; - $unavailablePkw->label = 'Eigene Anreise mit Rabatt'; - $unavailablePkw->available = 0; - - $travel->transportationServices = [$availablePkw, $unavailablePkw]; - - $result = $travel->getTransportationServicesByDirection('HIN', true); - - $this->assertCount(1, $result); - $this->assertSame($availablePkw, $result[0]); - } - - /** - * Test that services with null availability are included. - */ - public function testNullAvailabilityIncluded(): void - { - $travel = new Travel(); - - $pkwWithNullAvailability = new Service(); - $pkwWithNullAvailability->id = 1; - $pkwWithNullAvailability->subType = 'PKW'; - $pkwWithNullAvailability->direction = 'HIN'; - $pkwWithNullAvailability->label = 'Eigene Anreise'; - $pkwWithNullAvailability->available = null; - - $travel->transportationServices = [$pkwWithNullAvailability]; - - $result = $travel->getTransportationServicesByDirection('HIN', true); - - $this->assertCount(1, $result); - $this->assertSame($pkwWithNullAvailability, $result[0]); - } - /** * Test that multiple PKW services are all returned (no smart filtering at Travel level). */