TenantAtlas/specs/230-findings-notification-convergence/research.md
ahmido 742d65f0d9
Some checks failed
Main Confidence / confidence (push) Failing after 51s
feat: converge findings notification presentation (#265)
## Summary
- converge finding, queued, and completed database notifications on one shared `OperationUxPresenter` presentation contract
- preserve existing finding and operation deep-link authorities while standardizing title, body, status/icon treatment, and single primary action
- add focused notification, findings, and guard coverage plus the full feature 230 spec artifacts

## Validation
- `cd apps/platform && ./vendor/bin/sail bin pint --dirty --format agent`
- `cd apps/platform && ./vendor/bin/sail artisan test --compact tests/Feature/Notifications/SharedDatabaseNotificationContractTest.php tests/Feature/Notifications/OperationRunNotificationTest.php tests/Feature/Notifications/FindingNotificationLinkTest.php`
- `cd apps/platform && ./vendor/bin/sail artisan test --compact tests/Feature/Findings/FindingsNotificationEventTest.php tests/Feature/Findings/FindingsNotificationRoutingTest.php tests/Feature/OpsUx/Constitution/LegacyNotificationGuardTest.php`

## Filament / Platform Notes
- Livewire v4.0+ compliance preserved on Filament v5 primitives
- provider registration remains unchanged in `apps/platform/bootstrap/providers.php`
- no globally searchable resource behavior changed in this feature
- no destructive actions were introduced
- asset strategy is unchanged; the existing `cd apps/platform && php artisan filament:assets` deploy step remains sufficient

Co-authored-by: Ahmed Darrazi <ahmed.darrazi@live.de>
Reviewed-on: #265
2026-04-22 20:26:18 +00:00

67 lines
5.4 KiB
Markdown

# Research: Findings Notification Presentation Convergence
## Decision 1: Extend the existing `OperationUxPresenter` seam instead of creating a notification framework
**Decision**: Reuse `App\Support\OpsUx\OperationUxPresenter` as the shared anchor for operator-facing database notification presentation and extend it with one bounded shared contract for the in-scope consumers.
**Rationale**: `OperationRunCompleted` already proves that the repository has one real shared presenter path for this interaction family. Extending that seam solves the current drift with the smallest change and follows XCUT-001 without introducing a new registry, interface family, or universal notification platform.
**Alternatives considered**:
- Create a new notification presenter package or registry. Rejected because only three real consumers are in scope and the spec explicitly forbids broader framework work.
- Apply local copy changes in each notification class. Rejected because that would leave the shared interaction family without one explicit contract and would allow drift to continue.
## Decision 2: Standardize the primary structure only, not every metadata field
**Decision**: The shared contract will standardize title, body, one status emphasis with the existing Filament icon treatment, one primary action, and optional supporting lines. Existing domain-specific metadata remains namespaced and secondary.
**Rationale**: Findings notifications and operation notifications carry different domain truth. The operator needs one stable primary card structure, not one flattened metadata model. Keeping `finding_event`, `reason_translation`, and related diagnostic fields secondary preserves current domain fidelity without fragmenting the card grammar.
**Alternatives considered**:
- Normalize all notification metadata into one new envelope. Rejected because it adds unnecessary semantic machinery and risks losing useful domain-specific context.
- Leave metadata and structure fully local. Rejected because primary structure drift is the operator problem this spec is solving.
## Decision 3: Preserve canonical deep-link helpers and plane-specific route resolution
**Decision**: Continue to source finding links from `FindingResource::getUrl(...)`, admin operation links from `OperationRunLinks`, and platform operation links from `SystemOperationRunLinks`.
**Rationale**: The feature is about presentation convergence, not route ownership. Existing helper seams already encode the correct tenant, admin, tenantless, and system-plane behavior, so the shared contract must consume them rather than replace them.
**Alternatives considered**:
- Build URLs inline inside the shared presenter. Rejected because it would duplicate existing routing truth and make access bugs more likely.
- Collapse platform and admin operation destinations into one route family. Rejected because the spec requires the current plane-specific destination behavior to stay intact.
## Decision 4: Keep the existing Filament drawer and persistence artifacts unchanged
**Decision**: The feature will keep the current Filament database-notification drawer, the existing `notifications` table, and the current asset and polling behavior unchanged.
**Rationale**: The operator problem is inconsistent card grammar, not missing surfaces or storage. Reusing the current shell avoids scope creep and keeps the change inside the intended cleanup boundary.
**Alternatives considered**:
- Build a new notification center or work queue. Rejected because it adds a second collection surface that the spec explicitly forbids.
- Add new persistence for shared presentation. Rejected because the shared contract is derived and does not represent new product truth.
## Decision 5: Prove convergence through focused feature and guard tests only
**Decision**: Add one contract-level notification test, extend current findings and operations notification tests, and update the existing legacy-notification guard for both local-bypass prevention and the preserved FR-015 boundary.
**Rationale**: The proving burden is payload structure and route safety across current consumers. Feature-level assertions already cover these concerns and are the narrowest executable proof.
**Alternatives considered**:
- Add browser coverage. Rejected because the Filament drawer shell itself is unchanged and browser cost would not prove more than payload-level feature tests.
- Rely only on the legacy guard. Rejected because the guard can catch bypass patterns but cannot prove that the rendered payload structure stayed aligned across all current consumers.
## Decision 6: Preserve operation-completed guidance and reason translation as secondary context
**Decision**: `OperationRunCompleted` keeps its terminal presentation logic, summary lines, and `ReasonPresenter` output, but these remain secondary supporting context under the shared primary structure.
**Rationale**: Terminal run notifications carry more diagnostic signal than findings or queued runs. The shared contract should unify the card grammar without erasing the extra detail that makes completed-run notifications actionable.
**Alternatives considered**:
- Flatten completed-run detail into the same minimal body as queued notifications. Rejected because it would reduce useful operator signal on terminal outcomes.
- Keep completed-run notifications fully separate because of richer detail. Rejected because that would leave the current interaction-family drift unresolved.