feat(discover): Discover tuning card on the admin lab — #2377 (web admin)
test-web / test (push) Successful in 35s
test-web / test (push) Successful in 35s
Rule #25/#27: the two knobs slice 6 added server-side are now touchable — taste-tag weight and snooze length, with deviation dots, save, and reset, matching the existing profile/taste cards. Copy states what each knob does AND what it doesn't: the tag-weight hint says 0 turns the term off and that an untagged candidate is never penalised, and the snooze hint says it records no opinion about the artist and never feeds the taste profile. Those are the two properties most likely to be assumed backwards by whoever turns these next. Also fixed a latent fragility the new card exposed rather than caused: all three reset buttons had the accessible name "Reset to defaults", so the existing test picked the LAST one and assumed that meant taste. Adding a card below it would have silently retargeted that assertion at the wrong scope. Each reset button now names its scope — better for screen readers too, since three identical buttons on one page is a real a11y defect — and the test selects by name instead of position. The page's test fixture needed the new `discover` key in both `snapshot` and `shipped`: the `as TuningSnapshot` cast means a missing field is not a compile error, it's every test on the page throwing inside fillForm. Noted that in the fixture so the next scope doesn't rediscover it. Includes a test that a weight of 0 is actually SENT rather than dropped as falsy — the off switch is the one value a truthiness bug would eat. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -43,13 +43,25 @@ const taste = (over: Partial<Record<string, number>> = {}) => ({
|
||||
...over
|
||||
});
|
||||
|
||||
const discover = (over: Partial<Record<string, number>> = {}) => ({
|
||||
tag_overlap_weight: 1,
|
||||
snooze_days: 90,
|
||||
...over
|
||||
});
|
||||
|
||||
// NOTE the `as TuningSnapshot` cast below: it silences the type error when the
|
||||
// server snapshot grows a field, so a missing key here surfaces as every test
|
||||
// on this page throwing inside fillForm rather than as a compile failure. Add
|
||||
// new scopes to BOTH this fixture and `shipped`.
|
||||
function snapshot(over: Partial<TuningSnapshot> = {}): TuningSnapshot {
|
||||
return {
|
||||
profiles: { radio: weights({ taste_weight: 1 }), daily_mix: weights() },
|
||||
taste: taste(),
|
||||
discover: discover(),
|
||||
shipped: {
|
||||
profiles: { radio: weights({ taste_weight: 1 }), daily_mix: weights() },
|
||||
taste: taste()
|
||||
taste: taste(),
|
||||
discover: discover()
|
||||
},
|
||||
...over
|
||||
} as TuningSnapshot;
|
||||
@@ -99,13 +111,17 @@ describe('Admin tuning page', () => {
|
||||
await waitFor(() => expect(pushToast).toHaveBeenCalledWith('Nothing changed.'));
|
||||
});
|
||||
|
||||
// Targets the button by its accessible name, not its position. This used to
|
||||
// click the LAST reset button and assume that meant taste — which silently
|
||||
// became the wrong scope the moment a card was added below it.
|
||||
test('reset calls resetTuning for the scope', async () => {
|
||||
(getTuning as ReturnType<typeof vi.fn>).mockResolvedValue(snapshot());
|
||||
(resetTuning as ReturnType<typeof vi.fn>).mockResolvedValue(snapshot());
|
||||
render(TuningPage);
|
||||
await waitFor(() => expect(screen.getByText('Taste profile build')).toBeInTheDocument());
|
||||
const resetButtons = screen.getAllByRole('button', { name: /reset to defaults/i });
|
||||
await fireEvent.click(resetButtons[resetButtons.length - 1]);
|
||||
await fireEvent.click(
|
||||
screen.getByRole('button', { name: /reset taste profile build to defaults/i })
|
||||
);
|
||||
await waitFor(() => expect(resetTuning).toHaveBeenCalledWith('taste'));
|
||||
});
|
||||
|
||||
@@ -198,4 +214,62 @@ describe('Admin tuning page', () => {
|
||||
expect(screen.getByText(/trends appear once listening accumulates/i)).toBeInTheDocument()
|
||||
);
|
||||
});
|
||||
// --- Discover scope (#2377) ---
|
||||
|
||||
test('renders the Discover card with its two knobs', async () => {
|
||||
(getTuning as ReturnType<typeof vi.fn>).mockResolvedValue(snapshot());
|
||||
render(TuningPage);
|
||||
await waitFor(() => expect(screen.getByText('Discover requests')).toBeInTheDocument());
|
||||
expect(screen.getByLabelText(/taste-tag weight/i)).toBeInTheDocument();
|
||||
expect(screen.getByLabelText(/snooze length/i)).toBeInTheDocument();
|
||||
});
|
||||
|
||||
test('saving Discover patches only the changed field under the discover scope', async () => {
|
||||
(getTuning as ReturnType<typeof vi.fn>).mockResolvedValue(snapshot());
|
||||
(patchTuning as ReturnType<typeof vi.fn>).mockResolvedValue(snapshot());
|
||||
render(TuningPage);
|
||||
await waitFor(() => expect(screen.getByText('Discover requests')).toBeInTheDocument());
|
||||
await fireEvent.input(screen.getByLabelText(/taste-tag weight/i), {
|
||||
target: { value: '2.5' }
|
||||
});
|
||||
await fireEvent.click(screen.getByRole('button', { name: /save discover/i }));
|
||||
await waitFor(() =>
|
||||
expect(patchTuning).toHaveBeenCalledWith('discover', { tag_overlap_weight: 2.5 })
|
||||
);
|
||||
});
|
||||
|
||||
// 0 is the operator's off switch for the whole tag term, so the form must be
|
||||
// able to send it — a falsy-value bug here would silently make the feature
|
||||
// impossible to disable.
|
||||
test('a Discover weight of 0 is sent, not dropped as falsy', async () => {
|
||||
(getTuning as ReturnType<typeof vi.fn>).mockResolvedValue(snapshot());
|
||||
(patchTuning as ReturnType<typeof vi.fn>).mockResolvedValue(snapshot());
|
||||
render(TuningPage);
|
||||
await waitFor(() => expect(screen.getByText('Discover requests')).toBeInTheDocument());
|
||||
await fireEvent.input(screen.getByLabelText(/taste-tag weight/i), { target: { value: '0' } });
|
||||
await fireEvent.click(screen.getByRole('button', { name: /save discover/i }));
|
||||
await waitFor(() =>
|
||||
expect(patchTuning).toHaveBeenCalledWith('discover', { tag_overlap_weight: 0 })
|
||||
);
|
||||
});
|
||||
|
||||
test('reset targets the discover scope', async () => {
|
||||
(getTuning as ReturnType<typeof vi.fn>).mockResolvedValue(snapshot());
|
||||
(resetTuning as ReturnType<typeof vi.fn>).mockResolvedValue(snapshot());
|
||||
render(TuningPage);
|
||||
await waitFor(() => expect(screen.getByText('Discover requests')).toBeInTheDocument());
|
||||
await fireEvent.click(
|
||||
screen.getByRole('button', { name: /reset discover requests to defaults/i })
|
||||
);
|
||||
await waitFor(() => expect(resetTuning).toHaveBeenCalledWith('discover'));
|
||||
});
|
||||
|
||||
test('a deviating Discover knob is dotted', async () => {
|
||||
const snap = snapshot();
|
||||
snap.discover = discover({ tag_overlap_weight: 3 });
|
||||
(getTuning as ReturnType<typeof vi.fn>).mockResolvedValue(snap);
|
||||
render(TuningPage);
|
||||
await waitFor(() => expect(screen.getByText('Discover requests')).toBeInTheDocument());
|
||||
expect(screen.getByTitle(/deviates from the shipped default \(1\)/i)).toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user