TenantAtlas/specs/224-findings-notifications-escalation/research.md
ahmido e15d80cca5
Some checks failed
Main Confidence / confidence (push) Failing after 48s
Heavy Governance Lane / heavy-governance (push) Has been skipped
Browser Lane / browser (push) Has been skipped
feat: implement findings notifications escalation (#261)
## Summary
- implement Spec 224 findings notifications and escalation v1 on top of the existing alerts and Filament database notification infrastructure
- add finding assignment, reopen, due soon, and overdue event handling with direct recipient routing, dedupe, and optional external alert fan-out
- extend alert rule and alert delivery surfaces plus add the Spec 224 planning bundle and candidate-list promotion cleanup

## Validation
- `cd apps/platform && ./vendor/bin/sail bin pint --dirty --format agent`
- `cd apps/platform && ./vendor/bin/sail artisan test --compact tests/Feature/Findings/FindingsNotificationEventTest.php tests/Feature/Findings/FindingsNotificationRoutingTest.php`
- `cd apps/platform && ./vendor/bin/sail artisan test --compact tests/Feature/Alerts/FindingsAlertRuleIntegrationTest.php tests/Feature/Alerts/SlaDueAlertTest.php tests/Feature/Notifications/FindingNotificationLinkTest.php`

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

## Manual Smoke Note
- integrated-browser smoke testing confirmed the new alert rule event options, notification drawer entries, alert delivery history row, and tenant finding detail route on the active Sail host
- local notification deep links currently resolve from `APP_URL`, so a local `localhost` vs `127.0.0.1:8081` host mismatch can break the browser session if the app is opened on a different host/port combination

Co-authored-by: Ahmed Darrazi <ahmed.darrazi@live.de>
Reviewed-on: #261
2026-04-22 00:54:38 +00:00

69 lines
5.8 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

# Research: Findings Notifications & Escalation v1
## Decision 1: Reuse the existing alert event registry and alert-rule surfaces
**Decision**: Add the four finding event constants to `AlertRule` and expose them through `AlertRuleResource::eventTypeOptions()` and `AlertRuleResource::eventTypeLabel()`.
**Rationale**: Workspace alert rules, destinations, deliveries, cooldowns, quiet hours, and delivery-history viewing already exist and are the approved external-copy path. Extending that registry keeps rule configuration and delivery viewing in one place and avoids a second event catalog.
**Alternatives considered**:
- Create a findings-specific alert configuration surface. Rejected because the spec explicitly requires reuse of the existing alert system and forbids a second preference or notification center.
- Pass new event strings ad hoc without updating the central registry. Rejected because alert-rule selectors, delivery filters, and labels would drift immediately.
## Decision 2: Use existing Filament database notifications for direct personal delivery
**Decision**: Deliver direct operator notifications through Laravel database notifications with Filament payloads stored in the existing `notifications` table.
**Rationale**: `AdminPanelProvider` already enables `databaseNotifications()`, and the existing `OperationRunQueued` and `OperationRunCompleted` notification classes show the repositorys established pattern for title, body, and action-link payloads. This satisfies the requirement for in-app personal notifications without creating a new page or asset surface.
**Alternatives considered**:
- Build a custom Livewire inbox or findings-notification page. Rejected because it introduces a second notification surface and broader UI scope than the spec allows.
- Deliver only external copies through email or Teams. Rejected because the spec requires direct in-app notifications first and treats external channels as optional copies controlled by alert rules.
## Decision 3: Emit assignment and automatic-reopen events from `FindingWorkflowService` after commit
**Decision**: Hook `findings.assigned` and `findings.reopened` at the workflow-service boundary after `mutateAndAudit(...)` returns a refreshed `Finding` record.
**Rationale**: `FindingWorkflowService` already owns assignment, reopen, due-date reset, authorization, and audit semantics. Emitting after commit avoids dispatching notification side effects for rolled-back writes and keeps event truth attached to the seam that actually changes responsibility or lifecycle.
**Alternatives considered**:
- Emit from controllers, Filament actions, or pages. Rejected because some finding changes are system-driven and those surfaces do not own the canonical mutation truth.
- Emit inside the transaction. Rejected because direct or external delivery could start before the write commits.
- Notify on manual `reopen(...)` in the same pass. Rejected for v1 because the spec is limited to automatic reopen notification and keeping manual reopen silent reduces scope.
## Decision 4: Keep due-soon and overdue evaluation inside `EvaluateAlertsJob`
**Decision**: Extend `EvaluateAlertsJob` with finding due-soon and overdue candidate scans and route each candidate through the finding-notification delivery seam.
**Rationale**: The job already evaluates workspace-scoped alert events on a scheduled cadence, carries evaluation-window semantics, and runs inside the existing `alerts.evaluate` operational path. Reusing it avoids a second scheduler, command, or run family.
**Alternatives considered**:
- Create a dedicated due-notification job or Artisan command. Rejected because it duplicates scheduling, workspace scoping, and operational observability for no functional gain.
- Trigger due reminders from the UI. Rejected because reminder delivery must not depend on an operator visiting a page.
## Decision 5: Use notification-payload metadata for direct-delivery dedupe
**Decision**: Store a `fingerprint_key` and event metadata in the existing database notification `data` JSONB payload and query the existing `notifications` table to suppress duplicate direct deliveries.
**Rationale**: The spec forbids new persistence while still requiring one notification per due cycle and fingerprint-aware delivery semantics. Existing notification rows are already persisted delivery artifacts, so they are the narrowest place to record direct-delivery fingerprints without adding a new table.
**Alternatives considered**:
- Create a new `FindingNotificationDelivery` table. Rejected because it adds persistence and lifecycle ownership that the spec explicitly avoids.
- Skip direct dedupe entirely. Rejected because due-soon and overdue would repeat on every scheduled evaluation pass.
## Decision 6: Reuse tenant finding detail links and current entitlement checks
**Decision**: Build notification actions against the existing tenant-panel finding detail route and suppress send-time direct delivery if the selected recipient no longer has current tenant membership plus findings-view capability.
**Rationale**: The spec requires the notification to explain why the user is seeing it and send them to the existing finding detail route while preserving current `404` and `403` semantics. Rechecking entitlement at send time prevents stale assignments or ownership from generating a misleading in-app notification.
**Alternatives considered**:
- Link to the My Findings inbox instead of the single finding detail. Rejected because the spec calls for direct follow-up on the existing finding detail and the inbox adds avoidable navigation indirection.
- Add a new notification-detail page. Rejected because it duplicates finding detail and creates a second follow-up surface.
- Skip send-time entitlement revalidation. Rejected because tenant membership and capability state can change between assignment and delivery.