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

fix: stop offering unbookable services in the booking edit flow

Browse Source
This commit is contained in:
fromme committed 2026-09-16 15:28:07 +02:00
1 parent 672fc30d7e
commit 160ebef39e
18 files changed
+1094 -104
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

src/Service/BookingEditDataLoader.php
+19
View File
@@ -29,6 +29,9 @@ class BookingEditDataLoader
private bool $draftRestored = false;
/** @var list<string> */
private array $droppedDraftServiceLabels = [];
public function __construct(
private readonly ApiClient $apiClient,
private readonly BookingDataProcessor $bookingDataProcessor,
@@ -53,6 +56,17 @@ class BookingEditDataLoader
return $this->draftRestored;
}
/**
* Returns the drafted services dropped during the last load because they are no longer
* bookable. Reset on each call to loadFormData(), like isDraftRestored().
*
* @return list<string>
*/
public function getDroppedDraftServiceLabels(): array
{
return $this->droppedDraftServiceLabels;
}
/**
* Loads booking data from session or initializes from API.
*
@@ -72,6 +86,7 @@ class BookingEditDataLoader
public function loadFormData(Request $request, int $bookingId, User $user): ?BookingDto
{
$this->draftRestored = false;
$this->droppedDraftServiceLabels = [];
$formData = $this->bookingSessionService->getBookingDto($request, BookingDto::MODE_EDIT);
@@ -177,6 +192,10 @@ class BookingEditDataLoader
// it would tell the user their draft was restored for edits they never made.
$this->draftRestored = true === $applied
&& $formData->originalFingerprint !== $this->fingerprintService->generateFingerprint($formData);
if (true === $applied) {
$this->droppedDraftServiceLabels = $this->draftService->getDroppedServiceLabels();
}
}
$this->bookingSessionService->saveBookingDto($request, $formData, BookingDto::MODE_EDIT);
src/Service/BookingEditDraftManager.php
+30 -2
View File
@@ -23,6 +23,9 @@ use Psr\Log\LoggerInterface;
*/
class BookingEditDraftManager
{
/** @var list<string> */
private array $droppedServiceLabels = [];
public function __construct(
private readonly BookingEditDraftRepository $draftRepository,
private readonly EntityManagerInterface $entityManager,
@@ -32,6 +35,19 @@ class BookingEditDraftManager
) {
}
/**
* Returns the services dropped during the last applyDraftToDto() call.
*
* Reset on each call, so the caller can report them to the user. Mirrors
* BookingEditDataLoader::isDraftRestored().
*
* @return list<string>
*/
public function getDroppedServiceLabels(): array
{
return $this->droppedServiceLabels;
}
/**
* Finds an existing draft for a user and booking combination.
*
@@ -129,6 +145,9 @@ class BookingEditDraftManager
*/
public function applyDraftToDto(BookingEditDraft $draft, BookingDto $dto, Travel $travel): bool
{
$this->droppedServiceLabels = [];
$droppedServiceLabels = [];
try {
$formData = $draft->getFormData();
@@ -149,16 +168,25 @@ class BookingEditDraftManager
continue;
}
$this->participantApplier->apply(
$droppedServiceLabels = [...$droppedServiceLabels, ...$this->participantApplier->apply(
$dto,
$index,
$dto->participants[$index],
$participantData,
$travel,
);
)];
}
}
$this->droppedServiceLabels = array_values(array_unique($droppedServiceLabels));
if ([] !== $this->droppedServiceLabels) {
$this->logger->info('Dropped drafted services that are no longer bookable', [
'booking_id' => $draft->getBookingId(),
'services' => $this->droppedServiceLabels,
]);
}
$this->logger->info('Applied draft to booking DTO', [
'booking_id' => $draft->getBookingId(),
'draft_created_at' => $draft->getCreatedAt()->format('Y-m-d H:i:s'),
src/Service/BookingEditDraftMerger.php
+98 -23
View File
@@ -24,14 +24,24 @@ use App\Form\Model\ParticipantDto;
*/
class BookingEditDraftMerger
{
/** @param array<string, mixed> $data */
public function __construct(
private readonly ServiceAvailabilityCalculator $serviceAvailabilityCalculator,
) {
}
/**
* @param array<string, mixed> $data
*
* @return list<string> Labels of drafted services dropped because they are no longer bookable
*/
public function apply(
BookingDto $bookingDto,
int $participantIndex,
ParticipantDto $participant,
array $data,
Travel $travel,
): void {
): array {
$droppedServiceLabels = [];
$canApplyPersonalDataDraft = $this->canApplyPersonalDataDraft($bookingDto, $participantIndex, $participant);
// Personal data
@@ -68,10 +78,19 @@ class BookingEditDraftMerger
// Service selections (gated per-category by travel mutability flags)
if (isset($data['services']) && true === is_array($data['services'])) {
$this->applyServiceSelections($participant, $data['services'], $travel);
$this->applyServiceSelections(
$participant,
$data['services'],
$travel,
$bookingDto,
$participantIndex,
$droppedServiceLabels
);
}
$participant->normalizeLoadedData();
return array_values(array_unique($droppedServiceLabels));
}
private function canApplyPersonalDataDraft(BookingDto $bookingDto, int $participantIndex, ParticipantDto $participant): bool
@@ -205,16 +224,25 @@ class BookingEditDraftMerger
* Multi-select fields (checkboxes) and booleans use overwrite strategy: draft values
* always replace API data, since users can intentionally clear these selections.
*/
/** @param array<string, mixed> $data */
private function applyServiceSelections(ParticipantDto $participant, array $data, Travel $travel): void
{
/**
* @param array<string, mixed> $data
* @param list<string> $droppedServiceLabels Collects what was dropped, by reference
*/
private function applyServiceSelections(
ParticipantDto $participant,
array $data,
Travel $travel,
BookingDto $bookingDto,
int $participantIndex,
array &$droppedServiceLabels,
): void {
// Additional services category — only apply draft data when services are mutable.
// When immutable, the booking's current services must be preserved as-is to avoid
// API rejection (stale draft data could differ from the locked booking state).
if (true === $travel->additionalServicesMutable) {
// Ski pass (single service) - merge strategy: only apply if resolves to valid service
if (true === array_key_exists('skiPass', $data) && null !== $data['skiPass']) {
$resolved = $this->resolveService($data['skiPass'], $travel->additionalServices);
$resolved = $this->resolveService($data['skiPass'], $travel->additionalServices, $bookingDto, $participantIndex, $droppedServiceLabels);
if (null !== $resolved) {
$participant->skiPass = $resolved;
}
@@ -222,17 +250,17 @@ class BookingEditDraftMerger
// Courses (array) - overwrite strategy: user can deselect all
if (true === array_key_exists('courses', $data) && true === is_array($data['courses'])) {
$participant->courses = $this->resolveServiceArray($data['courses'], $travel->additionalServices);
$participant->courses = $this->resolveServiceArray($data['courses'], $travel->additionalServices, $bookingDto, $participantIndex, $droppedServiceLabels);
}
// Board (array) - overwrite strategy: user can deselect all
if (true === array_key_exists('board', $data) && true === is_array($data['board'])) {
$participant->board = $this->resolveServiceArray($data['board'], $travel->additionalServices);
$participant->board = $this->resolveServiceArray($data['board'], $travel->additionalServices, $bookingDto, $participantIndex, $droppedServiceLabels);
}
// Veg (single service) - merge strategy: only apply if resolves to valid service
if (true === array_key_exists('veg', $data) && null !== $data['veg']) {
$resolved = $this->resolveService($data['veg'], $travel->additionalServices);
$resolved = $this->resolveService($data['veg'], $travel->additionalServices, $bookingDto, $participantIndex, $droppedServiceLabels);
if (null !== $resolved) {
$participant->veg = $resolved;
}
@@ -240,12 +268,12 @@ class BookingEditDraftMerger
// Rentals (array) - overwrite strategy: user can deselect all
if (true === array_key_exists('rentals', $data) && true === is_array($data['rentals'])) {
$participant->rentals = $this->resolveServiceArray($data['rentals'], $travel->additionalServices);
$participant->rentals = $this->resolveServiceArray($data['rentals'], $travel->additionalServices, $bookingDto, $participantIndex, $droppedServiceLabels);
}
// Rental insurance (single) - merge strategy: only apply if resolves to valid service
if (true === array_key_exists('rentalInsurance', $data) && null !== $data['rentalInsurance']) {
$resolved = $this->resolveService($data['rentalInsurance'], $travel->additionalServices);
$resolved = $this->resolveService($data['rentalInsurance'], $travel->additionalServices, $bookingDto, $participantIndex, $droppedServiceLabels);
if (null !== $resolved) {
$participant->rentalInsurance = $resolved;
$participant->rentalInsuranceSelected = true;
@@ -255,7 +283,7 @@ class BookingEditDraftMerger
// Additional services (array) - overwrite strategy with mandatory service preservation
// User can deselect optional services, but mandatory services from API must be preserved
if (true === array_key_exists('additionalServices', $data) && true === is_array($data['additionalServices'])) {
$resolvedFromDraft = $this->resolveServiceArray($data['additionalServices'], $travel->additionalServices);
$resolvedFromDraft = $this->resolveServiceArray($data['additionalServices'], $travel->additionalServices, $bookingDto, $participantIndex, $droppedServiceLabels);
$participant->additionalServices = $this->preserveMandatoryServices(
$resolvedFromDraft,
$participant->additionalServices,
@@ -268,7 +296,7 @@ class BookingEditDraftMerger
if (true === $travel->transportationServicesMutable) {
// Transportation outbound (single) - merge strategy: only apply if resolves to valid service
if (true === array_key_exists('transportationOutbound', $data) && null !== $data['transportationOutbound']) {
$resolved = $this->resolveService($data['transportationOutbound'], $travel->transportationServices);
$resolved = $this->resolveService($data['transportationOutbound'], $travel->transportationServices, $bookingDto, $participantIndex, $droppedServiceLabels);
if (null !== $resolved) {
$participant->transportationOutbound = $resolved;
}
@@ -276,7 +304,7 @@ class BookingEditDraftMerger
// Transportation inbound (single) - merge strategy: only apply if resolves to valid service
if (true === array_key_exists('transportationInbound', $data) && null !== $data['transportationInbound']) {
$resolved = $this->resolveService($data['transportationInbound'], $travel->transportationServices);
$resolved = $this->resolveService($data['transportationInbound'], $travel->transportationServices, $bookingDto, $participantIndex, $droppedServiceLabels);
if (null !== $resolved) {
$participant->transportationInbound = $resolved;
}
@@ -368,37 +396,84 @@ class BookingEditDraftMerger
/**
* @param array<int, object> $services
* @param list<string> $droppedServiceLabels
*/
private function resolveService(?int $serviceId, array $services): ?object
{
private function resolveService(
?int $serviceId,
array $services,
BookingDto $bookingDto,
int $participantIndex,
array &$droppedServiceLabels,
): ?object {
if (null === $serviceId) {
return null;
}
return $services[$serviceId] ?? null;
$service = $services[$serviceId] ?? null;
if (null !== $service && true === $this->isDroppedFromDraft($service, $bookingDto, $participantIndex)) {
$droppedServiceLabels[] = (string) $service->label;
return null;
}
return $service;
}
/**
* @param array<int, int> $serviceIds
* @param array<int, object> $services
* @param list<string> $droppedServiceLabels
*
* @return array<int, object>
*/
private function resolveServiceArray(array $serviceIds, array $services): array
{
private function resolveServiceArray(
array $serviceIds,
array $services,
BookingDto $bookingDto,
int $participantIndex,
array &$droppedServiceLabels,
): array {
$resolved = [];
$addedIds = [];
foreach ($serviceIds as $serviceId) {
if (isset($services[$serviceId]) && false === isset($addedIds[$serviceId])) {
$resolved[] = $services[$serviceId];
$addedIds[$serviceId] = true;
if (false === isset($services[$serviceId]) || true === isset($addedIds[$serviceId])) {
continue;
}
$service = $services[$serviceId];
if (true === $this->isDroppedFromDraft($service, $bookingDto, $participantIndex)) {
$droppedServiceLabels[] = (string) $service->label;
$addedIds[$serviceId] = true;
continue;
}
$resolved[] = $service;
$addedIds[$serviceId] = true;
}
return $resolved;
}
/**
* Checks whether a drafted selection must not be restored.
*
* A draft can be weeks old. Restoring a selection for a service that has since sold out or
* moved to 'Anfrage' is what makes a stuck booking stuck: the stale choice is re-applied on
* every re-entry and BusPro rejects the whole update again. Services the participant already
* holds are carved out by the availability rule itself, so they still restore.
*/
private function isDroppedFromDraft(object $service, BookingDto $bookingDto, int $participantIndex): bool
{
if (false === $service instanceof Service || null === $service->id) {
return false;
}
return $this->serviceAvailabilityCalculator->isServiceUnavailable($service->id, $bookingDto, $participantIndex);
}
private function resolvePickup(?int $pickupId, Travel $travel): ?object
{
if (null === $pickupId) {
src/Service/BookingEditSubmitGuard.php
+63
View File
@@ -24,9 +24,72 @@ class BookingEditSubmitGuard
{
public function __construct(
private readonly BookingDataProcessor $bookingDataProcessor,
private readonly ServiceAvailabilityCalculator $serviceAvailabilityCalculator,
) {
}
/**
* Reverts selections of services that are no longer bookable.
*
* The read-only state in the participant form is presentation only, so a stale session or
* a replayed POST can still carry a service that has since sold out or moved to 'Anfrage'.
* Sending one makes BusPro reject the entire update - every participant's changes with it -
* so the selection is reverted to what the live booking already holds.
*
* Services the participant already holds are never touched: the availability rule carves
* them out, and withdrawing one would break the Leistung/Teilnehmer counts.
*
* Expects $workingDto->booking to be the fresh booking, as the availability rule reads its
* already-held carve-out from there.
*
* @return list<string> Labels of the services that were reverted, for user feedback
*/
public function revertUnbookableServiceAdditions(BookingDto $workingDto, Booking $freshBooking): array
{
$baselineDto = $this->bookingDataProcessor->createBookingDtoFromBooking($freshBooking, $workingDto->travel, $workingDto->isInternalAgencyBooking());
$revertedLabels = [];
foreach ($workingDto->participants as $index => $participant) {
$baseline = $baselineDto->participants[$index] ?? null;
if (null === $baseline) {
continue;
}
$isBlocked = fn (?Service $service): bool => null !== $service
&& null !== $service->id
&& $this->serviceAvailabilityCalculator->isServiceUnavailable($service->id, $workingDto, $index);
// Multi-selection fields: drop the blocked additions, keep everything else
foreach (['courses', 'additionalServices', 'board', 'rentals'] as $field) {
$kept = [];
foreach ($participant->{$field} as $service) {
if (true === $isBlocked($service)) {
$revertedLabels[] = (string) $service->label;
continue;
}
$kept[] = $service;
}
if (count($kept) !== count($participant->{$field})) {
$participant->{$field} = $kept;
}
}
// Single-selection fields: fall back to what the booking already holds rather than
// clearing, so a required field does not end up empty
foreach (['skiPass', 'veg', 'transportationOutbound', 'transportationInbound', 'parkingService'] as $field) {
if (false === $isBlocked($participant->{$field})) {
continue;
}
$revertedLabels[] = (string) $participant->{$field}->label;
$participant->{$field} = $baseline->{$field};
}
}
return array_values(array_unique($revertedLabels));
}
/**
* Reverts immutable category changes to fresh booking values.
*
src/Service/BookingEditSubmitter.php
+27 -1
View File
@@ -63,11 +63,32 @@ class BookingEditSubmitter
$this->travelDataService->patchMutability($bookingDto->travel, $mutableData);
}
// The guard checks service status and contingent, so it needs the live figures rather
// than whatever was cached when the edit session started.
$availabilities = $this->travelDataService->getAvailabilityData(
$freshBookingData->dateId,
cached: true,
forceRefresh: true
);
if (null !== $availabilities) {
$this->travelDataService->patchAvailabilities($bookingDto->travel, $availabilities);
}
$immutableChangesReverted = $this->submitGuard->reconcileImmutableCategories($bookingDto, $freshBookingData);
if (true === $immutableChangesReverted) {
$unbookableServicesReverted = $this->submitGuard->revertUnbookableServiceAdditions($bookingDto, $freshBookingData);
if (true === $immutableChangesReverted || [] !== $unbookableServicesReverted) {
$this->bookingSessionService->saveBookingDto($request, $bookingDto, BookingDto::MODE_EDIT);
}
if ([] !== $unbookableServicesReverted) {
$this->logger->warning('Reverted selections of services that are no longer bookable', [
'email' => $email,
'booking_id' => $bookingId,
'services' => $unbookableServicesReverted,
]);
}
try {
$response = $this->apiClient->updateBooking($bookingDto, true);
if ($response instanceof Notification) {
@@ -83,6 +104,7 @@ class BookingEditSubmitter
: BookingEditSubmissionResult::STATUS_NOTIFICATION_INFO,
$response->message,
$immutableChangesReverted,
$unbookableServicesReverted,
);
}
@@ -99,6 +121,7 @@ class BookingEditSubmitter
BookingEditSubmissionResult::STATUS_SUCCESS,
null,
$immutableChangesReverted,
$unbookableServicesReverted,
);
}
@@ -113,6 +136,7 @@ class BookingEditSubmitter
BookingEditSubmissionResult::STATUS_UNSUCCESSFUL,
$response->status ?? 'Buchung konnte nicht aktualisiert werden',
$immutableChangesReverted,
$unbookableServicesReverted,
);
} catch (TimeoutException) {
$this->logger->error('Booking update timeout', [
@@ -124,12 +148,14 @@ class BookingEditSubmitter
BookingEditSubmissionResult::STATUS_TIMEOUT,
null,
$immutableChangesReverted,
$unbookableServicesReverted,
);
} catch (ApiClientException) {
return new BookingEditSubmissionResult(
BookingEditSubmissionResult::STATUS_API_CLIENT_ERROR,
null,
$immutableChangesReverted,
$unbookableServicesReverted,
);
}
}
src/Service/ServiceAvailabilityCalculator.php
+94 -2
View File
@@ -55,6 +55,13 @@ class ServiceAvailabilityCalculator
*
* Delegates to isServiceUnavailable() so that both entry points share one rule set.
*
* Never empties a non-empty choice group on the on-request rule alone: on some termine
* every outbound transport option or every ski pass is 'Anfrage', and an empty required
* group makes the travel unbookable - ParticipantEligibilityChecker reads "offered but
* none selectable" as the participant being unable to travel. Sold-out and Buchungsstop
* still empty a group, preserving the pre-existing semantics. Inert in create mode, where
* the on-request rule cannot fire.
*
* @param array<int, Service> $services Array of Service objects to filter
* @param BookingDto $bookingDto The booking data with participant selections
* @param int $participantIndex The index of the participant currently filling the form
@@ -63,7 +70,7 @@ class ServiceAvailabilityCalculator
*/
public function filterAvailableServices(array $services, BookingDto $bookingDto, int $participantIndex): array
{
return array_filter($services, function (Service $service) use ($bookingDto, $participantIndex) {
$filtered = 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;
@@ -71,6 +78,17 @@ class ServiceAvailabilityCalculator
return false === $this->isServiceUnavailable($service->id, $bookingDto, $participantIndex);
});
if ([] !== $filtered || [] === $services) {
return $filtered;
}
return array_filter(
$services,
fn (Service $service): bool => $this->isBlockedByOnRequestStatus($service, $bookingDto)
&& Constants::STATUS_BLOCKED !== $service->status
&& false === $this->isContingentExhausted($service, $bookingDto, $participantIndex)
);
}
/**
@@ -80,6 +98,16 @@ class ServiceAvailabilityCalculator
* contingent is exhausted, either at the API level or through selections made by the
* other participants of the current booking.
*
* In edit mode one further rule applies: a service that is only available on request
* (Anfrage) cannot be *acquired*. The participants of an existing booking are fixed at
* status 'F', and BusPro derives a Leistung's status from the travel data and refuses to
* attach it when the two differ ("Status der Leistung (A) ist unterschiedlich zum Status
* des Teilnehmers (F)"), rejecting the whole update. The create flow has no such problem,
* because the booking status can still move to 'A' there - hence the mode precondition.
*
* The rule is asymmetric: a service the participant already holds in the live booking
* stays available, so it can be kept and re-sent.
*
* @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
@@ -103,11 +131,32 @@ class ServiceAvailabilityCalculator
return false;
}
// A service the participant already holds is always keepable, whatever its state now
if (true === $this->isServiceHeldByParticipant($service, $bookingDto, $participantIndex)) {
return false;
}
// A booking stop blocks the service regardless of its contingent
if (Constants::STATUS_BLOCKED === $service->status) {
return true;
}
// On request cannot be added to a participant whose status is already fixed
if (true === $this->isBlockedByOnRequestStatus($service, $bookingDto)) {
return true;
}
return $this->isContingentExhausted($service, $bookingDto, $participantIndex);
}
/**
* Checks whether a service has no contingent left.
*
* Covers both the API-level figure and the seats taken by the other participants of the
* current booking.
*/
private function isContingentExhausted(Service $service, BookingDto $bookingDto, int $participantIndex): bool
{
// If no availability tracking (null), service is unlimited and available
if (null === $service->available) {
return false;
@@ -121,7 +170,50 @@ class ServiceAvailabilityCalculator
// For services with positive availability, calculate remaining based on booking selections
$remainingAvailability = $this->calculateRemainingAvailability($bookingDto, $participantIndex);
return ($remainingAvailability[$serviceId] ?? $service->available) <= 0;
return ($remainingAvailability[$service->id] ?? $service->available) <= 0;
}
/**
* Checks whether the on-request rule blocks a service.
*
* Only applies in edit mode, and never to services the form would then be unable to
* satisfy: auto-booked services and mandatory services outside the transport categories
* are required by MandatoryAdditionalServicesSelectedValidator, so blocking one makes the
* form unsatisfiable. Mandatory transport legs are deliberately not exempt - there
* pflicht="True" means "pick one of this group" rather than "compulsory", and exempting
* them would exempt the bus legs this rule exists for.
*/
private function isBlockedByOnRequestStatus(Service $service, BookingDto $bookingDto): bool
{
if (BookingDto::MODE_EDIT !== $bookingDto->getMode()) {
return false;
}
if (Constants::STATUS_ON_REQUEST !== $service->status) {
return false;
}
if (true === $service->autoBook) {
return false;
}
return false === ($service->mandatory && Constants::CATEGORY_TRANSPORTATION !== $service->category);
}
/**
* Checks whether the participant already holds the service in the live booking.
*
* Reads the BusPro-side baseline (BookingDto::$booking), never the working selection,
* which already carries whatever the customer just picked. Always false in create mode,
* where there is no booking yet.
*/
private function isServiceHeldByParticipant(Service $service, BookingDto $bookingDto, int $participantIndex): bool
{
if (null === $bookingDto->booking || null === $service->id) {
return false;
}
return $bookingDto->booking->hasServiceForParticipant($participantIndex, $service->id);
}
/**