feat(server/web/coverart): manual Re-search missing art button
POST /api/admin/cover-sources/research bumps cover_art_sources_meta. current_version unconditionally; the next enrichment pass re-eligibles every 'none' row. Existing positively-sourced rows are not affected — the version-mismatch eligibility rule only kicks in for 'none'. Admin Cover Art panel gains a "Re-search missing art" button that calls the endpoint and shows a confirmation toast. Operator's escape hatch when: - They've added a Last.fm key and want to retry failed rows immediately rather than waiting for the next scheduled scan. - An upstream provider's catalog has updated (e.g. Deezer added a back-catalog import). - They want to verify a fix end-to-end before the next scheduled scan fires. SettingsService.BumpVersion (the unconditional version of T5's BumpVersionIfProvidersChanged) is the single-call DB writer; both endpoints route through it. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,22 @@
|
|||||||
|
package api
|
||||||
|
|
||||||
|
import "net/http"
|
||||||
|
|
||||||
|
// handleResearchMissingArt bumps the cover-art version unconditionally
|
||||||
|
// so every 'none' row becomes eligible at the next enrichment pass.
|
||||||
|
// Operator's escape hatch when adding a Last.fm key, when an upstream
|
||||||
|
// provider's catalog has updated, or when verifying a fix without
|
||||||
|
// waiting for the next scheduled scan.
|
||||||
|
//
|
||||||
|
// Existing positively-sourced rows (sidecar/embedded/mbcaa/theaudiodb/
|
||||||
|
// deezer/lastfm) are not affected — the version-mismatch eligibility
|
||||||
|
// rule only kicks in for 'none'.
|
||||||
|
func (h *handlers) handleResearchMissingArt(w http.ResponseWriter, r *http.Request) {
|
||||||
|
newVer, err := h.coverSettings.BumpVersion(r.Context())
|
||||||
|
if err != nil {
|
||||||
|
h.logger.Error("admin: research missing art bump failed", "err", err)
|
||||||
|
writeErr(w, http.StatusInternalServerError, "server_error", "bump failed")
|
||||||
|
return
|
||||||
|
}
|
||||||
|
writeJSON(w, http.StatusOK, map[string]any{"version": newVer})
|
||||||
|
}
|
||||||
@@ -0,0 +1,74 @@
|
|||||||
|
package api
|
||||||
|
|
||||||
|
import (
|
||||||
|
"encoding/json"
|
||||||
|
"net/http"
|
||||||
|
"net/http/httptest"
|
||||||
|
"os"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"github.com/go-chi/chi/v5"
|
||||||
|
|
||||||
|
"git.fabledsword.com/bvandeusen/minstrel/internal/auth"
|
||||||
|
"git.fabledsword.com/bvandeusen/minstrel/internal/coverart"
|
||||||
|
)
|
||||||
|
|
||||||
|
func newResearchRouter(h *handlers) chi.Router {
|
||||||
|
r := chi.NewRouter()
|
||||||
|
r.Route("/api/admin", func(admin chi.Router) {
|
||||||
|
admin.Use(auth.RequireAdmin())
|
||||||
|
admin.Post("/cover-sources/research", h.handleResearchMissingArt)
|
||||||
|
})
|
||||||
|
return r
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestAdminResearchMissingArt_BumpsAndReturnsVersion(t *testing.T) {
|
||||||
|
if os.Getenv("MINSTREL_TEST_DATABASE_URL") == "" {
|
||||||
|
t.Skip("MINSTREL_TEST_DATABASE_URL not set")
|
||||||
|
}
|
||||||
|
coverart.ResetRegistryForTests()
|
||||||
|
t.Cleanup(coverart.ResetRegistryForTests)
|
||||||
|
|
||||||
|
coverart.Register(&apiTestAlbumProvider{id: "research-prov", display: "Research Provider"})
|
||||||
|
|
||||||
|
h, _ := testHandlersWithCoverSettings(t)
|
||||||
|
admin := seedUser(t, h.pool, "researchadmin", "pw", true)
|
||||||
|
|
||||||
|
req := httptest.NewRequest(http.MethodPost, "/api/admin/cover-sources/research", nil)
|
||||||
|
req = withUser(req, admin)
|
||||||
|
rec := httptest.NewRecorder()
|
||||||
|
newResearchRouter(h).ServeHTTP(rec, req)
|
||||||
|
|
||||||
|
if rec.Code != http.StatusOK {
|
||||||
|
t.Fatalf("status = %d, want 200; body=%s", rec.Code, rec.Body.String())
|
||||||
|
}
|
||||||
|
var resp map[string]any
|
||||||
|
if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil {
|
||||||
|
t.Fatalf("decode: %v", err)
|
||||||
|
}
|
||||||
|
if _, ok := resp["version"]; !ok {
|
||||||
|
t.Errorf("response missing 'version' field; got %v", resp)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestAdminResearchMissingArt_NonAdmin_403(t *testing.T) {
|
||||||
|
if os.Getenv("MINSTREL_TEST_DATABASE_URL") == "" {
|
||||||
|
t.Skip("MINSTREL_TEST_DATABASE_URL not set")
|
||||||
|
}
|
||||||
|
coverart.ResetRegistryForTests()
|
||||||
|
t.Cleanup(coverart.ResetRegistryForTests)
|
||||||
|
|
||||||
|
coverart.Register(&apiTestAlbumProvider{id: "research-prov2", display: "Research Provider 2"})
|
||||||
|
|
||||||
|
h, _ := testHandlersWithCoverSettings(t)
|
||||||
|
user := seedUser(t, h.pool, "researchnonadmin", "pw", false)
|
||||||
|
|
||||||
|
req := httptest.NewRequest(http.MethodPost, "/api/admin/cover-sources/research", nil)
|
||||||
|
req = withUser(req, user)
|
||||||
|
rec := httptest.NewRecorder()
|
||||||
|
newResearchRouter(h).ServeHTTP(rec, req)
|
||||||
|
|
||||||
|
if rec.Code != http.StatusForbidden {
|
||||||
|
t.Errorf("status = %d, want 403", rec.Code)
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -124,6 +124,7 @@ func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger, events *playev
|
|||||||
admin.Get("/cover-sources", h.handleListCoverSources)
|
admin.Get("/cover-sources", h.handleListCoverSources)
|
||||||
admin.Patch("/cover-sources/{provider_id}", h.handleUpdateCoverSource)
|
admin.Patch("/cover-sources/{provider_id}", h.handleUpdateCoverSource)
|
||||||
admin.Post("/cover-sources/{provider_id}/test", h.handleTestCoverSource)
|
admin.Post("/cover-sources/{provider_id}/test", h.handleTestCoverSource)
|
||||||
|
admin.Post("/cover-sources/research", h.handleResearchMissingArt)
|
||||||
})
|
})
|
||||||
|
|
||||||
authed.Get("/playlists", h.handleListPlaylists)
|
authed.Get("/playlists", h.handleListPlaylists)
|
||||||
|
|||||||
@@ -311,6 +311,29 @@ func registeredProvidersHash() string {
|
|||||||
return hex.EncodeToString(sum[:])
|
return hex.EncodeToString(sum[:])
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// BumpVersion unconditionally increments cover_art_sources_meta.current_version
|
||||||
|
// and returns the new value. Used by the manual "Re-search missing art"
|
||||||
|
// admin endpoint when the operator wants every 'none' row re-attempted
|
||||||
|
// on the next enrichment pass.
|
||||||
|
//
|
||||||
|
// Distinct from BumpVersionIfProvidersChanged: that method is the boot-
|
||||||
|
// time auto-bump and skips the DB write when the registered-provider
|
||||||
|
// hash matches. This method always writes.
|
||||||
|
func (s *SettingsService) BumpVersion(ctx context.Context) (int32, error) {
|
||||||
|
q := dbq.New(s.pool)
|
||||||
|
// Reuse BumpVersionAndSetProvidersHash with the current registered-
|
||||||
|
// providers hash; this keeps the stored hash consistent (a manual
|
||||||
|
// bump is conceptually equivalent to "force the auto-bump path").
|
||||||
|
newVer, err := q.BumpVersionAndSetProvidersHash(ctx, registeredProvidersHash())
|
||||||
|
if err != nil {
|
||||||
|
return 0, fmt.Errorf("bump version: %w", err)
|
||||||
|
}
|
||||||
|
s.mu.Lock()
|
||||||
|
s.currentVersion = newVer
|
||||||
|
s.mu.Unlock()
|
||||||
|
return newVer, nil
|
||||||
|
}
|
||||||
|
|
||||||
// TestProvider invokes TestConnection on the named provider if it
|
// TestProvider invokes TestConnection on the named provider if it
|
||||||
// implements TestableProvider. Returns ErrNotTestable if the
|
// implements TestableProvider. Returns ErrNotTestable if the
|
||||||
// provider doesn't, ErrProviderNotFound if not registered.
|
// provider doesn't, ErrProviderNotFound if not registered.
|
||||||
|
|||||||
@@ -295,6 +295,44 @@ func TestSettingsService_BumpVersionIfProvidersChanged_NoChange(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestSettingsService_BumpVersion_Increments verifies the manual bump
|
||||||
|
// path. Distinct from BumpVersionIfProvidersChanged, this always writes
|
||||||
|
// regardless of the stored provider-set hash.
|
||||||
|
func TestSettingsService_BumpVersion_Increments(t *testing.T) {
|
||||||
|
pool := newPool(t)
|
||||||
|
resetRegistryForTests()
|
||||||
|
defer resetRegistryForTests()
|
||||||
|
|
||||||
|
Register(&fakeAlbumProvider{fakeProvider: fakeProvider{id: "fake-album-bump", defaultOn: true}})
|
||||||
|
|
||||||
|
s, err := NewSettingsService(context.Background(), pool, discardLogger())
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("init: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
before := s.CurrentVersion()
|
||||||
|
newVer, err := s.BumpVersion(context.Background())
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("bump: %v", err)
|
||||||
|
}
|
||||||
|
if newVer <= before {
|
||||||
|
t.Errorf("newVer = %d, want > %d", newVer, before)
|
||||||
|
}
|
||||||
|
if s.CurrentVersion() != newVer {
|
||||||
|
t.Errorf("in-memory version = %d, want %d", s.CurrentVersion(), newVer)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Calling again increments by another 1 (always writes).
|
||||||
|
afterFirst := newVer
|
||||||
|
newVer2, err := s.BumpVersion(context.Background())
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("second bump: %v", err)
|
||||||
|
}
|
||||||
|
if newVer2 != afterFirst+1 {
|
||||||
|
t.Errorf("second newVer = %d, want %d", newVer2, afterFirst+1)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// TestRegisteredProvidersHash_StableAcrossOrder: registry order
|
// TestRegisteredProvidersHash_StableAcrossOrder: registry order
|
||||||
// shouldn't matter because we sort before hashing.
|
// shouldn't matter because we sort before hashing.
|
||||||
func TestRegisteredProvidersHash_StableAcrossOrder(t *testing.T) {
|
func TestRegisteredProvidersHash_StableAcrossOrder(t *testing.T) {
|
||||||
|
|||||||
@@ -0,0 +1,19 @@
|
|||||||
|
import { describe, it, expect, vi, beforeEach } from 'vitest';
|
||||||
|
import { researchMissingArt } from './admin';
|
||||||
|
|
||||||
|
vi.mock('./client', () => ({
|
||||||
|
api: { get: vi.fn(), post: vi.fn(), patch: vi.fn() }
|
||||||
|
}));
|
||||||
|
|
||||||
|
import { api } from './client';
|
||||||
|
|
||||||
|
describe('admin research missing art API', () => {
|
||||||
|
beforeEach(() => vi.clearAllMocks());
|
||||||
|
|
||||||
|
it('researchMissingArt POSTs the correct path and returns version', async () => {
|
||||||
|
(api.post as unknown as ReturnType<typeof vi.fn>).mockResolvedValueOnce({ version: 7 });
|
||||||
|
const got = await researchMissingArt();
|
||||||
|
expect(api.post).toHaveBeenCalledWith('/api/admin/cover-sources/research', {});
|
||||||
|
expect(got.version).toBe(7);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -320,6 +320,17 @@ export async function testCoverProvider(id: string): Promise<TestCoverProviderRe
|
|||||||
return api.post<TestCoverProviderResponse>(`/api/admin/cover-sources/${id}/test`, {});
|
return api.post<TestCoverProviderResponse>(`/api/admin/cover-sources/${id}/test`, {});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Re-search missing art --------------------------------------------------
|
||||||
|
|
||||||
|
export type ResearchMissingArtResponse = { version: number };
|
||||||
|
|
||||||
|
// Bumps cover_art_sources_meta.current_version. Every 'none' row becomes
|
||||||
|
// eligible for retry against the current provider chain on the next
|
||||||
|
// enrichment pass. Existing positively-sourced rows are not affected.
|
||||||
|
export async function researchMissingArt(): Promise<ResearchMissingArtResponse> {
|
||||||
|
return api.post<ResearchMissingArtResponse>('/api/admin/cover-sources/research', {});
|
||||||
|
}
|
||||||
|
|
||||||
// Settings rarely change in operator time. 60s staleTime; refetches
|
// Settings rarely change in operator time. 60s staleTime; refetches
|
||||||
// on window focus + after any mutation rather than polling.
|
// on window focus + after any mutation rather than polling.
|
||||||
export function createCoverProvidersQuery() {
|
export function createCoverProvidersQuery() {
|
||||||
|
|||||||
@@ -19,6 +19,7 @@
|
|||||||
deleteQuarantineViaLidarr,
|
deleteQuarantineViaLidarr,
|
||||||
refetchMissingCovers,
|
refetchMissingCovers,
|
||||||
triggerScan,
|
triggerScan,
|
||||||
|
researchMissingArt,
|
||||||
type ScanSchedule,
|
type ScanSchedule,
|
||||||
type ScanScheduleMode
|
type ScanScheduleMode
|
||||||
} from '$lib/api/admin';
|
} from '$lib/api/admin';
|
||||||
@@ -315,6 +316,21 @@
|
|||||||
let bulkBusy = $state(false);
|
let bulkBusy = $state(false);
|
||||||
let bulkResult = $state<string | null>(null);
|
let bulkResult = $state<string | null>(null);
|
||||||
|
|
||||||
|
// ---- Re-search missing art ----
|
||||||
|
let researchSaving = $state(false);
|
||||||
|
|
||||||
|
async function onResearchMissingArt() {
|
||||||
|
researchSaving = true;
|
||||||
|
try {
|
||||||
|
await researchMissingArt();
|
||||||
|
showToast('All previously-failed art will be re-attempted on the next scan.');
|
||||||
|
} catch (e) {
|
||||||
|
showToast(`Re-search failed: ${(e as { code?: string })?.code ?? 'unknown'}`);
|
||||||
|
} finally {
|
||||||
|
researchSaving = false;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
async function onBulkRefetch() {
|
async function onBulkRefetch() {
|
||||||
if (bulkBusy) return;
|
if (bulkBusy) return;
|
||||||
bulkBusy = true;
|
bulkBusy = true;
|
||||||
@@ -603,6 +619,14 @@
|
|||||||
>
|
>
|
||||||
{bulkBusy ? 'Queueing…' : 'Refetch missing covers'}
|
{bulkBusy ? 'Queueing…' : 'Refetch missing covers'}
|
||||||
</button>
|
</button>
|
||||||
|
<button
|
||||||
|
type="button"
|
||||||
|
onclick={onResearchMissingArt}
|
||||||
|
disabled={researchSaving}
|
||||||
|
class="rounded-md border border-border px-3 py-1.5 text-sm text-text-secondary hover:bg-surface-hover hover:text-text-primary disabled:opacity-50"
|
||||||
|
>
|
||||||
|
{researchSaving ? 'Bumping…' : 'Re-search missing art'}
|
||||||
|
</button>
|
||||||
|
|
||||||
{#if coverage}
|
{#if coverage}
|
||||||
{#if coverage.total === 0}
|
{#if coverage.total === 0}
|
||||||
|
|||||||
@@ -42,6 +42,7 @@ vi.mock('$lib/api/admin', async () => {
|
|||||||
deleteQuarantineViaLidarr: vi.fn().mockResolvedValue({}),
|
deleteQuarantineViaLidarr: vi.fn().mockResolvedValue({}),
|
||||||
triggerScan: vi.fn().mockResolvedValue({}),
|
triggerScan: vi.fn().mockResolvedValue({}),
|
||||||
refetchMissingCovers: vi.fn().mockResolvedValue({ queued: 0 }),
|
refetchMissingCovers: vi.fn().mockResolvedValue({ queued: 0 }),
|
||||||
|
researchMissingArt: vi.fn().mockResolvedValue({ version: 1 }),
|
||||||
createScanScheduleQuery: vi.fn(() =>
|
createScanScheduleQuery: vi.fn(() =>
|
||||||
readable({
|
readable({
|
||||||
data: {
|
data: {
|
||||||
@@ -111,7 +112,8 @@ import {
|
|||||||
rejectRequest,
|
rejectRequest,
|
||||||
resolveQuarantine,
|
resolveQuarantine,
|
||||||
deleteQuarantineFile,
|
deleteQuarantineFile,
|
||||||
deleteQuarantineViaLidarr
|
deleteQuarantineViaLidarr,
|
||||||
|
researchMissingArt
|
||||||
} from '$lib/api/admin';
|
} from '$lib/api/admin';
|
||||||
import OverviewPage from './+page.svelte';
|
import OverviewPage from './+page.svelte';
|
||||||
|
|
||||||
@@ -264,6 +266,15 @@ describe('admin Overview page', () => {
|
|||||||
expect(select.value).toBe('off');
|
expect(select.value).toBe('off');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test('renders Re-search missing art button and invokes on click', async () => {
|
||||||
|
setupOverview();
|
||||||
|
render(OverviewPage);
|
||||||
|
const btn = screen.getByRole('button', { name: /re-search missing art/i });
|
||||||
|
expect(btn).toBeInTheDocument();
|
||||||
|
await fireEvent.click(btn);
|
||||||
|
expect(researchMissingArt).toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
test('Library scan card shows Processing badge when library tally is populated and in-flight', async () => {
|
test('Library scan card shows Processing badge when library tally is populated and in-flight', async () => {
|
||||||
const { readable } = await import('svelte/store');
|
const { readable } = await import('svelte/store');
|
||||||
(createScanStatusQuery as ReturnType<typeof vi.fn>).mockReturnValueOnce(
|
(createScanStatusQuery as ReturnType<typeof vi.fn>).mockReturnValueOnce(
|
||||||
|
|||||||
Reference in New Issue
Block a user