ahmido
a770b32e87
Implements Spec 082 updates to the Filament Action Surface Contract: - New required list/table slot: InspectAffordance (clickable row via recordUrl preferred; also supports View action or primary link column) - Retrofit view-only tables to remove lone View row action buttons and use clickable rows - Update validator + guard tests, add golden regression assertions - Add docs: docs/ui/action-surface-contract.md Tests (local via Sail): - vendor/bin/sail artisan test --compact tests/Feature/Guards/ActionSurfaceContractTest.php - vendor/bin/sail artisan test --compact tests/Feature/Guards/ActionSurfaceValidatorTest.php - vendor/bin/sail artisan test --compact tests/Feature/Rbac/ActionSurfaceRbacSemanticsTest.php - vendor/bin/sail artisan test --compact tests/Feature/Filament/EntraGroupSyncRunResourceTest.php Notes: - Filament v5 / Livewire v4 compatible. - No destructive-action behavior changed in this PR. Co-authored-by: Ahmed Darrazi <ahmeddarrazi@MacBookPro.fritz.box> Reviewed-on: #100
105 lines
5.0 KiB
Markdown
105 lines
5.0 KiB
Markdown
# Research — Action Surface Contract + CI Enforcement
|
|
|
|
This document resolves planning unknowns for Spec 082 and records key design decisions.
|
|
|
|
## Decision 1 — Scope enumeration (Tenant + Admin panels; exclude Widgets)
|
|
|
|
**Decision**: The validator enumerates in-scope Filament classes by scanning `app/Filament/**` for Resources, Pages, and RelationManagers and then filtering to:
|
|
- `app/Filament/Resources/**` (tenant panel discovery)
|
|
- `app/Filament/Pages/**` (tenant panel discovery)
|
|
- `app/Filament/RelationManagers/**` (used by resources)
|
|
- plus any admin-panel explicit resources/pages registered via `app/Providers/Filament/AdminPanelProvider.php`
|
|
|
|
Widgets are explicitly excluded.
|
|
|
|
**Rationale**:
|
|
- Matches clarified spec scope (Tenant + Admin panels; Resources/Pages/RelationManagers; exclude Widgets).
|
|
- Deterministic and fast (filesystem + reflection; no UI runtime required).
|
|
- Aligns with existing repo patterns (guard tests that scan filesystem and fail with actionable output).
|
|
|
|
**Alternatives considered**:
|
|
- Discovering via Filament panel runtime discovery: rejected (adds runtime coupling and can be non-deterministic in CI).
|
|
- Enumerating only by filesystem: accepted as baseline, but we still need a small allowlist for admin explicit registrations.
|
|
|
|
## Decision 2 — Declaration attachment mechanism
|
|
|
|
**Decision**: Each in-scope class provides a declaration via a static method (e.g., `public static function actionSurfaceDeclaration(): ActionSurfaceDeclaration`).
|
|
|
|
**Rationale**:
|
|
- PHP-first, simple, no attribute parsing or config conventions.
|
|
- Easy to require/validate by reflection.
|
|
- Plays well with existing Filament static patterns (Resources already use statics like `$recordTitleAttribute`).
|
|
|
|
**Alternatives considered**:
|
|
- PHP 8 attributes: rejected for now (slower adoption, harder to keep structured without additional tooling).
|
|
- Central registry config: rejected (drifts away from the component, discourages ownership).
|
|
|
|
## Decision 3 — Contract enforcement: “declare or exempt” + representative runtime checks
|
|
|
|
**Decision**: CI enforces that each in-scope class has a declaration and that every required slot is either marked satisfied or has an explicit exemption with a non-empty reason.
|
|
|
|
The validator does not attempt to prove that every underlying Filament `table()` / `getHeaderActions()` actually contains those actions. Instead, representative runtime tests cover constitution-critical behavior (grouping, confirmation, RBAC semantics, canonical run links).
|
|
|
|
**Rationale**:
|
|
- Deterministic and robust across Filament internal changes.
|
|
- Avoids brittle source parsing and avoids needing Livewire/Filament runtime contexts.
|
|
- Still forces explicit human intent on each surface, which is the governance goal.
|
|
|
|
**Alternatives considered**:
|
|
- AST/static parsing of PHP to verify `->actions()` / `->bulkActions()` usage: rejected (high complexity and brittleness).
|
|
- Instantiating Filament objects and introspecting action definitions for every surface in CI: rejected initially (risk of boot/DI coupling). Representative runtime tests are used instead for critical guarantees.
|
|
|
|
## Decision 4 — Profiles and required “slots”
|
|
|
|
**Decision**: Model “profiles” that map to component archetypes (e.g., list-only, list+detail, read-only list, run-log list). Each profile defines required slots:
|
|
- List header actions
|
|
- List row actions (max 2 visible; rest in “More”)
|
|
- List bulk actions (grouped; “More” label)
|
|
- List empty-state actions
|
|
- Detail header actions (if the component has a detail view/edit/view page)
|
|
|
|
**Rationale**:
|
|
- Keeps enforcement simple and consistent.
|
|
- Allows targeted exceptions by profile without exploding per-component logic.
|
|
|
|
**Alternatives considered**:
|
|
- One giant universal profile: rejected (too many exemptions, low signal).
|
|
|
|
## Decision 5 — Exemption policy
|
|
|
|
**Decision**: Exemptions require:
|
|
- a non-empty reason string
|
|
- optional tracking reference (issue/PR link)
|
|
- no deadline requirement
|
|
|
|
**Rationale**:
|
|
- Matches clarified spec.
|
|
- Keeps CI deterministic (no time-based expiry).
|
|
|
|
**Alternatives considered**:
|
|
- Required expiry dates: rejected (creates “time bombs” in CI).
|
|
|
|
## Decision 6 — Standard grouping label and safe-default Export
|
|
|
|
**Decision**:
|
|
- The canonical group label for secondary row actions and bulk action groups is `More`.
|
|
- For `ReadOnly` + `RunLog` profiles, the default expected safe bulk action is `Export` (otherwise exemption required).
|
|
|
|
**Rationale**:
|
|
- Matches clarified spec.
|
|
- Provides a consistent, predictable UX baseline.
|
|
|
|
**Alternatives considered**:
|
|
- Allowing arbitrary labels: rejected (inconsistent UX; harder to audit).
|
|
|
|
## Decision 7 — Typed confirmation threshold
|
|
|
|
**Decision**: The contract model includes a `requiresTypedConfirmation` flag for actions that are destructive/irreversible, affect > 25 records, or change access/credentials/safety gates.
|
|
|
|
**Rationale**:
|
|
- Matches clarified spec.
|
|
- Keeps enforcement declarative and reviewable.
|
|
|
|
**Alternatives considered**:
|
|
- Always requiring typed confirmations: rejected (too heavy for routine admin workflows).
|