feat(notifications): requests and flags reach the people who act on them (#5340)
- A new request still pending after any auto-approval notifies the admins (request_pending), but not the requester if they are an admin. A request that dedups into one already in flight is not announced again. - Approving or rejecting a request notifies the requester, and a rejection carries the admin's notes as the reason. An admin deciding their own request gets nothing. - The reconciler notifies the requester when their request arrives (request_completed), linking the matched album or artist. - A request the re-acquisition sweeper files and cannot approve itself notifies the admins. - A quarantine flag notifies every admin except the flagger, naming the track, the flagger and the reason. lidarrrequests.Service.CreateTracked reports whether a request was inserted or deduped; Create wraps it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -7,12 +7,14 @@ import (
|
||||
"log/slog"
|
||||
"time"
|
||||
|
||||
"github.com/google/uuid"
|
||||
"github.com/jackc/pgx/v5"
|
||||
"github.com/jackc/pgx/v5/pgtype"
|
||||
"github.com/jackc/pgx/v5/pgxpool"
|
||||
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/lidarrrequests"
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/notifications"
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/tags"
|
||||
)
|
||||
|
||||
@@ -20,7 +22,7 @@ import (
|
||||
// narrowed to an interface so the pass can be tested without a Lidarr client
|
||||
// or an approval path that talks to one.
|
||||
type requestCreator interface {
|
||||
Create(ctx context.Context, userID pgtype.UUID, p lidarrrequests.CreateParams) (dbq.LidarrRequest, error)
|
||||
CreateTracked(ctx context.Context, userID pgtype.UUID, p lidarrrequests.CreateParams) (dbq.LidarrRequest, bool, error)
|
||||
Approve(ctx context.Context, requestID, adminID pgtype.UUID, ov lidarrrequests.ApproveOverrides) (dbq.LidarrRequest, error)
|
||||
}
|
||||
|
||||
@@ -37,6 +39,9 @@ type Sweeper struct {
|
||||
settings *SettingsService
|
||||
requests requestCreator
|
||||
logger *slog.Logger
|
||||
// notifier tells admins about a request the sweeper filed that still
|
||||
// needs their approval (M489). Nil: nobody is told.
|
||||
notifier *notifications.Notifier
|
||||
tick time.Duration
|
||||
// releaseGroup names the MusicBrainz release group of a release id, for an
|
||||
// album whose tags never carried one (#5241). Lidarr knows albums only by
|
||||
@@ -92,6 +97,10 @@ type PassResult struct {
|
||||
Unresolved int
|
||||
}
|
||||
|
||||
// SetNotifier makes requests the sweeper files, and cannot approve itself,
|
||||
// reach the admins' notifications inbox (M489).
|
||||
func (s *Sweeper) SetNotifier(n *notifications.Notifier) { s.notifier = n }
|
||||
|
||||
// SweepOnce runs one pass. Exported so the admin surface can offer a "run
|
||||
// now" without waiting out the tick, and so tests drive it directly.
|
||||
func (s *Sweeper) SweepOnce(ctx context.Context) error {
|
||||
@@ -182,7 +191,7 @@ func (s *Sweeper) attempt(
|
||||
return fmt.Errorf("release group: %w", err)
|
||||
}
|
||||
|
||||
req, err := s.requests.Create(ctx, adminID, lidarrrequests.CreateParams{
|
||||
req, created, err := s.requests.CreateTracked(ctx, adminID, lidarrrequests.CreateParams{
|
||||
Kind: "album",
|
||||
LidarrArtistMBID: *album.ArtistMbid,
|
||||
ArtistName: album.ArtistName,
|
||||
@@ -206,11 +215,15 @@ func (s *Sweeper) attempt(
|
||||
return fmt.Errorf("record attempt: %w", err)
|
||||
}
|
||||
|
||||
// A request the sweeper filed and could not approve waits on an admin.
|
||||
// One it deduped into was announced when it was first filed.
|
||||
awaitingAdmin := created
|
||||
if cfg.AutoApprove {
|
||||
_, aerr := s.requests.Approve(ctx, req.ID, adminID, lidarrrequests.ApproveOverrides{})
|
||||
switch {
|
||||
case aerr == nil:
|
||||
res.Approved++
|
||||
awaitingAdmin = false
|
||||
case errors.Is(aerr, lidarrrequests.ErrLidarrDisabled):
|
||||
// Leave it pending rather than treating it as a failure. The
|
||||
// request is still the right record of intent, and it becomes
|
||||
@@ -225,6 +238,16 @@ func (s *Sweeper) attempt(
|
||||
}
|
||||
}
|
||||
|
||||
if awaitingAdmin {
|
||||
s.notifier.NotifyLogged(ctx, notifications.KindRequestPending, notifications.ToAdmins(pgtype.UUID{}),
|
||||
notifications.Payload{
|
||||
RequestID: uuid.UUID(req.ID.Bytes).String(),
|
||||
RequestKind: string(req.Kind),
|
||||
Name: lidarrrequests.DisplayName(req),
|
||||
Actor: "Re-acquisition",
|
||||
}.Map())
|
||||
}
|
||||
|
||||
if row.Attempts >= cfg.MaxAttempts {
|
||||
if err := q.MarkReacquisitionGaveUp(ctx, album.AlbumID); err != nil {
|
||||
return fmt.Errorf("mark gave up: %w", err)
|
||||
|
||||
@@ -15,6 +15,7 @@ import (
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/dbtest"
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/lidarrconfig"
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/lidarrrequests"
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/notifications"
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/tags"
|
||||
)
|
||||
|
||||
@@ -84,9 +85,10 @@ func TestSweep_RequestsAlbumsByReleaseGroup_Integration(t *testing.T) {
|
||||
logger := slog.New(slog.NewTextHandler(io.Discard, nil))
|
||||
q := dbq.New(pool)
|
||||
|
||||
if _, err := q.CreateUser(ctx, dbq.CreateUserParams{
|
||||
admin, err := q.CreateUser(ctx, dbq.CreateUserParams{
|
||||
Username: dbtest.TestUserPrefix + "sweepadmin", PasswordHash: "x", ApiTokenHash: "x", IsAdmin: true,
|
||||
}); err != nil {
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
settings, err := NewSettingsService(ctx, pool, logger)
|
||||
@@ -105,6 +107,7 @@ func TestSweep_RequestsAlbumsByReleaseGroup_Integration(t *testing.T) {
|
||||
|
||||
requests := lidarrrequests.NewService(pool, lidarrconfig.New(pool), nil, nil)
|
||||
s := NewSweeper(pool, settings, requests, logger)
|
||||
s.SetNotifier(notifications.New(pool, nil, nil))
|
||||
asked := map[string]int{}
|
||||
s.releaseGroup = func(_ context.Context, release string) (string, error) {
|
||||
asked[release]++
|
||||
@@ -153,6 +156,20 @@ SELECT lr.lidarr_album_mbid
|
||||
if attempts != 0 {
|
||||
t.Errorf("unresolvable album spent an attempt (%d state rows)", attempts)
|
||||
}
|
||||
// M489: the two requests filed without auto-approval wait on an admin,
|
||||
// so each reaches the admins' inbox; the unresolvable album filed none.
|
||||
inbox, err := q.ListNotifications(ctx, dbq.ListNotificationsParams{UserID: admin.ID, PageLimit: 10})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if len(inbox) != 2 {
|
||||
t.Errorf("admin inbox holds %d notifications, want 2", len(inbox))
|
||||
}
|
||||
for _, n := range inbox {
|
||||
if n.Kind != string(notifications.KindRequestPending) {
|
||||
t.Errorf("admin inbox holds %q, want request_pending", n.Kind)
|
||||
}
|
||||
}
|
||||
var stray int
|
||||
if err := pool.QueryRow(ctx, "SELECT count(*) FROM lidarr_requests WHERE lidarr_album_mbid LIKE 'release-%'").Scan(&stray); err != nil {
|
||||
t.Fatal(err)
|
||||
|
||||
Reference in New Issue
Block a user