feat(integration-requests): add done status and fix review command crash on orphaned author

The review command rendered $request->user->email directly, throwing
"Attempt to read property email on null" whenever a request's author no
longer exists. Fall back to a dash instead.

Add a terminal "done" status that marks a request as shipped: it is shown
on the board (sinking to the bottom like not-doable), can no longer be
voted on or unvoted, and drops its public comment when set.

app/Console/Commands/ReviewIntegrationRequestsCommand.php

@@ -42,7 +42,7 @@ class ReviewIntegrationRequestsCommand extends Command
         foreach ($pending as $request) {
             $decision = $this->choice(
                 "Review \"{$request->name}\" ({$request->url})",
-                ['approve', 'in progress', 'reject', 'not doable', 'skip'],
+                ['approve', 'in progress', 'reject', 'not doable', 'done', 'skip'],
                 'skip',
             );
 
@@ -78,7 +78,7 @@ class ReviewIntegrationRequestsCommand extends Command
 
         $this->apply($request, $this->choice(
             "New status for \"{$request->name}\"",
-            ['approve', 'in progress', 'reject', 'not doable'],
+            ['approve', 'in progress', 'reject', 'not doable', 'done'],
         ));
 
         $this->info("\"{$request->name}\" is now {$request->status->label()}.");
@@ -93,6 +93,7 @@ class ReviewIntegrationRequestsCommand extends Command
             'in progress' => IntegrationRequestStatus::InProgress,
             'reject' => IntegrationRequestStatus::Rejected,
             'not doable' => IntegrationRequestStatus::NotDoable,
+            'done' => IntegrationRequestStatus::Done,
             default => null,
         };
 
@@ -124,7 +125,7 @@ class ReviewIntegrationRequestsCommand extends Command
             return $this->ask('Add a comment for this request (optional, shown to users)') ?: null;
         }
 
-        // Approving, rejecting or re-queuing drops any stale public comment.
+        // Approving, rejecting, re-queuing or marking done drops any stale public comment.
         return null;
     }
 
@@ -157,7 +158,7 @@ class ReviewIntegrationRequestsCommand extends Command
                 $request->name,
                 $request->url,
                 $request->status->label(),
-                $request->user->email,
+                $request->user?->email ?? '—',
                 $request->votes_count,
                 $request->created_at?->format('Y-m-d') ?? '—',
             ];

app/Enums/IntegrationRequestStatus.php

@@ -9,6 +9,7 @@ enum IntegrationRequestStatus: string
     case InProgress = 'in_progress';
     case Rejected = 'rejected';
     case NotDoable = 'not_doable';
+    case Done = 'done';
 
     public function label(): string
     {
@@ -18,6 +19,7 @@ enum IntegrationRequestStatus: string
             self::InProgress => 'In progress',
             self::Rejected => 'Rejected',
             self::NotDoable => 'Not doable',
+            self::Done => 'Done',
         };
     }
 }

app/Http/Controllers/IntegrationRequestController.php

@@ -79,8 +79,8 @@ class IntegrationRequestController extends Controller
     {
         $user = $request->user();
 
-        // Not-doable requests are frozen: their tally can no longer be touched.
-        if ($integrationRequest->status === IntegrationRequestStatus::NotDoable) {
+        // Closed requests (not-doable or done) are frozen: their tally can no longer be touched.
+        if (in_array($integrationRequest->status, [IntegrationRequestStatus::NotDoable, IntegrationRequestStatus::Done], true)) {
             abort(404);
         }
 
@@ -106,7 +106,7 @@ class IntegrationRequestController extends Controller
     {
         return IntegrationRequest::query()
             ->where(function ($query) use ($user) {
-                $query->whereIn('status', [IntegrationRequestStatus::Approved, IntegrationRequestStatus::InProgress, IntegrationRequestStatus::NotDoable])
+                $query->whereIn('status', [IntegrationRequestStatus::Approved, IntegrationRequestStatus::InProgress, IntegrationRequestStatus::NotDoable, IntegrationRequestStatus::Done])
                     ->orWhere(function ($inner) use ($user) {
                         $inner->where('status', IntegrationRequestStatus::Pending)
                             ->where('user_id', $user->id);
@@ -118,8 +118,8 @@ class IntegrationRequestController extends Controller
                 'votes as can_unvote' => fn ($query) => $query->where('user_id', $user->id)
                     ->where('created_at', '>=', now()->startOfMonth()),
             ])
-            // Not-doable requests sink to the bottom regardless of their votes.
-            ->orderByRaw('CASE WHEN status = ? THEN 1 ELSE 0 END', [IntegrationRequestStatus::NotDoable->value])
+            // Closed requests (not-doable or done) sink to the bottom regardless of their votes.
+            ->orderByRaw('CASE WHEN status IN (?, ?) THEN 1 ELSE 0 END', [IntegrationRequestStatus::NotDoable->value, IntegrationRequestStatus::Done->value])
             ->orderByDesc('votes_count')
             ->orderByDesc('created_at')
             ->get();

database/factories/IntegrationRequestFactory.php

@@ -47,4 +47,9 @@ class IntegrationRequestFactory extends Factory
             'comment' => fake()->sentence(),
         ]);
     }
+
+    public function done(): static
+    {
+        return $this->state(['status' => IntegrationRequestStatus::Done]);
+    }
 }

lang/es.json

@@ -35,6 +35,7 @@
     "Pending review": "Pendiente de revisión",
     "Not doable": "No viable",
     "In progress": "En proceso",
+    "Done": "Hecho",
     "Something went wrong.": "Algo salió mal.",
     "Integration requests": "Solicitudes de integración",
     "Request integration": "Solicitar integración",

resources/js/components/integration-requests/integration-requests-board.tsx

@@ -21,7 +21,13 @@ export interface IntegrationRequestItem {
     id: string;
     name: string;
     url: string;
-    status: 'pending' | 'approved' | 'in_progress' | 'rejected' | 'not_doable';
+    status:
+        | 'pending'
+        | 'approved'
+        | 'in_progress'
+        | 'rejected'
+        | 'not_doable'
+        | 'done';
     comment: string | null;
     votes_count: number;
     has_voted: boolean;
@@ -34,6 +40,11 @@ interface BoardPayload {
     actionsRemaining: number;
 }
 
+// Closed requests (not-doable or done) are frozen: no more votes in or out.
+function isFrozen(status: IntegrationRequestItem['status']): boolean {
+    return status === 'not_doable' || status === 'done';
+}
+
 interface Props {
     initialRequests?: IntegrationRequestItem[];
     initialActionsRemaining?: number;
@@ -115,7 +126,7 @@ export function IntegrationRequestsBoard({
     };
 
     const handleVote = async (item: IntegrationRequestItem) => {
-        if (busy || item.status === 'not_doable' || actionsRemaining <= 0) {
+        if (busy || isFrozen(item.status) || actionsRemaining <= 0) {
             return;
         }
 
@@ -138,7 +149,7 @@ export function IntegrationRequestsBoard({
     };
 
     const handleRemoveVote = async (item: IntegrationRequestItem) => {
-        if (busy || !item.can_unvote || item.status === 'not_doable') {
+        if (busy || !item.can_unvote || isFrozen(item.status)) {
             return;
         }
 
@@ -271,10 +282,13 @@ export function IntegrationRequestsBoard({
                                             {__('Not doable')}
                                         </Badge>
                                     )}
+                                    {item.status === 'done' && (
+                                        <Badge>{__('Done')}</Badge>
+                                    )}
                                 </div>
                                 <div className="flex items-center gap-1">
                                     {item.can_unvote &&
-                                        item.status !== 'not_doable' && (
+                                        !isFrozen(item.status) && (
                                             <Button
                                                 variant="ghost"
                                                 size="sm"
@@ -298,7 +312,7 @@ export function IntegrationRequestsBoard({
                                         size="sm"
                                         disabled={
                                             busy ||
-                                            item.status === 'not_doable' ||
+                                            isFrozen(item.status) ||
                                             outOfActions
                                         }
                                         onClick={() => handleVote(item)}

tests/Feature/IntegrationRequestTest.php

@@ -3,6 +3,8 @@
 use App\Enums\IntegrationRequestStatus;
 use App\Models\IntegrationRequest;
 use App\Models\User;
+use Illuminate\Support\Facades\Schema;
+use Illuminate\Support\Str;
 
 // With subscriptions enabled, a freshly created user is on the free plan
 // (three monthly actions). Subscribed users get the pro quota of nine.
@@ -247,7 +249,7 @@ test('the review command approves a pending request', function () {
         ->expectsChoice(
             "Review \"{$request->name}\" ({$request->url})",
             'approve',
-            ['approve', 'in progress', 'reject', 'not doable', 'skip'],
+            ['approve', 'in progress', 'reject', 'not doable', 'done', 'skip'],
         )
         ->assertSuccessful();
 
@@ -261,7 +263,7 @@ test('the review command rejects a pending request', function () {
         ->expectsChoice(
             "Review \"{$request->name}\" ({$request->url})",
             'reject',
-            ['approve', 'in progress', 'reject', 'not doable', 'skip'],
+            ['approve', 'in progress', 'reject', 'not doable', 'done', 'skip'],
         )
         ->assertSuccessful();
 
@@ -344,7 +346,7 @@ test('the review command marks a request as not doable with a comment', function
         ->expectsChoice(
             "Review \"{$request->name}\" ({$request->url})",
             'not doable',
-            ['approve', 'in progress', 'reject', 'not doable', 'skip'],
+            ['approve', 'in progress', 'reject', 'not doable', 'done', 'skip'],
         )
         ->expectsQuestion('Why is this integration not doable? (shown to users)', 'No public API available.')
         ->assertSuccessful();
@@ -362,7 +364,7 @@ test('the review command with --all moves any request to a new status with a com
         ->expectsChoice(
             'New status for "Revolut"',
             'in progress',
-            ['approve', 'in progress', 'reject', 'not doable'],
+            ['approve', 'in progress', 'reject', 'not doable', 'done'],
         )
         ->expectsQuestion('Add a comment for this request (optional, shown to users)', 'Building it now.')
         ->assertSuccessful();
@@ -397,3 +399,80 @@ test('a user can vote on an in progress request', function () {
         'integration_request_id' => $request->id,
     ]);
 });
+
+test('done requests are visible to everyone without a comment', function () {
+    IntegrationRequest::factory()->done()->create(['name' => 'Shipped Bank']);
+
+    $this->actingAs(User::factory()->create())
+        ->getJson('/integration-requests/data')
+        ->assertOk()
+        ->assertJsonCount(1, 'requests')
+        ->assertJsonPath('requests.0.status', 'done')
+        ->assertJsonPath('requests.0.comment', null);
+});
+
+test('done requests are listed after the rest', function () {
+    IntegrationRequest::factory()->done()->create(['name' => 'Done']);
+    $approved = IntegrationRequest::factory()->approved()->create(['name' => 'Approved']);
+
+    $this->actingAs(User::factory()->create())
+        ->getJson('/integration-requests/data')
+        ->assertOk()
+        ->assertJsonPath('requests.0.id', $approved->id)
+        ->assertJsonPath('requests.1.name', 'Done');
+});
+
+test('nobody can vote on a done request', function () {
+    $request = IntegrationRequest::factory()->done()->create();
+
+    $this->actingAs(User::factory()->create())
+        ->postJson("/integration-requests/{$request->id}/vote")
+        ->assertNotFound();
+});
+
+test('a vote on a request later marked done cannot be removed', function () {
+    $user = User::factory()->create();
+    $request = IntegrationRequest::factory()->approved()->create();
+    $request->votes()->create(['user_id' => $user->id]);
+
+    $request->update(['status' => IntegrationRequestStatus::Done]);
+
+    $this->actingAs($user)
+        ->deleteJson("/integration-requests/{$request->id}/vote")
+        ->assertNotFound();
+
+    expect($request->votes()->count())->toBe(1);
+});
+
+test('the review command marks a request as done and drops its comment', function () {
+    $request = IntegrationRequest::factory()->inProgress()->create([
+        'name' => 'Revolut',
+        'comment' => 'Working on it.',
+    ]);
+
+    $this->artisan('integration-requests:review --all')
+        ->expectsQuestion('Which request do you want to update? (number)', '1')
+        ->expectsChoice(
+            'New status for "Revolut"',
+            'done',
+            ['approve', 'in progress', 'reject', 'not doable', 'done'],
+        )
+        ->assertSuccessful();
+
+    $request->refresh();
+    expect($request->status)->toBe(IntegrationRequestStatus::Done);
+    expect($request->comment)->toBeNull();
+});
+
+test('the review command lists requests whose author no longer exists', function () {
+    $request = IntegrationRequest::factory()->approved()->create();
+
+    Schema::withoutForeignKeyConstraints(function () use ($request) {
+        $request->forceFill(['user_id' => (string) Str::uuid()])->saveQuietly();
+    });
+
+    $this->artisan('integration-requests:review --all')
+        ->expectsQuestion('Which request do you want to update? (number)', '0')
+        ->expectsOutput('That number is not on the list.')
+        ->assertFailed();
+});