fix(04): revise plans based on checker feedback
All checks were successful
CI / build-test (push) Successful in 1m31s
All checks were successful
CI / build-test (push) Successful in 1m31s
This commit is contained in:
@@ -133,12 +133,14 @@ mux.HandleFunc("/api/updates", srv.UpdatesHandler)
|
||||
<tasks>
|
||||
|
||||
<task type="auto" tdd="true">
|
||||
<name>Task 1: Extend Store interface and implement AcknowledgeAll + AcknowledgeByTag in both stores</name>
|
||||
<files>pkg/diunwebhook/store.go, pkg/diunwebhook/sqlite_store.go, pkg/diunwebhook/postgres_store.go</files>
|
||||
<name>Task 1: Extend Store interface and implement AcknowledgeAll + AcknowledgeByTag with store-level tests</name>
|
||||
<files>pkg/diunwebhook/store.go, pkg/diunwebhook/sqlite_store.go, pkg/diunwebhook/postgres_store.go, pkg/diunwebhook/diunwebhook_test.go, pkg/diunwebhook/export_test.go</files>
|
||||
<read_first>
|
||||
- pkg/diunwebhook/store.go
|
||||
- pkg/diunwebhook/sqlite_store.go
|
||||
- pkg/diunwebhook/postgres_store.go
|
||||
- pkg/diunwebhook/diunwebhook_test.go
|
||||
- pkg/diunwebhook/export_test.go
|
||||
</read_first>
|
||||
<behavior>
|
||||
- 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
|
||||
</behavior>
|
||||
<action>
|
||||
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<Function>_<Scenario>` 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).
|
||||
</action>
|
||||
<verify>
|
||||
<automated>cd /home/jean-luc-makiola/Development/projects/DiunDashboard && go build ./...</automated>
|
||||
<automated>cd /home/jean-luc-makiola/Development/projects/DiunDashboard && go test -v -run "TestAcknowledge(All|ByTag)_" ./pkg/diunwebhook/</automated>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- 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
|
||||
</acceptance_criteria>
|
||||
<done>Store interface extended with 2 new methods; both SQLiteStore and PostgresStore compile and implement the interface</done>
|
||||
<done>Store interface extended with 2 new methods; both SQLiteStore and PostgresStore compile and implement the interface; 6 store-level tests pass</done>
|
||||
</task>
|
||||
|
||||
<task type="auto" tdd="true">
|
||||
<name>Task 2: Add HTTP handlers, route registration, and tests for bulk acknowledge endpoints</name>
|
||||
<name>Task 2: Add HTTP handlers, route registration, and handler tests for bulk acknowledge endpoints</name>
|
||||
<files>pkg/diunwebhook/diunwebhook.go, pkg/diunwebhook/diunwebhook_test.go, pkg/diunwebhook/export_test.go, cmd/diunwebhook/main.go</files>
|
||||
<read_first>
|
||||
- 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<Handler>_<Scenario>` 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<Handler>_<Scenario>` 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`.
|
||||
</action>
|
||||
|
||||
@@ -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'`).
|
||||
</action>
|
||||
<verify>
|
||||
<automated>cd /home/jean-luc-makiola/Development/projects/DiunDashboard/frontend && npx tsc --noEmit</automated>
|
||||
<automated>cd /home/jean-luc-makiola/Development/projects/DiunDashboard/frontend && bunx tsc --noEmit</automated>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- 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
|
||||
</acceptance_criteria>
|
||||
<done>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</done>
|
||||
</task>
|
||||
@@ -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.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>cd /home/jean-luc-makiola/Development/projects/DiunDashboard/frontend && npx tsc --noEmit && bun run build</automated>
|
||||
<automated>cd /home/jean-luc-makiola/Development/projects/DiunDashboard/frontend && bunx tsc --noEmit && bun run build</automated>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- FilterBar.tsx exists and exports `FilterBar` component
|
||||
@@ -388,7 +388,7 @@ From frontend/src/index.css (CSS vars - note: no --destructive or --card defined
|
||||
<verification>
|
||||
```bash
|
||||
cd /home/jean-luc-makiola/Development/projects/DiunDashboard/frontend
|
||||
npx tsc --noEmit
|
||||
bunx tsc --noEmit
|
||||
bun run build
|
||||
```
|
||||
</verification>
|
||||
|
||||
@@ -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):
|
||||
```
|
||||
</action>
|
||||
<verify>
|
||||
<automated>cd /home/jean-luc-makiola/Development/projects/DiunDashboard/frontend && npx tsc --noEmit</automated>
|
||||
<automated>cd /home/jean-luc-makiola/Development/projects/DiunDashboard/frontend && bunx tsc --noEmit</automated>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- 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<string[]>`
|
||||
- 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
|
||||
</acceptance_criteria>
|
||||
<done>useUpdates hook returns acknowledgeAll, acknowledgeByTag, newArrivals, and clearNewArrivals; toast detection fires on new images during polling</done>
|
||||
</task>
|
||||
@@ -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):
|
||||
</Badge>
|
||||
)}
|
||||
```
|
||||
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 && (
|
||||
<Button
|
||||
@@ -342,7 +342,7 @@ From frontend/src/App.tsx (after Plan 02):
|
||||
onAcknowledgeGroup?: (tagId: number) => void
|
||||
}
|
||||
```
|
||||
Add a "Dismiss Group" button in the section header, next to the delete button, only when `tag !== null` and `onAcknowledgeGroup` is provided and at least one row is unacknowledged. Use two-click confirm pattern:
|
||||
Add a "Dismiss Group" button in the section header, next to the delete button, only when `tag !== null` and `onAcknowledgeGroup` is provided and at least one row is unacknowledged. Use inline two-click confirm pattern (per D-04):
|
||||
```typescript
|
||||
const [confirmDismissGroup, setConfirmDismissGroup] = useState(false)
|
||||
const hasPending = rows.some(r => !r.entry.acknowledged)
|
||||
@@ -499,7 +499,7 @@ From frontend/src/App.tsx (after Plan 02):
|
||||
```
|
||||
</action>
|
||||
<verify>
|
||||
<automated>cd /home/jean-luc-makiola/Development/projects/DiunDashboard/frontend && npx tsc --noEmit && bun run build</automated>
|
||||
<automated>cd /home/jean-luc-makiola/Development/projects/DiunDashboard/frontend && bunx tsc --noEmit && bun run build</automated>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- Toast.tsx exists and exports `Toast` component
|
||||
@@ -524,7 +524,7 @@ From frontend/src/App.tsx (after Plan 02):
|
||||
- TagSection.tsx TagSectionRow interface contains `isNew`
|
||||
- `bun run build` exits 0
|
||||
</acceptance_criteria>
|
||||
<done>Bulk dismiss buttons work (dismiss-all in header with two-click confirm, dismiss-group in each tag section); pending badge shows in header; tab title reflects count; toast appears for new arrivals; new-since-last-visit items have amber left border highlight</done>
|
||||
<done>Bulk dismiss buttons work (dismiss-all in header with inline two-click confirm, dismiss-group in each tag section with inline two-click confirm); pending badge shows in header; tab title reflects count; toast appears for new arrivals; new-since-last-visit items have amber left border highlight</done>
|
||||
</task>
|
||||
|
||||
</tasks>
|
||||
@@ -532,7 +532,7 @@ From frontend/src/App.tsx (after Plan 02):
|
||||
<verification>
|
||||
```bash
|
||||
cd /home/jean-luc-makiola/Development/projects/DiunDashboard/frontend
|
||||
npx tsc --noEmit
|
||||
bunx tsc --noEmit
|
||||
bun run build
|
||||
# Full stack verification:
|
||||
cd /home/jean-luc-makiola/Development/projects/DiunDashboard
|
||||
@@ -544,7 +544,7 @@ go build ./...
|
||||
<success_criteria>
|
||||
- Dismiss All button in header triggers POST /api/updates/acknowledge-all
|
||||
- Per-group Dismiss Group button triggers POST /api/updates/acknowledge-by-tag with correct tag_id
|
||||
- Both dismiss buttons use two-click confirmation
|
||||
- Both dismiss buttons use inline two-click confirmation (matching tag delete UX pattern)
|
||||
- Pending count badge visible in header when > 0
|
||||
- Browser tab title shows "DiunDash (N)" or "DiunDash"
|
||||
- Toast appears at bottom-right when polling detects new images
|
||||
|
||||
@@ -17,7 +17,7 @@ Deliver UX features that make the dashboard genuinely usable at scale: bulk dism
|
||||
- **D-01:** Add two new Store methods: `AcknowledgeAll() (count int, err error)` and `AcknowledgeByTag(tagID int) (count int, err error)` — consistent with existing `AcknowledgeUpdate(image)` pattern
|
||||
- **D-02:** Two new API endpoints: `POST /api/updates/acknowledge-all` and `POST /api/updates/acknowledge-by-tag` (with `tag_id` in body) — returning the count of dismissed items
|
||||
- **D-03:** UI placement: "Dismiss All" button in the header/stats area; "Dismiss Group" button in each TagSection header next to the existing delete button
|
||||
- **D-04:** Confirmation: modal/dialog confirmation for dismiss-all (high-impact action); inline confirm pattern (matching existing tag delete) for per-group dismiss
|
||||
- **D-04:** Confirmation: inline two-click confirm pattern for both dismiss-all and per-group dismiss — consistent with existing tag delete UX, zero additional dependencies (modal/dialog originally considered but inline is simpler and matches established patterns)
|
||||
|
||||
### Search and filter (SRCH-01 through SRCH-04)
|
||||
- **D-05:** Client-side filtering only — all data is already in memory from polling, no new API endpoints needed
|
||||
|
||||
Reference in New Issue
Block a user