fix(69): key Auto-Tune selection by axis+value; zero-templates UX (review)
Build ErsatzTV Image / Docs update reminder (pull_request) Successful in 7s
Build ErsatzTV Image / decisions.md append-only (pull_request) Successful in 7s
Build ErsatzTV Image / Functional E2E (curl contracts) (pull_request) Successful in 5m43s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Successful in 9s
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Successful in 8s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 8m3s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 12m10s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
Build ErsatzTV Image / Docs update reminder (pull_request) Successful in 7s
Build ErsatzTV Image / decisions.md append-only (pull_request) Successful in 7s
Build ErsatzTV Image / Functional E2E (curl contracts) (pull_request) Successful in 5m43s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Successful in 9s
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Successful in 8s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 8m3s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 12m10s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
Addresses the cold-review findings on PR #389: - Medium: selection was keyed by proposal name, coupling two same-named proposals (e.g. a show titled "Comedy" and the "Comedy" genre) so toggling one flipped both. Now keyed by axis+value (unique); adds a regression test. - Low: zero channel templates now shows a hint on Configure and the Create tooltip explains the missing template instead of a misleading positive label. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -150,4 +150,54 @@ describe('AutoTuneScreen', () => {
|
||||
{ axis: 'TvGenre', value: 'Comedy', name: 'Comedy', number: '502' }
|
||||
]);
|
||||
});
|
||||
|
||||
it('keys selection by axis+value so same-named proposals stay independent', async () => {
|
||||
// A TV show literally titled "Comedy" and the "Comedy" TV genre — identical names, different
|
||||
// axes. Keying selection on the name would couple them (toggling one flips both).
|
||||
const collision = [
|
||||
{ axis: 'TvShow', value: 'Comedy', name: 'Comedy', number: '500', itemCount: 30, alreadyExists: false },
|
||||
{ axis: 'TvGenre', value: 'Comedy', name: 'Comedy', number: '501', itemCount: 640, alreadyExists: false }
|
||||
];
|
||||
const fetchSpy = vi.spyOn(window, 'fetch').mockImplementation((input: RequestInfo | URL, init?: RequestInit) => {
|
||||
const path = input.toString();
|
||||
const method = (init?.method ?? 'GET').toUpperCase();
|
||||
if (path === '/api/v1/channel-templates') {
|
||||
return Promise.resolve(jsonResponse(templates));
|
||||
}
|
||||
if (path === '/api/v1/channel-templates/default') {
|
||||
return Promise.resolve(jsonResponse(templates[0]));
|
||||
}
|
||||
if (path === '/api/v1/channels/auto-tune/preview' && method === 'POST') {
|
||||
return Promise.resolve(jsonResponse(collision));
|
||||
}
|
||||
if (path === '/api/v1/channels/auto-tune' && method === 'POST') {
|
||||
return Promise.resolve(jsonResponse({ results: [], createdCount: 0, skippedCount: 0, failedCount: 0 }));
|
||||
}
|
||||
return Promise.resolve(jsonResponse(null, 404));
|
||||
});
|
||||
|
||||
render(<AutoTuneScreen />);
|
||||
fireEvent.click(screen.getByRole('button', { name: /Preview channels/ }));
|
||||
await screen.findByText('TV Shows');
|
||||
|
||||
// Two distinct row checkboxes both named "Comedy" (one per axis), both selected by default.
|
||||
const boxes = screen.getAllByRole('checkbox', { name: 'Comedy' });
|
||||
expect(boxes).toHaveLength(2);
|
||||
expect(boxes[0]).toHaveAttribute('aria-checked', 'true');
|
||||
expect(boxes[1]).toHaveAttribute('aria-checked', 'true');
|
||||
|
||||
// Deselecting the TV-show "Comedy" must NOT deselect the genre "Comedy".
|
||||
fireEvent.click(boxes[0]);
|
||||
const after = screen.getAllByRole('checkbox', { name: 'Comedy' });
|
||||
expect(after[0]).toHaveAttribute('aria-checked', 'false');
|
||||
expect(after[1]).toHaveAttribute('aria-checked', 'true');
|
||||
|
||||
// Exactly one remains selected → create posts only the still-selected genre proposal.
|
||||
fireEvent.click(screen.getByRole('button', { name: /Create 1 channel/ }));
|
||||
const createCall = fetchSpy.mock.calls.find(
|
||||
(call) => call[0] === '/api/v1/channels/auto-tune' && (call[1]?.method ?? '').toUpperCase() === 'POST'
|
||||
);
|
||||
const body = JSON.parse(String(createCall?.[1]?.body));
|
||||
expect(body.channels).toEqual([{ axis: 'TvGenre', value: 'Comedy', name: 'Comedy', number: '501' }]);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -61,6 +61,12 @@ const eyebrowStyle: CSSProperties = {
|
||||
|
||||
const monoStyle: CSSProperties = { fontFamily: 'var(--font-mono)', fontVariantNumeric: 'tabular-nums' };
|
||||
|
||||
// Selection identity — axis+value uniquely identifies a proposal. Keying on the display
|
||||
// name would couple two same-named proposals (e.g. a TV show titled "Comedy" and the
|
||||
// "Comedy" genre), flipping/creating both. An axis id never contains a space, so a single
|
||||
// space separates it unambiguously from the metadata value.
|
||||
const proposalKey = (proposal: { axis: string; value: string }) => `${proposal.axis} ${proposal.value}`;
|
||||
|
||||
type TemplatesState =
|
||||
| { status: 'loading' }
|
||||
| { status: 'error'; message: string }
|
||||
@@ -163,7 +169,7 @@ export function AutoTuneScreen() {
|
||||
}
|
||||
setPreviewState({ status: 'success', proposals });
|
||||
// Default selection: every proposal that doesn't already exist.
|
||||
setSelected(new Set(proposals.filter((proposal) => !proposal.alreadyExists).map((proposal) => proposal.name)));
|
||||
setSelected(new Set(proposals.filter((proposal) => !proposal.alreadyExists).map(proposalKey)));
|
||||
})
|
||||
.catch((error: unknown) => {
|
||||
if (activeRef.current && id === previewSeqRef.current) {
|
||||
@@ -173,7 +179,9 @@ export function AutoTuneScreen() {
|
||||
};
|
||||
|
||||
const proposals = previewState.status === 'success' ? previewState.proposals : [];
|
||||
const selectedProposals = proposals.filter((proposal) => !proposal.alreadyExists && selected.has(proposal.name));
|
||||
const selectedProposals = proposals.filter(
|
||||
(proposal) => !proposal.alreadyExists && selected.has(proposalKey(proposal))
|
||||
);
|
||||
|
||||
const runCreate = () => {
|
||||
if (selectedProposals.length === 0 || templateId === '') {
|
||||
@@ -276,7 +284,13 @@ export function AutoTuneScreen() {
|
||||
</Button>
|
||||
<Tooltip
|
||||
placement="bottom"
|
||||
label={selectedProposals.length ? 'Create the selected channels' : 'Select at least one channel'}
|
||||
label={
|
||||
selectedProposals.length === 0
|
||||
? 'Select at least one channel'
|
||||
: templateId === ''
|
||||
? 'Choose a channel template on the Configure step first'
|
||||
: 'Create the selected channels'
|
||||
}
|
||||
>
|
||||
<Button
|
||||
variant="primary"
|
||||
@@ -544,6 +558,13 @@ function ConfigureStep({
|
||||
Retry
|
||||
</Button>
|
||||
</div>
|
||||
) : templateOptions.length === 0 ? (
|
||||
<div
|
||||
role="status"
|
||||
style={{ font: 'var(--text-xs)/1.4 var(--font-sans)', color: 'var(--text-secondary)', padding: '4px 2px' }}
|
||||
>
|
||||
No channel templates yet — create one under Templates first.
|
||||
</div>
|
||||
) : (
|
||||
<Select
|
||||
size="sm"
|
||||
@@ -606,7 +627,7 @@ function PreviewStep({
|
||||
|
||||
const selectable = proposals.filter((proposal) => !proposal.alreadyExists);
|
||||
const existingCount = proposals.length - selectable.length;
|
||||
const selectedCount = selectable.filter((proposal) => selected.has(proposal.name)).length;
|
||||
const selectedCount = selectable.filter((proposal) => selected.has(proposalKey(proposal))).length;
|
||||
|
||||
const setAll = (rows: AutoTuneProposal[], on: boolean) => {
|
||||
setSelected((current) => {
|
||||
@@ -614,9 +635,9 @@ function PreviewStep({
|
||||
rows.forEach((row) => {
|
||||
if (!row.alreadyExists) {
|
||||
if (on) {
|
||||
next.add(row.name);
|
||||
next.add(proposalKey(row));
|
||||
} else {
|
||||
next.delete(row.name);
|
||||
next.delete(proposalKey(row));
|
||||
}
|
||||
}
|
||||
});
|
||||
@@ -624,13 +645,13 @@ function PreviewStep({
|
||||
});
|
||||
};
|
||||
|
||||
const toggle = (name: string) => {
|
||||
const toggle = (key: string) => {
|
||||
setSelected((current) => {
|
||||
const next = new Set(current);
|
||||
if (next.has(name)) {
|
||||
next.delete(name);
|
||||
if (next.has(key)) {
|
||||
next.delete(key);
|
||||
} else {
|
||||
next.add(name);
|
||||
next.add(key);
|
||||
}
|
||||
return next;
|
||||
});
|
||||
@@ -663,8 +684,8 @@ function PreviewStep({
|
||||
|
||||
{groups.map(({ axis, rows }) => {
|
||||
const groupSelectable = rows.filter((row) => !row.alreadyExists);
|
||||
const allOn = groupSelectable.length > 0 && groupSelectable.every((row) => selected.has(row.name));
|
||||
const someOn = groupSelectable.some((row) => selected.has(row.name));
|
||||
const allOn = groupSelectable.length > 0 && groupSelectable.every((row) => selected.has(proposalKey(row)));
|
||||
const someOn = groupSelectable.some((row) => selected.has(proposalKey(row)));
|
||||
const AxisIcon = axis.icon;
|
||||
const OrderIcon = axis.shuffled ? Shuffle : ListOrdered;
|
||||
return (
|
||||
@@ -707,10 +728,10 @@ function PreviewStep({
|
||||
</div>
|
||||
<div>
|
||||
{rows.map((row, i) => {
|
||||
const on = selected.has(row.name);
|
||||
const on = selected.has(proposalKey(row));
|
||||
return (
|
||||
<div
|
||||
key={row.name}
|
||||
key={proposalKey(row)}
|
||||
style={{
|
||||
borderTop: i ? '1px solid var(--border-hairline)' : 'none',
|
||||
opacity: row.alreadyExists ? 0.55 : 1
|
||||
@@ -721,7 +742,7 @@ function PreviewStep({
|
||||
checked={on && !row.alreadyExists}
|
||||
disabled={row.alreadyExists}
|
||||
label={row.name}
|
||||
onChange={() => toggle(row.name)}
|
||||
onChange={() => toggle(proposalKey(row))}
|
||||
/>
|
||||
<span style={{ ...monoStyle, minWidth: 42, font: 'var(--text-sm) var(--font-mono)', color: 'var(--text-secondary)' }}>
|
||||
{row.number}
|
||||
|
||||
Reference in New Issue
Block a user