feat: add mbid column to artist_settings

Additive migration 008 adds an mbid column to artist_settings, extend the
ArtistSettings struct and persistence functions (SaveArtistSettings,
GetArtistSettings, GetAllArtistSettings, UpdateArtistSettings) to carry the
MusicBrainz ID, and add round-trip tests covering empty and set MBID values.
This commit is contained in:
2026-07-19 22:13:57 +03:00
parent 35d6a5234f
commit 85c42ec858
5 changed files with 346 additions and 11 deletions

View File

@@ -0,0 +1,163 @@
# 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
- [ ] add `ResolveArtistMBID(ctx, client, name) (string, error)` in `internal/musicbrainz` using `/ws/2/artist/?query=artist:<name>&fmt=json`
- [ ] parse first matching artist ID from JSON response; return error if none
- [ ] write tests with httptest stub (match found, no match, HTTP error)
- [ ] run tests - must pass before task 3
### Task 3: Periodic sync pipeline
- [ ] add `SyncAll(ctx, ndClient, mbClient, db, ttl)` orchestrator: for each monitored artist → ensure MBID (resolve + persist if missing) → `musicbrainz.SyncArtistDiscography` → `navidrome.SyncAlbums`
- [ ] wire `navidrome.NewClient` into `App`; add `ndClient` field
- [ ] write tests for `SyncAll` with stubbed clients + `:memory:` DB (new artist gets MBID, existing MBID reused, unmonitored skipped)
- [ ] run tests - must pass before task 4
### Task 4: Main loop wiring (sync → scan)
- [ ] replace compute-only `run()` with: one immediate sync+scan, then a ticker-driven periodic sync+scan goroutine; keep graceful shutdown via ctx
- [ ] add a `syncInterval` config field (default e.g. 6h) to `config.go` + defaults + validation
- [ ] write tests for the loop scheduling logic where feasible (ticker fires, ctx cancels cleanly)
- [ ] 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.