fix: soft delete bug + workflow

RustyBraze · RustyBraze · commit eb0a033915ef · 2026-06-14T17:42:44.000+02:00
diff --git a/.github/workflows/docker.yml b/.github/workflows/docker.yml
@@ -6,8 +6,7 @@ on:
     tags:
       - 'v[0-9]+.[0-9]+.[0-9]'
       - 'v[0-9]+.[0-9]+.[0-9]-**'
-      - 'v*.*.*-*'
-      - 'v*'
+
 jobs:
   build-and-push-image:
     uses: eurofurence/workflows/.github/workflows/docker-buildx-push.yml@main
diff --git a/app/Models/Fursuit/Fursuit.php b/app/Models/Fursuit/Fursuit.php
@@ -4,7 +4,6 @@
 
 use App\Models\Badge\Badge;
 use App\Models\Event;
-use App\Models\EventUser;
 use App\Models\FCEA\UserCatchLog;
 use App\Models\Fursuit\States\FursuitStatusState;
 use App\Models\Species;
@@ -26,6 +25,12 @@ class Fursuit extends Model
 {
     use HasFactory, HasStates, LogsActivity, SoftDeletes;
 
+    /**
+     * Guard flag set while a fursuit is cascading its own deletion to its badges, so the badge
+     * delete events don't try to (re-)delete the fursuit. See FursuitObserver / BadgeObserver.
+     */
+    public static bool $isCascadingDelete = false;
+
     protected $guarded = [];
 
     protected $casts = [
diff --git a/app/Observers/BadgeObserver.php b/app/Observers/BadgeObserver.php
@@ -3,9 +3,44 @@
 namespace App\Observers;
 
 use App\Models\Badge\Badge;
+use App\Models\Fursuit\Fursuit;
 
 class BadgeObserver
 {
+    /**
+     * Keep the soft-delete state consistent between a badge and its connected rows
+     * (spare copies + the parent fursuit). Fires for soft deletes only — a force delete
+     * deliberately removes the row, so it is left untouched. See docs/bugfix-04-fix.md.
+     */
+    public function deleted(Badge $badge): void
+    {
+        if ($badge->isForceDeleting()) {
+            return;
+        }
+
+        // When a whole fursuit is cascading, it already soft-deletes every one of its badges
+        // (main + copies), so skip per-badge cascading to avoid redundant work / recursion.
+        if (Fursuit::$isCascadingDelete) {
+            return;
+        }
+
+        // Soft-delete the spare copies of this main badge so they aren't left orphaned.
+        // newQuery() excludes already soft-deleted copies, so this is idempotent.
+        if ($badge->extra_copy_of === null) {
+            $badge->newQuery()
+                ->where('extra_copy_of', $badge->id)
+                ->get()
+                ->each
+                ->delete();
+        }
+
+        // If the parent fursuit no longer has any active badge, soft-delete it too.
+        $fursuit = $badge->fursuit;
+        if ($fursuit && ! $fursuit->trashed() && $fursuit->badges()->count() === 0) {
+            $fursuit->delete();
+        }
+    }
+
     public function updated(Badge $badge): void
     {
         // Based on tax_rate, calculate tax and update subtotal
diff --git a/app/Observers/FursuitObserver.php b/app/Observers/FursuitObserver.php
@@ -7,6 +7,34 @@
 
 class FursuitObserver
 {
+    /**
+     * Cascade a fursuit soft-delete to its badges so they aren't left orphaned with a NULL
+     * deleted_at. See docs/bugfix-04-fix.md.
+     */
+    public function deleting(Fursuit $fursuit): bool
+    {
+        // A force delete deliberately removes the row (and the FK cascade removes its badges);
+        // leave that logic untouched.
+        if ($fursuit->isForceDeleting()) {
+            return true;
+        }
+
+        // Re-entrant soft delete on an already-trashed fursuit (e.g. the public controller's
+        // redundant cleanup after the badge cascade already deleted it): abort cleanly.
+        if ($fursuit->trashed()) {
+            return false;
+        }
+
+        Fursuit::$isCascadingDelete = true;
+        try {
+            $fursuit->badges()->get()->each->delete();
+        } finally {
+            Fursuit::$isCascadingDelete = false;
+        }
+
+        return true;
+    }
+
     public function created(Fursuit $fursuit): void
     {
         if ($fursuit->catch_em_all === true) {
diff --git a/database/migrations/2026_06_14_120000_soft_delete_orphaned_fursuits_and_badges.php b/database/migrations/2026_06_14_120000_soft_delete_orphaned_fursuits_and_badges.php
@@ -0,0 +1,47 @@
+<?php
+
+use Illuminate\Database\Migrations\Migration;
+use Illuminate\Support\Facades\DB;
+
+/**
+ * Backfill for bugfix-04: badge/fursuit soft-deletes were not cascaded, leaving orphans
+ * (20+ fursuits with no remaining badges, and possibly badges under already-deleted fursuits).
+ *
+ * Repairs the existing inconsistency by soft-deleting the orphaned rows. Idempotent: both steps
+ * are WHERE-guarded UPDATEs that only touch still-inconsistent rows, so re-running converges
+ * (safe under the ArgoCD PreSync migrate job — see CLAUDE.md "Migrations must be idempotent").
+ */
+return new class extends Migration
+{
+    public function up(): void
+    {
+        $now = now();
+
+        // 1) Soft-delete active fursuits that have no remaining (non-trashed) badge.
+        DB::table('fursuits')
+            ->whereNull('deleted_at')
+            ->whereNotExists(function ($query) {
+                $query->select(DB::raw(1))
+                    ->from('badges')
+                    ->whereColumn('badges.fursuit_id', 'fursuits.id')
+                    ->whereNull('badges.deleted_at');
+            })
+            ->update(['deleted_at' => $now, 'updated_at' => $now]);
+
+        // 2) Symmetrically, soft-delete active badges whose parent fursuit is already trashed.
+        DB::table('badges')
+            ->whereNull('deleted_at')
+            ->whereExists(function ($query) {
+                $query->select(DB::raw(1))
+                    ->from('fursuits')
+                    ->whereColumn('fursuits.id', 'badges.fursuit_id')
+                    ->whereNotNull('fursuits.deleted_at');
+            })
+            ->update(['deleted_at' => $now, 'updated_at' => $now]);
+    }
+
+    public function down(): void
+    {
+        // Data repair only; not reversible.
+    }
+};
diff --git a/tests/Feature/SoftDeleteCascadeTest.php b/tests/Feature/SoftDeleteCascadeTest.php
@@ -0,0 +1,137 @@
+<?php
+
+use App\Models\Badge\Badge;
+use App\Models\Event;
+use App\Models\EventUser;
+use App\Models\Fursuit\Fursuit;
+use App\Models\User;
+use Illuminate\Foundation\Testing\RefreshDatabase;
+use Illuminate\Support\Facades\DB;
+
+use function Pest\Laravel\actingAs;
+use function Pest\Laravel\delete;
+
+uses(RefreshDatabase::class);
+
+function badgeDeletedAt(Badge $badge)
+{
+    return DB::table('badges')->where('id', $badge->id)->value('deleted_at');
+}
+
+function fursuitDeletedAt(Fursuit $fursuit)
+{
+    return DB::table('fursuits')->where('id', $fursuit->id)->value('deleted_at');
+}
+
+test('soft-deleting a fursuit cascades to its badges', function () {
+    $fursuit = Fursuit::factory()->create();
+    $b1 = Badge::factory()->for($fursuit)->create(['extra_copy_of' => null]);
+    $b2 = Badge::factory()->for($fursuit)->create(['extra_copy_of' => null]);
+
+    $fursuit->delete();
+
+    expect(fursuitDeletedAt($fursuit))->not->toBeNull();
+    expect(badgeDeletedAt($b1))->not->toBeNull();
+    expect(badgeDeletedAt($b2))->not->toBeNull();
+});
+
+test('soft-deleting the last badge cascades to its fursuit', function () {
+    $fursuit = Fursuit::factory()->create();
+    $badge = Badge::factory()->for($fursuit)->create(['extra_copy_of' => null]);
+
+    $badge->delete();
+
+    expect(badgeDeletedAt($badge))->not->toBeNull();
+    expect(fursuitDeletedAt($fursuit))->not->toBeNull();
+});
+
+test('deleting one badge keeps the fursuit while another active badge remains', function () {
+    $fursuit = Fursuit::factory()->create();
+    $b1 = Badge::factory()->for($fursuit)->create(['extra_copy_of' => null]);
+    $b2 = Badge::factory()->for($fursuit)->create(['extra_copy_of' => null]);
+
+    $b1->delete();
+
+    expect(badgeDeletedAt($b1))->not->toBeNull();
+    expect(fursuitDeletedAt($fursuit))->toBeNull();
+    expect(badgeDeletedAt($b2))->toBeNull();
+});
+
+test('soft-deleting a main badge cascades to its spare copies and the empty fursuit', function () {
+    $fursuit = Fursuit::factory()->create();
+    $main = Badge::factory()->for($fursuit)->create(['extra_copy_of' => null]);
+    $copy = Badge::factory()->for($fursuit)->create(['extra_copy_of' => $main->id, 'extra_copy' => true]);
+
+    $main->delete();
+
+    expect(badgeDeletedAt($main))->not->toBeNull();
+    expect(badgeDeletedAt($copy))->not->toBeNull();
+    expect(fursuitDeletedAt($fursuit))->not->toBeNull();
+});
+
+test('public badge deletion soft-deletes the badge and its now-empty fursuit', function () {
+    $event = Event::factory()->create([
+        'starts_at' => now()->subDays(5),
+        'ends_at' => now()->addDays(20),
+        'order_starts_at' => now()->subDays(5),
+        'order_ends_at' => now()->addDays(20),
+    ]);
+    $user = User::factory()->create();
+    EventUser::create([
+        'user_id' => $user->id,
+        'event_id' => $event->id,
+        'attendee_id' => 'TEST-'.$user->id,
+        'valid_registration' => true,
+        'prepaid_badges' => 0,
+    ]);
+
+    $fursuit = Fursuit::factory()->recycle($user)->recycle($event)->create(['status' => 'approved']);
+    $badge = Badge::factory()->for($fursuit)->create([
+        'status_fulfillment' => 'pending',
+        'extra_copy_of' => null,
+    ]);
+
+    actingAs($user);
+    delete(route('badges.destroy', $badge))->assertRedirect();
+
+    expect(badgeDeletedAt($badge))->not->toBeNull();
+    expect(fursuitDeletedAt($fursuit))->not->toBeNull();
+});
+
+test('force-deleting a badge removes the row and does not cascade to the fursuit', function () {
+    $fursuit = Fursuit::factory()->create();
+    $badge = Badge::factory()->for($fursuit)->create(['extra_copy_of' => null]);
+
+    $badge->forceDelete();
+
+    expect(Badge::withTrashed()->find($badge->id))->toBeNull();
+    // Force delete deliberately removes the row; the soft-delete cascade does not run.
+    expect(fursuitDeletedAt($fursuit))->toBeNull();
+});
+
+test('backfill migration soft-deletes existing orphans and leaves healthy rows alone', function () {
+    // Orphan A: active fursuit whose only badge was soft-deleted without cascade (legacy state).
+    $fursuitA = Fursuit::factory()->create();
+    $badgeA = Badge::factory()->for($fursuitA)->create(['extra_copy_of' => null]);
+    DB::table('badges')->where('id', $badgeA->id)->update(['deleted_at' => now()]);
+
+    // Orphan B: trashed fursuit with a still-active badge (the other direction).
+    $fursuitB = Fursuit::factory()->create();
+    $badgeB = Badge::factory()->for($fursuitB)->create(['extra_copy_of' => null]);
+    DB::table('fursuits')->where('id', $fursuitB->id)->update(['deleted_at' => now()]);
+
+    // Healthy: active fursuit with an active badge — must be left untouched.
+    $healthy = Fursuit::factory()->create();
+    Badge::factory()->for($healthy)->create(['extra_copy_of' => null]);
+
+    expect(fursuitDeletedAt($fursuitA))->toBeNull();
+    expect(badgeDeletedAt($badgeB))->toBeNull();
+
+    $migration = include database_path('migrations/2026_06_14_120000_soft_delete_orphaned_fursuits_and_badges.php');
+    $migration->up();
+    $migration->up(); // idempotent — running twice changes nothing further
+
+    expect(fursuitDeletedAt($fursuitA))->not->toBeNull();
+    expect(badgeDeletedAt($badgeB))->not->toBeNull();
+    expect(fursuitDeletedAt($healthy))->toBeNull();
+});
