Merge pull request 'Make it obvious everywhere when you are looking at things you do not own' (#117) from feat/38-ownership-visibility into main
Owned is plain; unowned is dimmed, named and requestable; a partly-held album says how partly. Ownership is a file (`localId`), never the `in_library` ratchet. Closes #38
This commit was merged in pull request #117.
This commit is contained in:
@@ -3534,3 +3534,49 @@ silent.** `artifactHasTotals` and `artifactHasCredits` are both correct
|
||||
and both mean a feature can ship, pass every test, and produce nothing
|
||||
for anybody without a single failure anywhere. Checking the *published
|
||||
file* is one query and is not implied by any tick in CI.
|
||||
|
||||
## "Do I own this" has two answers in the schema, and one of them is a flag (2026-08-19)
|
||||
|
||||
Decided while doing #38, and it outlives it because every future
|
||||
catalog surface has to pick one.
|
||||
|
||||
`explore_index` carries both `in_library` and `local_artist_id` /
|
||||
`local_release_group_id` / `local_recording_id`. They are written by
|
||||
the same pass (`collectLibraryEntities`), so on a healthy database they
|
||||
agree, and the code read them as an OR — `inLibrary || localId > 0` —
|
||||
at eight call sites.
|
||||
|
||||
They are not the same kind of thing:
|
||||
|
||||
- **`local_*_id` is a fact with an owner.** Every query that sets one
|
||||
joins `audio_files`, and `pruneStaleLocalCrossReferences` clears it
|
||||
with an existence test that is a file test in all three cases. It is
|
||||
the same rule `explore-album-details`'s `filePaths` implements, one
|
||||
layer down and computed once per scan.
|
||||
- **`in_library` is a ratchet.** `upsertBatch` raises it with
|
||||
`MAX(in_library, excluded.in_library)` and the prune is the only
|
||||
thing that lowers it — gated on the local id being non-null, so a row
|
||||
holding the flag *without* an id is a fixed point nothing can clear.
|
||||
Filed as #118; it still drives search scoring, the popularity-floor
|
||||
bypass and two Explore shelves, so routing the UI around it was not a
|
||||
fix.
|
||||
|
||||
What made the choice concrete rather than theoretical: on
|
||||
`explore-artist-details` the *same card* used both. The context menu
|
||||
gated Play on `localId > 0`; the badge used `inLibrary`. An album with
|
||||
the flag and no local row drew a green tick saying it was in your
|
||||
library, offered no Play, and — the request item being gated on *not*
|
||||
owned — offered no way to ask for it either.
|
||||
|
||||
The rejected alternative is worth keeping: batching a real file lookup
|
||||
per screenful, the way `credit-store` coalesces. It would have answered
|
||||
for **recordings** (`GetFilePathsByRecordingMBIDs`) and most of the
|
||||
cards on these surfaces are release groups, so it would have made track
|
||||
rows strong, left album cards exactly where they were, and cost a new
|
||||
store. The batch that *was* worth adding is a different question —
|
||||
`GetAlbumsCompleteness`, "how much of this album is here", which no
|
||||
per-card flag can answer at all.
|
||||
|
||||
The general point: **two columns that agree today are not one column.**
|
||||
Which of them a new surface reads should be decided by which one has
|
||||
something that can un-set it.
|
||||
|
||||
@@ -1658,6 +1658,59 @@ not about plumbing — it says rows may be missing from the page
|
||||
altogether, which nothing on screen can show. (`explore-artist-details`
|
||||
still uses `loading`; it has no equivalent per-row signal.)
|
||||
|
||||
**And that treatment is the app's, not the page's.**
|
||||
`utils/ownership.ts` is the rule written once, because it was written
|
||||
at eight call sites and so none of them had the whole of it: Explore's
|
||||
cards, `top-results-row` and the artist page's three card shapes all
|
||||
mixed owned and unowned with a small badge as the only difference, and
|
||||
the badge on the *owned* ones was a green tick — the mark on the common
|
||||
case this tracklist removed. Owned is plain and draws no badge at all;
|
||||
unowned is dimmed, says so in its accessible name, and keeps its
|
||||
request affordance; a partly-held album says how partly.
|
||||
|
||||
Four things about it are load-bearing.
|
||||
|
||||
**Ownership is `localId`, and `inLibrary` is deliberately not
|
||||
consulted.** The album page answers with `filePaths`, a real file per
|
||||
displayed track, and a card grid cannot afford that — but it does not
|
||||
need to, because `explore_index.local_*_id` is built by
|
||||
`collectLibraryEntities` from queries that every one join `audio_files`
|
||||
and cleared by `pruneStaleLocalCrossReferences`, whose existence test
|
||||
is a file test in all three cases. That is the same "ownership is a
|
||||
file" rule computed once per scan instead of once per screenful.
|
||||
`in_library` is written by the same pass, so the two agree in a healthy
|
||||
database, but it is a one-way ratchet
|
||||
(`MAX(in_library, excluded.in_library)`) whose only clearing pass is
|
||||
gated on a non-null local id: it cannot be un-set on its own (#118).
|
||||
One is a fact with an owner; the other is a flag that happens to agree.
|
||||
Both `explore-view` and `explore-artist-details` additionally kept a
|
||||
`libraryMBIDs` set that accumulated every MBID ever seen with the flag
|
||||
and cleared it never, in views that never unmount; both are gone.
|
||||
|
||||
**The two answers used to sit on one card.**
|
||||
`renderReleaseMenuItems` gates Play on `release.localId > 0` while the
|
||||
badge used `inLibrary`, so an album with the flag and no local row drew
|
||||
a tick saying it was in your library, offered no Play, and — the
|
||||
request item being gated on *not* owned — offered no way to ask for it
|
||||
either. Any new surface that asks the question twice will reproduce it.
|
||||
|
||||
**`aria-disabled` goes on rows and not on cards.** An unowned *row*
|
||||
cannot be activated; an unowned *card* still navigates to the catalog
|
||||
page for it, which is a perfectly good thing to do with something you
|
||||
do not own. The accessible name carries the state either way, which is
|
||||
why it is one helper and not a class.
|
||||
|
||||
**The count is batched, not looked up.** `store/completeness-store.ts`
|
||||
is `credit-store` one question over: `request()` is per-card and
|
||||
coalesces a screenful into one `GetAlbumsCompleteness`, absence is
|
||||
cached as an answer (or the albums with no totals re-ask forever), and
|
||||
the whole cache is dropped on a scan, a retag or a removal rather than
|
||||
aged. `library-status.ts`'s `albumBadgeFor` is where that meets
|
||||
`Known`: a total that was never declared is a plain `in-library`, never
|
||||
a ring at 0%. One consequence in the badge itself — a `partial` badge
|
||||
is *actionable*, and a control named after its action alone dropped the
|
||||
count from the one state the ring exists for, so its name is both.
|
||||
|
||||
**A partly-owned album draws the release, not the part.** Once the tags
|
||||
say nine of twelve, `buildLibraryEntry` shows the *catalog's* twelve
|
||||
with three dimmed, rather than the nine on disk — the missing tracks
|
||||
|
||||
@@ -135,3 +135,49 @@ SELECT
|
||||
) AS INTEGER) AS known
|
||||
FROM audio_files a
|
||||
WHERE a.album_id = sqlc.arg(album_id);
|
||||
|
||||
-- name: GetAlbumsCompleteness :many
|
||||
-- The same question as GetAlbumCompleteness, asked of a screenful of
|
||||
-- albums at once.
|
||||
--
|
||||
-- A card grid cannot afford one query per card, and the answer it wants
|
||||
-- is the one thing a badge cannot guess: an album held 9 tracks of 12
|
||||
-- must show the count, never a bare tick. So this is one query for the
|
||||
-- whole grid, asked only of the cards that have a local album id.
|
||||
--
|
||||
-- It is two grouping levels rather than the single-album form's
|
||||
-- correlated subqueries, because a correlated subquery in the FROM
|
||||
-- clause is not something SQLite will reliably do -- and because the
|
||||
-- slice may only be spelled once, or sqlc expands it twice with
|
||||
-- independently numbered placeholders.
|
||||
--
|
||||
-- The per-disc level is where the meaning is, and it is the same
|
||||
-- meaning as the single-album query. `owned` counts DISTINCT track
|
||||
-- numbers within a disc (this app detects duplicates, and counting two
|
||||
-- files of track 3 twice would report a short album as complete), with
|
||||
-- a file that declares no track number falling back to its own id
|
||||
-- because three untagged files are three tracks and not one.
|
||||
-- `expected` takes each disc's declared total and sums over discs,
|
||||
-- since a total is declared per disc and a release total written on
|
||||
-- every file of a two-disc album would double its expectation. A disc
|
||||
-- whose files declared nothing contributes a NULL that SUM ignores,
|
||||
-- and `known` is what says the album is therefore unanswerable.
|
||||
WITH per_disc AS (
|
||||
SELECT
|
||||
album_id AS album_id,
|
||||
COUNT(DISTINCT COALESCE(CAST(track_number AS TEXT), 'f' || id))
|
||||
AS owned_on_disc,
|
||||
MAX(total_tracks) AS disc_total,
|
||||
SUM(CASE WHEN total_tracks IS NULL THEN 1 ELSE 0 END)
|
||||
AS discs_without_a_total
|
||||
FROM audio_files
|
||||
WHERE album_id IN (sqlc.slice('album_ids'))
|
||||
GROUP BY album_id, COALESCE(disc_number, 1)
|
||||
)
|
||||
SELECT
|
||||
CAST(album_id AS INTEGER) AS album_id,
|
||||
CAST(SUM(owned_on_disc) AS INTEGER) AS owned,
|
||||
CAST(COALESCE(SUM(disc_total), 0) AS INTEGER) AS expected,
|
||||
CAST(SUM(discs_without_a_total) = 0 AS INTEGER) AS known
|
||||
FROM per_disc
|
||||
GROUP BY album_id;
|
||||
|
||||
@@ -8,6 +8,7 @@ package sqlcgen
|
||||
import (
|
||||
"context"
|
||||
"database/sql"
|
||||
"strings"
|
||||
)
|
||||
|
||||
const deleteAlbum = `-- name: DeleteAlbum :exec
|
||||
@@ -234,6 +235,98 @@ func (q *Queries) GetAlbumsByArtistName(ctx context.Context, arg GetAlbumsByArti
|
||||
return items, nil
|
||||
}
|
||||
|
||||
const getAlbumsCompleteness = `-- name: GetAlbumsCompleteness :many
|
||||
WITH per_disc AS (
|
||||
SELECT
|
||||
album_id AS album_id,
|
||||
COUNT(DISTINCT COALESCE(CAST(track_number AS TEXT), 'f' || id))
|
||||
AS owned_on_disc,
|
||||
MAX(total_tracks) AS disc_total,
|
||||
SUM(CASE WHEN total_tracks IS NULL THEN 1 ELSE 0 END)
|
||||
AS discs_without_a_total
|
||||
FROM audio_files
|
||||
WHERE album_id IN (/*SLICE:album_ids*/?)
|
||||
GROUP BY album_id, COALESCE(disc_number, 1)
|
||||
)
|
||||
SELECT
|
||||
CAST(album_id AS INTEGER) AS album_id,
|
||||
CAST(SUM(owned_on_disc) AS INTEGER) AS owned,
|
||||
CAST(COALESCE(SUM(disc_total), 0) AS INTEGER) AS expected,
|
||||
CAST(SUM(discs_without_a_total) = 0 AS INTEGER) AS known
|
||||
FROM per_disc
|
||||
GROUP BY album_id
|
||||
`
|
||||
|
||||
type GetAlbumsCompletenessRow struct {
|
||||
AlbumID int64
|
||||
Owned int64
|
||||
Expected int64
|
||||
Known int64
|
||||
}
|
||||
|
||||
// The same question as GetAlbumCompleteness, asked of a screenful of
|
||||
// albums at once.
|
||||
//
|
||||
// A card grid cannot afford one query per card, and the answer it wants
|
||||
// is the one thing a badge cannot guess: an album held 9 tracks of 12
|
||||
// must show the count, never a bare tick. So this is one query for the
|
||||
// whole grid, asked only of the cards that have a local album id.
|
||||
//
|
||||
// It is two grouping levels rather than the single-album form's
|
||||
// correlated subqueries, because a correlated subquery in the FROM
|
||||
// clause is not something SQLite will reliably do -- and because the
|
||||
// slice may only be spelled once, or sqlc expands it twice with
|
||||
// independently numbered placeholders.
|
||||
//
|
||||
// The per-disc level is where the meaning is, and it is the same
|
||||
// meaning as the single-album query. `owned` counts DISTINCT track
|
||||
// numbers within a disc (this app detects duplicates, and counting two
|
||||
// files of track 3 twice would report a short album as complete), with
|
||||
// a file that declares no track number falling back to its own id
|
||||
// because three untagged files are three tracks and not one.
|
||||
// `expected` takes each disc's declared total and sums over discs,
|
||||
// since a total is declared per disc and a release total written on
|
||||
// every file of a two-disc album would double its expectation. A disc
|
||||
// whose files declared nothing contributes a NULL that SUM ignores,
|
||||
// and `known` is what says the album is therefore unanswerable.
|
||||
func (q *Queries) GetAlbumsCompleteness(ctx context.Context, albumIds []sql.NullInt64) ([]GetAlbumsCompletenessRow, error) {
|
||||
query := getAlbumsCompleteness
|
||||
var queryParams []interface{}
|
||||
if len(albumIds) > 0 {
|
||||
for _, v := range albumIds {
|
||||
queryParams = append(queryParams, v)
|
||||
}
|
||||
query = strings.Replace(query, "/*SLICE:album_ids*/?", strings.Repeat(",?", len(albumIds))[1:], 1)
|
||||
} else {
|
||||
query = strings.Replace(query, "/*SLICE:album_ids*/?", "NULL", 1)
|
||||
}
|
||||
rows, err := q.db.QueryContext(ctx, query, queryParams...)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
defer rows.Close()
|
||||
var items []GetAlbumsCompletenessRow
|
||||
for rows.Next() {
|
||||
var i GetAlbumsCompletenessRow
|
||||
if err := rows.Scan(
|
||||
&i.AlbumID,
|
||||
&i.Owned,
|
||||
&i.Expected,
|
||||
&i.Known,
|
||||
); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
items = append(items, i)
|
||||
}
|
||||
if err := rows.Close(); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
if err := rows.Err(); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return items, nil
|
||||
}
|
||||
|
||||
const getAlbumsWithPendingReleaseMBID = `-- name: GetAlbumsWithPendingReleaseMBID :many
|
||||
SELECT id, pending_release_mbid FROM albums
|
||||
WHERE pending_release_mbid IS NOT NULL AND pending_release_mbid != ''
|
||||
|
||||
@@ -2212,6 +2212,7 @@ func (e *Service) gatherTopCandidates(
|
||||
ArtistType: a.Type,
|
||||
Country: a.Country,
|
||||
InLibrary: a.InLibrary,
|
||||
LocalID: a.LocalID,
|
||||
},
|
||||
category: "artist",
|
||||
qualityScore: quality,
|
||||
@@ -2243,6 +2244,7 @@ func (e *Service) gatherTopCandidates(
|
||||
ArtistType: a.Type,
|
||||
Country: a.Country,
|
||||
InLibrary: a.InLibrary,
|
||||
LocalID: a.LocalID,
|
||||
},
|
||||
category: "artist",
|
||||
qualityScore: quality,
|
||||
@@ -2275,6 +2277,7 @@ func (e *Service) gatherTopCandidates(
|
||||
PrimaryType: rg.PrimaryType,
|
||||
Year: year,
|
||||
InLibrary: rg.InLibrary,
|
||||
LocalID: rg.LocalID,
|
||||
},
|
||||
category: "release_group",
|
||||
qualityScore: quality,
|
||||
@@ -2320,6 +2323,7 @@ func (e *Service) gatherTopCandidates(
|
||||
PrimaryType: rg.PrimaryType,
|
||||
Year: year,
|
||||
InLibrary: rg.InLibrary,
|
||||
LocalID: rg.LocalID,
|
||||
},
|
||||
category: "release_group",
|
||||
qualityScore: quality,
|
||||
@@ -2347,6 +2351,7 @@ func (e *Service) gatherTopCandidates(
|
||||
CAAReleaseMBID: r.CAAReleaseMBID,
|
||||
ReleaseName: r.ReleaseName,
|
||||
InLibrary: r.InLibrary,
|
||||
LocalID: r.LocalID,
|
||||
},
|
||||
category: "recording",
|
||||
qualityScore: quality,
|
||||
@@ -2403,6 +2408,7 @@ func (e *Service) gatherTopCandidates(
|
||||
CAAReleaseMBID: r.CAAReleaseMBID,
|
||||
ReleaseName: r.ReleaseName,
|
||||
InLibrary: r.InLibrary,
|
||||
LocalID: r.LocalID,
|
||||
},
|
||||
category: "recording",
|
||||
qualityScore: quality,
|
||||
@@ -2425,6 +2431,7 @@ func (e *Service) gatherTopCandidates(
|
||||
ArtistType: m.ArtistType,
|
||||
Country: m.Country,
|
||||
InLibrary: m.InLibrary || m.LocalArtistID > 0,
|
||||
LocalID: m.LocalArtistID,
|
||||
},
|
||||
category: "artist",
|
||||
qualityScore: quality,
|
||||
@@ -2445,6 +2452,7 @@ func (e *Service) gatherTopCandidates(
|
||||
PrimaryType: m.PrimaryType,
|
||||
Year: year,
|
||||
InLibrary: m.InLibrary || m.LocalReleaseGroupID > 0,
|
||||
LocalID: m.LocalReleaseGroupID,
|
||||
},
|
||||
category: "release_group",
|
||||
qualityScore: quality,
|
||||
@@ -2459,6 +2467,7 @@ func (e *Service) gatherTopCandidates(
|
||||
ArtistMBID: m.ArtistMBID,
|
||||
Length: m.Duration,
|
||||
InLibrary: m.InLibrary || m.LocalRecordingID > 0,
|
||||
LocalID: m.LocalRecordingID,
|
||||
},
|
||||
category: "recording",
|
||||
qualityScore: quality,
|
||||
|
||||
@@ -41,7 +41,16 @@ type TopResult struct {
|
||||
ReleaseGroupMBID string `json:"releaseGroupMbid,omitempty"`
|
||||
ReleaseName string `json:"releaseName,omitempty"`
|
||||
// Library status — populated from index cross-reference columns.
|
||||
InLibrary bool `json:"inLibrary"`
|
||||
//
|
||||
// LocalID is the one the cards read. It is the local row behind
|
||||
// this entity — an album, a file, an artist — and it is set and
|
||||
// cleared by a test against `audio_files`, so it means "there is
|
||||
// something of mine here". InLibrary is written by the same pass
|
||||
// but is a one-way ratchet the prune can only clear alongside a
|
||||
// local id, so it is the weaker of the two and stays for scoring
|
||||
// (`fwInLibrary`), which is where an approximate answer is fine.
|
||||
InLibrary bool `json:"inLibrary"`
|
||||
LocalID int64 `json:"localId,omitempty"`
|
||||
}
|
||||
|
||||
// MBArtist is a Wails-friendly projection of a MusicBrainz artist.
|
||||
|
||||
@@ -232,3 +232,112 @@ func TestGetAlbumCompleteness_EmptyAlbum(t *testing.T) {
|
||||
t.Errorf("empty album reported %+v, want zero and unknown", got)
|
||||
}
|
||||
}
|
||||
|
||||
// The batch and the single-album query are two spellings of one
|
||||
// question, and the thing worth pinning is that they never disagree.
|
||||
//
|
||||
// They are genuinely different SQL — the single-album form is
|
||||
// correlated subqueries over one album, the batch is two grouping
|
||||
// levels over a slice — so the risk is not a typo but a drift in
|
||||
// meaning: a disc's total counted once per file, a duplicate counted
|
||||
// twice, a disc with no total silently covered by one that had one.
|
||||
// Every shape the table above cares about is staged here at once,
|
||||
// because a batch that is only ever asked about one album is not being
|
||||
// asked the question that can go wrong.
|
||||
func TestGetAlbumsCompletenessAgreesWithTheSingleAlbumQuery(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
lib, _ := setupTestLibrary(t)
|
||||
|
||||
shapes := map[int][]track{
|
||||
1: disc(1, 100, 12, 12),
|
||||
2: disc(1, 200, 9, 12),
|
||||
3: disc(1, 300, 13, 12),
|
||||
4: {{recordingID: 400, disc: 1, number: 1}},
|
||||
5: append(disc(1, 500, 10, 10), disc(2, 600, 2, 5)...),
|
||||
6: append(
|
||||
disc(1, 700, 10, 10),
|
||||
track{recordingID: 750, disc: 2, number: 1},
|
||||
),
|
||||
7: append(
|
||||
disc(1, 800, 5, 6),
|
||||
track{recordingID: 899, disc: 1, number: 3, total: 6},
|
||||
),
|
||||
}
|
||||
|
||||
ids := make([]int64, 0, len(shapes))
|
||||
|
||||
for albumID, tracks := range shapes {
|
||||
stageAlbum(t, lib, albumID, tracks)
|
||||
ids = append(ids, albumIDFor(t, lib, albumID))
|
||||
}
|
||||
|
||||
batch, err := lib.GetAlbumsCompleteness(ids)
|
||||
if err != nil {
|
||||
t.Fatalf("GetAlbumsCompleteness: %v", err)
|
||||
}
|
||||
|
||||
if len(batch) != len(ids) {
|
||||
t.Fatalf("batch answered for %d albums, want %d", len(batch), len(ids))
|
||||
}
|
||||
|
||||
for _, id := range ids {
|
||||
one, err := lib.GetAlbumCompleteness(id)
|
||||
if err != nil {
|
||||
t.Fatalf("GetAlbumCompleteness(%d): %v", id, err)
|
||||
}
|
||||
|
||||
if got := batch[id]; got != one {
|
||||
t.Errorf("album %d: batch says %+v, single says %+v", id, got, one)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// An album with no files is absent from the batch, not zeroed.
|
||||
//
|
||||
// "I have none of this" and "I have no idea" are the third state Known
|
||||
// exists to keep apart, and a caller reading a missing key gets nothing
|
||||
// rather than a confident zero it would have to know to distrust.
|
||||
func TestGetAlbumsCompletenessOmitsAnAlbumWithNoFiles(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
lib, _ := setupTestLibrary(t)
|
||||
|
||||
stageAlbum(t, lib, 1, disc(1, 100, 3, 3))
|
||||
|
||||
held := albumIDFor(t, lib, 1)
|
||||
|
||||
got, err := lib.GetAlbumsCompleteness([]int64{held, 4242})
|
||||
if err != nil {
|
||||
t.Fatalf("GetAlbumsCompleteness: %v", err)
|
||||
}
|
||||
|
||||
if _, ok := got[4242]; ok {
|
||||
t.Errorf("an album with no files answered %+v, want absent", got[4242])
|
||||
}
|
||||
|
||||
if !got[held].Complete {
|
||||
t.Errorf("held album reported %+v, want complete", got[held])
|
||||
}
|
||||
}
|
||||
|
||||
// A caller with nothing to ask about must not issue a query at all —
|
||||
// sqlc's empty-slice branch rewrites the placeholder to NULL, which is
|
||||
// a perfectly valid query returning nothing, so this is about the round
|
||||
// trip rather than the answer.
|
||||
func TestGetAlbumsCompletenessAsksNothingForAnEmptyList(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
lib, _ := setupTestLibrary(t)
|
||||
|
||||
for _, ids := range [][]int64{nil, {}, {0}, {-1, 0}} {
|
||||
got, err := lib.GetAlbumsCompleteness(ids)
|
||||
if err != nil {
|
||||
t.Fatalf("GetAlbumsCompleteness(%v): %v", ids, err)
|
||||
}
|
||||
|
||||
if len(got) != 0 {
|
||||
t.Errorf("GetAlbumsCompleteness(%v) = %+v, want empty", ids, got)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -244,6 +244,65 @@ func (l *Library) GetAlbumCompleteness(albumID int64) (AlbumCompleteness, error)
|
||||
}, nil
|
||||
}
|
||||
|
||||
// GetAlbumsCompleteness answers the same question for a screenful of
|
||||
// albums in one query, keyed by album id.
|
||||
//
|
||||
// A card grid asks this about every card that has a local album behind
|
||||
// it, and one query per card is how a grid of fifty albums becomes
|
||||
// fifty round trips. The answer matters there for the reason it
|
||||
// matters on the album page: an album held 9 tracks of 12 has to show
|
||||
// the count, and a bare tick saying "in your library" is the complaint
|
||||
// this whole rule came from.
|
||||
//
|
||||
// An album with no row in the result is one with no files, and it is
|
||||
// absent rather than zeroed — "I have none of this" and "I have no
|
||||
// idea" are the same third state `Known` exists to keep apart, and a
|
||||
// caller reading a missing key gets nothing rather than a confident 0.
|
||||
func (l *Library) GetAlbumsCompleteness(
|
||||
albumIDs []int64,
|
||||
) (map[int64]AlbumCompleteness, error) {
|
||||
out := make(map[int64]AlbumCompleteness, len(albumIDs))
|
||||
|
||||
if len(albumIDs) == 0 {
|
||||
return out, nil
|
||||
}
|
||||
|
||||
keys := make([]sql.NullInt64, 0, len(albumIDs))
|
||||
|
||||
for _, id := range albumIDs {
|
||||
if id <= 0 {
|
||||
continue
|
||||
}
|
||||
|
||||
keys = append(keys, sql.NullInt64{Int64: id, Valid: true})
|
||||
}
|
||||
|
||||
if len(keys) == 0 {
|
||||
return out, nil
|
||||
}
|
||||
|
||||
rows, err := l.db.ReadQueries.GetAlbumsCompleteness(l.ctx, keys)
|
||||
if err != nil {
|
||||
l.logger.Error("could not get album completeness in batch",
|
||||
"albums", len(keys), "error", err)
|
||||
|
||||
return nil, fmt.Errorf("could not get album completeness: %w", err)
|
||||
}
|
||||
|
||||
for _, row := range rows {
|
||||
known := row.Known != 0 && row.Expected > 0
|
||||
|
||||
out[row.AlbumID] = AlbumCompleteness{
|
||||
Owned: int(row.Owned),
|
||||
Expected: int(row.Expected),
|
||||
Known: known,
|
||||
Complete: known && row.Owned >= row.Expected,
|
||||
}
|
||||
}
|
||||
|
||||
return out, nil
|
||||
}
|
||||
|
||||
// GetAlbumTracks returns one album's tracks in disc/track order.
|
||||
func (l *Library) GetAlbumTracks(albumID, libraryID int64) ([]Track, error) {
|
||||
rows, err := l.db.ReadQueries.GetTracksByAlbum(
|
||||
|
||||
@@ -431,8 +431,17 @@ export interface TopResult {
|
||||
|
||||
/**
|
||||
* Library status — populated from index cross-reference columns.
|
||||
*
|
||||
* LocalID is the one the cards read. It is the local row behind
|
||||
* this entity — an album, a file, an artist — and it is set and
|
||||
* cleared by a test against `audio_files`, so it means "there is
|
||||
* something of mine here". InLibrary is written by the same pass
|
||||
* but is a one-way ratchet the prune can only clear alongside a
|
||||
* local id, so it is the weaker of the two and stays for scoring
|
||||
* (`fwInLibrary`), which is where an approximate answer is fine.
|
||||
*/
|
||||
"inLibrary": boolean;
|
||||
"localId"?: number;
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -98,6 +98,26 @@ export function GetAlbumsByArtist(artist: string, libraryID: number): $Cancellab
|
||||
return $Call.ByID(1456840721, artist, libraryID);
|
||||
}
|
||||
|
||||
/**
|
||||
* GetAlbumsCompleteness answers the same question for a screenful of
|
||||
* albums in one query, keyed by album id.
|
||||
*
|
||||
* A card grid asks this about every card that has a local album behind
|
||||
* it, and one query per card is how a grid of fifty albums becomes
|
||||
* fifty round trips. The answer matters there for the reason it
|
||||
* matters on the album page: an album held 9 tracks of 12 has to show
|
||||
* the count, and a bare tick saying "in your library" is the complaint
|
||||
* this whole rule came from.
|
||||
*
|
||||
* An album with no row in the result is one with no files, and it is
|
||||
* absent rather than zeroed — "I have none of this" and "I have no
|
||||
* idea" are the same third state `Known` exists to keep apart, and a
|
||||
* caller reading a missing key gets nothing rather than a confident 0.
|
||||
*/
|
||||
export function GetAlbumsCompleteness(albumIDs: number[] | null): $CancellablePromise<{ [_ in `${number}`]?: $models.AlbumCompleteness } | null> {
|
||||
return $Call.ByID(531636827, albumIDs);
|
||||
}
|
||||
|
||||
/**
|
||||
* GetAllLibrariesWithTrackCounts lists the libraries and their sizes.
|
||||
*/
|
||||
|
||||
@@ -3,6 +3,7 @@ import { customElement, property, state, query } from 'lit/decorators.js';
|
||||
import { classMap } from 'lit/directives/class-map.js';
|
||||
import { designTokens } from '../../styles/tokens.css';
|
||||
import { srOnly } from '../../styles/sr-only.css';
|
||||
import { unownedLabel, unownedStyles } from '@utils/ownership';
|
||||
import {
|
||||
LookupReleaseGroup,
|
||||
BrowseReleases,
|
||||
@@ -316,6 +317,7 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
|
||||
exploreLinkStyles,
|
||||
contextMenuStyles,
|
||||
srOnly,
|
||||
unownedStyles,
|
||||
css`
|
||||
:host {
|
||||
display: flex;
|
||||
@@ -687,20 +689,14 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
|
||||
white-space: nowrap;
|
||||
}
|
||||
|
||||
/* A track the library does not have, on the pattern a
|
||||
* streaming service uses for something it cannot play: the
|
||||
* row stays, dimmed, so the album reads as the album rather
|
||||
* than as the subset that happens to be here.
|
||||
*
|
||||
* The dimming is a colour, so it cannot be the only signal
|
||||
* — the row also carries aria-disabled, which is what
|
||||
* reaches anyone not seeing it. Secondary rather than
|
||||
* tertiary because the row's hover background is
|
||||
* bgOverlay, which tertiary does not clear. */
|
||||
.track-row.unowned .track-title {
|
||||
color: var(--yj-text-secondary, #b3b3b3);
|
||||
font-weight: 400;
|
||||
}
|
||||
/* The dimming itself is unownedStyles, from
|
||||
* utils/ownership.ts, imported above. It was written here
|
||||
* first — this tracklist is where the treatment came from —
|
||||
* and moved out when seven other surfaces had to draw the
|
||||
* same thing, because two of them would otherwise have
|
||||
* ended up drawing it slightly differently. (No backticks or
|
||||
* apostrophes-as-quotes here: this is inside a tagged
|
||||
* template literal.) */
|
||||
|
||||
/* The request control is offered on every row that has
|
||||
* something to request, and is not revealed on hover.
|
||||
@@ -3273,7 +3269,7 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
|
||||
aria-disabled=${owned ? 'false' : 'true'}
|
||||
aria-label=${owned
|
||||
? `Play “${track.title}”`
|
||||
: `${track.title} — not in your library`}
|
||||
: unownedLabel(track.title, 'track')}
|
||||
@dblclick=${() => this.onTrackRowDblClick(track)}
|
||||
@contextmenu=${(e: MouseEvent) => this.onTrackContextMenu(e, track)}
|
||||
@keydown=${(e: KeyboardEvent) => this.onTrackRowKeydown(e, track)}
|
||||
|
||||
@@ -39,7 +39,17 @@ import { EventsOn } from '@runtime/runtime';
|
||||
import { Events } from '../../events';
|
||||
import '@awesome.me/webawesome/dist/components/icon/icon.js';
|
||||
import '../library-status-indicator/library-status-indicator.js';
|
||||
import { libraryStatusFor, toggleRequest } from '@utils/library-status';
|
||||
import {
|
||||
albumBadgeFor,
|
||||
libraryStatusFor,
|
||||
toggleRequest,
|
||||
} from '@utils/library-status';
|
||||
import {
|
||||
isOwned,
|
||||
ownershipLabel,
|
||||
unownedStyles,
|
||||
} from '@utils/ownership';
|
||||
import { completenessStore } from '@store/completeness-store';
|
||||
import '../catalog-scope-notice/catalog-scope-notice.js';
|
||||
import type { CatalogScope } from '../catalog-scope-notice/catalog-scope-notice.js';
|
||||
import { queueStore } from '../../store/queue-store';
|
||||
@@ -178,7 +188,6 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
|
||||
@state() private discoRowSize = 5;
|
||||
private discoObserver?: ResizeObserver;
|
||||
@state() private similarExpanded = false;
|
||||
private libraryMBIDs = new Set<string>();
|
||||
|
||||
/* ── Release prefetch ── */
|
||||
|
||||
@@ -257,6 +266,7 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
|
||||
designTokens,
|
||||
exploreLinkStyles,
|
||||
contextMenuStyles,
|
||||
unownedStyles,
|
||||
css`
|
||||
:host {
|
||||
display: flex;
|
||||
@@ -995,6 +1005,9 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
|
||||
/** Unsubscribe handle for the requests list. */
|
||||
private unsubRequests: (() => void) | null = null;
|
||||
|
||||
/** Unsubscribes the "how much of this album is here" repaint. */
|
||||
private unsubCompleteness: (() => void) | null = null;
|
||||
|
||||
override connectedCallback() {
|
||||
super.connectedCallback();
|
||||
if (this.artistMBID || this.localArtistId) {
|
||||
@@ -1007,6 +1020,12 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
|
||||
this.unsubRequests = downloadStore.subscribe(() => this.requestUpdate());
|
||||
void downloadStore.init().then(() => this.requestUpdate());
|
||||
|
||||
// The count behind a partly-held album lands a frame after the
|
||||
// cards do, since the store batches a screenful into one query.
|
||||
this.unsubCompleteness = completenessStore.subscribe(() =>
|
||||
this.requestUpdate(),
|
||||
);
|
||||
|
||||
// A background discography fetch (top tracks / top releases for an
|
||||
// artist that wasn't indexed yet) finished — re-fetch those two
|
||||
// sections, once per artist, so they fill in without the initial
|
||||
@@ -1045,6 +1064,8 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
|
||||
super.disconnectedCallback();
|
||||
this.unsubRequests?.();
|
||||
this.unsubRequests = null;
|
||||
this.unsubCompleteness?.();
|
||||
this.unsubCompleteness = null;
|
||||
this.unsubDiscogReady?.();
|
||||
this.unsubSimilarReady?.();
|
||||
if (this.discogFallbackTimer) clearTimeout(this.discogFallbackTimer);
|
||||
@@ -1573,10 +1594,6 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
|
||||
this.catalogPending = false;
|
||||
}
|
||||
|
||||
// Populate libraryMBIDs from the inLibrary flag (already
|
||||
// set by the backend via local_release_group_id cross-ref).
|
||||
this.checkLibrary();
|
||||
|
||||
// Batch-resolve cover art for discography (lower priority — loaded after top sections).
|
||||
void this.batchResolveThumbnails(
|
||||
rgs?.map((r) => ({ mbid: r.mbid, albumName: r.title, artistName: r.artistCredit }))
|
||||
@@ -1903,23 +1920,6 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
|
||||
}
|
||||
}
|
||||
|
||||
private checkLibrary() {
|
||||
// Backend now populates `inLibrary` directly on each MBReleaseGroup
|
||||
// via the local_release_group_id cross-reference column. Just read it.
|
||||
let updated = false;
|
||||
|
||||
for (const rg of this.releaseGroups) {
|
||||
if (rg.mbid && rg.inLibrary && !this.libraryMBIDs.has(rg.mbid)) {
|
||||
this.libraryMBIDs.add(rg.mbid);
|
||||
updated = true;
|
||||
}
|
||||
}
|
||||
|
||||
if (updated) {
|
||||
this.requestUpdate();
|
||||
}
|
||||
}
|
||||
|
||||
/* ── Playback ── */
|
||||
|
||||
/**
|
||||
@@ -2015,11 +2015,14 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
|
||||
|
||||
/**
|
||||
* File path for one top track, resolved by recording MBID — the
|
||||
* same key `inLibrary`/`localId` were set from. Works whether or
|
||||
* not the containing release itself matched a local album.
|
||||
* same key `localId` was set from. Works whether or not the
|
||||
* containing release itself matched a local album.
|
||||
*
|
||||
* Gated on the same answer the row is drawn from, or a row drawn
|
||||
* dimmed and `aria-disabled` would still try to play and fail.
|
||||
*/
|
||||
private async trackFilePath(track: LBTopRecording): Promise<string | null> {
|
||||
if (!(track.inLibrary || track.localId) || !track.recordingMbid) return null;
|
||||
if (!isOwned(track) || !track.recordingMbid) return null;
|
||||
|
||||
const libraryID = libraryStore.getSelectedLibraryId() ?? 0;
|
||||
const byMBID = await dictByName(
|
||||
@@ -2063,7 +2066,7 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
|
||||
}
|
||||
|
||||
private isTrackOwned(track: LBTopRecording): boolean {
|
||||
return Boolean(track.inLibrary || track.localId);
|
||||
return isOwned(track);
|
||||
}
|
||||
|
||||
private onTrackRowDblClick(track: LBTopRecording): void {
|
||||
@@ -2106,7 +2109,7 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
|
||||
mbid: rg.releaseGroupMbid || '',
|
||||
localId: rg.localId ?? 0,
|
||||
title: rg.title,
|
||||
owned: Boolean(rg.inLibrary || rg.localId),
|
||||
owned: isOwned(rg),
|
||||
};
|
||||
}
|
||||
|
||||
@@ -2126,10 +2129,12 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
|
||||
mbid: isLocal ? '' : rg.mbid || '',
|
||||
localId: Number.isFinite(localId) ? localId : 0,
|
||||
title: rg.title,
|
||||
owned:
|
||||
this.libraryMBIDs.has(rg.mbid) ||
|
||||
Boolean(rg.inLibrary) ||
|
||||
localId > 0,
|
||||
// The same answer the menu gates Play on, which is the
|
||||
// point: this used to be `inLibrary` too, so a card could
|
||||
// report itself owned, be offered no Play (that item is
|
||||
// gated on the local id) and be offered no request either
|
||||
// (that one is gated on *not* owned).
|
||||
owned: localId > 0,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -2944,15 +2949,20 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
|
||||
<div class="top-section-col top-section-col-tracks">
|
||||
<h3 class="section-header">Top Tracks</h3>
|
||||
<div class="track-list">
|
||||
${tracks.map(
|
||||
(t, i) => html`
|
||||
${tracks.map((t, i) => {
|
||||
const owned = this.isTrackOwned(t);
|
||||
|
||||
return html`
|
||||
<div
|
||||
class=${classMap({ 'track-item': true, owned: this.isTrackOwned(t) })}
|
||||
class=${classMap({
|
||||
'track-item': true,
|
||||
owned,
|
||||
unowned: !owned,
|
||||
})}
|
||||
tabindex="0"
|
||||
role="button"
|
||||
aria-label=${this.isTrackOwned(t)
|
||||
? `Play “${t.trackName}”`
|
||||
: `${t.trackName} — not in your library`}
|
||||
aria-disabled=${owned ? 'false' : 'true'}
|
||||
aria-label=${ownershipLabel(owned, 'Play', t.trackName, 'track')}
|
||||
@dblclick=${() => this.onTrackRowDblClick(t)}
|
||||
@contextmenu=${(e: MouseEvent) => this.onTrackContextMenu(e, t)}
|
||||
@keydown=${(e: KeyboardEvent) => this.onTrackRowKeydown(e, t)}
|
||||
@@ -2977,16 +2987,18 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
|
||||
<span class="track-listens">
|
||||
${formatListenCount(t.totalListenCount)} plays
|
||||
</span>
|
||||
<library-status-indicator
|
||||
status=${libraryStatusFor(Boolean(t.inLibrary || t.localId), t.recordingMbid)}
|
||||
entity-type="track"
|
||||
label=${t.trackName}
|
||||
request-mbid=${t.recordingMbid}
|
||||
request-artist=${t.artistName ?? ''}
|
||||
></library-status-indicator>
|
||||
${owned
|
||||
? nothing
|
||||
: html`<library-status-indicator
|
||||
status=${libraryStatusFor(false, t.recordingMbid)}
|
||||
entity-type="track"
|
||||
label=${t.trackName}
|
||||
request-mbid=${t.recordingMbid}
|
||||
request-artist=${t.artistName ?? ''}
|
||||
></library-status-indicator>`}
|
||||
</div>
|
||||
`,
|
||||
)}
|
||||
`;
|
||||
})}
|
||||
</div>
|
||||
${canExpandTracks
|
||||
? html`
|
||||
@@ -3057,10 +3069,16 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
|
||||
private renderTopReleaseCard(rg: LBTopReleaseGroup) {
|
||||
const artURL = this.thumbnailURLs.get(rg.releaseGroupMbid) || '';
|
||||
const target = this.topReleaseTarget(rg);
|
||||
const owned = target.owned;
|
||||
const badge = albumBadgeFor(
|
||||
{ localId: target.localId },
|
||||
rg.releaseGroupMbid,
|
||||
);
|
||||
|
||||
return html`
|
||||
<div
|
||||
class="top-release-card"
|
||||
class=${classMap({ 'top-release-card': true, unowned: !owned })}
|
||||
aria-label=${ownershipLabel(owned, 'Album', rg.title, 'album')}
|
||||
@click=${() => this.navigateToTopRelease(rg)}
|
||||
role="button"
|
||||
tabindex="0"
|
||||
@@ -3092,14 +3110,18 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
|
||||
<div class="top-release-meta-text">
|
||||
${rg.date ? html`<span>${extractYear(rg.date)}</span>` : nothing}
|
||||
</div>
|
||||
<library-status-indicator
|
||||
status=${libraryStatusFor(Boolean(rg.inLibrary || rg.localId), rg.releaseGroupMbid)}
|
||||
entity-type="album"
|
||||
label=${rg.title}
|
||||
request-mbid=${rg.releaseGroupMbid}
|
||||
request-artist=${this.artist?.name ?? ''}
|
||||
size="18"
|
||||
></library-status-indicator>
|
||||
${badge.status === 'in-library'
|
||||
? nothing
|
||||
: html`<library-status-indicator
|
||||
status=${badge.status}
|
||||
owned=${badge.owned}
|
||||
expected=${badge.expected}
|
||||
entity-type="album"
|
||||
label=${rg.title}
|
||||
request-mbid=${rg.releaseGroupMbid}
|
||||
request-artist=${this.artist?.name ?? ''}
|
||||
size="18"
|
||||
></library-status-indicator>`}
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
@@ -3189,14 +3211,15 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
|
||||
private renderAlbumCard(rg: MBReleaseGroup) {
|
||||
const artURL = this.thumbnailURLs.get(rg.mbid) || '';
|
||||
const year = extractYear(rg.firstReleaseDate);
|
||||
const inLibrary = this.libraryMBIDs.has(rg.mbid) || Boolean(rg.inLibrary);
|
||||
const status = libraryStatusFor(inLibrary, rg.mbid);
|
||||
|
||||
const target = this.albumTarget(rg);
|
||||
const owned = target.owned;
|
||||
const badge = albumBadgeFor({ localId: target.localId }, target.mbid);
|
||||
|
||||
return html`
|
||||
<div
|
||||
class="album-card"
|
||||
class=${classMap({ 'album-card': true, unowned: !owned })}
|
||||
aria-label=${ownershipLabel(owned, 'Album', rg.title, 'album')}
|
||||
@click=${() => this.navigateToAlbum(rg)}
|
||||
role="button"
|
||||
tabindex="0"
|
||||
@@ -3225,13 +3248,17 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
|
||||
<div class="album-meta-text">
|
||||
${year ? html`<span>${year}</span>` : nothing}
|
||||
</div>
|
||||
<library-status-indicator
|
||||
status=${status}
|
||||
entity-type="album"
|
||||
label=${rg.title}
|
||||
request-mbid=${rg.mbid}
|
||||
request-artist=${this.artist?.name ?? ''}
|
||||
></library-status-indicator>
|
||||
${badge.status === 'in-library'
|
||||
? nothing
|
||||
: html`<library-status-indicator
|
||||
status=${badge.status}
|
||||
owned=${badge.owned}
|
||||
expected=${badge.expected}
|
||||
entity-type="album"
|
||||
label=${rg.title}
|
||||
request-mbid=${rg.mbid}
|
||||
request-artist=${this.artist?.name ?? ''}
|
||||
></library-status-indicator>`}
|
||||
</div>
|
||||
</div>
|
||||
`;
|
||||
|
||||
@@ -1,5 +1,11 @@
|
||||
import { avatarBackground } from '@utils/avatar-color';
|
||||
import { libraryStatusFor } from '@utils/library-status';
|
||||
import { albumBadgeFor, libraryStatusFor } from '@utils/library-status';
|
||||
import {
|
||||
isOwned,
|
||||
ownershipLabel,
|
||||
unownedStyles,
|
||||
} from '@utils/ownership';
|
||||
import { completenessStore } from '@store/completeness-store';
|
||||
import { downloadStore } from '@store/download-store';
|
||||
import { LitElement, html, css, nothing } from 'lit';
|
||||
import { customElement, state, query as litQuery } from 'lit/decorators.js';
|
||||
@@ -171,7 +177,6 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) implements Conte
|
||||
private searchDebounceTimer?: ReturnType<typeof setTimeout>;
|
||||
private thumbnailCache = new LRUMap<string, string>(THUMBNAIL_CACHE_LIMIT);
|
||||
private artistImageCache = new LRUMap<string, string>(ARTIST_IMAGE_CACHE_LIMIT);
|
||||
private libraryMBIDs = new Set<string>();
|
||||
|
||||
constructor() {
|
||||
super();
|
||||
@@ -226,6 +231,7 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) implements Conte
|
||||
srOnly,
|
||||
exploreLinkStyles,
|
||||
contextMenuStyles,
|
||||
unownedStyles,
|
||||
css`
|
||||
:host {
|
||||
display: block;
|
||||
@@ -812,6 +818,13 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) implements Conte
|
||||
// Explore should not pay for it.
|
||||
this.whileActive(downloadStore.subscribe(() => this.requestUpdate()));
|
||||
void downloadStore.init().then(() => this.requestUpdate());
|
||||
|
||||
// How much of an owned album is here arrives a frame after the
|
||||
// cards do — the store coalesces a screenful into one query —
|
||||
// so a card that turns out to be 9 of 12 repaints when the
|
||||
// answer lands rather than showing a plain tick until something
|
||||
// else happens to re-render the grid.
|
||||
this.whileActive(completenessStore.subscribe(() => this.requestUpdate()));
|
||||
}
|
||||
|
||||
/** A debounced search that lands after the user has left the page is
|
||||
@@ -1083,7 +1096,6 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) implements Conte
|
||||
this.results?.artists ?? [],
|
||||
this.results?.releaseGroups ?? [],
|
||||
);
|
||||
this.checkLibrary();
|
||||
} catch (err) {
|
||||
if (version !== this.searchVersion) return;
|
||||
|
||||
@@ -1229,8 +1241,11 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) implements Conte
|
||||
void this.playAlbum(rg, false);
|
||||
}
|
||||
|
||||
private onRecordingRowDblClick(r: { mbid: string; inLibrary: boolean; localId?: number }): void {
|
||||
if (!r.inLibrary && !r.localId) return;
|
||||
// The same answer the row is drawn from. It used to accept
|
||||
// `inLibrary` as well, so a row drawn dimmed and `aria-disabled`
|
||||
// would still try to play and fail with a notification.
|
||||
private onRecordingRowDblClick(r: { mbid: string; localId?: number }): void {
|
||||
if (!isOwned(r)) return;
|
||||
|
||||
void this.playRecording(r.mbid);
|
||||
}
|
||||
@@ -1665,42 +1680,6 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) implements Conte
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Check which result MBIDs exist in the local library.
|
||||
*/
|
||||
private checkLibrary() {
|
||||
if (!this.results) return;
|
||||
|
||||
// Backend now populates `inLibrary` directly on each MB result
|
||||
// via the local_*_id cross-reference columns. Just read those.
|
||||
let updated = false;
|
||||
|
||||
for (const a of this.results.artists ?? []) {
|
||||
if (a.mbid && a.inLibrary && !this.libraryMBIDs.has(a.mbid)) {
|
||||
this.libraryMBIDs.add(a.mbid);
|
||||
updated = true;
|
||||
}
|
||||
}
|
||||
|
||||
for (const rg of this.results.releaseGroups ?? []) {
|
||||
if (rg.mbid && rg.inLibrary && !this.libraryMBIDs.has(rg.mbid)) {
|
||||
this.libraryMBIDs.add(rg.mbid);
|
||||
updated = true;
|
||||
}
|
||||
}
|
||||
|
||||
for (const r of this.results.recordings ?? []) {
|
||||
if (r.mbid && r.inLibrary && !this.libraryMBIDs.has(r.mbid)) {
|
||||
this.libraryMBIDs.add(r.mbid);
|
||||
updated = true;
|
||||
}
|
||||
}
|
||||
|
||||
if (updated) {
|
||||
this.requestUpdate();
|
||||
}
|
||||
}
|
||||
|
||||
/* ── Navigation ── */
|
||||
|
||||
private navigateToArtist(artist: MBArtist) {
|
||||
@@ -2107,12 +2086,16 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) implements Conte
|
||||
: nothing}
|
||||
<div class="horizontal-row">
|
||||
${artists.map((a) => {
|
||||
const owned = isOwned(a);
|
||||
const name = a.englishName || a.name;
|
||||
|
||||
return html`
|
||||
<div
|
||||
class="artist-card"
|
||||
class=${classMap({ 'artist-card': true, unowned: !owned })}
|
||||
@click=${() => this.navigateToArtist(a)}
|
||||
role="button"
|
||||
tabindex="0"
|
||||
aria-label=${ownershipLabel(owned, 'Artist', name, 'artist')}
|
||||
@keydown=${(e: KeyboardEvent) => {
|
||||
if (e.key === 'Enter' || e.key === ' ') {
|
||||
e.preventDefault();
|
||||
@@ -2171,11 +2154,17 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) implements Conte
|
||||
const artURL = this.thumbnailCache.get(rg.mbid) || '';
|
||||
const year = extractYear(rg.firstReleaseDate);
|
||||
|
||||
const owned = Boolean(rg.localId);
|
||||
const owned = isOwned(rg);
|
||||
const badge = albumBadgeFor(rg, rg.mbid);
|
||||
|
||||
return html`
|
||||
<div
|
||||
class=${classMap({ 'album-card': true, owned })}
|
||||
class=${classMap({
|
||||
'album-card': true,
|
||||
owned,
|
||||
unowned: !owned,
|
||||
})}
|
||||
aria-label=${ownershipLabel(owned, 'Album', rg.title, 'album')}
|
||||
@click=${() => this.navigateToAlbum(rg)}
|
||||
@dblclick=${() => this.onAlbumCardDblClick(rg)}
|
||||
@contextmenu=${(e: MouseEvent) =>
|
||||
@@ -2228,13 +2217,17 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) implements Conte
|
||||
: nothing}
|
||||
${year ? html`<span>${year}</span>` : nothing}
|
||||
</div>
|
||||
<library-status-indicator
|
||||
status=${libraryStatusFor(this.libraryMBIDs.has(rg.mbid) || Boolean(rg.inLibrary), rg.mbid)}
|
||||
entity-type="album"
|
||||
label=${rg.title}
|
||||
request-mbid=${rg.mbid}
|
||||
request-artist=${rg.artistCredit ?? ''}
|
||||
></library-status-indicator>
|
||||
${badge.status === 'in-library'
|
||||
? nothing
|
||||
: html`<library-status-indicator
|
||||
status=${badge.status}
|
||||
owned=${badge.owned}
|
||||
expected=${badge.expected}
|
||||
entity-type="album"
|
||||
label=${rg.title}
|
||||
request-mbid=${rg.mbid}
|
||||
request-artist=${rg.artistCredit ?? ''}
|
||||
></library-status-indicator>`}
|
||||
</div>
|
||||
</div>
|
||||
`;
|
||||
@@ -2249,12 +2242,20 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) implements Conte
|
||||
<section>
|
||||
<h3 class="section-header">Tracks</h3>
|
||||
<div class="track-list">
|
||||
${recordings.map(
|
||||
(r) => html`
|
||||
${recordings.map((r) => {
|
||||
const owned = isOwned(r);
|
||||
|
||||
return html`
|
||||
<div
|
||||
class=${classMap({ 'track-item': true, owned: Boolean(r.inLibrary || r.localId) })}
|
||||
class=${classMap({
|
||||
'track-item': true,
|
||||
owned,
|
||||
unowned: !owned,
|
||||
})}
|
||||
role="button"
|
||||
tabindex="0"
|
||||
aria-disabled=${owned ? 'false' : 'true'}
|
||||
aria-label=${ownershipLabel(owned, 'Play', r.title, 'track')}
|
||||
@dblclick=${() => this.onRecordingRowDblClick(r)}
|
||||
@contextmenu=${(e: MouseEvent) =>
|
||||
this.onExploreContextMenu(e, {
|
||||
@@ -2291,16 +2292,18 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) implements Conte
|
||||
? html`<span class="track-duration">${formatDuration(r.length)}</span>`
|
||||
: nothing}
|
||||
</div>
|
||||
<library-status-indicator
|
||||
status=${libraryStatusFor(this.libraryMBIDs.has(r.mbid) || Boolean(r.inLibrary), r.mbid)}
|
||||
entity-type="track"
|
||||
label=${r.title}
|
||||
request-mbid=${r.mbid}
|
||||
request-artist=${r.artistCredit ?? ''}
|
||||
></library-status-indicator>
|
||||
${owned
|
||||
? nothing
|
||||
: html`<library-status-indicator
|
||||
status=${libraryStatusFor(false, r.mbid)}
|
||||
entity-type="track"
|
||||
label=${r.title}
|
||||
request-mbid=${r.mbid}
|
||||
request-artist=${r.artistCredit ?? ''}
|
||||
></library-status-indicator>`}
|
||||
</div>
|
||||
`,
|
||||
)}
|
||||
`;
|
||||
})}
|
||||
</div>
|
||||
</section>
|
||||
`;
|
||||
|
||||
@@ -297,9 +297,23 @@ export class LibraryStatusIndicator extends LitElement {
|
||||
// row to one and nothing to the other, and "Add … to library"
|
||||
// was the old button's promise written into the copy.
|
||||
if (this.actionable) {
|
||||
return this.status === 'queued'
|
||||
? `Cancel the request for ${kind}${name}`
|
||||
: `Request ${kind}${name}`;
|
||||
if (this.status === 'queued') {
|
||||
return `Cancel the request for ${kind}${name}`;
|
||||
}
|
||||
|
||||
// A partly-held album is actionable *and* has a count, and
|
||||
// the count does not survive being named after the action
|
||||
// alone. The `partial` case below says why it matters — a
|
||||
// ring says "some" to a sighted user and nothing to anyone
|
||||
// else — and that argument does not stop applying because
|
||||
// the badge became clickable. This branch used to say only
|
||||
// "Request album X", so the one state the ring exists for
|
||||
// was the one state whose name did not mention it.
|
||||
if (this.status === 'partial') {
|
||||
return `Request the rest of ${kind}${name} — ${this.owned} of ${this.expected} tracks are in your library`;
|
||||
}
|
||||
|
||||
return `Request ${kind}${name}`;
|
||||
}
|
||||
|
||||
switch (this.status) {
|
||||
|
||||
@@ -11,8 +11,16 @@ import '../library-status-indicator/library-status-indicator.js';
|
||||
import type { LibraryStatus } from '../library-status-indicator/library-status-indicator.js';
|
||||
import { creditLink, exploreLinkStyles } from '../../utils/explore-link';
|
||||
import { creditStore } from '@store/credit-store';
|
||||
import { libraryStatusFor } from '../../utils/library-status';
|
||||
import { albumBadgeFor, libraryStatusFor } from '../../utils/library-status';
|
||||
import {
|
||||
isOwned,
|
||||
ownershipLabel,
|
||||
unownedStyles,
|
||||
type OwnableKind,
|
||||
} from '../../utils/ownership';
|
||||
import { completenessStore } from '../../store/completeness-store';
|
||||
import { downloadStore } from '../../store/download-store';
|
||||
import { classMap } from 'lit/directives/class-map.js';
|
||||
|
||||
/** Format milliseconds as m:ss. */
|
||||
function formatDuration(ms: number | undefined): string {
|
||||
@@ -65,6 +73,9 @@ export class TopResultsRow extends LitElement {
|
||||
/** Unsubscribes the credit-arrival repaint. */
|
||||
private creditsUnsub?: () => void;
|
||||
|
||||
/** Unsubscribes the "how much of this album is here" repaint. */
|
||||
private unsubCompleteness?: () => void;
|
||||
|
||||
override connectedCallback(): void {
|
||||
super.connectedCallback();
|
||||
|
||||
@@ -74,6 +85,9 @@ export class TopResultsRow extends LitElement {
|
||||
this.unsubRequests = downloadStore.subscribe(() =>
|
||||
this.requestUpdate(),
|
||||
);
|
||||
this.unsubCompleteness = completenessStore.subscribe(() =>
|
||||
this.requestUpdate(),
|
||||
);
|
||||
}
|
||||
|
||||
override disconnectedCallback(): void {
|
||||
@@ -81,12 +95,15 @@ export class TopResultsRow extends LitElement {
|
||||
this.creditsUnsub = undefined;
|
||||
this.unsubRequests?.();
|
||||
this.unsubRequests = undefined;
|
||||
this.unsubCompleteness?.();
|
||||
this.unsubCompleteness = undefined;
|
||||
super.disconnectedCallback();
|
||||
}
|
||||
|
||||
static override styles = [
|
||||
designTokens,
|
||||
exploreLinkStyles,
|
||||
unownedStyles,
|
||||
css`
|
||||
:host {
|
||||
display: block;
|
||||
@@ -289,23 +306,41 @@ export class TopResultsRow extends LitElement {
|
||||
? r.year || ''
|
||||
: formatDuration(r.length) || '';
|
||||
|
||||
const status: LibraryStatus = libraryStatusFor(
|
||||
Boolean(r.inLibrary),
|
||||
r.mbid,
|
||||
);
|
||||
const entityType: 'artist' | 'album' | 'track' =
|
||||
const entityType: OwnableKind =
|
||||
r.entityType === 'artist'
|
||||
? 'artist'
|
||||
: r.entityType === 'release_group'
|
||||
? 'album'
|
||||
: 'track';
|
||||
|
||||
// Ownership is the local row, not the catalog's flag — see
|
||||
// `utils/ownership.ts`. An album additionally says *how much*
|
||||
// of it is here, which is the one thing a tick cannot.
|
||||
const owned = isOwned(r);
|
||||
const badge =
|
||||
entityType === 'album'
|
||||
? albumBadgeFor(r, r.mbid)
|
||||
: {
|
||||
status: libraryStatusFor(owned, r.mbid) as LibraryStatus,
|
||||
owned: 0,
|
||||
expected: 0,
|
||||
};
|
||||
|
||||
// A card navigates whether or not the entity is owned, so it is
|
||||
// not `aria-disabled` the way an unplayable track row is — the
|
||||
// name is what carries the state to anyone not seeing the
|
||||
// dimming.
|
||||
return html`
|
||||
<div
|
||||
class="card"
|
||||
class=${classMap({ card: true, unowned: !owned })}
|
||||
role="button"
|
||||
tabindex="0"
|
||||
aria-label=${`${badgeLabel(r.entityType)}: ${r.name}`}
|
||||
aria-label=${ownershipLabel(
|
||||
owned,
|
||||
`${badgeLabel(r.entityType)}:`,
|
||||
r.name,
|
||||
entityType,
|
||||
)}
|
||||
@click=${() => this.handleClick(r)}
|
||||
@keydown=${(e: KeyboardEvent) => {
|
||||
if (e.key !== 'Enter' && e.key !== ' ') return;
|
||||
@@ -345,10 +380,12 @@ export class TopResultsRow extends LitElement {
|
||||
: nothing}
|
||||
</div>
|
||||
</div>
|
||||
${isArtist
|
||||
${isArtist || badge.status === 'in-library'
|
||||
? nothing
|
||||
: html`<library-status-indicator
|
||||
status=${status}
|
||||
status=${badge.status}
|
||||
owned=${badge.owned}
|
||||
expected=${badge.expected}
|
||||
entity-type=${entityType}
|
||||
label=${r.name}
|
||||
request-mbid=${r.mbid}
|
||||
|
||||
@@ -0,0 +1,208 @@
|
||||
/**
|
||||
* How much of an album is here, keyed by local album id.
|
||||
*
|
||||
* The album page has always been able to say "9 of 12"; a card could
|
||||
* not, so an album held two tracks of ten wore a plain green tick on
|
||||
* every grid in the app — which is the complaint the badge-accuracy
|
||||
* issue was filed about, one surface over. A ring with a count needs a
|
||||
* numerator and a denominator per album, and one `GetAlbumCompleteness`
|
||||
* per card is fifty round trips for a grid of fifty.
|
||||
*
|
||||
* So this is `credit-store` one question over, and for the same three
|
||||
* reasons.
|
||||
*
|
||||
* **The lookup is batched and coalesced across callers.** `request()`
|
||||
* is per-card, cheap and safe to call from a render: it collects into a
|
||||
* pending set and flushes once on the next frame, which turns a
|
||||
* screenful into exactly one `GetAlbumsCompleteness`. A frame rather
|
||||
* than a microtask because the point is to collect every card an update
|
||||
* pass renders, and those do not land inside one microtask checkpoint.
|
||||
*
|
||||
* **Absence is cached as an answer.** An album with no files at all is
|
||||
* omitted by the backend deliberately — "I have none of this" and "I
|
||||
* have no idea" are the third state `known` exists to keep apart — so a
|
||||
* miss stored as a hit would re-ask about it forever. What is cached is
|
||||
* *asked*, and the unknown answer is a first-class value.
|
||||
*
|
||||
* **It is invalidated rather than aged.** Completeness is a fact about
|
||||
* files on disk, and the two events that change it — a scan finishing
|
||||
* and tags being rewritten — are exactly the ones `library-store`
|
||||
* already discards everything on. `TracksRemovedFromLibrary` is the
|
||||
* third: it changes a numerator without a rescan.
|
||||
*/
|
||||
|
||||
import { GetAlbumsCompleteness } from '@go/library/library.js';
|
||||
import type * as library from '@go/library/models.js';
|
||||
import { EventsOn } from '@runtime/runtime';
|
||||
import { Events } from '../events';
|
||||
import { LRUMap } from '../utils/lru-map';
|
||||
import { compact } from '../utils/binding';
|
||||
import { registerCacheProbe } from '../utils/cache-stats';
|
||||
|
||||
/**
|
||||
* Entries retained. Four small numbers each, so the bound is about
|
||||
* unbounded growth rather than bytes — and it is set well above any one
|
||||
* grid, since a cap below the visible count evicts cards that are still
|
||||
* on screen and the re-render fetches them straight back.
|
||||
*/
|
||||
export const COMPLETENESS_CACHE_LIMIT = 20_000;
|
||||
|
||||
/** What the app asks of a cached entry. */
|
||||
export type Completeness = library.AlbumCompleteness;
|
||||
|
||||
/**
|
||||
* The answer for an album nobody has a file of.
|
||||
*
|
||||
* Not a zero-of-zero: `known` false is what every consumer already
|
||||
* reads as "say nothing", and it is the same value the album page's
|
||||
* `completenessAnswer()` treats as unknowable.
|
||||
*/
|
||||
const NOTHING_HERE: Completeness = {
|
||||
owned: 0,
|
||||
expected: 0,
|
||||
known: false,
|
||||
complete: false,
|
||||
} as Completeness;
|
||||
|
||||
class CompletenessStore {
|
||||
private cache = new LRUMap<number, Completeness>(COMPLETENESS_CACHE_LIMIT);
|
||||
|
||||
/** Ids with a request in flight, so a re-render does not refetch. */
|
||||
private inFlight = new Set<number>();
|
||||
|
||||
private listeners = new Set<() => void>();
|
||||
|
||||
/** Collected by request(), flushed as one batch on the next frame. */
|
||||
private pending = new Set<number>();
|
||||
|
||||
private flushHandle: number | null = null;
|
||||
|
||||
constructor() {
|
||||
registerCacheProbe('albumCompleteness', () => ({
|
||||
entries: this.cache.size,
|
||||
chars: this.cache.size * 4,
|
||||
limit: COMPLETENESS_CACHE_LIMIT,
|
||||
}));
|
||||
|
||||
EventsOn(Events.LibraryScanComplete, () => this.invalidate());
|
||||
EventsOn(Events.TrackMetadataChanged, () => this.invalidate());
|
||||
EventsOn(Events.TracksRemovedFromLibrary, () => this.invalidate());
|
||||
}
|
||||
|
||||
/**
|
||||
* Subscribe to "some answers arrived".
|
||||
*
|
||||
* Deliberately not per-album, for `credit-store`'s reason: a grid
|
||||
* fetches its cards in one call and re-renders once, so a
|
||||
* fine-grained signal would buy nothing and cost a listener a card.
|
||||
*/
|
||||
subscribe(fn: () => void): () => void {
|
||||
this.listeners.add(fn);
|
||||
|
||||
return () => this.listeners.delete(fn);
|
||||
}
|
||||
|
||||
/** The answer for one album, or undefined until it has been asked. */
|
||||
get(albumID: number | undefined | null): Completeness | undefined {
|
||||
if (!albumID || albumID <= 0) return undefined;
|
||||
|
||||
return this.cache.get(albumID);
|
||||
}
|
||||
|
||||
/**
|
||||
* Ask about one album, joining whatever batch is forming.
|
||||
*
|
||||
* Safe from inside a render: a set insert and a scheduled flush,
|
||||
* with anything cached or in flight dropped. It does not loop —
|
||||
* after a flush every id asked for is cached, so the re-render's
|
||||
* requests are all dropped and nothing notifies again.
|
||||
*/
|
||||
request(albumID: number | undefined | null): void {
|
||||
if (!albumID || albumID <= 0) return;
|
||||
if (this.cache.has(albumID)) return;
|
||||
if (this.inFlight.has(albumID)) return;
|
||||
if (this.pending.has(albumID)) return;
|
||||
|
||||
this.pending.add(albumID);
|
||||
|
||||
if (this.flushHandle !== null) return;
|
||||
|
||||
this.flushHandle = requestAnimationFrame(() => {
|
||||
this.flushHandle = null;
|
||||
|
||||
const batch = [...this.pending];
|
||||
|
||||
this.pending.clear();
|
||||
|
||||
void this.ensure(batch);
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Ask and read in one call, for use inside a template.
|
||||
*
|
||||
* A getter with a side effect, deliberately — `credit-store` makes
|
||||
* the same trade and for the same reason: the alternative is every
|
||||
* call site writing `request(x)` beside `get(x)` and one of them
|
||||
* eventually forgetting, which renders a permanently unknown
|
||||
* completeness that looks exactly like an album with no totals.
|
||||
*/
|
||||
completeness(albumID: number | undefined | null): Completeness | undefined {
|
||||
this.request(albumID);
|
||||
|
||||
return this.get(albumID);
|
||||
}
|
||||
|
||||
/** Fetch for a list, skipping anything known or already in flight. */
|
||||
async ensure(albumIDs: readonly number[]): Promise<void> {
|
||||
const wanted = new Set<number>();
|
||||
|
||||
for (const id of albumIDs) {
|
||||
if (!id || id <= 0) continue;
|
||||
// `has` rather than `get`: probing must not mark an entry
|
||||
// recently-used, or scrolling past a card would keep it
|
||||
// alive ahead of one actually being rendered.
|
||||
if (this.cache.has(id)) continue;
|
||||
if (this.inFlight.has(id)) continue;
|
||||
|
||||
wanted.add(id);
|
||||
}
|
||||
|
||||
if (wanted.size === 0) return;
|
||||
|
||||
const batch = [...wanted];
|
||||
|
||||
for (const id of batch) this.inFlight.add(id);
|
||||
|
||||
try {
|
||||
const found = compact(await GetAlbumsCompleteness(batch));
|
||||
|
||||
for (const id of batch) {
|
||||
this.cache.set(id, found[String(id)] ?? NOTHING_HERE);
|
||||
}
|
||||
|
||||
this.notify();
|
||||
} catch (err) {
|
||||
// A count is an enrichment: without it a card shows the
|
||||
// plain "you have this", which is what it showed before and
|
||||
// is a weaker answer rather than a broken one.
|
||||
console.error('Failed to load album completeness', err);
|
||||
} finally {
|
||||
for (const id of batch) this.inFlight.delete(id);
|
||||
}
|
||||
}
|
||||
|
||||
/** Drop everything: the files on disk changed. */
|
||||
invalidate(): void {
|
||||
this.cache = new LRUMap<number, Completeness>(
|
||||
COMPLETENESS_CACHE_LIMIT,
|
||||
);
|
||||
this.notify();
|
||||
}
|
||||
|
||||
private notify(): void {
|
||||
for (const fn of this.listeners) fn();
|
||||
}
|
||||
}
|
||||
|
||||
export const completenessStore = new CompletenessStore();
|
||||
@@ -1,7 +1,9 @@
|
||||
import { completenessStore } from '@store/completeness-store';
|
||||
import { downloadStore } from '@store/download-store';
|
||||
import { libraryStore } from '@store/library-store';
|
||||
import type * as download from '@go/download/models.js';
|
||||
import type { LibraryStatus } from '../components/library-status-indicator/library-status-indicator';
|
||||
import { isOwned, type Ownable } from './ownership';
|
||||
|
||||
/**
|
||||
* What the tick/hourglass/plus badge should say about one entity.
|
||||
@@ -46,6 +48,61 @@ export function libraryStatusFor(
|
||||
return 'not-in-library';
|
||||
}
|
||||
|
||||
/**
|
||||
* Everything a badge needs about one entity, decided in one place.
|
||||
*
|
||||
* `status` is the state; `owned`/`expected` are the counts behind
|
||||
* `partial` and are zero for every other state, which is what the badge
|
||||
* requires — it documents that a caller with no total must not pass a
|
||||
* ring at 0%.
|
||||
*/
|
||||
export interface BadgeState {
|
||||
status: LibraryStatus;
|
||||
owned: number;
|
||||
expected: number;
|
||||
}
|
||||
|
||||
/**
|
||||
* What the badge on an album card should say.
|
||||
*
|
||||
* Three rules, and the middle one is the whole point of this issue.
|
||||
*
|
||||
* **Ownership is the local album id**, per `utils/ownership.ts` — a
|
||||
* file, not the catalog's `inLibrary` ratchet.
|
||||
*
|
||||
* **A partly-held album says how partly.** The count comes from
|
||||
* `completenessStore`, which batches a screenful into one query;
|
||||
* reading it is what asks for it. Before this, an album held 2 tracks
|
||||
* of 10 wore the same green tick as one held whole on every grid in
|
||||
* the app.
|
||||
*
|
||||
* **A total that was never declared is not a total of zero.** Where
|
||||
* `known` is false — most of an untagged library, and every album until
|
||||
* a rescan repopulates `audio_files.total_tracks` — this is a plain
|
||||
* `in-library` and says nothing, which is the rule the badge's own
|
||||
* documentation states and the reason `Known` exists at all.
|
||||
*/
|
||||
export function albumBadgeFor(
|
||||
album: Ownable | null | undefined,
|
||||
mbid?: string | null,
|
||||
): BadgeState {
|
||||
if (!isOwned(album)) {
|
||||
return { status: libraryStatusFor(false, mbid), owned: 0, expected: 0 };
|
||||
}
|
||||
|
||||
const held = completenessStore.completeness(album?.localId);
|
||||
|
||||
if (held?.known && !held.complete) {
|
||||
return {
|
||||
status: 'partial',
|
||||
owned: held.owned,
|
||||
expected: held.expected,
|
||||
};
|
||||
}
|
||||
|
||||
return { status: 'in-library', owned: 0, expected: 0 };
|
||||
}
|
||||
|
||||
/** What a badge can ask for. Artists are deliberately absent: a
|
||||
* discography subscription is `explore-artist-details`'s Follow
|
||||
* button, which can say what it is committing to. */
|
||||
|
||||
@@ -0,0 +1,134 @@
|
||||
/**
|
||||
* What "I do not own this" looks like, and how the app decides it.
|
||||
*
|
||||
* The rule the user asked for, in their words: *owned content is the
|
||||
* default, normal, unadorned presentation; unowned content is what gets
|
||||
* marked*. `explore-album-details` implemented it for one tracklist —
|
||||
* dimmed in place, `aria-disabled` because dimming is a colour and
|
||||
* cannot be the only signal, and nothing at all drawn on the owned rows
|
||||
* — and every other catalog surface still mixed the two with a small
|
||||
* badge as the only difference. This is that rule, written once, so
|
||||
* eight surfaces cannot each keep their own version of it.
|
||||
*
|
||||
* ## Ownership is a file, and `localId` is the flag that says so
|
||||
*
|
||||
* The album page answers "do I own this row" with `filePaths`, a map
|
||||
* from a displayed track to a real file. A card grid cannot afford a
|
||||
* lookup per card — and does not need one, because the answer is
|
||||
* already on every model.
|
||||
*
|
||||
* `explore_index.local_artist_id` / `local_release_group_id` /
|
||||
* `local_recording_id` are built by `collectLibraryEntities` from
|
||||
* queries that every one join `audio_files`, and cleared by
|
||||
* `pruneStaleLocalCrossReferences` whose existence test is a file test
|
||||
* in all three cases. That is the same "ownership is a file" rule,
|
||||
* computed once per scan instead of once per screenful.
|
||||
*
|
||||
* **`inLibrary` is the weaker one and is deliberately not consulted.**
|
||||
* It is written by the same pass, so today the two agree — but it is a
|
||||
* one-way ratchet (`in_library = MAX(in_library, excluded.in_library)`)
|
||||
* whose only clearing pass is gated on a non-null `local_*_id`, so it
|
||||
* cannot be un-set on its own. One of the two is a fact with an owner;
|
||||
* the other is a flag that happens to agree with it.
|
||||
*
|
||||
* The divergence was observable before this: both `explore-view` and
|
||||
* `explore-artist-details` kept a `libraryMBIDs` set that accumulated
|
||||
* every MBID ever seen with `inLibrary` and cleared it never, in a view
|
||||
* that never unmounts. And on one artist-detail card the two answers
|
||||
* were used side by side — the context menu gated Play on
|
||||
* `localId > 0` while the badge said "in your library" from
|
||||
* `inLibrary`, so a card could claim to be owned, offer no Play, and
|
||||
* (the request item being gated on *not* owned) offer no way to ask for
|
||||
* it either.
|
||||
*/
|
||||
|
||||
import { css } from 'lit';
|
||||
|
||||
/** Anything a card or row can be drawn from, as far as this is concerned. */
|
||||
export interface Ownable {
|
||||
/** The local row id behind this entity: an album, a file, an artist. */
|
||||
localId?: number | null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether there is something of the user's behind this entity.
|
||||
*
|
||||
* Deliberately narrow: a local id and nothing else. Passing the model
|
||||
* straight in is the point — a call site that has to remember which of
|
||||
* two fields to read is a call site that will eventually read the other
|
||||
* one, which is exactly how the two answers came to sit on one card.
|
||||
*/
|
||||
export function isOwned(entity: Ownable | null | undefined): boolean {
|
||||
return (entity?.localId ?? 0) > 0;
|
||||
}
|
||||
|
||||
/** The kinds of thing a catalog surface can draw. */
|
||||
export type OwnableKind = 'album' | 'track' | 'artist';
|
||||
|
||||
/**
|
||||
* The sentence an unowned thing says, once.
|
||||
*
|
||||
* It reaches whoever is not seeing the dimming, so it has to name the
|
||||
* thing as well as the state — "not in your library" alone, repeated
|
||||
* down a grid, identifies nothing. The em dash matches the album
|
||||
* tracklist's existing phrasing, which is where this came from.
|
||||
*/
|
||||
export function unownedLabel(name: string, kind: OwnableKind): string {
|
||||
return `${name} — not in your library, ${
|
||||
kind === 'artist' ? 'browsing the catalog' : 'available to request'
|
||||
}`;
|
||||
}
|
||||
|
||||
/**
|
||||
* The accessible name for a card or row, owned or not.
|
||||
*
|
||||
* `activates` is what the thing does when it is yours: "Play", "Album",
|
||||
* whatever the surface's own verb is. An unowned one does not get that
|
||||
* verb, because it cannot do it.
|
||||
*/
|
||||
export function ownershipLabel(
|
||||
owned: boolean,
|
||||
activates: string,
|
||||
name: string,
|
||||
kind: OwnableKind,
|
||||
): string {
|
||||
return owned ? `${activates} ${name}` : unownedLabel(name, kind);
|
||||
}
|
||||
|
||||
/**
|
||||
* The dimming, shared so it cannot drift across surfaces.
|
||||
*
|
||||
* Two things about it are load-bearing.
|
||||
*
|
||||
* **The text dims to a token, not with `opacity`.** `theme-store`'s
|
||||
* ramps are checked by `theme-contrast.test.ts` against every surface
|
||||
* text can sit on; an opacity multiplier is outside that check and
|
||||
* would quietly drop a dimmed title under 4.5:1 on the light ramps.
|
||||
* Secondary rather than tertiary for the reason the album tracklist
|
||||
* gives: these rows and cards have a `bgOverlay` hover background,
|
||||
* which tertiary does not clear.
|
||||
*
|
||||
* **Only the artwork takes an `opacity`.** A cover is not text, so it
|
||||
* is outside the contrast rule entirely, and it is the part of a card
|
||||
* that carries the most weight — dimming it is what makes a grid read
|
||||
* as catalog at a glance rather than needing the badge to be found.
|
||||
*/
|
||||
export const unownedStyles = css`
|
||||
.unowned .album-title,
|
||||
.unowned .track-title,
|
||||
.unowned .card-name,
|
||||
.unowned .top-release-title,
|
||||
.unowned .artist-name {
|
||||
color: var(--yj-text-secondary, #b3b3b3);
|
||||
font-weight: 400;
|
||||
}
|
||||
|
||||
.unowned .album-art-container,
|
||||
.unowned .top-release-art,
|
||||
.unowned .track-art,
|
||||
.unowned .card-image,
|
||||
.unowned .card-image-placeholder,
|
||||
.unowned .artist-avatar {
|
||||
opacity: 0.55;
|
||||
}
|
||||
`;
|
||||
@@ -77,3 +77,30 @@ describe('the library status badge', () => {
|
||||
expect(name).toContain('Glass Harbour');
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* A partly-held album is the one state that is *actionable and
|
||||
* counted*: there are tracks left to ask for, so the badge is a button
|
||||
* — and a control is named after what activating it does, which is how
|
||||
* the count came to be dropped from exactly the state the ring exists
|
||||
* for. Both, or the ring says "some" to an eye and nothing to anyone
|
||||
* else.
|
||||
*/
|
||||
describe('a partial badge that can act', () => {
|
||||
it('names the action and keeps the count', async () => {
|
||||
const el = await fixture('library-status-indicator', {
|
||||
status: 'partial',
|
||||
owned: 9,
|
||||
expected: 12,
|
||||
entityType: 'album',
|
||||
label: 'Glass Harbour',
|
||||
requestMbid: 'rg-1',
|
||||
});
|
||||
|
||||
const name = shadow(el, '.badge')?.getAttribute('aria-label') ?? '';
|
||||
|
||||
expect(shadow(el, 'button.badge')).not.toBeNull();
|
||||
expect(name).toContain('Request the rest of');
|
||||
expect(name).toContain('9 of 12');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -0,0 +1,363 @@
|
||||
/**
|
||||
* Owned is plain; unowned is what gets marked.
|
||||
*
|
||||
* `explore-album-details` had this right for one tracklist and nothing
|
||||
* else did: Explore's cards, the top-results row and the artist page's
|
||||
* three card shapes all mixed owned and unowned with a small badge as
|
||||
* the only difference — and drew a green tick on the *common* case,
|
||||
* which is the treatment the album page's own green ticks were removed
|
||||
* for.
|
||||
*
|
||||
* What is pinned here is the rule rather than any one surface, because
|
||||
* the fault this replaced was eight call sites each holding their own
|
||||
* version of it:
|
||||
*
|
||||
* - an owned thing draws **no badge at all**;
|
||||
* - an unowned one is dimmed *and* says so in its accessible name,
|
||||
* because dimming is a colour and cannot be the only signal;
|
||||
* - ownership is a **file** (`localId`), never the catalog's
|
||||
* `inLibrary` ratchet, which is a flag that happens to agree;
|
||||
* - and a partly-held album says *how* partly, which is the one thing
|
||||
* a tick cannot.
|
||||
*/
|
||||
import { beforeEach, describe, expect, it } from 'vitest';
|
||||
import { page } from 'vitest/browser';
|
||||
|
||||
import '@components/explore-view/explore-view';
|
||||
import '@components/top-results-row/top-results-row';
|
||||
import { flush, stub, resetHarness } from '@test/support/harness';
|
||||
import { fixture, shadow, shadowAll, update } from '@test/support/render';
|
||||
import { completenessStore } from '@store/completeness-store';
|
||||
|
||||
const SEARCH = 'explore.Service.SearchLocal';
|
||||
const SHELVES = 'explore.Service.GetExploreShelves';
|
||||
const COMPLETENESS = 'library.Library.GetAlbumsCompleteness';
|
||||
|
||||
/** A release group as the backend projects one. */
|
||||
function album(
|
||||
title: string,
|
||||
{ localId = 0, inLibrary = false }: { localId?: number; inLibrary?: boolean },
|
||||
) {
|
||||
return {
|
||||
mbid: `rg-${title}`,
|
||||
title,
|
||||
artistCredit: 'An Artist',
|
||||
artistMbid: 'ar-1',
|
||||
primaryType: 'Album',
|
||||
firstReleaseDate: '1994-05-01',
|
||||
popularity: 100,
|
||||
listenerCount: 10,
|
||||
secondaryTypes: [],
|
||||
inLibrary,
|
||||
localId,
|
||||
};
|
||||
}
|
||||
|
||||
/** A recording as the backend projects one. */
|
||||
function recording(
|
||||
title: string,
|
||||
{ localId = 0, inLibrary = false }: { localId?: number; inLibrary?: boolean },
|
||||
) {
|
||||
return {
|
||||
mbid: `rec-${title}`,
|
||||
title,
|
||||
artistCredit: 'An Artist',
|
||||
artistMbid: 'ar-1',
|
||||
length: 200000,
|
||||
popularity: 0,
|
||||
listenerCount: 0,
|
||||
inLibrary,
|
||||
localId,
|
||||
};
|
||||
}
|
||||
|
||||
/** Mount Explore showing one page of results. */
|
||||
async function exploreShowing(results: {
|
||||
releaseGroups?: unknown[];
|
||||
recordings?: unknown[];
|
||||
artists?: unknown[];
|
||||
}) {
|
||||
stub(SHELVES, { shelves: [], state: 'ready' });
|
||||
stub(SEARCH, {
|
||||
artists: [],
|
||||
releaseGroups: [],
|
||||
recordings: [],
|
||||
...results,
|
||||
});
|
||||
|
||||
const el = await fixture('explore-view');
|
||||
|
||||
// A cached primary view only fetches on arrival, and the search is
|
||||
// what these cards come from.
|
||||
(el as unknown as { onViewActivate: () => void }).onViewActivate?.();
|
||||
await update(el, { results: { artists: [], releaseGroups: [], recordings: [], ...results } });
|
||||
await flush();
|
||||
await el.updateComplete;
|
||||
|
||||
return el;
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
resetHarness();
|
||||
stub(COMPLETENESS, {});
|
||||
|
||||
// The store is a singleton and caches an *answer*, including the
|
||||
// absent one — which is the point, or 87% of a grid re-asks forever.
|
||||
// Two tests in one file are two sessions as far as it is concerned,
|
||||
// so a stale entry from the test above would otherwise decide the
|
||||
// one below. Found by writing the assertion the wrong way round.
|
||||
completenessStore.invalidate();
|
||||
});
|
||||
|
||||
describe('an owned thing is plain', () => {
|
||||
it('draws no badge on an album card it has files for', async () => {
|
||||
const el = await exploreShowing({
|
||||
releaseGroups: [album('Held', { localId: 7 })],
|
||||
});
|
||||
|
||||
expect(shadowAll(el, '.album-card')).toHaveLength(1);
|
||||
expect(shadow(el, '.album-card library-status-indicator')).toBeNull();
|
||||
});
|
||||
|
||||
it('draws no badge on a track row it has a file for', async () => {
|
||||
const el = await exploreShowing({
|
||||
recordings: [recording('Held', { localId: 9 })],
|
||||
});
|
||||
|
||||
expect(shadowAll(el, '.track-item')).toHaveLength(1);
|
||||
expect(shadow(el, '.track-item library-status-indicator')).toBeNull();
|
||||
});
|
||||
|
||||
it('does not dim it', async () => {
|
||||
const el = await exploreShowing({
|
||||
releaseGroups: [album('Held', { localId: 7 })],
|
||||
});
|
||||
|
||||
expect(shadow(el, '.album-card')?.classList.contains('unowned')).toBe(
|
||||
false,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('an unowned thing is marked', () => {
|
||||
it('dims the card and keeps its request badge', async () => {
|
||||
const el = await exploreShowing({
|
||||
releaseGroups: [album('Absent', {})],
|
||||
});
|
||||
|
||||
expect(shadow(el, '.album-card')?.classList.contains('unowned')).toBe(true);
|
||||
expect(shadow(el, '.album-card library-status-indicator')).not.toBeNull();
|
||||
});
|
||||
|
||||
/**
|
||||
* The name is the half of this that reaches anyone not seeing the
|
||||
* dimming, so it has to be the browser's own answer — a shadow-root
|
||||
* query cannot compute a name, and this repo has shipped a nameless
|
||||
* control three times.
|
||||
*/
|
||||
it('says so in the name the browser computes', async () => {
|
||||
await exploreShowing({ releaseGroups: [album('Absent', {})] });
|
||||
|
||||
await expect
|
||||
.element(page.getByRole('button', { name: /Absent — not in your library/ }))
|
||||
.toBeInTheDocument();
|
||||
});
|
||||
|
||||
/**
|
||||
* A track row is `aria-disabled` and a card is not, and the
|
||||
* difference is not cosmetic: activating an unowned row does nothing
|
||||
* (`onRecordingRowDblClick` returns early), while a card navigates to
|
||||
* the catalog page for it, which is a perfectly good thing to do with
|
||||
* something you do not own.
|
||||
*/
|
||||
it('marks a row that cannot be played as disabled', async () => {
|
||||
const el = await exploreShowing({
|
||||
recordings: [recording('Absent', {})],
|
||||
});
|
||||
|
||||
expect(shadow(el, '.track-item')?.getAttribute('aria-disabled')).toBe(
|
||||
'true',
|
||||
);
|
||||
});
|
||||
|
||||
it('leaves a card that still navigates enabled', async () => {
|
||||
const el = await exploreShowing({
|
||||
releaseGroups: [album('Absent', {})],
|
||||
});
|
||||
|
||||
expect(shadow(el, '.album-card')?.getAttribute('aria-disabled')).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* The decision this issue turned on.
|
||||
*
|
||||
* `inLibrary` is written by the same pass that writes the local ids, so
|
||||
* the two agree in a healthy database — but it is a one-way ratchet
|
||||
* (`MAX(in_library, excluded.in_library)`) whose only clearing pass is
|
||||
* gated on a non-null `local_*_id`, so it cannot be un-set on its own.
|
||||
* A row carrying it with no local id behind it is a claim of ownership
|
||||
* with no file, which is exactly what the album page refuses to trust.
|
||||
*/
|
||||
describe('ownership is a file, not a flag', () => {
|
||||
it('treats a card flagged inLibrary with no local row as unowned', async () => {
|
||||
const el = await exploreShowing({
|
||||
releaseGroups: [album('Phantom', { inLibrary: true, localId: 0 })],
|
||||
});
|
||||
|
||||
expect(shadow(el, '.album-card')?.classList.contains('unowned')).toBe(true);
|
||||
expect(shadow(el, '.album-card library-status-indicator')).not.toBeNull();
|
||||
});
|
||||
|
||||
it('does the same for a track row', async () => {
|
||||
const el = await exploreShowing({
|
||||
recordings: [recording('Phantom', { inLibrary: true, localId: 0 })],
|
||||
});
|
||||
|
||||
expect(shadow(el, '.track-item')?.getAttribute('aria-disabled')).toBe(
|
||||
'true',
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* The count, which is what `#16`'s deferred third step asked for: an
|
||||
* album held 2 tracks of 10 wore the same green tick as one held whole,
|
||||
* on every grid in the app.
|
||||
*/
|
||||
describe('a partly-held album says how partly', () => {
|
||||
it('draws the ring and puts the count in the badge name', async () => {
|
||||
stub(COMPLETENESS, {
|
||||
'7': { owned: 9, expected: 12, known: true, complete: false },
|
||||
});
|
||||
|
||||
const el = await exploreShowing({
|
||||
releaseGroups: [album('Partly', { localId: 7 })],
|
||||
});
|
||||
|
||||
// The store batches into the next frame, so the answer lands one
|
||||
// repaint after the cards do — which is the thing the subscription
|
||||
// exists for.
|
||||
await new Promise((r) => requestAnimationFrame(() => r(null)));
|
||||
await flush();
|
||||
await el.updateComplete;
|
||||
|
||||
const badge = shadow(el, '.album-card library-status-indicator');
|
||||
|
||||
expect(badge?.getAttribute('status')).toBe('partial');
|
||||
|
||||
// A partly-held album is *actionable* — it has three tracks left to
|
||||
// ask for — so the badge is a button, and the name has to carry the
|
||||
// action and the count. Naming it after the action alone left the
|
||||
// one state the ring exists for as the one state whose name did not
|
||||
// mention it.
|
||||
await expect
|
||||
.element(
|
||||
page.getByRole('button', {
|
||||
name: /Request the rest of album .*Partly.* — 9 of 12 tracks/,
|
||||
}),
|
||||
)
|
||||
.toBeInTheDocument();
|
||||
});
|
||||
|
||||
/**
|
||||
* Where the tags never declared a total, `known` is false and the
|
||||
* card must say nothing — most of an untagged library is in that
|
||||
* state, and a ring drawn from its absence would mark all of it
|
||||
* incomplete on no evidence. That is the rule `Known` exists for.
|
||||
*/
|
||||
it('says nothing when the total was never declared', async () => {
|
||||
stub(COMPLETENESS, {
|
||||
'7': { owned: 3, expected: 0, known: false, complete: false },
|
||||
});
|
||||
|
||||
const el = await exploreShowing({
|
||||
releaseGroups: [album('Untotalled', { localId: 7 })],
|
||||
});
|
||||
|
||||
await new Promise((r) => requestAnimationFrame(() => r(null)));
|
||||
await flush();
|
||||
await el.updateComplete;
|
||||
|
||||
expect(shadow(el, '.album-card library-status-indicator')).toBeNull();
|
||||
});
|
||||
|
||||
it('asks about the owned albums only, in one call', async () => {
|
||||
const seen: unknown[][] = [];
|
||||
|
||||
stub(COMPLETENESS, (...args: unknown[]) => {
|
||||
seen.push(args);
|
||||
|
||||
return {};
|
||||
});
|
||||
|
||||
await exploreShowing({
|
||||
releaseGroups: [
|
||||
album('Held', { localId: 7 }),
|
||||
album('Also held', { localId: 8 }),
|
||||
album('Absent', {}),
|
||||
album('Phantom', { inLibrary: true, localId: 0 }),
|
||||
],
|
||||
});
|
||||
|
||||
await new Promise((r) => requestAnimationFrame(() => r(null)));
|
||||
await flush();
|
||||
|
||||
expect(seen).toHaveLength(1);
|
||||
expect(seen[0]?.[0]).toEqual([7, 8]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('the top-results row follows the same rule', () => {
|
||||
const result = (
|
||||
name: string,
|
||||
entityType: string,
|
||||
extra: Record<string, unknown> = {},
|
||||
) => ({
|
||||
entityType,
|
||||
mbid: `top-${name}`,
|
||||
name,
|
||||
artistCredit: 'An Artist',
|
||||
intentScore: 1,
|
||||
inLibrary: false,
|
||||
...extra,
|
||||
});
|
||||
|
||||
it('draws no badge on something it owns', async () => {
|
||||
const el = await fixture('top-results-row', {
|
||||
results: [result('Held', 'release_group', { localId: 7 })],
|
||||
query: 'held',
|
||||
});
|
||||
|
||||
expect(shadow(el, '.card library-status-indicator')).toBeNull();
|
||||
expect(shadow(el, '.card')?.classList.contains('unowned')).toBe(false);
|
||||
});
|
||||
|
||||
it('dims and names something it does not', async () => {
|
||||
const el = await fixture('top-results-row', {
|
||||
results: [result('Absent', 'release_group')],
|
||||
query: 'absent',
|
||||
});
|
||||
|
||||
expect(shadow(el, '.card')?.classList.contains('unowned')).toBe(true);
|
||||
|
||||
await expect
|
||||
.element(page.getByRole('button', { name: /Absent — not in your library/ }))
|
||||
.toBeInTheDocument();
|
||||
});
|
||||
|
||||
/**
|
||||
* An artist card has never had a badge — a discography subscription
|
||||
* is the artist page's Follow button, which can say what it commits
|
||||
* to — so the dimming and the name are the whole signal there.
|
||||
*/
|
||||
it('marks an unowned artist without offering a request', async () => {
|
||||
const el = await fixture('top-results-row', {
|
||||
results: [result('An Artist', 'artist')],
|
||||
query: 'an artist',
|
||||
});
|
||||
|
||||
expect(shadow(el, '.card')?.classList.contains('unowned')).toBe(true);
|
||||
expect(shadow(el, '.card library-status-indicator')).toBeNull();
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user