fix: review pack generation UX + notifications

This commit is contained in:
Ahmed Darrazi 2026-02-23 17:57:29 +01:00
parent 58f5519e94
commit dc112664df
10 changed files with 209 additions and 208 deletions

View File

@ -10,6 +10,7 @@
use App\Support\Auth\Capabilities; use App\Support\Auth\Capabilities;
use App\Support\Badges\BadgeDomain; use App\Support\Badges\BadgeDomain;
use App\Support\Badges\BadgeRenderer; use App\Support\Badges\BadgeRenderer;
use App\Support\OpsUx\OperationUxPresenter;
use App\Support\Rbac\UiEnforcement; use App\Support\Rbac\UiEnforcement;
use App\Support\ReviewPackStatus; use App\Support\ReviewPackStatus;
use App\Support\Ui\ActionSurface\ActionSurfaceDeclaration; use App\Support\Ui\ActionSurface\ActionSurfaceDeclaration;
@ -329,8 +330,23 @@ public static function executeGeneration(array $data): void
'include_operations' => (bool) ($data['include_operations'] ?? true), 'include_operations' => (bool) ($data['include_operations'] ?? true),
]; ];
$service->generate($tenant, $user, $options); $reviewPack = $service->generate($tenant, $user, $options);
Notification::make()->success()->title('Review pack generation started.')->send(); if (! $reviewPack->wasRecentlyCreated) {
Notification::make()
->success()
->title('Review pack already available')
->body('A matching review pack is already ready. No new run was started.')
->actions([
Actions\Action::make('view_pack')
->label('View pack')
->url(static::getUrl('view', ['record' => $reviewPack], tenant: $tenant)),
])
->send();
return;
}
OperationUxPresenter::queuedToast('tenant.review_pack.generate')->send();
} }
} }

View File

@ -9,7 +9,7 @@
use App\Models\ReviewPack; use App\Models\ReviewPack;
use App\Models\StoredReport; use App\Models\StoredReport;
use App\Models\Tenant; use App\Models\Tenant;
use App\Notifications\ReviewPackStatusNotification; use App\Services\OperationRunService;
use App\Services\ReviewPackService; use App\Services\ReviewPackService;
use App\Support\OperationRunOutcome; use App\Support\OperationRunOutcome;
use App\Support\OperationRunStatus; use App\Support\OperationRunStatus;
@ -30,7 +30,7 @@ public function __construct(
public int $operationRunId, public int $operationRunId,
) {} ) {}
public function handle(): void public function handle(OperationRunService $operationRunService): void
{ {
$reviewPack = ReviewPack::query()->find($this->reviewPackId); $reviewPack = ReviewPack::query()->find($this->reviewPackId);
$operationRun = OperationRun::query()->find($this->operationRunId); $operationRun = OperationRun::query()->find($this->operationRunId);
@ -47,28 +47,25 @@ public function handle(): void
$tenant = $reviewPack->tenant; $tenant = $reviewPack->tenant;
if (! $tenant instanceof Tenant) { if (! $tenant instanceof Tenant) {
$this->markFailed($reviewPack, $operationRun, 'tenant_not_found', 'Tenant not found'); $this->markFailed($reviewPack, $operationRun, $operationRunService, 'tenant_not_found', 'Tenant not found');
return; return;
} }
// Mark running // Mark running via OperationRunService (auto-sets started_at)
$operationRun->update([ $operationRunService->updateRun($operationRun, OperationRunStatus::Running->value);
'status' => OperationRunStatus::Running->value,
'started_at' => now(),
]);
$reviewPack->update(['status' => ReviewPackStatus::Generating->value]); $reviewPack->update(['status' => ReviewPackStatus::Generating->value]);
try { try {
$this->executeGeneration($reviewPack, $operationRun, $tenant); $this->executeGeneration($reviewPack, $operationRun, $tenant, $operationRunService);
} catch (Throwable $e) { } catch (Throwable $e) {
$this->markFailed($reviewPack, $operationRun, 'generation_error', $e->getMessage()); $this->markFailed($reviewPack, $operationRun, $operationRunService, 'generation_error', $e->getMessage());
throw $e; throw $e;
} }
} }
private function executeGeneration(ReviewPack $reviewPack, OperationRun $operationRun, Tenant $tenant): void private function executeGeneration(ReviewPack $reviewPack, OperationRun $operationRun, Tenant $tenant, OperationRunService $operationRunService): void
{ {
$options = $reviewPack->options ?? []; $options = $reviewPack->options ?? [];
$includePii = (bool) ($options['include_pii'] ?? true); $includePii = (bool) ($options['include_pii'] ?? true);
@ -175,16 +172,13 @@ private function executeGeneration(ReviewPack $reviewPack, OperationRun $operati
'summary' => $summary, 'summary' => $summary,
]); ]);
// 13. Mark OperationRun completed // 13. Mark OperationRun completed (auto-sends OperationRunCompleted notification)
$operationRun->update([ $operationRunService->updateRun(
'status' => OperationRunStatus::Completed->value, $operationRun,
'outcome' => OperationRunOutcome::Succeeded->value, status: OperationRunStatus::Completed->value,
'completed_at' => now(), outcome: OperationRunOutcome::Succeeded->value,
'summary_counts' => $summary, summaryCounts: $summary,
]); );
// 14. Notify initiator
$this->notifyInitiator($reviewPack, 'ready');
} }
/** /**
@ -382,38 +376,17 @@ private function assembleZip(string $tempFile, array $fileMap): void
$zip->close(); $zip->close();
} }
private function markFailed(ReviewPack $reviewPack, OperationRun $operationRun, string $reasonCode, string $errorMessage): void private function markFailed(ReviewPack $reviewPack, OperationRun $operationRun, OperationRunService $operationRunService, string $reasonCode, string $errorMessage): void
{ {
$reviewPack->update(['status' => ReviewPackStatus::Failed->value]); $reviewPack->update(['status' => ReviewPackStatus::Failed->value]);
$operationRun->update([ $operationRunService->updateRun(
'status' => OperationRunStatus::Completed->value, $operationRun,
'outcome' => OperationRunOutcome::Failed->value, status: OperationRunStatus::Completed->value,
'completed_at' => now(), outcome: OperationRunOutcome::Failed->value,
'context' => array_merge($operationRun->context ?? [], [ failures: [
'reason_code' => $reasonCode, ['code' => $reasonCode, 'message' => mb_substr($errorMessage, 0, 500)],
'error_message' => mb_substr($errorMessage, 0, 500), ],
]), );
]);
$this->notifyInitiator($reviewPack, 'failed', $reasonCode);
}
private function notifyInitiator(ReviewPack $reviewPack, string $status, ?string $reasonCode = null): void
{
$initiator = $reviewPack->initiator;
if (! $initiator) {
return;
}
try {
$initiator->notify(new ReviewPackStatusNotification($reviewPack, $status, $reasonCode));
} catch (Throwable $e) {
Log::warning('Failed to send ReviewPack notification', [
'review_pack_id' => $reviewPack->getKey(),
'error' => $e->getMessage(),
]);
}
} }
} }

View File

@ -1,91 +0,0 @@
<?php
declare(strict_types=1);
namespace App\Notifications;
use App\Models\ReviewPack;
use App\Models\Tenant;
use Filament\Actions\Action;
use Illuminate\Notifications\Notification;
class ReviewPackStatusNotification extends Notification
{
public function __construct(
public ReviewPack $reviewPack,
public string $status,
public ?string $reasonCode = null,
) {}
/**
* @return array<int, string>
*/
public function via(object $notifiable): array
{
return ['database'];
}
/**
* @return array<string, mixed>
*/
public function toDatabase(object $notifiable): array
{
$title = match ($this->status) {
'ready' => 'Review Pack ready',
'failed' => 'Review Pack generation failed',
default => 'Review Pack status updated',
};
$body = match ($this->status) {
'ready' => 'Your tenant review pack has been generated and is ready for download.',
'failed' => sprintf(
'Review pack generation failed%s.',
$this->reasonCode ? ": {$this->reasonCode}" : '',
),
default => 'Review pack status changed.',
};
$color = match ($this->status) {
'ready' => 'success',
'failed' => 'danger',
default => 'gray',
};
$icon = match ($this->status) {
'ready' => 'heroicon-o-document-arrow-down',
'failed' => 'heroicon-o-exclamation-triangle',
default => 'heroicon-o-document-text',
};
$actions = [];
$tenant = $this->reviewPack->tenant;
if ($tenant instanceof Tenant && $this->status === 'ready') {
$actions[] = Action::make('view_pack')
->label('View pack')
->url(route('filament.admin.resources.review-packs.view', [
'tenant' => $tenant->external_id,
'record' => $this->reviewPack->getKey(),
]))
->toArray();
}
return [
'format' => 'filament',
'title' => $title,
'body' => $body,
'color' => $color,
'duration' => 'persistent',
'actions' => $actions,
'icon' => $icon,
'iconColor' => $color,
'status' => null,
'view' => null,
'viewData' => [
'review_pack_id' => $this->reviewPack->getKey(),
'status' => $this->status,
'reason_code' => $this->reasonCode,
],
];
}
}

View File

@ -56,10 +56,12 @@ public function generate(Tenant $tenant, User $user, array $options = []): Revie
'summary' => [], 'summary' => [],
]); ]);
$this->operationRunService->dispatchOrFail($operationRun, function () use ($reviewPack, $operationRun): void {
GenerateReviewPackJob::dispatch( GenerateReviewPackJob::dispatch(
reviewPackId: (int) $reviewPack->getKey(), reviewPackId: (int) $reviewPack->getKey(),
operationRunId: (int) $operationRun->getKey(), operationRunId: (int) $operationRun->getKey(),
); );
});
return $reviewPack; return $reviewPack;
} }

View File

@ -36,6 +36,9 @@ public static function all(): array
'report_created', 'report_created',
'report_deduped', 'report_deduped',
'alert_events_produced', 'alert_events_produced',
'finding_count',
'report_count',
'operation_count',
]; ];
} }
} }

View File

@ -6,14 +6,15 @@
return; return;
} }
const applyShim = () => {
const Livewire = window.Livewire; const Livewire = window.Livewire;
if (!Livewire || typeof Livewire.interceptMessage !== 'function') { if (!Livewire || typeof Livewire.interceptMessage !== 'function') {
return; return false;
} }
if (Livewire.__tenantpilotInterceptMessageShimApplied) { if (Livewire.__tenantpilotInterceptMessageShimApplied) {
return; return true;
} }
const original = Livewire.interceptMessage.bind(Livewire); const original = Livewire.interceptMessage.bind(Livewire);
@ -68,4 +69,30 @@
}; };
Livewire.__tenantpilotInterceptMessageShimApplied = true; Livewire.__tenantpilotInterceptMessageShimApplied = true;
return true;
};
if (applyShim()) {
return;
}
// Livewire may not be initialized yet when this script runs (depending on
// script tag order). Try again on `livewire:init` and with a short fallback poll.
const onInit = () => {
applyShim();
};
window.addEventListener('livewire:init', onInit, { once: true });
document.addEventListener('livewire:init', onInit, { once: true });
let tries = 0;
const maxTries = 50;
const timer = setInterval(() => {
tries += 1;
if (applyShim() || tries >= maxTries) {
clearInterval(timer);
}
}, 100);
})(); })();

View File

@ -184,7 +184,7 @@ ### Functional Requirements
### Canonical allowed summary keys (single source of truth) ### Canonical allowed summary keys (single source of truth)
The following keys are the ONLY allowed summary keys for Ops-UX rendering: The following keys are the ONLY allowed summary keys for Ops-UX rendering:
`total, processed, succeeded, failed, skipped, compliant, noncompliant, unknown, created, updated, deleted, items, tenants, high, medium, low, findings_created, findings_resolved, findings_reopened, findings_unchanged, errors_recorded, posture_score, report_created, report_deduped, alert_events_produced` `total, processed, succeeded, failed, skipped, compliant, noncompliant, unknown, created, updated, deleted, items, tenants, high, medium, low, findings_created, findings_resolved, findings_reopened, findings_unchanged, errors_recorded, posture_score, report_created, report_deduped, alert_events_produced, finding_count, report_count, operation_count`
All normalizers/renderers MUST reference this canonical list (no duplicated lists in multiple places). All normalizers/renderers MUST reference this canonical list (no duplicated lists in multiple places).

View File

@ -0,0 +1,16 @@
<?php
declare(strict_types=1);
it('injects the Livewire intercept shim into Filament pages', function (): void {
$this->get('/admin/login')
->assertSuccessful()
->assertSee('js/tenantpilot/livewire-intercept-shim.js', escape: false);
});
it('ships a shim that waits for Livewire initialization', function (): void {
$js = file_get_contents(public_path('js/tenantpilot/livewire-intercept-shim.js'));
expect($js)->toBeString();
expect($js)->toContain('livewire:init');
});

View File

@ -8,7 +8,8 @@
use App\Models\ReviewPack; use App\Models\ReviewPack;
use App\Models\StoredReport; use App\Models\StoredReport;
use App\Models\Tenant; use App\Models\Tenant;
use App\Notifications\ReviewPackStatusNotification; use App\Notifications\OperationRunCompleted;
use App\Notifications\OperationRunQueued;
use App\Services\ReviewPackService; use App\Services\ReviewPackService;
use App\Support\OperationRunOutcome; use App\Support\OperationRunOutcome;
use App\Support\OperationRunStatus; use App\Support\OperationRunStatus;
@ -84,7 +85,7 @@ function seedTenantWithData(Tenant $tenant): void
reviewPackId: (int) $pack->getKey(), reviewPackId: (int) $pack->getKey(),
operationRunId: (int) $pack->operation_run_id, operationRunId: (int) $pack->operation_run_id,
); );
$job->handle(); app()->call([$job, 'handle']);
$pack->refresh(); $pack->refresh();
@ -108,8 +109,8 @@ function seedTenantWithData(Tenant $tenant): void
expect($opRun->status)->toBe(OperationRunStatus::Completed->value); expect($opRun->status)->toBe(OperationRunStatus::Completed->value);
expect($opRun->outcome)->toBe(OperationRunOutcome::Succeeded->value); expect($opRun->outcome)->toBe(OperationRunOutcome::Succeeded->value);
// Notification sent // Notification sent (standard OperationRunCompleted via OperationRunService)
Notification::assertSentTo($user, ReviewPackStatusNotification::class); Notification::assertSentTo($user, OperationRunCompleted::class);
}); });
// ─── Failure Path ────────────────────────────────────────────── // ─── Failure Path ──────────────────────────────────────────────
@ -138,7 +139,7 @@ function seedTenantWithData(Tenant $tenant): void
); );
try { try {
$job->handle(); app()->call([$job, 'handle']);
} catch (\RuntimeException) { } catch (\RuntimeException) {
// Expected — the job re-throws after marking failed // Expected — the job re-throws after marking failed
} }
@ -150,11 +151,10 @@ function seedTenantWithData(Tenant $tenant): void
$opRun = OperationRun::query()->find($pack->operation_run_id); $opRun = OperationRun::query()->find($pack->operation_run_id);
expect($opRun->status)->toBe(OperationRunStatus::Completed->value); expect($opRun->status)->toBe(OperationRunStatus::Completed->value);
expect($opRun->outcome)->toBe(OperationRunOutcome::Failed->value); expect($opRun->outcome)->toBe(OperationRunOutcome::Failed->value);
expect($opRun->context['reason_code'])->toBe('generation_error'); expect($opRun->failure_summary)->toBeArray();
expect($opRun->failure_summary[0]['code'])->toBe('generation_error');
Notification::assertSentTo($user, ReviewPackStatusNotification::class, function ($notification) { Notification::assertSentTo($user, OperationRunCompleted::class);
return $notification->status === 'failed';
});
}); });
// ─── Empty Reports ────────────────────────────────────────────── // ─── Empty Reports ──────────────────────────────────────────────
@ -172,7 +172,7 @@ function seedTenantWithData(Tenant $tenant): void
reviewPackId: (int) $pack->getKey(), reviewPackId: (int) $pack->getKey(),
operationRunId: (int) $pack->operation_run_id, operationRunId: (int) $pack->operation_run_id,
); );
$job->handle(); app()->call([$job, 'handle']);
$pack->refresh(); $pack->refresh();
@ -198,7 +198,7 @@ function seedTenantWithData(Tenant $tenant): void
reviewPackId: (int) $pack->getKey(), reviewPackId: (int) $pack->getKey(),
operationRunId: (int) $pack->operation_run_id, operationRunId: (int) $pack->operation_run_id,
); );
$job->handle(); app()->call([$job, 'handle']);
$pack->refresh(); $pack->refresh();
expect($pack->status)->toBe(ReviewPackStatus::Ready->value); expect($pack->status)->toBe(ReviewPackStatus::Ready->value);
@ -254,7 +254,7 @@ function seedTenantWithData(Tenant $tenant): void
reviewPackId: (int) $pack->getKey(), reviewPackId: (int) $pack->getKey(),
operationRunId: (int) $pack->operation_run_id, operationRunId: (int) $pack->operation_run_id,
); );
$job->handle(); app()->call([$job, 'handle']);
$pack->refresh(); $pack->refresh();
@ -303,6 +303,19 @@ function seedTenantWithData(Tenant $tenant): void
}); });
}); });
it('sends queued database notification when review pack generation is requested', function (): void {
Queue::fake();
Notification::fake();
[$user, $tenant] = createUserWithTenant();
/** @var ReviewPackService $service */
$service = app(ReviewPackService::class);
$service->generate($tenant, $user);
Notification::assertSentTo($user, OperationRunQueued::class);
});
// ─── OperationRun Type ────────────────────────────────────────── // ─── OperationRun Type ──────────────────────────────────────────
it('creates an OperationRun of type review_pack_generate', function (): void { it('creates an OperationRun of type review_pack_generate', function (): void {

View File

@ -5,12 +5,15 @@
use App\Filament\Resources\ReviewPackResource; use App\Filament\Resources\ReviewPackResource;
use App\Filament\Resources\ReviewPackResource\Pages\ListReviewPacks; use App\Filament\Resources\ReviewPackResource\Pages\ListReviewPacks;
use App\Filament\Resources\ReviewPackResource\Pages\ViewReviewPack; use App\Filament\Resources\ReviewPackResource\Pages\ViewReviewPack;
use App\Models\OperationRun;
use App\Models\ReviewPack; use App\Models\ReviewPack;
use App\Models\Tenant; use App\Models\Tenant;
use App\Services\ReviewPackService;
use App\Support\Auth\UiTooltips; use App\Support\Auth\UiTooltips;
use App\Support\ReviewPackStatus; use App\Support\ReviewPackStatus;
use Filament\Facades\Filament; use Filament\Facades\Filament;
use Illuminate\Foundation\Testing\RefreshDatabase; use Illuminate\Foundation\Testing\RefreshDatabase;
use Illuminate\Support\Facades\Queue;
use Illuminate\Support\Facades\Storage; use Illuminate\Support\Facades\Storage;
use Livewire\Livewire; use Livewire\Livewire;
@ -95,6 +98,45 @@
->assertActionExists('generate_pack', fn ($action): bool => $action->getTooltip() === UiTooltips::insufficientPermission()); ->assertActionExists('generate_pack', fn ($action): bool => $action->getTooltip() === UiTooltips::insufficientPermission());
}); });
it('reuses an existing ready pack instead of starting a new run', function (): void {
Queue::fake();
$tenant = Tenant::factory()->create();
[$user, $tenant] = createUserWithTenant(tenant: $tenant, role: 'owner');
$fingerprint = app(ReviewPackService::class)->computeFingerprint($tenant, [
'include_pii' => true,
'include_operations' => true,
]);
ReviewPack::factory()->ready()->create([
'tenant_id' => (int) $tenant->getKey(),
'workspace_id' => (int) $tenant->workspace_id,
'initiated_by_user_id' => (int) $user->getKey(),
'fingerprint' => $fingerprint,
'expires_at' => now()->addDay(),
]);
$tenant->makeCurrent();
Filament::setTenant($tenant, true);
$operationRunsBefore = OperationRun::query()->count();
$reviewPacksBefore = ReviewPack::query()->count();
Livewire::actingAs($user)
->test(ListReviewPacks::class)
->callAction('generate_pack', [
'include_pii' => true,
'include_operations' => true,
])
->assertNotified();
expect(OperationRun::query()->count())->toBe($operationRunsBefore);
expect(ReviewPack::query()->count())->toBe($reviewPacksBefore);
Queue::assertNothingPushed();
});
// ─── Table Row Actions ─────────────────────────────────────── // ─── Table Row Actions ───────────────────────────────────────
it('shows the download action for a ready pack', function (): void { it('shows the download action for a ready pack', function (): void {