feat(web): re-acquisition settings card on the missing-files page — #2527
test-web / test (push) Successful in 34s
test-web / test (push) Successful in 34s
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.
This commit is contained in:
@@ -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<ReacquisitionSettings> = {}) {
|
||||
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();
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user