feat: refine onboarding draft flow and RBAC diff UX

This commit is contained in:
Ahmed Darrazi 2026-03-14 21:08:32 +01:00
parent 0b5cadc234
commit 25e0dcc353
30 changed files with 2011 additions and 197 deletions

View File

@ -73,6 +73,8 @@ ## Active Technologies
- PHP 8.4.15 + Laravel 12, Filament v5, Livewire v4, PostgreSQL, Laravel Sail, Pest v4, existing `OperationRunService`, `ProviderOperationStartGate`, onboarding services, workspace audit logging (140-onboarding-lifecycle-operation-checkpoints-concurrency-mvp)
- PostgreSQL tables including `managed_tenant_onboarding_sessions`, `operation_runs`, `tenants`, and provider-connection-backed tenant records (140-onboarding-lifecycle-operation-checkpoints-concurrency-mvp)
- PostgreSQL via Laravel Sail for existing source records and JSON payloads; no new persistence introduced (141-shared-diff-presentation-foundation)
- PHP 8.4.15 / Laravel 12 + Filament v5, Livewire v4.0+, Tailwind CSS v4, shared `App\Support\Diff` foundation from Spec 141 (142-rbac-role-definition-diff-ux-upgrade)
- PostgreSQL via Laravel Sail for existing `findings.evidence_jsonb`; no schema or persistence changes (142-rbac-role-definition-diff-ux-upgrade)
- PHP 8.4.15 (feat/005-bulk-operations)
@ -92,8 +94,8 @@ ## Code Style
PHP 8.4.15: Follow standard conventions
## Recent Changes
- 142-rbac-role-definition-diff-ux-upgrade: Added PHP 8.4.15 / Laravel 12 + Filament v5, Livewire v4.0+, Tailwind CSS v4, shared `App\Support\Diff` foundation from Spec 141
- 141-shared-diff-presentation-foundation: Added PHP 8.4.15 / Laravel 12 + Filament v5, Livewire v4.0+, Tailwind CSS v4
- 140-onboarding-lifecycle-operation-checkpoints-concurrency-mvp: Added PHP 8.4.15 + Laravel 12, Filament v5, Livewire v4, PostgreSQL, Laravel Sail, Pest v4, existing `OperationRunService`, `ProviderOperationStartGate`, onboarding services, workspace audit logging
- 139-verify-access-permissions-assist: Added PHP 8.4 (Laravel 12) + Filament v5 (Livewire v4), Laravel Blade, existing onboarding/verification support classes
<!-- MANUAL ADDITIONS START -->
<!-- MANUAL ADDITIONS END -->

View File

@ -147,7 +147,7 @@ protected function getHeaderActions(): array
{
$actions = [];
if ($this->onboardingSession instanceof TenantOnboardingSession) {
if ($this->shouldShowDraftLandingAction()) {
$actions[] = Action::make('back_to_onboarding_landing')
->label('All onboarding drafts')
->color('gray')
@ -921,6 +921,8 @@ private function cancelOnboardingDraft(): void
->title('Onboarding draft cancelled')
->success()
->send();
$this->redirect(route('admin.onboarding.draft', ['onboardingDraft' => $this->onboardingSession]));
}
private function showsNonResumableSummary(): bool
@ -929,6 +931,29 @@ private function showsNonResumableSummary(): bool
&& ! $this->onboardingSession->isResumable();
}
private function shouldShowDraftLandingAction(): bool
{
if (! $this->onboardingSession instanceof TenantOnboardingSession) {
return false;
}
if (! $this->onboardingSession->isResumable()) {
return false;
}
if (! isset($this->workspace)) {
return false;
}
$user = $this->currentUser();
if (! $user instanceof User) {
return false;
}
return $this->availableDraftsFor($user)->count() > 1;
}
private function draftTitle(TenantOnboardingSession $draft): string
{
$state = is_array($draft->state) ? $draft->state : [];

View File

@ -1,8 +1,14 @@
<?php
declare(strict_types=1);
namespace App\Filament\Resources\TenantResource\Pages;
use App\Filament\Resources\TenantResource;
use App\Models\User;
use App\Models\Workspace;
use App\Services\Onboarding\OnboardingDraftResolver;
use App\Support\Workspaces\WorkspaceContext;
use Filament\Actions;
use Filament\Resources\Pages\ListRecords;
@ -13,10 +19,7 @@ class ListTenants extends ListRecords
protected function getHeaderActions(): array
{
return [
Actions\Action::make('add_tenant')
->label('Add tenant')
->icon('heroicon-m-plus')
->url(route('admin.onboarding'))
$this->makeOnboardingEntryAction()
->visible(fn (): bool => $this->getTableRecords()->count() > 0),
];
}
@ -24,10 +27,60 @@ protected function getHeaderActions(): array
protected function getTableEmptyStateActions(): array
{
return [
Actions\Action::make('add_tenant')
->label('Add tenant')
->icon('heroicon-m-plus')
->url(route('admin.onboarding')),
$this->makeOnboardingEntryAction(),
];
}
private function makeOnboardingEntryAction(): Actions\Action
{
return Actions\Action::make('add_tenant')
->label($this->onboardingEntryLabel())
->icon($this->onboardingEntryIcon())
->url(route('admin.onboarding'));
}
private function onboardingEntryLabel(): string
{
$draftCount = $this->accessibleResumableDraftCount();
return match (true) {
$draftCount === 1 => 'Continue onboarding',
$draftCount > 1 => 'Choose onboarding draft',
default => 'Add tenant',
};
}
private function onboardingEntryIcon(): string
{
$draftCount = $this->accessibleResumableDraftCount();
return match (true) {
$draftCount === 1 => 'heroicon-m-arrow-path',
$draftCount > 1 => 'heroicon-m-queue-list',
default => 'heroicon-m-plus',
};
}
private function accessibleResumableDraftCount(): int
{
$user = auth()->user();
if (! $user instanceof User) {
return 0;
}
$workspaceId = app(WorkspaceContext::class)->currentWorkspaceId(request());
if (! is_int($workspaceId)) {
return 0;
}
$workspace = Workspace::query()->whereKey($workspaceId)->first();
if (! $workspace instanceof Workspace) {
return 0;
}
return app(OnboardingDraftResolver::class)->resumableDraftsFor($user, $workspace)->count();
}
}

View File

@ -0,0 +1,205 @@
<?php
declare(strict_types=1);
namespace App\Support\Diff;
final class RbacRoleDefinitionDiffBuilder
{
private const ROLE_FIELD_ORDER = [
'Role definition > Display name' => 100,
'Role definition > Description' => 200,
'Role definition > Role source' => 300,
'Role definition > Permission blocks' => 400,
'Role definition > Scope tag IDs' => 500,
];
private const PERMISSION_FIELD_ORDER = [
'Allowed actions' => 100,
'Denied actions' => 200,
'Conditions' => 300,
];
public function __construct(
private readonly DiffPresenter $presenter,
) {}
/**
* @param array<string, mixed> $payload
*/
public function build(array $payload): DiffPresentation
{
$baseline = $this->normalizeSide(
is_array($payload['baseline'] ?? null) ? $payload['baseline'] : [],
);
$current = $this->normalizeSide(
is_array($payload['current'] ?? null) ? $payload['current'] : [],
);
$changedKeys = $this->changedKeys($payload);
$presentation = $this->presenter->present(
baseline: $baseline,
current: $current,
changedKeys: $changedKeys,
labels: $this->labelsFor($baseline, $current, $changedKeys),
);
if ($presentation->rows === []) {
return $presentation;
}
$rows = array_map(
fn (DiffRow $row): DiffRow => $this->normalizeRow($row),
$presentation->rows,
);
usort($rows, fn (DiffRow $left, DiffRow $right): int => $this->sortKey($left->key) <=> $this->sortKey($right->key));
return new DiffPresentation(
summary: DiffSummary::fromRows($rows, $presentation->summary->message),
rows: $rows,
);
}
/**
* @param array<string, mixed> $payload
* @return list<string>
*/
private function changedKeys(array $payload): array
{
$changedKeys = is_array($payload['changed_keys'] ?? null) ? $payload['changed_keys'] : [];
return array_values(array_filter(
$changedKeys,
fn (mixed $key): bool => is_string($key) && trim($key) !== '',
));
}
/**
* @param array<string, mixed> $side
* @return array<string, mixed>
*/
private function normalizeSide(array $side): array
{
$normalized = [];
$rows = is_array($side['normalized'] ?? null) ? $side['normalized'] : [];
foreach ($rows as $key => $value) {
if (! is_string($key) || trim($key) === '') {
continue;
}
$normalized[trim($key)] = $value;
}
if (! array_key_exists('Role definition > Role source', $normalized)) {
$roleSource = $this->roleSourceLabel($side['is_built_in'] ?? null);
if ($roleSource !== null) {
$normalized['Role definition > Role source'] = $roleSource;
}
}
if (
! array_key_exists('Role definition > Permission blocks', $normalized)
&& is_numeric($side['role_permission_count'] ?? null)
) {
$normalized['Role definition > Permission blocks'] = (int) $side['role_permission_count'];
}
return $normalized;
}
private function roleSourceLabel(mixed $isBuiltIn): ?string
{
return match ($isBuiltIn) {
true => 'Built-in',
false => 'Custom',
default => null,
};
}
/**
* @param array<string, mixed> $baseline
* @param array<string, mixed> $current
* @param list<string> $changedKeys
* @return array<string, string>
*/
private function labelsFor(array $baseline, array $current, array $changedKeys): array
{
$labels = [];
foreach ([array_keys($baseline), array_keys($current), $changedKeys] as $sourceKeys) {
foreach ($sourceKeys as $key) {
if (! is_string($key) || trim($key) === '') {
continue;
}
$labels[$key] = $key;
}
}
return $labels;
}
private function normalizeRow(DiffRow $row): DiffRow
{
$shouldRenderAsList = $row->isListLike && $this->isDesignatedListField($row->key);
if ($row->isListLike === $shouldRenderAsList) {
return $row;
}
return new DiffRow(
key: $row->key,
label: $row->label,
status: $row->status,
oldValue: $row->oldValue,
newValue: $row->newValue,
isListLike: $shouldRenderAsList,
addedItems: $shouldRenderAsList ? $row->addedItems : [],
removedItems: $shouldRenderAsList ? $row->removedItems : [],
unchangedItems: $shouldRenderAsList ? $row->unchangedItems : [],
meta: $row->meta,
);
}
private function isDesignatedListField(string $key): bool
{
if ($key === 'Role definition > Scope tag IDs') {
return true;
}
return preg_match('/^Permission block \d+ > (Allowed actions|Denied actions|Conditions)$/', $key) === 1;
}
/**
* @return array{int, int, int, string}
*/
private function sortKey(string $key): array
{
if (array_key_exists($key, self::ROLE_FIELD_ORDER)) {
return [0, self::ROLE_FIELD_ORDER[$key], 0, mb_strtolower($key)];
}
if (preg_match('/^Role definition > (?P<label>.+)$/', $key, $matches) === 1) {
return [0, 900, 0, mb_strtolower((string) ($matches['label'] ?? $key))];
}
if (preg_match('/^Permission block (?P<block>\d+) > (?P<label>.+)$/', $key, $matches) === 1) {
return [
1,
(int) ($matches['block'] ?? 0),
$this->permissionFieldOrder((string) ($matches['label'] ?? '')),
mb_strtolower((string) ($matches['label'] ?? $key)),
];
}
return [2, 0, 0, mb_strtolower($key)];
}
private function permissionFieldOrder(string $label): int
{
return self::PERMISSION_FIELD_ORDER[$label] ?? 900;
}
}

View File

@ -27,7 +27,6 @@ ## Do Not Use It When
The current specialized renderers stay specialized unless a later spec chooses otherwise:
- `resources/views/filament/infolists/entries/normalized-diff.blade.php`
- `resources/views/filament/infolists/entries/rbac-role-definition-diff.blade.php`
- `resources/views/filament/infolists/entries/assignments-diff.blade.php`
- `resources/views/filament/infolists/entries/scope-tags-diff.blade.php`
@ -76,6 +75,27 @@ ## Blade Usage
`compact` reduces spacing for dense layouts. `dimUnchanged` keeps unchanged content quieter than meaningful changes.
## RBAC Role Definition Adoption
`resources/views/filament/infolists/entries/rbac-role-definition-diff.blade.php` is the first concrete consumer of the shared diff foundation.
Its consumer-local builder lives in `app/Support/Diff/RbacRoleDefinitionDiffBuilder.php` and is responsible for:
- preserving the existing RBAC evidence payload as the input contract
- adding fallback rows for `Role source` and `Permission blocks` when side metadata exists
- keeping top-level role metadata ahead of permission-block detail in a deterministic order
- designating these RBAC list-like keys for inline add/remove rendering:
- `Role definition > Scope tag IDs`
- `Permission block * > Allowed actions`
- `Permission block * > Denied actions`
- `Permission block * > Conditions`
Consumer-local choices for the first RBAC pass:
- unchanged rows stay visible but muted through `dimUnchanged`
- one-sided RBAC rows render through the shared `added` and `removed` states
- no local “show only changes” toggle is shipped in this first pass
## Standalone Stringifier Usage
If a specialized renderer should not adopt `DiffPresenter`, it can still reuse `ValueStringifier`:

View File

@ -1,62 +1,9 @@
@php
$payload = $getState() ?? [];
$changedKeys = is_array($payload['changed_keys'] ?? null) ? $payload['changed_keys'] : [];
$baseline = is_array($payload['baseline'] ?? null) ? $payload['baseline'] : [];
$current = is_array($payload['current'] ?? null) ? $payload['current'] : [];
$baselineNormalized = is_array($baseline['normalized'] ?? null) ? $baseline['normalized'] : [];
$currentNormalized = is_array($current['normalized'] ?? null) ? $current['normalized'] : [];
use App\Support\Diff\RbacRoleDefinitionDiffBuilder;
$payload = is_array($getState()) ? $getState() : [];
$diffKind = is_string($payload['diff_kind'] ?? null) ? (string) $payload['diff_kind'] : 'permission_change';
$stringify = static function (mixed $value): string {
if ($value === null) {
return '-';
}
if (is_bool($value)) {
return $value ? 'Yes' : 'No';
}
if (is_array($value)) {
return implode(', ', array_map(static fn (mixed $item): string => (string) $item, $value));
}
return (string) $value;
};
$roleSourceLabel = static function (mixed $isBuiltIn): string {
return match ($isBuiltIn) {
true => __('findings.rbac.built_in'),
false => __('findings.rbac.custom'),
default => '-',
};
};
$sideRows = static function (array $normalized, array $side) use ($roleSourceLabel): array {
$rows = [];
foreach ($normalized as $key => $value) {
if (! is_string($key) || $key === '') {
continue;
}
$rows[$key] = $value;
}
if (! array_key_exists('Role definition > Role source', $rows)) {
$rows['Role definition > Role source'] = $roleSourceLabel($side['is_built_in'] ?? null);
}
if (! array_key_exists('Role definition > Permission blocks', $rows) && is_numeric($side['role_permission_count'] ?? null)) {
$rows['Role definition > Permission blocks'] = (int) $side['role_permission_count'];
}
ksort($rows);
return $rows;
};
$baselineRows = $sideRows($baselineNormalized, $baseline);
$currentRows = $sideRows($currentNormalized, $current);
$presentation = app(RbacRoleDefinitionDiffBuilder::class)->build($payload);
@endphp
<div class="space-y-4">
@ -64,55 +11,27 @@
:heading="__('findings.rbac.detail_heading')"
:description="__('findings.rbac.' . $diffKind)"
>
<div class="flex flex-wrap gap-2">
<x-filament::badge color="gray">
{{ __('findings.rbac.' . $diffKind) }}
</x-filament::badge>
@if ($changedKeys !== [])
<x-filament::badge color="warning">
{{ __('findings.rbac.changed_fields') }}: {{ count($changedKeys) }}
</x-filament::badge>
@endif
</div>
<div class="space-y-4">
@include('filament.partials.diff.summary-badges', [
'summary' => $presentation->summary,
])
@if ($changedKeys !== [])
<div class="mt-4 space-y-2">
<div class="text-sm font-semibold text-gray-950 dark:text-white">{{ __('findings.rbac.changed_fields') }}</div>
<div class="flex flex-wrap gap-2">
@foreach ($changedKeys as $changedKey)
<x-filament::badge color="gray">{{ $changedKey }}</x-filament::badge>
@if ($presentation->rows !== [])
<div class="space-y-3">
@foreach ($presentation->rows as $row)
@include('filament.partials.diff.row', [
'row' => $row,
'compact' => false,
'dimUnchanged' => true,
])
@endforeach
</div>
</div>
@endif
<div class="mt-4 grid gap-4 lg:grid-cols-2">
@foreach ([
['heading' => __('findings.rbac.baseline'), 'rows' => $baselineRows],
['heading' => __('findings.rbac.current'), 'rows' => $currentRows],
] as $section)
<div class="rounded-xl border border-gray-200 bg-white p-4 shadow-sm dark:border-white/10 dark:bg-gray-900">
<div class="text-sm font-semibold text-gray-950 dark:text-white">{{ $section['heading'] }}</div>
@if ($section['rows'] === [])
<div class="mt-3 text-sm text-gray-500 dark:text-gray-400">{{ __('findings.rbac.absent') }}</div>
@else
<dl class="mt-3 space-y-3">
@foreach ($section['rows'] as $label => $value)
<div>
<dt class="text-xs font-medium uppercase tracking-wide text-gray-500 dark:text-gray-400">{{ $label }}</dt>
<dd class="mt-1 text-sm text-gray-900 dark:text-gray-100">{{ $stringify($value) }}</dd>
</div>
@endforeach
</dl>
@endif
</div>
@endforeach
</div>
<div class="mt-4 rounded-lg border border-gray-200 bg-gray-50 p-3 text-sm text-gray-700 dark:border-white/10 dark:bg-gray-950 dark:text-gray-300">
<div class="rounded-lg border border-gray-200 bg-gray-50 p-3 text-sm text-gray-700 dark:border-white/10 dark:bg-gray-950 dark:text-gray-300">
{{ __('findings.rbac.assignments_excluded') }}
{{ __('findings.rbac.restore_unsupported') }}
</div>
</div>
</x-filament::section>
</div>

View File

@ -33,7 +33,9 @@
if ($canSeeAllWorkspaceTenants) {
$tenants = Tenant::query()
->where('workspace_id', (int) $workspace->getKey())
->where('status', Tenant::STATUS_ACTIVE)
->orderBy('name')
->orderBy('environment')
->get();
} else {
$tenants = collect($user->getTenants(Filament::getCurrentOrDefaultPanel()))

View File

@ -12,7 +12,7 @@
<article
aria-labelledby="{{ $rowId }}"
class="rounded-xl border border-warning-200 bg-warning-50/70 {{ $padding }} dark:border-warning-500/30 dark:bg-warning-500/10"
class="rounded-xl border border-warning-200 bg-warning-50/70 shadow-sm shadow-warning-950/5 {{ $padding }} dark:border-warning-500/30 dark:bg-warning-500/10"
>
<div class="flex flex-wrap items-start justify-between gap-3">
<div class="space-y-1">

View File

@ -15,7 +15,7 @@
<article
aria-labelledby="{{ $rowId }}"
class="rounded-xl border border-gray-200 bg-white {{ $padding }} dark:border-white/10 dark:bg-gray-900"
class="rounded-xl border border-gray-200 bg-white border-dashed opacity-95 {{ $padding }} dark:border-white/10 dark:bg-gray-900"
>
<div class="flex flex-wrap items-start justify-between gap-3">
<div class="space-y-1">

View File

@ -29,6 +29,9 @@ ### Filament v5 Implementation Notes
- **Destructive actions**: Any new cancel-draft or terminate-draft controls must execute through confirmed Filament actions with server-side authorization.
- **Asset strategy**: No new Filament assets are planned. Existing deployment practice still includes `php artisan filament:assets`, but this feature does not add new asset registrations.
- **Testing plan**: Add focused feature or Livewire tests for landing and draft routes, unit tests for resume-step derivation and draft-query helpers, and mandatory browser tests for hard refresh and multi-draft selection.
- **Concrete draft navigation polish**: The draft header must only expose `All onboarding drafts` when the landing route can resolve to a distinct multi-draft picker state; single-draft routes should not advertise a self-redirect.
- **Tenant list onboarding CTA clarity**: Tenant list entry actions that target `/admin/onboarding` must mirror the landing state in their labels so operators are not told `Add tenant` when the result is actually resume or draft selection.
- **Cancellation UX clarity**: Destructive draft cancellation should finish with a canonical redirect back to the draft URL so the operator immediately sees the non-editable cancelled summary from fresh DB state.
## Constitution Check

View File

@ -146,6 +146,8 @@ ### Functional Requirements
- **FR-138-26 Activation safety preserved**: Refresh and revisit must not bypass existing activation safeguards, blocked-verification override requirements, confirmations, or authorization.
- **FR-138-27 Multiple-tab determinism**: Same-draft multi-tab usage must be deterministic, with last confirmed write winning while server-side validation protects invariants.
- **FR-138-28 Browser regression coverage**: Browser tests must validate the hard-refresh experience on a concrete draft URL.
- **FR-138-29 Onboarding entry CTA clarity**: Entry points that link to `/admin/onboarding` must label the action according to landing behavior: `Add tenant` when no resumable draft exists, `Continue onboarding` when exactly one resumable draft exists, and `Choose onboarding draft` when multiple resumable drafts exist.
- **FR-138-30 Cancellation transition clarity**: After cancelling a resumable onboarding draft, the UI must redirect to the canonical draft URL and immediately render the non-editable cancelled summary instead of leaving the operator on the stale editable wizard surface.
### Non-Goals
@ -183,7 +185,7 @@ ## 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 |
|---|---|---|---|---|---|---|---|---|---|---|
| Onboarding landing | `/admin/onboarding` | Start new onboarding | Draft list rows when multiple drafts exist | Resume, More | None | Start managed tenant onboarding | None | Continue and Cancel within wizard only | Yes | Auto-redirect is allowed only when exactly one resumable draft exists. |
| Concrete onboarding draft wizard | `/admin/onboarding/{draft}` | Resume context actions only | Stepper plus status banner | None | None | Not applicable | Verify, Bootstrap, Activate as already supported | Continue and Cancel | Yes | Sensitive fields stay empty after reload; destructive actions require confirmation. |
| Concrete onboarding draft wizard | `/admin/onboarding/{draft}` | Resume context actions only | Stepper plus status banner | None | None | Not applicable | Verify, Bootstrap, Activate as already supported | Continue and Cancel | Yes | Sensitive fields stay empty after reload; destructive actions require confirmation; an `All onboarding drafts` header action is shown only when more than one resumable draft exists so the landing link never no-ops back to the same draft. |
| Draft picker list | Landing screen when multiple drafts exist | Start new onboarding | Row click-through to detail or resume | Resume, View summary | None | Start new onboarding | None | Not applicable | Yes | Must show attribution, stage, status, and age. |
| Non-resumable draft summary | Direct access to completed or cancelled draft | View summary | Summary sections | None or policy-approved actions only | None | Return to onboarding landing | None | Not applicable | Yes | Must not re-enter editable wizard mode. |

View File

@ -134,6 +134,9 @@ ## Phase 7: Polish & Cross-Cutting Concerns
- [X] T036 [P] Align onboarding draft terminology across wizard, headers, notifications, and actions in `app/Filament/Pages/Workspaces/ManagedTenantOnboardingWizard.php` and related UI copy sources
- [X] T037 [P] Validate focused quickstart scenarios from `specs/138-managed-tenant-onboarding-draft-identity/quickstart.md` using the targeted onboarding and browser suites as a quality gate
- [X] T038 Run formatting and final cleanup with `vendor/bin/sail bin pint --dirty --format agent` after implementation changes
- [X] T039 [P] Hide the `All onboarding drafts` header action when the current draft is the only resumable draft and add focused routing regression coverage in `app/Filament/Pages/Workspaces/ManagedTenantOnboardingWizard.php` and `tests/Feature/Onboarding/OnboardingDraftRoutingTest.php`
- [X] T040 [P] Align tenant-list onboarding entry labels with landing semantics (`Add tenant` / `Continue onboarding` / `Choose onboarding draft`) in `app/Filament/Resources/TenantResource/Pages/ListTenants.php` and `tests/Feature/Filament/CreateCtaPlacementTest.php`
- [X] T041 [P] Redirect cancelled onboarding drafts back to their canonical draft URL and assert the immediate non-editable cancelled summary in `app/Filament/Pages/Workspaces/ManagedTenantOnboardingWizard.php` and `tests/Feature/Onboarding/OnboardingDraftLifecycleTest.php`
---

View File

@ -0,0 +1,34 @@
# Specification Quality Checklist: RBAC Role Definition Diff UX Upgrade
**Purpose**: Validate specification completeness and quality before proceeding to planning
**Created**: 2026-03-14
**Feature**: [spec.md](../spec.md)
## Content Quality
- [x] No implementation details (languages, frameworks, APIs)
- [x] Focused on user value and business needs
- [x] Written for non-technical stakeholders
- [x] All mandatory sections completed
## Requirement Completeness
- [x] No [NEEDS CLARIFICATION] markers remain
- [x] Requirements are testable and unambiguous
- [x] Success criteria are measurable
- [x] Success criteria are technology-agnostic (no implementation details)
- [x] All acceptance scenarios are defined
- [x] Edge cases are identified
- [x] Scope is clearly bounded
- [x] Dependencies and assumptions identified
## Feature Readiness
- [x] All functional requirements have clear acceptance criteria
- [x] User scenarios cover primary flows
- [x] Feature meets measurable outcomes defined in Success Criteria
- [x] No implementation details leak into specification
## Notes
- Validation passed on the initial draft. The spec stays scoped to the existing RBAC finding detail consumer, references Spec 141 as a dependency rather than a generic framework rewrite, and introduces no open clarification markers.

View File

@ -0,0 +1,135 @@
openapi: 3.1.0
info:
title: RBAC Finding Diff View Contract
version: 1.0.0
description: >-
Internal presentation contract for the RBAC role definition diff consumer on the Findings
view page. This feature adds no external HTTP endpoints. The contract documents the
authoritative evidence input and the render-ready diff presentation output used by the
consumer.
paths: {}
components:
schemas:
RbacRoleDefinitionEvidence:
type: object
required:
- changed_keys
- baseline
- current
properties:
diff_kind:
type: string
diff_fingerprint:
type: string
changed_keys:
type: array
items:
type: string
metadata_keys:
type: array
items:
type: string
permission_keys:
type: array
items:
type: string
baseline:
$ref: '#/components/schemas/RbacRoleDefinitionSide'
current:
$ref: '#/components/schemas/RbacRoleDefinitionSide'
RbacRoleDefinitionSide:
type: object
properties:
normalized:
type: object
additionalProperties: true
is_built_in:
type:
- boolean
- 'null'
role_permission_count:
type:
- integer
- 'null'
DiffPresentation:
type: object
required:
- summary
- rows
properties:
summary:
$ref: '#/components/schemas/DiffSummary'
rows:
type: array
items:
$ref: '#/components/schemas/DiffRow'
DiffSummary:
type: object
required:
- changedCount
- addedCount
- removedCount
- unchangedCount
- hasRows
properties:
changedCount:
type: integer
minimum: 0
addedCount:
type: integer
minimum: 0
removedCount:
type: integer
minimum: 0
unchangedCount:
type: integer
minimum: 0
hasRows:
type: boolean
message:
type:
- string
- 'null'
DiffRow:
type: object
required:
- key
- label
- status
- isListLike
- addedItems
- removedItems
- unchangedItems
- meta
properties:
key:
type: string
minLength: 1
label:
type: string
minLength: 1
status:
type: string
enum:
- changed
- unchanged
- added
- removed
oldValue:
nullable: true
newValue:
nullable: true
isListLike:
type: boolean
addedItems:
type: array
items: {}
removedItems:
type: array
items: {}
unchangedItems:
type: array
items: {}
meta:
type: object
additionalProperties: true

View File

@ -0,0 +1,137 @@
# Data Model: RBAC Role Definition Diff UX Upgrade
## Overview
This feature introduces no database schema changes. The data model is a presentation-layer model that adapts existing `Finding.evidence_jsonb.rbac_role_definition` content into shared diff presentation objects.
## Existing Input Model
### RbacRoleDefinitionEvidence
- **Source**: `Finding.evidence_jsonb.rbac_role_definition`
- **Purpose**: authoritative comparison payload already produced by the baseline comparison engine
- **Core fields**:
- `diff_kind`: string such as `metadata_only`, `missing`, or `unexpected`
- `diff_fingerprint`: stable change fingerprint for recurrence logic
- `changed_keys`: array of operator-facing diff keys that materially changed
- `metadata_keys`: array of metadata-related changed keys
- `permission_keys`: array of permission-related changed keys
- `baseline`: `RbacRoleDefinitionSide`
- `current`: `RbacRoleDefinitionSide`
### RbacRoleDefinitionSide
- **Source**: `rbac_role_definition.baseline` and `rbac_role_definition.current`
- **Purpose**: stores one side of the RBAC comparison
- **Core fields**:
- `normalized`: map of `field_key => value`
- `is_built_in`: nullable boolean used for role source display fallback
- `role_permission_count`: nullable integer used for permission-block summary fallback
## New Presentation Model
### RbacRoleDefinitionDiffBuilder
- **Type**: new consumer-local adapter class
- **Purpose**: translate `RbacRoleDefinitionEvidence` into shared `DiffPresentation`
- **Responsibilities**:
- merge baseline and current normalized rows
- apply RBAC label mapping
- apply deterministic field ordering
- mark Allowed Actions and other approved list-like fields for inline list rendering
- pass `changed_keys` through to shared state classification
- preserve no-change and sparse-data behavior
### DiffPresentation
- **Type**: existing shared DTO from Spec 141
- **Fields**:
- `summary`: `DiffSummary`
- `rows`: ordered array of `DiffRow`
- **Purpose**: single render-ready result consumed by the RBAC Blade entry
### DiffSummary
- **Type**: existing shared DTO
- **Fields**:
- `changedCount`
- `addedCount`
- `removedCount`
- `unchangedCount`
- `hasRows`
- `message`
- **Purpose**: drive summary badge counts and no-change/no-data messaging
### DiffRow
- **Type**: existing shared DTO
- **Fields**:
- `key`: stable field key
- `label`: operator-facing label
- `status`: `changed | unchanged | added | removed`
- `oldValue`
- `newValue`
- `isListLike`
- `addedItems`
- `removedItems`
- `unchangedItems`
- `meta`
- **Purpose**: represent one renderable field row in the RBAC diff region
## Consumer Configuration Model
### RbacFieldLabelMap
- **Type**: internal associative map in the builder
- **Purpose**: keep operator-facing labels stable and concise when internal keys need normalization or fallback handling
- **Examples**:
- `Role definition > Display name`
- `Role definition > Description`
- `Role definition > Role source`
- `Role definition > Permission blocks`
- `Permission block 1 > Allowed actions`
### RbacFieldOrderRules
- **Type**: internal ordering strategy in the builder
- **Purpose**: preserve predictable review order
- **Desired order**:
1. top-level role metadata
2. role source and permission-block summary
3. permission-block details
4. alphabetical fallback for unknown but valid keys
### RbacListFieldRules
- **Type**: internal field designation rules
- **Purpose**: identify which RBAC fields should intentionally use inline list diff rendering
- **Initial designated fields**:
- any `Permission block * > Allowed actions`
- optionally other simple list-valued RBAC keys that fit the same operator model, such as denied actions or conditions, if present and still readable
## State Derivation Rules
### Row status
- **Added**: key exists only on current side
- **Removed**: key exists only on baseline side
- **Changed**: key exists on both sides and either appears in `changed_keys` or has unequal values
- **Unchanged**: key exists on both sides and values are equal without change designation
### List fragments
- **Added items**: values present only in current list
- **Removed items**: values present only in baseline list
- **Unchanged items**: values present in both lists in shared order-preserving form
## Validation Rules
- Ignore empty or non-string field keys.
- Treat absent `baseline.normalized` and `current.normalized` maps as empty arrays.
- Preserve explicit `null`, boolean, scalar, and list values so `ValueStringifier` can render them consistently.
- Treat unknown metadata as optional and never required for row rendering.
## No Schema Change Statement
- No new tables, columns, indexes, or migrations are required.
- `findings.evidence_jsonb` remains the authoritative source for the RBAC consumer.

View File

@ -0,0 +1,223 @@
# Implementation Plan: RBAC Role Definition Diff UX Upgrade
**Branch**: `142-rbac-role-definition-diff-ux-upgrade` | **Date**: 2026-03-14 | **Spec**: [spec.md](./spec.md)
**Input**: Feature specification from `/specs/142-rbac-role-definition-diff-ux-upgrade/spec.md`
**Note**: This template is filled in by the `/speckit.plan` command. See `.specify/scripts/` for helper scripts.
## Summary
Adopt the shared diff presentation foundation from Spec 141 on the existing RBAC role definition finding detail surface so operators can scan changed rows immediately, treat unchanged rows as quieter context, and understand Allowed Actions through inline add/remove list semantics. The implementation stays presentation-only inside the existing Laravel 12 / Filament v5 / Livewire v4 stack, reuses the current shared `App\Support\Diff` primitives and shared Blade partials, adds a thin RBAC-specific shaping layer at the consumer boundary, preserves the existing Findings resource and route structure, and limits test changes to shared diff support coverage plus the current RBAC finding detail feature test.
## Technical Context
**Language/Version**: PHP 8.4.15 / Laravel 12
**Primary Dependencies**: Filament v5, Livewire v4.0+, Tailwind CSS v4, shared `App\Support\Diff` foundation from Spec 141
**Storage**: PostgreSQL via Laravel Sail for existing `findings.evidence_jsonb`; no schema or persistence changes
**Testing**: Pest v4 feature and unit tests on PHPUnit 12
**Target Platform**: Laravel Sail web application with the Filament admin panel at `/admin` and the existing Findings view page
**Project Type**: Laravel monolith / Filament web application
**Performance Goals**: Keep diff shaping server-side and lightweight, preserve responsive finding detail rendering for representative RBAC payloads, and keep inline list rendering readable for permission lists up to roughly 25 actions without client-side diffing
**Constraints**: No new routes, no Graph calls, no `OperationRun`, no authorization-rule changes, no backend diff-producer rewrite, no unrelated diff-screen migrations, no heavy Alpine/Livewire toggle complexity, and dark-mode-safe accessible semantics are required
**Scale/Scope**: One existing RBAC consumer on the finding detail page, reuse of the shared diff support layer and shared Blade partials, one RBAC-local shaping adapter, and focused regression coverage for view rendering and consumer shaping
## Constitution Check
*GATE: Must pass before Phase 0 research. Re-check after Phase 1 design.*
- Inventory-first: PASS — the feature only changes how existing finding evidence is presented and does not alter inventory, snapshot, or finding ownership.
- Read/write separation: PASS — the feature is presentation-only and adds no mutation flow.
- Graph contract path: PASS — no Microsoft Graph call is introduced.
- Deterministic capabilities: PASS — no capability derivation or registry mapping changes are introduced.
- RBAC-UX planes and isolation: PASS — tenant/admin and platform planes remain unchanged, and the RBAC consumer renders only already-authorized finding evidence.
- Workspace isolation: PASS — no new workspace-scoped data access path is introduced.
- RBAC-UX destructive confirmation: PASS / N/A — no destructive action is added or modified.
- RBAC-UX global search: PASS / N/A — no global-search behavior changes; the Findings resource already has a View page and this feature does not alter searchability.
- Tenant isolation: PASS — the feature remains within the existing authorized finding detail view.
- Run observability and Ops-UX: PASS / N/A — no long-running, remote, queued, or scheduled work is introduced.
- Data minimization: PASS — the consumer uses existing evidence payloads and shared stringification without broadening stored or fetched data.
- Badge semantics (BADGE-001): PASS — diff-state badges continue to use centralized `BadgeCatalog` mapping through the shared diff foundation.
- UI naming (UI-NAMING-001): PASS — no new operator-facing actions, toasts, runs, or audit prose are added; existing descriptive copy stays domain-specific.
- Filament UI Action Surface Contract: PASS — the Findings `View` page remains the same action surface and introduces no new actions or mutations.
- Filament UI UX-001: PASS — the feature upgrades one infolist entry within the existing View page rather than changing page layout structure.
- Filament v5 / Livewire v4 compliance: PASS — the implementation stays inside the current Filament v5 panel and Livewire v4 runtime.
- Provider registration (`bootstrap/providers.php`): PASS / unchanged — no new provider or panel registration is introduced.
- Asset strategy: PASS — no new global or on-demand assets are needed; deployment continues to rely on the existing Filament asset pipeline and standard `php artisan filament:assets` step when assets change generally.
Gate result before research: PASS.
Gate result after Phase 1 design: PASS.
- The chosen design uses existing shared diff primitives rather than inventing new diff semantics in Blade.
- No constitution exceptions or complexity justifications are required.
## Project Structure
### Documentation (this feature)
```text
specs/142-rbac-role-definition-diff-ux-upgrade/
├── plan.md
├── research.md
├── data-model.md
├── quickstart.md
├── contracts/
│ └── rbac-finding-diff-view.openapi.yaml
├── checklists/
│ └── requirements.md
└── tasks.md
```
### Source Code (repository root)
```text
app/
├── Filament/
│ └── Resources/
│ └── FindingResource.php # existing finding detail surface; may host consumer wiring
├── Support/
│ ├── Diff/
│ │ ├── DiffPresenter.php # existing shared presenter from Spec 141
│ │ ├── DiffPresentation.php # existing shared presentation DTO
│ │ ├── DiffRow.php # existing shared row DTO
│ │ ├── DiffSummary.php # existing shared summary DTO
│ │ ├── DiffRowStatus.php # existing shared status enum
│ │ ├── ValueStringifier.php # existing shared value formatter
│ │ └── RbacRoleDefinitionDiffBuilder.php # planned thin RBAC-specific adapter
│ └── Badges/
│ └── Domains/
│ └── DiffRowStatusBadge.php # existing centralized diff badge semantics
resources/
└── views/
└── filament/
├── infolists/
│ └── entries/
│ └── rbac-role-definition-diff.blade.php # existing RBAC consumer to refactor
└── partials/
└── diff/
├── summary-badges.blade.php # existing shared summary partial
├── row.blade.php # existing shared row dispatcher
├── row-changed.blade.php # existing changed-row partial
├── row-added.blade.php # existing added-row partial
├── row-removed.blade.php # existing removed-row partial
├── row-unchanged.blade.php # existing unchanged-row partial
└── inline-list.blade.php # existing inline list partial
tests/
├── Feature/
│ ├── Filament/
│ │ └── FindingViewRbacEvidenceTest.php # existing RBAC finding detail coverage to extend
│ └── Support/
│ └── Diff/
│ ├── SharedDiffRowPartialTest.php # existing shared row partial coverage
│ ├── SharedDiffSummaryPartialTest.php # existing shared summary coverage
│ └── SharedInlineListDiffPartialTest.php # existing inline list coverage
└── Unit/
└── Support/
└── Diff/
├── DiffPresenterTest.php # existing presenter coverage to extend if RBAC-specific shaping is added
└── ValueStringifierTest.php # existing stringifier coverage if formatting assumptions change
```
**Structure Decision**: Keep the implementation inside the existing Laravel/Filament monolith and localize the feature to the Findings consumer plus the shared diff support layer it already depends on. Introduce only a thin RBAC-specific adapter under `app/Support/Diff` to translate `rbac_role_definition` evidence into `DiffPresentation`, refactor the RBAC infolist entry to compose shared partials, and extend existing feature/support tests instead of building a new module or page.
## Complexity Tracking
> No Constitution Check violations. No justifications needed.
| Violation | Why Needed | Simpler Alternative Rejected Because |
|-----------|------------|-------------------------------------|
| — | — | — |
## Phase 0 — Research (DONE)
Output:
- `specs/142-rbac-role-definition-diff-ux-upgrade/research.md`
Key findings captured:
- The shared Spec 141 diff foundation already exists in `app/Support/Diff` and in `resources/views/filament/partials/diff`, so this feature is a consumer-adoption pass, not new framework work.
- The current RBAC renderer in `resources/views/filament/infolists/entries/rbac-role-definition-diff.blade.php` still duplicates summary, formatting, and side-by-side rendering logic that should move to the shared partials.
- `FindingResource` already exposes the RBAC diff region through a View entry on the Findings `view` page, so no route, page, or resource restructuring is needed.
- The existing `FindingViewRbacEvidenceTest` is the right feature-level anchor, while shared presenter and partial tests already exist for lower-level reuse.
- A local “show only changes” toggle is not needed for the first consumer because the specs core UX goals can be met through stronger row emphasis and quieter unchanged rows.
## Phase 1 — Design & Contracts (DONE)
Outputs:
- `specs/142-rbac-role-definition-diff-ux-upgrade/data-model.md`
- `specs/142-rbac-role-definition-diff-ux-upgrade/contracts/rbac-finding-diff-view.openapi.yaml`
- `specs/142-rbac-role-definition-diff-ux-upgrade/quickstart.md`
Design highlights:
- Add a thin `RbacRoleDefinitionDiffBuilder` that converts `rbac_role_definition` finding evidence into shared `DiffPresentation` rows using explicit RBAC field labels, deterministic field order, and explicit list-field treatment for Allowed Actions and similar simple arrays.
- Refactor the existing RBAC Blade entry so it renders shared summary badges and shared row partials instead of custom side-by-side blocks.
- Keep unchanged rows visible but visually quieter by reusing the shared unchanged-row partial with dimmed semantics rather than hiding rows by default.
- Keep one-sided rows explicit by relying on the shared added/removed states instead of treating them as generic changed rows.
- Do not implement the optional local toggle in this first pass unless it is still clearly low-risk during implementation.
## Phase 1 — Agent Context Update (DONE)
Run:
- `.specify/scripts/bash/update-agent-context.sh copilot`
## Phase 2 — Implementation Outline (tasks created in `/speckit.tasks`)
### Step 1 — Add RBAC consumer shaping
Goal: turn the existing `rbac_role_definition` evidence payload into a shared `DiffPresentation` without changing the authoritative comparison engine.
Changes:
- Add a thin RBAC-specific builder under `app/Support/Diff`.
- Normalize baseline/current rows from `baseline.normalized` and `current.normalized`.
- Apply a stable RBAC field label map and explicit row-order rules.
- Mark Allowed Actions and other simple RBAC list fields for inline list rendering.
Tests:
- Add or extend unit coverage for RBAC-specific row shaping, state assignment, and summary counts.
### Step 2 — Refactor the finding detail consumer
Goal: replace the one-off RBAC Blade rendering with shared summary and row partial composition.
Changes:
- Update the existing RBAC infolist entry to consume the shared presentation output.
- Render summary badges through `filament.partials.diff.summary-badges`.
- Render each row through `filament.partials.diff.row`.
- Keep the existing explanatory copy about assignments exclusion and unsupported restore.
Tests:
- Extend the existing finding detail feature test to assert changed-row emphasis, summary/detail coherence, and one-sided row semantics.
### Step 3 — Harden inline list readability
Goal: ensure Allowed Actions and similar list fields are readable as added/removed differences.
Changes:
- Feed shared inline list rendering with RBAC list rows.
- Preserve unchanged list items only in muted form where helpful.
- Keep list rendering accessible and dark-mode-safe.
Tests:
- Add or extend feature and partial tests for added, removed, and unchanged list chips in the RBAC consumer context.
### Step 4 — Validate no-change and sparse states
Goal: make the consumer safe for empty, sparse, and no-change payloads.
Changes:
- Ensure the builder returns empty or no-change summary states consistently.
- Keep layout stable when optional metadata is absent.
Tests:
- Add feature assertions for no-change messaging and sparse payload safety.
### Step 5 — Keep the rollout consumer-local
Goal: prove Spec 141 through RBAC without dragging in other diff screens.
Changes:
- Do not migrate assignments, scope tags, normalized diff, or other compare consumers in this feature.
- Keep any RBAC-specific logic at the consumer boundary rather than expanding shared foundation scope.
Tests:
- No new cross-consumer tests required; existing unrelated diff consumers should remain untouched.

View File

@ -0,0 +1,49 @@
# Quickstart: RBAC Role Definition Diff UX Upgrade
## Goal
Upgrade the existing RBAC role definition diff region on the finding detail page so it becomes the first concrete consumer of the shared diff presentation foundation from Spec 141.
## Implementation Steps
1. Add a small RBAC-specific builder that reads `Finding.evidence_jsonb.rbac_role_definition` and produces `DiffPresentation` using the shared `DiffPresenter`.
2. Move RBAC-specific field-label mapping, field-order rules, and list-field designation into that builder rather than the Blade view.
3. Refactor `resources/views/filament/infolists/entries/rbac-role-definition-diff.blade.php` so it renders:
- the shared summary badge partial
- the shared row partial for each row
- the existing RBAC explanatory copy about assignments exclusion and restore support
4. Keep unchanged rows visible but visually quieter by relying on the shared unchanged-row partial.
5. Render Allowed Actions and any other approved simple list fields through the shared inline list partial.
6. Do not add a local “show only changes” toggle unless the implementation remains clearly low-risk and unnecessary complexity does not appear.
## Verification Targets
Update and run the smallest focused set first:
```bash
vendor/bin/sail artisan test --compact tests/Feature/Filament/FindingViewRbacEvidenceTest.php
vendor/bin/sail artisan test --compact tests/Unit/Support/Diff/DiffPresenterTest.php
vendor/bin/sail artisan test --compact tests/Feature/Support/Diff/SharedDiffRowPartialTest.php
vendor/bin/sail artisan test --compact tests/Feature/Support/Diff/SharedInlineListDiffPartialTest.php
```
Then format:
```bash
vendor/bin/sail bin pint --dirty --format agent
```
## Acceptance Checklist
- Changed rows are visually stronger than unchanged rows.
- Summary badges match the rendered row states.
- Allowed Actions shows added and removed actions as inline list chips.
- Baseline-only and current-only rows render as removed and added, not generic changed rows.
- No-change and sparse payloads render safely.
- No unrelated diff consumer is migrated.
## Notes
- Livewire compliance remains on v4 because the feature stays within the existing Filament v5 surface.
- Provider registration remains unchanged in `bootstrap/providers.php`; this feature introduces no new provider or panel.
- The Findings resource already has a View page, so there is no global-search eligibility issue introduced by this work.

View File

@ -0,0 +1,57 @@
# Research: RBAC Role Definition Diff UX Upgrade
## Decision 1: Reuse the existing shared diff foundation instead of extending it broadly
- **Decision**: Build this feature as a thin RBAC consumer on top of the existing `App\Support\Diff` classes and `resources/views/filament/partials/diff/*` partials introduced by Spec 141.
- **Rationale**: The shared presenter, DTOs, centralized badge mapping, summary partial, row partials, and inline list partial already exist. The current gap is consumer adoption on the RBAC finding detail surface, not missing foundation primitives.
- **Alternatives considered**:
- Extend the shared foundation first with additional generic abstractions. Rejected because it broadens scope without first validating the current foundation on a real surface.
- Keep the RBAC Blade view custom and copy over only visual classes. Rejected because it would duplicate summary, row-state, and value-formatting logic again.
## Decision 2: Add a thin RBAC-specific shaping adapter at the consumer boundary
- **Decision**: Introduce a small RBAC-specific builder that reads `finding.evidence_jsonb.rbac_role_definition`, applies label/order/list-field rules, and outputs `DiffPresentation` for the Blade consumer.
- **Rationale**: The spec explicitly allows consumer-local shaping while keeping the underlying comparison engine authoritative. This avoids pushing RBAC-specific label maps and ordering rules into generic shared classes.
- **Alternatives considered**:
- Compute RBAC-specific view semantics directly in Blade. Rejected because the current one-off view already demonstrates why that becomes hard to maintain and test.
- Change the baseline comparison job payload to emit a render-ready shared shape. Rejected because this spec is presentation-only and must not rewrite the authoritative diff producer.
## Decision 3: Treat Allowed Actions as the primary explicit list-diff field
- **Decision**: Use explicit RBAC field designation to render Allowed Actions, and any other simple list-like RBAC fields that fit the same shape, through the shared inline list partial.
- **Rationale**: Allowed Actions is the clearest operator pain point and already appears as array data in the RBAC evidence payload. Explicit consumer-local designation is safer than inferring every array field automatically.
- **Alternatives considered**:
- Rely only on automatic list detection. Rejected because RBAC presentation needs predictable behavior for operator-facing fields, not just shape-based guesses.
- Keep side-by-side text blocks for permission fields. Rejected because that preserves the main usability problem this spec exists to fix.
## Decision 4: Prefer deterministic logical row order over interactive sorting
- **Decision**: Render RBAC rows in a stable, review-friendly order using an explicit consumer ordering strategy with alphabetical fallback.
- **Rationale**: The spec prioritizes predictable review readability over advanced interactive sorting. Operators should see core role-definition metadata first and permission-block details after that.
- **Alternatives considered**:
- Pure alphabetical ordering from the shared presenter only. Rejected because it is deterministic but not ideal for operator review.
- User-driven sorting or grouping controls. Rejected because it adds UI complexity that is out of scope for this consumer hardening pass.
## Decision 5: Do not ship the optional “show only changes” toggle in the first pass
- **Decision**: Plan the implementation without a local toggle unless it proves trivial during implementation.
- **Rationale**: The spec makes the toggle optional and explicitly says the default view must already be understandable. The shared changed/unchanged row styling already provides the core value with less interaction risk.
- **Alternatives considered**:
- Make the toggle part of the required implementation. Rejected because it adds Livewire/Alpine behavior without being necessary to satisfy the primary goals.
- Collapse unchanged rows by default. Rejected because the product decision for this spec favors quieter audit context rather than aggressive hiding.
## Decision 6: Use existing feature and support tests as the regression backbone
- **Decision**: Extend `tests/Feature/Filament/FindingViewRbacEvidenceTest.php` for consumer behavior and only add focused unit/support coverage where the RBAC-specific builder or shared rendering needs it.
- **Rationale**: The repo already has strong shared diff tests for presenter logic, value stringification, badges, and shared partials. The missing confidence is at the RBAC consumer integration layer.
- **Alternatives considered**:
- Add only feature tests. Rejected because any new RBAC builder would benefit from targeted unit coverage for ordering and field designation.
- Add browser tests. Rejected because the surface can be validated well through existing Pest feature/view tests for this scope.
## Decision 7: Keep deployment and asset implications unchanged
- **Decision**: Use existing Blade partials and Tailwind/Filament styling without adding new asset bundles or on-demand asset registration.
- **Rationale**: The feature is a server-rendered Blade composition change within an existing Filament page. No new JS or CSS distribution mechanism is necessary.
- **Alternatives considered**:
- Add consumer-specific JS for list diffing or filters. Rejected because the spec explicitly avoids heavy client-side diffing and over-building advanced controls.
- Add dedicated panel assets for the RBAC diff region. Rejected because the shared partials and existing Filament/Tailwind classes are sufficient.

View File

@ -0,0 +1,126 @@
# Feature Specification: RBAC Role Definition Diff UX Upgrade
**Feature Branch**: `142-rbac-role-definition-diff-ux-upgrade`
**Created**: 2026-03-14
**Status**: Draft
**Input**: User description: "Upgrade the RBAC role definition diff experience on the finding detail screen by adopting the shared diff presentation foundation from Spec 141 as its first concrete consumer. Make changed rows immediately obvious, de-emphasize unchanged rows, render Allowed Actions as meaningful add/remove inline diffs, keep the implementation tightly scoped to the RBAC consumer, and do not migrate unrelated diff screens."
## Spec Scope Fields *(mandatory)*
- **Scope**: workspace
- **Primary Routes**: Existing Findings resource detail surface for RBAC role definition findings, including the current finding view page under the admin panel; no new routes are introduced.
- **Data Ownership**: Existing `Finding` records and their RBAC evidence remain authoritative. This feature only changes how already-available RBAC comparison evidence is presented on the finding detail screen.
- **RBAC**: Existing finding detail membership and capability checks remain unchanged. The feature must only render data already available to authorized viewers and must not widen access.
## User Scenarios & Testing *(mandatory)*
### User Story 1 - Spot Real RBAC Changes Fast (Priority: P1)
An operator reviewing an RBAC role definition finding can immediately see which fields changed without manually comparing every baseline and current value.
**Why this priority**: Faster triage is the primary business value. If the changed field is not immediately obvious, the screen remains operationally slow even if the underlying data is correct.
**Independent Test**: Can be fully tested by rendering an RBAC finding with a small number of changed fields and many unchanged fields, then verifying that the changed fields are explicitly emphasized and the summary counts align with what is shown.
**Acceptance Scenarios**:
1. **Given** an RBAC finding where only a small subset of fields changed, **When** the operator opens the finding detail page, **Then** the changed rows are immediately distinguishable from unchanged rows.
2. **Given** an RBAC finding with changed, added, and removed fields, **When** the operator reviews the diff region, **Then** each row clearly communicates its change state without relying on color alone.
---
### User Story 2 - Understand Permission List Differences (Priority: P2)
An operator reviewing Allowed Actions can tell which permissions were added or removed without reading large before-and-after text blobs.
**Why this priority**: Permission-list changes are the highest-friction part of RBAC review today and create the most avoidable operator effort.
**Independent Test**: Can be tested by rendering representative Allowed Actions differences and confirming that added and removed actions appear as meaningful inline differences rather than as broad side-by-side value blocks.
**Acceptance Scenarios**:
1. **Given** an RBAC finding where Allowed Actions differs between baseline and current, **When** the diff region renders, **Then** added actions and removed actions are shown with explicit list-difference semantics.
2. **Given** an RBAC finding where some actions are unchanged, **When** the permission list is rendered, **Then** unchanged actions remain readable without visually overwhelming the added and removed actions.
---
### User Story 3 - Reuse the Shared Diff Language Safely (Priority: P3)
A product team can treat RBAC as the first real consumer of the shared diff presentation foundation from Spec 141 without forcing unrelated compare screens to migrate at the same time.
**Why this priority**: The feature should validate the shared foundation in a real operator workflow while staying narrow enough to avoid accidental platform-wide churn.
**Independent Test**: Can be tested by confirming that the RBAC finding detail surface adopts the shared presentation language and that unrelated diff screens remain unchanged after delivery.
**Acceptance Scenarios**:
1. **Given** the shared diff presentation foundation from Spec 141 exists, **When** the RBAC finding detail screen is upgraded, **Then** it uses the shared change vocabulary and summary/detail semantics.
2. **Given** another diff surface outside RBAC is not part of this feature, **When** this feature is released, **Then** that unrelated surface remains unchanged.
### Edge Cases
- What happens when an RBAC finding contains no meaningful differences? The summary and detail region must clearly communicate that no changes were detected and must not imply missing data is a change.
- What happens when a field exists only in baseline or only in current evidence? The screen must distinguish one-sided fields from ordinary changed rows and present them as explicit added or removed values.
- What happens when Allowed Actions or another list-like value is empty, sparse, or medium length? The screen must remain readable and must not collapse into ambiguous empty or noisy text states.
- What happens when optional RBAC metadata is absent from the evidence payload? The diff region must remain stable, legible, and safe to render.
## Requirements *(mandatory)*
This feature is presentation-only. It introduces no Microsoft Graph calls, no write workflow, no queued or scheduled work, and no `OperationRun` usage.
This feature does not change authorization rules. Existing server-side finding access controls remain authoritative, and the upgraded diff region must only render evidence already loaded for the authorized finding detail page.
This feature modifies an existing Filament view surface within the Findings resource. The Action Surface Contract remains satisfied because no new actions, destructive flows, or audit-producing mutations are introduced. UX-001 remains satisfied because the finding detail page continues to use the existing View page and infolist structure; this feature only upgrades one RBAC diff region inside that layout.
This feature changes diff-state badges inside the RBAC finding detail region. Badge semantics must remain centralized through the shared change-state vocabulary from Spec 141 rather than ad hoc local meanings, and tests must verify that summary badges match rendered row states.
### Functional Requirements
- **FR-001**: The RBAC role definition diff region on the finding detail screen MUST adopt the shared change-state vocabulary from Spec 141: changed, unchanged, added, and removed.
- **FR-002**: The RBAC diff region MUST use the shared diff presentation language for both summary badges and row-level detail so the same change states mean the same thing in both places.
- **FR-003**: Changed rows MUST receive stronger visual emphasis than unchanged rows, and that emphasis MUST be understandable without relying on color alone.
- **FR-004**: Unchanged rows MUST remain available for audit context but MUST be presented in a quieter way so they do not dominate the screen.
- **FR-005**: Summary badges MUST accurately reflect the states of the rows rendered in the RBAC diff region.
- **FR-006**: If the summary indicates only a small number of changed fields, the detailed rows MUST make those changed fields easy to identify at first read.
- **FR-007**: Allowed Actions MUST render as meaningful list differences that explicitly show added and removed actions instead of only broad before-and-after value blocks.
- **FR-008**: The RBAC diff region MAY show unchanged actions in a muted form when that improves readability, but unchanged actions MUST NOT overshadow added or removed actions.
- **FR-009**: Fields that exist only in baseline evidence MUST render as removed, and fields that exist only in current evidence MUST render as added.
- **FR-010**: One-sided rows MUST be visually and semantically distinct from ordinary changed rows.
- **FR-011**: Null values, boolean values, simple scalar values, and list-like values MUST follow the shared value-display behavior established by Spec 141 so RBAC does not introduce ad hoc formatting drift.
- **FR-012**: The RBAC consumer MUST preserve a stable, deterministic row order that supports operator review instead of arbitrary noise.
- **FR-013**: The upgraded diff region MUST remain legible in both light and dark mode.
- **FR-014**: The RBAC consumer MUST render safely when there are no changes, when values are empty, and when optional RBAC metadata is sparse or missing.
- **FR-015**: Any optional local “show only changes” behavior is allowed only if it remains local to the RBAC consumer, is not required for core comprehension, and does not add disproportionate interaction fragility.
- **FR-016**: This feature MUST remain limited to the RBAC role definition diff region on the finding detail screen and MUST NOT migrate unrelated diff screens.
- **FR-017**: The feature MUST establish RBAC as the first validated consumer of the shared diff presentation foundation from Spec 141.
- **FR-018**: Developer-facing guidance for this feature MUST identify RBAC as the first consumer, note which RBAC fields are treated as list-like, and explain any deliberate consumer-local choices that future adopters should understand.
## 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 |
|---|---|---|---|---|---|---|---|---|---|---|
| Findings resource view page | `app/Filament/Resources/FindingResource.php` and the existing RBAC diff entry view on the finding detail page | Unchanged | Existing record view from the Findings list | None introduced | None | None introduced | Existing view-page header actions remain unchanged | Not applicable | No | Presentation-only upgrade inside an existing infolist entry. No new actions, mutations, confirmations, or authorization surfaces are added. |
### Key Entities *(include if feature involves data)*
- **RBAC Diff Row**: One operator-facing field in the role definition comparison, including its label, its current change state, and the values needed to explain the difference.
- **RBAC Diff Summary**: The compact set of counts and labels that tells the operator how many rows are changed, unchanged, added, or removed.
- **List Difference**: The operator-facing explanation of list-like RBAC fields, especially Allowed Actions, showing which items were added, removed, or retained.
- **RBAC Comparison Evidence**: The existing finding evidence payload that already contains the baseline and current RBAC values used by the finding detail screen.
## Assumptions
- The existing RBAC comparison evidence already contains enough structured baseline, current, and changed-field information to support a presentation upgrade without changing the authoritative diff producer.
- Allowed Actions is the primary list-like field that needs meaningful add/remove rendering in this feature, while any other simple list-like RBAC fields may adopt the same presentation if they fit cleanly.
- Existing finding detail authorization, record loading, tenant isolation, and routing remain unchanged.
- Spec 141 remains the source of truth for the shared diff vocabulary and shared presentation behavior used by this consumer.
## Success Criteria *(mandatory)*
### Measurable Outcomes
- **SC-001**: In acceptance review with representative RBAC findings, 100% of rendered RBAC diff rows are classified using the shared change vocabulary of changed, unchanged, added, or removed.
- **SC-002**: In representative RBAC findings that contain mixed row states, summary badge counts match the rendered detailed rows with zero mismatches.
- **SC-003**: In representative RBAC findings where only 1 to 5 fields differ, the changed fields are identifiable at first read without requiring operators to inspect every unchanged row manually.
- **SC-004**: In representative Allowed Actions comparisons containing up to 25 actions, operators can identify added and removed permissions directly from the rendered list-difference view without switching to raw evidence.
- **SC-005**: Representative no-change and sparse-payload RBAC findings render a stable explanatory state with no broken layout, ambiguous empty values, or misleading change labels.

View File

@ -0,0 +1,190 @@
# Tasks: RBAC Role Definition Diff UX Upgrade
**Input**: Design documents from `/specs/142-rbac-role-definition-diff-ux-upgrade/`
**Prerequisites**: plan.md, spec.md, research.md, data-model.md, contracts/rbac-finding-diff-view.openapi.yaml, quickstart.md
**Tests**: Tests are REQUIRED for this feature because it changes runtime presentation behavior on the Findings detail screen and adds a new RBAC-specific shaping adapter.
**Operations**: No new `OperationRun`, queued workflow, remote call, or audit-log mutation flow is introduced.
**RBAC**: Existing workspace and tenant authorization remain unchanged. The RBAC diff consumer must render only already-authorized finding evidence and must not add any new scope behavior.
**Filament UI**: This feature modifies an existing Findings `View` infolist entry only. No new header, row, bulk, or destructive actions are introduced.
**Badges**: Diff-state badges must continue to use centralized semantics through `BadgeCatalog` and the shared Spec 141 diff foundation.
## Phase 1: Setup (Shared Infrastructure)
**Purpose**: Prepare the feature-specific test anchors and implementation file stubs for RBAC consumer adoption.
- [X] T001 Create the RBAC builder test scaffold in /Users/ahmeddarrazi/Documents/projects/TenantAtlas/tests/Unit/Support/Diff/RbacRoleDefinitionDiffBuilderTest.php
- [X] T002 [P] Expand the RBAC finding detail test fixture coverage scaffold in /Users/ahmeddarrazi/Documents/projects/TenantAtlas/tests/Feature/Filament/FindingViewRbacEvidenceTest.php
---
## Phase 2: Foundational (Blocking Prerequisites)
**Purpose**: Establish the RBAC-specific shaping adapter that every user story depends on.
**⚠️ CRITICAL**: No user story work should begin until this phase is complete.
- [X] T003 Create the RBAC consumer adapter skeleton in /Users/ahmeddarrazi/Documents/projects/TenantAtlas/app/Support/Diff/RbacRoleDefinitionDiffBuilder.php
- [X] T004 [P] Add shared RBAC payload fixture helpers for the new adapter in /Users/ahmeddarrazi/Documents/projects/TenantAtlas/tests/Unit/Support/Diff/RbacRoleDefinitionDiffBuilderTest.php and /Users/ahmeddarrazi/Documents/projects/TenantAtlas/tests/Feature/Filament/FindingViewRbacEvidenceTest.php
**Checkpoint**: RBAC-specific shaping infrastructure exists and user-story work can begin.
---
## Phase 3: User Story 1 - Spot Real RBAC Changes Fast (Priority: P1) 🎯 MVP
**Goal**: Make changed RBAC rows immediately obvious, keep unchanged rows quieter, and align the summary badges with the detailed rows.
**Independent Test**: Open a representative RBAC finding detail page and verify that changed, unchanged, added, and removed rows are visually distinct, summary counts match the rendered rows, and unchanged context no longer dominates the screen.
### Tests for User Story 1
- [X] T005 [P] [US1] Add row-state, summary-count, and deterministic-order coverage in /Users/ahmeddarrazi/Documents/projects/TenantAtlas/tests/Unit/Support/Diff/RbacRoleDefinitionDiffBuilderTest.php
- [X] T006 [P] [US1] Extend changed-versus-unchanged rendering assertions in /Users/ahmeddarrazi/Documents/projects/TenantAtlas/tests/Feature/Filament/FindingViewRbacEvidenceTest.php
- [X] T007 [P] [US1] Extend shared changed and unchanged row partial coverage in /Users/ahmeddarrazi/Documents/projects/TenantAtlas/tests/Feature/Support/Diff/SharedDiffRowPartialTest.php
- [X] T008 [P] [US1] Extend summary-badge coherence coverage in /Users/ahmeddarrazi/Documents/projects/TenantAtlas/tests/Feature/Support/Diff/SharedDiffSummaryPartialTest.php and /Users/ahmeddarrazi/Documents/projects/TenantAtlas/tests/Feature/Filament/FindingViewRbacEvidenceTest.php
### Implementation for User Story 1
- [X] T009 [US1] Implement baseline/current normalization, field-label mapping, and stable field ordering in /Users/ahmeddarrazi/Documents/projects/TenantAtlas/app/Support/Diff/RbacRoleDefinitionDiffBuilder.php
- [X] T010 [US1] Refactor the RBAC infolist entry to render shared summary badges and shared row partials in /Users/ahmeddarrazi/Documents/projects/TenantAtlas/resources/views/filament/infolists/entries/rbac-role-definition-diff.blade.php
- [X] T011 [US1] Tighten changed-row emphasis and unchanged-row de-emphasis in /Users/ahmeddarrazi/Documents/projects/TenantAtlas/resources/views/filament/partials/diff/row-changed.blade.php and /Users/ahmeddarrazi/Documents/projects/TenantAtlas/resources/views/filament/partials/diff/row-unchanged.blade.php
**Checkpoint**: The RBAC finding detail screen makes material changes obvious and summary-to-detail coherence is in place.
---
## Phase 4: User Story 2 - Understand Permission List Differences (Priority: P2)
**Goal**: Render Allowed Actions and similar RBAC list fields as meaningful add/remove diffs instead of broad text blobs.
**Independent Test**: Open an RBAC finding whose Allowed Actions changed and verify that added, removed, and optionally unchanged actions are readable as inline list chips without raw side-by-side blob comparison.
### Tests for User Story 2
- [X] T012 [P] [US2] Add Allowed Actions added, removed, and unchanged rendering assertions in /Users/ahmeddarrazi/Documents/projects/TenantAtlas/tests/Feature/Filament/FindingViewRbacEvidenceTest.php
- [X] T013 [P] [US2] Extend RBAC-style inline list partial coverage in /Users/ahmeddarrazi/Documents/projects/TenantAtlas/tests/Feature/Support/Diff/SharedInlineListDiffPartialTest.php
- [X] T014 [P] [US2] Add explicit RBAC value-formatting expectations for null, boolean, scalar, and empty-list cases in /Users/ahmeddarrazi/Documents/projects/TenantAtlas/tests/Unit/Support/Diff/ValueStringifierTest.php and /Users/ahmeddarrazi/Documents/projects/TenantAtlas/tests/Unit/Support/Diff/RbacRoleDefinitionDiffBuilderTest.php
### Implementation for User Story 2
- [X] T015 [US2] Mark Allowed Actions and approved simple RBAC list fields for inline list rendering in /Users/ahmeddarrazi/Documents/projects/TenantAtlas/app/Support/Diff/RbacRoleDefinitionDiffBuilder.php
- [X] T016 [US2] Route RBAC list-like rows through the shared inline diff partial in /Users/ahmeddarrazi/Documents/projects/TenantAtlas/resources/views/filament/infolists/entries/rbac-role-definition-diff.blade.php
**Checkpoint**: Allowed Actions renders as an inline add/remove diff and permission review is materially easier.
---
## Phase 5: User Story 3 - Reuse the Shared Diff Language Safely (Priority: P3)
**Goal**: Validate RBAC as the first consumer of Spec 141 while keeping no-change states safe and unrelated diff consumers untouched.
**Independent Test**: Render sparse and no-change RBAC findings and verify stable empty-state messaging, explicit one-sided semantics, and no regressions in the shared adoption guidance.
### Tests for User Story 3
- [X] T017 [P] [US3] Add no-change, sparse-payload, and one-sided row assertions in /Users/ahmeddarrazi/Documents/projects/TenantAtlas/tests/Feature/Filament/FindingViewRbacEvidenceTest.php
- [X] T018 [P] [US3] Add empty-input and missing-metadata fallback coverage in /Users/ahmeddarrazi/Documents/projects/TenantAtlas/tests/Unit/Support/Diff/RbacRoleDefinitionDiffBuilderTest.php
### Implementation for User Story 3
- [X] T019 [US3] Finalize added-only, removed-only, and no-data summary behavior in /Users/ahmeddarrazi/Documents/projects/TenantAtlas/app/Support/Diff/RbacRoleDefinitionDiffBuilder.php and /Users/ahmeddarrazi/Documents/projects/TenantAtlas/resources/views/filament/infolists/entries/rbac-role-definition-diff.blade.php
- [X] T020 [US3] Update shared consumer guidance to document RBAC as the first adopter, its list-field conventions, the no-toggle first-pass decision, and any remaining consumer-local rendering choices in /Users/ahmeddarrazi/Documents/projects/TenantAtlas/docs/ui/shared-diff-presentation-foundation.md
**Checkpoint**: RBAC is a documented first consumer of the shared diff foundation and renders safe no-change and sparse states.
---
## Phase 6: Polish & Cross-Cutting Concerns
**Purpose**: Run focused verification, formatting, and quickstart validation across the completed feature.
- [X] T021 [P] Run the focused RBAC diff test pack covering /Users/ahmeddarrazi/Documents/projects/TenantAtlas/tests/Unit/Support/Diff/RbacRoleDefinitionDiffBuilderTest.php, /Users/ahmeddarrazi/Documents/projects/TenantAtlas/tests/Unit/Support/Diff/ValueStringifierTest.php, /Users/ahmeddarrazi/Documents/projects/TenantAtlas/tests/Feature/Filament/FindingViewRbacEvidenceTest.php, /Users/ahmeddarrazi/Documents/projects/TenantAtlas/tests/Feature/Support/Diff/SharedDiffSummaryPartialTest.php, /Users/ahmeddarrazi/Documents/projects/TenantAtlas/tests/Feature/Support/Diff/SharedDiffRowPartialTest.php, and /Users/ahmeddarrazi/Documents/projects/TenantAtlas/tests/Feature/Support/Diff/SharedInlineListDiffPartialTest.php
- [X] T022 [P] Run formatting for changes in /Users/ahmeddarrazi/Documents/projects/TenantAtlas/app/Support/Diff/RbacRoleDefinitionDiffBuilder.php, /Users/ahmeddarrazi/Documents/projects/TenantAtlas/resources/views/filament/infolists/entries/rbac-role-definition-diff.blade.php, /Users/ahmeddarrazi/Documents/projects/TenantAtlas/resources/views/filament/partials/diff/row-changed.blade.php, /Users/ahmeddarrazi/Documents/projects/TenantAtlas/resources/views/filament/partials/diff/row-unchanged.blade.php, /Users/ahmeddarrazi/Documents/projects/TenantAtlas/tests/Unit/Support/Diff/RbacRoleDefinitionDiffBuilderTest.php, /Users/ahmeddarrazi/Documents/projects/TenantAtlas/tests/Unit/Support/Diff/ValueStringifierTest.php, /Users/ahmeddarrazi/Documents/projects/TenantAtlas/tests/Feature/Filament/FindingViewRbacEvidenceTest.php, and /Users/ahmeddarrazi/Documents/projects/TenantAtlas/docs/ui/shared-diff-presentation-foundation.md with `vendor/bin/sail bin pint --dirty --format agent`
- [X] T023 [P] Validate the implementation against /Users/ahmeddarrazi/Documents/projects/TenantAtlas/specs/142-rbac-role-definition-diff-ux-upgrade/quickstart.md and confirm /Users/ahmeddarrazi/Documents/projects/TenantAtlas/resources/views/filament/infolists/entries/assignments-diff.blade.php, /Users/ahmeddarrazi/Documents/projects/TenantAtlas/resources/views/filament/infolists/entries/scope-tags-diff.blade.php, and /Users/ahmeddarrazi/Documents/projects/TenantAtlas/resources/views/filament/infolists/entries/normalized-diff.blade.php remain outside scope
---
## Dependencies & Execution Order
### Phase Dependencies
- **Setup (Phase 1)**: No dependencies; can start immediately.
- **Foundational (Phase 2)**: Depends on Setup completion; blocks all user stories.
- **User Story 1 (Phase 3)**: Depends on Foundational completion and is the MVP slice.
- **User Story 2 (Phase 4)**: Depends on User Story 1 because list rendering builds on the adopted shared row and summary flow.
- **User Story 3 (Phase 5)**: Depends on User Stories 1 and 2 because fallback behavior and guidance should reflect the final consumer behavior.
- **Polish (Phase 6)**: Depends on all desired user stories being complete.
### User Story Dependencies
- **User Story 1 (P1)**: No dependency on other stories after Foundational.
- **User Story 2 (P2)**: Builds on the RBAC consumer and shared partial adoption completed in US1.
- **User Story 3 (P3)**: Builds on the consumer behavior completed in US1 and US2.
### Within Each User Story
- Tests should be written or updated first and observed failing before implementation is finalized.
- Builder logic should be completed before final Blade rendering changes.
- Consumer documentation should follow the implemented behavior, not precede it.
### Parallel Opportunities
- `T002` can run in parallel with `T001`.
- `T004` can run in parallel with `T003` once the file exists.
- `T005`, `T006`, `T007`, and `T008` can run in parallel inside US1.
- `T012`, `T013`, and `T014` can run in parallel inside US2.
- `T017` and `T018` can run in parallel inside US3.
- `T021`, `T022`, and `T023` can run in parallel in the polish phase.
---
## Parallel Example: User Story 1
```bash
# Launch the US1 test updates together:
Task: "Add row-state, summary-count, and deterministic-order coverage in tests/Unit/Support/Diff/RbacRoleDefinitionDiffBuilderTest.php"
Task: "Extend changed-versus-unchanged rendering assertions in tests/Feature/Filament/FindingViewRbacEvidenceTest.php"
Task: "Extend shared changed and unchanged row partial coverage in tests/Feature/Support/Diff/SharedDiffRowPartialTest.php"
Task: "Extend summary-badge coherence coverage in tests/Feature/Support/Diff/SharedDiffSummaryPartialTest.php and tests/Feature/Filament/FindingViewRbacEvidenceTest.php"
```
## Parallel Example: User Story 2
```bash
# Launch the US2 verification work together:
Task: "Add Allowed Actions added, removed, and unchanged rendering assertions in tests/Feature/Filament/FindingViewRbacEvidenceTest.php"
Task: "Extend RBAC-style inline list partial coverage in tests/Feature/Support/Diff/SharedInlineListDiffPartialTest.php"
Task: "Add explicit RBAC value-formatting expectations for null, boolean, scalar, and empty-list cases in tests/Unit/Support/Diff/ValueStringifierTest.php and tests/Unit/Support/Diff/RbacRoleDefinitionDiffBuilderTest.php"
```
## Parallel Example: User Story 3
```bash
# Launch the US3 fallback-safety work together:
Task: "Add no-change, sparse-payload, and one-sided row assertions in tests/Feature/Filament/FindingViewRbacEvidenceTest.php"
Task: "Add empty-input and missing-metadata fallback coverage in tests/Unit/Support/Diff/RbacRoleDefinitionDiffBuilderTest.php"
```
---
## Implementation Strategy
### MVP First (User Story 1 Only)
1. Complete Phase 1: Setup.
2. Complete Phase 2: Foundational.
3. Complete Phase 3: User Story 1.
4. Validate the RBAC finding detail screen independently before moving on.
### Incremental Delivery
1. Deliver US1 to establish RBAC as a working consumer of the shared diff foundation.
2. Deliver US2 to make permission-list diffs meaningfully readable.
3. Deliver US3 to harden empty and sparse states and document RBAC adoption boundaries.
### Parallel Team Strategy
1. One engineer can own the builder and unit tests while another extends the feature and shared partial tests.
2. After US1 lands, one engineer can focus on RBAC list-field shaping while another validates shared inline list behavior.
3. Finish with focused Sail tests, Pint, and quickstart validation.

View File

@ -263,6 +263,10 @@ function getPlacementEmptyStateAction(Testable $component, string $name): ?Actio
$component = Livewire::test(ListTenants::class)
->assertTableEmptyStateActionsExistInOrder(['add_tenant']);
$emptyStateCreate = getPlacementEmptyStateAction($component, 'add_tenant');
expect($emptyStateCreate)->not->toBeNull();
expect($emptyStateCreate?->getLabel())->toBe('Add tenant');
$headerCreate = getHeaderAction($component, 'add_tenant');
expect($headerCreate)->not->toBeNull();
expect($headerCreate?->isVisible())->toBeFalse();
@ -279,6 +283,103 @@ function getPlacementEmptyStateAction(Testable $component, string $name): ?Actio
$headerCreate = getHeaderAction($component, 'add_tenant');
expect($headerCreate)->not->toBeNull();
expect($headerCreate?->isVisible())->toBeTrue();
expect($headerCreate?->getLabel())->toBe('Add tenant');
});
it('labels the empty-state tenant action as continue onboarding when one draft exists', function (): void {
$workspace = Workspace::factory()->create([
'archived_at' => now(),
]);
$user = User::factory()->create();
WorkspaceMembership::factory()->create([
'workspace_id' => (int) $workspace->getKey(),
'user_id' => (int) $user->getKey(),
'role' => 'owner',
]);
createOnboardingDraft([
'workspace' => $workspace,
'started_by' => $user,
'updated_by' => $user,
'state' => [
'entra_tenant_id' => 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa',
'tenant_name' => 'Onboarding Tenant',
],
]);
$user->forceFill([
'last_workspace_id' => (int) $workspace->getKey(),
])->save();
$this->actingAs($user)
->withSession([WorkspaceContext::SESSION_KEY => (int) $workspace->getKey()]);
$component = Livewire::test(ListTenants::class)
->assertTableEmptyStateActionsExistInOrder(['add_tenant']);
$emptyStateCreate = getPlacementEmptyStateAction($component, 'add_tenant');
expect($emptyStateCreate)->not->toBeNull();
expect($emptyStateCreate?->getLabel())->toBe('Continue onboarding');
});
it('labels the tenant header action as continue onboarding when one draft exists', function (): void {
[$user, $tenant] = createUserWithTenant(role: 'owner');
createOnboardingDraft([
'workspace' => $tenant->workspace,
'started_by' => $user,
'updated_by' => $user,
'state' => [
'entra_tenant_id' => 'bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb',
'tenant_name' => 'Resumable Draft',
],
]);
$this->actingAs($user);
$component = Livewire::test(ListTenants::class)
->assertCountTableRecords(1);
$headerCreate = getHeaderAction($component, 'add_tenant');
expect($headerCreate)->not->toBeNull();
expect($headerCreate?->isVisible())->toBeTrue();
expect($headerCreate?->getLabel())->toBe('Continue onboarding');
});
it('labels the tenant header action as choose onboarding draft when multiple drafts exist', function (): void {
[$user, $tenant] = createUserWithTenant(role: 'owner');
createOnboardingDraft([
'workspace' => $tenant->workspace,
'started_by' => $user,
'updated_by' => $user,
'state' => [
'entra_tenant_id' => 'cccccccc-cccc-cccc-cccc-cccccccccccc',
'tenant_name' => 'Draft One',
],
]);
createOnboardingDraft([
'workspace' => $tenant->workspace,
'started_by' => $user,
'updated_by' => $user,
'state' => [
'entra_tenant_id' => 'dddddddd-dddd-dddd-dddd-dddddddddddd',
'tenant_name' => 'Draft Two',
],
]);
$this->actingAs($user);
$component = Livewire::test(ListTenants::class)
->assertCountTableRecords(1);
$headerCreate = getHeaderAction($component, 'add_tenant');
expect($headerCreate)->not->toBeNull();
expect($headerCreate?->isVisible())->toBeTrue();
expect($headerCreate?->getLabel())->toBe('Choose onboarding draft');
});
it('shows create only in empty state when provider connections table is empty', function (): void {

View File

@ -10,87 +10,113 @@
use Filament\Facades\Filament;
use Livewire\Livewire;
it('renders readable role definition evidence and no-restore messaging on the finding detail page', function (): void {
it('renders shared summary badges and RBAC row states on the finding detail page', function (): void {
[$user, $tenant] = createUserWithTenant(role: 'owner');
$subjectExternalId = BaselineSubjectKey::workspaceSafeSubjectExternalIdForPolicy(
'intuneRoleDefinition',
'Security Reader',
'rbac-role-1',
);
$finding = Finding::factory()->create([
'tenant_id' => (int) $tenant->getKey(),
'finding_type' => Finding::FINDING_TYPE_DRIFT,
'source' => 'baseline.compare',
'subject_type' => 'policy',
'subject_external_id' => (string) $subjectExternalId,
'evidence_fidelity' => 'content',
'severity' => Finding::SEVERITY_LOW,
'evidence_jsonb' => [
'change_type' => 'different_version',
'policy_type' => 'intuneRoleDefinition',
'subject_key' => hash('sha256', 'intuneRoleDefinition|rbac-role-1'),
'display_name' => 'Security Reader',
'summary' => [
'kind' => 'rbac_role_definition',
],
'baseline' => [
'policy_version_id' => 10,
'hash' => 'baseline',
],
'current' => [
'policy_version_id' => 11,
'hash' => 'current',
],
'rbac_role_definition' => [
'diff_kind' => 'metadata_only',
'diff_fingerprint' => 'rbac-diff-1',
'changed_keys' => ['Role definition > Description'],
'metadata_keys' => ['Role definition > Description'],
'permission_keys' => [],
'baseline' => [
'normalized' => [
'Role definition > Display name' => 'Security Reader',
'Role definition > Description' => 'Baseline description',
'Role definition > Role source' => 'Custom',
],
'is_built_in' => false,
'role_permission_count' => 1,
],
'current' => [
'normalized' => [
'Role definition > Display name' => 'Security Reader',
'Role definition > Description' => 'Updated description',
'Role definition > Role source' => 'Custom',
],
'is_built_in' => false,
'role_permission_count' => 1,
],
],
'fidelity' => 'content',
'provenance' => [
'baseline_profile_id' => 1,
'baseline_snapshot_id' => 1,
'compare_operation_run_id' => 1,
'inventory_sync_run_id' => 1,
],
],
]);
$finding = findingViewRbacFinding($tenant);
$this->actingAs($user)
->get(FindingResource::getUrl('view', ['record' => $finding], tenant: $tenant))
->assertOk()
->assertSee('Intune RBAC Role Definition drift')
->assertSee('Metadata-only change')
->assertSee('Changed fields')
->assertSee('2 changed')
->assertSee('1 added')
->assertSee('1 removed')
->assertSee('4 unchanged')
->assertSee('Changed')
->assertSee('Added')
->assertSee('Removed')
->assertSee('Unchanged')
->assertSee('Changed value')
->assertSee('No material change')
->assertSee('Role definition > Description')
->assertSee('Permission block 1 > Denied actions')
->assertSee('Permission block 1 > Conditions')
->assertSee('Baseline description')
->assertSee('Updated description')
->assertSee('border-warning-200')
->assertSee('text-gray-500')
->assertSee('Role Assignments are not included')
->assertSee('RBAC restore is not supported');
});
it('renders Allowed Actions as inline added removed and unchanged list chips', function (): void {
[$user, $tenant] = createUserWithTenant(role: 'owner');
$finding = findingViewRbacFinding($tenant);
$this->actingAs($user)
->get(FindingResource::getUrl('view', ['record' => $finding], tenant: $tenant))
->assertOk()
->assertSee('Permission block 1 > Allowed actions')
->assertSee('Added items')
->assertSee('Removed items')
->assertSee('Unchanged items')
->assertSee('Microsoft.Intune/deviceConfigurations/create')
->assertSee('Microsoft.Intune/deviceConfigurations/delete')
->assertSee('Microsoft.Intune/deviceConfigurations/read')
->assertSee('@Resource[Microsoft.Intune/deviceConfigurations] Exists')
->assertSee('Microsoft.Intune/deviceConfigurations/wipe');
});
it('renders a no-change RBAC summary when the evidence only contains unchanged rows', function (): void {
[$user, $tenant] = createUserWithTenant(role: 'owner');
$finding = findingViewRbacFinding($tenant, [
'changed_keys' => [],
'metadata_keys' => [],
'permission_keys' => [],
'baseline' => [
'normalized' => [
'Role definition > Display name' => 'Security Reader',
'Role definition > Description' => 'Baseline description',
],
'is_built_in' => false,
'role_permission_count' => 1,
],
'current' => [
'normalized' => [
'Role definition > Display name' => 'Security Reader',
'Role definition > Description' => 'Baseline description',
],
'is_built_in' => false,
'role_permission_count' => 1,
],
]);
$this->actingAs($user)
->get(FindingResource::getUrl('view', ['record' => $finding], tenant: $tenant))
->assertOk()
->assertSee('0 changed')
->assertSee('0 added')
->assertSee('0 removed')
->assertSee('4 unchanged')
->assertSee('No changes detected.');
});
it('renders a stable sparse fallback when the RBAC evidence payload is empty', function (): void {
[$user, $tenant] = createUserWithTenant(role: 'owner');
$finding = findingViewRbacFinding($tenant, []);
$evidence = is_array($finding->evidence_jsonb) ? $finding->evidence_jsonb : [];
$evidence['rbac_role_definition'] = [];
$finding->forceFill([
'evidence_jsonb' => $evidence,
])->save();
$this->actingAs($user)
->get(FindingResource::getUrl('view', ['record' => $finding], tenant: $tenant))
->assertOk()
->assertSee('0 changed')
->assertSee('0 added')
->assertSee('0 removed')
->assertSee('0 unchanged')
->assertSee('No diff data available.')
->assertSee('RBAC restore is not supported');
});
it('shows RBAC labels and display-name fallback in the recent drift findings widget', function (): void {
[$user, $tenant] = createUserWithTenant(role: 'owner');
@ -148,3 +174,137 @@
->assertSee('Security Reader')
->assertSee('Intune RBAC Role Definition drift');
});
/**
* @param array<string, mixed> $rbacOverrides
*/
function findingViewRbacFinding(\App\Models\Tenant $tenant, array $rbacOverrides = []): Finding
{
$subjectExternalId = BaselineSubjectKey::workspaceSafeSubjectExternalIdForPolicy(
'intuneRoleDefinition',
'Security Reader',
'rbac-role-1',
);
return Finding::factory()->create([
'tenant_id' => (int) $tenant->getKey(),
'finding_type' => Finding::FINDING_TYPE_DRIFT,
'source' => 'baseline.compare',
'subject_type' => 'policy',
'subject_external_id' => (string) $subjectExternalId,
'evidence_fidelity' => 'content',
'severity' => Finding::SEVERITY_LOW,
'evidence_jsonb' => [
'change_type' => 'different_version',
'policy_type' => 'intuneRoleDefinition',
'subject_key' => hash('sha256', 'intuneRoleDefinition|rbac-role-1'),
'display_name' => 'Security Reader',
'summary' => [
'kind' => 'rbac_role_definition',
],
'baseline' => [
'policy_version_id' => 10,
'hash' => 'baseline',
],
'current' => [
'policy_version_id' => 11,
'hash' => 'current',
],
'rbac_role_definition' => findingViewRbacEvidenceFixture($rbacOverrides),
'fidelity' => 'content',
'provenance' => [
'baseline_profile_id' => 1,
'baseline_snapshot_id' => 1,
'compare_operation_run_id' => 1,
'inventory_sync_run_id' => 1,
],
],
]);
}
/**
* @param array<string, mixed> $overrides
* @return array<string, mixed>
*/
function findingViewRbacEvidenceFixture(array $overrides = []): array
{
return findingViewRbacFixtureMerge([
'diff_kind' => 'metadata_only',
'diff_fingerprint' => 'rbac-diff-1',
'changed_keys' => [
'Role definition > Description',
'Permission block 1 > Allowed actions',
],
'metadata_keys' => [
'Role definition > Description',
],
'permission_keys' => [
'Permission block 1 > Allowed actions',
],
'baseline' => [
'normalized' => [
'Role definition > Display name' => 'Security Reader',
'Role definition > Description' => 'Baseline description',
'Role definition > Scope tag IDs' => ['0', 'scope-1'],
'Permission block 1 > Allowed actions' => [
'Microsoft.Intune/deviceConfigurations/delete',
'Microsoft.Intune/deviceConfigurations/read',
],
'Permission block 1 > Denied actions' => [
'Microsoft.Intune/deviceConfigurations/wipe',
],
],
'is_built_in' => false,
'role_permission_count' => 1,
],
'current' => [
'normalized' => [
'Role definition > Display name' => 'Security Reader',
'Role definition > Description' => 'Updated description',
'Role definition > Scope tag IDs' => ['0', 'scope-1'],
'Permission block 1 > Allowed actions' => [
'Microsoft.Intune/deviceConfigurations/create',
'Microsoft.Intune/deviceConfigurations/read',
],
'Permission block 1 > Conditions' => [
'@Resource[Microsoft.Intune/deviceConfigurations] Exists',
],
],
'is_built_in' => false,
'role_permission_count' => 1,
],
], $overrides);
}
/**
* @param array<string, mixed> $base
* @param array<string, mixed> $overrides
* @return array<string, mixed>
*/
function findingViewRbacFixtureMerge(array $base, array $overrides): array
{
foreach ($overrides as $key => $value) {
if ($key === 'normalized') {
$base[$key] = $value;
continue;
}
if (
is_string($key)
&& array_key_exists($key, $base)
&& is_array($value)
&& is_array($base[$key])
&& ! array_is_list($value)
&& ! array_is_list($base[$key])
) {
$base[$key] = findingViewRbacFixtureMerge($base[$key], $value);
continue;
}
$base[$key] = $value;
}
return $base;
}

View File

@ -131,6 +131,33 @@
->assertSee($tenantB->getFilamentName());
});
it('hides onboarding tenants from the header tenant picker even for workspace owners', function (): void {
$tenantA = Tenant::factory()->create([
'status' => 'active',
'name' => 'YPTW2',
'environment' => 'other',
]);
[$user, $tenantA] = createUserWithTenant($tenantA, role: 'owner', workspaceRole: 'owner');
$onboardingTenant = Tenant::factory()->create([
'status' => Tenant::STATUS_ONBOARDING,
'workspace_id' => (int) $tenantA->workspace_id,
'name' => 'YPTW2',
'environment' => 'dev',
]);
Filament::setTenant(null, true);
$this->actingAs($user)
->withSession([
WorkspaceContext::SESSION_KEY => (int) $tenantA->workspace_id,
])
->get('/admin/operations')
->assertOk()
->assertSee($tenantA->getFilamentName())
->assertDontSee($onboardingTenant->getFilamentName());
});
it('does not implicitly switch tenant when opening canonical operation deep links', function (): void {
$tenantA = Tenant::factory()->create(['status' => 'active']);
[$user, $tenantA] = createUserWithTenant($tenantA, role: 'owner');

View File

@ -104,7 +104,8 @@
])
->mountAction('cancel_onboarding_draft')
->callMountedAction()
->assertNotified('Onboarding draft cancelled');
->assertNotified('Onboarding draft cancelled')
->assertRedirect(route('admin.onboarding.draft', ['onboardingDraft' => (int) $draft->getKey()]));
$draft->refresh();
@ -112,6 +113,12 @@
->and($draft->isResumable())->toBeFalse()
->and($draft->current_step)->toBe('cancelled');
$this->actingAs($user)
->get(route('admin.onboarding.draft', ['onboardingDraft' => (int) $draft->getKey()]))
->assertSuccessful()
->assertSee('This onboarding draft is Cancelled.')
->assertDontSee('Cancel draft');
expect(AuditLog::query()
->where('workspace_id', (int) $workspace->getKey())
->where('action', AuditActionId::ManagedTenantOnboardingCancelled->value)

View File

@ -101,6 +101,75 @@
->assertSee($user->name);
});
it('hides the all drafts header action when the current draft is the only resumable draft', function (): void {
$workspace = Workspace::factory()->create();
$user = User::factory()->create();
WorkspaceMembership::factory()->create([
'workspace_id' => (int) $workspace->getKey(),
'user_id' => (int) $user->getKey(),
'role' => 'owner',
]);
session()->put(WorkspaceContext::SESSION_KEY, (int) $workspace->getKey());
$draft = createOnboardingDraft([
'workspace' => $workspace,
'started_by' => $user,
'updated_by' => $user,
'state' => [
'entra_tenant_id' => '77777777-7777-7777-7777-777777777777',
'tenant_name' => 'Single Draft Tenant',
'environment' => 'prod',
],
]);
$this->actingAs($user)
->get(route('admin.onboarding.draft', ['onboardingDraft' => $draft->getKey()]))
->assertSuccessful()
->assertDontSee('All onboarding drafts');
});
it('shows the all drafts header action when multiple resumable drafts exist', function (): void {
$workspace = Workspace::factory()->create();
$user = User::factory()->create();
WorkspaceMembership::factory()->create([
'workspace_id' => (int) $workspace->getKey(),
'user_id' => (int) $user->getKey(),
'role' => 'owner',
]);
session()->put(WorkspaceContext::SESSION_KEY, (int) $workspace->getKey());
createOnboardingDraft([
'workspace' => $workspace,
'started_by' => $user,
'updated_by' => $user,
'state' => [
'entra_tenant_id' => '88888888-8888-8888-8888-888888888888',
'tenant_name' => 'First Draft Tenant',
'environment' => 'prod',
],
]);
$draft = createOnboardingDraft([
'workspace' => $workspace,
'started_by' => $user,
'updated_by' => $user,
'state' => [
'entra_tenant_id' => '99999999-9999-9999-9999-999999999999',
'tenant_name' => 'Second Draft Tenant',
'environment' => 'staging',
],
]);
$this->actingAs($user)
->get(route('admin.onboarding.draft', ['onboardingDraft' => $draft->getKey()]))
->assertSuccessful()
->assertSee('All onboarding drafts');
});
it('redirects to the canonical draft route immediately after step one identifies a tenant', function (): void {
$workspace = Workspace::factory()->create();
$user = User::factory()->create();

View File

@ -19,10 +19,13 @@
])
->assertSee('Changed')
->assertSee('Description')
->assertSee('Changed value')
->assertSee('From')
->assertSee('Before')
->assertSee('To')
->assertSee('After');
->assertSee('After')
->assertSee('border-warning-200')
->assertSee('bg-warning-50/70');
});
it('renders added removed and unchanged rows through the shared dispatcher', function (): void {
@ -103,3 +106,34 @@
->and($unchanged)->toContain('text-gray-500')
->and($unchanged)->toContain('dark:text-gray-400');
});
it('keeps unchanged rows quieter than changed rows while preserving shared state badges', function (): void {
$changed = (string) $this->view('filament.partials.diff.row', [
'row' => new DiffRow(
key: 'description',
label: 'Description',
status: DiffRowStatus::Changed,
oldValue: 'Before',
newValue: 'After',
),
'compact' => false,
'dimUnchanged' => true,
]);
$unchanged = (string) $this->view('filament.partials.diff.row', [
'row' => new DiffRow(
key: 'display_name',
label: 'Display name',
status: DiffRowStatus::Unchanged,
oldValue: 'TenantPilot',
newValue: 'TenantPilot',
),
'compact' => false,
'dimUnchanged' => true,
]);
expect($changed)->toContain('border-warning-200')
->and($changed)->toContain('Changed value')
->and($unchanged)->toContain('No material change')
->and($unchanged)->toContain('rounded-xl border border-gray-200 bg-white');
});

View File

@ -30,3 +30,20 @@
->assertSee('0 unchanged')
->assertSee('No diff data available.');
});
it('renders summary counts from array payloads and preserves explicit no-change messaging', function (): void {
$this->view('filament.partials.diff.summary-badges', [
'summary' => [
'changed' => 0,
'added' => 0,
'removed' => 0,
'unchanged' => 4,
'message' => 'No changes detected.',
],
])
->assertSee('0 changed')
->assertSee('0 added')
->assertSee('0 removed')
->assertSee('4 unchanged')
->assertSee('No changes detected.');
});

View File

@ -5,27 +5,27 @@
use App\Support\Diff\DiffRow;
use App\Support\Diff\DiffRowStatus;
it('renders inline list fragments with semantic labels muted unchanged items and stable reading order', function (): void {
it('renders RBAC-style inline list fragments with semantic labels muted unchanged items and stable reading order', function (): void {
$baseline = array_map(
static fn (int $index): string => sprintf('Group %02d', $index),
static fn (int $index): string => sprintf('Microsoft.Intune/deviceConfigurations/action-%02d', $index),
range(1, 24),
);
$current = array_map(
static fn (int $index): string => sprintf('Group %02d', $index),
static fn (int $index): string => sprintf('Microsoft.Intune/deviceConfigurations/action-%02d', $index),
range(2, 25),
);
$html = (string) $this->view('filament.partials.diff.inline-list', [
'row' => new DiffRow(
key: 'group_assignments',
label: 'Group assignments',
key: 'Permission block 1 > Allowed actions',
label: 'Permission block 1 > Allowed actions',
status: DiffRowStatus::Changed,
oldValue: $baseline,
newValue: $current,
isListLike: true,
addedItems: ['Group 25'],
removedItems: ['Group 01'],
addedItems: ['Microsoft.Intune/deviceConfigurations/action-25'],
removedItems: ['Microsoft.Intune/deviceConfigurations/action-01'],
unchangedItems: array_values(array_intersect($current, $baseline)),
),
'compact' => false,
@ -35,9 +35,9 @@
expect($html)->toContain('Added items')
->and($html)->toContain('Removed items')
->and($html)->toContain('Unchanged items')
->and($html)->toContain('Group 25')
->and($html)->toContain('Group 01')
->and($html)->toContain('Group 12')
->and($html)->toContain('Microsoft.Intune/deviceConfigurations/action-25')
->and($html)->toContain('Microsoft.Intune/deviceConfigurations/action-01')
->and($html)->toContain('Microsoft.Intune/deviceConfigurations/action-12')
->and($html)->toContain('text-gray-500')
->and($html)->toContain('dark:text-gray-400')
->and(substr_count($html, 'rounded-full'))->toBe(25);

View File

@ -0,0 +1,212 @@
<?php
declare(strict_types=1);
use App\Support\Diff\DiffRowStatus;
use App\Support\Diff\RbacRoleDefinitionDiffBuilder;
it('builds RBAC rows in deterministic review order with matching summary counts', function (): void {
$presentation = app(RbacRoleDefinitionDiffBuilder::class)->build(rbacBuilderEvidenceFixture());
expect(array_map(
static fn ($row): string => $row->key,
$presentation->rows,
))->toBe([
'Role definition > Display name',
'Role definition > Description',
'Role definition > Role source',
'Role definition > Permission blocks',
'Role definition > Scope tag IDs',
'Permission block 1 > Allowed actions',
'Permission block 1 > Denied actions',
'Permission block 1 > Conditions',
]);
$rows = collect($presentation->rows)->keyBy('key');
expect($presentation->summary->changedCount)->toBe(2)
->and($presentation->summary->addedCount)->toBe(1)
->and($presentation->summary->removedCount)->toBe(1)
->and($presentation->summary->unchangedCount)->toBe(4)
->and($presentation->summary->message)->toBeNull()
->and($rows->get('Role definition > Description')?->status)->toBe(DiffRowStatus::Changed)
->and($rows->get('Permission block 1 > Allowed actions')?->status)->toBe(DiffRowStatus::Changed)
->and($rows->get('Permission block 1 > Denied actions')?->status)->toBe(DiffRowStatus::Removed)
->and($rows->get('Permission block 1 > Conditions')?->status)->toBe(DiffRowStatus::Added)
->and($rows->get('Permission block 1 > Allowed actions')?->isListLike)->toBeTrue()
->and($rows->get('Permission block 1 > Allowed actions')?->addedItems)->toBe([
'Microsoft.Intune/deviceConfigurations/create',
])
->and($rows->get('Permission block 1 > Allowed actions')?->removedItems)->toBe([
'Microsoft.Intune/deviceConfigurations/delete',
])
->and($rows->get('Permission block 1 > Allowed actions')?->unchangedItems)->toBe([
'Microsoft.Intune/deviceConfigurations/read',
]);
});
it('preserves null boolean scalar and empty-list values for shared formatting', function (): void {
$presentation = app(RbacRoleDefinitionDiffBuilder::class)->build(rbacBuilderEvidenceFixture([
'changed_keys' => [
'Role definition > Description',
'Role definition > Preview enabled',
'Role definition > Scope tag IDs',
],
'baseline' => [
'normalized' => [
'Role definition > Description' => null,
'Role definition > Preview enabled' => false,
'Role definition > Scope tag IDs' => [],
],
'is_built_in' => false,
'role_permission_count' => 0,
],
'current' => [
'normalized' => [
'Role definition > Description' => 'Updated description',
'Role definition > Preview enabled' => true,
'Role definition > Scope tag IDs' => ['scope-1'],
],
'is_built_in' => false,
'role_permission_count' => 0,
],
]));
$rows = collect($presentation->rows)->keyBy('key');
expect($rows->get('Role definition > Description')?->oldValue)->toBeNull()
->and($rows->get('Role definition > Description')?->newValue)->toBe('Updated description')
->and($rows->get('Role definition > Preview enabled')?->oldValue)->toBeFalse()
->and($rows->get('Role definition > Preview enabled')?->newValue)->toBeTrue()
->and($rows->get('Role definition > Scope tag IDs')?->oldValue)->toBe([])
->and($rows->get('Role definition > Scope tag IDs')?->newValue)->toBe(['scope-1'])
->and($rows->get('Role definition > Scope tag IDs')?->isListLike)->toBeTrue()
->and($rows->get('Role definition > Scope tag IDs')?->addedItems)->toBe(['scope-1'])
->and($rows->get('Role definition > Scope tag IDs')?->removedItems)->toBe([])
->and($rows->get('Role definition > Role source')?->newValue)->toBe('Custom')
->and($rows->get('Role definition > Permission blocks')?->newValue)->toBe(0);
});
it('derives identical fallback rows into a no-change summary when normalized metadata is sparse', function (): void {
$presentation = app(RbacRoleDefinitionDiffBuilder::class)->build([
'changed_keys' => [],
'baseline' => [
'normalized' => [],
'is_built_in' => true,
'role_permission_count' => 1,
],
'current' => [
'normalized' => [],
'is_built_in' => true,
'role_permission_count' => 1,
],
]);
expect(array_map(
static fn ($row): string => $row->key,
$presentation->rows,
))->toBe([
'Role definition > Role source',
'Role definition > Permission blocks',
])
->and($presentation->summary->changedCount)->toBe(0)
->and($presentation->summary->addedCount)->toBe(0)
->and($presentation->summary->removedCount)->toBe(0)
->and($presentation->summary->unchangedCount)->toBe(2)
->and($presentation->summary->message)->toBe('No changes detected.');
});
it('returns a no-data presentation for empty or invalid RBAC payloads', function (): void {
$presentation = app(RbacRoleDefinitionDiffBuilder::class)->build([
'changed_keys' => ['Ghost key'],
'baseline' => ['normalized' => ['' => 'ignored']],
'current' => ['normalized' => [' ' => 'ignored']],
]);
expect($presentation->rows)->toBe([])
->and($presentation->summary->hasRows)->toBeFalse()
->and($presentation->summary->changedCount)->toBe(0)
->and($presentation->summary->addedCount)->toBe(0)
->and($presentation->summary->removedCount)->toBe(0)
->and($presentation->summary->unchangedCount)->toBe(0)
->and($presentation->summary->message)->toBe('No diff data available.');
});
/**
* @param array<string, mixed> $overrides
* @return array<string, mixed>
*/
function rbacBuilderEvidenceFixture(array $overrides = []): array
{
return rbacBuilderFixtureMerge([
'changed_keys' => [
'Role definition > Description',
'Permission block 1 > Allowed actions',
],
'baseline' => [
'normalized' => [
'Role definition > Display name' => 'Security Reader',
'Role definition > Description' => 'Baseline description',
'Role definition > Scope tag IDs' => ['0', 'scope-1'],
'Permission block 1 > Allowed actions' => [
'Microsoft.Intune/deviceConfigurations/delete',
'Microsoft.Intune/deviceConfigurations/read',
],
'Permission block 1 > Denied actions' => [
'Microsoft.Intune/deviceConfigurations/wipe',
],
],
'is_built_in' => false,
'role_permission_count' => 1,
],
'current' => [
'normalized' => [
'Role definition > Display name' => 'Security Reader',
'Role definition > Description' => 'Updated description',
'Role definition > Scope tag IDs' => ['0', 'scope-1'],
'Permission block 1 > Allowed actions' => [
'Microsoft.Intune/deviceConfigurations/create',
'Microsoft.Intune/deviceConfigurations/read',
],
'Permission block 1 > Conditions' => [
'@Resource[Microsoft.Intune/deviceConfigurations] Exists',
],
],
'is_built_in' => false,
'role_permission_count' => 1,
],
], $overrides);
}
/**
* @param array<string, mixed> $base
* @param array<string, mixed> $overrides
* @return array<string, mixed>
*/
function rbacBuilderFixtureMerge(array $base, array $overrides): array
{
foreach ($overrides as $key => $value) {
if ($key === 'normalized') {
$base[$key] = $value;
continue;
}
if (
is_string($key)
&& array_key_exists($key, $base)
&& is_array($value)
&& is_array($base[$key])
&& ! array_is_list($value)
&& ! array_is_list($base[$key])
) {
$base[$key] = rbacBuilderFixtureMerge($base[$key], $value);
continue;
}
$base[$key] = $value;
}
return $base;
}

View File

@ -4,7 +4,7 @@
use App\Support\Diff\ValueStringifier;
it('formats null booleans scalars lists and compact structured values consistently', function (): void {
it('formats RBAC diff values for null booleans scalars empty lists and compact structures consistently', function (): void {
$stringifier = new ValueStringifier;
expect($stringifier->stringify(null))->toBe('—')
@ -12,8 +12,10 @@
->and($stringifier->stringify(false))->toBe('Disabled')
->and($stringifier->stringify('TenantPilot'))->toBe('TenantPilot')
->and($stringifier->stringify(''))->toBe('""')
->and($stringifier->stringify(0))->toBe('0')
->and($stringifier->stringify(42))->toBe('42')
->and($stringifier->stringify([]))->toBe('[]')
->and($stringifier->stringify(['Alpha', 'Beta']))->toBe('Alpha, Beta')
->and($stringifier->stringify(['Scope tag 1', null]))->toBe('Scope tag 1, —')
->and($stringifier->stringify(['label' => 'Primary', 'enabled' => true]))->toBe('{"label":"Primary","enabled":true}');
});