feat: add review publication resolution ux spec and tests (#458)
Automated PR created by Codex via Gitea API. Co-authored-by: Ahmed Darrazi <ahmed.darrazi@live.de> Reviewed-on: #458
@ -84,7 +84,7 @@ public function mount(int|string $record): void
|
||||
|
||||
$this->resolutionCaseId = (int) $case->getKey();
|
||||
$this->heading = $case->status === ReviewPublicationResolutionCaseStatus::ReadyToContinue->value
|
||||
? 'Review is ready to return to publication'
|
||||
? 'Review is ready to continue'
|
||||
: 'Review can\'t be published yet';
|
||||
$this->subheading = $this->record->tenant?->getFilamentName();
|
||||
}
|
||||
@ -109,7 +109,7 @@ protected function getHeaderActions(): array
|
||||
return array_values(array_filter([
|
||||
$this->executeCurrentStepAction(),
|
||||
Actions\Action::make('back_to_review')
|
||||
->label('Back to review')
|
||||
->label('Return to review')
|
||||
->icon('heroicon-o-arrow-left')
|
||||
->color('gray')
|
||||
->url(fn (): string => $this->reviewUrl()),
|
||||
@ -166,13 +166,13 @@ private function executeCurrentStepAction(): Actions\Action
|
||||
->label(fn (): string => $this->currentStepActionLabel())
|
||||
->icon(fn (): string => $this->currentStepActionIcon())
|
||||
->color('primary')
|
||||
->visible(fn (): bool => $this->hasExecutableCurrentStep())
|
||||
->visible(fn (): bool => $this->hasExecutableCurrentStep() && ! $this->currentStepIsRunning())
|
||||
->disabled(fn (): bool => $this->currentStepIsRunning() || ! $this->canExecuteCurrentStep())
|
||||
->tooltip(fn (): ?string => $this->currentStepActionTooltip())
|
||||
->requiresConfirmation()
|
||||
->modalHeading(fn (): string => $this->currentStepActionLabel())
|
||||
->modalHeading(fn (): string => $this->currentStepConfirmationHeading())
|
||||
->modalDescription(fn (): string => $this->currentStepConfirmationDescription())
|
||||
->modalSubmitActionLabel('Continue')
|
||||
->modalSubmitActionLabel(fn (): string => $this->currentStepActionLabel())
|
||||
->action(function (): void {
|
||||
$case = $this->resolutionCase();
|
||||
$user = auth()->user();
|
||||
@ -294,7 +294,7 @@ private function pageOutcomeTitle(): string
|
||||
|
||||
if ($case instanceof ReviewPublicationResolutionCase
|
||||
&& $case->status === ReviewPublicationResolutionCaseStatus::ReadyToContinue->value) {
|
||||
return 'Review is ready to return to publication';
|
||||
return 'Review is ready to continue';
|
||||
}
|
||||
|
||||
return 'Review can\'t be published yet';
|
||||
@ -353,8 +353,8 @@ private function currentStepActionLabelFor(?ReviewPublicationResolutionStepKey $
|
||||
ReviewPublicationResolutionStepKey::CompleteRequiredReports => 'Update required reports',
|
||||
ReviewPublicationResolutionStepKey::CollectEvidenceSnapshot => 'Collect evidence',
|
||||
ReviewPublicationResolutionStepKey::RefreshReviewComposition => 'Refresh review',
|
||||
ReviewPublicationResolutionStepKey::GenerateReviewPack => 'Generate review pack',
|
||||
ReviewPublicationResolutionStepKey::ReturnToPublication => 'Return to publication',
|
||||
ReviewPublicationResolutionStepKey::GenerateReviewPack => 'Prepare export',
|
||||
ReviewPublicationResolutionStepKey::ReturnToPublication => 'Return to review',
|
||||
default => 'Continue',
|
||||
};
|
||||
}
|
||||
@ -406,19 +406,53 @@ private function decisionState(ReviewPublicationResolutionCase $case, ?ReviewPub
|
||||
->all();
|
||||
$blockerCount = max((int) ($summary['publication_blocker_count'] ?? 0), count($blockers), count($missingReports));
|
||||
$stepKey = $currentStep?->stepKeyEnum();
|
||||
$stepStatus = $currentStep?->statusEnum();
|
||||
$stepIsRunning = $stepStatus === ReviewPublicationResolutionStepStatus::Running;
|
||||
$stepFailed = $stepStatus === ReviewPublicationResolutionStepStatus::Failed;
|
||||
$readyToReturn = (string) $case->status === ReviewPublicationResolutionCaseStatus::ReadyToContinue->value
|
||||
|| $stepKey === ReviewPublicationResolutionStepKey::ReturnToPublication;
|
||||
$permissionNotice = $currentStep instanceof ReviewPublicationResolutionStep
|
||||
&& $this->hasExecutableCurrentStep()
|
||||
&& ! $stepIsRunning
|
||||
&& ! $this->canExecuteCurrentStep()
|
||||
? 'You can inspect this preparation flow, but you do not have permission to run the next action.'
|
||||
: null;
|
||||
$stateNotice = match (true) {
|
||||
$stepIsRunning => [
|
||||
'label' => 'Operation in progress',
|
||||
'color' => 'info',
|
||||
'body' => 'TenantPilot is waiting for the linked operation to finish. No new start action is available while it runs.',
|
||||
],
|
||||
$stepFailed => [
|
||||
'label' => 'Action needed',
|
||||
'color' => 'danger',
|
||||
'body' => 'The last operation did not complete. Review the linked operation, then retry the current preparation action when you are ready.',
|
||||
],
|
||||
default => null,
|
||||
};
|
||||
|
||||
return [
|
||||
'headline' => $readyToReturn ? 'Review is ready to return to publication' : 'Review can\'t be published yet',
|
||||
'status_badge_label' => $readyToReturn ? 'Ready to continue' : 'Publication blocked',
|
||||
'status_badge_color' => $readyToReturn ? 'success' : 'warning',
|
||||
'headline' => $readyToReturn ? 'Review is ready to continue' : 'Review can\'t be published yet',
|
||||
'status_badge_label' => match (true) {
|
||||
$stepIsRunning => 'Operation in progress',
|
||||
$stepFailed => 'Action needed',
|
||||
$readyToReturn => 'Ready to continue',
|
||||
default => 'Publication blocked',
|
||||
},
|
||||
'status_badge_color' => match (true) {
|
||||
$stepIsRunning => 'info',
|
||||
$stepFailed => 'danger',
|
||||
$readyToReturn => 'success',
|
||||
default => 'warning',
|
||||
},
|
||||
'blocked_summary' => $this->blockedSummary($blockerCount, count($missingReports)),
|
||||
'blockers' => $blockers,
|
||||
'missing_reports' => $missingReports,
|
||||
'next_action_label' => $this->currentStepActionLabelFor($stepKey),
|
||||
'next_action_description' => $this->nextStepDescription($stepKey),
|
||||
'next_action_label' => $stepIsRunning ? 'Operation in progress' : $this->currentStepActionLabelFor($stepKey),
|
||||
'next_action_description' => $this->nextStepDescription($stepKey, $stepStatus),
|
||||
'after_this' => $this->afterNextStepDescription($stepKey),
|
||||
'state_notice' => $stateNotice,
|
||||
'permission_notice' => $permissionNotice,
|
||||
];
|
||||
}
|
||||
|
||||
@ -504,13 +538,23 @@ private function reportRequirementLabel(string $dimension): string
|
||||
};
|
||||
}
|
||||
|
||||
private function nextStepDescription(?ReviewPublicationResolutionStepKey $stepKey): string
|
||||
{
|
||||
private function nextStepDescription(
|
||||
?ReviewPublicationResolutionStepKey $stepKey,
|
||||
?ReviewPublicationResolutionStepStatus $stepStatus = null,
|
||||
): string {
|
||||
if ($stepStatus === ReviewPublicationResolutionStepStatus::Running) {
|
||||
return 'TenantPilot is waiting for the linked operation to finish. No new start action is available while it runs.';
|
||||
}
|
||||
|
||||
if ($stepStatus === ReviewPublicationResolutionStepStatus::Failed) {
|
||||
return 'The last operation did not complete. Review the linked operation, then retry the current preparation action when you are ready.';
|
||||
}
|
||||
|
||||
return match ($stepKey) {
|
||||
ReviewPublicationResolutionStepKey::CompleteRequiredReports => 'TenantPilot will update the missing required reports. It will not publish the review.',
|
||||
ReviewPublicationResolutionStepKey::CollectEvidenceSnapshot => 'This will collect a current evidence snapshot for the review. It will not publish the review.',
|
||||
ReviewPublicationResolutionStepKey::RefreshReviewComposition => 'This will refresh the review from current evidence. It will not publish the review.',
|
||||
ReviewPublicationResolutionStepKey::GenerateReviewPack => 'This will generate the review pack needed for customer-ready output. It will not publish the review.',
|
||||
ReviewPublicationResolutionStepKey::GenerateReviewPack => 'This will prepare the customer-ready export package. It will not publish the review.',
|
||||
ReviewPublicationResolutionStepKey::ReturnToPublication => 'All checks are resolved. Return to the review and use the existing publish action when you are ready.',
|
||||
default => 'TenantPilot will continue the next safe preparation step. It will not publish the review.',
|
||||
};
|
||||
@ -520,7 +564,7 @@ private function afterNextStepDescription(?ReviewPublicationResolutionStepKey $s
|
||||
{
|
||||
return match ($stepKey) {
|
||||
ReviewPublicationResolutionStepKey::ReturnToPublication => 'Publishing remains a separate action on the review page.',
|
||||
default => 'After this, TenantPilot re-checks the evidence, refreshes the review if needed, generates the review pack if needed, and sends you back to the review. Publishing remains a separate action.',
|
||||
default => 'After this, TenantPilot re-checks the evidence, refreshes the review if needed, prepares the export if needed, and sends you back to the review. Publishing remains a separate action.',
|
||||
};
|
||||
}
|
||||
|
||||
@ -530,12 +574,17 @@ private function currentStepConfirmationDescription(): string
|
||||
ReviewPublicationResolutionStepKey::CompleteRequiredReports => 'TenantPilot will update the missing required reports. This will not publish the review.',
|
||||
ReviewPublicationResolutionStepKey::CollectEvidenceSnapshot => 'TenantPilot will collect a current evidence snapshot for this review. This will not publish the review.',
|
||||
ReviewPublicationResolutionStepKey::RefreshReviewComposition => 'TenantPilot will refresh the review from current evidence. This will not publish the review.',
|
||||
ReviewPublicationResolutionStepKey::GenerateReviewPack => 'TenantPilot will generate the review pack needed for customer-ready output. This will not publish the review.',
|
||||
ReviewPublicationResolutionStepKey::GenerateReviewPack => 'TenantPilot will prepare the customer-ready export package for this review. This will not publish the review.',
|
||||
ReviewPublicationResolutionStepKey::ReturnToPublication => 'TenantPilot will return you to the review. Publishing remains a separate action.',
|
||||
default => 'TenantPilot will continue the next safe preparation step. This will not publish the review.',
|
||||
};
|
||||
}
|
||||
|
||||
private function currentStepConfirmationHeading(): string
|
||||
{
|
||||
return $this->currentStepActionLabel().'?';
|
||||
}
|
||||
|
||||
private function blockedSummary(int $blockerCount, int $missingReportCount): string
|
||||
{
|
||||
if ($missingReportCount > 0) {
|
||||
@ -577,10 +626,10 @@ private function operatorStepLabel(?ReviewPublicationResolutionStepKey $stepKey)
|
||||
return match ($stepKey) {
|
||||
ReviewPublicationResolutionStepKey::ValidateReviewReadiness => 'Check readiness',
|
||||
ReviewPublicationResolutionStepKey::CompleteRequiredReports => 'Update required reports',
|
||||
ReviewPublicationResolutionStepKey::CollectEvidenceSnapshot => 'Collect evidence snapshot',
|
||||
ReviewPublicationResolutionStepKey::CollectEvidenceSnapshot => 'Collect evidence',
|
||||
ReviewPublicationResolutionStepKey::RefreshReviewComposition => 'Refresh review',
|
||||
ReviewPublicationResolutionStepKey::GenerateReviewPack => 'Generate review pack',
|
||||
ReviewPublicationResolutionStepKey::ReturnToPublication => 'Return to publication',
|
||||
ReviewPublicationResolutionStepKey::GenerateReviewPack => 'Prepare export',
|
||||
ReviewPublicationResolutionStepKey::ReturnToPublication => 'Return to review',
|
||||
default => 'Continue preparation',
|
||||
};
|
||||
}
|
||||
@ -592,7 +641,7 @@ private function operatorStepDescription(?ReviewPublicationResolutionStepKey $st
|
||||
ReviewPublicationResolutionStepKey::CompleteRequiredReports => 'Required reports must be current before publication can continue.',
|
||||
ReviewPublicationResolutionStepKey::CollectEvidenceSnapshot => 'The review needs a complete and current evidence snapshot.',
|
||||
ReviewPublicationResolutionStepKey::RefreshReviewComposition => 'The review is rebuilt from the latest evidence and report state.',
|
||||
ReviewPublicationResolutionStepKey::GenerateReviewPack => 'A current review pack is prepared before returning to publication.',
|
||||
ReviewPublicationResolutionStepKey::GenerateReviewPack => 'The customer-ready export is prepared before returning to the review.',
|
||||
ReviewPublicationResolutionStepKey::ReturnToPublication => 'Return to the review and decide whether to publish.',
|
||||
default => 'TenantPilot prepares the next safe publication prerequisite.',
|
||||
};
|
||||
|
||||
@ -21,6 +21,39 @@
|
||||
</div>
|
||||
</div>
|
||||
|
||||
@php
|
||||
$stateNotice = is_array($decision['state_notice'] ?? null) ? $decision['state_notice'] : null;
|
||||
$permissionNotice = is_string($decision['permission_notice'] ?? null) ? $decision['permission_notice'] : null;
|
||||
@endphp
|
||||
|
||||
@if ($stateNotice !== null || filled($permissionNotice))
|
||||
<div class="space-y-2" data-testid="review-publication-decision-notices">
|
||||
@if ($stateNotice !== null)
|
||||
<div
|
||||
@class([
|
||||
'rounded-lg border px-4 py-3',
|
||||
'border-info-200 bg-info-50 text-info-900 dark:border-info-700/60 dark:bg-info-500/10 dark:text-info-100' => ($stateNotice['color'] ?? null) === 'info',
|
||||
'border-danger-200 bg-danger-50 text-danger-900 dark:border-danger-700/60 dark:bg-danger-500/10 dark:text-danger-100' => ($stateNotice['color'] ?? null) === 'danger',
|
||||
'border-gray-200 bg-gray-50 text-gray-800 dark:border-white/10 dark:bg-white/5 dark:text-gray-100' => ! in_array(($stateNotice['color'] ?? null), ['info', 'danger'], true),
|
||||
])
|
||||
>
|
||||
<div class="text-sm font-semibold">
|
||||
{{ (string) ($stateNotice['label'] ?? '') }}
|
||||
</div>
|
||||
<div class="mt-1 text-sm leading-6">
|
||||
{{ (string) ($stateNotice['body'] ?? '') }}
|
||||
</div>
|
||||
</div>
|
||||
@endif
|
||||
|
||||
@if (filled($permissionNotice))
|
||||
<div class="rounded-lg border border-gray-200 bg-gray-50 px-4 py-3 text-sm leading-6 text-gray-700 dark:border-white/10 dark:bg-white/5 dark:text-gray-200">
|
||||
{{ $permissionNotice }}
|
||||
</div>
|
||||
@endif
|
||||
</div>
|
||||
@endif
|
||||
|
||||
<div class="space-y-3" data-testid="review-publication-preparation-progress">
|
||||
<div class="flex flex-col gap-1 sm:flex-row sm:items-center sm:justify-between">
|
||||
<div>
|
||||
|
||||
@ -49,7 +49,7 @@
|
||||
->assertSee('Required reports')
|
||||
->assertSee('Permission posture')
|
||||
->assertSee('Technical proof and operation history')
|
||||
->assertSee('Back to review')
|
||||
->assertSee('Return to review')
|
||||
->assertDontSee('Cancel resolution')
|
||||
->assertDontSee('Report-backed evidence')
|
||||
->click('Update required reports')
|
||||
|
||||
@ -0,0 +1,229 @@
|
||||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
use App\Filament\Pages\Reviews\CustomerReviewWorkspace;
|
||||
use App\Filament\Resources\EnvironmentReviewResource;
|
||||
use App\Models\EnvironmentReview;
|
||||
use App\Models\EvidenceSnapshot;
|
||||
use App\Models\ManagedEnvironment;
|
||||
use App\Models\User;
|
||||
use App\Support\EnvironmentReviewStatus;
|
||||
use App\Support\Evidence\EvidenceCompletenessState;
|
||||
use App\Support\ReviewPublicationResolution\ReviewPublicationResolutionService;
|
||||
use App\Support\Workspaces\WorkspaceContext;
|
||||
use Illuminate\Foundation\Testing\RefreshDatabase;
|
||||
use Illuminate\Support\Facades\Storage;
|
||||
|
||||
uses(RefreshDatabase::class);
|
||||
|
||||
pest()->browser()->timeout(60_000);
|
||||
|
||||
beforeEach(function (): void {
|
||||
Storage::fake('exports');
|
||||
});
|
||||
|
||||
it('Spec387 smokes the decision-first publication resolution flow', function (): void {
|
||||
[$user, $environment, $review] = spec387BrowserBlockedReviewFixture();
|
||||
|
||||
spec387AuthenticateBrowser($this, $user, $environment);
|
||||
|
||||
$reviewDetailPage = visit(EnvironmentReviewResource::environmentScopedUrl('view', ['record' => $review], $environment))
|
||||
->resize(1236, 900)
|
||||
->waitForText('Resolve publication blockers')
|
||||
->assertSee('Resolve publication blockers')
|
||||
->assertNoJavaScriptErrors()
|
||||
->assertNoConsoleLogs();
|
||||
|
||||
$reviewDetailPage->screenshot(true, spec387BrowserScreenshotName('01-review-detail-blocked-cta'));
|
||||
spec387CopyBrowserScreenshot('01-review-detail-blocked-cta');
|
||||
|
||||
$resolutionPage = $reviewDetailPage
|
||||
->click('Resolve publication blockers')
|
||||
->waitForText('Review can\'t be published yet')
|
||||
->assertSee('Publication preparation')
|
||||
->assertSee('Why publication is blocked')
|
||||
->assertSee('Next safe action')
|
||||
->assertSee('Update required reports')
|
||||
->assertSee('Prepare export')
|
||||
->assertSee('Return to review')
|
||||
->assertSee('TenantPilot will update the missing required reports. It will not publish the review.')
|
||||
->assertSee('Technical proof and operation history')
|
||||
->assertDontSee('Generate review pack')
|
||||
->assertDontSee('Return to publication')
|
||||
->assertDontSee('Report-backed evidence')
|
||||
->assertDontSee('OperationRun')
|
||||
->assertDontSee('Artifact proof')
|
||||
->assertNoJavaScriptErrors()
|
||||
->assertNoConsoleLogs();
|
||||
|
||||
$resolutionPage->screenshot(true, spec387BrowserScreenshotName('02-resolution-decision-desktop'));
|
||||
spec387CopyBrowserScreenshot('02-resolution-decision-desktop');
|
||||
|
||||
$modalPage = $resolutionPage
|
||||
->click('Update required reports')
|
||||
->waitForText('Update required reports?')
|
||||
->assertSee('TenantPilot will update the missing required reports. This will not publish the review.')
|
||||
->assertSee('Update required reports')
|
||||
->assertNoJavaScriptErrors()
|
||||
->assertNoConsoleLogs();
|
||||
|
||||
$modalPage->screenshot(true, spec387BrowserScreenshotName('03-confirmation-modal'));
|
||||
spec387CopyBrowserScreenshot('03-confirmation-modal');
|
||||
|
||||
$expandedProofPage = $modalPage
|
||||
->click('Cancel')
|
||||
->click('Technical proof and operation history')
|
||||
->waitForText('Proof and operation links are supporting evidence only.')
|
||||
->assertSee('Proof and operation links are supporting evidence only.')
|
||||
->assertNoJavaScriptErrors()
|
||||
->assertNoConsoleLogs();
|
||||
|
||||
$expandedProofPage->screenshot(true, spec387BrowserScreenshotName('04-technical-proof-expanded'));
|
||||
spec387CopyBrowserScreenshot('04-technical-proof-expanded');
|
||||
|
||||
$mobilePage = $expandedProofPage
|
||||
->resize(390, 844)
|
||||
->waitForText('Publication preparation')
|
||||
->assertSee('Review can\'t be published yet')
|
||||
->assertSee('Update required reports')
|
||||
->assertSee('Return to review')
|
||||
->assertNoJavaScriptErrors()
|
||||
->assertNoConsoleLogs();
|
||||
|
||||
$mobilePage->screenshot(true, spec387BrowserScreenshotName('05-resolution-decision-mobile'));
|
||||
spec387CopyBrowserScreenshot('05-resolution-decision-mobile');
|
||||
|
||||
$customerWorkspace = visit(CustomerReviewWorkspace::environmentFilterUrl($environment))
|
||||
->resize(1236, 900)
|
||||
->waitForText('Customer Review Workspace')
|
||||
->assertDontSee('Resolution Case')
|
||||
->assertDontSee('Current step')
|
||||
->assertDontSee('OperationRun')
|
||||
->assertDontSee('Artifact proof')
|
||||
->assertDontSee('complete_required_reports')
|
||||
->assertDontSee('generate_review_pack')
|
||||
->assertDontSee('return_to_publication')
|
||||
->assertNoJavaScriptErrors()
|
||||
->assertNoConsoleLogs();
|
||||
|
||||
$customerWorkspace->screenshot(true, spec387BrowserScreenshotName('06-customer-no-leakage'));
|
||||
spec387CopyBrowserScreenshot('06-customer-no-leakage');
|
||||
});
|
||||
|
||||
it('Spec387 smokes readonly inspection copy on the resolution page', function (): void {
|
||||
[, $environment, $review] = spec387BrowserBlockedReviewFixture();
|
||||
[$readonly] = createUserWithTenant(
|
||||
tenant: $environment,
|
||||
user: User::factory()->create(),
|
||||
role: 'readonly',
|
||||
workspaceRole: 'readonly',
|
||||
);
|
||||
|
||||
spec387AuthenticateBrowser($this, $readonly, $environment);
|
||||
|
||||
$page = visit(EnvironmentReviewResource::environmentScopedUrl('resolve-publication', ['record' => $review], $environment))
|
||||
->resize(1236, 900)
|
||||
->waitForText('You can inspect this preparation flow, but you do not have permission to run the next action.')
|
||||
->assertSee('Review can\'t be published yet')
|
||||
->assertSee('You can inspect this preparation flow, but you do not have permission to run the next action.')
|
||||
->assertSee('Update required reports')
|
||||
->assertDontSee('Generate review pack')
|
||||
->assertNoJavaScriptErrors()
|
||||
->assertNoConsoleLogs();
|
||||
|
||||
$page->screenshot(true, spec387BrowserScreenshotName('07-readonly-inspection'));
|
||||
spec387CopyBrowserScreenshot('07-readonly-inspection');
|
||||
});
|
||||
|
||||
/**
|
||||
* @return array{0: User, 1: ManagedEnvironment, 2: EnvironmentReview}
|
||||
*/
|
||||
function spec387BrowserBlockedReviewFixture(): array
|
||||
{
|
||||
$environment = ManagedEnvironment::factory()->create(['name' => 'Spec387 Browser Resolution']);
|
||||
[$user, $environment] = createUserWithTenant(tenant: $environment, role: 'owner', workspaceRole: 'manager');
|
||||
$snapshot = spec387BrowserPartialEvidence($environment);
|
||||
$review = composeEnvironmentReviewForTest($environment, $user, $snapshot);
|
||||
$review->forceFill([
|
||||
'status' => EnvironmentReviewStatus::Draft->value,
|
||||
'published_at' => null,
|
||||
'published_by_user_id' => null,
|
||||
])->save();
|
||||
|
||||
app(ReviewPublicationResolutionService::class)->openOrResume($review, $user);
|
||||
|
||||
return [$user, $environment, $review];
|
||||
}
|
||||
|
||||
function spec387BrowserPartialEvidence(ManagedEnvironment $environment): EvidenceSnapshot
|
||||
{
|
||||
$snapshot = seedPartialEnvironmentReviewEvidence($environment, findingCount: 0, driftCount: 0);
|
||||
$snapshot->items()->whereIn('dimension_key', ['permission_posture', 'entra_admin_roles'])->update([
|
||||
'state' => EvidenceCompletenessState::Missing->value,
|
||||
'source_record_id' => null,
|
||||
'source_fingerprint' => null,
|
||||
]);
|
||||
|
||||
return $snapshot->fresh('items');
|
||||
}
|
||||
|
||||
function spec387BrowserScreenshotName(string $name): string
|
||||
{
|
||||
return 'spec387-review-publication-resolution-'.$name;
|
||||
}
|
||||
|
||||
function spec387CopyBrowserScreenshot(string $name): void
|
||||
{
|
||||
$filename = spec387BrowserScreenshotName($name).'.png';
|
||||
$primarySource = base_path('tests/Browser/Screenshots/'.$filename);
|
||||
$fallbackSource = \Pest\Browser\Support\Screenshot::path($filename);
|
||||
$targetDirectory = repo_path('specs/387-review-publication-resolution-decision-ux-v1/artifacts/screenshots');
|
||||
|
||||
if (! is_dir($targetDirectory)) {
|
||||
@mkdir($targetDirectory, 0755, true);
|
||||
}
|
||||
|
||||
$source = null;
|
||||
|
||||
for ($attempt = 0; $attempt < 50 && $source === null; $attempt++) {
|
||||
foreach ([$primarySource, $fallbackSource] as $candidate) {
|
||||
if (is_file($candidate)) {
|
||||
$source = $candidate;
|
||||
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
if ($source !== null) {
|
||||
break;
|
||||
}
|
||||
|
||||
usleep(100_000);
|
||||
clearstatcache(true, $primarySource);
|
||||
clearstatcache(true, $fallbackSource);
|
||||
}
|
||||
|
||||
if (is_string($source) && is_file($source) && is_dir($targetDirectory) && is_writable($targetDirectory)) {
|
||||
@copy($source, $targetDirectory.DIRECTORY_SEPARATOR.$name.'.png');
|
||||
}
|
||||
}
|
||||
|
||||
function spec387AuthenticateBrowser(mixed $test, User $user, ManagedEnvironment $environment): void
|
||||
{
|
||||
$workspaceId = (int) $environment->workspace_id;
|
||||
|
||||
$test->actingAs($user)->withSession([
|
||||
WorkspaceContext::SESSION_KEY => $workspaceId,
|
||||
WorkspaceContext::LAST_ENVIRONMENT_IDS_SESSION_KEY => [
|
||||
(string) $workspaceId => (int) $environment->getKey(),
|
||||
],
|
||||
]);
|
||||
|
||||
session()->put(WorkspaceContext::SESSION_KEY, $workspaceId);
|
||||
session()->put(WorkspaceContext::LAST_ENVIRONMENT_IDS_SESSION_KEY, [
|
||||
(string) $workspaceId => (int) $environment->getKey(),
|
||||
]);
|
||||
|
||||
setAdminPanelContext($environment);
|
||||
}
|
||||
@ -0,0 +1,389 @@
|
||||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
use App\Filament\Pages\Reviews\CustomerReviewWorkspace;
|
||||
use App\Filament\Resources\EnvironmentReviewResource\Pages\ResolveReviewPublication;
|
||||
use App\Filament\Resources\EnvironmentReviewResource\Pages\ViewEnvironmentReview;
|
||||
use App\Models\EnvironmentReview;
|
||||
use App\Models\ManagedEnvironment;
|
||||
use App\Models\OperationRun;
|
||||
use App\Models\ReviewPack;
|
||||
use App\Models\ReviewPublicationResolutionCase;
|
||||
use App\Models\User;
|
||||
use App\Services\EnvironmentReviews\EnvironmentReviewReadinessGate;
|
||||
use App\Support\EnvironmentReviewStatus;
|
||||
use App\Support\Evidence\EvidenceCompletenessState;
|
||||
use App\Support\OperationRunOutcome;
|
||||
use App\Support\OperationRunStatus;
|
||||
use App\Support\OperationRunType;
|
||||
use App\Support\ReviewPublicationResolution\ReviewPublicationResolutionActionService;
|
||||
use App\Support\ReviewPublicationResolution\ReviewPublicationResolutionCaseStatus;
|
||||
use App\Support\ReviewPublicationResolution\ReviewPublicationResolutionService;
|
||||
use App\Support\ReviewPublicationResolution\ReviewPublicationResolutionStepKey;
|
||||
use App\Support\ReviewPublicationResolution\ReviewPublicationResolutionStepStatus;
|
||||
use App\Support\Workspaces\WorkspaceContext;
|
||||
use Filament\Actions\Action;
|
||||
use Filament\Actions\ActionGroup;
|
||||
use Illuminate\Auth\Access\AuthorizationException;
|
||||
use Illuminate\Support\Facades\Queue;
|
||||
use Illuminate\Support\Facades\Storage;
|
||||
use Livewire\Features\SupportTesting\Testable;
|
||||
use Livewire\Livewire;
|
||||
|
||||
beforeEach(function (): void {
|
||||
Storage::fake('exports');
|
||||
});
|
||||
|
||||
it('Spec387 renders the decision page with operator labels and proof secondary', function (): void {
|
||||
[$owner, $tenant, $review] = spec387BlockedReviewFixture();
|
||||
|
||||
setAdminEnvironmentContext($tenant);
|
||||
|
||||
Livewire::actingAs($owner)
|
||||
->test(ResolveReviewPublication::class, ['record' => $review->getKey()])
|
||||
->assertSee('Review can\'t be published yet')
|
||||
->assertSeeInOrder([
|
||||
'Publication preparation',
|
||||
'Why publication is blocked',
|
||||
'Next safe action',
|
||||
'What happens after this',
|
||||
'Technical proof and operation history',
|
||||
])
|
||||
->assertSee('Update required reports')
|
||||
->assertSee('Collect evidence')
|
||||
->assertSee('Refresh review')
|
||||
->assertSee('Prepare export')
|
||||
->assertSee('Return to review')
|
||||
->assertSee('Required reports')
|
||||
->assertSee('Permission posture')
|
||||
->assertSee('Entra admin roles')
|
||||
->assertSee('will not publish the review')
|
||||
->assertSeeHtml('collapsed')
|
||||
->assertDontSeeText('Generate review pack')
|
||||
->assertDontSeeText('Return to publication')
|
||||
->assertDontSeeText('Resolution Case')
|
||||
->assertDontSeeText('Case Status')
|
||||
->assertDontSeeText('Current step')
|
||||
->assertDontSeeText('Resolution steps')
|
||||
->assertDontSeeText('Report-backed evidence')
|
||||
->assertDontSeeText('OperationRun')
|
||||
->assertDontSeeText('Artifact proof')
|
||||
->assertActionDoesNotExist('publish_review')
|
||||
->assertActionExists('back_to_review', fn (Action $action): bool => $action->getLabel() === 'Return to review');
|
||||
});
|
||||
|
||||
it('Spec387 uses action-specific confirmation copy for each current step', function (
|
||||
ReviewPublicationResolutionStepKey $stepKey,
|
||||
string $expectedLabel,
|
||||
string $expectedDescription,
|
||||
): void {
|
||||
[$owner, $tenant, $review, $case] = spec387BlockedReviewFixture();
|
||||
[$readonly] = createUserWithTenant(
|
||||
tenant: $tenant,
|
||||
user: User::factory()->create(),
|
||||
role: 'readonly',
|
||||
workspaceRole: 'readonly',
|
||||
);
|
||||
|
||||
spec387ForceCurrentStep($case, $stepKey);
|
||||
setAdminEnvironmentContext($tenant);
|
||||
|
||||
Livewire::actingAs($readonly)
|
||||
->test(ResolveReviewPublication::class, ['record' => $review->getKey()])
|
||||
->assertActionExists('execute_current_step', function (Action $action) use ($expectedLabel, $expectedDescription): bool {
|
||||
return $action->getLabel() === $expectedLabel
|
||||
&& $action->isConfirmationRequired()
|
||||
&& (string) $action->getModalHeading() === $expectedLabel.'?'
|
||||
&& $action->getModalSubmitActionLabel() === $expectedLabel
|
||||
&& (string) $action->getModalDescription() === $expectedDescription;
|
||||
});
|
||||
})->with([
|
||||
'required reports' => [
|
||||
ReviewPublicationResolutionStepKey::CompleteRequiredReports,
|
||||
'Update required reports',
|
||||
'TenantPilot will update the missing required reports. This will not publish the review.',
|
||||
],
|
||||
'collect evidence' => [
|
||||
ReviewPublicationResolutionStepKey::CollectEvidenceSnapshot,
|
||||
'Collect evidence',
|
||||
'TenantPilot will collect a current evidence snapshot for this review. This will not publish the review.',
|
||||
],
|
||||
'refresh review' => [
|
||||
ReviewPublicationResolutionStepKey::RefreshReviewComposition,
|
||||
'Refresh review',
|
||||
'TenantPilot will refresh the review from current evidence. This will not publish the review.',
|
||||
],
|
||||
'prepare export' => [
|
||||
ReviewPublicationResolutionStepKey::GenerateReviewPack,
|
||||
'Prepare export',
|
||||
'TenantPilot will prepare the customer-ready export package for this review. This will not publish the review.',
|
||||
],
|
||||
'return to review' => [
|
||||
ReviewPublicationResolutionStepKey::ReturnToPublication,
|
||||
'Return to review',
|
||||
'TenantPilot will return you to the review. Publishing remains a separate action.',
|
||||
],
|
||||
]);
|
||||
|
||||
it('Spec387 makes readonly inspection explicit and keeps direct execution denied', function (): void {
|
||||
[$owner, $tenant, $review, $case] = spec387BlockedReviewFixture();
|
||||
[$readonly] = createUserWithTenant(
|
||||
tenant: $tenant,
|
||||
user: User::factory()->create(),
|
||||
role: 'readonly',
|
||||
workspaceRole: 'readonly',
|
||||
);
|
||||
|
||||
Queue::fake();
|
||||
setAdminEnvironmentContext($tenant);
|
||||
|
||||
expect($readonly->can('view', $case))->toBeTrue()
|
||||
->and($readonly->can('executeStep', $case))->toBeFalse();
|
||||
|
||||
Livewire::actingAs($readonly)
|
||||
->test(ResolveReviewPublication::class, ['record' => $review->getKey()])
|
||||
->assertSee('You can inspect this preparation flow, but you do not have permission to run the next action.')
|
||||
->assertActionVisible('execute_current_step')
|
||||
->assertActionDisabled('execute_current_step');
|
||||
|
||||
expect(fn () => app(ReviewPublicationResolutionActionService::class)->executeCurrentStep($case->fresh(), $readonly))
|
||||
->toThrow(AuthorizationException::class);
|
||||
|
||||
Queue::assertNothingPushed();
|
||||
});
|
||||
|
||||
it('Spec387 shows waiting and failed operation states without exposing a duplicate running action', function (): void {
|
||||
[$owner, $tenant, $review, $case] = spec387BlockedReviewFixture();
|
||||
$run = OperationRun::factory()->forTenant($tenant)->create([
|
||||
'type' => OperationRunType::EntraAdminRolesScan->value,
|
||||
'status' => OperationRunStatus::Running->value,
|
||||
'outcome' => OperationRunOutcome::Pending->value,
|
||||
]);
|
||||
|
||||
spec387ForceCurrentStep($case, ReviewPublicationResolutionStepKey::CompleteRequiredReports, ReviewPublicationResolutionStepStatus::Running, $run);
|
||||
setAdminEnvironmentContext($tenant);
|
||||
|
||||
Livewire::actingAs($owner)
|
||||
->test(ResolveReviewPublication::class, ['record' => $review->getKey()])
|
||||
->assertSee('Operation in progress')
|
||||
->assertSee('TenantPilot is waiting for the linked operation to finish. No new start action is available while it runs.')
|
||||
->assertActionHidden('execute_current_step');
|
||||
|
||||
$run->forceFill([
|
||||
'status' => OperationRunStatus::Completed->value,
|
||||
'outcome' => OperationRunOutcome::Failed->value,
|
||||
'completed_at' => now(),
|
||||
])->save();
|
||||
|
||||
spec387ForceCurrentStep($case, ReviewPublicationResolutionStepKey::CompleteRequiredReports, ReviewPublicationResolutionStepStatus::Failed, $run);
|
||||
|
||||
Livewire::actingAs($owner)
|
||||
->test(ResolveReviewPublication::class, ['record' => $review->getKey()])
|
||||
->assertSee('Action needed')
|
||||
->assertSee('The last operation did not complete. Review the linked operation, then retry the current preparation action when you are ready.')
|
||||
->assertActionVisible('execute_current_step')
|
||||
->assertActionEnabled('execute_current_step');
|
||||
});
|
||||
|
||||
it('Spec387 shows ready-to-continue copy without moving publish onto the resolution page', function (): void {
|
||||
[$owner, $tenant, $review, $case] = spec387BlockedReviewFixture();
|
||||
|
||||
spec387MakeReviewReadyForReturn($review, $owner);
|
||||
$case = app(ReviewPublicationResolutionService::class)->openOrResume($review->fresh(['sections', 'evidenceSnapshot.items', 'currentExportReviewPack']), $owner);
|
||||
setAdminEnvironmentContext($tenant);
|
||||
|
||||
expect(app(EnvironmentReviewReadinessGate::class)->blockersForReview($review->fresh('sections')))->toBe([]);
|
||||
|
||||
expect($case?->current_step_key)->toBe(ReviewPublicationResolutionStepKey::ReturnToPublication->value)
|
||||
->and($case?->status)->toBe(ReviewPublicationResolutionCaseStatus::ReadyToContinue->value);
|
||||
|
||||
Livewire::actingAs($owner)
|
||||
->test(ResolveReviewPublication::class, ['record' => $review->getKey()])
|
||||
->assertSee('Review is ready to continue')
|
||||
->assertSee('Ready to continue')
|
||||
->assertSee('Return to review')
|
||||
->assertSee('Publishing remains a separate action on the review page.')
|
||||
->assertActionDoesNotExist('publish_review');
|
||||
});
|
||||
|
||||
it('Spec387 keeps the blocked review detail CTA primary while publish stays non-primary', function (): void {
|
||||
[$owner, $tenant, $review] = spec387BlockedReviewFixture();
|
||||
|
||||
setAdminEnvironmentContext($tenant);
|
||||
|
||||
$component = Livewire::actingAs($owner)
|
||||
->test(ViewEnvironmentReview::class, ['record' => $review->getKey()])
|
||||
->assertActionVisible('resolve_publication_blockers')
|
||||
->assertActionExists('resolve_publication_blockers', fn (Action $action): bool => $action->getLabel() === 'Resolve publication blockers');
|
||||
|
||||
$topLevelActionNames = collect(spec387EnvironmentReviewHeaderActions($component))
|
||||
->reject(static fn ($action): bool => $action instanceof ActionGroup)
|
||||
->map(static fn ($action): ?string => $action instanceof Action ? $action->getName() : null)
|
||||
->filter()
|
||||
->values()
|
||||
->all();
|
||||
|
||||
expect($topLevelActionNames)->toBe(['resolve_publication_blockers']);
|
||||
});
|
||||
|
||||
it('Spec387 keeps customer review workspace free of resolution internals', function (): void {
|
||||
[$owner, $tenant, $review] = spec387BlockedReviewFixture();
|
||||
|
||||
$review = markEnvironmentReviewCustomerSafeReady($review);
|
||||
$review->forceFill([
|
||||
'status' => EnvironmentReviewStatus::Published->value,
|
||||
'published_at' => now(),
|
||||
'published_by_user_id' => (int) $owner->getKey(),
|
||||
])->save();
|
||||
|
||||
Storage::disk('exports')->put('review-packs/spec387-customer-safe.zip', 'PK-test');
|
||||
|
||||
$pack = ReviewPack::factory()->ready()->create([
|
||||
'managed_environment_id' => (int) $tenant->getKey(),
|
||||
'workspace_id' => (int) $tenant->workspace_id,
|
||||
'environment_review_id' => (int) $review->getKey(),
|
||||
'evidence_snapshot_id' => (int) $review->evidence_snapshot_id,
|
||||
'initiated_by_user_id' => (int) $owner->getKey(),
|
||||
'file_path' => 'review-packs/spec387-customer-safe.zip',
|
||||
'file_disk' => 'exports',
|
||||
]);
|
||||
|
||||
$review->forceFill(['current_export_review_pack_id' => (int) $pack->getKey()])->save();
|
||||
|
||||
session()->put(WorkspaceContext::SESSION_KEY, (int) $tenant->workspace_id);
|
||||
setAdminPanelContext();
|
||||
|
||||
Livewire::actingAs($owner)
|
||||
->test(CustomerReviewWorkspace::class)
|
||||
->assertSee('Customer Review Workspace')
|
||||
->assertDontSee('Resolution Case')
|
||||
->assertDontSee('Current step')
|
||||
->assertDontSee('OperationRun')
|
||||
->assertDontSee('Artifact proof')
|
||||
->assertDontSee('complete_required_reports')
|
||||
->assertDontSee('generate_review_pack')
|
||||
->assertDontSee('return_to_publication');
|
||||
});
|
||||
|
||||
/**
|
||||
* @return array{0: User, 1: ManagedEnvironment, 2: EnvironmentReview, 3: ReviewPublicationResolutionCase}
|
||||
*/
|
||||
function spec387BlockedReviewFixture(): array
|
||||
{
|
||||
$tenant = ManagedEnvironment::factory()->create(['name' => 'Spec387 Resolution']);
|
||||
[$owner, $tenant] = createUserWithTenant(tenant: $tenant, role: 'owner', workspaceRole: 'manager');
|
||||
$snapshot = seedPartialEnvironmentReviewEvidence($tenant, findingCount: 0, driftCount: 0);
|
||||
$snapshot->items()->whereIn('dimension_key', ['permission_posture', 'entra_admin_roles'])->update([
|
||||
'state' => EvidenceCompletenessState::Missing->value,
|
||||
'source_record_id' => null,
|
||||
'source_fingerprint' => null,
|
||||
]);
|
||||
$review = composeEnvironmentReviewForTest($tenant, $owner, $snapshot);
|
||||
$case = app(ReviewPublicationResolutionService::class)->openOrResume($review, $owner);
|
||||
|
||||
expect($case)->toBeInstanceOf(ReviewPublicationResolutionCase::class);
|
||||
|
||||
return [$owner, $tenant, $review, $case];
|
||||
}
|
||||
|
||||
function spec387MakeReviewReadyForReturn(EnvironmentReview $review, User $owner): EnvironmentReview
|
||||
{
|
||||
$review = markEnvironmentReviewCustomerSafeReady($review);
|
||||
|
||||
$review->sections->each(function ($section): void {
|
||||
$summaryPayload = is_array($section->summary_payload) ? $section->summary_payload : [];
|
||||
$baselineReadiness = is_array($summaryPayload['baseline_readiness'] ?? null)
|
||||
? $summaryPayload['baseline_readiness']
|
||||
: [];
|
||||
|
||||
$summaryPayload['publication_blockers'] = [];
|
||||
|
||||
if ($baselineReadiness !== []) {
|
||||
$summaryPayload['baseline_readiness'] = array_replace($baselineReadiness, [
|
||||
'publication_blockers' => [],
|
||||
]);
|
||||
}
|
||||
|
||||
$section->forceFill([
|
||||
'summary_payload' => $summaryPayload,
|
||||
])->save();
|
||||
});
|
||||
|
||||
$review->evidenceSnapshot?->items()
|
||||
->whereIn('dimension_key', ['permission_posture', 'entra_admin_roles'])
|
||||
->update([
|
||||
'state' => EvidenceCompletenessState::Complete->value,
|
||||
'source_record_id' => '1',
|
||||
'source_fingerprint' => 'spec387-ready-report',
|
||||
'updated_at' => now(),
|
||||
]);
|
||||
|
||||
Storage::disk('exports')->put('review-packs/spec387-ready-return.zip', 'PK-test');
|
||||
|
||||
$pack = ReviewPack::factory()->ready()->create([
|
||||
'managed_environment_id' => (int) $review->managed_environment_id,
|
||||
'workspace_id' => (int) $review->workspace_id,
|
||||
'environment_review_id' => (int) $review->getKey(),
|
||||
'evidence_snapshot_id' => (int) $review->evidence_snapshot_id,
|
||||
'initiated_by_user_id' => (int) $owner->getKey(),
|
||||
'file_path' => 'review-packs/spec387-ready-return.zip',
|
||||
'file_disk' => 'exports',
|
||||
]);
|
||||
|
||||
$review->forceFill([
|
||||
'status' => EnvironmentReviewStatus::Ready->value,
|
||||
'published_at' => null,
|
||||
'published_by_user_id' => null,
|
||||
'current_export_review_pack_id' => (int) $pack->getKey(),
|
||||
])->save();
|
||||
|
||||
return $review->fresh(['evidenceSnapshot.items', 'currentExportReviewPack', 'sections']);
|
||||
}
|
||||
|
||||
function spec387ForceCurrentStep(
|
||||
ReviewPublicationResolutionCase $case,
|
||||
ReviewPublicationResolutionStepKey $stepKey,
|
||||
ReviewPublicationResolutionStepStatus $status = ReviewPublicationResolutionStepStatus::Actionable,
|
||||
?OperationRun $operationRun = null,
|
||||
): ReviewPublicationResolutionCase {
|
||||
$case->loadMissing('steps');
|
||||
|
||||
foreach ($case->steps as $step) {
|
||||
$step->forceFill([
|
||||
'status' => $step->step_key === $stepKey->value
|
||||
? $status->value
|
||||
: ReviewPublicationResolutionStepStatus::Completed->value,
|
||||
'operation_run_id' => $step->step_key === $stepKey->value ? $operationRun?->getKey() : null,
|
||||
'proof_type' => $step->step_key === $stepKey->value && $operationRun instanceof OperationRun ? 'operation_run' : $step->proof_type,
|
||||
'proof_id' => $step->step_key === $stepKey->value && $operationRun instanceof OperationRun ? (int) $operationRun->getKey() : $step->proof_id,
|
||||
'proof_status' => $step->step_key === $stepKey->value && $operationRun instanceof OperationRun ? (string) $operationRun->outcome : $step->proof_status,
|
||||
])->save();
|
||||
}
|
||||
|
||||
$caseStatus = match ($status) {
|
||||
ReviewPublicationResolutionStepStatus::Running => ReviewPublicationResolutionCaseStatus::WaitingForRun,
|
||||
ReviewPublicationResolutionStepStatus::Failed => ReviewPublicationResolutionCaseStatus::Blocked,
|
||||
default => $stepKey === ReviewPublicationResolutionStepKey::ReturnToPublication
|
||||
? ReviewPublicationResolutionCaseStatus::ReadyToContinue
|
||||
: ReviewPublicationResolutionCaseStatus::InProgress,
|
||||
};
|
||||
|
||||
$case->forceFill([
|
||||
'current_step_key' => $stepKey->value,
|
||||
'status' => $caseStatus->value,
|
||||
])->save();
|
||||
|
||||
return $case->fresh('steps.operationRun');
|
||||
}
|
||||
|
||||
function spec387EnvironmentReviewHeaderActions(Testable $component): array
|
||||
{
|
||||
$instance = $component->instance();
|
||||
|
||||
if ($instance->getCachedHeaderActions() === []) {
|
||||
$instance->cacheInteractsWithHeaderActions();
|
||||
}
|
||||
|
||||
return $instance->getCachedHeaderActions();
|
||||
}
|
||||
@ -8,8 +8,8 @@ # UI-101 Review Publication Resolution
|
||||
| Archetype | Reviews |
|
||||
| Design depth | Strategic Surface |
|
||||
| Repo truth | browser-verified route; feature-tested |
|
||||
| Screenshot | [desktop](../../../specs/386-review-publication-resolution-workflow-v1/artifacts/screenshots/01-resolution-page-desktop.png), [mobile](../../../specs/386-review-publication-resolution-workflow-v1/artifacts/screenshots/02-resolution-page-mobile.png) |
|
||||
| Browser status | Browser smoke passed for blocked-review CTA handoff, decision-first resolution page rendering, compact preparation progress, confirmation copy, technical disclosure, and narrow viewport readability. |
|
||||
| Screenshot | Spec 386 baseline: [desktop](../../../specs/386-review-publication-resolution-workflow-v1/artifacts/screenshots/01-resolution-page-desktop.png), [mobile](../../../specs/386-review-publication-resolution-workflow-v1/artifacts/screenshots/02-resolution-page-mobile.png). Spec 387 hardening: [detail CTA](../../../specs/387-review-publication-resolution-decision-ux-v1/artifacts/screenshots/01-review-detail-blocked-cta.png), [decision desktop](../../../specs/387-review-publication-resolution-decision-ux-v1/artifacts/screenshots/02-resolution-decision-desktop.png), [confirmation modal](../../../specs/387-review-publication-resolution-decision-ux-v1/artifacts/screenshots/03-confirmation-modal.png), [proof expanded](../../../specs/387-review-publication-resolution-decision-ux-v1/artifacts/screenshots/04-technical-proof-expanded.png), [mobile](../../../specs/387-review-publication-resolution-decision-ux-v1/artifacts/screenshots/05-resolution-decision-mobile.png), [customer boundary](../../../specs/387-review-publication-resolution-decision-ux-v1/artifacts/screenshots/06-customer-no-leakage.png), [readonly](../../../specs/387-review-publication-resolution-decision-ux-v1/artifacts/screenshots/07-readonly-inspection.png) |
|
||||
| Browser status | Browser smoke passed for blocked-review CTA handoff, decision-first resolution page rendering, compact preparation progress, action-specific confirmation copy, technical disclosure, readonly inspection, customer boundary non-leakage, and narrow viewport readability. |
|
||||
|
||||
## First Five Seconds
|
||||
|
||||
@ -18,6 +18,7 @@ ## First Five Seconds
|
||||
## Productization Review
|
||||
|
||||
- Decision-first: operator-state page title, blocked reason, missing required reports, next safe action, compact preparation progress, and no-auto-publish copy lead the page.
|
||||
- Operator vocabulary: review-pack generation is presented as `Prepare export`, return-to-publication completion is presented as `Return to review`, and readonly users receive page-level inspection copy.
|
||||
- Evidence-first: report, evidence, review, pack, and operation proof remain available without becoming the default workflow source of truth.
|
||||
- Context: route is owned by the Environment Review record and remains environment-scoped.
|
||||
- Customer/auditor safety: high, because internal remediation details stay in the admin plane and customer workspace does not expose the case.
|
||||
@ -25,11 +26,11 @@ ## Productization Review
|
||||
|
||||
## Information Inventory
|
||||
|
||||
Default content shows publication blocked state, required reports, compact preparation progress, the next safe action, what happens after the action, and the back-to-review action. Case status, proof links, operation links, and implementation terms such as report-backed evidence are technical details behind disclosure or normalized out of operator copy.
|
||||
Default content shows publication blocked state, required reports, compact preparation progress, the next safe action, what happens after the action, and the return-to-review action. Running and failed operation states use normalized operator copy. Case status, proof links, operation links, and implementation terms such as report-backed evidence are technical details behind disclosure or normalized out of operator copy.
|
||||
|
||||
## Dangerous Actions
|
||||
|
||||
`Cancel resolution` is destructive to the local resolution case only, is demoted into the grouped More action, and requires confirmation plus `ENVIRONMENT_REVIEW_MANAGE`. Step execution is high-impact and uses the owning source action: provider verification or Entra scan for required reports, evidence snapshot generation for evidence, review refresh for composition, review-pack generation for export proof, and a non-publishing return-to-review completion step.
|
||||
`Cancel resolution` is destructive to the local resolution case only, is demoted into the grouped More action, and requires confirmation plus `ENVIRONMENT_REVIEW_MANAGE`. Step execution is high-impact, uses action-specific confirmation heading/body/submit copy, hides duplicate start actions while a linked operation is running, and delegates to the owning source action: provider verification or Entra scan for required reports, evidence snapshot generation for evidence, review refresh for composition, review-pack generation for export proof, and a non-publishing return-to-review completion step.
|
||||
|
||||
## Scores
|
||||
|
||||
|
||||
|
After Width: | Height: | Size: 465 KiB |
|
After Width: | Height: | Size: 231 KiB |
|
After Width: | Height: | Size: 216 KiB |
|
After Width: | Height: | Size: 289 KiB |
|
After Width: | Height: | Size: 240 KiB |
|
After Width: | Height: | Size: 121 KiB |
|
After Width: | Height: | Size: 238 KiB |
@ -0,0 +1,17 @@
|
||||
# Spec 387 Screenshot Evidence Index
|
||||
|
||||
## Captured Browser Evidence
|
||||
|
||||
- `01-review-detail-blocked-cta.png`: Environment Review detail page with `Resolve publication blockers` as the primary blocked-review CTA.
|
||||
- `02-resolution-decision-desktop.png`: Desktop resolution page with the decision summary first, default technical proof collapsed, and no publish action.
|
||||
- `03-confirmation-modal.png`: Current-step confirmation modal with action-specific no-auto-publish copy.
|
||||
- `04-technical-proof-expanded.png`: Technical proof/history disclosure expanded below the decision content.
|
||||
- `05-resolution-decision-mobile.png`: Mobile resolution page proving the decision summary remains first and controls do not overlap.
|
||||
- `06-customer-no-leakage.png`: Customer-facing workspace boundary with no resolution mechanics exposed.
|
||||
- `07-readonly-inspection.png`: Readonly inspection state with page-level permission copy and no executable primary action.
|
||||
|
||||
## States Covered Without Browser Screenshots
|
||||
|
||||
- Running operation copy and hidden duplicate start action are covered by `Spec387ReviewPublicationResolutionDecisionUxTest` feature assertions. A browser screenshot was not widened for this state because the existing workflow fixture can force the state safely at the Livewire boundary without introducing product-only queue controls.
|
||||
- Failed operation copy is covered by `Spec387ReviewPublicationResolutionDecisionUxTest` feature assertions. A browser screenshot was not widened for the same fixture-scope reason.
|
||||
- Ready-to-continue and ready-for-publication copy is covered by `Spec387ReviewPublicationResolutionDecisionUxTest` feature assertions. Publishing remains on the Review Detail page only.
|
||||
@ -0,0 +1,64 @@
|
||||
# Requirements Checklist: Spec 387 - Review Publication Resolution Decision UX v1
|
||||
|
||||
**Purpose**: Preparation-readiness checklist for Spec 387 artifacts only.
|
||||
**Created**: 2026-06-18
|
||||
**Feature**: `specs/387-review-publication-resolution-decision-ux-v1/spec.md`
|
||||
|
||||
## Candidate Selection And Scope
|
||||
|
||||
- [x] CHK001 The candidate source is the direct user-provided Spec 387 draft attachment.
|
||||
- [x] CHK002 The active auto-prep queue in `docs/product/spec-candidates.md` is acknowledged as empty/no-safe-automatic-target, and this spec is treated as manual user-provided promotion.
|
||||
- [x] CHK003 Completed Spec 386 is treated as implementation context only and is not rewritten or normalized.
|
||||
- [x] CHK004 The original draft is narrowed to residual UX/copy/state/confirmation/readonly hardening already not fully covered by repo truth.
|
||||
- [x] CHK005 Close alternatives are deferred instead of hidden inside the scope.
|
||||
|
||||
## Constitution And Architecture Fit
|
||||
|
||||
- [x] CHK006 The spec forbids new persistence, migrations, route families, global search resources, navigation entries, workflow engines, adapter registries, and auto-publish behavior.
|
||||
- [x] CHK007 The spec requires existing Spec 386 RBAC, audit, source-owned actions, failure redaction, and no-auto-publish guarantees to remain intact.
|
||||
- [x] CHK008 The plan keeps OperationRun start/completion/link UX delegated to existing helpers and services.
|
||||
- [x] CHK009 Provider-specific details remain behind existing proof/diagnostic boundaries.
|
||||
- [x] CHK010 Proportionality review rejects a new generic presenter/framework by default.
|
||||
|
||||
## UI/Productization Coverage
|
||||
|
||||
- [x] CHK011 UI Surface Impact is explicit and consistent with an existing page/copy/action-modal hardening pass without claiming a new modal/action or planned customer-facing surface change.
|
||||
- [x] CHK012 UI/Productization Coverage reuses UI-101, UI-040, and UI-006 context without inventing a new route taxonomy.
|
||||
- [x] CHK013 Decision-first surface role, audience-aware disclosure, UI classification, and operator surface contract are complete for the affected surfaces.
|
||||
- [x] CHK014 Technical proof is required to stay collapsed/default-secondary.
|
||||
- [x] CHK015 Customer-safe non-leakage is a required negative regression, not a new customer resolution feature.
|
||||
|
||||
## Filament / Livewire / Actions
|
||||
|
||||
- [x] CHK016 Filament v5 / Livewire v4 compliance is stated.
|
||||
- [x] CHK017 Laravel 12 panel provider location is stated as `apps/platform/bootstrap/providers.php`, with no provider change planned.
|
||||
- [x] CHK018 No global-searchable Resource is added.
|
||||
- [x] CHK019 Destructive/high-impact actions remain `->action(...)` actions with `->requiresConfirmation()`, server authorization, audit, and tests.
|
||||
- [x] CHK020 Asset strategy is stated as no new registered Filament assets expected.
|
||||
|
||||
## Tests And Validation
|
||||
|
||||
- [x] CHK021 Tasks include tests before implementation tasks.
|
||||
- [x] CHK022 Tasks include focused Feature/Filament assertions for decision copy, internal term absence, confirmation copy, readonly execution denial, and no Publish action.
|
||||
- [x] CHK023 Tasks include browser smoke/screenshot coverage for desktop, mobile, modal, proof disclosure, readonly, and customer no-leakage states.
|
||||
- [x] CHK024 Validation commands use Sail-first paths and include focused Spec 387, Spec 386 regression, browser, Pint, and `git diff --check`.
|
||||
- [x] CHK025 No full-suite success is implied by the preparation artifacts.
|
||||
|
||||
## Analyze Outcome
|
||||
|
||||
- [x] CHK026 Preparation artifacts are consistent on scope: residual UX hardening only.
|
||||
- [x] CHK027 No requirement in `tasks.md` expands beyond `spec.md` or `plan.md`.
|
||||
- [x] CHK028 Open questions are non-blocking.
|
||||
- [x] CHK029 Candidate Selection Gate passes with scope reduction.
|
||||
- [x] CHK030 Spec Readiness Gate passes for a later implementation loop.
|
||||
|
||||
## Review Outcome
|
||||
|
||||
- [x] CHK031 Review outcome class: `acceptable-special-case`.
|
||||
- [x] CHK032 Workflow outcome: `keep`.
|
||||
- [x] CHK033 Final note location: active feature PR close-out entry `Smoke Coverage / UX Hardening / No New Workflow Mechanics`.
|
||||
|
||||
## Notes
|
||||
|
||||
- This checklist validates preparation readiness only.
|
||||
- No application implementation, runtime tests, browser smoke execution, or application code changes have been performed in this preparation step.
|
||||
200
specs/387-review-publication-resolution-decision-ux-v1/plan.md
Normal file
@ -0,0 +1,200 @@
|
||||
# Implementation Plan: Spec 387 - Review Publication Resolution Decision UX v1
|
||||
|
||||
**Branch**: `387-review-publication-resolution-decision-ux-v1` | **Date**: 2026-06-18 | **Spec**: `specs/387-review-publication-resolution-decision-ux-v1/spec.md`
|
||||
**Input**: Feature specification from `/specs/387-review-publication-resolution-decision-ux-v1/spec.md`
|
||||
|
||||
## Summary
|
||||
|
||||
Harden the existing Spec 386 Review Publication Resolution page so its remaining visible labels, confirmation modals, readonly states, proof disclosure, and state-specific messages consistently read as decision-first publication preparation. Keep all workflow mechanics, persistence, policies, OperationRun behavior, audit behavior, routes, and navigation from Spec 386 unchanged.
|
||||
|
||||
## Technical Context
|
||||
|
||||
**Language/Version**: PHP 8.4.15, Laravel 12.52.0
|
||||
**Primary Dependencies**: Filament 5.2.1, Livewire 4.1.4, Pest 4.3.1
|
||||
**Storage**: PostgreSQL via Sail; no schema changes in this spec
|
||||
**Testing**: Pest 4, Filament/Livewire component tests, focused browser smoke
|
||||
**Validation Lanes**: confidence + browser; focused fast-feedback feature tests
|
||||
**Target Platform**: Laravel monolith under `apps/platform`
|
||||
**Project Type**: web application / Filament admin panel
|
||||
**Performance Goals**: no new render-time Graph calls; no new remote work; no added polling
|
||||
**Constraints**: no new workflow engine, no new persistence, no top-level navigation, no global search, no auto-publish
|
||||
**Scale/Scope**: one existing subject-owned Review Publication Resolution workflow page and related blocked Review Detail CTA
|
||||
|
||||
## UI / Surface Guardrail Plan
|
||||
|
||||
- **Guardrail scope**: changed surfaces.
|
||||
- **Affected routes/pages/actions/states/navigation/panel/provider surfaces**:
|
||||
- `App\Filament\Resources\EnvironmentReviewResource\Pages\ResolveReviewPublication`
|
||||
- `apps/platform/resources/views/filament/resources/environment-review-resource/pages/resolve-review-publication.blade.php`
|
||||
- `ViewEnvironmentReview` / Environment Review blocked CTA state if current copy needs adjustment
|
||||
- Customer Review Workspace leakage tests only
|
||||
- **No-impact class, if applicable**: N/A.
|
||||
- **Native vs custom classification summary**: mixed existing Filament page + native Filament components + existing Blade composition.
|
||||
- **Shared-family relevance**: action labels, confirmation modals, proof disclosure, OperationRun links, customer-safe review boundary.
|
||||
- **State layers in scope**: page, action modal, proof disclosure, existing detail entry point.
|
||||
- **Audience modes in scope**: operator-MSP, manager, readonly inspector, support-platform; customer/read-only only for negative leakage.
|
||||
- **Decision/diagnostic/raw hierarchy plan**: decision-first, diagnostics-second, raw/support absent by default.
|
||||
- **Raw/support gating plan**: technical proof collapsed; raw provider/report/evidence payloads not rendered.
|
||||
- **One-primary-action / duplicate-truth control**: keep one current-step primary action when executable; demote operation/proof links and navigation.
|
||||
- **Handling modes by drift class or surface**: report-only for UI audit files unless rendered structure materially changes; review-mandatory for confirmation/action labels and customer non-leakage.
|
||||
- **Repository-signal treatment**: existing UI-101 report and Spec 386 tests are context; update only when rendered UI changes.
|
||||
- **Special surface test profiles**: workflow-detail surface, standard-native-filament, browser smoke.
|
||||
- **Required tests or manual smoke**: Filament/Livewire action tests and browser smoke for first-screen hierarchy, modal copy, collapsed proof, readonly state, mobile, and customer non-leakage.
|
||||
- **Exception path and spread control**: none approved.
|
||||
- **Active feature PR close-out entry**: Smoke Coverage / UX Hardening / No New Workflow Mechanics.
|
||||
- **UI/Productization coverage decision**: coverage artifacts update only if material rendered copy/structure changes; otherwise implementation close-out records no new route/archetype.
|
||||
- **Coverage artifacts to update**: likely `docs/ui-ux-enterprise-audit/page-reports/ui-101-review-publication-resolution.md`; route inventory/design matrix should need no route/archetype change.
|
||||
- **No-impact rationale**: N/A.
|
||||
- **Navigation / Filament provider-panel handling**: no provider registration or panel path change; Laravel 12 panel providers remain in `apps/platform/bootstrap/providers.php`.
|
||||
- **Screenshot or page-report need**: yes for changed user-facing states; proportional fallback notes are acceptable when a state cannot be produced with current fixtures.
|
||||
|
||||
## Shared Pattern & System Fit
|
||||
|
||||
- **Cross-cutting feature marker**: yes, but bounded to one workflow page.
|
||||
- **Systems touched**:
|
||||
- `ResolveReviewPublication`
|
||||
- `ReviewPublicationResolutionStepAuthorizer`
|
||||
- existing OperationRun UX/link helpers
|
||||
- existing customer workspace negative leakage checks
|
||||
- **Shared abstractions reused**:
|
||||
- native Filament `Actions\Action`
|
||||
- `UiEnforcement`
|
||||
- `OperationUxPresenter`
|
||||
- `OperationRunLinks`
|
||||
- existing scoped URL helpers
|
||||
- **New abstraction introduced? why?**: none by default. Page-local extraction is allowed only if it replaces duplication and remains review-publication-specific.
|
||||
- **Why the existing abstraction was sufficient or insufficient**: existing Spec 386 services and page architecture are sufficient; only labels/messages need hardening.
|
||||
- **Bounded deviation / spread control**: do not introduce a shared decision presenter or generic resolution framework.
|
||||
|
||||
## OperationRun UX Impact
|
||||
|
||||
- **Touches OperationRun start/completion/link UX?**: yes for visible link/copy only.
|
||||
- **Central contract reused**: existing OperationRun UX/link behavior from Spec 386.
|
||||
- **Delegated UX behaviors**: queued toast, run link, artifact link, browser event, dedupe messaging, safe URL resolution, and terminal notifications remain delegated to existing services/helpers.
|
||||
- **Surface-owned behavior kept local**: action label, modal copy, no-auto-publish copy, and proof disclosure ordering.
|
||||
- **Queued DB-notification policy**: unchanged.
|
||||
- **Terminal notification path**: unchanged.
|
||||
- **Exception path**: none.
|
||||
|
||||
## Provider Boundary & Portability Fit
|
||||
|
||||
- **Shared provider/platform boundary touched?**: no new provider/platform seam.
|
||||
- **Provider-owned seams**: required report generation, evidence generation, review refresh, and review-pack generation remain source-owned by existing Spec 386 services.
|
||||
- **Platform-core seams**: publication preparation copy, workflow page hierarchy, customer-safe boundary, action naming.
|
||||
- **Neutral platform terms / contracts preserved**: publication preparation, required reports, evidence, review, export, operation, technical proof.
|
||||
- **Retained provider-specific semantics and why**: "Permission posture" and "Entra admin roles" remain report labels because they are operator-relevant required reports.
|
||||
- **Bounded extraction or follow-up path**: follow-up spec only if proof/currentness, inbox intake, or restore adapters need new runtime semantics.
|
||||
|
||||
## Constitution Check
|
||||
|
||||
- Inventory-first: no inventory or snapshot source-of-truth changes.
|
||||
- Read/write separation: existing mutating/high-impact step actions keep confirmation, authorization, audit, and tests.
|
||||
- Graph contract path: no new Graph calls; existing source-owned actions remain authoritative.
|
||||
- Deterministic capabilities: no capability resolver changes.
|
||||
- RBAC-UX: existing workspace/environment policies and `ReviewPublicationResolutionStepAuthorizer` remain authoritative; readonly inspection must be explicit and non-executable.
|
||||
- Workspace isolation: existing scoped review/case resolution remains mandatory.
|
||||
- Tenant isolation: no cross-tenant data path is added.
|
||||
- Run observability: no new OperationRun types or transitions; existing links and start UX reused.
|
||||
- OperationRun start UX: no local queued toast/link/event composition beyond existing helpers.
|
||||
- Ops-UX lifecycle: no direct `OperationRun.status` or `OperationRun.outcome` changes.
|
||||
- Data minimization: no raw provider payloads, raw report contents, evidence JSON, secrets, or exception messages in default UI.
|
||||
- Test governance: focused Feature/Filament/Browser proof; no hidden heavy-governance expansion.
|
||||
- Proportionality: no new persistence, status family, generic presenter, or framework by default.
|
||||
- No premature abstraction: page-local mapping preferred over a new presenter.
|
||||
- Persisted truth: none added.
|
||||
- Behavioral state: no new state/status values.
|
||||
- UI semantics: direct mapping from existing step keys/statuses to operator copy; no new taxonomy.
|
||||
- Shared pattern first: existing Filament and OperationRun helpers reused.
|
||||
- Provider boundary: no provider-specific semantics spread into platform-core truth.
|
||||
- V1 explicitness / few layers: direct local implementation.
|
||||
- Badge semantics: existing Filament badges/shared badge semantics only; no ad-hoc status color language.
|
||||
- Filament-native UI: native Filament components and existing Blade composition retained; no new independent button/card/status system.
|
||||
- UI/Productization coverage: UI-101 coverage reused/updated proportionally.
|
||||
- Filament v5 / Livewire v4: implementation must remain Livewire 4.1.4 compatible and avoid Livewire v3 APIs.
|
||||
- Panel provider registration: no panel provider changes; Laravel 12 providers remain in `apps/platform/bootstrap/providers.php`.
|
||||
- Global search: no Resource is added; no global-search surface is introduced.
|
||||
- Destructive/high-impact actions: step execution and cancel remain `->action(...)` actions with `->requiresConfirmation()`, authorization, audit, and tests.
|
||||
- Asset strategy: no registered Filament assets expected; no `filament:assets` deploy change unless implementation unexpectedly registers assets and updates this plan first.
|
||||
|
||||
## Test Governance Check
|
||||
|
||||
- **Test purpose / classification by changed surface**: Feature for copy/RBAC/leakage, Filament/Livewire for page actions/modals, Browser for visual hierarchy/disclosure/mobile.
|
||||
- **Affected validation lanes**: confidence + browser; focused fast-feedback for feature tests.
|
||||
- **Why this lane mix is the narrowest sufficient proof**: this is visible Filament UI behavior over existing services, not schema/provider/runtime behavior.
|
||||
- **Narrowest proving command(s)**:
|
||||
- `cd apps/platform && ./vendor/bin/sail artisan test --compact tests/Feature/EnvironmentReview/Spec387ReviewPublicationResolutionDecisionUxTest.php`
|
||||
- `cd apps/platform && ./vendor/bin/sail artisan test --compact tests/Feature/EnvironmentReview/Spec386ReviewPublicationResolutionWorkflowTest.php`
|
||||
- `cd apps/platform && ./vendor/bin/sail php vendor/bin/pest tests/Browser/Spec387ReviewPublicationResolutionDecisionUxTest.php`
|
||||
- `cd apps/platform && ./vendor/bin/sail bin pint --dirty --format agent`
|
||||
- `git diff --check`
|
||||
- **Fixture / helper / factory / seed / context cost risks**: reuse Spec 386 fixtures/helpers; avoid new global defaults.
|
||||
- **Expensive defaults or shared helper growth introduced?**: no.
|
||||
- **Heavy-family additions, promotions, or visibility changes**: one explicit browser family/file if not extending existing Spec 386 browser smoke.
|
||||
- **Surface-class relief / special coverage rule**: workflow-detail surface requires browser smoke; otherwise standard-native-filament relief applies.
|
||||
- **Closing validation and reviewer handoff**: verify no new app mechanics, no new route/resource/global search, confirmation copy, readonly non-execution, no customer leakage, and screenshot/index evidence.
|
||||
- **Budget / baseline / trend follow-up**: none unless browser runtime grows materially.
|
||||
- **Review-stop questions**: did scope stay copy/UI-only; did disabled/denied states stay server-enforced; did proof stay secondary; did no-publish remain explicit.
|
||||
- **Escalation path**: document-in-feature if some browser states cannot be captured; follow-up-spec for proof/currentness or new adapter mechanics.
|
||||
- **Active feature PR close-out entry**: Smoke Coverage / UX Hardening / No New Workflow Mechanics.
|
||||
- **Why no dedicated follow-up spec is needed**: this is the dedicated residual UX hardening slice; broader proof/currentness/inbox/restore concerns are separate candidates.
|
||||
|
||||
## Project Structure
|
||||
|
||||
### Documentation (this feature)
|
||||
|
||||
```text
|
||||
specs/387-review-publication-resolution-decision-ux-v1/
|
||||
+-- checklists/
|
||||
| +-- requirements.md
|
||||
+-- plan.md
|
||||
+-- spec.md
|
||||
+-- tasks.md
|
||||
```
|
||||
|
||||
### Source Code (repository root)
|
||||
|
||||
```text
|
||||
apps/platform/app/Filament/Resources/EnvironmentReviewResource/Pages/
|
||||
+-- ResolveReviewPublication.php
|
||||
|
||||
apps/platform/app/Filament/Resources/EnvironmentReviewResource/Pages/
|
||||
+-- ViewEnvironmentReview.php
|
||||
|
||||
apps/platform/resources/views/filament/resources/environment-review-resource/pages/
|
||||
+-- resolve-review-publication.blade.php
|
||||
|
||||
apps/platform/tests/Feature/EnvironmentReview/
|
||||
+-- Spec386ReviewPublicationResolutionWorkflowTest.php
|
||||
+-- Spec387ReviewPublicationResolutionDecisionUxTest.php
|
||||
|
||||
apps/platform/tests/Browser/
|
||||
+-- Spec387ReviewPublicationResolutionDecisionUxTest.php
|
||||
|
||||
docs/ui-ux-enterprise-audit/page-reports/
|
||||
+-- ui-101-review-publication-resolution.md
|
||||
```
|
||||
|
||||
**Structure Decision**: Use existing Laravel/Filament app structure and existing Spec 386 test families. Do not create new base folders or runtime packages.
|
||||
|
||||
## Complexity Tracking
|
||||
|
||||
| Violation | Why Needed | Simpler Alternative Rejected Because |
|
||||
|-----------|------------|-------------------------------------|
|
||||
| N/A | No constitution violation is approved. | N/A |
|
||||
|
||||
## Proportionality Review
|
||||
|
||||
- **Current operator problem**: remaining implementation-first labels and generic confirmation affordances weaken a recently introduced decision workflow.
|
||||
- **Existing structure is insufficient because**: the existing structure is good but needs copy/state hardening; no new structure is required by default.
|
||||
- **Narrowest correct implementation**: update current page mappings, localization-backed Blade/page copy, action modal labels, tests, screenshots, and UI coverage notes.
|
||||
- **Ownership cost created**: focused tests and screenshots only.
|
||||
- **Alternative intentionally rejected**: a new generic presenter/framework or new workflow mechanics.
|
||||
- **Release truth**: current-release UX hardening.
|
||||
|
||||
## Implementation Phases
|
||||
|
||||
1. Confirm repo truth and current visible strings against Spec 386 implementation.
|
||||
2. Add focused tests for residual copy, modal, readonly, disclosure, and leakage behavior.
|
||||
3. Adjust existing page/action/view copy through existing localization-backed local mappings only.
|
||||
4. Capture browser screenshots and update UI-101 coverage notes if material.
|
||||
5. Run focused validation and record no implementation scope expansion.
|
||||
403
specs/387-review-publication-resolution-decision-ux-v1/spec.md
Normal file
@ -0,0 +1,403 @@
|
||||
# Feature Specification: Spec 387 - Review Publication Resolution Decision UX v1
|
||||
|
||||
**Feature Branch**: `387-review-publication-resolution-decision-ux-v1`
|
||||
**Created**: 2026-06-18
|
||||
**Status**: Draft / Ready for implementation planning
|
||||
**Input**: User-provided draft candidate "Spec 387 - Review Publication Resolution Decision UX v1" from `/Users/ahmeddarrazi/.codex/attachments/68c5630e-4d21-42fa-bba6-e49ee38ab08a/pasted-text.txt`.
|
||||
|
||||
## Repo-Truth Adjustment
|
||||
|
||||
The supplied draft describes a broad UX/productization pass after Spec 386. Current repo truth shows that Spec 386 already implemented much of the decision-first target:
|
||||
|
||||
- `ResolveReviewPublication` already renders "Review can't be published yet".
|
||||
- The page already shows "Publication preparation".
|
||||
- Technical proof is already placed under "Technical proof and operation history".
|
||||
- Existing Spec 386 Feature and Browser tests assert decision-first copy, "Update required reports", hidden "Report-backed evidence", and confirmation behavior.
|
||||
- UI-101 already registers the route as a browser-verified strategic surface with decision-first direction.
|
||||
|
||||
This Spec 387 package therefore narrows the user draft to residual hardening only:
|
||||
|
||||
- finish remaining operator-facing terminology mismatches such as "Generate review pack" versus "Prepare export" and "Return to publication" versus "Return to review";
|
||||
- tighten modal heading, body, and submit labels so each mutating step names the actual action and states that it will not publish the review;
|
||||
- add explicit readonly/capability-denied inspection copy instead of relying only on a disabled action tooltip;
|
||||
- verify state-specific copy for waiting, failed, ready-to-continue, ready-for-publication, and no-blocker states;
|
||||
- verify technical proof remains collapsed by default and does not compete with the next safe action;
|
||||
- update or confirm UI audit coverage and screenshot evidence for this narrower hardening pass.
|
||||
|
||||
Spec 387 must not duplicate the Spec 386 workflow, data model, persistence, services, actions, route, navigation, or global search behavior.
|
||||
|
||||
## Candidate Selection Gate
|
||||
|
||||
- **Selected candidate**: Spec 387 - Review Publication Resolution Decision UX v1.
|
||||
- **Source**: Direct user-provided candidate attachment.
|
||||
- **Why selected**: The user explicitly supplied a ready Spec 387 draft, and repo inspection found residual copy/state/confirmation/readonly hardening gaps that are smaller than the original draft but still visible to operators.
|
||||
- **Roadmap relationship**: Supports the roadmap's current productization and moat priority around customer-safe review consumption and decision-first governance workflows. This is a manual user-provided candidate; it does not reopen the active auto-prep queue, which currently says there is no safe automatic next-best target.
|
||||
- **Close alternatives deferred**:
|
||||
- Active auto-prep queue candidates remain closed or manual-promotion only in `docs/product/spec-candidates.md`.
|
||||
- Management Report PDF staging/runtime validation remains tied to Specs 378-380.
|
||||
- Governance artifact lifecycle/retention runtime remains manual-promotion backlog.
|
||||
- Governance Inbox resolution intake and generic resolution adapters remain follow-up candidates, not hidden scope.
|
||||
- Spec 386 must not be rewritten or converted back into preparation state.
|
||||
- **Completed-spec guardrail result**:
|
||||
- `specs/386-review-publication-resolution-workflow-v1/` is completed implementation context with route, UI, tests, screenshots, and UI audit registry evidence; it is not modified by this preparation.
|
||||
- Related Specs 350, 351, 385, and UI-101 are dependency/context only.
|
||||
- No existing `specs/387-*` package or `*387*` branch was found before Spec Kit creation.
|
||||
- **Smallest viable implementation slice**: A focused UX/copy hardening pass over the existing Review Publication Resolution page and Review Detail blocked CTA, preserving all Spec 386 runtime behavior.
|
||||
- **Gate result**: PASS, with scope reduction. The original draft is too broad relative to current repo truth; this narrowed slice is not already fully covered.
|
||||
|
||||
## Spec Candidate Check *(mandatory - SPEC-GATE-001)*
|
||||
|
||||
- **Problem**: The functional Review Publication Resolution page is already decision-first in structure, but a few remaining labels, modal affordances, disabled-action states, and state-specific messages can still expose internal workflow wording or leave the operator inferring the safest next step.
|
||||
- **Today's failure**: Operators may still see "Generate review pack", "Return to publication", a generic confirmation submit label, or a disabled action without an explicit page-level permission explanation, even though the product intent is "prepare export", "return to review", and "this will not publish".
|
||||
- **User-visible improvement**: The existing page becomes more immediately understandable: one next safe action, action-specific confirmation, no publish ambiguity, clear readonly inspection state, and technical proof available without dominating the first decision.
|
||||
- **Smallest enterprise-capable version**: Adjust labels/copy/state handling on the existing page, add or update focused Feature/Filament/Browser tests, and refresh screenshot/UI coverage notes only where the rendered UI changes.
|
||||
- **Explicit non-goals**: No new resolution data model, migrations, workflow engine, adapter framework, top-level navigation, global-search resource, auto-publish path, provider/evidence/review-pack service changes, customer-facing resolution flow, or new OperationRun lifecycle behavior.
|
||||
- **Permanent complexity imported**: Focused copy mappings, tests, screenshots, and possibly a page-local extraction only if it replaces duplicated mapping. No new durable source of truth, no new enum/status family, no generic presenter/framework, and no new queue or storage behavior.
|
||||
- **Why now**: Spec 386 just introduced the workflow, so this is the right time to remove residual implementation-first wording before the surface becomes a repeated operator workflow.
|
||||
- **Why not local**: This is local to the existing resolution page and should stay local; the spec exists because the change is user-facing and must carry UI, RBAC, audit, and browser-smoke expectations without reopening Spec 386.
|
||||
- **Approval class**: Workflow Compression.
|
||||
- **Red flags triggered**: UI polish over a recently completed workflow. Defense: the scope is explicitly narrowed, no new architecture is approved, and implementation must prefer direct local mapping over a new framework.
|
||||
- **Score**: Nutzen: 2 | Dringlichkeit: 1 | Scope: 2 | Komplexitaet: 2 | Produktnaehe: 2 | Wiederverwendung: 1 | **Gesamt: 10/12**
|
||||
- **Decision**: approve as a narrowed residual UX hardening slice.
|
||||
|
||||
## Problem Statement
|
||||
|
||||
Spec 386 gave TenantPilot a resumable Review Publication Resolution workflow for blocked Environment Reviews. Current code already presents the main decision-first structure, but residual copy and state handling can still make the surface feel partly implementation-led.
|
||||
|
||||
The operator should be able to answer within five seconds:
|
||||
|
||||
1. Can I publish? No, not yet.
|
||||
2. Why not? Required preparation inputs are missing, stale, running, or failed.
|
||||
3. What should I do now? Take the one next safe action.
|
||||
4. Will this publish automatically? No.
|
||||
5. Where is proof? In collapsed technical proof and operation history.
|
||||
|
||||
## Business / Product Value
|
||||
|
||||
- Reduces operator hesitation during blocked review publication.
|
||||
- Preserves the trust boundary that preparation actions never publish automatically.
|
||||
- Keeps customer-safe review output separate from internal remediation mechanics.
|
||||
- Makes support/proof details available without turning the page into a workflow-engine debug surface.
|
||||
|
||||
## Primary Users / Operators
|
||||
|
||||
- MSP or workspace operator preparing an Environment Review for publication.
|
||||
- Workspace manager reviewing blocked publication state and next action.
|
||||
- Readonly or support user inspecting an existing resolution case without execution rights.
|
||||
- Customer-facing review consumers indirectly protected by the non-leakage boundary.
|
||||
|
||||
## Spec Scope Fields *(mandatory)*
|
||||
|
||||
- **Scope**: workspace-owned / managed-environment-scoped Review Publication Resolution UI over existing Spec 386 persistence and services.
|
||||
- **Primary Routes**:
|
||||
- `/admin/workspaces/{workspace}/environments/{environment}/environment-reviews/{record}`
|
||||
- `/admin/workspaces/{workspace}/environments/{environment}/environment-reviews/{record}/resolve-publication`
|
||||
- Customer Review Workspace only for leakage regression checks; no new customer resolution UI.
|
||||
- **Data Ownership**:
|
||||
- `ReviewPublicationResolutionCase` and `ReviewPublicationResolutionStep` remain Spec 386 workflow state.
|
||||
- `OperationRun` remains execution/proof truth.
|
||||
- Evidence Snapshot, Environment Review, Review Pack, and Stored Report remain artifact/review truth.
|
||||
- Spec 387 adds no persisted truth.
|
||||
- **RBAC**: Existing policies, capabilities, `ReviewPublicationResolutionStepAuthorizer`, `UiEnforcement`, workspace membership, and managed-environment entitlement remain authoritative. Non-member/not-entitled access remains deny-as-not-found; entitled members without execution capability may inspect only when current policies allow it.
|
||||
|
||||
For canonical-view specs:
|
||||
|
||||
- **Default filter behavior when tenant-context is active**: N/A. This spec does not add canonical-view filtering or revive retired `/admin/t` routes.
|
||||
- **Explicit entitlement checks preventing cross-tenant leakage**: Existing `EnvironmentReviewResource` scoped record resolution and case policy checks remain mandatory; customer workspace leakage tests must continue to prove no internal resolution detail is exposed.
|
||||
|
||||
## UI Surface Impact *(mandatory - UI-COV-001)*
|
||||
|
||||
Does this spec add, remove, rename, or materially change any reachable UI surface?
|
||||
|
||||
- [ ] No UI surface impact
|
||||
- [x] Existing page changed
|
||||
- [ ] New page/route added
|
||||
- [ ] Navigation changed
|
||||
- [ ] Filament panel/provider surface changed
|
||||
- [ ] New modal/drawer/wizard/action added
|
||||
- [ ] New table/form/state added
|
||||
- [ ] Customer-facing surface changed
|
||||
- [x] Dangerous action changed
|
||||
- [x] Status/evidence/review presentation changed
|
||||
- [ ] Workspace/environment context presentation changed
|
||||
|
||||
Clarification: no new modal/action class is approved. Existing Filament action modal copy and existing page action states are changed. Customer-facing scope is negative leakage/regression coverage only unless implementation proves existing copy leaks or overclaims and updates this spec before changing customer-visible UI.
|
||||
|
||||
## UI/Productization Coverage *(mandatory when UI Surface Impact is not "No UI surface impact")*
|
||||
|
||||
- **Route/page/surface**:
|
||||
- Review Publication Resolution page (`ResolveReviewPublication`)
|
||||
- Environment Review detail blocked publication CTA
|
||||
- Customer Review Workspace leakage boundary checks only
|
||||
- **Current or new page archetype**: existing UI-101 Reviews strategic workflow surface; existing UI-040 Environment Review detail; existing UI-006 Customer Review Workspace for no-leakage boundary only.
|
||||
- **Design depth**: Strategic Surface for UI-101; Domain Pattern Surface for CTA copy on UI-040; customer-safe regression only for UI-006.
|
||||
- **Repo-truth level**: repo-verified and browser-verified through Spec 386.
|
||||
- **Existing pattern reused**: UI-101 page report, Spec 386 page/action implementation, native Filament sections/badges/actions/modals, existing `UiEnforcement`, existing OperationRun link helpers.
|
||||
- **New pattern required**: none. Direct local mapping is preferred. A new generic presenter or framework is not approved.
|
||||
- **Screenshot required**: yes for changed states where feasible, under `specs/387-review-publication-resolution-decision-ux-v1/artifacts/screenshots/`.
|
||||
- **Page audit required**: update UI-101 only if rendered structure/copy materially changes; otherwise record a no-new-route coverage note in the implementation close-out.
|
||||
- **Customer-safe review required**: yes, negative leakage check only; no customer-facing resolution UI is planned.
|
||||
- **Dangerous-action review required**: yes for existing high-impact/mutating step action and existing cancel action. Step execution keeps `->action(...)`, `->requiresConfirmation()`, server authorization, audit, and notifications from Spec 386.
|
||||
- **Coverage files updated or explicitly not needed**:
|
||||
- [ ] `docs/ui-ux-enterprise-audit/route-inventory.md`
|
||||
- [ ] `docs/ui-ux-enterprise-audit/design-coverage-matrix.md`
|
||||
- [x] `docs/ui-ux-enterprise-audit/page-reports/...`
|
||||
- [ ] `docs/ui-ux-enterprise-audit/strategic-surfaces.md`
|
||||
- [ ] `docs/ui-ux-enterprise-audit/grouped-follow-up-candidates.md`
|
||||
- [ ] `docs/ui-ux-enterprise-audit/unresolved-pages.md`
|
||||
- [ ] `N/A - no reachable UI surface impact`
|
||||
- **No-impact rationale when applicable**: N/A.
|
||||
|
||||
## Cross-Cutting / Shared Pattern Reuse *(mandatory)*
|
||||
|
||||
- **Cross-cutting feature?**: yes, but limited to one existing workflow surface.
|
||||
- **Interaction class(es)**: status messaging, action labels, header actions, confirmation modal copy, OperationRun links, proof disclosure, customer-safe review boundary.
|
||||
- **Systems touched**:
|
||||
- `ResolveReviewPublication`
|
||||
- `ViewEnvironmentReview`
|
||||
- `ReviewPublicationResolutionStepAuthorizer`
|
||||
- `OperationUxPresenter` / `OperationRunLinks` only as existing link/copy context
|
||||
- Customer Review Workspace leakage tests only
|
||||
- **Existing pattern(s) to extend**: existing Spec 386 page methods, native Filament Actions, `UiEnforcement`, Filament badges/sections, existing OperationRun UX link helpers.
|
||||
- **Shared contract / presenter / builder / renderer to reuse**: existing page-local mapping and shared OperationRun helpers. Do not introduce a cross-domain presenter.
|
||||
- **Why the existing shared path is sufficient or insufficient**: The existing path is sufficient for runtime behavior. It is insufficient only in a few labels and state messages.
|
||||
- **Allowed deviation and why**: page-local extraction is allowed only if it replaces duplicated mapping and stays review-publication-specific.
|
||||
- **Consistency impact**: Button labels, modal headings, modal submit labels, run titles/notifications where touched, tests, and audit-facing copy must keep the same domain vocabulary.
|
||||
- **Review focus**: no new workflow engine, no action-start bypass, no local OperationRun UX composition, no internal terms in the default operator layer.
|
||||
|
||||
## OperationRun UX Impact *(mandatory)*
|
||||
|
||||
- **Touches OperationRun start/completion/link UX?**: yes, copy and link presentation only. No new OperationRun start/completion semantics.
|
||||
- **Shared OperationRun UX contract/layer reused**: existing `OperationUxPresenter`, `OperationRunLinks`, and Spec 386 operation-link behavior.
|
||||
- **Delegated start/completion UX behaviors**: queued toast/link/event/dedupe/terminal notification behavior remains unchanged and service-owned.
|
||||
- **Local surface-owned behavior that remains**: action label, modal copy, "Open operation" secondary link, and proof disclosure ordering.
|
||||
- **Queued DB-notification policy**: unchanged; no new queued DB notification.
|
||||
- **Terminal notification path**: unchanged.
|
||||
- **Exception required?**: none.
|
||||
|
||||
## Provider Boundary / Platform Core Check *(mandatory)*
|
||||
|
||||
- **Shared provider/platform boundary touched?**: no new shared seam. Existing provider/evidence/report/review/export steps remain source-owned.
|
||||
- **Boundary classification**: platform-core UI vocabulary over a review workflow; provider-owned details stay behind existing proof/diagnostics.
|
||||
- **Seams affected**: visible labels for existing step keys and proof links only.
|
||||
- **Neutral platform terms preserved or introduced**: publication preparation, required reports, collect evidence, refresh review, prepare export, return to review, technical proof, operation.
|
||||
- **Provider-specific semantics retained and why**: "Entra admin roles" and "Permission posture" may remain as report requirement labels because they are current report names visible to operators.
|
||||
- **Why this does not deepen provider coupling accidentally**: no Graph/provider code is changed; provider-specific proof remains diagnostic and existing.
|
||||
- **Follow-up path**: Resolution Proof & Currentness Contract, Governance Inbox Resolution Intake, and Restore Readiness Resolution Adapter remain follow-up candidates only.
|
||||
|
||||
## UI / Surface Guardrail Impact *(mandatory)*
|
||||
|
||||
| Surface / Change | Operator-facing surface change? | Native vs Custom | Shared-Family Relevance | State Layers Touched | Exception Needed? | Low-Impact / N/A Note |
|
||||
|---|---|---|---|---|---|---|
|
||||
| Review Publication Resolution page copy/state hardening | yes | Native Filament page + existing Blade composition | action labels, status messaging, proof disclosure | page, detail | no | existing route only |
|
||||
| Step confirmation modal copy | yes | Filament Action modal | confirmation/dangerous-action safety | action modal | no | existing action only |
|
||||
| Environment Review blocked CTA copy | yes | existing Filament resource page/action | review publication action link | detail | no | copy/summary only |
|
||||
| Customer Review Workspace leakage regression | no material customer feature change | existing page | customer-safe boundary | page | no | negative test/smoke only |
|
||||
|
||||
## Decision-First Surface Role *(mandatory when operator-facing surfaces are changed)*
|
||||
|
||||
| Surface | Decision Role | Human-in-the-loop Moment | Immediately Visible for First Decision | On-Demand Detail / Evidence | Why This Is Primary or Why Not | Workflow Alignment | Attention-load Reduction |
|
||||
|---|---|---|---|---|---|---|---|
|
||||
| Review Publication Resolution page | Primary Decision Surface | Operator decides and executes the next safe preparation step | blocked/ready state, missing requirement summary, one primary action, no-auto-publish copy | operation links, proof artifacts, technical step keys if needed | Primary because it owns the blocked-publication fix flow | review publication preparation | removes internal workflow wording from the first decision |
|
||||
| Environment Review detail blocked CTA | Primary Decision entry point | Operator decides whether to resolve blockers before publication | publication blocked explanation and one CTA | review sections/evidence/proof | Primary entry point only, not the whole workflow | review lifecycle | avoids promoting refresh/publish while blocked |
|
||||
| Customer Review Workspace | Customer-safe context | Customer/output reader sees no internal resolution mechanics | customer-safe unavailable/preparing state where applicable | none from resolution case | Not a resolution surface | customer review consumption | avoids leakage and action confusion |
|
||||
|
||||
## Audience-Aware Disclosure *(mandatory when detail/status surfaces change)*
|
||||
|
||||
| Surface | Audience Modes In Scope | Decision-First Default-Visible Content | Operator Diagnostics | Support / Raw Evidence | One Dominant Next Action | Hidden / Gated By Default | Duplicate-Truth Prevention |
|
||||
|---|---|---|---|---|---|---|---|
|
||||
| Review Publication Resolution page | operator-MSP, manager, readonly inspector, support-platform | can/cannot publish, why, next action, no-auto-publish | proof links, operation status, evaluated time, safe reason code | raw provider/report/evidence payloads remain hidden | current step action, or open operation/return to review by state | proof/history collapsed; raw data absent | blocker stated once; checklist and proof add context only |
|
||||
| Environment Review detail | operator-MSP, manager, readonly inspector | publication blocked explanation and CTA | evidence/review sections | raw details in existing deeper surfaces | resolve publication blockers while blocked | case internals hidden | detail points to workflow instead of duplicating it |
|
||||
| Customer Review Workspace | customer/read-only, operator-MSP | customer-safe review availability only | none from internal resolution | internal resolution proof absent | none introduced | all internal resolution mechanics hidden | no duplicate or leaked blocker mechanics |
|
||||
|
||||
## UI/UX Surface Classification *(mandatory when operator-facing surfaces change)*
|
||||
|
||||
| Surface | Action Surface Class | Surface Type | Likely Next Operator Action | Primary Inspect/Open Model | Row Click | Secondary Actions Placement | Destructive Actions Placement | Canonical Collection Route | Canonical Detail Route | Scope Signals | Canonical Noun | Critical Truth Visible by Default | Exception Type / Justification |
|
||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
||||
| Review Publication Resolution | Workflow / Primary Decision | Subject-driven workflow detail | run current preparation step, open operation, or return to review | direct page route from review | N/A | related links and proof disclosure | cancel in More with confirmation | none | existing resolve-publication route | workspace + environment + review | Publication preparation | can publish, blocker, next step, no-auto-publish | no collection/global search by design |
|
||||
| Environment Review detail | Detail / Review Context | Existing review detail | resolve blockers or continue existing lifecycle | existing detail page | existing behavior only | secondary evidence/proof links | existing destructive actions remain separated | existing review collection | existing review detail route | workspace + environment | Environment review | publication blocked state | none |
|
||||
|
||||
## Operator Surface Contract *(mandatory when operator-facing surfaces change)*
|
||||
|
||||
| Surface | Primary Persona | Decision / Operator Action Supported | Surface Type | Primary Operator Question | Default-visible Information | Diagnostics-only Information | Status Dimensions Used | Mutation Scope | Primary Actions | Dangerous Actions |
|
||||
|---|---|---|---|---|---|---|---|---|---|---|
|
||||
| Review Publication Resolution page | MSP/workspace operator | Resolve the current publication blocker without publishing | workflow detail | Why is publication blocked and what is the next safe action? | blocker summary, required reports, next safe action, checklist, no-auto-publish note | proof links, operation history, technical step keys | preparation readiness, operation execution state, artifact proof state | TenantPilot artifact/workflow work only; provider work only through existing source-owned actions | update required reports, collect evidence, refresh review, prepare export, return to review | cancel resolution; step execution is high-impact and confirmation-gated |
|
||||
| Environment Review detail CTA | MSP/workspace operator | Enter resolution flow when publication is blocked | review detail | Can I publish, and if not where do I resolve blockers? | blocked explanation and resolve CTA | review/evidence details | review lifecycle/readiness | existing review lifecycle only | resolve publication blockers | existing publish/archive actions remain separate |
|
||||
|
||||
## Proportionality Review *(mandatory when structural complexity is introduced)*
|
||||
|
||||
- **New source of truth?**: no.
|
||||
- **New persisted entity/table/artifact?**: no.
|
||||
- **New abstraction?**: no by default. A page-local helper extraction is allowed only if it replaces duplicated mapping and stays bounded to `ResolveReviewPublication`.
|
||||
- **New enum/state/reason family?**: no.
|
||||
- **New cross-domain UI framework/taxonomy?**: no.
|
||||
- **Current operator problem**: residual internal wording and generic modal affordances can slow or confuse the publication preparation decision.
|
||||
- **Existing structure is insufficient because**: the existing page has the right architecture and most decision-first copy, but several labels and state messages still need alignment.
|
||||
- **Narrowest correct implementation**: update existing local mappings, Blade copy, Filament action modal labels, and tests.
|
||||
- **Ownership cost**: focused test/screenshot maintenance only.
|
||||
- **Alternative intentionally rejected**: a new `ReviewPublicationResolutionDecisionPresenter` or generic resolution UI framework is rejected unless implementation proves the existing page methods are unsafe to maintain.
|
||||
- **Release truth**: current-release UX hardening over an already implemented workflow.
|
||||
|
||||
### Compatibility posture
|
||||
|
||||
This feature assumes a pre-production environment. Backward compatibility shims, legacy aliases, and migration compatibility code are out of scope.
|
||||
|
||||
## Testing / Lane / Runtime Impact *(mandatory for runtime behavior changes)*
|
||||
|
||||
- **Test purpose / classification**: Feature + Filament/Livewire + Browser.
|
||||
- **Validation lane(s)**: confidence + browser; fast-feedback for focused feature tests.
|
||||
- **Why this classification and these lanes are sufficient**: The risk is visible UI and action confirmation behavior on an existing Filament page, plus customer non-leakage. No schema or Graph behavior changes are approved.
|
||||
- **New or expanded test families**: add/update focused Spec 387 tests near existing Spec 386 Environment Review and Browser tests.
|
||||
- **Fixture / helper cost impact**: reuse existing Spec 386 factories/helpers/fixtures; no new expensive global setup.
|
||||
- **Heavy-family visibility / justification**: browser coverage is explicit because the feature is layout/copy/disclosure-sensitive.
|
||||
- **Special surface test profile**: workflow-detail surface / standard-native-filament with browser smoke.
|
||||
- **Standard-native relief or required special coverage**: ordinary Filament/Livewire action tests plus browser smoke for first-screen order, modal copy, collapsed disclosure, mobile, and customer non-leakage.
|
||||
- **Reviewer handoff**: verify no new workflow mechanics, no enabled action for unauthorized execution, no internal terms in default UI, and no publish action on the resolution page.
|
||||
- **Budget / baseline / trend impact**: low; one focused browser smoke may add explicit browser lane runtime.
|
||||
- **Escalation needed**: document-in-feature if screenshots for some states cannot be produced with current fixtures.
|
||||
- **Active feature PR close-out entry**: Smoke Coverage / UX Hardening / No New Workflow Mechanics.
|
||||
- **Planned validation commands**:
|
||||
- `cd apps/platform && ./vendor/bin/sail artisan test --compact tests/Feature/EnvironmentReview/Spec387ReviewPublicationResolutionDecisionUxTest.php`
|
||||
- `cd apps/platform && ./vendor/bin/sail artisan test --compact tests/Feature/EnvironmentReview/Spec386ReviewPublicationResolutionWorkflowTest.php`
|
||||
- `cd apps/platform && ./vendor/bin/sail php vendor/bin/pest tests/Browser/Spec387ReviewPublicationResolutionDecisionUxTest.php`
|
||||
- `cd apps/platform && ./vendor/bin/sail bin pint --dirty --format agent`
|
||||
- `git diff --check`
|
||||
|
||||
## User Scenarios & Testing *(mandatory)*
|
||||
|
||||
### User Story 1 - Understand the Blocked Publication Decision (Priority: P1)
|
||||
|
||||
As an operator opening a blocked review's publication preparation page, I need the first visible area to explain why the review cannot be published, which requirements are missing, and the one next safe action.
|
||||
|
||||
**Why this priority**: This is the core 5-second decision the page exists to support.
|
||||
|
||||
**Independent Test**: Open a blocked Environment Review resolution page and verify the first heading/card uses decision-first copy, required report labels, one primary next action, and no prominent internal case/step/proof terms.
|
||||
|
||||
**Acceptance Scenarios**:
|
||||
|
||||
1. **Given** a blocked review with missing required reports, **When** an authorized operator opens the resolution page, **Then** the page shows "Review can't be published yet", the missing required reports, "Update required reports", and visible no-auto-publish copy before technical proof.
|
||||
2. **Given** the same page, **When** the operator scans the preparation checklist, **Then** the checklist uses operator labels such as "Update required reports", "Collect evidence", "Refresh review", "Prepare export", and "Return to review".
|
||||
|
||||
---
|
||||
|
||||
### User Story 2 - Execute the Next Step With Clear Confirmation (Priority: P1)
|
||||
|
||||
As an authorized operator, I need the confirmation modal to name the exact preparation action, use a matching confirm button, and clearly state that the action will not publish the review.
|
||||
|
||||
**Why this priority**: Step actions can queue work or touch provider/evidence/report services; the safety boundary must be explicit.
|
||||
|
||||
**Independent Test**: Mount the Filament page, open each reachable step action confirmation, and verify title, body, submit label, `->requiresConfirmation()`, and server authorization remain intact.
|
||||
|
||||
**Acceptance Scenarios**:
|
||||
|
||||
1. **Given** the current step is required reports, **When** the operator starts the action, **Then** the modal heading is "Update required reports?", the confirm button is "Update required reports", and the body says the review will not be published.
|
||||
2. **Given** the current step is prepare export, **When** the operator starts the action, **Then** the visible copy uses "Prepare export" instead of "Generate review pack" by default.
|
||||
|
||||
---
|
||||
|
||||
### User Story 3 - Inspect Without Execution Rights (Priority: P2)
|
||||
|
||||
As a readonly user who is allowed to inspect an existing case, I need a clear explanation that I can inspect the preparation flow but cannot run the next action.
|
||||
|
||||
**Why this priority**: Disabled actions without page-level explanation can look broken or like a permission leak.
|
||||
|
||||
**Independent Test**: Open an existing resolution case as a readonly actor and verify the primary action is disabled or absent, execution remains denied server-side, and an explicit page-level permission message appears.
|
||||
|
||||
**Acceptance Scenarios**:
|
||||
|
||||
1. **Given** a readonly actor with view access but no step execution capability, **When** the actor opens the resolution page, **Then** the page shows "You can inspect this preparation flow, but you do not have permission to run the next action." or equivalent.
|
||||
2. **Given** the same actor attempts direct execution, **When** the action handler runs, **Then** the server denies execution and no operation/report/evidence/review-pack job is dispatched.
|
||||
|
||||
---
|
||||
|
||||
### User Story 4 - Keep Proof Secondary and Customer Surfaces Clean (Priority: P2)
|
||||
|
||||
As an operator or customer-safe reviewer, I need proof available for inspection without exposing internal resolution mechanics on the default page or customer workspace.
|
||||
|
||||
**Why this priority**: The workflow should feel like guided governance, not a visible workflow engine or support console.
|
||||
|
||||
**Independent Test**: Browser smoke verifies technical proof is collapsed by default on the resolution page and customer workspace does not show resolution case, step, OperationRun, proof, or internal blocker terms.
|
||||
|
||||
**Acceptance Scenarios**:
|
||||
|
||||
1. **Given** an operator opens the resolution page, **When** the page first renders, **Then** "Technical proof and operation history" is collapsed by default and below the decision content.
|
||||
2. **Given** a customer-facing review workspace, **When** a review is being prepared internally, **Then** no resolution case, step key, OperationRun link, proof link, or internal blocker reason code is visible.
|
||||
|
||||
### Edge Cases
|
||||
|
||||
- The linked operation is running: show operation-in-progress copy and do not expose another start/retry action while the operation is active.
|
||||
- The linked operation failed: show safe failed-operation copy, safe reason only, and a retry action only if existing Spec 386 rules allow it.
|
||||
- No blockers remain: do not create or emphasize a workflow; return the operator to the normal review publication path.
|
||||
- Ready for publication: keep publish on the Environment Review detail page; never publish from the resolution page.
|
||||
- Missing fixtures for a browser state: document the unavailable state in the screenshot index rather than widening product code.
|
||||
|
||||
## Requirements *(mandatory)*
|
||||
|
||||
### Functional Requirements
|
||||
|
||||
- **FR-387-001**: The resolution page MUST keep decision-first heading and summary copy as the first default-visible content.
|
||||
- **FR-387-002**: The visible checklist and secondary return navigation/action label MUST use operator labels: `Check readiness`, `Update required reports`, `Collect evidence`, `Refresh review`, `Prepare export`, and `Return to review`.
|
||||
- **FR-387-003**: Default operator-facing UI MUST NOT prominently show "Resolution Case", "Case Status", "Current step", "Resolution steps", "Report-backed evidence", "OperationRun", "Artifact proof", or raw step keys.
|
||||
- **FR-387-004**: Technical proof and operation history MUST remain collapsed by default and below decision/next-action content.
|
||||
- **FR-387-005**: Every mutating/high-impact step action MUST continue to execute through `Action::make(...)->action(...)`, keep `->requiresConfirmation()`, and use existing server-side authorization.
|
||||
- **FR-387-006**: Step confirmation copy MUST include action-specific heading/body/submit labels and MUST state that the action will not publish the review, except the return-to-review step which must state that publishing remains separate. Each reachable current-step state MUST be covered by a test assertion or a named fixture-unavailable note.
|
||||
- **FR-387-007**: The resolution page MUST NOT show a Publish action.
|
||||
- **FR-387-008**: The Environment Review detail blocked state MUST keep `Resolve publication blockers` as the primary blocked-publication CTA and MUST NOT promote refresh or publish while blockers remain.
|
||||
- **FR-387-009**: Readonly/capability-denied inspectors MUST see an explicit page-level permission message and no enabled executable primary action.
|
||||
- **FR-387-010**: Customer-facing surfaces MUST NOT expose resolution case details, step keys, proof links, operation links, or internal blocker reason codes.
|
||||
- **FR-387-011**: No new route, navigation item, global-search resource, model, migration, queue family, provider adapter, workflow engine, or auto-publish path may be added.
|
||||
- **FR-387-012**: Existing Spec 386 audit, RBAC, source-owned action, failure redaction, and no-auto-publish guarantees MUST remain intact.
|
||||
- **FR-387-013**: New or changed visible UI copy MUST use existing localization files/keys where practical; any bounded operator-only localization debt MUST be recorded in the implementation close-out.
|
||||
|
||||
## UI Action Matrix *(mandatory when Filament is changed)*
|
||||
|
||||
| Surface | Location | Header Actions | Inspect Affordance (List/Table) | Row Actions (max 2 visible) | Bulk Actions (grouped) | Empty-State CTA(s) | View Header Actions | Create/Edit Save+Cancel | Audit log? | Notes / Exemptions |
|
||||
|---|---|---|---|---|---|---|---|---|---|---|
|
||||
| Review Publication Resolution page | `ResolveReviewPublication` | one current-step action when executable, `Return to review`/secondary nav, More with `Cancel resolution` | N/A | N/A | N/A | N/A | current-step action confirmation required; cancel confirmation required | N/A | Spec 386 audit remains required for mutating steps/cancel | No publish action; no global search; no top-level nav |
|
||||
| Environment Review detail | `ViewEnvironmentReview` / `EnvironmentReviewResource` | existing lifecycle actions plus blocked CTA | existing detail route | N/A | N/A | existing | `Resolve publication blockers` primary while blocked | N/A | existing audit rules remain | Refresh/publish must not compete while blockers remain |
|
||||
|
||||
### Key Entities *(include if feature involves data)*
|
||||
|
||||
- **ReviewPublicationResolutionCase**: Existing Spec 386 workflow-state record; no schema or lifecycle expansion in Spec 387.
|
||||
- **ReviewPublicationResolutionStep**: Existing Spec 386 step record; no new step keys or statuses in Spec 387.
|
||||
- **OperationRun**: Existing execution proof; no new start/completion semantics in Spec 387.
|
||||
|
||||
## Success Criteria *(mandatory)*
|
||||
|
||||
### Measurable Outcomes
|
||||
|
||||
- **SC-387-001**: Browser smoke shows the first visible resolution page content answers blocked state, reason, next action, and no-auto-publish before proof/history.
|
||||
- **SC-387-002**: Focused tests prove internal terms are absent from default operator UI and proof remains collapsed by default.
|
||||
- **SC-387-003**: Focused tests prove each current-step confirmation keeps `->requiresConfirmation()` and action-specific no-auto-publish copy.
|
||||
- **SC-387-004**: Focused tests prove readonly inspectors cannot execute the current step and see explicit permission copy.
|
||||
- **SC-387-005**: Customer workspace regression proves no internal resolution mechanics leak.
|
||||
- **SC-387-006**: No migrations, model changes, route additions, global-search resource, top-level navigation, or auto-publish behavior appear in the diff.
|
||||
|
||||
## Out of Scope
|
||||
|
||||
- New resolution persistence or status families.
|
||||
- Generic resolution/workflow adapter frameworks.
|
||||
- Provider/evidence/review-pack service behavior changes except label wiring to existing actions.
|
||||
- Governance Inbox intake.
|
||||
- Restore readiness adapters.
|
||||
- Customer-facing resolution workflows.
|
||||
- PDF/report runtime validation.
|
||||
- Broad UI redesign beyond the existing resolution page and CTA copy.
|
||||
|
||||
## Assumptions
|
||||
|
||||
- Spec 386 is the source of runtime workflow truth and remains implemented.
|
||||
- The current branch contains the latest Spec 386 code and tests.
|
||||
- The user-provided draft is treated as manual promotion and is narrowed against repo truth.
|
||||
- Any implementation that discovers a real runtime bug must update this spec/plan before expanding beyond UX hardening.
|
||||
|
||||
## Open Questions
|
||||
|
||||
None blocking. Implementation may choose whether small mapping cleanup stays as page-local methods or a page-owned helper; a generic/shared presenter is not approved without a spec update.
|
||||
|
||||
## Follow-up Spec Candidates
|
||||
|
||||
- Resolution Proof & Currentness Contract v1.
|
||||
- Governance Inbox Resolution Intake v1.
|
||||
- Restore Readiness Resolution Adapter v1.
|
||||
- Provider/readiness-specific wording improvements if browser/user evidence shows continued friction.
|
||||
113
specs/387-review-publication-resolution-decision-ux-v1/tasks.md
Normal file
@ -0,0 +1,113 @@
|
||||
# Tasks: Spec 387 - Review Publication Resolution Decision UX v1
|
||||
|
||||
**Input**: Design documents from `/specs/387-review-publication-resolution-decision-ux-v1/`
|
||||
**Prerequisites**: `spec.md`, `plan.md`, `checklists/requirements.md`
|
||||
**Tests**: Required. This feature changes visible Filament/Livewire UI behavior, confirmation copy, authorization affordances, and browser-visible hierarchy over an existing high-impact workflow.
|
||||
|
||||
## Test Governance Checklist
|
||||
|
||||
- [x] Lane assignment is named and is the narrowest sufficient proof for changed UI/action behavior.
|
||||
- [x] New or changed tests stay in focused Feature/Filament/Browser families.
|
||||
- [x] Shared helpers, factories, seeds, fixtures, and context defaults stay cheap by default.
|
||||
- [x] Planned validation commands cover the change without pulling unrelated lane cost.
|
||||
- [x] The workflow-detail surface test profile and browser smoke need are explicit.
|
||||
- [x] Any unavailable browser state screenshot is documented in the active spec artifacts rather than widened through product code.
|
||||
|
||||
## Phase 1: Preparation and Repo Truth
|
||||
|
||||
**Purpose**: Confirm the implementation starts from Spec 386 runtime truth and avoid duplicate workflow work.
|
||||
|
||||
- [x] T001 Confirm current branch/status and re-read `specs/387-review-publication-resolution-decision-ux-v1/spec.md`, `plan.md`, and `tasks.md`.
|
||||
- [x] T002 Re-read `specs/386-review-publication-resolution-workflow-v1/spec.md`, `plan.md`, and `tasks.md` as completed implementation context only; do not rewrite that package.
|
||||
- [x] T003 Inspect current visible labels in `apps/platform/app/Filament/Resources/EnvironmentReviewResource/Pages/ResolveReviewPublication.php`.
|
||||
- [x] T004 Inspect current rendered structure in `apps/platform/resources/views/filament/resources/environment-review-resource/pages/resolve-review-publication.blade.php`.
|
||||
- [x] T005 Inspect Environment Review blocked CTA behavior in `ViewEnvironmentReview` and `EnvironmentReviewResource`.
|
||||
- [x] T006 Inspect existing Spec 386 Feature and Browser tests to extend them without broad fixture duplication.
|
||||
- [x] T007 Confirm no migration, model, route, navigation, global-search resource, panel provider, queue family, provider service, or auto-publish behavior is required.
|
||||
- [x] T008 Confirm Filament v5 / Livewire v4.0+ compliance and that panel provider registration remains `apps/platform/bootstrap/providers.php`.
|
||||
|
||||
## Phase 2: Tests First - Decision Copy and Internal Terms
|
||||
|
||||
**Purpose**: Prove the residual UX contract before changing copy.
|
||||
|
||||
- [x] T009 [P] Add or update a Feature/Filament test proving the first visible resolution page copy includes `Review can't be published yet`, required reports, one next safe action, and no-auto-publish copy.
|
||||
- [x] T010 [P] Add assertions that default operator UI does not prominently show `Resolution Case`, `Case Status`, `Current step`, `Resolution steps`, `Report-backed evidence`, `OperationRun`, `Artifact proof`, or raw step keys.
|
||||
- [x] T011 [P] Add assertions that checklist labels and the secondary return navigation/header action use `Check readiness`, `Update required reports`, `Collect evidence`, `Refresh review`, `Prepare export`, and `Return to review` where applicable.
|
||||
- [x] T012 [P] Add assertions that technical proof/history is collapsed by default and appears below decision content.
|
||||
- [x] T013 [P] Add assertions that the resolution page does not show a Publish action.
|
||||
|
||||
## Phase 3: Tests First - Confirmation and Authorization UX
|
||||
|
||||
**Purpose**: Lock the high-impact action safety contract.
|
||||
|
||||
- [x] T014 [P] Add Filament action tests proving the current step action still requires confirmation through `->requiresConfirmation()`.
|
||||
- [x] T015 [P] Add confirmation copy assertions for required reports: heading `Update required reports?`, submit label `Update required reports`, and no-auto-publish body copy.
|
||||
- [x] T016 [P] Add confirmation copy assertions for collect evidence, refresh review, prepare export, and return-to-review states; if a state cannot be produced safely with existing fixtures, document that specific state and reason in the screenshot/test evidence index.
|
||||
- [x] T017 [P] Add readonly inspection assertions proving page-level permission copy appears and the executable action is disabled or absent.
|
||||
- [x] T018 Add a negative execution assertion proving readonly/direct execution denial dispatches no operation/report/evidence/review-pack job.
|
||||
|
||||
## Phase 4: Tests First - State Copy and Customer Boundary
|
||||
|
||||
**Purpose**: Prove edge states and customer-safe non-leakage.
|
||||
|
||||
- [x] T019 [P] Add or update tests for waiting/running operation copy and no duplicate start/retry action while the operation is running.
|
||||
- [x] T020 [P] Add or update tests for failed operation copy using safe reason code/normalized copy only.
|
||||
- [x] T021 [P] Add or update tests for ready-to-continue and ready-for-publication copy, proving Publish remains on the Review Detail page only.
|
||||
- [x] T022 [P] Add or update customer workspace regression tests proving no resolution case, step key, OperationRun link, proof link, or internal blocker reason code leaks.
|
||||
|
||||
## Phase 5: Implementation - Copy and Local Mappings
|
||||
|
||||
**Purpose**: Apply the narrow UX hardening with no workflow mechanics.
|
||||
|
||||
- [x] T023 Update `ResolveReviewPublication::currentStepActionLabelFor()` and the secondary `back_to_review` header action to use `Prepare export` for review-pack generation and `Return to review` for return-to-publication copy where visible to operators.
|
||||
- [x] T024 Update `ResolveReviewPublication::operatorStepLabel()` and `operatorStepDescription()` to use the Spec 387 operator vocabulary consistently through existing localization files/keys where practical.
|
||||
- [x] T025 Update confirmation modal heading/body/submit label logic so each reachable mutating step has localization-backed action-specific copy and a matching submit button.
|
||||
- [x] T026 Add explicit page-level readonly/capability-denied inspection copy when the user may inspect but cannot execute the next step.
|
||||
- [x] T027 Update running, failed, ready-to-continue, ready-for-publication, and no-blocker state copy through existing localization files/keys only as needed to satisfy tests.
|
||||
- [x] T028 Keep `Proof`/`Operation` links neutral and secondary inside collapsed technical proof/history.
|
||||
- [x] T029 Keep `Cancel resolution` inside More, confirmation-gated, authorization-gated, and visually secondary/destructive.
|
||||
- [x] T030 Confirm no page-local Blade logic starts jobs, infers capabilities, calls providers, or decides business readiness.
|
||||
|
||||
## Phase 6: Environment Review CTA and Customer Boundary
|
||||
|
||||
**Purpose**: Align entry and non-leakage surfaces without adding a customer flow.
|
||||
|
||||
- [x] T031 Update Environment Review blocked CTA surrounding copy if current copy does not clearly state that required publication inputs are missing/stale and `Resolve publication blockers` is the primary action.
|
||||
- [x] T032 Ensure Refresh or Publish is not visually primary while blockers remain.
|
||||
- [x] T033 Ensure Customer Review Workspace receives no new resolution UI; update only safe unavailable/preparing copy if a regression test proves existing copy leaks or overclaims.
|
||||
|
||||
## Phase 7: UI/Productization Coverage and Screenshots
|
||||
|
||||
**Purpose**: Record proportional coverage for the changed strategic surface.
|
||||
|
||||
- [x] T034 Capture desktop screenshot of Review Detail blocked CTA under `specs/387-review-publication-resolution-decision-ux-v1/artifacts/screenshots/`.
|
||||
- [x] T035 Capture desktop screenshot of resolution decision summary with missing reports.
|
||||
- [x] T036 Capture screenshot of confirmation modal showing action-specific no-auto-publish copy.
|
||||
- [x] T037 Capture screenshots of technical proof collapsed and expanded states.
|
||||
- [x] T038 Capture readonly inspection screenshot with no executable primary action.
|
||||
- [x] T039 Capture mobile screenshot proving the decision card remains first and actions do not overlap.
|
||||
- [x] T040 Capture or document customer workspace no-leakage evidence.
|
||||
- [x] T041 Document unavailable state screenshots in a screenshot index if fixtures cannot produce running, failed, or ready states safely.
|
||||
- [x] T042 Update `docs/ui-ux-enterprise-audit/page-reports/ui-101-review-publication-resolution.md` only if rendered copy/structure materially changes; otherwise record no-new-route/no-archetype rationale in implementation close-out.
|
||||
|
||||
## Phase 8: Validation
|
||||
|
||||
**Purpose**: Prove the narrowed UX hardening and no application scope expansion beyond UI copy/action affordance.
|
||||
|
||||
- [x] T043 Run `cd apps/platform && ./vendor/bin/sail artisan test --compact tests/Feature/EnvironmentReview/Spec387ReviewPublicationResolutionDecisionUxTest.php`.
|
||||
- [x] T044 Run `cd apps/platform && ./vendor/bin/sail artisan test --compact tests/Feature/EnvironmentReview/Spec386ReviewPublicationResolutionWorkflowTest.php`.
|
||||
- [x] T045 Run `cd apps/platform && ./vendor/bin/sail php vendor/bin/pest tests/Browser/Spec387ReviewPublicationResolutionDecisionUxTest.php`.
|
||||
- [x] T046 Run `cd apps/platform && ./vendor/bin/sail bin pint --dirty --format agent`.
|
||||
- [x] T047 Run `git diff --check`.
|
||||
- [x] T048 Record implementation close-out with Livewire v4 compliance, provider registration location, global search status, destructive/high-impact action handling, asset strategy, localization handling or bounded localization debt, tests run, browser smoke result, deployment impact, and explicit no-new-workflow confirmation.
|
||||
|
||||
## Explicit Non-Goals
|
||||
|
||||
- [x] NT001 Do not modify completed Spec 386 artifacts except as read-only context.
|
||||
- [x] NT002 Do not create migrations, models, new persisted entities, new status/enum families, or new source-of-truth records.
|
||||
- [x] NT003 Do not create a generic workflow engine, adapter registry, cross-domain presenter, or broad resolution UI framework.
|
||||
- [x] NT004 Do not add top-level navigation, a collection route, a Resource, or global search for resolution cases.
|
||||
- [x] NT005 Do not auto-publish reviews or move Publish onto the resolution page.
|
||||
- [x] NT006 Do not change provider, evidence, review refresh, report, or review-pack service behavior except label/copy wiring through existing actions.
|
||||
- [x] NT007 Do not expose internal resolution mechanics to customer-facing surfaces.
|
||||
- [x] NT008 Do not register new Filament assets unless the spec/plan are updated first.
|
||||