TenantAtlas/specs/141-shared-diff-presentation-foundation/research.md
ahmido 0b5cadc234 feat: add shared diff presentation foundation (#170)
## Summary
- add a shared diff presentation layer under `app/Support/Diff` with deterministic row classification, summary derivation, and value stringification
- centralize diff-state badge semantics through `BadgeCatalog` with a dedicated `DiffRowStatusBadge`
- add reusable Filament diff partials, focused Pest coverage, and the full SpecKit artifact set for spec 141

## Testing
- `vendor/bin/sail artisan test --compact tests/Unit/Support/Diff/DiffRowStatusTest.php tests/Unit/Support/Diff/DiffRowTest.php tests/Unit/Support/Diff/DiffPresenterTest.php tests/Unit/Support/Diff/ValueStringifierTest.php tests/Unit/Badges/DiffRowStatusBadgeTest.php tests/Feature/Support/Diff/SharedDiffSummaryPartialTest.php tests/Feature/Support/Diff/SharedDiffRowPartialTest.php tests/Feature/Support/Diff/SharedInlineListDiffPartialTest.php`
- `vendor/bin/sail bin pint --dirty --format agent`

## Filament / Livewire Contract
- Livewire v4.0+ compliance: unchanged and respected; this feature adds presentation support only within the existing Filament v5 / Livewire v4 stack
- Provider registration: unchanged; no panel/provider changes were required, so `bootstrap/providers.php` remains the correct registration location
- Global search: unchanged; no Resource or global-search behavior was added or modified
- Destructive actions: none introduced in this feature
- Asset strategy: no new registered Filament assets; shared Blade partials rely on the existing asset pipeline and standard deploy step for `php artisan filament:assets` when assets change generally
- Testing coverage: presenter, DTOs, stringifier, badge semantics, summary partial, row partial, and inline-list partial are covered by focused Pest unit and feature tests

## Notes
- Spec checklist status is complete for `specs/141-shared-diff-presentation-foundation/checklists/requirements.md`
- This PR preserves specialized diff renderers and documents incremental adoption rather than forcing migration in the same change

Co-authored-by: Ahmed Darrazi <ahmed.darrazi@live.de>
Reviewed-on: #170
2026-03-14 12:32:08 +00:00

66 lines
7.4 KiB
Markdown

# Research: Shared Diff Presentation Foundation
## Decision 1: Keep the foundation in a dedicated presentation support layer under `app/Support/Diff`
- **Decision**: Introduce the shared diff primitives in `app/Support/Diff/` as presentation-only support classes: `DiffRowStatus`, `DiffRow`, `DiffSummary`, `DiffPresenter`, and `ValueStringifier`.
- **Rationale**: The repo already uses support-oriented layers for reusable UI and rendering concerns, while current diff behavior is fragmented across Blade templates and a few domain services. A support-layer location makes the new foundation easy to consume from Findings, PolicyVersion, RestoreRun, and future baseline/evidence screens without implying domain ownership or database coupling.
- **Alternatives considered**:
- Put the foundation inside `app/Services/Intune/`: rejected because the consumers are broader than Intune policy versioning and restore preview flows.
- Put the foundation inside one Filament resource: rejected because the feature is intentionally consumer-agnostic.
- Build a generic diff engine package: rejected because the spec explicitly forbids a full generic diff framework.
## Decision 2: Treat existing compare producers as inputs, not migration targets
- **Decision**: The foundation will adapt simple structured compare payloads from existing producers such as `VersionDiff`, `RestoreDiffGenerator`, and diff payloads already rendered in Findings and PolicyVersion surfaces, while leaving specialized normalized diff rendering out of scope for now.
- **Rationale**: Current compare outputs already vary: `normalized-diff` renders grouped dot-path changes with script-specific handling, RBAC diff uses side-by-side baseline/current cards, assignments and scope tags use custom section layouts, and restore preview/results wrap policy-level diff summaries. Forcing them into one canonical business payload now would add risk and block incremental adoption.
- **Alternatives considered**:
- Rewrite all existing compare producers to emit one canonical format: rejected because it would expand the feature into a cross-domain refactor.
- Limit the feature to one local consumer such as RBAC diff only: rejected because the product needs a reusable base for multiple upcoming compare surfaces.
## Decision 3: Centralize state badge semantics through the existing badge catalog
- **Decision**: Map shared diff states and summary badges through the existing badge catalog pattern rather than hardcoding colors in new partials.
- **Rationale**: The repo already centralizes badge semantics via `BadgeCatalog`, `BadgeDomain`, and focused badge tests. Existing diff templates still inline `success`, `danger`, and `warning` decisions; this feature is the right place to stop that drift for the shared foundation. Centralizing diff-state badge semantics aligns with BADGE-001 and keeps future consumers consistent.
- **Alternatives considered**:
- Inline state-to-color mappings in each partial: rejected because it recreates the current inconsistency problem.
- Use Tailwind-only classes without a semantic mapping layer: rejected because the repo already standardizes status-like badges centrally.
## Decision 4: Use composable Blade partials under `resources/views/filament/partials/diff`
- **Decision**: Build low-magic Blade partials for summary badges, generic row rendering by state, and inline list diff rendering under `resources/views/filament/partials/diff/`.
- **Rationale**: Current consumers already use `ViewEntry` and Blade partials as the main extension point. Adding composable partials fits existing Filament patterns, works inside infolists and custom page content, and avoids introducing a custom Filament field or entry framework before it is justified.
- **Alternatives considered**:
- Introduce a custom Filament field/entry component framework now: rejected because the spec explicitly says to avoid that unless Blade composition proves insufficient later.
- Create a single master diff page component: rejected because consumers vary between summary-first views, side-by-side views, and specialized normalized diffs.
## Decision 5: Keep Blade presentational and move formatting/row shaping into PHP
- **Decision**: Put row-state classification, list-fragment preparation, and value stringification into PHP support classes, leaving Blade responsible only for rendering.
- **Rationale**: Existing templates duplicate stringification rules and state grouping logic. The new foundation exists to improve consistency and testability, so the plan shifts logic into unit-testable support classes while keeping the partial APIs simple.
- **Alternatives considered**:
- Continue using per-template closures for formatting: rejected because it duplicates logic and weakens regression coverage.
- Put all formatting in one presenter and no standalone stringifier: rejected because the spec requires value formatting to be reusable even without the row presenter.
## Decision 6: Define a small internal contract rather than a new HTTP API
- **Decision**: Document the foundation contract as an internal OpenAPI-style schema with no new paths and component schemas describing presenter input, row output, summary counts, and inline list fragments.
- **Rationale**: The feature adds no routes, but SpecKit still expects a contract artifact. A pathless schema captures the internal handoff clearly for follow-up specs and keeps the feature aligned with its “no new routes” boundary.
- **Alternatives considered**:
- Skip the contracts artifact entirely: rejected because the planning workflow expects one.
- Invent a fake endpoint solely for the plan: rejected because it would contradict the specification.
## Decision 7: Cover rendering behavior with focused Pest unit and view-level tests
- **Decision**: Add unit tests for enum, row, presenter, and stringifier behavior plus view-level tests that render the shared partials with representative fixtures and assert accessible state cues, summary counts, dark-mode-safe class presence, and inline list diff output.
- **Rationale**: The spec requires both PHP-layer determinism and reusable rendering safety. The repo already uses Pest heavily, including badge mapping tests and feature tests around Filament rendering. This keeps the foundation verifiable without a full browser workflow.
- **Alternatives considered**:
- Rely on manual UI verification only: rejected because the foundation is intended for repeated reuse.
- Add browser tests first: rejected because the behavior under test is mostly server-side presentation and Blade output.
## Key integration observations
- `app/Services/Intune/VersionDiff.php` already produces simple `added`, `removed`, and `changed` buckets for normalized comparisons.
- `app/Services/Intune/RestoreDiffGenerator.php` already computes policy-level preview summaries and truncation metadata that a consumer can adapt into the shared presentation layer.
- `resources/views/filament/infolists/entries/normalized-diff.blade.php` contains substantial specialized logic including stringification, grouped rows, and script-aware diff handling; it should remain specialized in this feature.
- `resources/views/filament/infolists/entries/rbac-role-definition-diff.blade.php`, `assignments-diff.blade.php`, and `scope-tags-diff.blade.php` show three separate simple diff rendering patterns that are strong future adoption candidates.
- `app/Support/Badges/BadgeCatalog.php` and existing badge tests provide the canonical place to centralize diff state badge semantics.