66 lines
7.4 KiB
Markdown
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. |