diff --git a/.planning/phases/04-ux-improvements/04-01-PLAN.md b/.planning/phases/04-ux-improvements/04-01-PLAN.md index e03f459..4ea92cd 100644 --- a/.planning/phases/04-ux-improvements/04-01-PLAN.md +++ b/.planning/phases/04-ux-improvements/04-01-PLAN.md @@ -133,12 +133,14 @@ mux.HandleFunc("/api/updates", srv.UpdatesHandler) - Task 1: Extend Store interface and implement AcknowledgeAll + AcknowledgeByTag in both stores - pkg/diunwebhook/store.go, pkg/diunwebhook/sqlite_store.go, pkg/diunwebhook/postgres_store.go + Task 1: Extend Store interface and implement AcknowledgeAll + AcknowledgeByTag with store-level tests + pkg/diunwebhook/store.go, pkg/diunwebhook/sqlite_store.go, pkg/diunwebhook/postgres_store.go, pkg/diunwebhook/diunwebhook_test.go, pkg/diunwebhook/export_test.go - pkg/diunwebhook/store.go - pkg/diunwebhook/sqlite_store.go - pkg/diunwebhook/postgres_store.go + - pkg/diunwebhook/diunwebhook_test.go + - pkg/diunwebhook/export_test.go - Test 1: AcknowledgeAll on empty DB returns count=0, no error @@ -149,22 +151,47 @@ mux.HandleFunc("/api/updates", srv.UpdatesHandler) - Test 6: AcknowledgeByTag does not affect updates in other tags or untagged updates - 1. Add two methods to the Store interface in `store.go` (per D-01): + TDD approach -- write tests first, then implement: + + 1. Add test helper exports to `export_test.go`: + ```go + func (s *Server) TestAcknowledgeAll() (int, error) { + return s.Store().AcknowledgeAll() + } + func (s *Server) TestAcknowledgeByTag(tagID int) (int, error) { + return s.Store().AcknowledgeByTag(tagID) + } + ``` + (Add a `Store() Store` accessor method on Server if not already present, or access the store field directly via an existing test export pattern.) + + 2. Write store-level tests in `diunwebhook_test.go` following existing `Test_` convention: + - `TestAcknowledgeAll_Empty`: create server, call TestAcknowledgeAll, assert count=0, no error + - `TestAcknowledgeAll_AllUnacknowledged`: upsert 3 events via TestUpsertEvent, call TestAcknowledgeAll, assert count=3, then call GetUpdates and verify all have acknowledged=true + - `TestAcknowledgeAll_MixedState`: upsert 3 events, acknowledge 1 via existing dismiss, call TestAcknowledgeAll, assert count=2 + - `TestAcknowledgeByTag_MatchingTag`: upsert 2 events, create tag, assign both to tag, call TestAcknowledgeByTag(tagID), assert count=2 + - `TestAcknowledgeByTag_NonExistentTag`: call TestAcknowledgeByTag(9999), assert count=0, no error + - `TestAcknowledgeByTag_OnlyAffectsTargetTag`: upsert 3 events, create 2 tags, assign 2 events to tag1 and 1 to tag2, call TestAcknowledgeByTag(tag1.ID), assert count=2, verify tag2's event is still unacknowledged via GetUpdates + + Run tests -- they must FAIL (RED) since methods don't exist yet. + + 3. Add two methods to the Store interface in `store.go` (per D-01): ```go AcknowledgeAll() (count int, err error) AcknowledgeByTag(tagID int) (count int, err error) ``` - 2. Implement in `sqlite_store.go` (following AcknowledgeUpdate pattern with mutex): + 4. Implement in `sqlite_store.go` (following AcknowledgeUpdate pattern with mutex): - `AcknowledgeAll`: `s.mu.Lock()`, `s.db.Exec("UPDATE updates SET acknowledged_at = datetime('now') WHERE acknowledged_at IS NULL")`, return `int(RowsAffected())` - `AcknowledgeByTag`: `s.mu.Lock()`, `s.db.Exec("UPDATE updates SET acknowledged_at = datetime('now') WHERE acknowledged_at IS NULL AND image IN (SELECT image FROM tag_assignments WHERE tag_id = ?)", tagID)`, return `int(RowsAffected())` - 3. Implement in `postgres_store.go` (no mutex, use NOW() and $1 positional param): + 5. Implement in `postgres_store.go` (no mutex, use NOW() and $1 positional param): - `AcknowledgeAll`: `s.db.Exec("UPDATE updates SET acknowledged_at = NOW() WHERE acknowledged_at IS NULL")`, return `int(RowsAffected())` - `AcknowledgeByTag`: `s.db.Exec("UPDATE updates SET acknowledged_at = NOW() WHERE acknowledged_at IS NULL AND image IN (SELECT image FROM tag_assignments WHERE tag_id = $1)", tagID)`, return `int(RowsAffected())` + + 6. Run tests again -- they must PASS (GREEN). - cd /home/jean-luc-makiola/Development/projects/DiunDashboard && go build ./... + cd /home/jean-luc-makiola/Development/projects/DiunDashboard && go test -v -run "TestAcknowledge(All|ByTag)_" ./pkg/diunwebhook/ - store.go contains `AcknowledgeAll() (count int, err error)` in the Store interface @@ -177,13 +204,15 @@ mux.HandleFunc("/api/updates", srv.UpdatesHandler) - postgres_store.go contains `func (s *PostgresStore) AcknowledgeAll() (int, error)` - postgres_store.go contains `func (s *PostgresStore) AcknowledgeByTag(tagID int) (int, error)` - postgres_store.go AcknowledgeByTag contains `$1` (positional param) - - `go build ./...` exits 0 + - diunwebhook_test.go contains `TestAcknowledgeAll_Empty` + - diunwebhook_test.go contains `TestAcknowledgeByTag_OnlyAffectsTargetTag` + - `go test -v -run "TestAcknowledge(All|ByTag)_" ./pkg/diunwebhook/` exits 0 - Store interface extended with 2 new methods; both SQLiteStore and PostgresStore compile and implement the interface + Store interface extended with 2 new methods; both SQLiteStore and PostgresStore compile and implement the interface; 6 store-level tests pass - Task 2: Add HTTP handlers, route registration, and tests for bulk acknowledge endpoints + Task 2: Add HTTP handlers, route registration, and handler tests for bulk acknowledge endpoints pkg/diunwebhook/diunwebhook.go, pkg/diunwebhook/diunwebhook_test.go, pkg/diunwebhook/export_test.go, cmd/diunwebhook/main.go - pkg/diunwebhook/diunwebhook.go @@ -268,7 +297,7 @@ mux.HandleFunc("/api/updates", srv.UpdatesHandler) } ``` - 5. Write tests in `diunwebhook_test.go` following the existing `Test_` naming convention. Use `NewTestServer()` for each test. Setup: use `TestUpsertEvent` to create events, `TestCreateTag` + `TestAssignTag` to setup tag assignments. + 5. Write handler tests in `diunwebhook_test.go` following the existing `Test_` naming convention. Use `NewTestServer()` for each test. Setup: use `TestUpsertEvent` to create events, `TestCreateTag` + `TestAssignTag` to setup tag assignments. 6. Also add the Vite dev proxy for the two new endpoints in `frontend/vite.config.ts` -- NOT needed, the existing proxy config already proxies all `/api` requests to `:8080`. diff --git a/.planning/phases/04-ux-improvements/04-02-PLAN.md b/.planning/phases/04-ux-improvements/04-02-PLAN.md index ed32f46..7aebf6f 100644 --- a/.planning/phases/04-ux-improvements/04-02-PLAN.md +++ b/.planning/phases/04-ux-improvements/04-02-PLAN.md @@ -233,7 +233,7 @@ From frontend/src/index.css (CSS vars - note: no --destructive or --card defined Then in ServiceCard.tsx, remove the local `getRegistry` function and add `import { getRegistry } from '@/lib/utils'` (alongside the existing `cn` import: `import { cn, getRegistry } from '@/lib/utils'`). - cd /home/jean-luc-makiola/Development/projects/DiunDashboard/frontend && npx tsc --noEmit + cd /home/jean-luc-makiola/Development/projects/DiunDashboard/frontend && bunx tsc --noEmit - main.tsx contains `localStorage.getItem('theme')` and `prefers-color-scheme` @@ -247,7 +247,7 @@ From frontend/src/index.css (CSS vars - note: no --destructive or --card defined - ServiceCard.tsx does NOT contain `opacity-0 group-hover:opacity-100` on the drag handle - lib/utils.ts contains `export function getRegistry` - ServiceCard.tsx contains `import` with `getRegistry` from `@/lib/utils` - - `npx tsc --noEmit` exits 0 + - `bunx tsc --noEmit` exits 0 Theme toggle works (sun/moon icon in header, persists to localStorage, respects system preference on first visit); drag handle always visible at 40% opacity; getRegistry is a shared utility @@ -363,7 +363,7 @@ From frontend/src/index.css (CSS vars - note: no --destructive or --card defined Import `UpdateEntry` type if needed for the `as` cast. - cd /home/jean-luc-makiola/Development/projects/DiunDashboard/frontend && npx tsc --noEmit && bun run build + cd /home/jean-luc-makiola/Development/projects/DiunDashboard/frontend && bunx tsc --noEmit && bun run build - FilterBar.tsx exists and exports `FilterBar` component @@ -388,7 +388,7 @@ From frontend/src/index.css (CSS vars - note: no --destructive or --card defined ```bash cd /home/jean-luc-makiola/Development/projects/DiunDashboard/frontend -npx tsc --noEmit +bunx tsc --noEmit bun run build ``` diff --git a/.planning/phases/04-ux-improvements/04-03-PLAN.md b/.planning/phases/04-ux-improvements/04-03-PLAN.md index 08a87fc..18472b3 100644 --- a/.planning/phases/04-ux-improvements/04-03-PLAN.md +++ b/.planning/phases/04-ux-improvements/04-03-PLAN.md @@ -26,7 +26,7 @@ must_haves: truths: - "User can dismiss all pending updates with a Dismiss All button in the header area" - "User can dismiss all pending updates within a tag group via a per-section button" - - "Dismiss All requires a two-click confirmation before executing" + - "Dismiss All requires an inline two-click confirmation before executing (matching tag delete UX pattern)" - "A pending-count badge is always visible in the Header" - "The browser tab title shows 'DiunDash (N)' when N > 0 and 'DiunDash' when 0" - "A toast notification appears when new updates arrive during polling" @@ -36,7 +36,7 @@ must_haves: provides: "acknowledgeAll, acknowledgeByTag callbacks; newArrivals state; tab title effect" contains: "acknowledgeAll" - path: "frontend/src/components/Header.tsx" - provides: "Pending badge, dismiss-all button with confirm" + provides: "Pending badge, dismiss-all button with inline two-click confirm" contains: "pendingCount" - path: "frontend/src/components/TagSection.tsx" provides: "Per-group dismiss button" @@ -229,7 +229,7 @@ From frontend/src/App.tsx (after Plan 02): ``` - cd /home/jean-luc-makiola/Development/projects/DiunDashboard/frontend && npx tsc --noEmit + cd /home/jean-luc-makiola/Development/projects/DiunDashboard/frontend && bunx tsc --noEmit - useUpdates.ts contains `const acknowledgeAll = useCallback` @@ -240,7 +240,7 @@ From frontend/src/App.tsx (after Plan 02): - useUpdates.ts contains `const [newArrivals, setNewArrivals] = useState` - useUpdates.ts contains `clearNewArrivals` in the return object - useUpdates.ts return object includes `acknowledgeAll` and `acknowledgeByTag` - - `npx tsc --noEmit` exits 0 + - `bunx tsc --noEmit` exits 0 useUpdates hook returns acknowledgeAll, acknowledgeByTag, newArrivals, and clearNewArrivals; toast detection fires on new images during polling @@ -288,7 +288,7 @@ From frontend/src/App.tsx (after Plan 02): } ``` - 2. **Header.tsx** (per D-03, D-04, D-09): Extend HeaderProps and add pending badge + dismiss-all button. + 2. **Header.tsx** (per D-03, D-04, D-09): Extend HeaderProps and add pending badge + dismiss-all button with inline two-click confirm pattern (per D-04, matching existing tag delete UX -- no modal needed). Update the interface: ```typescript interface HeaderProps { @@ -306,7 +306,7 @@ From frontend/src/App.tsx (after Plan 02): )} ``` - Add dismiss-all button with two-click confirm pattern (per D-04, matching existing tag delete pattern in TagSection). Add local state `const [confirmDismissAll, setConfirmDismissAll] = useState(false)`. The button: + Add dismiss-all button with inline two-click confirm pattern (per D-04). Add local state `const [confirmDismissAll, setConfirmDismissAll] = useState(false)`. The button: ```tsx {pendingCount > 0 && (