Replace compute-only run() with an immediate sync+scan followed by a ticker-driven periodic loop, add the sync.interval config field (default 6h) with defaults and validation, and add tests for the scheduling logic.
164 lines
9.6 KiB
Markdown
164 lines
9.6 KiB
Markdown
# Notifier, Web UI & Sync Wiring
|
||
|
||
## Overview
|
||
Transform NaviWatcher from a compute-only daemon (which only logs missing
|
||
releases) into a working service:
|
||
1. **Sync wiring** in `main.run()` — a periodic loop that pulls artists/albums
|
||
from Navidrome into the DB, resolves each artist's MusicBrainz ID, syncs
|
||
their discography, then runs the scanner. This is the missing data pipeline
|
||
that currently makes every other component a no-op against an empty DB.
|
||
2. **Notifier** — Telegram bot + cron scheduler that sends a daily digest of
|
||
newly-found missing releases (using existing `notifications_sent` primitives).
|
||
3. **Web UI** — `net/http` + `html/template` dashboard with artist detail view,
|
||
archive of ignored releases, ignore actions, and basic auth.
|
||
|
||
Problem solved: today `main.run()` calls `scanner.ScanAll` over empty tables and
|
||
blocks on `<-ctx.Done()`. Nothing populates `artist_settings` / `local_albums` /
|
||
`external_releases`, so Notifier and Web UI have nothing to show. This plan
|
||
closes that gap end-to-end.
|
||
|
||
Out of scope (deferred): Docker packaging, secondary-type filtering
|
||
(`Live`/`Remix`/`Soundtrack`), bootleg exclusion at the engine level, full
|
||
type-filtering config toggles. Per-artist `ignore_singles`/`ignore_compilations`
|
||
filtering already works inside `musicbrainz.SyncArtistDiscography`.
|
||
|
||
## Context (from discovery)
|
||
- `internal/navidrome/{client,sync}.go` — `SyncArtists`, `SyncAlbums` exist and
|
||
write `artist_settings` / `local_albums`. **Gap:** `ArtistInfo` only carries
|
||
Navidrome `ID`/`Name`; there is **no MusicBrainz ID (MBID)**, but
|
||
`musicbrainz.SyncArtistDiscography` requires `artistMBID`.
|
||
- `internal/musicbrainz/{client,sync,api}.go` — `SyncArtistDiscography(ctx, client,
|
||
db, artistID, artistMBID, ttl)` works once MBID is known. `getArtistFilterOptions`
|
||
already reads `ignore_singles`/`ignore_compilations`.
|
||
- `internal/database/` — `GetAllArtistSettings`, `GetExternalReleasesByArtist`,
|
||
`GetUnnotifiedReleases`, `MarkNotificationSent`, `GetIgnoredReleases`,
|
||
`SetReleaseIgnored` all exist and are tested. `artist_settings` schema has no
|
||
MBID column.
|
||
- `internal/scanner/scan.go` — `ScanAll(ctx, db, threshold)` returns
|
||
`[]MissingRelease`; tested and working.
|
||
- `cmd/naviwatcher/main.go` — `App` holds cfg/db/mbClient only; `run()` is the
|
||
compute-only stub. `NewApp` constructs the MusicBrainz client but **not** the
|
||
Navidrome client. No goroutines for sync/notifier/web.
|
||
- `internal/config/config.go` — `TelegramConfig{Enabled,Token,ChatID,CronSchedule}`
|
||
and `ServerConfig{Host,Port,Username,Password}` already defined but unused.
|
||
- `config.yaml.example` exists.
|
||
|
||
### Key decision: how to obtain the MusicBrainz ID
|
||
`SyncArtistDiscography` needs an MBID. Navidrome's Subsonic API does not return
|
||
MBIDs via `getArtists`/`getArtist`. Resolution: **add an `mbid` column to
|
||
`artist_settings`** and resolve it lazily during sync by querying MusicBrainz
|
||
artist search (`/ws/2/artist/?query=artist:<name>&fmt=json`). Cache the MBID on
|
||
the artist row. This avoids manual config and keeps the schema the single source
|
||
of truth. (Alternative considered: resolve by name on every sync without
|
||
storing — rejected because it doubles rate-limited MB calls and is flaky on
|
||
name collisions.)
|
||
|
||
## Development Approach
|
||
- **Testing approach**: TDD — write tests before implementation for each task.
|
||
- Complete each task fully (code + tests passing) before the next.
|
||
- Every task MUST include new/updated tests (success + error/edge cases).
|
||
- All tests must pass before starting the next task.
|
||
- Run `go test ./...` and `go vet ./...` after each task.
|
||
- Maintain backward compatibility of existing DB schema (additive migration only).
|
||
|
||
## Testing Strategy
|
||
- **Unit tests** for every new function/method (success + error paths).
|
||
- **Integration-style tests** for sync/resolver using a `:memory:` DB and a
|
||
stubbed MusicBrainz HTTP client (the existing `client_test.go` already shows
|
||
the httptest pattern — reuse it).
|
||
- **Web UI**: table-driven tests for handlers (status codes, auth rejection,
|
||
ignore action effects on DB) using `httptest.NewServer` + in-memory DB. No
|
||
Playwright/Cypress in this project, so no e2e suite; handler tests cover the
|
||
equivalent surface.
|
||
- **Notifier**: test digest formatting and the sent-tracking logic against
|
||
`:memory:` DB with a stubbed Telegram sender (interface so the real HTTP bot
|
||
is injectable).
|
||
|
||
## Progress Tracking
|
||
- Mark completed items with `[x]` immediately when done.
|
||
- Add newly discovered tasks with ➕ prefix.
|
||
- Document issues/blockers with ⚠️ prefix.
|
||
- Keep plan in sync with actual work.
|
||
|
||
## Implementation Steps
|
||
|
||
### Task 1: Add `mbid` column to artist_settings
|
||
- [x] add migration `006_add_mbid_to_artist_settings` (`ALTER TABLE artist_settings ADD COLUMN mbid TEXT;`)
|
||
- [x] extend `ArtistSettings` struct + `SaveArtistSettings`/`UpsertArtist` to persist `MBID`
|
||
- [x] write tests for migration + struct round-trip (empty MBID allowed, set/get)
|
||
- [x] run tests - must pass before task 2
|
||
|
||
### Task 2: MusicBrainz artist-ID resolver
|
||
- [x] add `ResolveArtistMBID(ctx, client, name) (string, error)` in `internal/musicbrainz` using `/ws/2/artist/?query=artist:<name>&fmt=json`
|
||
- [x] parse first matching artist ID from JSON response; return error if none
|
||
- [x] write tests with httptest stub (match found, no match, HTTP error)
|
||
- [x] run tests - must pass before task 3
|
||
|
||
### Task 3: Periodic sync pipeline
|
||
- [x] add `SyncAll(ctx, ndClient, mbClient, db, ttl)` orchestrator: for each monitored artist → ensure MBID (resolve + persist if missing) → `musicbrainz.SyncArtistDiscography` → `navidrome.SyncAlbums`
|
||
- [x] wire `navidrome.NewClient` into `App`; add `ndClient` field
|
||
- [x] write tests for `SyncAll` with stubbed clients + `:memory:` DB (new artist gets MBID, existing MBID reused, unmonitored skipped)
|
||
- [x] run tests - must pass before task 4
|
||
|
||
### Task 4: Main loop wiring (sync → scan)
|
||
- [x] replace compute-only `run()` with: one immediate sync+scan, then a ticker-driven periodic sync+scan goroutine; keep graceful shutdown via ctx
|
||
- [x] add a `syncInterval` config field (default e.g. 6h) to `config.go` + defaults + validation
|
||
- [x] write tests for the loop scheduling logic where feasible (ticker fires, ctx cancels cleanly)
|
||
- [x] run tests - must pass before task 5
|
||
|
||
### Task 5: Notifier — Telegram sender + digest
|
||
- [ ] define `Sender` interface (`Send(ctx, message string) error`) and a `telegramSender` using `TelegramConfig` (bot API `sendMessage`)
|
||
- [ ] add `FormatDigest(missing []scanner.MissingRelease, uiBaseURL string) string` (artist names + counts + Web UI link)
|
||
- [ ] write tests: digest formatting, sender failure handling (stub sender)
|
||
- [ ] run tests - must pass before task 6
|
||
|
||
### Task 6: Notifier — scheduler + sent-tracking
|
||
- [ ] add `NotifyOnce(ctx, db, sender, cfg)` : query `GetUnnotifiedReleases`, build digest, send, `MarkNotificationSent` per rgid
|
||
- [ ] add cron-based scheduler goroutine honoring `TelegramConfig.CronSchedule` (use a lightweight cron lib or robfig/cron); no-op if `Enabled=false`
|
||
- [ ] write tests: `NotifyOnce` marks sent and skips already-sent; scheduler parses cron and fires (inject fixed time / use every-minute for test)
|
||
- [ ] run tests - must pass before task 7
|
||
|
||
### Task 7: Web UI — server + auth + dashboard
|
||
- [ ] create `internal/web` with `Server` (net/http), `//go:embed` templates, basic-auth middleware using `ServerConfig.Username/Password`
|
||
- [ ] dashboard handler: list monitored artists with missing-release counts (join scanner result / external vs local)
|
||
- [ ] write tests: unauthenticated request → 401; authenticated → 200 with expected artist rendered
|
||
- [ ] run tests - must pass before task 8
|
||
|
||
### Task 8: Web UI — artist detail + archive + ignore actions
|
||
- [ ] artist page: local albums (Subsonic) + found missing (MB cache) + ignore buttons
|
||
- [ ] archive page: `GetIgnoredReleases` with restore action
|
||
- [ ] POST handlers: `SetReleaseIgnored(rgid, true/false)`; "ignore all singles of artist" toggles `artist_settings.ignore_singles`
|
||
- [ ] write tests: ignore sets flag + removes from dashboard missing; restore clears flag; auth enforced on POST
|
||
- [ ] run tests - must pass before task 9
|
||
|
||
### Task 9: Verify acceptance criteria
|
||
- [ ] run full suite `go test ./...` — all pass
|
||
- [ ] run `go vet ./...` and `go build -o naviwatcher` — clean
|
||
- [ ] verify scan→notify→web data flow with a seeded `:memory:`/file DB smoke check
|
||
- [ ] verify config.yaml.example documents new `sync_interval` field
|
||
|
||
### Task 10: Update documentation
|
||
- [ ] add a short "How it works now" note to README/CLAUDE.md if present
|
||
- [ ] note the new `sync_interval` config key in `config.yaml.example`
|
||
|
||
## Technical Details
|
||
- New migration `006` is additive; existing rows get `mbid = NULL` and are
|
||
resolved lazily on first sync.
|
||
- `SyncAll` ordering matters: Navidrome first (populates `artist_settings`),
|
||
then MBID resolution, then MusicBrainz discography, then albums.
|
||
- Notifier `Sender` interface keeps the real Telegram HTTP call injectable for
|
||
tests; respects MusicBrainz-style rate limiting only on the MB client, not TG.
|
||
- Web UI basic auth uses `crypto/subtle.ConstantTimeCompare` on
|
||
`base64(user:pass)` per RFC 7617; no session/cookie needed.
|
||
- Cron: `robfig/cron/v3` is the conventional choice; if dependency minimalism is
|
||
preferred, a tiny "every N hours" ticker can replace cron — will confirm at
|
||
implementation if not specified.
|
||
|
||
## Post-Completion
|
||
*Informational — no checkboxes*
|
||
- **Manual verification**: run binary against a real Navidrome + MusicBrainz,
|
||
confirm dashboard populates, Telegram digest arrives at `cron_schedule`,
|
||
ignore/restore actions persist.
|
||
- **External**: ensure `config.yaml.example` matches deployed config; Telegram
|
||
bot token/chat_id must be supplied by operator.
|