feat: record disposition deleted event for statistics

docs/statistics.md

@@ -26,12 +26,13 @@ Two corollaries that surprise people:
 
 ## Why the table exists
 
-Three of the four metrics could not be answered from live data:
+Most of the metrics could not be answered from live data:
 
 - `Assignment::$jobProfile` holds only the current value. Every edit destroyed the previous
   one permanently.
 - Applications are hard deleted (no soft delete), so any count over them silently shrank
-  over time.
+  over time. Dispositions are hard deleted too, and leave even less behind — no `createdAt`,
+  no trace of the teamer they were on.
 - Call-offs persisted, but not *when* they happened — `updatedAt` is overwritten by any
   later edit — nor at what scope.
 
@@ -81,7 +82,7 @@ hotel code does not rewrite past periods. This is the same pattern `Feedback::fr
 
 ---
 
-## The four metrics
+## The five metrics
 
 Defined in `src/Enum/StatisticsEventName.php`. Adding a case here plus one collector that
 records it is the entire cost of a new metric.
@@ -91,6 +92,7 @@ records it is the entire cost of a new metric.
 | `assignment.job_profile_changed` | the job profile column changes on any write path | `from`, `to` (each `{id, name}` or null) |
 | `assignment.called_off` | an assignment's status becomes `called_off` | `dispositions_affected`, `previous_status` |
 | `disposition.called_off` | a disposition's status becomes `called_off` | `called_off_by`, `scope`, `reason`, `previous_status` |
+| `disposition.deleted` | a disposition is hard deleted, on any delete path | `previous_status`, `remarks`, `called_off_by` |
 | `application.created` | an application is persisted, by any path | — |
 
 Which dimensions each one fills:
@@ -100,8 +102,14 @@ Which dimensions each one fills:
 | `assignment.job_profile_changed` | ✓ | ✓ | ✓ | ✓ | — | — | — |
 | `assignment.called_off` | ✓ | ✓ | ✓ | ✓ | — | — | — |
 | `disposition.called_off` | ✓ | ✓ | ✓ | ✓ | ✓ | ✓ | — |
+| `disposition.deleted` | ✓ | ✓ | ✓ | ✓ | ✓ | ✓ | — |
 | `application.created` | ✓ | ✓ | ✓ | ✓ | ✓ | — | ✓ |
 
+`disposition.deleted` carries `remarks` because a hard delete destroys the entity outright:
+the justification an admin types on the delete form (`DeleteController`) lives nowhere else
+afterwards. `called_off_by` is set only when the placement had already been called off before
+it was deleted.
+
 ### Call-offs: three different "who" and "how far"
 
 This is the part most likely to be misread when building a chart.
@@ -174,6 +182,18 @@ someone's behalf, an import, or a fixture.
 A newly inserted entity has no id during `onFlush`. That is fine — Doctrine assigns it during
 the commit, and recording happens afterwards in `postFlush`.
 
+Collectors see **deletions** too, through `deletionsOf()`. `DispositionDeletedCollector`
+reads them for the same reason: every delete path is counted, and a hard delete leaves
+nothing to reconstruct afterwards. Deletions carry one wrinkle the other two do not.
+Recording still happens in `postFlush`, but by then Doctrine has removed the entity from the
+identity map and **nulled its generated id** (`UnitOfWork::executeDeletions()`), so
+`StatisticsRecorder` can no longer resolve the dimension columns from it. A deletion
+collector therefore snapshots its dimensions itself, during `onFlush`, via
+`StatisticsDimensions::forDisposition()`, and passes them on the `CollectedStatisticsEvent`;
+the listener writes that array verbatim instead of resolving from the subject. Anything a
+deletion metric needs in its payload — status, the form `remarks` — has to be read the same
+way, off the still-attached entity, before the flush commits.
+
 > **There is deliberately only one mechanism.** If a future metric is not a persisted state
 > change at all — an email sent, a login, a document downloaded — call `StatisticsRecorder`
 > directly from wherever that happens. Do not reintroduce a second listener layer for it.
@@ -217,7 +237,7 @@ before the season it belongs to. Picking the wrong one does not fail — it quie
 different question from the screen next to it.
 
 `SEASON` reaches the assignment through an arbitrary join and therefore excludes events with
-no `assignment_id`. All four metrics set it.
+no `assignment_id`. All five metrics set it.
 
 ### Crossing with live data
 
@@ -252,6 +272,7 @@ It writes only to `statistics_event`, so it is safe to run against production at
 | `disposition.called_off` | yes | `updatedAt` — **approximate** |
 | `assignment.called_off` | yes | `updatedAt` — **approximate** |
 | `assignment.job_profile_changed` | **no** | history does not exist |
+| `disposition.deleted` | **no** | the disposition is gone, nothing left to seed |
 
 Every seeded row carries `"backfilled": true`; the approximate ones also carry
 `"approximate_date": true`. So the choice is per chart, never table-wide:
@@ -322,6 +343,7 @@ and equally valid question — just not the same one.
 | Repository | `src/Repository/StatisticsEventRepository.php` |
 | Enums | `src/Enum/Statistics*.php` |
 | Write path | `src/Service/Statistics/StatisticsRecorder.php` |
+| Dimension columns resolved from an entity | `src/Service/Statistics/StatisticsDimensions.php` |
 | Flush orchestration | `src/EventListener/StatisticsChangeSetListener.php` |
 | Collectors (one per concern) | `src/Service/Statistics/Collector/` |
 | Flush context passed to collectors | `src/Service/Statistics/StatisticsFlush.php` |

src/Enum/StatisticsEventName.php

@@ -17,6 +17,7 @@ enum StatisticsEventName: string
     case ASSIGNMENT_JOB_PROFILE_CHANGED = 'assignment.job_profile_changed';
     case ASSIGNMENT_CALLED_OFF = 'assignment.called_off';
     case DISPOSITION_CALLED_OFF = 'disposition.called_off';
+    case DISPOSITION_DELETED = 'disposition.deleted';
     case APPLICATION_CREATED = 'application.created';
 
     public function label(): string
@@ -25,6 +26,7 @@ enum StatisticsEventName: string
             self::ASSIGNMENT_JOB_PROFILE_CHANGED => 'Tätigkeitsprofil geändert',
             self::ASSIGNMENT_CALLED_OFF => 'Reise abgesagt',
             self::DISPOSITION_CALLED_OFF => 'Einsatz abgesagt',
+            self::DISPOSITION_DELETED => 'Einteilung gelöscht',
             self::APPLICATION_CREATED => 'Bewerbung eingegangen',
         };
     }

src/EventListener/StatisticsChangeSetListener.php

@@ -75,6 +75,14 @@ class StatisticsChangeSetListener implements ResetInterface
         foreach ($pending as $event) {
             $subject = $event->subject;
 
+            // Dimensions the collector froze at onFlush time - the subject is gone or
+            // detached by now, so there is nothing to resolve them from.
+            if (null !== $event->dimensions) {
+                $this->recorder->record($event->name, $event->dimensions, $event->payload);
+
+                continue;
+            }
+
             if ($subject instanceof Assignment) {
                 $this->recorder->recordForAssignment($event->name, $subject, $event->payload);
 

src/Service/Statistics/CollectedStatisticsEvent.php

@@ -12,16 +12,23 @@ use App\Enum\StatisticsEventName;
  *
  * Collectors run during onFlush, where the changeset still exists, but nothing may be
  * written until the transaction has committed. This is what travels between the two.
+ *
+ * Normally the recorder resolves the dimension columns from $subject in postFlush. A
+ * collector that watches deletions cannot rely on that - the entity is detached and its id
+ * nulled by then - so it snapshots $dimensions now, and the listener writes them verbatim.
  */
 final class CollectedStatisticsEvent
 {
     /**
-     * @param array<string, mixed> $payload
+     * @param array<string, mixed>                $payload
+     * @param array<string, int|string|null>|null $dimensions pre-resolved at collect time; when
+     *                                                        set, written as-is instead of from $subject
      */
     public function __construct(
         public readonly StatisticsEventName $name,
         public readonly Assignment|Disposition|Application $subject,
         public readonly array $payload = [],
+        public readonly ?array $dimensions = null,
     ) {
     }
 }

src/Service/Statistics/Collector/DispositionDeletedCollector.php

@@ -0,0 +1,43 @@
+<?php
+
+namespace App\Service\Statistics\Collector;
+
+use App\Entity\Disposition;
+use App\Enum\StatisticsEventName;
+use App\Service\Statistics\CollectedStatisticsEvent;
+use App\Service\Statistics\StatisticsDimensions;
+use App\Service\Statistics\StatisticsFlush;
+
+/**
+ * A disposition being hard deleted by an admin.
+ *
+ * Its own concern: a deletion shares nothing with the call-off metrics, which have to be
+ * told apart together. Read from the flush rather than from the delete controller, so any
+ * path that removes a disposition is counted, and because a hard delete leaves nothing
+ * behind - this row is the only record the placement, its teamer, and the admin's
+ * justification ever existed.
+ *
+ * Dimensions are resolved here, during onFlush: by postFlush the entity is detached and its
+ * id nulled, so StatisticsRecorder could not resolve them from the subject.
+ */
+class DispositionDeletedCollector implements StatisticsCollectorInterface
+{
+    public function collect(StatisticsFlush $flush): iterable
+    {
+        foreach ($flush->deletionsOf(Disposition::class) as $disposition) {
+            yield new CollectedStatisticsEvent(
+                StatisticsEventName::DISPOSITION_DELETED,
+                $disposition,
+                [
+                    'previous_status' => $disposition->getStatus(),
+                    // The reason typed on the delete form, bound onto the entity before the
+                    // remove(). Nowhere else survives the deletion.
+                    'remarks' => $disposition->getRemarks(),
+                    // Non-null only if the placement had already been called off first.
+                    'called_off_by' => $disposition->getCalledOffBy(),
+                ],
+                StatisticsDimensions::forDisposition($disposition),
+            );
+        }
+    }
+}

src/Service/Statistics/StatisticsDimensions.php

@@ -0,0 +1,57 @@
+<?php
+
+namespace App\Service\Statistics;
+
+use App\Entity\Application;
+use App\Entity\Assignment;
+use App\Entity\Disposition;
+
+/**
+ * The dimension columns of a statistics_event, resolved from a live entity.
+ *
+ * One place, because the SER-stripping hotel-code rule is already reimplemented in three
+ * spots (Destination, DispositionRepository, FeedbackRepository) and must not gain a fourth.
+ * StatisticsRecorder resolves dimensions here in postFlush for inserts and updates; a
+ * deletion collector calls the same methods during onFlush, while the entity is still whole.
+ */
+final class StatisticsDimensions
+{
+    /**
+     * @return array<string, int|string|null>
+     */
+    public static function forAssignment(?Assignment $assignment): array
+    {
+        $destination = $assignment?->getDestination();
+
+        return [
+            'assignment_id' => $assignment?->getId(),
+            'destination_id' => $destination?->getId(),
+            'job_profile_id' => $assignment?->getJobProfile()?->getId(),
+            'hotel_code' => $destination?->getHotelCodeNormalized(),
+        ];
+    }
+
+    /**
+     * @return array<string, int|string|null>
+     */
+    public static function forDisposition(Disposition $disposition): array
+    {
+        $dimensions = self::forAssignment($disposition->getAssignment());
+        $dimensions['disposition_id'] = $disposition->getId();
+        $dimensions['teamer_id'] = $disposition->getTeamer()?->getId();
+
+        return $dimensions;
+    }
+
+    /**
+     * @return array<string, int|string|null>
+     */
+    public static function forApplication(Application $application): array
+    {
+        $dimensions = self::forAssignment($application->getAssignment());
+        $dimensions['application_id'] = $application->getId();
+        $dimensions['teamer_id'] = $application->getTeamer()?->getId();
+
+        return $dimensions;
+    }
+}

src/Service/Statistics/StatisticsFlush.php

@@ -71,6 +71,34 @@ final class StatisticsFlush
         ));
     }
 
+    /**
+     * @return array<int, object>
+     */
+    public function deletions(): array
+    {
+        return $this->unitOfWork->getScheduledEntityDeletions();
+    }
+
+    /**
+     * A scheduled-for-deletion entity still has its id and its initialised relations here,
+     * during onFlush. It does not in postFlush: Doctrine removes it from the identity map
+     * and nulls its generated id once the delete commits, so a collector that cares about a
+     * deletion has to read everything it needs off the entity now, not later.
+     *
+     * @template T of object
+     *
+     * @param class-string<T> $class
+     *
+     * @return array<int, T>
+     */
+    public function deletionsOf(string $class): array
+    {
+        return array_values(array_filter(
+            $this->deletions(),
+            static fn (object $entity): bool => $entity instanceof $class,
+        ));
+    }
+
     /**
      * @return array<string, mixed>
      */

src/Service/Statistics/StatisticsRecorder.php

@@ -85,7 +85,7 @@ class StatisticsRecorder
         array $payload = [],
         ?\DateTimeImmutable $occurredAt = null,
     ): void {
-        $this->record($name, $this->dimensionsFromAssignment($assignment), $payload, $occurredAt);
+        $this->record($name, StatisticsDimensions::forAssignment($assignment), $payload, $occurredAt);
     }
 
     /**
@@ -97,11 +97,7 @@ class StatisticsRecorder
         array $payload = [],
         ?\DateTimeImmutable $occurredAt = null,
     ): void {
-        $dimensions = $this->dimensionsFromAssignment($disposition->getAssignment());
+        $this->record($name, StatisticsDimensions::forDisposition($disposition), $payload, $occurredAt);
-        $dimensions['disposition_id'] = $disposition->getId();
-        $dimensions['teamer_id'] = $disposition->getTeamer()?->getId();
-
-        $this->record($name, $dimensions, $payload, $occurredAt);
     }
 
     /**
@@ -113,11 +109,7 @@ class StatisticsRecorder
         array $payload = [],
         ?\DateTimeImmutable $occurredAt = null,
     ): void {
-        $dimensions = $this->dimensionsFromAssignment($application->getAssignment());
+        $this->record($name, StatisticsDimensions::forApplication($application), $payload, $occurredAt);
-        $dimensions['application_id'] = $application->getId();
-        $dimensions['teamer_id'] = $application->getTeamer()?->getId();
-
-        $this->record($name, $dimensions, $payload, $occurredAt);
     }
 
     public function resolveActorRole(): StatisticsActorRole
@@ -136,19 +128,4 @@ class StatisticsRecorder
 
         return StatisticsActorRole::SYSTEM;
     }
-
-    /**
-     * @return array<string, int|string|null>
-     */
-    private function dimensionsFromAssignment(?Assignment $assignment): array
-    {
-        $destination = $assignment?->getDestination();
-
-        return [
-            'assignment_id' => $assignment?->getId(),
-            'destination_id' => $destination?->getId(),
-            'job_profile_id' => $assignment?->getJobProfile()?->getId(),
-            'hotel_code' => $destination?->getHotelCodeNormalized(),
-        ];
-    }
 }

tests/EventListener/StatisticsChangeSetListenerTest.php

@@ -14,6 +14,7 @@ use App\Enum\StatisticsEventName;
 use App\EventListener\StatisticsChangeSetListener;
 use App\Service\Statistics\Collector\ApplicationCreatedCollector;
 use App\Service\Statistics\Collector\CallOffCollector;
+use App\Service\Statistics\Collector\DispositionDeletedCollector;
 use App\Service\Statistics\Collector\JobProfileChangeCollector;
 use App\Service\Statistics\StatisticsRecorder;
 use Doctrine\ORM\EntityManagerInterface;
@@ -304,6 +305,46 @@ class StatisticsChangeSetListenerTest extends TestCase
         $this->flush([], [$application]);
     }
 
+    /**
+     * A hard delete leaves nothing behind, so this row is the only trace the placement, its
+     * teamer and the admin's justification ever existed. The dimensions have to be read
+     * while the entity is still whole - by postFlush Doctrine has nulled its id.
+     */
+    public function testRecordsADispositionDeletion(): void
+    {
+        $disposition = $this->disposition();
+        $disposition
+            ->setStatus(Disposition::STATUS_CONFIRMED)
+            ->setRemarks('Doppelt eingeteilt')
+        ;
+
+        $this->recorder
+            ->expects($this->once())
+            ->method('record')
+            ->with(
+                StatisticsEventName::DISPOSITION_DELETED,
+                $this->anything(),
+                $this->callback(static function (array $payload): bool {
+                    return Disposition::STATUS_CONFIRMED === $payload['previous_status']
+                        && 'Doppelt eingeteilt' === $payload['remarks']
+                        && null === $payload['called_off_by'];
+                })
+            )
+        ;
+
+        $this->flush([], [], [$disposition]);
+    }
+
+    /**
+     * A disposition being edited is not a disposition being deleted.
+     */
+    public function testADispositionUpdateIsNotADeletion(): void
+    {
+        $this->recorder->expects($this->never())->method('record');
+
+        $this->flush([[$this->disposition(), ['remarks' => ['alt', 'neu']]]]);
+    }
+
     /**
      * Editing an application later is not a second application.
      */
@@ -327,20 +368,22 @@ class StatisticsChangeSetListenerTest extends TestCase
     /**
      * @param array<int, array{0: object, 1: array<string, mixed>}> $changeSets
      * @param array<int, object>                                    $insertions
+     * @param array<int, object>                                    $deletions
      */
-    private function flush(array $changeSets, array $insertions = []): void
+    private function flush(array $changeSets, array $insertions = [], array $deletions = []): void
     {
         $listener = $this->listener();
 
-        $listener->onFlush($this->onFlushArgs($changeSets, $insertions));
+        $listener->onFlush($this->onFlushArgs($changeSets, $insertions, $deletions));
         $listener->postFlush($this->postFlushArgs());
     }
 
     /**
      * @param array<int, array{0: object, 1: array<string, mixed>}> $changeSets
      * @param array<int, object>                                    $insertions
+     * @param array<int, object>                                    $deletions
      */
-    private function onFlushArgs(array $changeSets, array $insertions = []): OnFlushEventArgs
+    private function onFlushArgs(array $changeSets, array $insertions = [], array $deletions = []): OnFlushEventArgs
     {
         $entities = [];
         $byEntity = new \SplObjectStorage();
@@ -353,6 +396,7 @@ class StatisticsChangeSetListenerTest extends TestCase
         $unitOfWork = $this->createMock(UnitOfWork::class);
         $unitOfWork->method('getScheduledEntityUpdates')->willReturn($entities);
         $unitOfWork->method('getScheduledEntityInsertions')->willReturn($insertions);
+        $unitOfWork->method('getScheduledEntityDeletions')->willReturn($deletions);
         $unitOfWork
             ->method('getEntityChangeSet')
             ->willReturnCallback(static fn (object $entity): array => $byEntity[$entity] ?? [])
@@ -377,6 +421,7 @@ class StatisticsChangeSetListenerTest extends TestCase
         return new StatisticsChangeSetListener($this->recorder, [
             new JobProfileChangeCollector(),
             new CallOffCollector(),
+            new DispositionDeletedCollector(),
             new ApplicationCreatedCollector(),
         ]);
     }

tests/Service/Statistics/StatisticsDimensionsTest.php

@@ -0,0 +1,65 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Tests\Service\Statistics;
+
+use App\Entity\Application;
+use App\Entity\Assignment;
+use App\Entity\Destination;
+use App\Entity\Disposition;
+use App\Entity\Teamer;
+use App\Service\Statistics\StatisticsDimensions;
+use PHPUnit\Framework\TestCase;
+
+/**
+ * The dimension columns are what every statistics screen groups by, so the SER-stripping
+ * hotel-code rule has to be applied here exactly as the recorder used to apply it inline.
+ */
+class StatisticsDimensionsTest extends TestCase
+{
+    public function testFreezesTheNormalizedHotelCodeOfTheAssignment(): void
+    {
+        $assignment = (new Assignment())->setDestination((new Destination())->setHotelCode('SERZIL'));
+
+        $this->assertSame('ZIL', StatisticsDimensions::forAssignment($assignment)['hotel_code']);
+    }
+
+    public function testKeepsAnUnprefixedHotelCodeAsItIs(): void
+    {
+        $assignment = (new Assignment())->setDestination((new Destination())->setHotelCode('SBW'));
+
+        $this->assertSame('SBW', StatisticsDimensions::forAssignment($assignment)['hotel_code']);
+    }
+
+    public function testADispositionCarriesItsTeamerAndTheAssignmentDimensions(): void
+    {
+        $teamer = new Teamer();
+        $assignment = (new Assignment())->setDestination((new Destination())->setHotelCode('SERZIL'));
+        $disposition = new Disposition(new Application($assignment, $teamer));
+
+        $dimensions = StatisticsDimensions::forDisposition($disposition);
+
+        $this->assertArrayHasKey('disposition_id', $dimensions);
+        $this->assertSame($teamer->getId(), $dimensions['teamer_id']);
+        $this->assertSame('ZIL', $dimensions['hotel_code']);
+    }
+
+    /**
+     * A disposition can lose its assignment or teamer; the columns just go null, they do not
+     * blow up the flush that is trying to record the event.
+     */
+    public function testDegradesToNullWithoutAnAssignmentOrTeamer(): void
+    {
+        $disposition = new Disposition(new Application(new Assignment(), new Teamer()));
+        $disposition->setAssignment(null);
+        $disposition->setTeamer(null);
+
+        $dimensions = StatisticsDimensions::forDisposition($disposition);
+
+        $this->assertNull($dimensions['assignment_id']);
+        $this->assertNull($dimensions['destination_id']);
+        $this->assertNull($dimensions['hotel_code']);
+        $this->assertNull($dimensions['teamer_id']);
+    }
+}