feat: order and format the pickup fields

src/Form/AssignmentType.php

@@ -7,6 +7,7 @@ use App\Entity\Destination;
 use App\Entity\Fee;
 use App\Entity\JobProfile;
 use App\Entity\User;
+use App\Service\Common\PickupOptions;
 use Doctrine\ORM\EntityManagerInterface;
 use Doctrine\ORM\EntityRepository;
 use Symfony\Bridge\Doctrine\Form\Type\EntityType;
@@ -130,6 +131,11 @@ class AssignmentType extends AbstractType
                 'label' => 'Busbegleitung ab',
                 'required' => false,
                 'placeholder' => 'Keine Busbegleitung',
+                'expanded' => true,
+                'block_prefix' => 'pickup_choice',
+                // there is nothing to offer until a destination is picked, and an empty
+                // select looks like a destination without boarding points
+                'attr' => ['data-empty-text' => 'Bitte erst eine Destination wählen …'],
             ])
             ->addEventListener(FormEvents::PRE_SET_DATA, function (FormEvent $event) {
                 $form = $event->getForm();
@@ -234,15 +240,16 @@ class AssignmentType extends AbstractType
 
     private function addPickupField(Destination $destination, FormInterface $form): void
     {
-        $choices = [];
+        $options = PickupOptions::inDepartureOrder($destination);
+        $labels = array_column($options, 'label', 'busProId');
+        $attributes = [];
 
-        foreach ($destination->getPickups() as $pickup) {
-            if ($pickup['time']) {
-                $label = sprintf('%s, %s Uhr', trim($pickup['city']), trim($pickup['time']));
-            } else {
-                $label = trim($pickup['city']);
-            }
-            $choices[$label] = $pickup['busProId'];
+        foreach ($options as $option) {
+            // read back by the pickup_choice_widget block in templates/forms.html.twig
+            $attributes[$option['busProId']] = [
+                'data-departure' => $option['departure'] ?? '',
+                'data-street' => $option['street'] ?? '',
+            ];
         }
 
         // Remove placeholder field first
@@ -252,7 +259,14 @@ class AssignmentType extends AbstractType
             'label' => 'Busbegleitung ab',
             'required' => false,
             'placeholder' => 'Keine Busbegleitung',
-            'choices' => $choices,
+            'choices' => array_column($options, 'busProId'),
+            'choice_label' => static fn (int $busProId): string => $labels[$busProId],
+            'choice_attr' => static fn (int $busProId): array => $attributes[$busProId],
+            // a radio list rather than a dropdown: the departure time and the street need
+            // their own place in the row, which a single option label cannot give them
+            'expanded' => true,
+            'block_prefix' => 'pickup_choice',
+            'attr' => ['data-empty-text' => 'Für diese Destination sind keine Zustiege hinterlegt.'],
         ]);
     }
 

src/Form/TeamerApplicationType.php

@@ -3,6 +3,7 @@
 namespace App\Form;
 
 use App\Entity\Application;
+use App\Service\Common\PickupOptions;
 use Symfony\Component\Form\AbstractType;
 use Symfony\Component\Form\Extension\Core\Type\CheckboxType;
 use Symfony\Component\Form\Extension\Core\Type\ChoiceType;
@@ -44,14 +45,14 @@ class TeamerApplicationType extends AbstractType
                 $teamer = $application->getTeamer();
 
                 if (0 === (int) $assignment->getPickup()) {
-                    $pickupChoices = [];
-                    foreach ($pickups as $pickup) {
-                        $pickupChoices[$pickup['city']] = $pickup['busProId'];
-                    }
-                    ksort($pickupChoices);
+                    // keyed by busProId, not by the town: two boarding points share a town
+                    // on six destinations, and keying on the label dropped the second one
+                    $options = PickupOptions::byTown($assignment->getDestination());
+                    $labels = array_column($options, 'label', 'busProId');
                     $form->add('pickup', ChoiceType::class, [
                         'label' => 'Zustieg in',
-                        'choices' => $pickupChoices,
+                        'choices' => array_column($options, 'busProId'),
+                        'choice_label' => static fn (int $busProId): string => $labels[$busProId],
                     ]);
                 } elseif (false === in_array($assignment->getPickup(), $teamer->getPickups())) {
                     $form->add('confirmPickup', CheckboxType::class, [

src/Form/TeamerDispositionType.php

@@ -3,6 +3,7 @@
 namespace App\Form;
 
 use App\Entity\Disposition;
+use App\Service\Common\PickupOptions;
 use Symfony\Component\Form\AbstractType;
 use Symfony\Component\Form\Extension\Core\Type\CheckboxType;
 use Symfony\Component\Form\Extension\Core\Type\ChoiceType;
@@ -24,14 +25,14 @@ class TeamerDispositionType extends AbstractType
 
                 // Add pickup select field only to assignments that are not including bus chaperon
                 if (0 === (int) $assignment->getPickup()) {
-                    $pickupChoices = [];
-                    foreach ($assignment->getDestination()->getPickups() as $pickup) {
-                        $pickupChoices[$pickup['city']] = $pickup['busProId'];
-                    }
-                    ksort($pickupChoices);
+                    // keyed by busProId, not by the town: two boarding points share a town
+                    // on six destinations, and keying on the label dropped the second one
+                    $options = PickupOptions::byTown($assignment->getDestination());
+                    $labels = array_column($options, 'label', 'busProId');
                     $event->getForm()->add('pickup', ChoiceType::class, [
                         'label' => 'Zustieg in',
-                        'choices' => $pickupChoices,
+                        'choices' => array_column($options, 'busProId'),
+                        'choice_label' => static fn (int $busProId): string => $labels[$busProId],
                     ]);
                 }
             });

src/Service/Assignment/PickupDepartureOrder.php

@@ -0,0 +1,176 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Service\Assignment;
+
+/**
+ * Puts a destination's boarding points into departure order.
+ *
+ * The stored order is the order BusProNet's travel export happens to list them in, which
+ * is not chronological: a real row reads 20:45, 00:30, 04:15, 02:45, 22:00, 01:30, 23:30.
+ *
+ * Ordering them is not a plain sort on the clock, because a coach leaves in the evening and
+ * keeps picking up through the night, so 00:30 comes *after* 23:30. Which of the two a given
+ * entry means is only decidable from the date, and the stored time carries one in some
+ * entries and not others - BusProNet writes both "08.03.2024 20:45:00" and "02:15", and 453
+ * of 996 destinations hold both forms at once.
+ *
+ * So an entry that carries a date is absolute and used as it stands, and a bare clock time
+ * is placed on the day the coach sets off, rolled to the next day when it falls before that
+ * first dated departure. Where nothing carries a date there is no overnight leg to resolve -
+ * none of those 240 destinations spans midnight - and the clock alone is the right order.
+ *
+ * Once MyE&P's travel export replaces the XML import, every entry carries a resolved
+ * `datetime` and that wins outright; the rest of this is then only reading legacy rows.
+ */
+final class PickupDepartureOrder
+{
+    /**
+     * @param array<int, array<string, mixed>> $pickups as stored on Destination::$pickups
+     *
+     * @return list<array<string, mixed>>
+     */
+    public static function sort(array $pickups): array
+    {
+        $entries = [];
+
+        foreach (array_values($pickups) as $index => $pickup) {
+            $entries[] = [
+                'index' => $index,
+                'pickup' => $pickup,
+                'timestamp' => self::absoluteTimestamp($pickup),
+                'minutes' => self::clockMinutes($pickup),
+            ];
+        }
+
+        $anchor = self::anchor($entries);
+
+        foreach ($entries as $i => $entry) {
+            if (null !== $entry['timestamp'] || null === $entry['minutes'] || null === $anchor) {
+                continue;
+            }
+
+            $entries[$i]['timestamp'] = self::onDepartureDay($entry['minutes'], $anchor);
+        }
+
+        usort($entries, static function (array $a, array $b): int {
+            $keyA = $a['timestamp'] ?? $a['minutes'];
+            $keyB = $b['timestamp'] ?? $b['minutes'];
+
+            // boarding points without a readable time keep their place at the end rather
+            // than jumping to the front of the list
+            if (null === $keyA || null === $keyB) {
+                return (null === $keyA ? 1 : 0) <=> (null === $keyB ? 1 : 0);
+            }
+
+            return $keyA <=> $keyB;
+        });
+
+        return array_column($entries, 'pickup');
+    }
+
+    /**
+     * The departure time as a plain 24 hour clock, for a label.
+     *
+     * BusProNet writes "08.03.2024 20:45:00" for some boarding points and "02:15" for the
+     * next one in the same travel, so rendering the stored value verbatim puts a full date
+     * in one label and a bare time beside it. Both shapes are read here, through the same
+     * parser the ordering uses, so a list cannot end up sorted by one reading of a time and
+     * labelled by another.
+     *
+     * @param array<string, mixed> $pickup
+     */
+    public static function clock(array $pickup): ?string
+    {
+        $minutes = self::clockMinutes($pickup);
+
+        if (null !== $minutes) {
+            return sprintf('%02d:%02d', intdiv($minutes, 60), $minutes % 60);
+        }
+
+        $timestamp = self::absoluteTimestamp($pickup);
+
+        if (null === $timestamp) {
+            return null;
+        }
+
+        return (new \DateTimeImmutable('@'.$timestamp))
+            ->setTimezone(new \DateTimeZone(date_default_timezone_get()))
+            ->format('H:i')
+        ;
+    }
+
+    /**
+     * The first dated departure, which is the moment the coach sets off.
+     *
+     * @param list<array<string, mixed>> $entries
+     */
+    private static function anchor(array $entries): ?int
+    {
+        $timestamps = array_filter(array_column($entries, 'timestamp'), static fn (?int $t) => null !== $t);
+
+        return [] === $timestamps ? null : min($timestamps);
+    }
+
+    private static function onDepartureDay(int $minutes, int $anchor): int
+    {
+        $day = (new \DateTimeImmutable('@'.$anchor))->setTimezone(new \DateTimeZone(date_default_timezone_get()));
+        $anchorMinutes = (int) $day->format('H') * 60 + (int) $day->format('i');
+
+        $placed = $day->setTime(intdiv($minutes, 60), $minutes % 60);
+
+        if ($minutes < $anchorMinutes) {
+            $placed = $placed->modify('+1 day');
+        }
+
+        return $placed->getTimestamp();
+    }
+
+    /**
+     * @param array<string, mixed> $pickup
+     */
+    private static function absoluteTimestamp(array $pickup): ?int
+    {
+        // what MyE&P's travel export will carry: already resolved across midnight
+        $datetime = $pickup['datetime'] ?? null;
+
+        if (is_string($datetime) && '' !== $datetime && false !== $parsed = strtotime($datetime)) {
+            return $parsed;
+        }
+
+        $time = self::time($pickup);
+
+        if (null === $time || 1 !== preg_match('/^(\d{2}\.\d{2}\.\d{4}) (\d{2}:\d{2})(?::\d{2})?$/', $time, $matches)) {
+            return null;
+        }
+
+        $date = \DateTimeImmutable::createFromFormat('!d.m.Y H:i', $matches[1].' '.$matches[2]);
+
+        return false === $date ? null : $date->getTimestamp();
+    }
+
+    /**
+     * @param array<string, mixed> $pickup
+     */
+    private static function clockMinutes(array $pickup): ?int
+    {
+        $time = self::time($pickup);
+
+        if (null === $time || 1 !== preg_match('/^(\d{2}):(\d{2})$/', $time, $matches)) {
+            return null;
+        }
+
+        return (int) $matches[1] * 60 + (int) $matches[2];
+    }
+
+    /**
+     * @param array<string, mixed> $pickup
+     */
+    private static function time(array $pickup): ?string
+    {
+        $time = trim((string) ($pickup['time'] ?? ''));
+
+        return '' === $time ? null : $time;
+    }
+}

src/Service/Common/PickupOptions.php

@@ -0,0 +1,93 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Service\Common;
+
+use App\Entity\Destination;
+use App\Service\Assignment\PickupDepartureOrder;
+
+/**
+ * The boarding points of a destination, as something a form can offer.
+ *
+ * Every caller keys its choices on the busProId rather than on the label, because the
+ * label is not unique: six destinations have two boarding points in the same town, and one
+ * of them has both at 03:15. Keying on the label, as all three pickup fields used to, made
+ * the second one unreachable - it never reached the form and could not be picked.
+ *
+ * Two orders, because the two audiences read the list differently. Disposition planners
+ * read a route and want it chronological; a teamer is looking up their own town in a list
+ * and wants it alphabetical.
+ */
+final class PickupOptions
+{
+    /**
+     * @return list<array{busProId: int, label: string, departure: ?string, street: ?string}>
+     */
+    public static function inDepartureOrder(Destination $destination): array
+    {
+        return self::rows(PickupDepartureOrder::sort($destination->getPickups()));
+    }
+
+    /**
+     * Alphabetical by town, with the street folded into the label where a town repeats -
+     * otherwise the teamer is offered the same word twice with nothing to choose between.
+     *
+     * @return list<array{busProId: int, label: string, departure: ?string, street: ?string}>
+     */
+    public static function byTown(Destination $destination): array
+    {
+        $options = self::rows($destination->getPickups());
+        $occurrences = array_count_values(array_column($options, 'label'));
+
+        foreach ($options as $index => $option) {
+            if ($occurrences[$option['label']] < 2) {
+                continue;
+            }
+
+            $options[$index]['label'] = match (true) {
+                null !== $option['street'] => sprintf('%s, %s', $option['label'], $option['street']),
+                null !== $option['departure'] => sprintf('%s, %s Uhr', $option['label'], $option['departure']),
+                default => sprintf('%s (Zustieg %d)', $option['label'], $option['busProId']),
+            };
+        }
+
+        usort($options, static fn (array $a, array $b): int => $a['label'] <=> $b['label']);
+
+        return $options;
+    }
+
+    /**
+     * @param array<int, array<string, mixed>> $pickups
+     *
+     * @return list<array{busProId: int, label: string, departure: ?string, street: ?string}>
+     */
+    private static function rows(array $pickups): array
+    {
+        $options = [];
+
+        foreach ($pickups as $pickup) {
+            $busProId = (int) ($pickup['busProId'] ?? 0);
+
+            if (0 === $busProId) {
+                continue;
+            }
+
+            $city = trim((string) ($pickup['city'] ?? ''));
+            $street = trim((string) ($pickup['street'] ?? ''));
+
+            $options[] = [
+                'busProId' => $busProId,
+                // a boarding point missing from the base data has no address, but it is
+                // still a real departure and has to stay selectable. None exist today; they
+                // arrive with MyE&P's travel export, which reports the ones whose master
+                // data it could not resolve rather than dropping them.
+                'label' => '' === $city ? 'Zustieg '.$busProId : $city,
+                'departure' => PickupDepartureOrder::clock($pickup),
+                'street' => '' === $street ? null : $street,
+            ];
+        }
+
+        return $options;
+    }
+}