Merge pull request #3669 from codeeu/fix/recurring-events-source-ref

Fix recurring events blocked by unique source_ref

Author: bernardhanna

app/Console/Commands/GenerateRecurringEvents.php

@@ -15,7 +15,9 @@
  * and whose next occurrence does not exceed end_date.
  *
  * Clones event data and related pivot tables (themes, tags, audiences).
- * Stores reference in `source_ref` as "parent:{event_id}" for traceability.
+ * Stores reference in `source_ref` as "parent:{event_id}:{Y-m-d}" so each
+ * occurrence is unique — `events.source_ref` has a unique index, and the old
+ * bare "parent:{event_id}" form could only ever store one child per parent.
  *
  * Options:
  *  --dry-run : simulate generation without DB writes.
@@ -48,6 +50,7 @@ public function handle(): int
 
         if ($events->isEmpty()) {
             $this->info('No eligible recurring events found.');
+
             return self::SUCCESS;
         }
 
@@ -64,30 +67,32 @@ public function handle(): int
                 // Skip if next start exceeds event's end_date
                 if ($nextStart->gt($event->end_date)) {
                     $bar->advance();
+
                     continue;
                 }
 
-                // Avoid duplicates: check for same parent + same next date
-                $exists = Event::query()
-                    ->where('source_ref', 'parent:' . $event->id)
-                    ->whereDate('start_date', $nextStart->toDateString())
-                    ->exists();
+                if (! $dryRun) {
+                    $this->rekeyLegacyChild($event);
+                }
 
-                if ($exists) {
+                $sourceRef = $this->occurrenceSourceRef($event->id, $nextStart);
+
+                if ($this->occurrenceExists($event->id, $nextStart, $sourceRef)) {
                     $bar->advance();
+
                     continue;
                 }
 
                 // Clone event data
                 $newEventData = $event->replicate([
-                    'id', 'created_at', 'updated_at'
+                    'id', 'created_at', 'updated_at',
                 ])->toArray();
 
                 $durationSeconds = Carbon::parse($event->end_date)->diffInSeconds(Carbon::parse($event->start_date));
 
                 $newEventData['start_date'] = $nextStart;
                 $newEventData['end_date'] = $nextStart->copy()->addSeconds($durationSeconds);
-                $newEventData['source_ref'] = 'parent:' . $event->id;
+                $newEventData['source_ref'] = $sourceRef;
                 $newEventData['status'] = $event->status;
 
                 if ($dryRun) {
@@ -145,4 +150,42 @@ private function getNextDate(Carbon $startDate, string $recurrence): Carbon
 
         return $next;
     }
+
+    private function occurrenceSourceRef(int $parentId, Carbon $start): string
+    {
+        return sprintf('parent:%d:%s', $parentId, $start->toDateString());
+    }
+
+    private function occurrenceExists(int $parentId, Carbon $start, string $sourceRef): bool
+    {
+        return Event::query()
+            ->where(function ($query) use ($parentId, $sourceRef) {
+                $query->where('source_ref', $sourceRef)
+                    ->orWhere('source_ref', 'parent:'.$parentId)
+                    ->orWhere('source_ref', 'like', 'parent:'.$parentId.':%');
+            })
+            ->whereDate('start_date', $start->toDateString())
+            ->exists();
+    }
+
+    /**
+     * Older runs wrote a single "parent:{id}" child. That value is unique, so it
+     * blocks every later occurrence. Rewrite it to the dated form once.
+     */
+    private function rekeyLegacyChild(Event $event): void
+    {
+        $legacy = Event::query()
+            ->where('source_ref', 'parent:'.$event->id)
+            ->first();
+
+        if ($legacy === null) {
+            return;
+        }
+
+        $legacy->source_ref = $this->occurrenceSourceRef(
+            $event->id,
+            Carbon::parse($legacy->start_date)
+        );
+        $legacy->save();
+    }
 }

tests/Feature/GenerateRecurringEventsTest.php

@@ -0,0 +1,96 @@
+<?php
+
+namespace Tests\Feature;
+
+use App\Event;
+use Carbon\Carbon;
+use Illuminate\Foundation\Testing\RefreshDatabase;
+use Illuminate\Support\Facades\Artisan;
+use PHPUnit\Framework\Attributes\Test;
+use Tests\TestCase;
+
+class GenerateRecurringEventsTest extends TestCase
+{
+    use RefreshDatabase;
+
+    private function makeWeeklyParent(): Event
+    {
+        return Event::factory()->create([
+            'status' => 'APPROVED',
+            'title' => 'Weekly Coding Club',
+            'recurring_event' => 'weekly',
+            'start_date' => Carbon::now()->subWeeks(4)->startOfDay()->setTime(9, 0),
+            'end_date' => Carbon::now()->addMonths(2)->endOfDay(),
+            'source_ref' => null,
+        ]);
+    }
+
+    #[Test]
+    public function it_writes_a_dated_source_ref_so_later_occurrences_can_coexist(): void
+    {
+        $parent = $this->makeWeeklyParent();
+
+        Artisan::call('events:generate-recurring');
+
+        $child = Event::query()
+            ->where('source_ref', 'like', 'parent:'.$parent->id.':%')
+            ->first();
+
+        $this->assertNotNull($child);
+        $this->assertMatchesRegularExpression(
+            '/^parent:'.$parent->id.':\d{4}-\d{2}-\d{2}$/',
+            $child->source_ref
+        );
+    }
+
+    #[Test]
+    public function it_rekeys_a_legacy_parent_ref_and_creates_the_next_occurrence(): void
+    {
+        $parent = $this->makeWeeklyParent();
+
+        // The unique index on source_ref meant the first implementation could only
+        // ever store one child, keyed as the bare "parent:{id}".
+        $legacyStart = Carbon::now()->subWeeks(3)->startOfDay()->setTime(9, 0);
+        $legacy = Event::factory()->create([
+            'status' => 'APPROVED',
+            'title' => 'Weekly Coding Club (legacy child)',
+            'recurring_event' => 'weekly',
+            'start_date' => $legacyStart,
+            'end_date' => $legacyStart->copy()->addHours(2),
+            'source_ref' => 'parent:'.$parent->id,
+        ]);
+
+        Artisan::call('events:generate-recurring');
+
+        $legacy->refresh();
+        $this->assertSame(
+            'parent:'.$parent->id.':'.$legacyStart->toDateString(),
+            $legacy->source_ref,
+            'The legacy row must be rewritten so it no longer blocks the unique index.'
+        );
+
+        $children = Event::query()
+            ->where('source_ref', 'like', 'parent:'.$parent->id.':%')
+            ->orderBy('start_date')
+            ->get();
+
+        $this->assertGreaterThanOrEqual(2, $children->count());
+        $this->assertTrue(
+            $children->contains(fn (Event $event) => $event->id === $legacy->id)
+        );
+    }
+
+    #[Test]
+    public function running_twice_does_not_duplicate_the_same_occurrence(): void
+    {
+        $parent = $this->makeWeeklyParent();
+
+        Artisan::call('events:generate-recurring');
+        Artisan::call('events:generate-recurring');
+
+        $this->assertSame(
+            1,
+            Event::query()->where('source_ref', 'like', 'parent:'.$parent->id.':%')->count()
+        );
+    }
+}