fix: applicant split in edit mode, drop create-mode agency checks

src/Controller/Booking/Create/Step2ParticipantController.php

@@ -153,12 +153,6 @@ class Step2ParticipantController extends AbstractController
         }
         $bookingDto = $result;
 
-        // Internal agency bookings are pre-populated with a non-travelling applicant already,
-        // so the choice is neither offered nor accepted for them.
-        if ($bookingDto->isInternalAgencyBooking()) {
-            return $this->htmxRedirect($request, $this->generateUrl('app_booking_create_step_2'));
-        }
-
         $this->travelDataService->enrichWithFreshAvailabilities($bookingDto->travel);
 
         // Captured before handleRequest(), which binds the new choice straight onto the DTO.
@@ -227,7 +221,7 @@ class Step2ParticipantController extends AbstractController
         }
         $bookingDto = $result;
 
-        if (true === $bookingDto->applicantTravels || $bookingDto->isInternalAgencyBooking()) {
+        if (true === $bookingDto->applicantTravels) {
             return $this->redirectToRoute('app_booking_create_step_2');
         }
 

src/Form/Model/BookingDto.php

@@ -68,7 +68,7 @@ class BookingDto
      *
      * Create mode only: it is the booking-level choice behind "Ich reise selbst mit".
      * In edit mode the split is derived from the loaded booking instead, see
-     * isFirstParticipantApplicant(). Internal agency bookings never flip this.
+     * isFirstParticipantApplicant().
      */
     public bool $applicantTravels = true;
 
@@ -201,21 +201,23 @@ class BookingDto
      * same thing, which only held because the internal agency was the one case where
      * applicant and participant[0] differed.
      *
-     * - internal agency: always false, their applicant is a staff member and never travels
+     * - create: the booking-level choice. The agency plays no part here: internal agency
-     * - create: the booking-level choice
+     *   bookings are made in the BusPro backend and only ever edited in this application.
-     * - edit: derived from the loaded booking. A null person id on either side counts as
+     * - edit, internal agency: always false, their applicant is a customer who is not the
-     *   "same", so incomplete BPN data keeps today's stricter, locked-down behaviour.
+     *   first traveller
+     * - edit, otherwise: derived from the loaded booking. A null person id on either side
+     *   counts as "same", so incomplete BPN data keeps today's stricter, locked-down behaviour.
      */
     public function isFirstParticipantApplicant(): bool
     {
-        if ($this->isInternalAgencyBooking()) {
-            return false;
-        }
-
         if (self::MODE_CREATE === $this->getMode()) {
             return $this->applicantTravels;
         }
 
+        if ($this->isInternalAgencyBooking()) {
+            return false;
+        }
+
         $applicantPersonId = $this->booking?->applicant?->personId;
         $firstParticipantPersonId = $this->getParticipant(0)?->personId;
 
@@ -289,17 +291,15 @@ class BookingDto
     }
 
     /**
-     * Requires a complete address from participant[0] only where they supply the anmelder block.
+     * Requires a complete address from participant[0] only where they are the applicant.
      *
      * BusPro does not require an anschrift from an ordinary participant, so on a split booking
-     * the traveller at index 0 is not asked for one. Keyed on $applicantTravels rather than
+     * the traveller at index 0 is not asked for one.
-     * isFirstParticipantApplicant() so internal agency bookings, whose payload still builds the
-     * anmelder from participant[0], keep asking for it.
      */
     #[Assert\Callback(groups: ['strict_required'])]
     public function validateFirstParticipantAddress(ExecutionContextInterface $context): void
     {
-        if (false === $this->applicantTravels) {
+        if (false === $this->isFirstParticipantApplicant()) {
             return;
         }
 

src/Form/Model/ParticipantEditDto.php

@@ -105,7 +105,7 @@ class ParticipantEditDto
             return;
         }
 
-        if (false === $this->bookingContext->applicantTravels) {
+        if (false === $this->bookingContext->isFirstParticipantApplicant()) {
             return;
         }
 

src/Form/Service/Condition/FirstParticipantIsApplicantCondition.php

@@ -8,22 +8,16 @@ use App\Form\Model\BookingDto;
 use App\Form\Service\Contract\FieldConditionInterface;
 
 /**
- * Condition for "participant[0] is the person the <anmelder> block is built from".
+ * Condition for "participant[0] is the booking's applicant".
  *
  * Use it where a field is only mandatory because BusPro asks it of the Anmelder rather than of
  * an ordinary participant, such as mobile and address (an empty anschrift passes for a
  * participant, measured on 2036).
  *
- * It deliberately tracks $applicantTravels, the same condition
- * BookingPayloadBuilder::buildCreatePayload() keys its $applicantSource on. That keeps internal
- * agency bookings unchanged: their applicant never travels, but their payload still takes the
- * anmelder from participant[0], so participant[0] still owes the Anmelder's fields. This is why
- * it is not isFirstParticipantApplicant(), which answers false for them.
- *
  * FirstParticipantCondition remains the right choice where plain index 0 is meant, such as
  * showing a field on the first card regardless of who that person is.
  */
-class FirstParticipantSuppliesApplicantDataCondition implements FieldConditionInterface
+class FirstParticipantIsApplicantCondition implements FieldConditionInterface
 {
     /**
      * @param BookingDto           $bookingDto       The current booking data
@@ -32,7 +26,7 @@ class FirstParticipantSuppliesApplicantDataCondition implements FieldConditionIn
      */
     public function evaluate(BookingDto $bookingDto, int $participantIndex, array $formData): bool
     {
-        return 0 === $participantIndex && $bookingDto->applicantTravels;
+        return 0 === $participantIndex && $bookingDto->isFirstParticipantApplicant();
     }
 
     /**
@@ -45,6 +39,6 @@ class FirstParticipantSuppliesApplicantDataCondition implements FieldConditionIn
 
     public function getDescription(): string
     {
-        return 'Checks if participant[0] supplies the anmelder block';
+        return 'Checks if participant[0] is the applicant';
     }
 }

src/Form/Service/Condition/FirstParticipantReadOnlyCondition.php

@@ -12,14 +12,14 @@ use App\Form\Service\Contract\FieldConditionInterface;
  *
  * It fires only where participant[0] IS the booking's applicant, which
  * BookingDto::isFirstParticipantApplicant() decides: the booking-level choice in create mode,
- * and anmelder vs. participant[0] person id in edit mode. Internal agency bookings answer
+ * and anmelder vs. participant[0] person id in edit mode, where internal agency bookings
- * false there, which preserves the behaviour this class used to spell out itself.
+ * always answer false.
  *
  * Business Rules:
  * - CREATE mode (anonymous user): First participant data is editable (user must fill it in)
  * - CREATE mode (logged-in, participant[0] is the applicant): data is prepopulated, read-only
- * - CREATE mode (applicant does not travel, or internal agency): participant[0] is an ordinary
+ * - CREATE mode (applicant does not travel): participant[0] is an ordinary traveller, data is
- *   traveller, data is editable
+ *   editable
  * - EDIT mode (participant[0] is the applicant): always read-only
  * - EDIT mode (split booking, or internal agency): participant[0] ≠ applicant, editable here
  *   (PersonalDataMutabilityCondition still applies separately)

src/Form/Service/CreateFieldStateProvider.php

@@ -16,8 +16,8 @@ use App\Form\Service\Condition\FamilyInsuranceActiveCondition;
 use App\Form\Service\Condition\FieldValueCondition;
 use App\Form\Service\Condition\FinalBookingOnlyCondition;
 use App\Form\Service\Condition\FirstParticipantCondition;
+use App\Form\Service\Condition\FirstParticipantIsApplicantCondition;
 use App\Form\Service\Condition\FirstParticipantReadOnlyCondition;
-use App\Form\Service\Condition\FirstParticipantSuppliesApplicantDataCondition;
 use App\Form\Service\Condition\MultipleParticipantsCondition;
 use App\Form\Service\Condition\RentalInsuranceAvailableCondition;
 use App\Form\Service\Condition\RentalSelectionCondition;
@@ -88,7 +88,7 @@ class CreateFieldStateProvider extends AbstractFieldStateProvider
         );
 
         // First participant read-only condition
-        // Render personal data fields as static text for first participant in non-internal agency bookings
+        // Render personal data fields as static text for the first participant where they are the applicant
         // when user is logged in (data is prepopulated from BPN account)
         $firstParticipantReadOnlyCondition = new FirstParticipantReadOnlyCondition();
 
@@ -122,7 +122,7 @@ class CreateFieldStateProvider extends AbstractFieldStateProvider
         // applicant (guest bookings) — BusPro does not ask a telefonmobil of an ordinary participant.
         $this->fieldStateConditions['mobile'] = [
             'static_text' => $firstParticipantReadOnlyCondition,
-            'required' => new FirstParticipantSuppliesApplicantDataCondition(),
+            'required' => new FirstParticipantIsApplicantCondition(),
         ];
 
         // Address subfields - must render as static text to prevent creating duplicate BPN records
@@ -319,7 +319,7 @@ class CreateFieldStateProvider extends AbstractFieldStateProvider
         // ordinary participant. Also rendered as static text for the applicant when read-only.
         $this->fieldStateConditions['address'] = [
             'static_text' => $firstParticipantReadOnlyCondition,
-            'required' => new FirstParticipantSuppliesApplicantDataCondition(),
+            'required' => new FirstParticipantIsApplicantCondition(),
         ];
 
         // Room assignment field:

src/Form/Service/ParticipantEmailFieldHandler.php

@@ -14,7 +14,8 @@ use App\Form\Service\Abstract\AbstractParticipantFieldHandler;
  * BusPro requires a Mail-Adresse for every participant once a room is present and rejects the
  * step-3 Anfrage with 650 otherwise, children included. It does accept an address identical to
  * the Anmelder's, so children inherit the applicant's rather than being asked for one they
- * typically do not have. The value stays editable and stays mandatory.
+ * typically do not have. The value stays editable and stays mandatory. This happens in the
+ * create flow only; in edit mode the handler just stores what was submitted.
  *
  * The copy only ever fills a blank, so an address somebody typed is never overwritten. A later
  * correction to the applicant's own address therefore does not propagate to children who already
@@ -65,6 +66,13 @@ class ParticipantEmailFieldHandler extends AbstractParticipantFieldHandler
      */
     public function applyApplicantEmail(BookingDto $bookingDto, ParticipantDto $participant): void
     {
+        // Create mode only. An existing booking already carries whatever BusPro accepted, a
+        // blank may be intended where personal data is optional, and participant[0] is not
+        // reliably the applicant there.
+        if (BookingDto::MODE_CREATE !== $bookingDto->getMode()) {
+            return;
+        }
+
         if (false === $participant->isChild()) {
             return;
         }

src/Service/ParticipantDataPrefiller.php

@@ -132,9 +132,6 @@ class ParticipantDataPrefiller
         return self::TOKEN === $participant->lastName;
     }
 
-    /**
-     * Fills the participant DTO with generated dummy personal data.
-     */
     /**
      * Fills the applicant DTO with generated dummy contact data.
      *
@@ -162,6 +159,9 @@ class ParticipantDataPrefiller
         $applicant->address = $address;
     }
 
+    /**
+     * Fills the participant DTO with generated dummy personal data.
+     */
     public function fillDummyParticipant(ParticipantDto $participant, int $participantIndex): void
     {
         $participantNumber = $participantIndex + 1;

templates/booking/create/step_2.html.twig

@@ -54,27 +54,25 @@
                         {% endif %}
 
                         {% set bookingDto = bookingCreateContext.bookingDto %}
-                        {% if not bookingDto.isInternalAgencyBooking() %}
+                        {# No form_start here: this sits inside the step 2 form and nested
-                            {# No form_start here: this sits inside the step 2 form and nested
+                           form tags are invalid. htmx posts the inputs enclosed below,
-                               form tags are invalid. htmx posts the inputs enclosed below,
+                           including the CSRF token, to the toggle route. #}
-                               including the CSRF token, to the toggle route. #}
+                        <div class="mb-6 rounded border border-gray-200 bg-white p-4"
-                            <div class="mb-6 rounded border border-gray-200 bg-white p-4"
+                             hx-post="{{ path('app_booking_create_step_2_applicant_toggle') }}"
-                                 hx-post="{{ path('app_booking_create_step_2_applicant_toggle') }}"
+                             hx-trigger="change"
-                                 hx-trigger="change"
+                             hx-include="this"
-                                 hx-include="this"
+                             {{ qa_attribute('applicant-travels') }}>
-                                 {{ qa_attribute('applicant-travels') }}>
+                            {{ form_row(applicantTravelsForm.applicantTravels) }}
-                                {{ form_row(applicantTravelsForm.applicantTravels) }}
+                            {{ form_rest(applicantTravelsForm) }}
-                                {{ form_rest(applicantTravelsForm) }}
 
-                                {% if not bookingDto.applicantTravels %}
+                            {% if not bookingDto.applicantTravels %}
-                                    {% include 'booking/create/_applicant_card.html.twig' with {
+                                {% include 'booking/create/_applicant_card.html.twig' with {
-                                        'applicant': bookingDto.applicant,
+                                    'applicant': bookingDto.applicant,
-                                        'isSubmitted': bookingCreateContext.isSubmitted,
+                                    'isSubmitted': bookingCreateContext.isSubmitted,
-                                        'applicantErrors': applicantErrors
+                                    'applicantErrors': applicantErrors
-                                    } %}
+                                } %}
-                                {% endif %}
+                            {% endif %}
-                            </div>
+                        </div>
-                        {% endif %}
 
                         <div class="divide-y divide-gray-200">
                             {% for cardData in bookingCreateContext.cardsData %}

tests/Form/Model/BookingDtoTest.php

@@ -148,14 +148,25 @@ class BookingDtoTest extends TestCase
         $this->assertFalse($dto->isFirstParticipantApplicant());
     }
 
-    public function testInternalAgencyBookingNeverHasTheFirstParticipantAsApplicant(): void
+    public function testInternalAgencyBookingNeverHasTheFirstParticipantAsApplicantInEditMode(): void
     {
-        $dto = $this->createCreateModeDto();
+        $dto = $this->createEditModeDto(100, 100);
         $dto->agencyCode = AgencyLoader::INTERNAL_AGENCY_CODE;
 
         $this->assertFalse(
             $dto->isFirstParticipantApplicant(),
-            'Their applicant is a staff member who never travels'
+            'Their applicant is a customer who is not the first traveller, whatever the ids say'
+        );
+    }
+
+    public function testAgencyCodeDoesNotDecideAnythingInCreateMode(): void
+    {
+        $dto = $this->createCreateModeDto();
+        $dto->agencyCode = AgencyLoader::INTERNAL_AGENCY_CODE;
+
+        $this->assertTrue(
+            $dto->isFirstParticipantApplicant(),
+            'Internal agency bookings are never created here, so the code is only passed on'
         );
     }
 
@@ -358,6 +369,42 @@ class BookingDtoTest extends TestCase
         $this->assertContains('Die anmeldende Person muss volljährig sein.', $messages);
     }
 
+    public function testEditModeDoesNotAskASplitBookingsFirstParticipantForAnAddress(): void
+    {
+        $dto = $this->createEditModeDto(100, 200);
+        $dto->participants[0]->index = 0;
+        $dto->participants[0]->address = null;
+
+        $this->assertSame([], $this->collectFirstParticipantAddressPaths($dto));
+    }
+
+    public function testEditModeStillAsksTheApplicantForAnAddress(): void
+    {
+        $dto = $this->createEditModeDto(100, 100);
+        $dto->participants[0]->index = 0;
+        $dto->participants[0]->address = null;
+
+        $this->assertSame(['participants[0].address'], $this->collectFirstParticipantAddressPaths($dto));
+    }
+
+    /**
+     * @return list<string>
+     */
+    private function collectFirstParticipantAddressPaths(BookingDto $dto): array
+    {
+        $validator = Validation::createValidatorBuilder()->enableAttributeMapping()->getValidator();
+        $violations = $validator->validate($dto, null, ['booking_edit', 'strict_required']);
+
+        $paths = [];
+        foreach ($violations as $violation) {
+            if (str_starts_with($violation->getPropertyPath(), 'participants[0].address')) {
+                $paths[] = $violation->getPropertyPath();
+            }
+        }
+
+        return $paths;
+    }
+
     private function createAddress(): Address
     {
         $address = new Address();

tests/Form/Model/ParticipantEditDtoTest.php

@@ -8,6 +8,7 @@ use App\BusProNet\Constants;
 use App\BusProNet\Model\Address;
 use App\BusProNet\Model\Booking;
 use App\BusProNet\Model\Insurance;
+use App\BusProNet\Model\PersonalData;
 use App\BusProNet\Model\Service;
 use App\BusProNet\Model\Travel;
 use App\BusProNet\XmlLoader\AgencyLoader;
@@ -703,7 +704,6 @@ class ParticipantEditDtoTest extends TestCase
 
         $bookingDto = new BookingDto($travel, 1);
         $bookingDto->booking = new Booking();
-        $bookingDto->agencyCode = AgencyLoader::INTERNAL_AGENCY_CODE;
 
         $participant0 = $this->createAdultParticipant('[email protected]');
         $participant0->index = 0;
@@ -740,6 +740,38 @@ class ParticipantEditDtoTest extends TestCase
         $this->assertCount(0, $addressViolations1, 'Non-applicant address should not be required in edit submission');
     }
 
+    public function testEditSubmissionDoesNotAskASplitBookingsFirstParticipantForAnAddress(): void
+    {
+        $travel = $this->createTravelOfferingSkiPasses();
+
+        $applicant = new PersonalData();
+        $applicant->personId = 100;
+
+        $bookingDto = new BookingDto($travel, 1);
+        $bookingDto->booking = new Booking();
+        $bookingDto->booking->applicant = $applicant;
+
+        $participant0 = $this->createAdultParticipant('[email protected]');
+        $participant0->index = 0;
+        $participant0->personId = 200;
+        $participant0->address = null;
+
+        $bookingDto->participants = [$participant0];
+
+        $wrapper0 = new ParticipantEditDto(
+            participant: $bookingDto->participants[0],
+            bookingContext: $bookingDto,
+        );
+
+        $violations0 = $this->validator->validate($wrapper0, null, ['booking_edit', 'strict_required']);
+        $addressViolations0 = array_filter(
+            iterator_to_array($violations0),
+            fn ($v) => 'address' === $v->getPropertyPath() || 'participant.address' === $v->getPropertyPath()
+        );
+
+        $this->assertCount(0, $addressViolations0, 'A traveller who is not the applicant owes no address');
+    }
+
     public function testSkiPassAgeCalculatedAtTravelDate(): void
     {
         $travel = $this->createTravelOfferingSkiPasses();

tests/Form/Service/ParticipantEmailFieldHandlerTest.php

@@ -4,7 +4,9 @@ declare(strict_types=1);
 
 namespace App\Tests\Form\Service;
 
+use App\BusProNet\Model\Booking;
 use App\BusProNet\Model\Travel;
+use App\BusProNet\XmlLoader\AgencyLoader;
 use App\Form\Model\BookingDto;
 use App\Form\Model\ParticipantDto;
 use App\Form\Service\ParticipantDateOfBirthFieldHandler;
@@ -79,6 +81,30 @@ class ParticipantEmailFieldHandlerTest extends TestCase
         $this->assertSame('[email protected]', $bookingDto->participants[0]->email);
     }
 
+    public function testEditModeNeverPreFills(): void
+    {
+        $bookingDto = $this->createBookingDto();
+        $bookingDto->booking = new Booking();
+
+        $this->handler->processField(['email' => ''], $bookingDto, 1);
+
+        $this->assertNull(
+            $bookingDto->participants[1]->email,
+            'participant[0] is not reliably the applicant in edit mode, and a blank may be intended'
+        );
+    }
+
+    public function testEditModeLeavesAnInternalAgencyChildWithoutEmail(): void
+    {
+        $bookingDto = $this->createBookingDto();
+        $bookingDto->booking = new Booking();
+        $bookingDto->agencyCode = AgencyLoader::INTERNAL_AGENCY_CODE;
+
+        $this->handler->processField(['email' => ''], $bookingDto, 1);
+
+        $this->assertNull($bookingDto->participants[1]->email);
+    }
+
     public function testHandlerRunsAfterDateOfBirth(): void
     {
         $this->assertSame(