From 6ee00f2bee3d4373f3f12308aac329cce8bd4703 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Fromme?= Date: Thu, 6 Aug 2026 13:08:46 +0200 Subject: [PATCH] feat: hide unavailable ski passes from selection in create flow --- .../ParticipantFieldOptionsProvider.php | 88 +++++++-- src/Service/ServiceAvailabilityCalculator.php | 33 ++-- .../CrmAttributesResponseParserTest.php | 1 - ...articipantFieldOptionsProviderBabyTest.php | 6 + ...OptionsProviderSkiPassAvailabilityTest.php | 185 ++++++++++++++++++ .../ServiceAvailabilityCalculatorTest.php | 46 +++++ 6 files changed, 329 insertions(+), 30 deletions(-) create mode 100644 tests/Form/Service/ParticipantFieldOptionsProviderSkiPassAvailabilityTest.php diff --git a/src/Form/Service/ParticipantFieldOptionsProvider.php b/src/Form/Service/ParticipantFieldOptionsProvider.php index dc5419d..ff6d1a4 100644 --- a/src/Form/Service/ParticipantFieldOptionsProvider.php +++ b/src/Form/Service/ParticipantFieldOptionsProvider.php @@ -471,11 +471,9 @@ class ParticipantFieldOptionsProvider extends AbstractFieldOptionsProvider return $attributes; } - // Make readonly if service is unavailable (intelligently handles edit mode) - if ($this->shouldMakeServiceReadonly($service, $bookingDto, $participantIndex, 'skiPass')) { - $attributes['readonly'] = true; - $attributes['data-tooltip'] = 'ausgebucht'; - } + // No availability readonly state here: unavailable ski passes are removed from + // the choices by filterSkiPassChoices(), and the ones that survive as protected + // selections have to stay selectable return $attributes; }, @@ -998,8 +996,13 @@ class ParticipantFieldOptionsProvider extends AbstractFieldOptionsProvider * Filters ski pass choices for display. * * Ski passes follow the same age evaluation rules as the rest of the booking flow. - * Participants only see passes that match their age or birth year. In edit mode, - * the currently booked ski pass is preserved even if it would otherwise be filtered + * Participants only see passes that match their age or birth year. Unlike the other + * service categories, unavailable ski passes - sold out or on Buchungsstop - are not + * rendered read-only but removed from the choices entirely, because a short-term, + * special-priced pass that is gone is no longer an offer worth showing. + * + * Passes the participant already holds are protected from that removal, and in edit + * mode the currently booked ski pass is preserved even if age rules would filter it * out, so existing bookings remain renderable and editable. * * @param Service[] $services Array of ski pass Service objects to filter @@ -1017,20 +1020,77 @@ class ParticipantFieldOptionsProvider extends AbstractFieldOptionsProvider return []; } - $filteredServices = $this->filterServicesByAgeConstraints($services, $bookingDto, $participantIndex); + $isEditMode = BookingDto::MODE_EDIT === $bookingDto->getMode(); + $ageFilteredServices = $this->filterServicesByAgeConstraints($services, $bookingDto, $participantIndex); - if (BookingDto::MODE_EDIT !== $bookingDto->getMode() || null === $participant->skiPass?->id) { - return $filteredServices; - } + $filteredServices = $this->serviceAvailabilityCalculator->filterAvailableServices( + $ageFilteredServices, + $bookingDto, + $participantIndex + ); - $currentSkiPassId = $participant->skiPass->id; - if (false === isset($filteredServices[$currentSkiPassId])) { - $filteredServices[$currentSkiPassId] = $services[$currentSkiPassId] ?? $participant->skiPass; + foreach ($this->getProtectedSkiPassIds($bookingDto, $participantIndex) as $protectedId) { + if (true === isset($filteredServices[$protectedId])) { + continue; + } + + $protectedService = $ageFilteredServices[$protectedId] ?? null; + + // Age rules are only bypassed in edit mode, where a booked pass has to stay renderable + if (null === $protectedService && true === $isEditMode) { + $protectedService = $services[$protectedId] ?? null; + + if (null === $protectedService && $participant->skiPass?->id === $protectedId) { + $protectedService = $participant->skiPass; + } + } + + if (null !== $protectedService) { + $filteredServices[$protectedId] = $protectedService; + } } return $filteredServices; } + /** + * Collects the ski pass IDs that have to stay selectable regardless of availability. + * + * Both the current selection and - in edit mode - the pass of the underlying booking are + * protected. Without this, a participant holding a pass that sold out or went on + * Buchungsstop meanwhile would lose it on the next refresh, and switching to another pass + * would be a one-way decision. + * + * @param BookingDto $bookingDto The booking DTO containing participant and booking data + * @param int $participantIndex Index of the participant to evaluate + * + * @return int[] Ski pass service IDs that must not be filtered out + */ + private function getProtectedSkiPassIds(BookingDto $bookingDto, int $participantIndex): array + { + $protectedIds = []; + + $currentSkiPassId = $bookingDto->getParticipant($participantIndex)?->skiPass?->id; + if (null !== $currentSkiPassId) { + $protectedIds[] = $currentSkiPassId; + } + + if (BookingDto::MODE_EDIT === $bookingDto->getMode() && null !== $bookingDto->booking) { + $bookedSkiPasses = $bookingDto->booking->getAdditionalServicesForParticipantByGroup( + $participantIndex, + Constants::TOKEN_SKI_PASS + ); + + foreach ($bookedSkiPasses as $bookedSkiPass) { + if (null !== $bookedSkiPass->id) { + $protectedIds[] = $bookedSkiPass->id; + } + } + } + + return array_values(array_unique($protectedIds)); + } + /** * Generates a German tooltip explaining age restrictions for a service. * diff --git a/src/Service/ServiceAvailabilityCalculator.php b/src/Service/ServiceAvailabilityCalculator.php index 4e931a9..f4abd01 100644 --- a/src/Service/ServiceAvailabilityCalculator.php +++ b/src/Service/ServiceAvailabilityCalculator.php @@ -51,7 +51,9 @@ class ServiceAvailabilityCalculator } /** - * Filter services array to only include those with remaining availability. + * Filter services array to only include those that are still bookable. + * + * Delegates to isServiceUnavailable() so that both entry points share one rule set. * * @param array $services Array of Service objects to filter * @param BookingDto $bookingDto The booking data with participant selections @@ -61,32 +63,28 @@ class ServiceAvailabilityCalculator */ public function filterAvailableServices(array $services, BookingDto $bookingDto, int $participantIndex): array { - $remainingAvailability = $this->calculateRemainingAvailability($bookingDto, $participantIndex); - - return array_filter($services, function (Service $service) use ($remainingAvailability) { - // If service has no availability limit set (null), treat as unlimited - if (null === $service->available) { + return array_filter($services, function (Service $service) use ($bookingDto, $participantIndex) { + // Services without an ID cannot be resolved against travel data - treat as available + if (null === $service->id) { return true; } - // 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; + return false === $this->isServiceUnavailable($service->id, $bookingDto, $participantIndex); }); } /** - * Check if a specific service is unavailable (sold out) for the current participant. + * Check if a specific service is unavailable for the current participant. + * + * A service is unavailable when it carries a booking stop (Buchungsstop) or when its + * contingent is exhausted, either at the API level or through selections made by the + * other participants of the current booking. * * @param int $serviceId The ID of the service to check * @param BookingDto $bookingDto The booking data with participant selections * @param int $participantIndex The index of the participant currently filling the form * - * @return bool True if the service is unavailable (has availability limit and remaining is 0) + * @return bool True if the service is unavailable */ public function isServiceUnavailable(int $serviceId, BookingDto $bookingDto, int $participantIndex): bool { @@ -105,6 +103,11 @@ class ServiceAvailabilityCalculator return false; } + // A booking stop blocks the service regardless of its contingent + if (Constants::STATUS_BLOCKED === $service->status) { + return true; + } + // If no availability tracking (null), service is unlimited and available if (null === $service->available) { return false; diff --git a/tests/BusProNet/XmlParser/CrmAttributesResponseParserTest.php b/tests/BusProNet/XmlParser/CrmAttributesResponseParserTest.php index 21dced6..27edf30 100644 --- a/tests/BusProNet/XmlParser/CrmAttributesResponseParserTest.php +++ b/tests/BusProNet/XmlParser/CrmAttributesResponseParserTest.php @@ -62,7 +62,6 @@ class CrmAttributesResponseParserTest extends TestCase self::assertContains('ROLE_ADMIN', $roles); self::assertContains('ROLE_MANAGER', $roles); self::assertContains('ROLE_TEAMER', $roles); - self::assertContains('ROLE_HOUSE_MANAGER', $roles); } /** diff --git a/tests/Form/Service/ParticipantFieldOptionsProviderBabyTest.php b/tests/Form/Service/ParticipantFieldOptionsProviderBabyTest.php index f5e8194..8246f4b 100644 --- a/tests/Form/Service/ParticipantFieldOptionsProviderBabyTest.php +++ b/tests/Form/Service/ParticipantFieldOptionsProviderBabyTest.php @@ -29,6 +29,12 @@ class ParticipantFieldOptionsProviderBabyTest extends TestCase protected function setUp(): void { $this->serviceAvailabilityCalculator = $this->createMock(ServiceAvailabilityCalculator::class); + // These tests target age filtering, so availability filtering passes everything through + // unless a test stubs it explicitly + $this->serviceAvailabilityCalculator + ->method('filterAvailableServices') + ->willReturnArgument(0) + ; $insuranceService = $this->createMock(InsuranceManager::class); $priceCalculatorService = $this->createMock(BookingPriceCalculator::class); $serviceLabelFormatter = new ServiceLabelFormatter(); diff --git a/tests/Form/Service/ParticipantFieldOptionsProviderSkiPassAvailabilityTest.php b/tests/Form/Service/ParticipantFieldOptionsProviderSkiPassAvailabilityTest.php new file mode 100644 index 0000000..ec8eed8 --- /dev/null +++ b/tests/Form/Service/ParticipantFieldOptionsProviderSkiPassAvailabilityTest.php @@ -0,0 +1,185 @@ +createMock(TranslatorInterface::class); + $translator->method('trans')->willReturnArgument(0); + + $this->provider = new ParticipantFieldOptionsProvider( + new ServiceAvailabilityCalculator(), + $this->createMock(InsuranceManager::class), + $this->createMock(BookingPriceCalculator::class), + new ServiceLabelFormatter(), + $translator + ); + } + + public function testSoldOutSkiPassIsNotRendered(): void + { + $bookingDto = $this->createBookingDto([ + $this->createSkiPass(id: 1, available: null), + $this->createSkiPass(id: 2, available: 0), + ]); + + $this->assertSame([1], $this->getSkiPassChoiceIds($bookingDto)); + } + + public function testSkiPassOnBookingStopIsNotRendered(): void + { + $bookingDto = $this->createBookingDto([ + $this->createSkiPass(id: 1, available: null), + $this->createSkiPass(id: 2, available: 20, status: Constants::STATUS_BLOCKED), + ]); + + $this->assertSame([1], $this->getSkiPassChoiceIds($bookingDto)); + } + + public function testSkiPassClaimedByOtherParticipantIsNotRendered(): void + { + $limitedSkiPass = $this->createSkiPass(id: 2, available: 1); + $bookingDto = $this->createBookingDto([ + $this->createSkiPass(id: 1, available: null), + $limitedSkiPass, + ]); + + $otherParticipant = new ParticipantDto(); + $otherParticipant->index = 1; + $otherParticipant->dateOfBirth = new \DateTimeImmutable('1990-01-01'); + $otherParticipant->skiPass = $limitedSkiPass; + $bookingDto->participants[1] = $otherParticipant; + + $this->assertSame([1], $this->getSkiPassChoiceIds($bookingDto)); + } + + public function testSelectedSkiPassStaysRenderedWhenItSoldOut(): void + { + $soldOutSkiPass = $this->createSkiPass(id: 2, available: 0); + $bookingDto = $this->createBookingDto([ + $this->createSkiPass(id: 1, available: null), + $soldOutSkiPass, + ]); + + $bookingDto->participants[0]->skiPass = $soldOutSkiPass; + + $this->assertSame([1, 2], $this->getSkiPassChoiceIds($bookingDto)); + } + + public function testBookedSkiPassStaysRenderedAfterSwitchingToAnotherPassInEditMode(): void + { + $blockedSkiPass = $this->createSkiPass(id: 2, available: null, status: Constants::STATUS_BLOCKED); + $bookableSkiPass = $this->createSkiPass(id: 1, available: null); + $bookingDto = $this->createBookingDto([$bookableSkiPass, $blockedSkiPass]); + + $bookedSkiPass = clone $blockedSkiPass; + $bookedSkiPass->mapping = [0]; + + $booking = new Booking(); + $booking->additionalServices = [2 => $bookedSkiPass]; + $bookingDto->booking = $booking; + + // The participant already moved away from the booked pass - it still has to be offered + // so the switch stays reversible + $bookingDto->participants[0]->skiPass = $bookableSkiPass; + + $this->assertSame(BookingDto::MODE_EDIT, $bookingDto->getMode()); + $this->assertSame([1, 2], $this->getSkiPassChoiceIds($bookingDto)); + } + + public function testRenderedSkiPassesCarryNoSoldOutTooltip(): void + { + $soldOutSkiPass = $this->createSkiPass(id: 2, available: 0); + $bookingDto = $this->createBookingDto([ + $this->createSkiPass(id: 1, available: null), + $soldOutSkiPass, + ]); + + $bookingDto->participants[0]->skiPass = $soldOutSkiPass; + + $options = $this->provider->getFieldOptions('skiPass', $bookingDto, 0); + $choiceAttr = $options['choice_attr']; + + foreach ($options['choices'] as $choice) { + $attributes = $choiceAttr($choice); + + $this->assertArrayNotHasKey('readonly', $attributes); + $this->assertArrayNotHasKey('data-tooltip', $attributes); + } + } + + /** @return int[] */ + private function getSkiPassChoiceIds(BookingDto $bookingDto): array + { + $options = $this->provider->getFieldOptions('skiPass', $bookingDto, 0); + + if (true === empty($options)) { + return []; + } + + $choiceIds = array_map(static fn (Service $service): ?int => $service->id, $options['choices']); + sort($choiceIds); + + return $choiceIds; + } + + /** @param Service[] $skiPasses */ + private function createBookingDto(array $skiPasses): BookingDto + { + $travel = new Travel(); + $travel->dateFrom = new \DateTimeImmutable('2027-01-09'); + $travel->dateTo = new \DateTimeImmutable('2027-01-16'); + + foreach ($skiPasses as $skiPass) { + $travel->additionalServices[$skiPass->id] = $skiPass; + } + + $bookingDto = new BookingDto($travel, 1); + + $participant = new ParticipantDto(); + $participant->index = 0; + $participant->dateOfBirth = new \DateTimeImmutable('1990-01-01'); + $bookingDto->participants[0] = $participant; + + return $bookingDto; + } + + private function createSkiPass(int $id, ?int $available, string $status = Constants::STATUS_AVAILABLE): Service + { + $service = new Service(); + $service->id = $id; + $service->label = sprintf('Skipass %d', $id); + $service->subType = Constants::TOKEN_SKI_PASS; + $service->price = (float) $id; + $service->available = $available; + $service->status = $status; + + return $service; + } +} diff --git a/tests/Service/ServiceAvailabilityCalculatorTest.php b/tests/Service/ServiceAvailabilityCalculatorTest.php index 6b27c64..d319335 100644 --- a/tests/Service/ServiceAvailabilityCalculatorTest.php +++ b/tests/Service/ServiceAvailabilityCalculatorTest.php @@ -51,6 +51,40 @@ class ServiceAvailabilityCalculatorTest extends TestCase $this->assertTrue($this->calculator->isServiceUnavailable(1, $bookingDto, 0)); } + public function testServiceOnBookingStopIsUnavailableDespiteUnlimitedQuota(): void + { + $skiPass = $this->createSkiPass(id: 1, available: null, status: Constants::STATUS_BLOCKED); + $bookingDto = $this->createBookingDtoWithServices([$skiPass]); + + $this->assertTrue($this->calculator->isServiceUnavailable(1, $bookingDto, 0)); + } + + public function testServiceOnBookingStopIsUnavailableDespiteRemainingQuota(): void + { + $skiPass = $this->createSkiPass(id: 1, available: 10, status: Constants::STATUS_BLOCKED); + $bookingDto = $this->createBookingDtoWithServices([$skiPass]); + + $this->assertTrue($this->calculator->isServiceUnavailable(1, $bookingDto, 0)); + } + + public function testFilterAvailableServicesRemovesSoldOutAndBlockedServices(): void + { + $bookable = $this->createSkiPass(id: 1, available: 5); + $soldOut = $this->createSkiPass(id: 2, available: 0); + $blocked = $this->createSkiPass(id: 3, available: null, status: Constants::STATUS_BLOCKED); + $unlimited = $this->createSkiPass(id: 4, available: null); + + $bookingDto = $this->createBookingDtoWithServices([$bookable, $soldOut, $blocked, $unlimited]); + + $filtered = $this->calculator->filterAvailableServices( + $bookingDto->travel->additionalServices, + $bookingDto, + 0 + ); + + $this->assertSame([1, 4], array_keys($filtered)); + } + private function createParkingService(int $id, int $available): Service { $service = new Service(); @@ -62,6 +96,18 @@ class ServiceAvailabilityCalculatorTest extends TestCase return $service; } + private function createSkiPass(int $id, ?int $available, string $status = Constants::STATUS_AVAILABLE): Service + { + $service = new Service(); + $service->id = $id; + $service->label = sprintf('Skipass %d', $id); + $service->subType = Constants::TOKEN_SKI_PASS; + $service->available = $available; + $service->status = $status; + + return $service; + } + private function createBookingDtoWithServices(array $services): BookingDto { $travel = new Travel();