feat(explore): offer the autotag match on the album page
The complaint was having to notice the metadata was missing, then go and hunt the album down on the Autotag page. The album page now says it while you are looking at the thing: "MusicBrainz has a match for this album: <release> by <artist>", with Apply tags and Review in Autotag. Four things about it are load-bearing. **Applying is offered only where it would do the whole album.** A tagging group is a folder, so a multi-disc album is several, and one button that applied to the best-scoring group would leave the album holding a mix of old and new tags — the exact case the app's Blocking notification level exists for. `groupCount` is the test, and the answer there is review rather than apply. **It rewrites files, so it asks.** `confirmAction()` with an impact line that says it cannot be undone and that nothing is moved or deleted, because "rewrites your files" reads worse than it is. The apply goes through `ApplyAsync`, the registered-job path, so progress belongs to the jobs indicator and this page does not grow a second one — what it owes the user is the acknowledgement, because the button is here. The suggestion clears itself on success rather than inviting a second click while the job runs. **The banner does not quote a percentage.** The backend has a score and deliberately keeps it out of the sentence: 0.95 reads as a probability and is not one. Which release it is, is the part a person can judge. **"Review in Autotag" lands on that album.** The queue is sorted by score so the intended folder is often near the top, and "often" is a link that sometimes opens a different album. Autotag is a cached primary view, so there is no construction to hand a payload to: the request goes on as an attribute and the view *consumes* it, or every later visit would reopen a folder the user finished with long ago. `ICON_AUTOTAG` joins the vocabulary at the same time, on the rule `ICON_PLAYLIST` was chosen by — an icon names the noun it acts on, so a suggestion pointing at Autotag wears the Autotag destination's own mark. It was written inline in the sidebar; two call sites is where a name stops being one component's detail, so the sweep governs it now. Verified against the running app with a staged match: the banner, the confirm dialog's wording, and the navigation landing on the right folder with the attribute consumed. Closes #28
This commit is contained in:
@@ -0,0 +1,286 @@
|
||||
/**
|
||||
* Being told about a match while looking at the album.
|
||||
*
|
||||
* The complaint (#28) is that the user had to notice their metadata
|
||||
* was missing and then go and hunt the album down on the Autotag page.
|
||||
* So the suggestion is drawn here, with something to do about it — and
|
||||
* the something rewrites files on disk, which is what most of this
|
||||
* file is about.
|
||||
*
|
||||
* The confidence tier behind "MusicBrainz has a match" is decided in
|
||||
* the backend (`autotag.ConfidentTier`) so this page and strict
|
||||
* auto-accept cannot disagree about what it means; what is pinned here
|
||||
* is only what the page does with the answer.
|
||||
*/
|
||||
import { describe, expect, it, beforeEach } from 'vitest';
|
||||
import type { LitElement } from 'lit';
|
||||
import { page } from 'vitest/browser';
|
||||
|
||||
import '@components/explore-album-details/explore-album-details';
|
||||
import { stub, stubFailure, flush, resetHarness, calls } from '@test/support/harness';
|
||||
import { notificationStore } from '@store/notification-store';
|
||||
import '@components/notifications/notification-host';
|
||||
import { fixture, shadow, shadowAll } from '@test/support/render';
|
||||
|
||||
const MATCH = 'autotagservice.Service.MatchForAlbum';
|
||||
const APPLY = 'autotagservice.Service.ApplyAsync';
|
||||
|
||||
function match(over: Record<string, unknown> = {}) {
|
||||
return {
|
||||
groupKey: 'grp-1',
|
||||
recommendation: 'strong',
|
||||
score: 0.95,
|
||||
releaseMbid: 'rel-1',
|
||||
title: 'Glass Harbour',
|
||||
artistCredit: 'Tideline',
|
||||
trackCount: 10,
|
||||
groupCount: 1,
|
||||
...over,
|
||||
};
|
||||
}
|
||||
|
||||
async function albumPage(): Promise<LitElement> {
|
||||
const el = await fixture<LitElement>('explore-album-details', {
|
||||
albumName: 'Glass Harbour',
|
||||
localAlbumId: 7,
|
||||
});
|
||||
|
||||
await flush();
|
||||
await el.updateComplete;
|
||||
|
||||
return el;
|
||||
}
|
||||
|
||||
/** The confirm dialog attaches itself to the document on first use. */
|
||||
function confirmHost(): (LitElement & { shadowRoot: ShadowRoot | null }) | null {
|
||||
return document.querySelector('confirm-dialog');
|
||||
}
|
||||
|
||||
/** Press one of the dialog's own buttons, the way a person would. */
|
||||
async function pressConfirm(testid: string): Promise<void> {
|
||||
const host = confirmHost();
|
||||
|
||||
if (!host) throw new Error('confirm-dialog did not mount itself');
|
||||
|
||||
await host.updateComplete;
|
||||
host.shadowRoot
|
||||
?.querySelector<HTMLButtonElement>(`[data-testid="${testid}"]`)
|
||||
?.click();
|
||||
await host.updateComplete;
|
||||
await flush();
|
||||
}
|
||||
|
||||
/** Click one of the banner's buttons by its label. */
|
||||
async function pressBanner(el: LitElement, label: string): Promise<void> {
|
||||
shadowAll<HTMLElement>(el, '.autotag-match-actions wa-button')
|
||||
.find((b) => b.textContent?.includes(label))
|
||||
?.click();
|
||||
await flush();
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
resetHarness();
|
||||
notificationStore.clear();
|
||||
stub('explore.Service.BrowseReleases', []);
|
||||
stub('explore.Service.LookupReleaseGroup', null);
|
||||
stub('explore.Service.GetThumbnail', '');
|
||||
stub('library.Library.GetAlbumTracks', []);
|
||||
stub('library.Library.GetAlbumCompleteness', {
|
||||
owned: 0,
|
||||
expected: 0,
|
||||
known: false,
|
||||
complete: false,
|
||||
});
|
||||
stub('library.Library.GetAllLibrariesWithTrackCounts', []);
|
||||
stub('library.Library.GetFilePathsByRecordingMBIDs', {});
|
||||
stub(MATCH, null);
|
||||
});
|
||||
|
||||
describe('the autotag suggestion', () => {
|
||||
it('says nothing when the backend has nothing confident', async () => {
|
||||
const el = await albumPage();
|
||||
|
||||
expect(shadow(el, '.autotag-match')).toBeNull();
|
||||
});
|
||||
|
||||
/**
|
||||
* A pure catalog page has no files to retag, so the question is not
|
||||
* asked at all — this runs on every album open and a call that
|
||||
* cannot have an answer is a call not worth making.
|
||||
*/
|
||||
it('is not even asked about an album with no local files', async () => {
|
||||
stub(MATCH, match());
|
||||
|
||||
const el = await fixture<LitElement>('explore-album-details', {
|
||||
albumName: 'Glass Harbour',
|
||||
releaseGroupMBID: 'rg-1',
|
||||
});
|
||||
|
||||
await flush();
|
||||
await el.updateComplete;
|
||||
|
||||
expect(calls(MATCH)).toHaveLength(0);
|
||||
expect(shadow(el, '.autotag-match')).toBeNull();
|
||||
});
|
||||
|
||||
/**
|
||||
* The banner names the release rather than quoting a number: 0.95
|
||||
* reads as a probability and is not one, and which release it is, is
|
||||
* the thing the user can actually judge.
|
||||
*/
|
||||
it('names the release it is offering', async () => {
|
||||
stub(MATCH, match());
|
||||
|
||||
const el = await albumPage();
|
||||
const text = shadow(el, '.autotag-match')?.textContent ?? '';
|
||||
|
||||
expect(text).toContain('MusicBrainz has a match');
|
||||
expect(text).toContain('Glass Harbour');
|
||||
expect(text).toContain('Tideline');
|
||||
expect(text).not.toContain('95');
|
||||
});
|
||||
|
||||
it('offers both an apply and a review', async () => {
|
||||
stub(MATCH, match());
|
||||
|
||||
await albumPage();
|
||||
|
||||
await expect
|
||||
.element(page.getByRole('button', { name: 'Apply tags' }))
|
||||
.toBeInTheDocument();
|
||||
await expect
|
||||
.element(page.getByRole('button', { name: 'Review in Autotag' }))
|
||||
.toBeInTheDocument();
|
||||
});
|
||||
|
||||
/**
|
||||
* A tagging group is a folder, so a multi-disc album is several. One
|
||||
* button that applied to the best-scoring one would leave the album
|
||||
* holding a mix of old and new tags — which is the case the app's
|
||||
* Blocking notification level exists for, and is worth not creating.
|
||||
*/
|
||||
it('will not apply to an album filed as several folders', async () => {
|
||||
stub(MATCH, match({ groupCount: 2 }));
|
||||
|
||||
const el = await albumPage();
|
||||
const labels = shadowAll(el, '.autotag-match-actions wa-button').map(
|
||||
(b) => b.textContent?.trim(),
|
||||
);
|
||||
|
||||
expect(labels).toEqual(['Review in Autotag']);
|
||||
expect(shadow(el, '.autotag-match')?.textContent).toContain('2 folders');
|
||||
});
|
||||
|
||||
it('navigates to Autotag carrying the group key', async () => {
|
||||
stub(MATCH, match());
|
||||
|
||||
const el = await albumPage();
|
||||
const seen: CustomEvent[] = [];
|
||||
|
||||
el.addEventListener('navigate', (e) => seen.push(e as CustomEvent));
|
||||
|
||||
await pressBanner(el, 'Review');
|
||||
|
||||
expect(seen).toHaveLength(1);
|
||||
expect(seen[0]?.detail).toMatchObject({
|
||||
view: 'autotag',
|
||||
groupKey: 'grp-1',
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('applying from the album page', () => {
|
||||
/**
|
||||
* This rewrites tags in files on disk and cannot be undone, so it
|
||||
* asks first — and cancelling has to be a true no-op, not a
|
||||
* confirmation that fires the call anyway.
|
||||
*/
|
||||
it('asks before it writes, and cancelling writes nothing', async () => {
|
||||
stub(MATCH, match());
|
||||
stub(APPLY, null);
|
||||
|
||||
const el = await albumPage();
|
||||
|
||||
await pressBanner(el, 'Apply');
|
||||
|
||||
expect(confirmHost()).not.toBeNull();
|
||||
expect(calls(APPLY)).toHaveLength(0);
|
||||
|
||||
await pressConfirm('confirm-cancel');
|
||||
await el.updateComplete;
|
||||
|
||||
expect(calls(APPLY)).toHaveLength(0);
|
||||
expect(shadow(el, '.autotag-match')).not.toBeNull();
|
||||
});
|
||||
|
||||
/**
|
||||
* The impact line has to say the thing that cannot be taken back, in
|
||||
* those words — "cannot be undone" — and that nothing is deleted,
|
||||
* because "rewrites your files" reads worse than it is.
|
||||
*/
|
||||
it('says what cannot be undone', async () => {
|
||||
stub(MATCH, match());
|
||||
|
||||
const el = await albumPage();
|
||||
|
||||
await pressBanner(el, 'Apply');
|
||||
|
||||
const text = confirmHost()?.shadowRoot?.textContent ?? '';
|
||||
|
||||
expect(text).toContain('cannot be');
|
||||
expect(text).toContain('undone');
|
||||
expect(text.toLowerCase()).toContain('nothing is moved or deleted');
|
||||
});
|
||||
|
||||
/**
|
||||
* `ApplyAsync` is the registered-job path, so progress belongs to
|
||||
* the jobs indicator and this page does not grow a second one. What
|
||||
* it owes the user is an acknowledgement, because the button is
|
||||
* here — and the suggestion has to stop inviting a second click.
|
||||
*/
|
||||
it('hands the work to the job registry and stands down', async () => {
|
||||
stub(MATCH, match());
|
||||
stub(APPLY, null);
|
||||
|
||||
const el = await albumPage();
|
||||
|
||||
await pressBanner(el, 'Apply');
|
||||
await pressConfirm('confirm-accept');
|
||||
await el.updateComplete;
|
||||
|
||||
expect(calls(APPLY)).toHaveLength(1);
|
||||
// The release is passed explicitly: a rescore between the page
|
||||
// rendering and the click must not swap the album out from under
|
||||
// a button the user has already read.
|
||||
expect(calls(APPLY)[0]?.args).toEqual(['grp-1', 'rel-1']);
|
||||
expect(shadow(el, '.autotag-match')).toBeNull();
|
||||
});
|
||||
|
||||
/**
|
||||
* A failure is Persistent, not Transient: the user asked for
|
||||
* something that did not happen and retrying is meaningful, which is
|
||||
* the notification store's own rule for choosing the level.
|
||||
*/
|
||||
it('keeps a failure on screen', async () => {
|
||||
stub(MATCH, match());
|
||||
stubFailure(APPLY, 'the tag writer refused');
|
||||
|
||||
const el = await albumPage();
|
||||
|
||||
await pressBanner(el, 'Apply');
|
||||
await pressConfirm('confirm-accept');
|
||||
await el.updateComplete;
|
||||
|
||||
// Read it the way a person would: the app's one notification
|
||||
// surface, rendered.
|
||||
const host = await fixture<LitElement>('notification-host');
|
||||
|
||||
await host.updateComplete;
|
||||
|
||||
const shown = shadowAll(host, '[data-testid="notification"]').map(
|
||||
(n) => n.textContent ?? '',
|
||||
);
|
||||
|
||||
expect(shown.join(' ')).toContain('could not be');
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,114 @@
|
||||
/**
|
||||
* "Review in Autotag" has to land on *that* album.
|
||||
*
|
||||
* The album page can now say the autotagger has a match for what you
|
||||
* are looking at (#28), and the review link is only worth having if it
|
||||
* opens the same album. The queue is sorted by score, so the intended
|
||||
* folder is often near the top — but "often" is a link that sometimes
|
||||
* opens a different album, which is worse than no link.
|
||||
*
|
||||
* Autotag is a **cached primary view**: `index.ts` creates it once and
|
||||
* reuses it, so there is no construction to pass a value to. The
|
||||
* request arrives as an attribute, which is why the interesting part
|
||||
* is that it is *consumed* — an attribute left on a cached element
|
||||
* would reopen the same folder on every later visit to the page.
|
||||
*/
|
||||
import { describe, expect, it, beforeEach } from 'vitest';
|
||||
import type { LitElement } from 'lit';
|
||||
|
||||
import '@components/autotag-view/autotag-view';
|
||||
import { flush, resetHarness, stub } from '@test/support/harness';
|
||||
import { fixture } from '@test/support/render';
|
||||
|
||||
const FOLDERS = 'autotagservice.Service.ListPendingFolders';
|
||||
const CANDIDATES = 'autotagservice.Service.GetCandidates';
|
||||
|
||||
function folder(groupKey: string, album: string) {
|
||||
return {
|
||||
groupKey,
|
||||
libraryId: 0,
|
||||
libraryName: 'Test',
|
||||
folderSubPath: album,
|
||||
trackCount: 10,
|
||||
albumName: album,
|
||||
albumArtist: 'Tideline',
|
||||
discNumber: 0,
|
||||
status: 'pending',
|
||||
score: groupKey === 'grp-top' ? 0.99 : 0.5,
|
||||
bestMatchReleaseMbid: 'rel-1',
|
||||
synthetic: false,
|
||||
likelyMixedBag: false,
|
||||
};
|
||||
}
|
||||
|
||||
/** Mount the view and run its activation, as navigation would. */
|
||||
async function autotag(groupKey?: string): Promise<LitElement> {
|
||||
const el = await fixture<LitElement>('autotag-view');
|
||||
|
||||
if (groupKey !== undefined) el.setAttribute('group-key', groupKey);
|
||||
|
||||
(el as unknown as { onViewActivate: () => void }).onViewActivate();
|
||||
|
||||
await flush();
|
||||
await el.updateComplete;
|
||||
await flush();
|
||||
await el.updateComplete;
|
||||
|
||||
return el;
|
||||
}
|
||||
|
||||
/** The folder the view has selected. */
|
||||
function selected(el: LitElement): string | undefined {
|
||||
return (el as unknown as { current?: { groupKey: string } }).current
|
||||
?.groupKey;
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
resetHarness();
|
||||
stub('autotagservice.Service.StartAutotagQueue', null);
|
||||
stub('autotagservice.Service.GetLocalCoverArt', '');
|
||||
stub('autotagservice.Service.AckLibraryWarning', null);
|
||||
stub('library.Library.GetAllLibrariesWithTrackCounts', []);
|
||||
stub(FOLDERS, [folder('grp-top', 'Loudest Match'), folder('grp-asked', 'Glass Harbour')]);
|
||||
stub(CANDIDATES, {
|
||||
groupKey: 'grp-asked',
|
||||
recommendation: 'strong',
|
||||
localTracks: [],
|
||||
candidates: [],
|
||||
synthetic: false,
|
||||
mixedBag: false,
|
||||
});
|
||||
});
|
||||
|
||||
describe('arriving at Autotag from an album page', () => {
|
||||
it('opens the folder that was asked for, not the top of the queue', async () => {
|
||||
const el = await autotag('grp-asked');
|
||||
|
||||
expect(selected(el)).toBe('grp-asked');
|
||||
});
|
||||
|
||||
it('still lands on the best pending folder when nothing was asked', async () => {
|
||||
const el = await autotag();
|
||||
|
||||
expect(selected(el)).toBe('grp-top');
|
||||
});
|
||||
|
||||
/**
|
||||
* The view is cached and never unmounts, so an attribute left behind
|
||||
* is a standing instruction: every later visit to Autotag would
|
||||
* reopen an album the user finished with three navigations ago.
|
||||
*/
|
||||
it('consumes the request rather than remembering it', async () => {
|
||||
const el = await autotag('grp-asked');
|
||||
|
||||
expect(el.hasAttribute('group-key')).toBe(false);
|
||||
|
||||
// A second visit, with no new request: whatever the user had
|
||||
// selected stays selected.
|
||||
(el as unknown as { onViewActivate: () => void }).onViewActivate();
|
||||
await flush();
|
||||
await el.updateComplete;
|
||||
|
||||
expect(selected(el)).toBe('grp-asked');
|
||||
});
|
||||
});
|
||||
@@ -43,6 +43,7 @@ const GOVERNED = [
|
||||
'solid/bookmark',
|
||||
'regular/bookmark',
|
||||
'bars-staggered',
|
||||
'tag',
|
||||
];
|
||||
|
||||
/** The one file allowed to say them, plus its own test. */
|
||||
|
||||
Reference in New Issue
Block a user