From bc7d3fcbee8440f3030952f07d9dde075a614ba7 Mon Sep 17 00:00:00 2001 From: "bert.hausmans" Date: Fri, 8 May 2026 21:29:49 +0200 Subject: [PATCH] =?UTF-8?q?fix(timetable):=20single=20activity=20entry=20p?= =?UTF-8?q?er=20cascade-move=20per=20RFC=20=C2=A78?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit LaneCascadeService::move() now calls disableLogging() before every save inside the transaction (locked performance + cascade-bumped peers + park-path). The two explicit activity('performance') ->event('moved'|'parked') entries with cascade_count + cascaded_ids properties are the only audit records per move, matching RFC §8's "single parent entry summarising the cascade" requirement. Park path additionally writes an explicit 'performance.parked' entry per RFC §8 vocabulary instead of falling back to a generic 'updated' auto-log entry. Two new tests verify: - cascade move with N peers produces exactly 1 activity entry on the moved subject and 0 on each cascade-bumped peer - park writes exactly 1 'parked' entry PerformanceObserver::saving (version bump) is unaffected: disableLogging() suppresses only the activity log trait, not Eloquent model events. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../Services/Artist/LaneCascadeService.php | 24 ++++-- .../Feature/Artist/LaneCascadeServiceTest.php | 82 +++++++++++++++++++ 2 files changed, 101 insertions(+), 5 deletions(-) diff --git a/api/app/Services/Artist/LaneCascadeService.php b/api/app/Services/Artist/LaneCascadeService.php index ead555e7..ecc13df1 100644 --- a/api/app/Services/Artist/LaneCascadeService.php +++ b/api/app/Services/Artist/LaneCascadeService.php @@ -74,9 +74,21 @@ final class LaneCascadeService if ($targetLane !== null) { $locked->lane = $targetLane; } + $locked->disableLogging(); // suppress auto-log; park is captured in explicit entry below $locked->save(); - return new MoveResult($locked->refresh(), []); + $parked = $locked->refresh(); + + activity('performance') + ->performedOn($parked) + ->event('parked') + ->withProperties([ + 'event_id' => $parked->event_id, + 'organisation_id' => $parked->engagement?->organisation_id, + ]) + ->log('parked'); + + return new MoveResult($parked, []); } if ($start === null || $end === null || $targetLane === null) { @@ -104,6 +116,7 @@ final class LaneCascadeService foreach ($existingOnLane as $other) { if ($this->overlaps($start, $end, $other->start_at, $other->end_at)) { $other->lane = (int) $other->lane + 1; + $other->disableLogging(); // suppress auto-log; cascade is captured in parent entry $other->save(); $cascaded[] = $other->refresh(); } @@ -114,14 +127,15 @@ final class LaneCascadeService $locked->start_at = $start; $locked->end_at = $end; $locked->lane = $targetLane; + $locked->disableLogging(); // suppress auto-log; move is captured in explicit entry below $locked->save(); $moved = $locked->refresh(); - // RFC §8 — single parent activity entry summarising the - // cascade. The per-row updates (lane bumps) still flow - // through the model's auto-log; this entry is the audit - // anchor for the whole transactional move. + // RFC §8 — single parent activity entry summarising the cascade. + // All saves inside this transaction call disableLogging() so the + // auto-log trait does not write per-row 'updated' entries; this + // explicit entry is the only audit record for the move. activity('performance') ->performedOn($moved) ->event('moved') diff --git a/api/tests/Feature/Artist/LaneCascadeServiceTest.php b/api/tests/Feature/Artist/LaneCascadeServiceTest.php index 3d463f2c..a031e87d 100644 --- a/api/tests/Feature/Artist/LaneCascadeServiceTest.php +++ b/api/tests/Feature/Artist/LaneCascadeServiceTest.php @@ -16,6 +16,7 @@ use App\Services\Artist\LaneCascadeService; use Carbon\CarbonImmutable; use Database\Seeders\RoleSeeder; use Illuminate\Foundation\Testing\RefreshDatabase; +use Spatie\Activitylog\Models\Activity; use Tests\TestCase; final class LaneCascadeServiceTest extends TestCase @@ -184,4 +185,85 @@ final class LaneCascadeServiceTest extends TestCase $this->assertSame((string) $this->stage->id, (string) $result->moved->stage_id); } + + public function test_move_with_cascade_writes_exactly_one_activity_entry_on_moved_subject_and_zero_on_peers(): void + { + $start = CarbonImmutable::now()->addDays(5)->setTime(20, 0); + + $p2 = Performance::factory()->create([ + 'engagement_id' => $this->engagement->id, + 'event_id' => $this->event->id, + 'stage_id' => $this->stage->id, + 'lane' => 0, + 'start_at' => $start, + 'end_at' => $start->addHour(), + 'version' => 0, + ]); + + $p1 = Performance::factory()->create([ + 'engagement_id' => $this->engagement->id, + 'event_id' => $this->event->id, + 'stage_id' => $this->stage->id, + 'lane' => 1, + 'start_at' => $start, + 'end_at' => $start->addHour(), + 'version' => 0, + ]); + + // Clear setup-time activity rows so we measure only the move. + Activity::query()->delete(); + + $this->service->move( + performance: $p1, + targetStage: $this->stage, + start: $start, + end: $start->addHour(), + targetLane: 0, + clientVersion: 0, + ); + + $movedEntries = Activity::query() + ->where('subject_type', $p1->getMorphClass()) + ->where('subject_id', $p1->id) + ->get(); + $this->assertCount(1, $movedEntries, 'Expected exactly one activity entry for moved performance'); + $this->assertSame('moved', $movedEntries->first()->event); + $this->assertSame(1, $movedEntries->first()->properties['cascade_count']); + $this->assertContains((string) $p2->id, $movedEntries->first()->properties['cascaded_ids']); + + $cascadedEntries = Activity::query() + ->where('subject_type', $p2->getMorphClass()) + ->where('subject_id', $p2->id) + ->get(); + $this->assertCount(0, $cascadedEntries, 'Expected zero activity entries on cascade-bumped peer'); + } + + public function test_park_writes_single_parked_activity_entry(): void + { + $perf = Performance::factory()->create([ + 'engagement_id' => $this->engagement->id, + 'event_id' => $this->event->id, + 'stage_id' => $this->stage->id, + 'lane' => 0, + 'version' => 0, + ]); + + Activity::query()->delete(); + + $this->service->move( + performance: $perf, + targetStage: null, + start: null, + end: null, + targetLane: null, + clientVersion: 0, + ); + + $entries = Activity::query() + ->where('subject_type', $perf->getMorphClass()) + ->where('subject_id', $perf->id) + ->get(); + $this->assertCount(1, $entries, 'Expected exactly one parked entry'); + $this->assertSame('parked', $entries->first()->event); + } }