cbaebcf649
- Add notifications table with Alembic migration
- Notification model with user-scoped indexing and partial index on unread
- NotificationService singleton with create/list/count/mark-read/dismiss
- FastAPI router: GET /notifications, GET /unread, PATCH /{id}/read,
POST /mark-all-read, DELETE /{id}
- Mute categories filtering from UserConfig
- 13 unit tests for NotificationService
- 10 integration tests for API endpoints
- Updated test_models.py with new table registration
Quality gates: pytest 23 new passed, ruff clean
869 lines
37 KiB
Markdown
869 lines
37 KiB
Markdown
# SDD Tasks: Notification Center
|
||
|
||
## Review Workload Forecast
|
||
|
||
| Field | Value |
|
||
|-------|-------|
|
||
| Estimated changed lines | ~1,800 total (PR-1 ~600; PR-2 ~250; PR-3 ~700; PR-4 ~250) |
|
||
| 400-line budget risk | High |
|
||
| Chained PRs recommended | Yes |
|
||
| Suggested split | PR 1 (Backend Core) → PR 2 (Backend Integration) → PR 3 (Frontend Core) → PR 4 (Toast Coordination) |
|
||
| Delivery strategy | auto-chain |
|
||
| Chain strategy | stacked-to-main |
|
||
|
||
```
|
||
Decision needed before apply: No
|
||
Chained PRs recommended: Yes
|
||
Chain strategy: stacked-to-main
|
||
400-line budget risk: High
|
||
```
|
||
|
||
> **Note:** PR-1 (~600 lines) and PR-3 (~700 lines) exceed the 400-line review budget. PR-3 in particular carries High risk. Tasks within each PR are grouped into autonomous work units. If review fanout is available, PR-3 can be split into (a) Provider + Hook + Styles and (b) NotificationCenter + NotificationItem + AppShell integration. PR-1 can be split into (a) Migration + Model + Service and (b) Router + Registration + Tests.
|
||
|
||
---
|
||
|
||
## PR-1: Backend Core
|
||
|
||
**Goal:** Establish the persistent notification backend: database schema, SQLAlchemy model, NotificationService singleton, FastAPI router with Pydantic schemas, and comprehensive unit + integration tests.
|
||
|
||
**Estimated Lines:** ~600
|
||
**Review Risk:** Medium
|
||
|
||
---
|
||
|
||
### NC-PR1-001: Create Alembic migration for notifications table
|
||
|
||
**Description:**
|
||
Write an Alembic revision that creates the `notifications` table with all columns, constraints, indexes, and the foreign key to `users.id` as specified in the design.
|
||
|
||
**Files to modify:**
|
||
- `apps/api/alembic/versions/2026_05_29_add_notifications_table.py` *(new)*
|
||
|
||
**Acceptance criteria:**
|
||
- [ ] Migration creates `notifications` table with columns: `id`, `user_id`, `category`, `severity`, `title`, `message`, `source_type`, `source_id`, `metadata`, `read_at`, `dismissed_at`, `created_at`.
|
||
- [ ] Foreign key `user_id` references `users.id` with `ON DELETE CASCADE`.
|
||
- [ ] Index `idx_notifications_user_created_at` on `(user_id, created_at DESC)`.
|
||
- [ ] Partial index `idx_notifications_user_unread` on `(user_id, read_at)` where `read_at IS NULL`.
|
||
- [ ] `upgrade()` and `downgrade()` are both implemented and pass `alembic upgrade head` / `alembic downgrade -1`.
|
||
- [ ] Migration depends on current `head` revision.
|
||
|
||
**Estimated effort:** Small (2–3 hours)
|
||
**Dependencies:** None
|
||
|
||
---
|
||
|
||
### NC-PR1-002: Create SQLAlchemy Notification model and export
|
||
|
||
**Description:**
|
||
Add the `Notification` SQLAlchemy model following the existing `UUIDPrimaryKeyMixin` + `Base` pattern. Export it from `models/__init__.py` for Alembic autogenerate discovery.
|
||
|
||
**Files to modify:**
|
||
- `apps/api/src/models/notification.py` *(new)*
|
||
- `apps/api/src/models/__init__.py`
|
||
|
||
**Acceptance criteria:**
|
||
- [ ] `Notification` model matches the design schema exactly with correct types (`UUID`, `String(32)`, `String(16)`, `String(255)`, `Text`, `JSONB`, `DateTime(timezone=True)`).
|
||
- [ ] `user_id` has `ForeignKey("users.id", ondelete="CASCADE")`, `nullable=False`, `index=True`.
|
||
- [ ] `read_at` and `created_at` are indexed.
|
||
- [ ] `metadata` column defaults to `{}`.
|
||
- [ ] Model is exported in `models/__init__.py`.
|
||
- [ ] `alembic revision --autogenerate` produces no drift against the hand-written migration.
|
||
|
||
**Estimated effort:** Small (2–3 hours)
|
||
**Dependencies:** NC-PR1-001
|
||
|
||
---
|
||
|
||
### NC-PR1-003: [RED] Write NotificationService unit tests — basic CRUD
|
||
|
||
**Description:**
|
||
Write failing pytest unit tests for `NotificationService` covering create, list, count, mark_read, mark_all_read, and dismiss happy paths.
|
||
|
||
**Files to modify:**
|
||
- `apps/api/tests/unit/test_notification_service.py` *(new)*
|
||
|
||
**Acceptance criteria:**
|
||
- [ ] `test_create_notification`: assert row inserted with correct values, `read_at` NULL, `dismissed_at` NULL.
|
||
- [ ] `test_list_notifications_orders_by_created_at_desc`: 3 rows inserted, newest first.
|
||
- [ ] `test_list_notifications_excludes_dismissed`: dismissed row not returned.
|
||
- [ ] `test_list_notifications_unread_only`: `unread_only=True` returns only unread.
|
||
- [ ] `test_get_unread_count`: 5 rows, 2 unread → count is 2.
|
||
- [ ] `test_mark_read_sets_read_at`: `read_at` is not NULL after call.
|
||
- [ ] `test_mark_all_read_affects_all_unread`: all unread rows updated.
|
||
- [ ] `test_dismiss_sets_dismissed_at`: `dismissed_at` is not NULL after call.
|
||
- [ ] Tests use `db_session` fixture and create test `User` rows in session.
|
||
|
||
**Estimated effort:** Small (3–4 hours)
|
||
**Dependencies:** NC-PR1-002
|
||
|
||
---
|
||
|
||
### NC-PR1-004: [GREEN] Implement NotificationService
|
||
|
||
**Description:**
|
||
Implement the `NotificationService` singleton with all methods. The service accepts `AsyncSession` explicitly and filters all queries by `user_id`.
|
||
|
||
**Files to modify:**
|
||
- `apps/api/src/services/notification_service.py` *(new)*
|
||
|
||
**Acceptance criteria:**
|
||
- [ ] `create_notification(session, user_id, *, category, severity, title, ...)` inserts row and returns `Notification`.
|
||
- [ ] `list_notifications(session, user_id, *, limit=20, offset=0, unread_only=False, mute_categories=None)` returns `(items, total)` tuple, excludes `dismissed_at IS NOT NULL`, orders by `created_at DESC`.
|
||
- [ ] `get_unread_count(session, user_id)` counts rows where `read_at IS NULL` and `dismissed_at IS NULL`.
|
||
- [ ] `mark_read(session, notification_id, user_id)` sets `read_at = now()`, returns updated row; raises 404-equivalent if not found or not owned.
|
||
- [ ] `mark_all_read(session, user_id)` sets `read_at = now()` on all unread rows for user; returns count updated.
|
||
- [ ] `dismiss(session, notification_id, user_id)` sets `dismissed_at = now()`; raises 404-equivalent if not found or not owned.
|
||
- [ ] All methods filter by `user_id`.
|
||
- [ ] `NC-PR1-003` tests pass.
|
||
|
||
**Estimated effort:** Medium (4–5 hours)
|
||
**Dependencies:** NC-PR1-003
|
||
|
||
---
|
||
|
||
### NC-PR1-005: [TRIANGULATE] NotificationService edge-case and isolation tests
|
||
|
||
**Description:**
|
||
Add unit tests for cross-user isolation, wrong-owner failures, mute category filtering, and partial index usage.
|
||
|
||
**Files to modify:**
|
||
- `apps/api/tests/unit/test_notification_service.py`
|
||
|
||
**Acceptance criteria:**
|
||
- [ ] `test_mark_read_wrong_owner_raises`: User A creates notification; User B calls `mark_read` → exception raised.
|
||
- [ ] `test_dismiss_wrong_owner_raises`: User A creates notification; User B calls `dismiss` → exception raised.
|
||
- [ ] `test_list_notifications_mute_categories`: pass `mute_categories=["instance"]`; instance rows excluded, system rows returned.
|
||
- [ ] `test_get_unread_count_excludes_dismissed`: unread but dismissed row → count is 0.
|
||
- [ ] `test_get_unread_count_query_uses_partial_index`: query plan uses `idx_notifications_user_unread` (verified via `EXPLAIN` or SQLite equivalent).
|
||
|
||
**Estimated effort:** Small (2–3 hours)
|
||
**Dependencies:** NC-PR1-004
|
||
|
||
---
|
||
|
||
### NC-PR1-006: [RED] Write API integration tests — basic endpoints
|
||
|
||
**Description:**
|
||
Write failing integration tests for the notifications API router covering list, unread count, mark read, mark all read, and dismiss.
|
||
|
||
**Files to modify:**
|
||
- `apps/api/tests/integration/test_notifications_api.py` *(new)*
|
||
|
||
**Acceptance criteria:**
|
||
- [ ] `test_list_requires_auth`: `GET /notifications` without auth → `401`.
|
||
- [ ] `test_list_returns_only_own_notifications`: create for user A; user B lists → not in response.
|
||
- [ ] `test_unread_count_endpoint`: create 3 unread; `GET /notifications/unread` → `{count: 3}`.
|
||
- [ ] `test_mark_read_endpoint`: create unread; `PATCH /notifications/{id}/read` → `200`, `read_at` set.
|
||
- [ ] `test_mark_all_read_endpoint`: create 4 unread; `POST /notifications/mark-all-read` → `{marked_count: 4}`.
|
||
- [ ] `test_dismiss_endpoint`: create notification; `DELETE /notifications/{id}` → `204`; subsequent list excludes it.
|
||
- [ ] Uses `authenticated_client` and `db_session` fixtures.
|
||
|
||
**Estimated effort:** Small (3–4 hours)
|
||
**Dependencies:** NC-PR1-004
|
||
|
||
---
|
||
|
||
### NC-PR1-007: [GREEN] Implement FastAPI notifications router and Pydantic schemas
|
||
|
||
**Description:**
|
||
Create the FastAPI `APIRouter` for `/notifications` with all endpoints and Pydantic response models. Read `mute_categories` from `UserConfig` and pass to `list_notifications`.
|
||
|
||
**Files to modify:**
|
||
- `apps/api/src/api/notifications.py` *(new)*
|
||
- `apps/api/src/api/__init__.py`
|
||
|
||
**Acceptance criteria:**
|
||
- [ ] `GET /notifications` with `limit`, `offset`, `unread_only` query params; returns `NotificationListResponse`.
|
||
- [ ] `GET /notifications/unread` returns `UnreadCountResponse`.
|
||
- [ ] `PATCH /notifications/{id}/read` returns `NotificationItem`; `404` if not owned.
|
||
- [ ] `POST /notifications/mark-all-read` returns `MarkAllReadResponse`.
|
||
- [ ] `DELETE /notifications/{id}` returns `204 No Content`; `404` if not owned.
|
||
- [ ] Router reads `notification_mute_categories` from user's `UserConfig.config` and passes to `list_notifications`.
|
||
- [ ] `limit` capped at 100.
|
||
- [ ] All endpoints use `get_current_user_id` / `get_db_session` dependencies.
|
||
- [ ] Router exported from `api/__init__.py`.
|
||
- [ ] `NC-PR1-006` tests pass.
|
||
|
||
**Estimated effort:** Medium (4–5 hours)
|
||
**Dependencies:** NC-PR1-006
|
||
|
||
---
|
||
|
||
### NC-PR1-008: [TRIANGULATE] API edge-case and ownership tests
|
||
|
||
**Description:**
|
||
Add integration tests for pagination, ownership enforcement, and mute categories filtering at the API layer.
|
||
|
||
**Files to modify:**
|
||
- `apps/api/tests/integration/test_notifications_api.py`
|
||
|
||
**Acceptance criteria:**
|
||
- [ ] `test_list_pagination`: create 25 notifications; `limit=10&offset=10` → items length 10, total 25.
|
||
- [ ] `test_mark_read_404_for_other_user`: create for user A; user B PATCH → `404`.
|
||
- [ ] `test_dismiss_404_for_other_user`: user B DELETE user A's notification → `404`.
|
||
- [ ] `test_mute_categories_filter_in_list`: set user config `mute_categories=["instance"]`, create instance + system notifications; `GET /notifications` returns only system.
|
||
- [ ] `test_mark_all_read_affects_only_caller`: user A has 3 unread, user B has 2; A calls mark-all-read → A=0, B=2.
|
||
|
||
**Estimated effort:** Small (2–3 hours)
|
||
**Dependencies:** NC-PR1-007
|
||
|
||
---
|
||
|
||
### NC-PR1-009: Register router in main.py and import model for Alembic
|
||
|
||
**Description:**
|
||
Import and include the notifications router in the FastAPI app. Import the `Notification` model in `main.py` for Alembic autogenerate discovery.
|
||
|
||
**Files to modify:**
|
||
- `apps/api/src/main.py`
|
||
|
||
**Acceptance criteria:**
|
||
- [ ] `notifications_router` imported and included with `app.include_router(...)`.
|
||
- [ ] `Notification` model imported in `main.py` (F401 noqa comment if unused).
|
||
- [ ] App boots without import cycles.
|
||
- [ ] `GET /health` still returns `200`.
|
||
- [ ] `GET /notifications` returns `401` when unauthenticated (smoke test).
|
||
|
||
**Estimated effort:** Small (1 hour)
|
||
**Dependencies:** NC-PR1-007
|
||
|
||
---
|
||
|
||
### NC-PR1-010: [REFACTOR] Backend code quality and type safety pass
|
||
|
||
**Description:**
|
||
Run `ruff check .`, `mypy .`, and `pytest` on the new code. Fix any lint errors, type annotations, or docstring gaps. Ensure no `print` statements or debug logs remain.
|
||
|
||
**Files to modify:**
|
||
- Any of the above files with lint/type issues.
|
||
|
||
**Acceptance criteria:**
|
||
- [ ] `ruff check .` passes with zero errors on new files.
|
||
- [ ] `mypy .` passes with zero type errors on new files.
|
||
- [ ] `pytest tests/unit/test_notification_service.py tests/integration/test_notifications_api.py` passes.
|
||
- [ ] All public methods have docstrings.
|
||
- [ ] No `print()` or leftover `logger.debug` from development.
|
||
|
||
**Estimated effort:** Small (1–2 hours)
|
||
**Dependencies:** NC-PR1-008, NC-PR1-009
|
||
|
||
---
|
||
|
||
## PR-2: Backend Integration
|
||
|
||
**Goal:** Wire lifecycle hooks and health monitor to create notifications, extend UserConfig for preferences, and validate the end-to-end event producer flow.
|
||
|
||
**Estimated Lines:** ~250
|
||
**Review Risk:** Low
|
||
|
||
---
|
||
|
||
### NC-PR2-001: Wire lifecycle_hooks.py to call NotificationService
|
||
|
||
**Description:**
|
||
After `publish_lifecycle_event()` publishes the raw event to `InstanceEventBus`, call `NotificationService.create_notification()` with `user_id=tool_instance.owner_id`. Wrap in `try/except` so event pipeline is never blocked.
|
||
|
||
**Files to modify:**
|
||
- `apps/api/src/services/lifecycle_hooks.py`
|
||
|
||
**Acceptance criteria:**
|
||
- [ ] `publish_lifecycle_event` calls `notification_service.create_notification(...)` after bus publish.
|
||
- [ ] `user_id` is set to `instance.owner_id`.
|
||
- [ ] `category="instance"`.
|
||
- [ ] `severity` mapped: `info` for created/started/stopped/restarted/deleted; `error` for error.
|
||
- [ ] `title` derived from event type (e.g., "Container started").
|
||
- [ ] `source_type="tool_instances"`, `source_id=instance.id`.
|
||
- [ ] Service call wrapped in `try/except`; on failure, error is logged with `correlation_id` and execution continues.
|
||
- [ ] Original event bus publish and audit row insert are unaffected by notification failure.
|
||
|
||
**Estimated effort:** Small (2–3 hours)
|
||
**Dependencies:** NC-PR1-010
|
||
|
||
---
|
||
|
||
### NC-PR2-002: Wire health_monitor.py to call NotificationService
|
||
|
||
**Description:**
|
||
After `HealthMonitor` detects a state change and publishes the event, call `NotificationService.create_notification()` with `user_id=instance.owner_id`. Wrap in `try/except`.
|
||
|
||
**Files to modify:**
|
||
- `apps/api/src/services/health_monitor.py`
|
||
|
||
**Acceptance criteria:**
|
||
- [ ] `_handle_state_change` calls `notification_service.create_notification(...)` after bus publish.
|
||
- [ ] `user_id` is set to `instance.owner_id`.
|
||
- [ ] `category="health"` for health changes; `"instance"` for errors.
|
||
- [ ] `severity` mapped: `error` for crash, `warning` for unhealthy, `info` for recovery.
|
||
- [ ] `source_type="tool_instances"`, `source_id=instance.id`.
|
||
- [ ] Service call wrapped in `try/except`; on failure, error is logged with `correlation_id` and loop continues.
|
||
- [ ] Original event bus publish and health check insert are unaffected.
|
||
|
||
**Estimated effort:** Small (2–3 hours)
|
||
**Dependencies:** NC-PR1-010
|
||
|
||
---
|
||
|
||
### NC-PR2-003: Extend UserConfig schema for notification preferences
|
||
|
||
**Description:**
|
||
Add `notification_mute_categories` and `notification_toast_level` to the `UserConfigResponse` and `UserConfigUpdate` Pydantic models. Apply `mute_categories` filtering in `list_notifications`.
|
||
|
||
**Files to modify:**
|
||
- `apps/api/src/api/user_config.py`
|
||
- `apps/api/src/services/notification_service.py`
|
||
|
||
**Acceptance criteria:**
|
||
- [ ] `UserConfigResponse` includes `notification_mute_categories: list[str] | None = None` and `notification_toast_level: str | None = None`.
|
||
- [ ] `UserConfigUpdate` includes the same optional fields.
|
||
- [ ] `list_notifications` in `NotificationService` accepts `mute_categories` and filters with `Notification.category.not_in(mute_categories)`.
|
||
- [ ] `GET /users/me/config` returns new keys when present in JSON blob.
|
||
- [ ] `PATCH /users/me/config` persists new keys into the JSON blob.
|
||
- [ ] Existing config keys are unaffected.
|
||
|
||
**Estimated effort:** Small (2–3 hours)
|
||
**Dependencies:** NC-PR1-010
|
||
|
||
---
|
||
|
||
### NC-PR2-004: [RED] Write event producer integration tests
|
||
|
||
**Description:**
|
||
Write integration tests that exercise real lifecycle and health monitor endpoints and assert notification rows are created for the instance owner.
|
||
|
||
**Files to modify:**
|
||
- `apps/api/tests/integration/test_notification_producers.py` *(new)*
|
||
|
||
**Acceptance criteria:**
|
||
- [ ] `test_lifecycle_event_creates_notification`: trigger `instance.started` via lifecycle hook; assert notification row exists with `category="instance"`, `severity="info"`, `user_id=owner_id`.
|
||
- [ ] `test_health_monitor_error_creates_notification`: simulate health monitor detecting crash; assert notification row with `severity="error"`.
|
||
- [ ] `test_notification_failure_does_not_block_event_pipeline`: mock `create_notification` to raise; assert event is still published and no exception escapes.
|
||
- [ ] `test_notification_ownership_matches_instance_owner`: create instance for user A; trigger event; assert notification `user_id` is A's ID, not the calling user's.
|
||
- [ ] Uses `authenticated_client`, `db_session`, and `test_project_and_repo` fixtures.
|
||
|
||
**Estimated effort:** Medium (3–4 hours)
|
||
**Dependencies:** NC-PR2-001, NC-PR2-002, NC-PR2-003
|
||
|
||
---
|
||
|
||
### NC-PR2-005: [GREEN / REFACTOR] Verify producer tests pass and clean up
|
||
|
||
**Description:**
|
||
Run the producer integration tests, fix any failures, and do a final lint/type check on all modified files.
|
||
|
||
**Files to modify:**
|
||
- Any files with issues found during test runs.
|
||
|
||
**Acceptance criteria:**
|
||
- [ ] `pytest tests/integration/test_notification_producers.py` passes.
|
||
- [ ] `ruff check .` passes on modified files.
|
||
- [ ] `mypy .` passes on modified files.
|
||
- [ ] No regressions in existing `pytest` suite.
|
||
|
||
**Estimated effort:** Small (1–2 hours)
|
||
**Dependencies:** NC-PR2-004
|
||
|
||
---
|
||
|
||
## PR-3: Frontend Core
|
||
|
||
**Goal:** Build the frontend notification surface: icon registry, React context with polling, hook, notification list components, styles, and AppShell integration.
|
||
|
||
**Estimated Lines:** ~700
|
||
**Review Risk:** High
|
||
|
||
---
|
||
|
||
### NC-PR3-001: Add bell icon to icon registry
|
||
|
||
**Description:**
|
||
Register the Phosphor `Bell` icon in the frontend icon registry under the name `"bell"`.
|
||
|
||
**Files to modify:**
|
||
- `apps/web/src/utils/icons.ts`
|
||
|
||
**Acceptance criteria:**
|
||
- [ ] `"bell"` added to `IconName` union type.
|
||
- [ ] `bell: Bell` added to `iconRegistry` map.
|
||
- [ ] `Bell` imported from `@phosphor-icons/react`.
|
||
- [ ] `<Icon name="bell" />` renders without error in a quick manual check.
|
||
|
||
**Estimated effort:** Small (30 minutes)
|
||
**Dependencies:** None (can be prepared before PR-2 merges)
|
||
|
||
---
|
||
|
||
### NC-PR3-002: [RED] Write useNotifications hook tests
|
||
|
||
**Description:**
|
||
Write failing tests for the `useNotifications` hook covering state exposure, optimistic updates, and revert behavior.
|
||
|
||
**Files to modify:**
|
||
- `apps/web/src/hooks/use-notifications.test.ts` *(new)*
|
||
|
||
**Acceptance criteria:**
|
||
- [ ] `test_returns_notifications_and_unreadCount_from_context`: mock provider value; assert hook returns same array and count.
|
||
- [ ] `test_optimistically_updates_on_markRead`: call `markRead`; assert local `read_at` set and `unreadCount` decremented before API resolves.
|
||
- [ ] `test_reverts_optimistic_update_on_markRead_failure`: mock API rejection; assert state reverted.
|
||
- [ ] `test_optimistically_updates_on_dismiss`: call `dismiss`; assert item removed and count decremented.
|
||
- [ ] `test_reverts_optimistic_update_on_dismiss_failure`: mock API rejection; assert item restored.
|
||
- [ ] `test_calls_refreshList_when_invoked`: assert `GET /notifications` called.
|
||
|
||
**Estimated effort:** Small (2–3 hours)
|
||
**Dependencies:** NC-PR3-001
|
||
|
||
---
|
||
|
||
### NC-PR3-003: [GREEN] Implement NotificationProvider context with polling
|
||
|
||
**Description:**
|
||
Create the `NotificationProvider` React context that polls the backend endpoints, manages notification list and unread count, and handles tab visibility pause/resume.
|
||
|
||
**Files to modify:**
|
||
- `apps/web/src/state/notifications.tsx` *(new)*
|
||
|
||
**Acceptance criteria:**
|
||
- [ ] Context maintains `notifications: NotificationItem[]` and `unreadCount: number`.
|
||
- [ ] Polls `GET /notifications/unread` every 15 seconds.
|
||
- [ ] Polls `GET /notifications` every 30 seconds when dropdown is closed.
|
||
- [ ] Pauses all polling when `document.hidden` is true; resumes on visible.
|
||
- [ ] On dropdown open: immediately fetches list, pauses 30s list poll.
|
||
- [ ] On dropdown close: restarts 30s list poll.
|
||
- [ ] On logout: stops polling and clears state.
|
||
- [ ] Polling errors are silently logged; next cycle proceeds.
|
||
- [ ] On `401` response: stops all polling.
|
||
|
||
**Estimated effort:** Medium (4–5 hours)
|
||
**Dependencies:** NC-PR3-002
|
||
|
||
---
|
||
|
||
### NC-PR3-004: [GREEN] Implement useNotifications hook
|
||
|
||
**Description:**
|
||
Create the `useNotifications()` consumer hook that exposes state and mutation callbacks with optimistic updates.
|
||
|
||
**Files to modify:**
|
||
- `apps/web/src/hooks/use-notifications.ts` *(new)*
|
||
|
||
**Acceptance criteria:**
|
||
- [ ] Hook returns `notifications`, `unreadCount`, `isLoading`, `error`, `markRead`, `markAllRead`, `dismiss`, `refreshList`.
|
||
- [ ] `markRead(id)`: optimistically sets `read_at` and decrements `unreadCount`; calls `PATCH /notifications/{id}/read`; reverts on failure.
|
||
- [ ] `markAllRead()`: optimistically sets `read_at` on all items and `unreadCount=0`; calls `POST /notifications/mark-all-read`; reverts on failure.
|
||
- [ ] `dismiss(id)`: optimistically removes item and decrements `unreadCount` if unread; calls `DELETE /notifications/{id}`; reverts on failure.
|
||
- [ ] `refreshList()`: calls `GET /notifications` and updates state.
|
||
- [ ] Errors are surfaced as `error` state but not thrown.
|
||
- [ ] `NC-PR3-002` tests pass.
|
||
|
||
**Estimated effort:** Medium (3–4 hours)
|
||
**Dependencies:** NC-PR3-003
|
||
|
||
---
|
||
|
||
### NC-PR3-005: [TRIANGULATE] Hook edge-case and error handling tests
|
||
|
||
**Description:**
|
||
Add tests for 401 handling, polling pause, and multiple rapid mutations.
|
||
|
||
**Files to modify:**
|
||
- `apps/web/src/hooks/use-notifications.test.ts`
|
||
|
||
**Acceptance criteria:**
|
||
- [ ] `test_stops_polling_on_401`: simulate 401; assert polling intervals cleared.
|
||
- [ ] `test_pauses_polling_when_document_hidden`: simulate `visibilitychange` to hidden; assert `clearInterval` called.
|
||
- [ ] `test_resumes_polling_when_document_visible`: simulate hidden then visible; assert intervals restarted and immediate fetches fired.
|
||
- [ ] `test_multiple_markRead_calls_decrement_correctly`: mark 3 items read rapidly; assert `unreadCount` decrements by 3.
|
||
|
||
**Estimated effort:** Small (2–3 hours)
|
||
**Dependencies:** NC-PR3-004
|
||
|
||
---
|
||
|
||
### NC-PR3-006: [RED] Write NotificationItem component tests
|
||
|
||
**Description:**
|
||
Write failing render tests for the `NotificationItem` presentational component.
|
||
|
||
**Files to modify:**
|
||
- `apps/web/src/components/notification-item.test.tsx` *(new)*
|
||
|
||
**Acceptance criteria:**
|
||
- [ ] `test_displays_title_and_relative_time`: render with sample data; assert title and relative time visible.
|
||
- [ ] `test_applies_unread_styling_when_read_at_is_null`: assert unread CSS class present.
|
||
- [ ] `test_applies_read_styling_when_read_at_is_set`: assert read CSS class present.
|
||
- [ ] `test_calls_onMarkRead_when_mark_read_clicked`: simulate click; assert callback with correct id.
|
||
- [ ] `test_calls_onDismiss_when_dismiss_clicked`: simulate click; assert callback with correct id.
|
||
- [ ] `test_displays_severity_icon`: assert severity icon element present.
|
||
|
||
**Estimated effort:** Small (2–3 hours)
|
||
**Dependencies:** NC-PR3-001
|
||
|
||
---
|
||
|
||
### NC-PR3-007: [GREEN] Implement NotificationItem component
|
||
|
||
**Description:**
|
||
Build the presentational row component for a single notification.
|
||
|
||
**Files to modify:**
|
||
- `apps/web/src/components/notification-item.tsx` *(new)*
|
||
|
||
**Acceptance criteria:**
|
||
- [ ] Accepts `notification: NotificationItem`, `onMarkRead: (id: string) => void`, `onDismiss: (id: string) => void`.
|
||
- [ ] Displays severity icon mapped from `severity` to Phosphor icon (`Info`, `Warning`, `XCircle`, `CheckCircle`).
|
||
- [ ] Displays `title` and relative timestamp (e.g., "2m ago").
|
||
- [ ] Unread rows have `.notification-item--unread` class (bolder text, accent border, background tint).
|
||
- [ ] Read rows have `.notification-item--read` class (reduced opacity).
|
||
- [ ] Renders "Mark read" and "Dismiss" action buttons.
|
||
- [ ] `NC-PR3-006` tests pass.
|
||
|
||
**Estimated effort:** Small (3–4 hours)
|
||
**Dependencies:** NC-PR3-006
|
||
|
||
---
|
||
|
||
### NC-PR3-008: [RED] Write NotificationCenter component tests
|
||
|
||
**Description:**
|
||
Write failing render and interaction tests for the `NotificationCenter` component.
|
||
|
||
**Files to modify:**
|
||
- `apps/web/src/components/notification-center.test.tsx` *(new)*
|
||
|
||
**Acceptance criteria:**
|
||
- [ ] `test_renders_bell_icon`: assert bell icon visible.
|
||
- [ ] `test_shows_badge_when_unread_count_gt_0`: provider state `unreadCount=3`; assert badge text is "3".
|
||
- [ ] `test_hides_badge_when_unread_count_is_0`: assert badge not in document.
|
||
- [ ] `test_opens_dropdown_on_bell_click`: simulate click; assert dropdown panel visible.
|
||
- [ ] `test_closes_dropdown_on_outside_click`: open dropdown; click outside; assert panel not visible.
|
||
- [ ] `test_closes_dropdown_on_escape`: open dropdown; fire `Escape` key; assert panel not visible.
|
||
- [ ] `test_renders_empty_state_when_no_notifications`: assert empty state text visible.
|
||
- [ ] `test_renders_notification_items`: list has 2 items; assert 2 `NotificationItem` components rendered.
|
||
- [ ] `test_calls_markAllRead_on_footer_button_click`: simulate click; assert mock called.
|
||
- [ ] `test_refreshes_list_immediately_on_open`: open dropdown; assert `refreshList` mock called.
|
||
|
||
**Estimated effort:** Small (3–4 hours)
|
||
**Dependencies:** NC-PR3-007
|
||
|
||
---
|
||
|
||
### NC-PR3-009: [GREEN] Implement NotificationCenter component
|
||
|
||
**Description:**
|
||
Build the `NotificationCenter` component: bell icon with badge, dropdown panel with list, empty state, footer actions, outside-click/Escape close, and mobile terminal hiding.
|
||
|
||
**Files to modify:**
|
||
- `apps/web/src/components/notification-center.tsx` *(new)*
|
||
|
||
**Acceptance criteria:**
|
||
- [ ] Renders bell icon (`<Icon name="bell" />`).
|
||
- [ ] Shows unread count badge when `unreadCount > 0`; caps display at "99+".
|
||
- [ ] Badge uses existing `nav-badge` CSS class.
|
||
- [ ] Dropdown opens on bell click, closes on outside click or `Escape`.
|
||
- [ ] Dropdown is a positioned panel below the bell, right-aligned.
|
||
- [ ] Contains scrollable list of `NotificationItem` components.
|
||
- [ ] Shows empty state message when list is empty (e.g., "No notifications").
|
||
- [ ] Footer has "Mark all as read" button calling `markAllRead()`.
|
||
- [ ] Calls `refreshList()` immediately when opening.
|
||
- [ ] Hidden when `isMobileTerminal` is true.
|
||
- [ ] Uses `useNotifications()` hook.
|
||
- [ ] `NC-PR3-008` tests pass.
|
||
|
||
**Estimated effort:** Medium (4–5 hours)
|
||
**Dependencies:** NC-PR3-008
|
||
|
||
---
|
||
|
||
### NC-PR3-010: Add notification CSS styles
|
||
|
||
**Description:**
|
||
Add utility classes for the notification dropdown, items, badge, and empty state to `styles.css`.
|
||
|
||
**Files to modify:**
|
||
- `apps/web/src/styles.css`
|
||
|
||
**Acceptance criteria:**
|
||
- [ ] `.notification-dropdown` has absolute positioning, `z-index` above header, `max-height`, scroll, shadow, and matches light/dark theme variables.
|
||
- [ ] `.notification-item` has padding, border-bottom, hover state.
|
||
- [ ] `.notification-item--unread` has distinct styling (accent left border, slightly different background).
|
||
- [ ] `.notification-item--read` has reduced opacity.
|
||
- [ ] `.notification-badge` reuses or extends existing `nav-badge` styles.
|
||
- [ ] `.notification-empty` has centered text and muted color.
|
||
- [ ] Styles work in both light and dark themes.
|
||
|
||
**Estimated effort:** Small (2–3 hours)
|
||
**Dependencies:** NC-PR3-009
|
||
|
||
---
|
||
|
||
### NC-PR3-011: Integrate NotificationCenter into AppShell
|
||
|
||
**Description:**
|
||
Mount `<NotificationCenter />` inside the `AppShell` `header-actions` area. Wrap with `NotificationProvider` at the appropriate level.
|
||
|
||
**Files to modify:**
|
||
- `apps/web/src/components/app-shell.tsx`
|
||
|
||
**Acceptance criteria:**
|
||
- [ ] `<NotificationProvider>` wraps the authenticated app layout (inside or alongside `EventProvider`).
|
||
- [ ] `<NotificationCenter />` rendered inside `header-actions` div, before the user chip.
|
||
- [ ] Component is hidden when `isMobileTerminal` is true.
|
||
- [ ] No visual regressions in existing header layout.
|
||
- [ ] Existing tests for `AppShell` still pass (or are updated if needed).
|
||
|
||
**Estimated effort:** Small (1–2 hours)
|
||
**Dependencies:** NC-PR3-009, NC-PR3-010
|
||
|
||
---
|
||
|
||
### NC-PR3-012: [REFACTOR] Frontend code quality and type check pass
|
||
|
||
**Description:**
|
||
Run `npm run typecheck`, `npm run lint`, and frontend tests. Fix any errors. Verify accessibility (keyboard navigation, ARIA labels).
|
||
|
||
**Files to modify:**
|
||
- Any files with type/lint issues.
|
||
|
||
**Acceptance criteria:**
|
||
- [ ] `npm run typecheck` passes with zero errors.
|
||
- [ ] `npm run lint` passes with zero errors.
|
||
- [ ] `npm test` (or `vitest run`) passes for all new test files.
|
||
- [ ] Bell icon has `aria-label="Notifications"`.
|
||
- [ ] Dropdown panel has `role="menu"` or `role="dialog"` and appropriate `aria-*` attributes.
|
||
- [ ] Mark read / dismiss buttons have accessible labels.
|
||
- [ ] No `console.log` left from development.
|
||
|
||
**Estimated effort:** Small (1–2 hours)
|
||
**Dependencies:** NC-PR3-011
|
||
|
||
---
|
||
|
||
## PR-4: Toast Coordination
|
||
|
||
**Goal:** Update the toast bridge to respect notification preferences, extend settings UI for preference controls, and verify coordination end-to-end.
|
||
|
||
**Estimated Lines:** ~250
|
||
**Review Risk:** Low
|
||
|
||
---
|
||
|
||
### NC-PR4-001: Extend toast-rules.ts with category/severity mapping
|
||
|
||
**Description:**
|
||
Add `mapEventToCategory` and `mapEventToSeverity` functions to `toast-rules.ts` so the bridge can evaluate events against user preferences.
|
||
|
||
**Files to modify:**
|
||
- `apps/web/src/components/toast-rules.ts`
|
||
|
||
**Acceptance criteria:**
|
||
- [ ] `mapEventToCategory(event)` returns `"instance"` for `instance.*` events, `"health"` for `health.*`, `"system"` otherwise.
|
||
- [ ] `mapEventToSeverity(event)` returns `"error"` for `instance.error` / `health.error`; `"warning"` for unhealthy health changes; `"info"` for created/started/stopped/restarted/deleted; `"success"` for recovery to running.
|
||
- [ ] Functions are pure and exported.
|
||
- [ ] Existing toast mapping behavior is preserved (no regressions in current tests).
|
||
- [ ] New functions have unit tests.
|
||
|
||
**Estimated effort:** Small (2–3 hours)
|
||
**Dependencies:** PR-3 merged (frontend types available)
|
||
|
||
---
|
||
|
||
### NC-PR4-002: [RED] Write EventToastBridge preference check tests
|
||
|
||
**Description:**
|
||
Write failing tests for the updated `EventToastBridge` that verify preference-based toast suppression.
|
||
|
||
**Files to modify:**
|
||
- `apps/web/src/components/event-toast-bridge.test.tsx` *(new)*
|
||
|
||
**Acceptance criteria:**
|
||
- [ ] `test_shows_toast_when_level_is_all_and_category_not_muted`: assert toast shown.
|
||
- [ ] `test_suppresses_toast_when_level_is_none`: assert no toast.
|
||
- [ ] `test_suppresses_info_toast_when_level_is_errors`: event severity `info`; assert no toast.
|
||
- [ ] `test_shows_error_toast_when_level_is_errors`: event severity `error`; assert toast shown.
|
||
- [ ] `test_suppresses_toast_when_category_is_muted`: config mute contains event category; assert no toast.
|
||
- [ ] Tests mock `useEventContext`, user config context, and `toast-rules.ts` as needed.
|
||
|
||
**Estimated effort:** Small (2–3 hours)
|
||
**Dependencies:** NC-PR4-001
|
||
|
||
---
|
||
|
||
### NC-PR4-003: [GREEN] Update EventToastBridge with preference checks
|
||
|
||
**Description:**
|
||
Modify `EventToastBridge` to read `notification_toast_level` and `notification_mute_categories` from user config and skip toasts based on the preference hierarchy.
|
||
|
||
**Files to modify:**
|
||
- `apps/web/src/components/event-toast-bridge.tsx`
|
||
|
||
**Acceptance criteria:**
|
||
- [ ] Bridge reads user config (from existing settings API / context).
|
||
- [ ] Evaluation order: mute categories first, then toast level.
|
||
- [ ] If `notification_toast_level === "none"`: no toasts shown.
|
||
- [ ] If `notification_toast_level === "errors"`: only toasts for `severity === "error"`.
|
||
- [ ] If `notification_toast_level === "all"`: toasts shown as before.
|
||
- [ ] If event category is in `notification_mute_categories`: toast suppressed.
|
||
- [ ] Backend notification creation is unaffected; bridge only controls toast surfacing.
|
||
- [ ] `NC-PR4-002` tests pass.
|
||
|
||
**Estimated effort:** Small (2–3 hours)
|
||
**Dependencies:** NC-PR4-002
|
||
|
||
---
|
||
|
||
### NC-PR4-004: [TRIANGULATE] Bridge edge-case and integration tests
|
||
|
||
**Description:**
|
||
Add tests for preference changes taking effect immediately, mixed mute + level constraints, and no regressions in existing deduplication.
|
||
|
||
**Files to modify:**
|
||
- `apps/web/src/components/event-toast-bridge.test.tsx`
|
||
|
||
**Acceptance criteria:**
|
||
- [ ] `test_preference_change_is_immediate`: config changes from `"all"` to `"none"`; next event suppressed.
|
||
- [ ] `test_muted_category_overrides_all_level`: level `"all"` but category muted; toast suppressed.
|
||
- [ ] `test_deduplication_still_works_with_preferences`: two identical allowed events within 1s → one toast.
|
||
- [ ] `test_unmapped_event_defaults_to_info`: unknown event type → category `"system"`, severity `"info"`.
|
||
|
||
**Estimated effort:** Small (1–2 hours)
|
||
**Dependencies:** NC-PR4-003
|
||
|
||
---
|
||
|
||
### NC-PR4-005: Extend settings UI with notification preferences
|
||
|
||
**Description:**
|
||
Add notification preference controls to the existing General settings tab: a multi-select/checkbox group for mute categories and a select for toast level.
|
||
|
||
**Files to modify:**
|
||
- `apps/web/src/pages/settings.tsx`
|
||
- `apps/web/src/api/settings.ts`
|
||
|
||
**Acceptance criteria:**
|
||
- [ ] `UserConfig` interface in `api/settings.ts` includes `notification_mute_categories?: string[]` and `notification_toast_level?: "all" | "errors" | "none"`.
|
||
- [ ] `UserConfigUpdate` interface includes the same optional fields.
|
||
- [ ] General settings tab has a "Notifications" section.
|
||
- [ ] Toast level select with options: "All", "Errors only", "None".
|
||
- [ ] Mute categories checkboxes for known categories: `instance`, `system`, `health`, `security`.
|
||
- [ ] Preferences save via existing `updateUserConfig` API.
|
||
- [ ] Saved preferences persist after page reload.
|
||
- [ ] Default values: `notification_toast_level="all"`, `notification_mute_categories=[]`.
|
||
|
||
**Estimated effort:** Small (3–4 hours)
|
||
**Dependencies:** NC-PR4-003
|
||
|
||
---
|
||
|
||
### NC-PR4-006: [REFACTOR] Final quality pass and verification
|
||
|
||
**Description:**
|
||
Run full frontend type check, lint, and test suite. Do a manual smoke test of the notification center + toast coordination.
|
||
|
||
**Files to modify:**
|
||
- Any files with issues found.
|
||
|
||
**Acceptance criteria:**
|
||
- [ ] `npm run typecheck` passes.
|
||
- [ ] `npm run lint` passes.
|
||
- [ ] `npm test` passes for all new and modified test files.
|
||
- [ ] Manual smoke test: trigger an `instance.error` event → notification appears in dropdown → toast appears (if level="all") → mark read → badge clears.
|
||
- [ ] Manual smoke test: set toast level to "none" → trigger event → no toast appears, but notification still created.
|
||
- [ ] No regressions in existing settings page functionality.
|
||
|
||
**Estimated effort:** Small (1–2 hours)
|
||
**Dependencies:** NC-PR4-004, NC-PR4-005
|
||
|
||
---
|
||
|
||
## Dependency Graph (PR Level)
|
||
|
||
```
|
||
PR-1: Backend Core
|
||
│
|
||
├─► NC-PR1-001 ──► NC-PR1-002
|
||
│
|
||
├─► NC-PR1-003 ──► NC-PR1-004 ──► NC-PR1-005
|
||
│
|
||
├─► NC-PR1-006 ──► NC-PR1-007 ──► NC-PR1-008
|
||
│
|
||
├─► NC-PR1-009
|
||
│
|
||
└─► NC-PR1-010
|
||
|
||
PR-2: Backend Integration (depends on PR-1 merged)
|
||
│
|
||
├─► NC-PR2-001
|
||
│
|
||
├─► NC-PR2-002
|
||
│
|
||
├─► NC-PR2-003
|
||
│
|
||
├─► NC-PR2-004
|
||
│
|
||
└─► NC-PR2-005
|
||
|
||
PR-3: Frontend Core (depends on PR-1/PR-2 merged)
|
||
│
|
||
├─► NC-PR3-001
|
||
│
|
||
├─► NC-PR3-002 ──► NC-PR3-003 ──► NC-PR3-004 ──► NC-PR3-005
|
||
│
|
||
├─► NC-PR3-006 ──► NC-PR3-007
|
||
│
|
||
├─► NC-PR3-008 ──► NC-PR3-009 ──► NC-PR3-010 ──► NC-PR3-011
|
||
│
|
||
└─► NC-PR3-012
|
||
|
||
PR-4: Toast Coordination (depends on PR-3 merged)
|
||
│
|
||
├─► NC-PR4-001
|
||
│
|
||
├─► NC-PR4-002 ──► NC-PR4-003 ──► NC-PR4-004
|
||
│
|
||
├─► NC-PR4-005
|
||
│
|
||
└─► NC-PR4-006
|
||
```
|
||
|
||
---
|
||
|
||
## Task Summary
|
||
|
||
| PR | Task ID | Description | TDD Phase | Effort |
|
||
|----|---------|-------------|-----------|--------|
|
||
| 1 | NC-PR1-001 | Alembic migration for notifications table | — | S |
|
||
| 1 | NC-PR1-002 | SQLAlchemy Notification model and export | — | S |
|
||
| 1 | NC-PR1-003 | Service unit tests — basic CRUD | RED | S |
|
||
| 1 | NC-PR1-004 | Implement NotificationService | GREEN | M |
|
||
| 1 | NC-PR1-005 | Service edge-case and isolation tests | TRIANGULATE | S |
|
||
| 1 | NC-PR1-006 | API integration tests — basic endpoints | RED | S |
|
||
| 1 | NC-PR1-007 | Implement FastAPI router and Pydantic schemas | GREEN | M |
|
||
| 1 | NC-PR1-008 | API edge-case and ownership tests | TRIANGULATE | S |
|
||
| 1 | NC-PR1-009 | Register router in main.py | — | S |
|
||
| 1 | NC-PR1-010 | Backend code quality and type safety pass | REFACTOR | S |
|
||
| 2 | NC-PR2-001 | Wire lifecycle_hooks.py to NotificationService | — | S |
|
||
| 2 | NC-PR2-002 | Wire health_monitor.py to NotificationService | — | S |
|
||
| 2 | NC-PR2-003 | Extend UserConfig schema for preferences | — | S |
|
||
| 2 | NC-PR2-004 | Event producer integration tests | RED | M |
|
||
| 2 | NC-PR2-005 | Verify producer tests and clean up | GREEN / REFACTOR | S |
|
||
| 3 | NC-PR3-001 | Add bell icon to icon registry | — | S |
|
||
| 3 | NC-PR3-002 | useNotifications hook tests | RED | S |
|
||
| 3 | NC-PR3-003 | Implement NotificationProvider context | GREEN | M |
|
||
| 3 | NC-PR3-004 | Implement useNotifications hook | GREEN | M |
|
||
| 3 | NC-PR3-005 | Hook edge-case and error handling tests | TRIANGULATE | S |
|
||
| 3 | NC-PR3-006 | NotificationItem component tests | RED | S |
|
||
| 3 | NC-PR3-007 | Implement NotificationItem component | GREEN | S |
|
||
| 3 | NC-PR3-008 | NotificationCenter component tests | RED | S |
|
||
| 3 | NC-PR3-009 | Implement NotificationCenter component | GREEN | M |
|
||
| 3 | NC-PR3-010 | Add notification CSS styles | — | S |
|
||
| 3 | NC-PR3-011 | Integrate NotificationCenter into AppShell | — | S |
|
||
| 3 | NC-PR3-012 | Frontend code quality and type check pass | REFACTOR | S |
|
||
| 4 | NC-PR4-001 | Extend toast-rules.ts with mapping | — | S |
|
||
| 4 | NC-PR4-002 | EventToastBridge preference check tests | RED | S |
|
||
| 4 | NC-PR4-003 | Update EventToastBridge with preference checks | GREEN | S |
|
||
| 4 | NC-PR4-004 | Bridge edge-case and integration tests | TRIANGULATE | S |
|
||
| 4 | NC-PR4-005 | Extend settings UI with notification preferences | — | S |
|
||
| 4 | NC-PR4-006 | Final quality pass and verification | REFACTOR | S |
|
||
|
||
**Total tasks:** 31
|
||
**Total estimated effort:** ~100 hours (backend ~40h, frontend ~45h, integration ~15h)
|