From 952132714e36eb73a78f7452f8a0a8f751c2fa97 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Mon, 17 Aug 2026 00:13:20 -0400 Subject: [PATCH] =?UTF-8?q?feat(web):=20re-acquisition=20settings=20card?= =?UTF-8?q?=20on=20the=20missing-files=20page=20=E2=80=94=20#2527?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rule #27: the sweeper has been running since bab9b168 with no way to see or change what it does. This is the half that makes it a feature. Placed above the list it governs rather than under Integrations. An operator looking at missing files is exactly the person deciding what should happen to them; Lidarr is the mechanism, not the subject, and separating the policy from the problem would mean finding one to understand the other. The card states the retry schedule the numbers add up to -- "6h -> 12h -> 24h" -- because the fields are meaningless individually. "First retry gap: 6" tells you nothing until you know it doubles and where it stops, and an operator should not have to simulate the algorithm to predict it. It recomputes as they type, including the clamp. It also states the unnameable-album count with its reason. Those albums will never produce a request no matter how long they sit in the list below, because Lidarr cannot be asked for a release MusicBrainz cannot name. Watching rows never move with no explanation is how a working feature gets reported as broken. Save errors surface the server's own message. The Go layer validates the same ranges the CHECKs enforce and names the field, so the operator reads "grace_hours must be 1-720" rather than a generic failure. The dirty check compares only the stored fields: unnameable_albums is server-computed, and including it would make the form look edited whenever the library changed underneath. Nine tests, including the schedule clamp, the disabled-until-dirty Save, the surfaced validation message, and a failed load offering a retry instead of an empty card. The existing missing-files page suite gains a stub for the card's own settings fetch -- it mocks the whole admin API module, so the card's imports would otherwise be undefined at mount. --- web/src/lib/api/admin.ts | 26 ++ .../ReacquisitionSettingsCard.svelte | 236 ++++++++++++++++++ .../ReacquisitionSettingsCard.test.ts | 120 +++++++++ .../routes/admin/missing-files/+page.svelte | 6 + .../admin/missing-files/missing-files.test.ts | 16 +- 5 files changed, 403 insertions(+), 1 deletion(-) create mode 100644 web/src/lib/components/ReacquisitionSettingsCard.svelte create mode 100644 web/src/lib/components/ReacquisitionSettingsCard.test.ts diff --git a/web/src/lib/api/admin.ts b/web/src/lib/api/admin.ts index c4cdd519..2428d79f 100644 --- a/web/src/lib/api/admin.ts +++ b/web/src/lib/api/admin.ts @@ -696,3 +696,29 @@ export function createMissingFilesQuery(offset: number = 0, limit: number = 50) staleTime: 120_000 }); } + +// Missing-file re-acquisition (#2527 / milestone #290) ---------------------- + +export type ReacquisitionSettings = { + enabled: boolean; + grace_hours: number; + backoff_base_hours: number; + backoff_max_hours: number; + max_attempts: number; + max_per_pass: number; + auto_approve: boolean; + // Albums with missing files that can never be auto-requested because + // neither they nor their artist carries an MBID. Read-only; the server + // computes it, and the card states it so the gap isn't a mystery. + unnameable_albums: number; +}; + +export async function getReacquisitionSettings(): Promise { + return api.get('/api/admin/library/reacquisition'); +} + +export async function updateReacquisitionSettings( + s: ReacquisitionSettings +): Promise { + return api.put('/api/admin/library/reacquisition', s); +} diff --git a/web/src/lib/components/ReacquisitionSettingsCard.svelte b/web/src/lib/components/ReacquisitionSettingsCard.svelte new file mode 100644 index 00000000..9f37b1dc --- /dev/null +++ b/web/src/lib/components/ReacquisitionSettingsCard.svelte @@ -0,0 +1,236 @@ + + +
+
+

Automatic re-acquisition

+

+ When a file has been missing for a while, Minstrel can ask Lidarr for the album + again by itself. Requests are made per album, not per track — a whole folder + going missing is one request, not forty. +

+
+ + {#if loadError} +

+ Couldn't load re-acquisition settings. + +

+ {:else if form === null} +

Loading…

+ {:else} + + +
+ + + + + + + + + +
+ +

+ Retry schedule: {schedule} after the + first attempt. +

+ + + + {#if saved && saved.unnameable_albums > 0} + +

+

+ {/if} + +
+ +
+ {/if} +
diff --git a/web/src/lib/components/ReacquisitionSettingsCard.test.ts b/web/src/lib/components/ReacquisitionSettingsCard.test.ts new file mode 100644 index 00000000..7cfea1dd --- /dev/null +++ b/web/src/lib/components/ReacquisitionSettingsCard.test.ts @@ -0,0 +1,120 @@ +import { afterEach, describe, expect, test, vi } from 'vitest'; +import { render, screen, fireEvent, waitFor } from '@testing-library/svelte'; +import type { ReacquisitionSettings } from '$lib/api/admin'; + +vi.mock('$lib/api/admin', () => ({ + getReacquisitionSettings: vi.fn(), + updateReacquisitionSettings: vi.fn() +})); + +vi.mock('$lib/stores/toast.svelte', () => ({ pushToast: vi.fn() })); + +import ReacquisitionSettingsCard from './ReacquisitionSettingsCard.svelte'; +import { getReacquisitionSettings, updateReacquisitionSettings } from '$lib/api/admin'; +import { pushToast } from '$lib/stores/toast.svelte'; + +const base: ReacquisitionSettings = { + enabled: true, + grace_hours: 24, + backoff_base_hours: 6, + backoff_max_hours: 168, + max_attempts: 3, + max_per_pass: 20, + auto_approve: true, + unnameable_albums: 0 +}; + +afterEach(() => vi.clearAllMocks()); + +async function renderCard(over: Partial = {}) { + vi.mocked(getReacquisitionSettings).mockResolvedValue({ ...base, ...over }); + const r = render(ReacquisitionSettingsCard); + await waitFor(() => expect(getReacquisitionSettings).toHaveBeenCalled()); + return r; +} + +describe('ReacquisitionSettingsCard', () => { + // The individual numbers say nothing on their own — "6 hours" is meaningless + // until you know it doubles — so the card spells the schedule out. + test('states the retry schedule the numbers add up to', async () => { + await renderCard(); + await waitFor(() => expect(screen.getByText('6h → 12h → 24h')).toBeTruthy()); + }); + + test('the schedule clamps at the longest gap', async () => { + await renderCard({ backoff_base_hours: 6, backoff_max_hours: 12, max_attempts: 4 }); + await waitFor(() => expect(screen.getByText('6h → 12h → 12h → 12h')).toBeTruthy()); + }); + + test('the schedule follows the attempt count', async () => { + await renderCard({ max_attempts: 1 }); + await waitFor(() => expect(screen.getByText('6h')).toBeTruthy()); + }); + + // Saving an unchanged form would be a pointless round-trip, and a live Save + // button invites the operator to wonder whether anything happened. + test('save is disabled until something changes', async () => { + await renderCard(); + const save = await screen.findByRole('button', { name: /save/i }); + expect(save).toHaveProperty('disabled', true); + + const grace = screen.getByRole('spinbutton', { name: /wait before the first attempt/i }); + await fireEvent.input(grace, { target: { value: '48' } }); + await waitFor(() => expect(save).toHaveProperty('disabled', false)); + }); + + test('saving sends the edited values', async () => { + vi.mocked(updateReacquisitionSettings).mockResolvedValue({ ...base, grace_hours: 48 }); + await renderCard(); + + const grace = screen.getByRole('spinbutton', { name: /wait before the first attempt/i }); + await fireEvent.input(grace, { target: { value: '48' } }); + await fireEvent.click(await screen.findByRole('button', { name: /save/i })); + + await waitFor(() => + expect(updateReacquisitionSettings).toHaveBeenCalledWith( + expect.objectContaining({ grace_hours: 48 }) + ) + ); + }); + + // The server names the offending field ("grace_hours must be 1-720"); a + // generic "couldn't save" would throw that away. + test('a rejected save surfaces the server message', async () => { + vi.mocked(updateReacquisitionSettings).mockRejectedValue( + new Error('grace_hours must be 1-720') + ); + await renderCard(); + + const grace = screen.getByRole('spinbutton', { name: /wait before the first attempt/i }); + await fireEvent.input(grace, { target: { value: '900' } }); + await fireEvent.click(await screen.findByRole('button', { name: /save/i })); + + await waitFor(() => + expect(pushToast).toHaveBeenCalledWith('grace_hours must be 1-720', 'error') + ); + }); + + // Without this the operator watches those albums never get a request and + // reasonably concludes the feature is broken. + test('unnameable albums are called out with the reason', async () => { + await renderCard({ unnameable_albums: 3 }); + await waitFor(() => expect(screen.getByText(/no\s+MusicBrainz ID/i)).toBeTruthy()); + expect(screen.getByText(/3\s+albums have/i)).toBeTruthy(); + }); + + test('no warning when every missing album is identifiable', async () => { + await renderCard({ unnameable_albums: 0 }); + await waitFor(() => expect(screen.getByText('6h → 12h → 24h')).toBeTruthy()); + expect(screen.queryByText(/MusicBrainz ID/i)).toBeNull(); + }); + + test('a failed load offers a retry rather than an empty card', async () => { + vi.mocked(getReacquisitionSettings).mockRejectedValue(new Error('nope')); + render(ReacquisitionSettingsCard); + await waitFor(() => + expect(screen.getByText(/couldn't load re-acquisition settings/i)).toBeTruthy() + ); + expect(screen.getByRole('button', { name: /try again/i })).toBeTruthy(); + }); +}); diff --git a/web/src/routes/admin/missing-files/+page.svelte b/web/src/routes/admin/missing-files/+page.svelte index 9c552ef7..3e8a4172 100644 --- a/web/src/routes/admin/missing-files/+page.svelte +++ b/web/src/routes/admin/missing-files/+page.svelte @@ -4,6 +4,7 @@ import { createMissingFilesQuery } from '$lib/api/admin'; import { relativeTime } from '$lib/utils/relativeTime'; import { coverUrl } from '$lib/media/covers'; + import ReacquisitionSettingsCard from '$lib/components/ReacquisitionSettingsCard.svelte'; import type { AdminMissingGroup } from '$lib/api/types'; // Files the scan looked for and could not find. Read-only on purpose: @@ -57,6 +58,11 @@

+ + + {#if query.isPending}

Checking what's missing…

{:else if query.isError} diff --git a/web/src/routes/admin/missing-files/missing-files.test.ts b/web/src/routes/admin/missing-files/missing-files.test.ts index e1e5c515..2ba2970e 100644 --- a/web/src/routes/admin/missing-files/missing-files.test.ts +++ b/web/src/routes/admin/missing-files/missing-files.test.ts @@ -3,8 +3,22 @@ import { render, screen } from '@testing-library/svelte'; import { mockQuery } from '../../../test-utils/query'; import type { AdminMissingResponse } from '$lib/api/types'; +// The page now embeds ReacquisitionSettingsCard, which loads its own settings +// from this same module on mount. Stubbing both keeps these tests about the +// missing-files list — the card has its own suite. vi.mock('$lib/api/admin', () => ({ - createMissingFilesQuery: vi.fn() + createMissingFilesQuery: vi.fn(), + getReacquisitionSettings: vi.fn().mockResolvedValue({ + enabled: true, + grace_hours: 24, + backoff_base_hours: 6, + backoff_max_hours: 168, + max_attempts: 3, + max_per_pass: 20, + auto_approve: true, + unnameable_albums: 0 + }), + updateReacquisitionSettings: vi.fn() })); import AdminMissingFilesPage from './+page.svelte';