fix: take the level check's unique lock after the commit is saved

app/Actions/CreateCommit.php

@@ -101,8 +101,13 @@ public function handle(User $user, string $body, ?Commit $parent = null, ?Commit
                 }
             }
 
+            // A unique job takes its lock when it is dispatched, even with afterCommit(). With the database cache, a lock
+            // still held by an earlier check would fail its insert and abort this transaction on Postgres, so the job is
+            // only dispatched once the commit is saved.
             if ($commit->status === CommitStatus::Approved) {
-                dispatch(new CheckUserLevel($user))->afterCommit();
+                DB::afterCommit(function () use ($user): void {
+                    dispatch(new CheckUserLevel($user));
+                });
             }
 
             if ($commit->status === CommitStatus::Pending && ! $hasGoodReputation) {

app/Actions/UpdateCommitStatuses.php

@@ -94,8 +94,15 @@ public function handle(array $commitIds, CommitStatus $status, ?string $reason =
                     }
                 }
 
-                User::query()->whereKey($commits->pluck('user_id')->filter()->unique()->values()->all())->get()
-                    ->each(fn (User $author) => dispatch(new CheckUserLevel($author))->afterCommit());
+                // Dispatched once the statuses are saved, as CreateCommit does, so a check still holding its unique lock
+                // can't abort the approval.
+                $authors = User::query()->whereKey($commits->pluck('user_id')->filter()->unique()->values()->all())->get();
+
+                DB::afterCommit(function () use ($authors): void {
+                    foreach ($authors as $author) {
+                        dispatch(new CheckUserLevel($author));
+                    }
+                });
             }
         });
     }

tests/Feature/Controllers/CommitApprovalControllerTest.php

@@ -5,6 +5,8 @@
 use App\Enums\CommitStatus;
 use App\Models\Commit;
 use App\Models\User;
+use Illuminate\Database\Events\QueryExecuted;
+use Illuminate\Support\Facades\DB;
 
 beforeEach(function (): void {
     config(['app.admins' => ['admin']]);
@@ -23,6 +25,29 @@
     expect($commit->refresh()->status)->toBe(CommitStatus::Approved);
 });
 
+it('takes the unique lock of the level check of the author only once the approval is saved', function (): void {
+    // On Postgres, a lock row left by an earlier check makes the insert fail and aborts the transaction it runs in.
+    config(['cache.default' => 'database']);
+    $admin = User::factory()->create(['username' => 'admin']);
+    $commit = Commit::factory()->pending()->create();
+    $outside = DB::transactionLevel();
+    $levels = [];
+    DB::listen(function (QueryExecuted $query) use (&$levels): void {
+        if (str_contains($query->sql, 'cache_locks')) {
+            $levels[] = DB::transactionLevel();
+        }
+    });
+
+    $this->actingAs($admin)
+        ->fromRoute('moderation.show')
+        ->post(route('commit-approval.store', $commit))
+        ->assertRedirectToRoute('moderation.show');
+
+    expect($commit->refresh()->status)->toBe(CommitStatus::Approved)
+        ->and($levels)->not->toBeEmpty()
+        ->and(array_unique($levels))->toBe([$outside]);
+});
+
 it('may not approve a deleted commit', function (): void {
     $admin = User::factory()->create(['username' => 'admin']);
     $commit = Commit::factory()->pending()->create();

tests/Feature/Controllers/CommitControllerTest.php

@@ -15,6 +15,7 @@
 use App\Models\Notification;
 use App\Models\User;
 use Illuminate\Database\Eloquent\Factories\Sequence;
+use Illuminate\Database\Events\QueryExecuted;
 use Illuminate\Http\UploadedFile;
 use Illuminate\Support\Facades\DB;
 use Illuminate\Support\Facades\Queue;
@@ -35,6 +36,25 @@
     expect($user->commits()->sole()->body)->toBe('Hello, devnull!');
 });
 
+it('takes the unique lock of the level check only once the commit is saved', function (): void {
+    // On Postgres, a lock row left by an earlier check makes the insert fail and aborts the transaction it runs in.
+    config(['cache.default' => 'database']);
+    $user = User::factory()->create();
+    config(['app.admins' => [$user->username]]);
+    $outside = DB::transactionLevel();
+    $levels = [];
+    DB::listen(function (QueryExecuted $query) use (&$levels): void {
+        if (str_contains($query->sql, 'cache_locks')) {
+            $levels[] = DB::transactionLevel();
+        }
+    });
+
+    $this->actingAs($user)->post(route('commit.store'), ['body' => 'It was DNS.'])->assertSessionHasNoErrors();
+
+    expect($levels)->not->toBeEmpty()
+        ->and(array_unique($levels))->toBe([$outside]);
+});
+
 it('flashes the created commit', function (): void {
     $user = User::factory()->create();