From d50995a21c43b24bf631c03146703ce0af252406 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Fromme?= Date: Mon, 9 Nov 2026 18:00:47 +0100 Subject: [PATCH] fix: pass BusPro room assignments through on edit --- .../DataProcessor/BookingDataProcessor.php | 1 + .../ParticipantServiceProcessor.php | 39 +++--- .../ParticipantAssignedRoomFieldHandler.php | 11 ++ src/Service/BookingEditDraftMerger.php | 21 ++-- .../BookingDataProcessorTest.php | 114 ++++++++++++++++++ ...articipantAssignedRoomFieldHandlerTest.php | 7 +- ...okingEditDraftMergerRoomAssignmentTest.php | 15 ++- 7 files changed, 163 insertions(+), 45 deletions(-) diff --git a/src/BusProNet/DataProcessor/BookingDataProcessor.php b/src/BusProNet/DataProcessor/BookingDataProcessor.php index ddb2f621..adad9106 100644 --- a/src/BusProNet/DataProcessor/BookingDataProcessor.php +++ b/src/BusProNet/DataProcessor/BookingDataProcessor.php @@ -341,6 +341,7 @@ class BookingDataProcessor $this->serviceProcessor->processParticipantServices($participant, $bookingData, $travelData); } + $this->serviceProcessor->removeCanceledParticipantsFromRooms($bookingData); $this->serviceProcessor->removeUnusedServices($bookingData); $this->personalDataSynchronizer->updateParticipantPersonalData( $formData->participants, diff --git a/src/BusProNet/DataProcessor/ParticipantServiceProcessor.php b/src/BusProNet/DataProcessor/ParticipantServiceProcessor.php index 9c2a6def..01957eb8 100644 --- a/src/BusProNet/DataProcessor/ParticipantServiceProcessor.php +++ b/src/BusProNet/DataProcessor/ParticipantServiceProcessor.php @@ -14,8 +14,9 @@ use Psr\Log\LoggerInterface; * Processes participant service selections for booking updates. * * Handles the assignment and mapping of services (additional, transportation, - * pickup, room, insurance) from participant DTOs to the booking model during - * the update request workflow. + * pickup, insurance) from participant DTOs to the booking model during the + * update request workflow. Rooms are not taken from the DTOs: they cannot be + * changed in edit mode, so the booking's own room assignments are sent back. */ class ParticipantServiceProcessor { @@ -28,7 +29,7 @@ class ParticipantServiceProcessor * Resets all existing participant-to-service mappings to start with a clean slate. * * This ensures that service assignments are rebuilt from scratch based on current form selections. - * Includes resetting room mappings to allow room reassignments during edit. + * Room mappings are left alone, see removeCanceledParticipantsFromRooms(). * * @param Booking $bookingData The booking data object containing services to reset */ @@ -39,7 +40,6 @@ class ParticipantServiceProcessor ...$bookingData->transportationServices, ...$bookingData->pickups, ...$bookingData->dropOffs, - ...$bookingData->rooms, ...$bookingData->insurances, ]; @@ -52,7 +52,7 @@ class ParticipantServiceProcessor * Processes all services for a single participant. * * This orchestrator method handles the complete service assignment workflow for one participant, - * including additional services, transportation services, pickup locations, and room assignments. + * including additional services, transportation services, pickup locations, and insurance. * * @param ParticipantDto $participant The participant data from the form * @param Booking $bookingData The booking data object to update @@ -68,7 +68,6 @@ class ParticipantServiceProcessor $this->processTransportationServices($participant, $bookingData, $travelData); $this->processPickupLocations($participant, $bookingData); $this->processDropOffLocations($participant, $bookingData); - $this->processRoomAssignment($participant, $bookingData); $this->processInsurance($participant, $bookingData, $travelData); } @@ -271,27 +270,23 @@ class ParticipantServiceProcessor } /** - * Processes room assignment for a participant. + * Removes canceled participants from the booking's room assignments. * - * Maps the participant to their assigned room. Allows room reassignment during - * booking edits while maintaining the constraint that participants can only be - * assigned to room types that have already been booked. + * Room assignments cannot be changed in edit mode, so they are sent back as BusPro reports + * them on the booking reloaded at submit, together with its anzahl. Rebuilding them from the + * session would undo any reassignment made in BusPro while the edit was open, and send a + * zuordnung that no longer matches anzahl. Canceled participants are left out, as for every + * other service. * - * @param ParticipantDto $participant The participant data from the form - * @param Booking $bookingData The booking data object to update + * @param Booking $bookingData The booking data object to update */ - private function processRoomAssignment(ParticipantDto $participant, Booking $bookingData): void + public function removeCanceledParticipantsFromRooms(Booking $bookingData): void { - if (null === $participant->assignedRoomId) { - return; - } - - // Find the room in the existing booking by ID foreach ($bookingData->rooms as $room) { - if ($room->id === $participant->assignedRoomId) { - $room->mapping[] = $participant->index; - break; - } + $room->mapping = array_values(array_filter( + $room->mapping, + fn (int $index) => 'S' !== ($bookingData->participantsStatus[$index] ?? null) + )); } } diff --git a/src/Form/Service/ParticipantAssignedRoomFieldHandler.php b/src/Form/Service/ParticipantAssignedRoomFieldHandler.php index 8d1f592b..d213b726 100644 --- a/src/Form/Service/ParticipantAssignedRoomFieldHandler.php +++ b/src/Form/Service/ParticipantAssignedRoomFieldHandler.php @@ -48,6 +48,17 @@ class ParticipantAssignedRoomFieldHandler extends AbstractParticipantFieldHandle return 'assignedRoomId'; } + /** + * Skips edit mode, where room assignments cannot be changed. + * + * The field is static text there, so a submitted value can only come from a replayed or + * hand-crafted POST. The edit request sends the booking's own assignments anyway. + */ + public function shouldProcess(array $submittedData, string $mode, int $participantIndex): bool + { + return BookingDto::MODE_EDIT !== $mode && parent::shouldProcess($submittedData, $mode, $participantIndex); + } + /** * Processes the assignedRoomId field for a specific participant. * diff --git a/src/Service/BookingEditDraftMerger.php b/src/Service/BookingEditDraftMerger.php index 990b1920..e48db082 100644 --- a/src/Service/BookingEditDraftMerger.php +++ b/src/Service/BookingEditDraftMerger.php @@ -63,7 +63,7 @@ class BookingEditDraftMerger // Room assignment if (true === isset($data['roomAssignment']) && true === is_array($data['roomAssignment'])) { - $this->applyRoomAssignment($participant, $data['roomAssignment'], $travel); + $this->applyRoomAssignment($participant, $data['roomAssignment']); } // License plate @@ -177,26 +177,19 @@ class BookingEditDraftMerger } /** - * Applies the drafted room assignment to the participant. + * Applies the drafted room remarks to the participant. * - * The room id uses the same merge strategy as single-select services: it is only - * applied when it resolves to a room of the current travel. A draft may therefore move - * a participant to another room, but never unassign one. Edit mode renders the room as - * static text and never submits assignedRoomId, so a null in the draft is only ever an - * artefact of a snapshot taken before BusPro reported the assignment - replaying it - * would drop the participant's room, both in the UI and in the outbound zuordnung. + * The drafted assignedRoomId is ignored. Edit mode renders the room as static text and + * never submits it, so the draft only holds what BusPro reported when it was saved, and + * replaying it would undo a reassignment the office made since. The room always comes + * from the booking as loaded. * * remarksRoom keeps overwrite semantics: it is free text the user can deliberately clear. * * @param array $data */ - private function applyRoomAssignment(ParticipantDto $participant, array $data, Travel $travel): void + private function applyRoomAssignment(ParticipantDto $participant, array $data): void { - $assignedRoomId = $data['assignedRoomId'] ?? null; - if (null !== $assignedRoomId && null !== $travel->getRoomById((int) $assignedRoomId)) { - $participant->assignedRoomId = (int) $assignedRoomId; - } - if (true === array_key_exists('remarksRoom', $data)) { $participant->remarksRoom = $data['remarksRoom']; } diff --git a/tests/BusProNet/DataProcessor/BookingDataProcessorTest.php b/tests/BusProNet/DataProcessor/BookingDataProcessorTest.php index 636af79a..5d8eb96f 100644 --- a/tests/BusProNet/DataProcessor/BookingDataProcessorTest.php +++ b/tests/BusProNet/DataProcessor/BookingDataProcessorTest.php @@ -342,6 +342,66 @@ class BookingDataProcessorTest extends TestCase $this->assertEquals('07.01.2024', $room['@abreise']); } + public function testUpdatePayloadSendsBookedRoomsUnchanged(): void + { + $formData = $this->createFormDataWithBookedRooms(); + + $result = $this->processor->createUpdateRequestPayload($formData); + + $this->assertSame( + [95 => ['1', '1,2,3,4'], 3 => ['3', '5,6,7,8,9,10']], + $this->roomsInPayload($result) + ); + } + + public function testUpdatePayloadDropsCanceledParticipantsFromRooms(): void + { + $formData = $this->createFormDataWithBookedRooms(); + $formData->participants[5]->status = 'S'; + $formData->booking->participantsStatus[5] = 'S'; + + $result = $this->processor->createUpdateRequestPayload($formData); + + $this->assertSame( + [95 => ['1', '1,2,3,4'], 3 => ['3', '5,7,8,9,10']], + $this->roomsInPayload($result) + ); + } + + public function testUpdatePayloadKeepsRoomChangesMadeInBusProDuringTheEdit(): void + { + $formData = $this->createFormDataWithBookedRooms(); + + // The office moved participant 4 into the doubles after the edit session was loaded + $formData->booking->rooms[95]->mapping = [0, 1, 2]; + $formData->booking->rooms[3]->totalCount = 4; + $formData->booking->rooms[3]->mapping = [3, 4, 5, 6, 7, 8, 9]; + + $result = $this->processor->createUpdateRequestPayload($formData); + + $this->assertSame( + [95 => ['1', '1,2,3'], 3 => ['4', '4,5,6,7,8,9,10']], + $this->roomsInPayload($result) + ); + } + + public function testUpdatePayloadKeepsARoomTypeAddedInBusProDuringTheEdit(): void + { + $formData = $this->createFormDataWithBookedRooms(); + + // The office moved participants 1 and 2 out of the 4-bed room into a new double + $formData->booking->rooms[95]->mapping = [2, 3]; + $formData->booking->rooms[95]->totalCount = 1; + $formData->booking->rooms[7] = $this->createBookedRoom(7, 'DZ', 1, [0, 1]); + + $result = $this->processor->createUpdateRequestPayload($formData); + + $this->assertSame( + [95 => ['1', '3,4'], 3 => ['3', '5,6,7,8,9,10'], 7 => ['1', '1,2']], + $this->roomsInPayload($result) + ); + } + public function testCommunicationObjectCreation(): void { $formData = $this->createFormDataWithoutExistingCommunication(); @@ -526,6 +586,60 @@ class BookingDataProcessorTest extends TestCase return $formData; } + /** + * Ten participants as BusPro reports them: four in one 4-bed room, six in three doubles. + */ + private function createFormDataWithBookedRooms(): BookingDto + { + $bookingDto = new BookingDto($this->createMockTravel(), 1); + $bookingDto->booking = $this->createMockBooking(); + $bookingDto->booking->participants = []; + $bookingDto->booking->participantsStatus = []; + $bookingDto->participants = []; + + for ($index = 0; $index < 10; ++$index) { + $bookingDto->booking->participants[$index] = $this->createMockPersonalData("Participant{$index}"); + $bookingDto->booking->participantsStatus[$index] = 'F'; + $bookingDto->participants[$index] = $this->createMockParticipantDto($index, 'F'); + $bookingDto->participants[$index]->assignedRoomId = $index < 4 ? 95 : 3; + } + + $bookingDto->booking->rooms = [ + 95 => $this->createBookedRoom(95, '4er', 1, [0, 1, 2, 3]), + 3 => $this->createBookedRoom(3, 'DZ', 3, [4, 5, 6, 7, 8, 9]), + ]; + + return $bookingDto; + } + + /** @param list $mapping */ + private function createBookedRoom(int $id, string $category, int $totalCount, array $mapping): Room + { + $room = new Room(); + $room->id = $id; + $room->category = $category; + $room->boardId = 2; + $room->totalCount = $totalCount; + $room->mapping = $mapping; + + return $room; + } + + /** + * @param array $payload + * + * @return array Room id => [anzahl, zuordnung] + */ + private function roomsInPayload(array $payload): array + { + $rooms = []; + foreach ($payload['ferienzielunterbringungen']['ferienzielunterbringung'] as $room) { + $rooms[$room['@idzimmer']] = [(string) $room['@anzahl'], $room['@zuordnung']]; + } + + return $rooms; + } + private function createFormDataWithoutExistingCommunication(): BookingDto { $formData = $this->createCompleteFormData(); diff --git a/tests/Form/Service/ParticipantAssignedRoomFieldHandlerTest.php b/tests/Form/Service/ParticipantAssignedRoomFieldHandlerTest.php index 907cc458..4ddd57ff 100644 --- a/tests/Form/Service/ParticipantAssignedRoomFieldHandlerTest.php +++ b/tests/Form/Service/ParticipantAssignedRoomFieldHandlerTest.php @@ -33,7 +33,12 @@ class ParticipantAssignedRoomFieldHandlerTest extends TestCase public function testShouldProcessReturnsTrueWhenFieldPresent(): void { $this->assertTrue($this->handler->shouldProcess(['assignedRoomId' => '1'], BookingDto::MODE_CREATE, 0)); - $this->assertTrue($this->handler->shouldProcess(['assignedRoomId' => ''], BookingDto::MODE_EDIT, 5)); + $this->assertTrue($this->handler->shouldProcess(['assignedRoomId' => ''], BookingDto::MODE_CREATE, 5)); + } + + public function testShouldProcessReturnsFalseInEditMode(): void + { + $this->assertFalse($this->handler->shouldProcess(['assignedRoomId' => '1'], BookingDto::MODE_EDIT, 0)); } public function testShouldProcessReturnsFalseWhenFieldNotPresent(): void diff --git a/tests/Service/BookingEditDraftMergerRoomAssignmentTest.php b/tests/Service/BookingEditDraftMergerRoomAssignmentTest.php index ab4d1125..e30b71b3 100644 --- a/tests/Service/BookingEditDraftMergerRoomAssignmentTest.php +++ b/tests/Service/BookingEditDraftMergerRoomAssignmentTest.php @@ -13,13 +13,12 @@ use App\Service\ServiceAvailabilityCalculator; use PHPUnit\Framework\TestCase; /** - * Tests that draft restoration never unassigns a room. + * Tests that draft restoration never changes a room assignment. * - * Edit mode renders the room as static text and never submits assignedRoomId, so a null in - * the draft payload is always an artefact of a snapshot taken before BusPro reported the - * assignment - typically for seats added to a group booking shortly before the draft was - * saved. Replaying such a null would strip the room from the UI and from the outbound - * zuordnung, so the room id uses the same merge strategy as single-select services. + * Edit mode renders the room as static text and never submits assignedRoomId, so the draft + * only holds what BusPro reported when it was saved: a null for seats added to a group booking + * shortly before, or a room the office has since reassigned. The room always comes from the + * booking as loaded; only the free-text remarks are restored. */ class BookingEditDraftMergerRoomAssignmentTest extends TestCase { @@ -43,7 +42,7 @@ class BookingEditDraftMergerRoomAssignmentTest extends TestCase $this->assertSame(74, $participant->assignedRoomId); } - public function testDraftAppliesRoomThatResolvesAgainstTravel(): void + public function testDraftDoesNotMoveParticipantToAnotherRoom(): void { $participant = new ParticipantDto(); $participant->mutable = true; @@ -56,7 +55,7 @@ class BookingEditDraftMergerRoomAssignmentTest extends TestCase $this->apply($travel, $participant, ['assignedRoomId' => 75]); - $this->assertSame(75, $participant->assignedRoomId); + $this->assertSame(74, $participant->assignedRoomId); } public function testDraftSkipsRoomThatIsUnknownToTravel(): void