Compare commits

..
Author SHA1 Message Date
logan a2ff0aed4c fix(ui): make the queue button say whether the queue is open
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m27s
CI / e2e (pull_request) Canceled after 5m49s
It looked identical in both states, so the only way to tell what
pressing it would do was to look at the other side of the window and
infer it -- and for anyone not looking there was nothing to infer from:
no aria-expanded, no aria-controls, no drawn state.

The state is reflected *from the panel* rather than kept beside the
click. This button is not the only thing that opens the queue --
now-playing-view sets the same attribute, because it hides the bar the
button lives in -- so a flag maintained by the click handler would be
right until something else opened the panel and then quietly wrong.
The panel's `open` attribute stays the one fact; a MutationObserver
reflects it.

Refs #26
2026-08-18 11:15:32 -04:00
17 changed files with 142 additions and 906 deletions
+10 -41
View File
@@ -279,19 +279,17 @@ func (h *MPRISHandler) enqueue(fn func()) {
} }
} }
// metadataMap builds the org.mpris.MediaPlayer2.Player Metadata value // UpdateMetadata pushes track metadata to D-Bus.
// for one track. func (h *MPRISHandler) UpdateMetadata(meta Metadata) {
// h.mu.Lock()
// It is separated from UpdateMetadata, which needs a live D-Bus h.trackID++
// connection, so the map's contents can be asserted on: this file is tid := h.trackID
// behind a build tag and everything in it that touches h is reachable h.mu.Unlock()
// only from a session bus, which is the same reason the Android
// contract lives in an untagged androidpayload.go. m := map[string]interface{}{
func metadataMap(meta Metadata, trackID uint64) map[string]any {
m := map[string]any{
"mpris:trackid": dbus.ObjectPath( "mpris:trackid": dbus.ObjectPath(
fmt.Sprintf( fmt.Sprintf(
"/org/yellowjacket/Track/%d", trackID, "/org/yellowjacket/Track/%d", tid,
), ),
), ),
} }
@@ -308,45 +306,16 @@ func metadataMap(meta Metadata, trackID uint64) map[string]any {
m["xesam:album"] = meta.Album m["xesam:album"] = meta.Album
} }
// Always present, even with nothing to point at.
//
// Every other key here can be omitted safely because a client
// reading the map sees a track with no title or no album and
// renders it that way. Art is different: KDE's applet (and
// others) treat an *absent* mpris:artUrl as "no news about the
// art" and keep drawing whatever the last track had, so playing
// something with no cover left the previous album's sleeve on
// screen — which reads as the wrong track playing rather than as
// missing artwork.
//
// An empty string is the honest answer and is what the spec's
// "URI" type degrades to; a client that cannot load it falls back
// to its own placeholder, which is the behaviour wanted.
artURL := ""
if meta.ArtFilePath != "" { if meta.ArtFilePath != "" {
artURL = "file://" + meta.ArtFilePath m["mpris:artUrl"] = "file://" + meta.ArtFilePath
} }
m["mpris:artUrl"] = artURL
if meta.DurationSec > 0 { if meta.DurationSec > 0 {
m["mpris:length"] = int64( m["mpris:length"] = int64(
meta.DurationSec, meta.DurationSec,
) * usPerSec ) * usPerSec
} }
return m
}
// UpdateMetadata pushes track metadata to D-Bus.
func (h *MPRISHandler) UpdateMetadata(meta Metadata) {
h.mu.Lock()
h.trackID++
tid := h.trackID
h.mu.Unlock()
m := metadataMap(meta, tid)
h.enqueue(func() { h.enqueue(func() {
h.props.SetMust(playerIf, "Metadata", m) h.props.SetMust(playerIf, "Metadata", m)
}) })
-86
View File
@@ -1,86 +0,0 @@
//go:build linux && !android
package mediacontrols
import "testing"
// The one key that must be present even when it is empty.
//
// Everything else in the map may be omitted, because a client reading
// it renders a track with no title as a track with no title. Art is
// different: KDE's applet treats an *absent* mpris:artUrl as no news
// about the art and keeps drawing the last one it saw, so a track with
// no cover wore the previous album's sleeve — which reads as the wrong
// track playing rather than as missing artwork.
func TestMetadataMapAlwaysCarriesArtURL(t *testing.T) {
t.Parallel()
tests := []struct {
name string
meta Metadata
want string
}{
{
name: "no art at all",
meta: Metadata{Title: "Blue in Green"},
want: "",
},
{
name: "art on disk",
meta: Metadata{
Title: "Blue in Green",
ArtFilePath: "/covers/kind-of-blue_lg.jpg",
},
want: "file:///covers/kind-of-blue_lg.jpg",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
m := metadataMap(tt.meta, 1)
got, ok := m["mpris:artUrl"]
if !ok {
t.Fatal("mpris:artUrl is absent; it must always be sent")
}
if got != tt.want {
t.Errorf("mpris:artUrl = %v, want %q", got, tt.want)
}
})
}
}
// The trackid has to change between tracks or a client is entitled to
// treat the metadata as describing the same track it already has.
func TestMetadataMapTrackIDVaries(t *testing.T) {
t.Parallel()
first := metadataMap(Metadata{Title: "A"}, 1)["mpris:trackid"]
second := metadataMap(Metadata{Title: "B"}, 2)["mpris:trackid"]
if first == second {
t.Errorf("trackid did not change: %v", first)
}
}
// The optional keys stay optional — this is what makes artUrl's
// always-present treatment a deliberate exception rather than drift.
func TestMetadataMapOmitsEmptyOptionalFields(t *testing.T) {
t.Parallel()
m := metadataMap(Metadata{}, 1)
for _, key := range []string{
"xesam:title",
"xesam:artist",
"xesam:album",
"mpris:length",
} {
if _, ok := m[key]; ok {
t.Errorf("%s is present for an empty Metadata", key)
}
}
}
-1
View File
@@ -81,7 +81,6 @@ func (q *Queue) emitTracksModified(
Index: index, Index: index,
Positions: positions, Positions: positions,
CurrentIndex: q.currentIndex, CurrentIndex: q.currentIndex,
Source: q.source,
}, },
) )
} }
-50
View File
@@ -219,56 +219,6 @@ func TestEmit_AddTrackSendsDeltaNotSnapshot(t *testing.T) {
} }
} }
// The append clears the source, and the delta is the only event those
// paths emit — so if it does not carry the source, the frontend keeps
// the label it was last given and goes on offering a link back to an
// album the queue no longer holds until something forces a full state.
func TestEmit_AppendDeltaCarriesClearedSource(t *testing.T) {
t.Parallel()
q, db, rec := setupRecordedQueue(t)
paths := seedAudioFiles(t, db, 4)
q.SetQueue(
paths[:3], 0, false,
Source{Type: "album", ID: 1, Label: "Abbey Road"},
)
if _, ok := rec.Wait(events.QueueChanged, waitFor); !ok {
t.Fatalf("no QueueChanged after SetQueue; got %v", rec.Names())
}
rec.Reset()
q.AddTrack(paths[3])
if got := modifiedOf(t, rec).Source; got != (Source{}) {
t.Errorf("delta source = %+v, want zero value", got)
}
}
// And a delta that did not clear it still reports the source it has,
// or the frontend would drop a perfectly good label on every removal.
func TestEmit_NonAppendDeltaCarriesSource(t *testing.T) {
t.Parallel()
q, db, rec := setupRecordedQueue(t)
paths := seedAudioFiles(t, db, 4)
album := Source{Type: "album", ID: 1, Label: "Abbey Road"}
q.SetQueue(paths, 0, false, album)
if _, ok := rec.Wait(events.QueueChanged, waitFor); !ok {
t.Fatalf("no QueueChanged after SetQueue; got %v", rec.Names())
}
rec.Reset()
q.RemoveTrack(3)
if got := modifiedOf(t, rec).Source; got != album {
t.Errorf("delta source = %+v, want %+v", got, album)
}
}
func TestEmit_RemoveTracksReportsPositions(t *testing.T) { func TestEmit_RemoveTracksReportsPositions(t *testing.T) {
t.Parallel() t.Parallel()
-44
View File
@@ -156,21 +156,12 @@ type PlaybackFailure struct {
} }
// TracksModified is the payload for the QueueTracksModified event. // TracksModified is the payload for the QueueTracksModified event.
//
// Source is carried because an append is exactly what can *invalidate*
// it: a queue built from one album stops being that album the moment a
// track from somewhere else is added to it. The delta is the only event
// those paths emit, so without this the frontend would keep the label
// it was last given and go on saying "Playing from" an album that is no
// longer what is queued — an event carrying what its consumer needs, so
// nothing has to invalidate anything.
type TracksModified struct { type TracksModified struct {
Action string `json:"action"` Action string `json:"action"`
Tracks []Track `json:"tracks,omitempty"` Tracks []Track `json:"tracks,omitempty"`
Index int `json:"index"` Index int `json:"index"`
Positions []int `json:"positions,omitempty"` Positions []int `json:"positions,omitempty"`
CurrentIndex int `json:"currentIndex"` CurrentIndex int `json:"currentIndex"`
Source Source `json:"source"`
} }
// Queue manages an ordered list of tracks for playback. // Queue manages an ordered list of tracks for playback.
@@ -464,8 +455,6 @@ func (q *Queue) AddTrack(filePath string) {
q.generateShuffleOrder() q.generateShuffleOrder()
} }
q.dropSource()
q.persistAddTrack(track) q.persistAddTrack(track)
q.persistState() q.persistState()
q.emitTracksModified( q.emitTracksModified(
@@ -516,8 +505,6 @@ func (q *Queue) AddTracks(filePaths []string) {
q.generateShuffleOrder() q.generateShuffleOrder()
} }
q.dropSource()
q.persistAddTracks(newTracks) q.persistAddTracks(newTracks)
q.persistState() q.persistState()
q.emitTracksModified( q.emitTracksModified(
@@ -576,8 +563,6 @@ func (q *Queue) InsertNextTracks(filePaths []string) {
q.generateShuffleOrder() q.generateShuffleOrder()
} }
q.dropSource()
q.persistInsertTracks(newTracks, insertPos) q.persistInsertTracks(newTracks, insertPos)
q.persistState() q.persistState()
q.emitTracksModified( q.emitTracksModified(
@@ -628,8 +613,6 @@ func (q *Queue) InsertNext(filePath string) {
q.generateShuffleOrder() q.generateShuffleOrder()
} }
q.dropSource()
q.persistInsertTracks([]Track{track}, insertPos) q.persistInsertTracks([]Track{track}, insertPos)
q.persistState() q.persistState()
q.emitTracksModified( q.emitTracksModified(
@@ -697,8 +680,6 @@ func (q *Queue) InsertTracksAt(filePaths []string, index int) {
q.generateShuffleOrder() q.generateShuffleOrder()
} }
q.dropSource()
q.persistInsertTracks(newTracks, index) q.persistInsertTracks(newTracks, index)
q.persistState() q.persistState()
q.emitTracksModified( q.emitTracksModified(
@@ -1556,31 +1537,6 @@ func (q *Queue) reindexPositions() {
} }
} }
// dropSource forgets which collection the queue was built from.
//
// A Source is a claim that everything queued came from one album,
// playlist, genre or artist, and the frontend renders it as a
// "Playing from X" link back to that page. Adding or inserting a track
// makes the claim false — the queue is now that album *plus* something
// else — so every path that does so calls this.
//
// It was set by SetQueue and cleared in exactly one place, Clear, so a
// label survived every append. It is persisted too (source_type /
// source_id / source_label on the queue state row), which is what made
// a wrong label outlive the session that earned it: an album queued on
// Monday, added to on Tuesday, still offered a link back to that album
// on Friday.
//
// Removing, reordering and shuffling deliberately do not call this. A
// queue with a track taken out of it, or played in another order, is
// still that album — the link still goes somewhere true. Only the
// arrival of a track from elsewhere makes it a lie.
//
// The caller must hold q.mu.
func (q *Queue) dropSource() {
q.source = Source{}
}
// commitMutation persists the current queue state after a mutation. // commitMutation persists the current queue state after a mutation.
// When reindex is true, track positions are renumbered first. // When reindex is true, track positions are renumbered first.
// The caller must hold q.mu. // The caller must hold q.mu.
-111
View File
@@ -126,117 +126,6 @@ func TestClear_ResetsSource(t *testing.T) {
} }
} }
// A queue built from one album stops being that album the moment a
// track from somewhere else joins it, so every path that adds one
// drops the source. Before this, SetQueue was the only writer and
// Clear the only clearer, so "Playing from Abbey Road" outlived every
// append — and, being persisted, every restart too.
func TestAppendPathsDropSource(t *testing.T) {
t.Parallel()
album := Source{Type: "album", ID: 1, Label: "Abbey Road"}
tests := []struct {
name string
append func(q *Queue, paths []string)
}{
{
name: "AddTrack",
append: func(q *Queue, paths []string) {
q.AddTrack(paths[5])
},
},
{
name: "AddTracks",
append: func(q *Queue, paths []string) {
q.AddTracks(paths[5:7])
},
},
{
name: "InsertNext",
append: func(q *Queue, paths []string) {
q.InsertNext(paths[5])
},
},
{
name: "InsertNextTracks",
append: func(q *Queue, paths []string) {
q.InsertNextTracks(paths[5:7])
},
},
{
name: "InsertTracksAt",
append: func(q *Queue, paths []string) {
q.InsertTracksAt(paths[5:7], 1)
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
q, db := setupTestQueue(t)
paths := seedAudioFiles(t, db, 8)
q.SetQueue(paths[:5], 0, false, album)
if got := q.GetState().Source; got != album {
t.Fatalf("source before append: got %+v, want %+v", got, album)
}
tt.append(q, paths)
if got := q.GetState().Source; got != (Source{}) {
t.Errorf(
"source after %s: got %+v, want zero value",
tt.name, got,
)
}
})
}
}
// Removing and reordering deliberately do not drop it: a queue with a
// track taken out of it is still that album, and the link still goes
// somewhere true.
func TestRemoveAndMoveKeepSource(t *testing.T) {
t.Parallel()
album := Source{Type: "album", ID: 1, Label: "Abbey Road"}
t.Run("RemoveTrack", func(t *testing.T) {
t.Parallel()
q, db := setupTestQueue(t)
paths := seedAudioFiles(t, db, 5)
q.SetQueue(paths, 0, false, album)
q.RemoveTrack(3)
if got := q.GetState().Source; got != album {
t.Errorf("source after RemoveTrack: got %+v, want %+v", got, album)
}
})
t.Run("MoveQueueTracks", func(t *testing.T) {
t.Parallel()
q, db := setupTestQueue(t)
paths := seedAudioFiles(t, db, 5)
q.SetQueue(paths, 0, false, album)
q.MoveQueueTracks([]int{0}, 3)
if got := q.GetState().Source; got != album {
t.Errorf(
"source after MoveQueueTracks: got %+v, want %+v",
got, album,
)
}
})
}
func TestSetQueue_WithStartIndex(t *testing.T) { func TestSetQueue_WithStartIndex(t *testing.T) {
t.Parallel() t.Parallel()
+7 -76
View File
@@ -28,7 +28,6 @@ var (
errUnsupportedOp = errors.New("unsupported operator") errUnsupportedOp = errors.New("unsupported operator")
errInvalidSortField = errors.New("invalid sort field: not in allowed field list") errInvalidSortField = errors.New("invalid sort field: not in allowed field list")
errNotNumeric = errors.New("value must be numeric") errNotNumeric = errors.New("value must be numeric")
errInvalidMatch = errors.New("match must be \"all\" or \"any\"")
) )
// Rule represents a single filter condition for a smart playlist. // Rule represents a single filter condition for a smart playlist.
@@ -38,45 +37,13 @@ type Rule struct {
Value string `json:"value"` Value string `json:"value"`
} }
// MatchType decides how a rule set's conditions combine.
//
// The rules used to be joined with " AND " and nothing else, so a
// playlist could only ever narrow: "jazz released after 1960" was
// expressible and "jazz or blues" was not, which is most of what
// anyone reaches for a second rule to say.
type MatchType string
const (
// MatchAll requires every rule to hold — the historical behaviour,
// and what an empty match means so that every rule set written
// before this existed keeps the meaning it was saved with.
MatchAll MatchType = "all"
// MatchAny requires at least one rule to hold.
MatchAny MatchType = "any"
)
// joiner returns the SQL keyword that combines two conditions.
// An unrecognised value cannot reach here — ParseRuleSet rejects one
// — so the default is about the empty string, which is every rule set
// saved before this field existed.
func (m MatchType) joiner() string {
if m == MatchAny {
return " OR "
}
return " AND "
}
// RuleSet holds the complete filter configuration for a smart // RuleSet holds the complete filter configuration for a smart
// playlist, including optional sort and limit. // playlist, including optional sort and limit.
type RuleSet struct { type RuleSet struct {
Rules []Rule `json:"rules"` Rules []Rule `json:"rules"`
// Match is "all" or "any"; empty means "all". It is omitempty so Limit int `json:"limit,omitempty"`
// an untouched playlist's stored JSON does not change shape. SortField string `json:"sort_field,omitempty"`
Match MatchType `json:"match,omitempty"` SortDir string `json:"sort_dir,omitempty"`
Limit int `json:"limit,omitempty"`
SortField string `json:"sort_field,omitempty"`
SortDir string `json:"sort_dir,omitempty"`
} }
// fieldMap maps user-facing rule field names to track_metadata column // fieldMap maps user-facing rule field names to track_metadata column
@@ -149,12 +116,7 @@ const genreDelimiter = "||"
// slice of rules. It is a pure function — no database access needed. // slice of rules. It is a pure function — no database access needed.
// Returns the clause (without the leading "WHERE"), the parameter // Returns the clause (without the leading "WHERE"), the parameter
// args, and any validation error. // args, and any validation error.
// func BuildWhereClause(rules []Rule) (string, []any, error) {
// match decides how the conditions combine; an empty match is MatchAll,
// which is what every rule set saved before the field existed means.
func BuildWhereClause(
rules []Rule, match MatchType,
) (string, []any, error) {
if len(rules) == 0 { if len(rules) == 0 {
return "", nil, nil return "", nil, nil
} }
@@ -217,28 +179,7 @@ func BuildWhereClause(
args = append(args, condArgs...) args = append(args, condArgs...)
} }
// Under OR, each condition is parenthesised; under AND it is not. return strings.Join(conditions, " AND "), args, nil
//
// The asymmetry is deliberate rather than an omission. AND is the
// tighter operator in SQL, so an OR-join has to protect any
// condition that contains a top-level AND of its own or the halves
// come apart: `days_since_played less_than` is
// `last_played IS NOT NULL AND <expr> < ?`, which read without
// brackets under an OR-join happens to still parse correctly and
// would stop doing so the moment a condition grows a top-level OR.
// Bracketing under AND would be a no-op semantically and would
// rewrite the clause every existing test pins, so the brackets go
// exactly where they change something.
if match == MatchAny {
bracketed := make([]string, len(conditions))
for i, cond := range conditions {
bracketed[i] = "(" + cond + ")"
}
conditions = bracketed
}
return strings.Join(conditions, match.joiner()), args, nil
} }
// validateOperator checks that the operator is valid for the field // validateOperator checks that the operator is valid for the field
@@ -658,7 +599,7 @@ func Evaluate(
start := time.Now() start := time.Now()
logger := db.Logger() logger := db.Logger()
where, args, err := BuildWhereClause(ruleSet.Rules, ruleSet.Match) where, args, err := BuildWhereClause(ruleSet.Rules)
if err != nil { if err != nil {
return nil, fmt.Errorf( return nil, fmt.Errorf(
"smart playlist rule error: %w", err, "smart playlist rule error: %w", err,
@@ -1095,16 +1036,6 @@ func ParseRuleSet(jsonStr string) (RuleSet, error) {
) )
} }
// A match nobody recognises would otherwise fall through to AND,
// which is a playlist quietly returning the wrong tracks rather
// than refusing to be saved. This is the only place a rule set
// enters the backend, so it is the only place that has to ask.
if rs.Match != "" && rs.Match != MatchAll && rs.Match != MatchAny {
return RuleSet{}, fmt.Errorf(
"%w: %q", errInvalidMatch, rs.Match,
)
}
return rs, nil return rs, nil
} }
+24 -203
View File
@@ -1,7 +1,6 @@
package smartplaylist package smartplaylist
import ( import (
"errors"
"strings" "strings"
"testing" "testing"
@@ -171,7 +170,7 @@ func TestBuildWhereClause_TextIs(t *testing.T) {
clause, args, err := BuildWhereClause([]Rule{ clause, args, err := BuildWhereClause([]Rule{
{Field: "artist", Operator: "is", Value: "Queen"}, {Field: "artist", Operator: "is", Value: "Queen"},
}, MatchAll) })
if err != nil { if err != nil {
t.Fatalf("unexpected error: %v", err) t.Fatalf("unexpected error: %v", err)
} }
@@ -190,7 +189,7 @@ func TestBuildWhereClause_TextIsNot(t *testing.T) {
clause, args, err := BuildWhereClause([]Rule{ clause, args, err := BuildWhereClause([]Rule{
{Field: "artist", Operator: "is_not", Value: "Queen"}, {Field: "artist", Operator: "is_not", Value: "Queen"},
}, MatchAll) })
if err != nil { if err != nil {
t.Fatalf("unexpected error: %v", err) t.Fatalf("unexpected error: %v", err)
} }
@@ -210,7 +209,7 @@ func TestBuildWhereClause_TextContains(t *testing.T) {
clause, args, err := BuildWhereClause([]Rule{ clause, args, err := BuildWhereClause([]Rule{
{Field: "title", Operator: "contains", Value: "Black"}, {Field: "title", Operator: "contains", Value: "Black"},
}, MatchAll) })
if err != nil { if err != nil {
t.Fatalf("unexpected error: %v", err) t.Fatalf("unexpected error: %v", err)
} }
@@ -232,7 +231,7 @@ func TestBuildWhereClause_TextDoesNotContain(t *testing.T) {
Field: "title", Operator: "does_not_contain", Field: "title", Operator: "does_not_contain",
Value: "Black", Value: "Black",
}, },
}, MatchAll) })
if err != nil { if err != nil {
t.Fatalf("unexpected error: %v", err) t.Fatalf("unexpected error: %v", err)
} }
@@ -252,7 +251,7 @@ func TestBuildWhereClause_TextStartsWith(t *testing.T) {
clause, args, err := BuildWhereClause([]Rule{ clause, args, err := BuildWhereClause([]Rule{
{Field: "title", Operator: "starts_with", Value: "Back"}, {Field: "title", Operator: "starts_with", Value: "Back"},
}, MatchAll) })
if err != nil { if err != nil {
t.Fatalf("unexpected error: %v", err) t.Fatalf("unexpected error: %v", err)
} }
@@ -271,7 +270,7 @@ func TestBuildWhereClause_TextEndsWith(t *testing.T) {
clause, args, err := BuildWhereClause([]Rule{ clause, args, err := BuildWhereClause([]Rule{
{Field: "title", Operator: "ends_with", Value: "Black"}, {Field: "title", Operator: "ends_with", Value: "Black"},
}, MatchAll) })
if err != nil { if err != nil {
t.Fatalf("unexpected error: %v", err) t.Fatalf("unexpected error: %v", err)
} }
@@ -293,7 +292,7 @@ func TestBuildWhereClause_TextIsAnyOf(t *testing.T) {
Field: "artist", Operator: "is_any_of", Field: "artist", Operator: "is_any_of",
Value: `["Queen","AC/DC"]`, Value: `["Queen","AC/DC"]`,
}, },
}, MatchAll) })
if err != nil { if err != nil {
t.Fatalf("unexpected error: %v", err) t.Fatalf("unexpected error: %v", err)
} }
@@ -313,7 +312,7 @@ func TestBuildWhereClause_NumericIs(t *testing.T) {
clause, args, err := BuildWhereClause([]Rule{ clause, args, err := BuildWhereClause([]Rule{
{Field: "year", Operator: "is", Value: "1980"}, {Field: "year", Operator: "is", Value: "1980"},
}, MatchAll) })
if err != nil { if err != nil {
t.Fatalf("unexpected error: %v", err) t.Fatalf("unexpected error: %v", err)
} }
@@ -332,7 +331,7 @@ func TestBuildWhereClause_NumericIsNot(t *testing.T) {
clause, args, err := BuildWhereClause([]Rule{ clause, args, err := BuildWhereClause([]Rule{
{Field: "year", Operator: "is_not", Value: "1980"}, {Field: "year", Operator: "is_not", Value: "1980"},
}, MatchAll) })
if err != nil { if err != nil {
t.Fatalf("unexpected error: %v", err) t.Fatalf("unexpected error: %v", err)
} }
@@ -351,7 +350,7 @@ func TestBuildWhereClause_NumericGreaterThan(t *testing.T) {
clause, args, err := BuildWhereClause([]Rule{ clause, args, err := BuildWhereClause([]Rule{
{Field: "year", Operator: "greater_than", Value: "2000"}, {Field: "year", Operator: "greater_than", Value: "2000"},
}, MatchAll) })
if err != nil { if err != nil {
t.Fatalf("unexpected error: %v", err) t.Fatalf("unexpected error: %v", err)
} }
@@ -370,7 +369,7 @@ func TestBuildWhereClause_NumericLessThan(t *testing.T) {
clause, args, err := BuildWhereClause([]Rule{ clause, args, err := BuildWhereClause([]Rule{
{Field: "year", Operator: "less_than", Value: "1980"}, {Field: "year", Operator: "less_than", Value: "1980"},
}, MatchAll) })
if err != nil { if err != nil {
t.Fatalf("unexpected error: %v", err) t.Fatalf("unexpected error: %v", err)
} }
@@ -392,7 +391,7 @@ func TestBuildWhereClause_NumericBetween(t *testing.T) {
Field: "year", Operator: "between", Field: "year", Operator: "between",
Value: "1975,1985", Value: "1975,1985",
}, },
}, MatchAll) })
if err != nil { if err != nil {
t.Fatalf("unexpected error: %v", err) t.Fatalf("unexpected error: %v", err)
} }
@@ -415,7 +414,7 @@ func TestBuildWhereClause_NumericBetweenJSON(t *testing.T) {
Field: "year", Operator: "between", Field: "year", Operator: "between",
Value: `["1975","1985"]`, Value: `["1975","1985"]`,
}, },
}, MatchAll) })
if err != nil { if err != nil {
t.Fatalf("unexpected error: %v", err) t.Fatalf("unexpected error: %v", err)
} }
@@ -435,7 +434,7 @@ func TestBuildWhereClause_GenreIsProducesSubquery(t *testing.T) {
clause, args, err := BuildWhereClause([]Rule{ clause, args, err := BuildWhereClause([]Rule{
{Field: "genre", Operator: "is", Value: "Rock"}, {Field: "genre", Operator: "is", Value: "Rock"},
}, MatchAll) })
if err != nil { if err != nil {
t.Fatalf("unexpected error: %v", err) t.Fatalf("unexpected error: %v", err)
} }
@@ -467,7 +466,7 @@ func TestBuildWhereClause_GenreIsNotProducesSubquery(t *testing.T) {
clause, args, err := BuildWhereClause([]Rule{ clause, args, err := BuildWhereClause([]Rule{
{Field: "genre", Operator: "is_not", Value: "Rock"}, {Field: "genre", Operator: "is_not", Value: "Rock"},
}, MatchAll) })
if err != nil { if err != nil {
t.Fatalf("unexpected error: %v", err) t.Fatalf("unexpected error: %v", err)
} }
@@ -496,7 +495,7 @@ func TestBuildWhereClause_GenreIsAnyOfProducesSubquery(t *testing.T) {
Field: "genre", Operator: "is_any_of", Field: "genre", Operator: "is_any_of",
Value: `["Rock","Pop"]`, Value: `["Rock","Pop"]`,
}, },
}, MatchAll) })
if err != nil { if err != nil {
t.Fatalf("unexpected error: %v", err) t.Fatalf("unexpected error: %v", err)
} }
@@ -525,7 +524,7 @@ func TestBuildWhereClause_GenreContainsUsesSubquery(t *testing.T) {
clause, args, err := BuildWhereClause([]Rule{ clause, args, err := BuildWhereClause([]Rule{
{Field: "genre", Operator: "contains", Value: "Rock"}, {Field: "genre", Operator: "contains", Value: "Rock"},
}, MatchAll) })
if err != nil { if err != nil {
t.Fatalf("unexpected error: %v", err) t.Fatalf("unexpected error: %v", err)
} }
@@ -558,7 +557,7 @@ func TestBuildWhereClause_MultipleRulesAND(t *testing.T) {
clause, args, err := BuildWhereClause([]Rule{ clause, args, err := BuildWhereClause([]Rule{
{Field: "artist", Operator: "is", Value: "Queen"}, {Field: "artist", Operator: "is", Value: "Queen"},
{Field: "year", Operator: "greater_than", Value: "1975"}, {Field: "year", Operator: "greater_than", Value: "1975"},
}, MatchAll) })
if err != nil { if err != nil {
t.Fatalf("unexpected error: %v", err) t.Fatalf("unexpected error: %v", err)
} }
@@ -573,110 +572,6 @@ func TestBuildWhereClause_MultipleRulesAND(t *testing.T) {
} }
} }
func TestBuildWhereClause_MultipleRulesOR(t *testing.T) {
t.Parallel()
clause, args, err := BuildWhereClause([]Rule{
{Field: "artist", Operator: "is", Value: "Queen"},
{Field: "year", Operator: "greater_than", Value: "1975"},
}, MatchAny)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
want := "(artist_name = ? COLLATE NOCASE) OR (year > ?)"
if clause != want {
t.Errorf("clause = %q, want %q", clause, want)
}
if len(args) != 2 || args[0] != "Queen" || args[1] != int64(1975) {
t.Errorf("args = %v, want [Queen 1975]", args)
}
}
// An empty match is what every rule set saved before the field existed
// carries, and it has to keep meaning AND — a playlist silently
// widening to OR on upgrade is the whole risk of adding this field.
func TestBuildWhereClause_EmptyMatchIsAll(t *testing.T) {
t.Parallel()
rules := []Rule{
{Field: "artist", Operator: "is", Value: "Queen"},
{Field: "year", Operator: "greater_than", Value: "1975"},
}
empty, _, err := BuildWhereClause(rules, "")
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
all, _, err := BuildWhereClause(rules, MatchAll)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if empty != all {
t.Errorf("empty match = %q, want the same as MatchAll %q",
empty, all)
}
}
// A condition carrying its own top-level AND is what makes the
// bracketing under OR load-bearing: `days_since_played less_than`
// is two predicates, and both belong to the same rule.
func TestBuildWhereClause_ORBracketsCompoundCondition(t *testing.T) {
t.Parallel()
clause, _, err := BuildWhereClause([]Rule{
{Field: "artist", Operator: "is", Value: "Queen"},
{
Field: "days_since_played",
Operator: "less_than",
Value: "30",
},
}, MatchAny)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if !strings.Contains(clause, "(last_played IS NOT NULL AND") {
t.Errorf(
"compound condition is not bracketed under OR: %q",
clause,
)
}
}
func TestParseRuleSet_RejectsUnknownMatch(t *testing.T) {
t.Parallel()
_, err := ParseRuleSet(`{"rules":[],"match":"either"}`)
if err == nil {
t.Fatal("expected an error for an unknown match type")
}
if !errors.Is(err, errInvalidMatch) {
t.Errorf("err = %v, want errInvalidMatch", err)
}
}
func TestParseRuleSet_AcceptsAnyAndAll(t *testing.T) {
t.Parallel()
for _, want := range []MatchType{MatchAll, MatchAny} {
rs, err := ParseRuleSet(
`{"rules":[],"match":"` + string(want) + `"}`,
)
if err != nil {
t.Fatalf("match %q: unexpected error: %v", want, err)
}
if rs.Match != want {
t.Errorf("match = %q, want %q", rs.Match, want)
}
}
}
func TestBuildWhereClause_SameFieldMultipleTimes(t *testing.T) { func TestBuildWhereClause_SameFieldMultipleTimes(t *testing.T) {
t.Parallel() t.Parallel()
@@ -686,7 +581,7 @@ func TestBuildWhereClause_SameFieldMultipleTimes(t *testing.T) {
Field: "genre", Operator: "does_not_contain", Field: "genre", Operator: "does_not_contain",
Value: "Punk", Value: "Punk",
}, },
}, MatchAll) })
if err != nil { if err != nil {
t.Fatalf("unexpected error: %v", err) t.Fatalf("unexpected error: %v", err)
} }
@@ -714,7 +609,7 @@ func TestBuildWhereClause_SameFieldMultipleTimes(t *testing.T) {
func TestBuildWhereClause_EmptyRules(t *testing.T) { func TestBuildWhereClause_EmptyRules(t *testing.T) {
t.Parallel() t.Parallel()
clause, args, err := BuildWhereClause(nil, MatchAll) clause, args, err := BuildWhereClause(nil)
if err != nil { if err != nil {
t.Fatalf("unexpected error: %v", err) t.Fatalf("unexpected error: %v", err)
} }
@@ -736,7 +631,7 @@ func TestBuildWhereClause_InvalidField(t *testing.T) {
Field: "nonexistent", Operator: "is", Field: "nonexistent", Operator: "is",
Value: "anything", Value: "anything",
}, },
}, MatchAll) })
if err == nil { if err == nil {
t.Fatal("expected error for invalid field, got nil") t.Fatal("expected error for invalid field, got nil")
} }
@@ -759,7 +654,7 @@ func TestBuildWhereClause_InvalidOperatorForNumeric(t *testing.T) {
_, _, err := BuildWhereClause([]Rule{ _, _, err := BuildWhereClause([]Rule{
{Field: "year", Operator: "contains", Value: "1980"}, {Field: "year", Operator: "contains", Value: "1980"},
}, MatchAll) })
if err == nil { if err == nil {
t.Fatal( t.Fatal(
"expected error for text operator on numeric field", "expected error for text operator on numeric field",
@@ -781,7 +676,7 @@ func TestBuildWhereClause_InvalidOperatorForText(t *testing.T) {
Field: "artist", Operator: "greater_than", Field: "artist", Operator: "greater_than",
Value: "Queen", Value: "Queen",
}, },
}, MatchAll) })
if err == nil { if err == nil {
t.Fatal( t.Fatal(
"expected error for numeric operator on text field", "expected error for numeric operator on text field",
@@ -828,80 +723,6 @@ func TestEvaluate_TextIs(t *testing.T) {
} }
} }
// Two rules that share no track at all: under AND this is empty, and
// under OR it is the union. Before Match existed only the first was
// expressible, so a playlist could only ever narrow — "jazz or blues"
// had no way to be said.
func TestEvaluate_MatchAnyUnionsWhereMatchAllIntersects(t *testing.T) {
t.Parallel()
db := database.NewTestDB(t)
seedSmartPlaylistData(t, db)
// Queen has two tracks; Beyoncé has one; no track is by both.
rules := []Rule{
{Field: "artist", Operator: "is", Value: "Queen"},
{Field: "artist", Operator: "is", Value: "Beyoncé"},
}
all, err := Evaluate(db, RuleSet{Rules: rules, Match: MatchAll})
if err != nil {
t.Fatalf("Evaluate(all): %v", err)
}
if len(all) != 0 {
t.Errorf("match=all returned %d tracks, want 0", len(all))
}
either, err := Evaluate(db, RuleSet{Rules: rules, Match: MatchAny})
if err != nil {
t.Fatalf("Evaluate(any): %v", err)
}
if len(either) != 3 {
t.Fatalf("match=any returned %d tracks, want 3", len(either))
}
for _, tr := range either {
if tr.ArtistName != "Queen" && tr.ArtistName != "Beyoncé" {
t.Errorf(
"track %q has artist %q, want Queen or Beyoncé",
tr.TrackName, tr.ArtistName,
)
}
}
}
// An empty match is what every playlist saved before the field existed
// carries, and it has to keep meaning AND all the way through Evaluate
// — a stored playlist silently widening on upgrade is the only real
// risk in adding this.
func TestEvaluate_EmptyMatchStillIntersects(t *testing.T) {
t.Parallel()
db := database.NewTestDB(t)
seedSmartPlaylistData(t, db)
tracks, err := Evaluate(db, RuleSet{
Rules: []Rule{
{Field: "artist", Operator: "is", Value: "Queen"},
{Field: "year", Operator: "greater_than", Value: "1979"},
},
})
if err != nil {
t.Fatalf("Evaluate: %v", err)
}
// Only "Another One Bites the Dust" (Queen, 1980) satisfies both.
if len(tracks) != 1 {
t.Fatalf("got %d tracks, want 1", len(tracks))
}
if want := "Another One Bites the Dust"; tracks[0].TrackName != want {
t.Errorf("got %q, want %q", tracks[0].TrackName, want)
}
}
// TestEvaluate_ArtworkEnrichment verifies the presentation-only // TestEvaluate_ArtworkEnrichment verifies the presentation-only
// cover-art and MusicBrainz-ID fields are attached to matched tracks // cover-art and MusicBrainz-ID fields are attached to matched tracks
// by the batched fetchArtwork pass (they are no longer part of the // by the batched fetchArtwork pass (they are no longer part of the
@@ -1519,7 +1340,7 @@ func TestSQLInjection_FieldName(t *testing.T) {
Field: "title; DROP TABLE playlists", Field: "title; DROP TABLE playlists",
Operator: "is", Value: "x", Operator: "is", Value: "x",
}, },
}, MatchAll) })
if err == nil { if err == nil {
t.Fatal( t.Fatal(
"expected error for injected field name, got nil", "expected error for injected field name, got nil",
+66
View File
@@ -0,0 +1,66 @@
import { test, expect } from '../support/fixtures.js';
/**
* The queue button says whether the queue is open.
*
* It used to look identical in both states, so the only way to tell
* what pressing it would do was to look at the other side of the window
* and infer it — and for anyone not looking at all there was nothing to
* infer from: no `aria-expanded`, no `aria-controls`, no pressed state.
*
* The state is reflected *from the panel*, not kept beside the click,
* because the button is not the only thing that opens the queue —
* `now-playing-view` sets the same attribute, since it hides the bar
* this button lives in. A flag maintained by the click handler would be
* right until something else opened the panel and then quietly wrong,
* which is the second test here.
*/
test.describe('the queue toggle', () => {
test('reports open and closed, and names what it controls', async ({
app,
}) => {
const toggle = app.locator('#queue-button');
await expect(toggle).toHaveAttribute('aria-controls', 'queue-panel');
await expect(toggle).toHaveAttribute('aria-expanded', 'false');
await toggle.click();
await expect(toggle).toHaveAttribute('aria-expanded', 'true');
// The state is not only in the accessibility tree: a control that
// announces a state it does not draw is half a fix.
//
// Background rather than colour, because the pointer is still on
// the button after the click and `:hover` paints it the same accent
// the open state does -- so a colour comparison here passes on the
// broken build and proves nothing.
const [open, closed] = await toggle.evaluate((el) => {
const now = getComputedStyle(el).backgroundColor;
el.setAttribute('aria-expanded', 'false');
const shut = getComputedStyle(el).backgroundColor;
el.setAttribute('aria-expanded', 'true');
return [now, shut];
});
expect(open).not.toBe(closed);
await toggle.click();
await expect(toggle).toHaveAttribute('aria-expanded', 'false');
});
test('follows the panel when something else opens it', async ({ app }) => {
const toggle = app.locator('#queue-button');
await expect(toggle).toHaveAttribute('aria-expanded', 'false');
// Exactly what `now-playing-view`'s queue button does.
await app.evaluate(() =>
document.getElementById('queue-panel')?.setAttribute('open', ''),
);
await expect(toggle).toHaveAttribute('aria-expanded', 'true');
});
});
+11
View File
@@ -207,6 +207,17 @@ body div.sidebar {
color: var(--yj-accent, #ffd43b); color: var(--yj-accent, #ffd43b);
} }
/* An open queue is a state this button can be in, and it used to
look exactly like the closed one -- so the only way to tell what
pressing it would do was to look at the other side of the window
and infer it. `aria-expanded` is the same fact for anyone not
looking at all, and it points at the panel it controls. */
#queue-button[aria-expanded='true'] {
color: var(--yj-accent, #ffd43b);
background: var(--yj-bg-overlay, #404040);
border-radius: 4px;
}
#queue-button.drag-over { #queue-button.drag-over {
color: var(--yj-accent, #ffd43b); color: var(--yj-accent, #ffd43b);
outline: 2px dashed var(--yj-accent, #ffd43b); outline: 2px dashed var(--yj-accent, #ffd43b);
+2 -1
View File
@@ -37,7 +37,8 @@
<footer class="bottom-bar"> <footer class="bottom-bar">
<now-playing></now-playing> <now-playing></now-playing>
<audio-player></audio-player> <audio-player></audio-player>
<button aria-label="Toggle queue" id="queue-button"> <button aria-label="Toggle queue" aria-controls="queue-panel" aria-expanded="false"
id="queue-button">
<wa-icon name="list"></wa-icon> <wa-icon name="list"></wa-icon>
</button> </button>
</footer> </footer>
+22
View File
@@ -521,6 +521,28 @@ if (queueButton && queuePanel) {
} }
}); });
// The button says whether the panel is open, and it learns that
// from the panel rather than from its own click handler.
//
// It is not the only thing that opens the queue -- `now-playing-view`
// sets the same attribute, because it hides the bar this button
// lives in -- so a state kept beside the click would be right until
// something else opened the panel and then quietly wrong. The panel's
// `open` attribute is the one fact; this reflects it.
const reflectQueueState = () => {
queueButton.setAttribute(
'aria-expanded',
String(queuePanel.hasAttribute('open')),
);
};
new MutationObserver(reflectQueueState).observe(queuePanel, {
attributes: true,
attributeFilter: ['open'],
});
reflectQueueState();
// --------------------------------------------------------------- // ---------------------------------------------------------------
// Queue button as drop target (when queue panel is closed) // Queue button as drop target (when queue panel is closed)
// --------------------------------------------------------------- // ---------------------------------------------------------------
@@ -720,28 +720,14 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
// The download button only appears once a client is connected, // The download button only appears once a client is connected,
// so this tracks the provider list rather than assuming. // so this tracks the provider list rather than assuming.
//
// The `requestUpdate` is what makes the *tracklist's* badges
// move. Both assignments below are reactive fields, so Lit
// repaints when either changes — but a track request changes
// neither: `canDownload` is about providers and `isRequested`
// is about this album's own release group. Each row's badge
// reads `libraryStatusFor(false, track.mbid)` at render time,
// which is a dependency on the store that Lit cannot see, so
// clicking one filed the request and left the plus exactly
// where it was. The other three hosts rendering these badges
// (`explore-artist-details`, `explore-view`, `top-results-row`)
// have always asked for the repaint here; this one did not.
this.downloadUnsub = downloadStore.subscribe(() => { this.downloadUnsub = downloadStore.subscribe(() => {
this.canDownload = downloadStore.available; this.canDownload = downloadStore.available;
this.syncRequested(); this.syncRequested();
this.requestUpdate();
}); });
void downloadStore.init().then(() => { void downloadStore.init().then(() => {
this.canDownload = downloadStore.available; this.canDownload = downloadStore.available;
this.syncRequested(); this.syncRequested();
this.requestUpdate();
}); });
void this.resolveTargetLibraryId(); void this.resolveTargetLibraryId();
@@ -193,10 +193,6 @@ export class SmartPlaylistEditor extends LitElement {
// ── Internal state ────────────────────────────────────────────── // ── Internal state ──────────────────────────────────────────────
@state() private ruleRows: RuleRow[] = [emptyRule()]; @state() private ruleRows: RuleRow[] = [emptyRule()];
/** Whether every rule must hold or any one of them. Mirrors the
* backend's `match`; 'all' is the default and the only thing a
* playlist saved before this existed can have meant. */
@state() private matchType: 'all' | 'any' = 'all';
@state() private limit = 0; @state() private limit = 0;
@state() private sortField = 'random'; @state() private sortField = 'random';
@state() private sortDir = ''; @state() private sortDir = '';
@@ -228,19 +224,6 @@ export class SmartPlaylistEditor extends LitElement {
flex-shrink: 0; flex-shrink: 0;
} }
.match-row {
display: flex;
align-items: center;
gap: 6px;
font-size: var(--yj-text-sm);
color: var(--yj-text-secondary, #b3b3b3);
flex-wrap: wrap;
}
.match-select {
min-width: 72px;
}
.rule-row { .rule-row {
display: grid; display: grid;
grid-template-columns: 160px 140px 1fr 28px; grid-template-columns: 160px 140px 1fr 28px;
@@ -528,10 +511,6 @@ export class SmartPlaylistEditor extends LitElement {
); );
this.ruleRows = rows.length > 0 ? rows : [emptyRule()]; this.ruleRows = rows.length > 0 ? rows : [emptyRule()];
// A playlist saved before this field existed has no match
// and means "all" — the backend reads an empty match the
// same way, so an upgrade cannot widen anyone's playlist.
this.matchType = parsed.match === 'any' ? 'any' : 'all';
this.limit = parsed.limit ?? 0; this.limit = parsed.limit ?? 0;
this.sortField = parsed.sort_field || 'random'; this.sortField = parsed.sort_field || 'random';
this.sortDir = parsed.sort_dir ?? ''; this.sortDir = parsed.sort_dir ?? '';
@@ -572,7 +551,6 @@ export class SmartPlaylistEditor extends LitElement {
return JSON.stringify({ return JSON.stringify({
rules, rules,
match: this.matchType,
limit: this.limit || 0, limit: this.limit || 0,
sort_field: this.sortField || '', sort_field: this.sortField || '',
sort_dir: this.sortDir || '', sort_dir: this.sortDir || '',
@@ -652,11 +630,6 @@ export class SmartPlaylistEditor extends LitElement {
this.onRulesChanged(); this.onRulesChanged();
} }
private updateMatchType(value: string) {
this.matchType = value === 'any' ? 'any' : 'all';
this.onRulesChanged();
}
private updateSortField(value: string) { private updateSortField(value: string) {
this.sortField = value; this.sortField = value;
if (!value) this.sortDir = ''; if (!value) this.sortDir = '';
@@ -758,7 +731,6 @@ export class SmartPlaylistEditor extends LitElement {
override render() { override render() {
return html` return html`
<div class="rule-rows"> <div class="rule-rows">
${this.renderMatchType()}
${this.ruleRows.map((row, index) => ${this.ruleRows.map((row, index) =>
this.renderRuleRow(row, index), this.renderRuleRow(row, index),
)} )}
@@ -771,46 +743,6 @@ export class SmartPlaylistEditor extends LitElement {
`; `;
} }
/**
* Whether every rule has to hold, or any one of them.
*
* It is a sentence with a control in the middle rather than a
* labelled field, because the two readings differ by one word and
* that word is the whole of the setting — "Match **all** of the
* following rules" says what the list below it means in a way a
* select labelled "Match" beside a list does not.
*
* Hidden while there is one rule: with nothing to combine, all and
* any are the same query, and a control whose two settings cannot
* differ is a question the user has no way to answer wrongly and
* no reason to answer at all.
*/
private renderMatchType() {
if (this.ruleRows.length < 2) return nothing;
return html`
<div class="match-row">
<span>Match</span>
<select
class="match-select"
aria-label="Match all or any of the following rules"
@change=${(e: Event) =>
this.updateMatchType(
(e.target as HTMLSelectElement).value,
)}
>
<option value="all" ?selected=${this.matchType === 'all'}>
all
</option>
<option value="any" ?selected=${this.matchType === 'any'}>
any
</option>
</select>
<span>of the following rules</span>
</div>
`;
}
private renderRuleRow(row: RuleRow, index: number) { private renderRuleRow(row: RuleRow, index: number) {
const isBetween = row.operator === 'between'; const isBetween = row.operator === 'between';
const operators = row.field ? getOperatorsForField(row.field) : []; const operators = row.field ? getOperatorsForField(row.field) : [];
-7
View File
@@ -57,12 +57,6 @@ interface TracksModified {
index: number; index: number;
positions?: number[]; positions?: number[];
currentIndex: number; currentIndex: number;
/** The queue's source *after* the mutation. An append clears it
* backend-side — a queue built from one album is not that album
* once a track from elsewhere joins it — and this delta is the
* only event those paths emit, so the label would otherwise keep
* pointing at a collection the queue no longer holds. */
source?: QueueSource;
} }
type Subscriber = () => void; type Subscriber = () => void;
@@ -204,7 +198,6 @@ class QueueStore {
} }
this.state.currentIndex = delta.currentIndex; this.state.currentIndex = delta.currentIndex;
this.state.source = delta.source ?? EMPTY_QUEUE_SOURCE;
} }
// =================================================================== // ===================================================================
@@ -1,149 +0,0 @@
/**
* Issue #33: "Want track" filed the request and left the badge alone.
*
* The tracklist's badges read `libraryStatusFor(false, track.mbid)` at
* render time, which is a dependency on `downloadStore` that Lit cannot
* see. `explore-album-details` did subscribe to that store, but its
* callback only assigned `canDownload` and `isRequested` — neither of
* which a *track* request changes — so nothing in the component's
* reactive state moved and the page never re-rendered. The request was
* real, the plus stayed a plus, and clicking again cancelled it.
*
* The other three hosts rendering these badges (`explore-artist-
* details`, `explore-view`, `top-results-row`) have always asked for
* the repaint in the same place, which is what made this one look
* correct on inspection.
*/
import { describe, expect, it, beforeEach } from 'vitest';
import type { LitElement } from 'lit';
import '@components/explore-album-details/explore-album-details';
import type { Request } from '@store/download-store';
import { Events } from '../../src/events';
import { stub, flush, resetHarness, emit } from '@test/support/harness';
import { fixture, shadowAll } from '@test/support/render';
type RequestOverrides = Partial<Omit<Request, 'state' | 'entity'>> & {
state?: `${Request['state']}`;
entity?: `${Request['entity']}`;
};
function request(overrides: RequestOverrides): Request {
return {
id: 1,
mbid: 'mbid-1',
entity: 'recording',
libraryId: 1,
artist: 'An Artist',
title: 'Track 1',
scope: 'future',
secondary: false,
state: 'wanted',
attempts: 0,
...overrides,
} as Request;
}
/** Put a request list into the store the way the backend does. */
async function withRequests(rows: Request[]): Promise<void> {
stub('download.Service.ListRequests', rows);
emit(Events.RequestsChanged);
await flush();
}
function track(n: number) {
return {
position: n,
discNumber: 1,
title: `Track ${n}`,
length: 200000,
mbid: `mbid-${n}`,
inLibrary: false,
};
}
/** An unowned catalog tracklist, which is the only case with badges:
* an owned row renders none, there being nothing left to ask for. */
async function withTracklist(count: number): Promise<LitElement> {
const el = await fixture<LitElement>('explore-album-details', {
albumName: 'Glass Harbour',
});
Object.assign(el, {
versionEntries: [
{
key: 'v1',
label: '2019',
sublabel: `${count} tracks`,
tracks: Array.from({ length: count }, (_, i) => track(i + 1)),
},
],
selectedVersionKey: 'v1',
loadingReleases: false,
loadingInfo: false,
});
el.requestUpdate();
await flush();
await el.updateComplete;
return el;
}
/** The status of each track badge, in tracklist order. */
function badgeStatuses(el: LitElement): string[] {
return shadowAll(el, 'library-status-indicator.track-request').map(
(b) => b.getAttribute('status') ?? '',
);
}
describe('the album tracklists request badges', () => {
beforeEach(async () => {
resetHarness();
stub('library.Library.GetFilePathsByRecordingMBIDs', {});
stub('library.Library.GetFilePathsByAlbums', {});
stub('library.Library.GetAlbumTracks', []);
stub('library.Library.GetAllLibrariesWithTrackCounts', []);
await withRequests([]);
});
it('starts as a plus on every unowned row', async () => {
const el = await withTracklist(3);
expect(badgeStatuses(el)).toEqual([
'not-in-library',
'not-in-library',
'not-in-library',
]);
});
it('repaints the row whose track has been requested', async () => {
const el = await withTracklist(3);
await withRequests([request({ mbid: 'mbid-2' })]);
await el.updateComplete;
// Only the requested row moves. A request is by MBID, so the two
// rows either side of it are still a plus.
expect(badgeStatuses(el)).toEqual([
'not-in-library',
'queued',
'not-in-library',
]);
});
it('repaints again when the request is cancelled', async () => {
const el = await withTracklist(3);
await withRequests([request({ mbid: 'mbid-2' })]);
await el.updateComplete;
await withRequests([]);
await el.updateComplete;
expect(badgeStatuses(el)).toEqual([
'not-in-library',
'not-in-library',
'not-in-library',
]);
});
});
-55
View File
@@ -198,61 +198,6 @@ describe('queue store: move', () => {
}); });
}); });
/**
* "Playing from X" is a claim that everything queued came from X, and
* appending a track from anywhere else makes it false. The backend
* clears the source on every add/insert path — but the delta is the
* only event those paths emit, so the label corrects itself here or
* not at all.
*/
describe('queue store: the source travels on the delta', () => {
function syncWithAlbum(): void {
emit(Events.QueueChanged, {
tracks: [track(1), track(2)],
currentIndex: 0,
shuffleMode: false,
repeatMode: 'off',
source: { type: 'album', id: 7, label: 'Abbey Road' },
});
}
beforeEach(() => {
syncWithAlbum();
});
it('drops the label when an append clears it backend-side', () => {
emit(Events.QueueTracksModified, {
action: 'add',
tracks: [track(3)],
index: 2,
currentIndex: 0,
source: { type: '', id: 0, label: '' },
});
expect(queueStore.getState().source).toEqual({
type: '',
id: 0,
label: '',
});
});
it('keeps a label the backend still reports, as on a removal', () => {
emit(Events.QueueTracksModified, {
action: 'remove',
positions: [1],
index: 0,
currentIndex: 0,
source: { type: 'album', id: 7, label: 'Abbey Road' },
});
expect(queueStore.getState().source).toEqual({
type: 'album',
id: 7,
label: 'Abbey Road',
});
});
});
describe('queue store: mode deltas', () => { describe('queue store: mode deltas', () => {
beforeEach(() => { beforeEach(() => {
sync([track(1)], 0); sync([track(1)], 0);