alex/headquarter
1
1
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity
Files
dev-clean-base
headquarter/openspec/changes/notification-center/apply-progress.md
T
alex 19242b4152 feat: notification center toast coordination (PR-4) 
- EventToastBridge checks notification_toast_level and notification_mute_categories
- toast-rules.ts: event-to-category/severity mapping functions
- Settings page: notification preferences section (toast level dropdown, mute checkboxes)
- Settings API types extended with notification preference fields
- 17 frontend tests (toast-rules + bridge)
- Preference hierarchy: mute categories → toast level → show/hide

Quality gates: vitest 17 passed, tsc clean, eslint clean
2026-05-29 13:52:49 +02:00

272 lines
19 KiB
Markdown
Raw Permalink Blame History

# Apply Progress: Notification Center
## TDD Cycle Evidence (PR-1)
| Cycle | Task | Test File | RED | GREEN | Evidence |
|-------|------|-----------|-----|-------|----------|
| 1 | NC-PR1-004 (basic CRUD) | `tests/unit/test_notification_service.py` | 13 tests written against missing service | All 13 pass | `pytest tests/unit/test_notification_service.py` → 13 passed |
| 2 | NC-PR1-005 (edge cases) | `tests/unit/test_notification_service.py` | Already included in cycle 1 | Added wrong-owner, mute-categories, cross-user isolation | Same 13 tests pass |
| 3 | NC-PR1-007 (basic endpoints) | `tests/integration/test_notifications_api.py` | 10 tests written against missing router | All 10 pass | `pytest tests/integration/test_notifications_api.py` → 10 passed |
| 4 | NC-PR1-008 (API edge cases) | `tests/integration/test_notifications_api.py` | Already included in cycle 3 | Pagination, 404 ownership, mute categories at API layer | Same 10 tests pass |
| 5 | NC-PR1-010 (REFACTOR) | All files | — | ruff clean, no regressions | `ruff check` passes on all new files; existing unit tests 223 passed (4 pre-existing failures unrelated) |
## TDD Cycle Evidence (PR-2)
| Cycle | Task | Test File | RED | GREEN | Evidence |
|-------|------|-----------|-----|-------|----------|
| 1 | NC-PR2-004 (producer flow) | `tests/integration/test_notifications_lifecycle.py` | 4 tests written against unwired producers | Wired `lifecycle_hooks.py` and `health_monitor.py` | 4 passed |
| 2 | NC-PR2-004 (severity mapping) | `tests/integration/test_notifications_lifecycle.py` | Added error + warning severity tests | Already green from implementation | 6 passed |
| 3 | NC-PR2-003 (UserConfig schema) | `src/api/user_config.py` | Schema extended with new optional fields | PATCH/GET endpoints validate correctly | Verified manually |
| 4 | NC-PR2-005 (REFACTOR) | All files | — | ruff clean, no regressions across 41 related tests | All pass |
## TDD Cycle Evidence (PR-3)
| Cycle | Task | Test File | RED | GREEN | Evidence |
|-------|------|-----------|-----|-------|----------|
| 1 | NC-PR3-009 (hook tests) | `src/hooks/use-notifications.test.tsx` | 9 tests written against stub provider/hook | Implemented `NotificationProvider` + `useNotifications` | 9 passed |
| 2 | NC-PR3-008 (component tests) | `src/components/notification-center.test.tsx` | 10 tests written against stub component | Implemented `NotificationCenter` + `NotificationItem` | 10 passed |
| 3 | NC-PR3-005 (NotificationItem tests) | `src/components/notification-item.test.tsx` | 6 tests written against stub component | Implemented `NotificationItem` | 6 passed |
| 4 | NC-PR3-012 (REFACTOR) | All files | — | `tsc --noEmit` clean, `eslint` clean | Zero errors |
## Completed Tasks
### PR-1: Backend Core
- [x] NC-PR1-001: Alembic migration for `notifications` table
- [x] NC-PR1-002: SQLAlchemy `Notification` model (`apps/api/src/models/notification.py`)
- [x] NC-PR1-003: Export `Notification` in `models/__init__.py`
- [x] NC-PR1-004: NotificationService unit tests — basic CRUD (RED)
- [x] NC-PR1-005: Implement `NotificationService` singleton (GREEN)
- [x] NC-PR1-006: Service edge-case and isolation tests (TRIANGULATE)
- [x] NC-PR1-007: API integration tests — basic endpoints (RED)
- [x] NC-PR1-008: Implement FastAPI router + Pydantic schemas (GREEN)
- [x] NC-PR1-009: API edge-case and ownership tests (TRIANGULATE)
- [x] NC-PR1-010: Register router in `main.py` + import `Notification` for Alembic
- [x] NC-PR1-011: Code quality pass — ruff, test regressions, smoke tests (REFACTOR)
### PR-2: Backend Integration
- [x] NC-PR2-001: Wire `lifecycle_hooks.py` to `NotificationService`
- [x] NC-PR2-002: Wire `health_monitor.py` to `NotificationService`
- [x] NC-PR2-003: Extend `UserConfig` schema for notification preferences
- [x] NC-PR2-004: Event producer integration tests (RED)
- [x] NC-PR2-005: Verify producer tests pass and clean up (GREEN / REFACTOR)
### PR-3: Frontend Core
- [x] NC-PR3-001: Add "bell" icon to icon registry (`apps/web/src/utils/icons.ts`, `apps/web/src/components/icon.tsx`)
- [x] NC-PR3-002: `NotificationProvider` context with polling (`apps/web/src/state/notifications.tsx`)
- [x] NC-PR3-003: `useNotifications()` hook (`apps/web/src/hooks/use-notifications.ts`)
- [x] NC-PR3-004: `NotificationCenter` component — bell + dropdown panel (`apps/web/src/components/notification-center.tsx`)
- [x] NC-PR3-005: `NotificationItem` component — single row (`apps/web/src/components/notification-item.tsx`)
- [x] NC-PR3-006: AppShell integration — mount `NotificationCenter` in header-actions (`apps/web/src/components/app-shell.tsx`)
- [x] NC-PR3-007: CSS styles for notification center (`apps/web/src/styles.css`)
- [x] NC-PR3-008: Component tests for `NotificationCenter` (`apps/web/src/components/notification-center.test.tsx`)
- [x] NC-PR3-009: Hook tests for `useNotifications` (`apps/web/src/hooks/use-notifications.test.tsx`)
## Files Changed
### PR-1 Files
1. `apps/api/alembic/versions/2026_05_29_add_notifications_table.py` *(new)* — Alembic migration
2. `apps/api/src/models/notification.py` *(new)* — SQLAlchemy model
3. `apps/api/src/models/__init__.py` — Export `Notification`
4. `apps/api/src/services/notification_service.py` *(new)* — `NotificationService` singleton
5. `apps/api/src/api/notifications.py` *(new)* — FastAPI router + Pydantic schemas
6. `apps/api/src/api/__init__.py` — Export `notifications_router`
7. `apps/api/src/main.py` — Register router, import `Notification` for Alembic
8. `apps/api/tests/unit/test_notification_service.py` *(new)* — 13 unit tests
9. `apps/api/tests/integration/test_notifications_api.py` *(new)* — 10 integration tests
10. `apps/api/tests/integration/test_models.py` — Updated expected tables list
### PR-2 Files
11. `apps/api/src/services/lifecycle_hooks.py` — Wired `NotificationService` after event bus publish
12. `apps/api/src/services/health_monitor.py` — Wired `NotificationService` after state change event publish
13. `apps/api/src/api/user_config.py` — Added `notification_mute_categories` and `notification_toast_level` to Pydantic schemas
14. `apps/api/tests/integration/test_notifications_lifecycle.py` *(new)* — 6 integration tests for event-to-notification flow
### PR-3 Files
15. `apps/web/src/api/notifications.ts` *(new)* — API client for notification endpoints
16. `apps/web/src/utils/icons.ts` — Added `"bell"` to `IconName` union and `iconRegistry`
17. `apps/web/src/components/icon.tsx` — Added `Bell` import and mapping
18. `apps/web/src/utils/time.ts` *(new)* — `formatRelativeTime` utility
19. `apps/web/src/state/notifications.tsx` *(new)* — `NotificationProvider` with polling, optimistic mutations, visibility pause
20. `apps/web/src/hooks/use-notifications.ts` *(new)* — `useNotifications` consumer hook
21. `apps/web/src/hooks/use-notifications.test.tsx` *(new)* — 9 hook tests (RED → GREEN)
22. `apps/web/src/components/notification-item.tsx` *(new)* — Presentational notification row
23. `apps/web/src/components/notification-item.test.tsx` *(new)* — 6 component tests (RED → GREEN)
24. `apps/web/src/components/notification-center.tsx` *(new)* — Bell icon, badge, dropdown panel
25. `apps/web/src/components/notification-center.test.tsx` *(new)* — 10 component tests (RED → GREEN)
26. `apps/web/src/styles.css` — Added notification center + item + dropdown CSS utilities
27. `apps/web/src/components/app-shell.tsx` — Mounted `NotificationProvider` and `NotificationCenter` in header-actions
## Test Commands & Exit Codes
### PR-1
```bash
# Unit tests for NotificationService (13 tests)
cd apps/api && python -m pytest tests/unit/test_notification_service.py -v
# Exit: 0 — 13 passed
# Integration tests for notifications API (10 tests)
cd apps/api && python -m pytest tests/integration/test_notifications_api.py -v
# Exit: 0 — 10 passed
# Existing unit tests (no regressions in our code)
cd apps/api && python -m pytest tests/unit/ -v
# Exit: 1 — 223 passed, 4 failed (pre-existing failures in test_config.py and test_git_repository_clone_preflight.py)
```
### PR-2
```bash
# New integration tests for event producers (6 tests)
cd apps/api && python -m pytest tests/integration/test_notifications_lifecycle.py -v
# Exit: 0 — 6 passed
# Combined relevant test suite (41 tests)
cd apps/api && python -m pytest \
tests/unit/test_notification_service.py \
tests/integration/test_notifications_api.py \
tests/integration/test_notifications_lifecycle.py \
tests/unit/test_health_monitor.py \
tests/integration/test_events.py \
-v
# Exit: 0 — 41 passed
# Ruff linting on all PR-2 modified files
cd apps/api && python -m ruff check \
src/services/lifecycle_hooks.py \
src/services/health_monitor.py \
src/api/user_config.py \
tests/integration/test_notifications_lifecycle.py
# Exit: 0 — All checks passed
```
### PR-3
```bash
# Hook tests (9 tests)
cd apps/web && npx vitest run src/hooks/use-notifications.test.tsx
# Exit: 0 — 9 passed
# NotificationItem tests (6 tests)
cd apps/web && npx vitest run src/components/notification-item.test.tsx
# Exit: 0 — 6 passed
# NotificationCenter tests (10 tests)
cd apps/web && npx vitest run src/components/notification-center.test.tsx
# Exit: 0 — 10 passed
# All new frontend tests combined (25 tests)
cd apps/web && npx vitest run src/hooks/use-notifications.test.tsx src/components/notification-item.test.tsx src/components/notification-center.test.tsx
# Exit: 0 — 25 passed
# Type check
cd apps/web && npx tsc --noEmit
# Exit: 0 — clean
# Lint new/modified files
cd apps/web && npx eslint src/api/notifications.ts src/state/notifications.tsx src/hooks/use-notifications.ts src/components/notification-item.tsx src/components/notification-center.tsx src/components/app-shell.tsx src/utils/icons.ts src/components/icon.tsx src/utils/time.ts src/hooks/use-notifications.test.tsx src/components/notification-item.test.tsx src/components/notification-center.test.tsx --ext ts,tsx
# Exit: 0 — clean
```
## Deviations from Design
### PR-1
- **SQLAlchemy `metadata` column name conflict:** `Base.metadata` is reserved by SQLAlchemy DeclarativeBase. Used `notification_metadata` as the Python attribute name with DB column name `"metadata"`. In the Pydantic response model, used `Field(serialization_alias="metadata")` so the JSON API still exposes `metadata` as specified.
- **`created_at` type in Pydantic:** Used `datetime` instead of `str` to leverage FastAPI's automatic ISO serialization.
### PR-2
- None. All mappings and behaviors match the design spec (section 1.3) and task requirements exactly.
### PR-3
- **Polling interval management:** The provider uses two `useEffect` hooks plus `startPolling`/`stopPolling` helpers. A race condition between the dropdown effect and the initial start effect in React Strict Mode was discovered and fixed by requiring `unreadIntervalRef.current` to be truthy before the dropdown effect resumes list polling. This ensures `startPolling` always owns initial list fetch.
- **`formatRelativeTime` utility:** Design did not specify a relative-time formatter. Added a lightweight custom utility (`apps/web/src/utils/time.ts`) rather than installing a date library, per the constraint not to add npm packages.
## Surprises / Decisions
### PR-1
1. **SQLite `func.now()` resolution:** `test_list_notifications_orders_by_created_at_desc` failed because multiple rapid INSERTs got identical timestamps. Fixed by explicitly setting `created_at` offsets in the test after creation.
2. **Pre-existing integration test failures:** ~40 integration tests fail due to missing `asyncpg` module and direct PostgreSQL connection attempts in their custom setup code. These are unrelated to our changes.
3. **Pre-existing `test_models.py` outdated:** The `test_expected_tables_are_registered` assertion had a hardcoded set missing many newer tables. Updated it to include all current tables (including `notifications`).
### PR-2
1. **Health monitor `test_health_monitor_unhealthy_creates_warning_notification` required `public_url`:** The health monitor only checks tunnel health when `instance.public_url` is truthy. Without setting it on the test fixture instance, `_derive_status` returned `"running"` instead of `"unhealthy"`, which created an `"info"` notification. Fixed by setting `test_instance.public_url` in the test before calling `_check_instance`.
2. **Patch target for failure test:** The `test_notification_failure_does_not_block_event_pipeline` patches `src.services.lifecycle_hooks.notification_service.create_notification`. This only works because `lifecycle_hooks.py` imports `notification_service` at module level, making the attribute resolvable by `unittest.mock.patch`.
3. **No schema migration needed for UserConfig:** Preferences are stored in the existing JSON `config` blob, consistent with the existing pattern (theme, editor, git identity). No Alembic migration required.
### PR-3
1. **React Strict Mode interval race:** In `NotificationProvider`, the dropdown `useEffect` was setting the list poll interval before the initial-start `useEffect` called `startPolling`, which caused `startPolling` to skip its initial `fetchList()` call. Fixed by adding `&& unreadIntervalRef.current` to the dropdown effect's resume branch, so it only resumes an already-active polling session.
2. **`toBeInTheDocument` type issues in tests:** Testing-library jest-dom matchers type definitions were not automatically picked up in `.test.tsx` files. The tests run and pass at runtime; the TypeScript LSP warnings are cosmetic and do not block compilation or execution.
3. **No npm packages installed:** All frontend work was done with existing dependencies (`@phosphor-icons/react`, `react`, etc.). Relative time formatting was implemented with a 20-line custom utility rather than adding `date-fns` or similar.
## TDD Cycle Evidence (PR-4)
| Cycle | Task | Test File | RED | GREEN | Evidence |
|-------|------|-----------|-----|-------|----------|
| 1 | NC-PR4-001 (toast-rules mapping) | `src/components/toast-rules.test.ts` | 8 tests written against missing functions | Added `mapEventToCategory` + `mapEventToSeverity` | `npx vitest run src/components/toast-rules.test.ts` → 8 passed |
| 2 | NC-PR4-002 (bridge preference tests) | `src/components/event-toast-bridge.test.tsx` | 5 tests written against bridge without preference logic | Updated `EventToastBridge` with config fetch + preference checks | `npx vitest run src/components/event-toast-bridge.test.tsx` → 5 passed |
| 3 | NC-PR4-004 (edge-case tests) | `src/components/event-toast-bridge.test.tsx` | Added immediate preference change, mute override, dedup, unmapped event tests | Already green from implementation | `npx vitest run src/components/event-toast-bridge.test.tsx` → 9 passed |
| 4 | NC-PR4-005 (settings UI) | `src/pages/settings.tsx` | — | Added notification controls + `UserConfig` type extension | `npx tsc --noEmit` clean, `npx eslint` clean |
| 5 | NC-PR4-006 (REFACTOR) | All files | — | Full type check, lint, and regression test | 17 new tests pass; 25 existing tests pass; zero lint/type errors |
## Completed Tasks
### PR-4: Toast Coordination
- [x] NC-PR4-001: Extend `toast-rules.ts` with `mapEventToCategory` and `mapEventToSeverity`
- [x] NC-PR4-002: Write `EventToastBridge` preference check tests (RED)
- [x] NC-PR4-003: Update `EventToastBridge` with preference checks (GREEN)
- [x] NC-PR4-004: Bridge edge-case and integration tests (TRIANGULATE)
- [x] NC-PR4-005: Extend settings UI with notification preferences
- [x] NC-PR4-006: Final quality pass — type check, lint, regression tests (REFACTOR)
## Files Changed (PR-4)
1. `apps/web/src/components/toast-rules.ts` — Added `mapEventToCategory` and `mapEventToSeverity`
2. `apps/web/src/components/toast-rules.test.ts` *(new)* — 8 unit tests for mapping functions
3. `apps/web/src/components/event-toast-bridge.tsx` — Fetches user config, listens for `userconfig:updated`, checks preferences before showing toasts
4. `apps/web/src/components/event-toast-bridge.test.tsx` *(new)* — 9 tests for preference-based suppression, immediate updates, dedup, unmapped events
5. `apps/web/src/api/settings.ts` — Added `notification_toast_level` and `notification_mute_categories` to `UserConfig` / `UserConfigUpdate`
6. `apps/web/src/pages/settings.tsx` — Added notification preference controls (toast level select + mute category checkboxes), dispatches `userconfig:updated` on save
## Test Commands & Exit Codes (PR-4)
```bash
# Toast-rules mapping tests (8 tests)
cd apps/web && npx vitest run src/components/toast-rules.test.ts
# Exit: 0 — 8 passed
# EventToastBridge preference tests (9 tests)
cd apps/web && npx vitest run src/components/event-toast-bridge.test.tsx
# Exit: 0 — 9 passed
# All new PR-4 tests combined
cd apps/web && npx vitest run src/components/toast-rules.test.ts src/components/event-toast-bridge.test.tsx
# Exit: 0 — 17 passed
# Existing frontend tests (no regressions)
cd apps/web && npx vitest run src/hooks/use-notifications.test.tsx src/components/notification-item.test.tsx src/components/notification-center.test.tsx
# Exit: 0 — 25 passed
# Type check
cd apps/web && npx tsc --noEmit
# Exit: 0 — clean
# Lint on modified files
cd apps/web && npx eslint src/components/toast-rules.ts src/components/toast-rules.test.ts src/components/event-toast-bridge.tsx src/components/event-toast-bridge.test.tsx src/api/settings.ts src/pages/settings.tsx --ext ts,tsx
# Exit: 0 — clean
```
## Deviations from Design (PR-4)
- **No global UserConfig context:** The design assumed an existing user-config context. The frontend did not have one, so `EventToastBridge` fetches config on mount via `getUserConfig` and listens for a `userconfig:updated` `CustomEvent` dispatched by the settings page after a successful save. This achieves immediate preference updates without introducing a new provider.
## Surprises / Decisions (PR-4)
1. **Bridge processes events before config loads:** The initial `useEffect` in `EventToastBridge` could process events while `config` is still `null`. Fixed by initializing `config` to `null` and skipping the event-processing effect until config resolves. This prevents toasts from leaking before preferences are known.
2. **`UserConfig` type extended without breaking existing consumers:** Adding optional fields to `UserConfig` and `UserConfigUpdate` in `api/settings.ts` did not require changes to `sessions.tsx` or `dashboard.tsx` because they only import the API functions, not the types.
3. **Custom event for immediate updates:** Using `window.dispatchEvent(new CustomEvent("userconfig:updated", { detail: updated }))` in `settings.tsx` and listening in `event-toast-bridge.tsx` is consistent with the existing `refresh-file-tree` custom-event pattern used in `repo-workspace.tsx`.
## Remaining Tasks
- [x] All PR-4 tasks complete.
## PR Boundary
This progress covers PR-1, PR-2, PR-3, and PR-4. The Notification Center feature is fully implemented.
Reference in New Issue View Git Blame Copy Permalink