fix: report an unresolvable hotel instead of failing on it

src/Controller/Administrative/System/Destination/DestinationTrait.php

@@ -7,27 +7,51 @@ use App\Model\DestinationDto;
 
 trait DestinationTrait
 {
+    /**
+     * Resolves the chosen boarding points against the base data.
+     *
+     * A stop the base data cannot resolve is kept with null city and street rather than dropped,
+     * the same choice App\Command\MyEpImportCommand makes for the export's own unresolvable ids:
+     * it is still a real departure point, and App\Service\Common\PickupOptions falls back to
+     * "Zustieg {busProId}" for one with no town. Dropping it would hide a departure the coach
+     * actually makes.
+     *
+     * The id comes from the form rather than from the resolved stop, so it survives a lookup
+     * that does not.
+     */
     private function mapPickups(DestinationDto $destinationDto): array
     {
         $pickups = [];
 
         foreach ($destinationDto->getPickups() as $pickupId) {
             $pickup = $this->pickupDataProvider->get($pickupId);
+
             $pickups[] = [
-                'busProId' => $pickup->getBusProId(),
-                'city' => $pickup->getCity(),
-                'street' => $pickup->getStreet(),
-                'time' => $pickup->getTime(),
+                'busProId' => (int) $pickupId,
+                'city' => $pickup?->getCity(),
+                'street' => $pickup?->getStreet(),
+                'time' => $pickup?->getTime(),
             ];
         }
 
         return $pickups;
     }
 
+    /**
+     * @throws \LogicException when the hotel does not resolve
+     */
     private function updateEntity(Destination $destination, DestinationDto $destinationDto): void
     {
-        $hotel = $this->hotelDataProvider->get($destinationDto->getHotelBusProId());
-        $pickups = $this->mapPickups($destinationDto);
+        $hotelBusProId = $destinationDto->getHotelBusProId();
+
+        // App\Validator\Constraints\ResolvableHotel is what reports this to the operator, on the
+        // field and in German, and every caller here runs behind $form->isValid(). This is the
+        // second line only: a caller that skips validation would otherwise write a destination
+        // with no house name, no cost unit and no country, and nothing reads those back until an
+        // invoice is stamped. Failing loudly beats a half built row.
+        if (null === $hotel = $this->hotelDataProvider->get((int) $hotelBusProId)) {
+            throw new \LogicException(sprintf('Refusing to write destination %s: hotel %s does not resolve against the MyE&P base data', $destination->getId() ?? 'new', var_export($hotelBusProId, true)));
+        }
 
         $destination
             ->setProduct($destinationDto->getProduct())
@@ -37,7 +61,7 @@ trait DestinationTrait
             ->setHotelCode($hotel->getCode())
             ->setHotelBusProId($hotel->getBusProId())
             ->setCountry($destinationDto->getCountry())
-            ->setPickups($pickups)
+            ->setPickups($this->mapPickups($destinationDto))
             ->setCostUnit($hotel->getCostUnit())
         ;
     }

src/Model/DestinationDto.php

@@ -3,6 +3,7 @@
 namespace App\Model;
 
 use App\Entity\Destination;
+use App\Validator\Constraints as AppAssert;
 use Symfony\Component\Validator\Constraints as Assert;
 
 class DestinationDto
@@ -36,6 +37,7 @@ class DestinationDto
     private ?\DateTimeImmutable $dateTo = null;
 
     #[Assert\NotBlank(message: 'Bitte angeben')]
+    #[AppAssert\ResolvableHotel]
     private ?string $hotelBusProId = null;
 
     private ?string $costUnit = null;

src/Validator/Constraints/ResolvableHotel.php

@@ -0,0 +1,21 @@
+<?php
+
+namespace App\Validator\Constraints;
+
+use Symfony\Component\Validator\Constraint;
+
+/**
+ * The chosen hotel must still resolve against MyE&P's base data.
+ *
+ * Everything a destination records about its house is derived from that lookup - name, code,
+ * cost unit, country - so an id that no longer resolves cannot produce a complete row. It used
+ * to reach App\Controller\Administrative\System\Destination\DestinationTrait::updateEntity()
+ * and fail there on a null, which is a 500 on a form submit rather than a message on a field.
+ */
+#[\Attribute]
+class ResolvableHotel extends Constraint
+{
+    public string $unknownMessage = 'Dieser Vertragspartner ist nicht mehr verfügbar';
+
+    public string $unavailableMessage = 'Die Stammdaten sind gerade nicht erreichbar – bitte später erneut versuchen';
+}

src/Validator/Constraints/ResolvableHotelValidator.php

@@ -0,0 +1,40 @@
+<?php
+
+namespace App\Validator\Constraints;
+
+use App\BusProNet\DataProvider\HotelDataProvider;
+use Symfony\Component\Validator\Constraint;
+use Symfony\Component\Validator\ConstraintValidator;
+
+class ResolvableHotelValidator extends ConstraintValidator
+{
+    public function __construct(private readonly HotelDataProvider $hotelDataProvider)
+    {
+    }
+
+    public function validate(mixed $value, Constraint $constraint): void
+    {
+        if (false === $constraint instanceof ResolvableHotel) {
+            return;
+        }
+
+        // Whether a hotel was chosen at all is NotBlank's business, not this one's.
+        if (null === $value || '' === $value) {
+            return;
+        }
+
+        // An empty list is MyE&P being unreachable rather than a bad choice: StaleWhileErrorCache
+        // answers [] instead of throwing, so this is the only place the two are still
+        // distinguishable. Reporting an outage as an invalid selection sends the operator looking
+        // for a different answer when the one they gave was right.
+        if ([] === $hotels = $this->hotelDataProvider->getAll()) {
+            $this->context->buildViolation($constraint->unavailableMessage)->addViolation();
+
+            return;
+        }
+
+        if (false === isset($hotels[(int) $value])) {
+            $this->context->buildViolation($constraint->unknownMessage)->addViolation();
+        }
+    }
+}