Compare commits
5
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
63ec068add | ||
|
|
185eb1b125 | ||
|
|
b3556d825c | ||
|
|
bf4f352117 | ||
|
|
1062b7c0bc |
@@ -279,17 +279,19 @@ func (h *MPRISHandler) enqueue(fn func()) {
|
||||
}
|
||||
}
|
||||
|
||||
// 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 := map[string]interface{}{
|
||||
// metadataMap builds the org.mpris.MediaPlayer2.Player Metadata value
|
||||
// for one track.
|
||||
//
|
||||
// It is separated from UpdateMetadata, which needs a live D-Bus
|
||||
// connection, so the map's contents can be asserted on: this file is
|
||||
// behind a build tag and everything in it that touches h is reachable
|
||||
// only from a session bus, which is the same reason the Android
|
||||
// contract lives in an untagged androidpayload.go.
|
||||
func metadataMap(meta Metadata, trackID uint64) map[string]any {
|
||||
m := map[string]any{
|
||||
"mpris:trackid": dbus.ObjectPath(
|
||||
fmt.Sprintf(
|
||||
"/org/yellowjacket/Track/%d", tid,
|
||||
"/org/yellowjacket/Track/%d", trackID,
|
||||
),
|
||||
),
|
||||
}
|
||||
@@ -306,16 +308,45 @@ func (h *MPRISHandler) UpdateMetadata(meta Metadata) {
|
||||
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 != "" {
|
||||
m["mpris:artUrl"] = "file://" + meta.ArtFilePath
|
||||
artURL = "file://" + meta.ArtFilePath
|
||||
}
|
||||
|
||||
m["mpris:artUrl"] = artURL
|
||||
|
||||
if meta.DurationSec > 0 {
|
||||
m["mpris:length"] = int64(
|
||||
meta.DurationSec,
|
||||
) * 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.props.SetMust(playerIf, "Metadata", m)
|
||||
})
|
||||
|
||||
@@ -0,0 +1,86 @@
|
||||
//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)
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -81,6 +81,7 @@ func (q *Queue) emitTracksModified(
|
||||
Index: index,
|
||||
Positions: positions,
|
||||
CurrentIndex: q.currentIndex,
|
||||
Source: q.source,
|
||||
},
|
||||
)
|
||||
}
|
||||
|
||||
@@ -219,6 +219,56 @@ 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) {
|
||||
t.Parallel()
|
||||
|
||||
|
||||
@@ -156,12 +156,21 @@ type PlaybackFailure struct {
|
||||
}
|
||||
|
||||
// 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 {
|
||||
Action string `json:"action"`
|
||||
Tracks []Track `json:"tracks,omitempty"`
|
||||
Index int `json:"index"`
|
||||
Positions []int `json:"positions,omitempty"`
|
||||
CurrentIndex int `json:"currentIndex"`
|
||||
Source Source `json:"source"`
|
||||
}
|
||||
|
||||
// Queue manages an ordered list of tracks for playback.
|
||||
@@ -455,6 +464,8 @@ func (q *Queue) AddTrack(filePath string) {
|
||||
q.generateShuffleOrder()
|
||||
}
|
||||
|
||||
q.dropSource()
|
||||
|
||||
q.persistAddTrack(track)
|
||||
q.persistState()
|
||||
q.emitTracksModified(
|
||||
@@ -505,6 +516,8 @@ func (q *Queue) AddTracks(filePaths []string) {
|
||||
q.generateShuffleOrder()
|
||||
}
|
||||
|
||||
q.dropSource()
|
||||
|
||||
q.persistAddTracks(newTracks)
|
||||
q.persistState()
|
||||
q.emitTracksModified(
|
||||
@@ -563,6 +576,8 @@ func (q *Queue) InsertNextTracks(filePaths []string) {
|
||||
q.generateShuffleOrder()
|
||||
}
|
||||
|
||||
q.dropSource()
|
||||
|
||||
q.persistInsertTracks(newTracks, insertPos)
|
||||
q.persistState()
|
||||
q.emitTracksModified(
|
||||
@@ -613,6 +628,8 @@ func (q *Queue) InsertNext(filePath string) {
|
||||
q.generateShuffleOrder()
|
||||
}
|
||||
|
||||
q.dropSource()
|
||||
|
||||
q.persistInsertTracks([]Track{track}, insertPos)
|
||||
q.persistState()
|
||||
q.emitTracksModified(
|
||||
@@ -680,6 +697,8 @@ func (q *Queue) InsertTracksAt(filePaths []string, index int) {
|
||||
q.generateShuffleOrder()
|
||||
}
|
||||
|
||||
q.dropSource()
|
||||
|
||||
q.persistInsertTracks(newTracks, index)
|
||||
q.persistState()
|
||||
q.emitTracksModified(
|
||||
@@ -1537,6 +1556,31 @@ 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.
|
||||
// When reindex is true, track positions are renumbered first.
|
||||
// The caller must hold q.mu.
|
||||
|
||||
@@ -126,6 +126,117 @@ 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) {
|
||||
t.Parallel()
|
||||
|
||||
|
||||
@@ -28,6 +28,7 @@ var (
|
||||
errUnsupportedOp = errors.New("unsupported operator")
|
||||
errInvalidSortField = errors.New("invalid sort field: not in allowed field list")
|
||||
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.
|
||||
@@ -37,13 +38,45 @@ type Rule struct {
|
||||
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
|
||||
// playlist, including optional sort and limit.
|
||||
type RuleSet struct {
|
||||
Rules []Rule `json:"rules"`
|
||||
Limit int `json:"limit,omitempty"`
|
||||
SortField string `json:"sort_field,omitempty"`
|
||||
SortDir string `json:"sort_dir,omitempty"`
|
||||
Rules []Rule `json:"rules"`
|
||||
// Match is "all" or "any"; empty means "all". It is omitempty so
|
||||
// an untouched playlist's stored JSON does not change shape.
|
||||
Match MatchType `json:"match,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
|
||||
@@ -116,7 +149,12 @@ const genreDelimiter = "||"
|
||||
// slice of rules. It is a pure function — no database access needed.
|
||||
// Returns the clause (without the leading "WHERE"), the parameter
|
||||
// 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 {
|
||||
return "", nil, nil
|
||||
}
|
||||
@@ -179,7 +217,28 @@ func BuildWhereClause(rules []Rule) (string, []any, error) {
|
||||
args = append(args, condArgs...)
|
||||
}
|
||||
|
||||
return strings.Join(conditions, " AND "), args, nil
|
||||
// Under OR, each condition is parenthesised; under AND it is not.
|
||||
//
|
||||
// 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
|
||||
@@ -599,7 +658,7 @@ func Evaluate(
|
||||
start := time.Now()
|
||||
logger := db.Logger()
|
||||
|
||||
where, args, err := BuildWhereClause(ruleSet.Rules)
|
||||
where, args, err := BuildWhereClause(ruleSet.Rules, ruleSet.Match)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf(
|
||||
"smart playlist rule error: %w", err,
|
||||
@@ -1036,6 +1095,16 @@ 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
|
||||
}
|
||||
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
package smartplaylist
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
@@ -170,7 +171,7 @@ func TestBuildWhereClause_TextIs(t *testing.T) {
|
||||
|
||||
clause, args, err := BuildWhereClause([]Rule{
|
||||
{Field: "artist", Operator: "is", Value: "Queen"},
|
||||
})
|
||||
}, MatchAll)
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
@@ -189,7 +190,7 @@ func TestBuildWhereClause_TextIsNot(t *testing.T) {
|
||||
|
||||
clause, args, err := BuildWhereClause([]Rule{
|
||||
{Field: "artist", Operator: "is_not", Value: "Queen"},
|
||||
})
|
||||
}, MatchAll)
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
@@ -209,7 +210,7 @@ func TestBuildWhereClause_TextContains(t *testing.T) {
|
||||
|
||||
clause, args, err := BuildWhereClause([]Rule{
|
||||
{Field: "title", Operator: "contains", Value: "Black"},
|
||||
})
|
||||
}, MatchAll)
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
@@ -231,7 +232,7 @@ func TestBuildWhereClause_TextDoesNotContain(t *testing.T) {
|
||||
Field: "title", Operator: "does_not_contain",
|
||||
Value: "Black",
|
||||
},
|
||||
})
|
||||
}, MatchAll)
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
@@ -251,7 +252,7 @@ func TestBuildWhereClause_TextStartsWith(t *testing.T) {
|
||||
|
||||
clause, args, err := BuildWhereClause([]Rule{
|
||||
{Field: "title", Operator: "starts_with", Value: "Back"},
|
||||
})
|
||||
}, MatchAll)
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
@@ -270,7 +271,7 @@ func TestBuildWhereClause_TextEndsWith(t *testing.T) {
|
||||
|
||||
clause, args, err := BuildWhereClause([]Rule{
|
||||
{Field: "title", Operator: "ends_with", Value: "Black"},
|
||||
})
|
||||
}, MatchAll)
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
@@ -292,7 +293,7 @@ func TestBuildWhereClause_TextIsAnyOf(t *testing.T) {
|
||||
Field: "artist", Operator: "is_any_of",
|
||||
Value: `["Queen","AC/DC"]`,
|
||||
},
|
||||
})
|
||||
}, MatchAll)
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
@@ -312,7 +313,7 @@ func TestBuildWhereClause_NumericIs(t *testing.T) {
|
||||
|
||||
clause, args, err := BuildWhereClause([]Rule{
|
||||
{Field: "year", Operator: "is", Value: "1980"},
|
||||
})
|
||||
}, MatchAll)
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
@@ -331,7 +332,7 @@ func TestBuildWhereClause_NumericIsNot(t *testing.T) {
|
||||
|
||||
clause, args, err := BuildWhereClause([]Rule{
|
||||
{Field: "year", Operator: "is_not", Value: "1980"},
|
||||
})
|
||||
}, MatchAll)
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
@@ -350,7 +351,7 @@ func TestBuildWhereClause_NumericGreaterThan(t *testing.T) {
|
||||
|
||||
clause, args, err := BuildWhereClause([]Rule{
|
||||
{Field: "year", Operator: "greater_than", Value: "2000"},
|
||||
})
|
||||
}, MatchAll)
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
@@ -369,7 +370,7 @@ func TestBuildWhereClause_NumericLessThan(t *testing.T) {
|
||||
|
||||
clause, args, err := BuildWhereClause([]Rule{
|
||||
{Field: "year", Operator: "less_than", Value: "1980"},
|
||||
})
|
||||
}, MatchAll)
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
@@ -391,7 +392,7 @@ func TestBuildWhereClause_NumericBetween(t *testing.T) {
|
||||
Field: "year", Operator: "between",
|
||||
Value: "1975,1985",
|
||||
},
|
||||
})
|
||||
}, MatchAll)
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
@@ -414,7 +415,7 @@ func TestBuildWhereClause_NumericBetweenJSON(t *testing.T) {
|
||||
Field: "year", Operator: "between",
|
||||
Value: `["1975","1985"]`,
|
||||
},
|
||||
})
|
||||
}, MatchAll)
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
@@ -434,7 +435,7 @@ func TestBuildWhereClause_GenreIsProducesSubquery(t *testing.T) {
|
||||
|
||||
clause, args, err := BuildWhereClause([]Rule{
|
||||
{Field: "genre", Operator: "is", Value: "Rock"},
|
||||
})
|
||||
}, MatchAll)
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
@@ -466,7 +467,7 @@ func TestBuildWhereClause_GenreIsNotProducesSubquery(t *testing.T) {
|
||||
|
||||
clause, args, err := BuildWhereClause([]Rule{
|
||||
{Field: "genre", Operator: "is_not", Value: "Rock"},
|
||||
})
|
||||
}, MatchAll)
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
@@ -495,7 +496,7 @@ func TestBuildWhereClause_GenreIsAnyOfProducesSubquery(t *testing.T) {
|
||||
Field: "genre", Operator: "is_any_of",
|
||||
Value: `["Rock","Pop"]`,
|
||||
},
|
||||
})
|
||||
}, MatchAll)
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
@@ -524,7 +525,7 @@ func TestBuildWhereClause_GenreContainsUsesSubquery(t *testing.T) {
|
||||
|
||||
clause, args, err := BuildWhereClause([]Rule{
|
||||
{Field: "genre", Operator: "contains", Value: "Rock"},
|
||||
})
|
||||
}, MatchAll)
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
@@ -557,7 +558,7 @@ func TestBuildWhereClause_MultipleRulesAND(t *testing.T) {
|
||||
clause, args, err := BuildWhereClause([]Rule{
|
||||
{Field: "artist", Operator: "is", Value: "Queen"},
|
||||
{Field: "year", Operator: "greater_than", Value: "1975"},
|
||||
})
|
||||
}, MatchAll)
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
@@ -572,6 +573,110 @@ 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) {
|
||||
t.Parallel()
|
||||
|
||||
@@ -581,7 +686,7 @@ func TestBuildWhereClause_SameFieldMultipleTimes(t *testing.T) {
|
||||
Field: "genre", Operator: "does_not_contain",
|
||||
Value: "Punk",
|
||||
},
|
||||
})
|
||||
}, MatchAll)
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
@@ -609,7 +714,7 @@ func TestBuildWhereClause_SameFieldMultipleTimes(t *testing.T) {
|
||||
func TestBuildWhereClause_EmptyRules(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
clause, args, err := BuildWhereClause(nil)
|
||||
clause, args, err := BuildWhereClause(nil, MatchAll)
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
@@ -631,7 +736,7 @@ func TestBuildWhereClause_InvalidField(t *testing.T) {
|
||||
Field: "nonexistent", Operator: "is",
|
||||
Value: "anything",
|
||||
},
|
||||
})
|
||||
}, MatchAll)
|
||||
if err == nil {
|
||||
t.Fatal("expected error for invalid field, got nil")
|
||||
}
|
||||
@@ -654,7 +759,7 @@ func TestBuildWhereClause_InvalidOperatorForNumeric(t *testing.T) {
|
||||
|
||||
_, _, err := BuildWhereClause([]Rule{
|
||||
{Field: "year", Operator: "contains", Value: "1980"},
|
||||
})
|
||||
}, MatchAll)
|
||||
if err == nil {
|
||||
t.Fatal(
|
||||
"expected error for text operator on numeric field",
|
||||
@@ -676,7 +781,7 @@ func TestBuildWhereClause_InvalidOperatorForText(t *testing.T) {
|
||||
Field: "artist", Operator: "greater_than",
|
||||
Value: "Queen",
|
||||
},
|
||||
})
|
||||
}, MatchAll)
|
||||
if err == nil {
|
||||
t.Fatal(
|
||||
"expected error for numeric operator on text field",
|
||||
@@ -723,6 +828,80 @@ 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
|
||||
// cover-art and MusicBrainz-ID fields are attached to matched tracks
|
||||
// by the batched fetchArtwork pass (they are no longer part of the
|
||||
@@ -1340,7 +1519,7 @@ func TestSQLInjection_FieldName(t *testing.T) {
|
||||
Field: "title; DROP TABLE playlists",
|
||||
Operator: "is", Value: "x",
|
||||
},
|
||||
})
|
||||
}, MatchAll)
|
||||
if err == nil {
|
||||
t.Fatal(
|
||||
"expected error for injected field name, got nil",
|
||||
|
||||
@@ -111,32 +111,13 @@ const gridStyles = css`
|
||||
scale: 0.95;
|
||||
}
|
||||
|
||||
/* Title and year on one line, and only the title truncates.
|
||||
|
||||
The year used to be part of the same run of text, so it was the
|
||||
first thing an ellipsis ate: a card wide enough for a long album
|
||||
name never showed its year, and browsing by year showed years
|
||||
only for the albums with short names -- the sort said one thing
|
||||
and the cards showed another.
|
||||
|
||||
A flex row rather than a second line, because the card's height
|
||||
is what the virtualizer measures rows by. */
|
||||
.album-name {
|
||||
font-size: var(--album-name-font, 14px);
|
||||
font-weight: 400;
|
||||
color: var(--yj-text-primary, #fff);
|
||||
display: flex;
|
||||
justify-content: center;
|
||||
align-items: baseline;
|
||||
gap: 0.35em;
|
||||
min-width: 0;
|
||||
}
|
||||
|
||||
.album-title {
|
||||
white-space: nowrap;
|
||||
overflow: hidden;
|
||||
text-overflow: ellipsis;
|
||||
min-width: 0;
|
||||
}
|
||||
|
||||
.artist-name {
|
||||
@@ -150,8 +131,6 @@ const gridStyles = css`
|
||||
|
||||
.album-year {
|
||||
color: var(--yj-text-tertiary, #888);
|
||||
flex: 0 0 auto;
|
||||
white-space: nowrap;
|
||||
}
|
||||
|
||||
/* ========================================
|
||||
|
||||
@@ -1898,10 +1898,10 @@ export class CoverGrid
|
||||
class="album-name"
|
||||
title="${album.Name}"
|
||||
>
|
||||
<span class="album-title">${album.Name}</span
|
||||
>${album.Year
|
||||
? html`<span class="album-year"
|
||||
>(${album.Year})</span
|
||||
${album.Name}${album.Year
|
||||
? html`
|
||||
<span class="album-year">
|
||||
(${album.Year})</span
|
||||
>`
|
||||
: nothing}
|
||||
</div>
|
||||
|
||||
@@ -720,14 +720,28 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
|
||||
|
||||
// The download button only appears once a client is connected,
|
||||
// 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.canDownload = downloadStore.available;
|
||||
this.syncRequested();
|
||||
this.requestUpdate();
|
||||
});
|
||||
|
||||
void downloadStore.init().then(() => {
|
||||
this.canDownload = downloadStore.available;
|
||||
this.syncRequested();
|
||||
this.requestUpdate();
|
||||
});
|
||||
|
||||
void this.resolveTargetLibraryId();
|
||||
|
||||
@@ -193,6 +193,10 @@ export class SmartPlaylistEditor extends LitElement {
|
||||
// ── Internal state ──────────────────────────────────────────────
|
||||
|
||||
@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 sortField = 'random';
|
||||
@state() private sortDir = '';
|
||||
@@ -224,6 +228,19 @@ export class SmartPlaylistEditor extends LitElement {
|
||||
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 {
|
||||
display: grid;
|
||||
grid-template-columns: 160px 140px 1fr 28px;
|
||||
@@ -511,6 +528,10 @@ export class SmartPlaylistEditor extends LitElement {
|
||||
);
|
||||
|
||||
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.sortField = parsed.sort_field || 'random';
|
||||
this.sortDir = parsed.sort_dir ?? '';
|
||||
@@ -551,6 +572,7 @@ export class SmartPlaylistEditor extends LitElement {
|
||||
|
||||
return JSON.stringify({
|
||||
rules,
|
||||
match: this.matchType,
|
||||
limit: this.limit || 0,
|
||||
sort_field: this.sortField || '',
|
||||
sort_dir: this.sortDir || '',
|
||||
@@ -630,6 +652,11 @@ export class SmartPlaylistEditor extends LitElement {
|
||||
this.onRulesChanged();
|
||||
}
|
||||
|
||||
private updateMatchType(value: string) {
|
||||
this.matchType = value === 'any' ? 'any' : 'all';
|
||||
this.onRulesChanged();
|
||||
}
|
||||
|
||||
private updateSortField(value: string) {
|
||||
this.sortField = value;
|
||||
if (!value) this.sortDir = '';
|
||||
@@ -731,6 +758,7 @@ export class SmartPlaylistEditor extends LitElement {
|
||||
override render() {
|
||||
return html`
|
||||
<div class="rule-rows">
|
||||
${this.renderMatchType()}
|
||||
${this.ruleRows.map((row, index) =>
|
||||
this.renderRuleRow(row, index),
|
||||
)}
|
||||
@@ -743,6 +771,46 @@ 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) {
|
||||
const isBetween = row.operator === 'between';
|
||||
const operators = row.field ? getOperatorsForField(row.field) : [];
|
||||
|
||||
@@ -57,6 +57,12 @@ interface TracksModified {
|
||||
index: number;
|
||||
positions?: 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;
|
||||
@@ -198,6 +204,7 @@ class QueueStore {
|
||||
}
|
||||
|
||||
this.state.currentIndex = delta.currentIndex;
|
||||
this.state.source = delta.source ?? EMPTY_QUEUE_SOURCE;
|
||||
}
|
||||
|
||||
// ===================================================================
|
||||
|
||||
@@ -1,87 +0,0 @@
|
||||
/**
|
||||
* The year on an album card survives a long album name.
|
||||
*
|
||||
* The year used to be part of the same run of text as the title, inside
|
||||
* one `text-overflow: ellipsis` box — so it was the first thing the
|
||||
* ellipsis ate. A card wide enough for a long name never showed its
|
||||
* year at all, which means sorting the grid *by year* showed years only
|
||||
* for the albums with short names: the sort said one thing and the
|
||||
* cards showed another.
|
||||
*
|
||||
* The fix is a flex row in which only the title truncates, rather than
|
||||
* a second line, because the card's height is what the virtualizer
|
||||
* measures rows by.
|
||||
*/
|
||||
import { describe, expect, it, beforeEach } from 'vitest';
|
||||
import type { LitElement } from 'lit';
|
||||
|
||||
import '@components/cover-grid/cover-grid';
|
||||
import { emit, stub, flush, resetHarness } from '@test/support/harness';
|
||||
import { Events } from '../../src/events';
|
||||
import { fixture, shadowAll } from '@test/support/render';
|
||||
|
||||
const LONG =
|
||||
'The Rise and Fall of a Midwest Princess in the Key of Everything';
|
||||
|
||||
/** Long names throughout: the fault only shows on a card under
|
||||
* pressure, and a grid of "Album 3" proves nothing. */
|
||||
const ALBUMS = Array.from({ length: 12 }, (_, i) => ({
|
||||
ID: i + 1,
|
||||
Name: `${LONG} ${i + 1}`,
|
||||
ArtistName: 'Aurora Fields',
|
||||
Year: 2019 + (i % 5),
|
||||
}));
|
||||
|
||||
/** Give the virtualizer a viewport; a zero-height host renders nothing. */
|
||||
function sized(el: HTMLElement): void {
|
||||
el.style.display = 'block';
|
||||
el.style.height = '600px';
|
||||
el.style.width = '900px';
|
||||
}
|
||||
|
||||
async function settle(el: LitElement): Promise<void> {
|
||||
await flush();
|
||||
await el.updateComplete;
|
||||
await new Promise((r) => setTimeout(r, 80));
|
||||
}
|
||||
|
||||
describe('the album card’s year', () => {
|
||||
beforeEach(() => {
|
||||
resetHarness();
|
||||
stub('library.Library.GetAlbums', ALBUMS);
|
||||
stub('library.Library.GetTracks', []);
|
||||
emit(Events.LibraryScanComplete);
|
||||
});
|
||||
|
||||
it('is rendered on every card, however long the name', async () => {
|
||||
const el = await fixture<LitElement>('cover-grid');
|
||||
|
||||
sized(el);
|
||||
await settle(el);
|
||||
|
||||
const cards = shadowAll(el, '.album-card');
|
||||
const years = shadowAll(el, '.album-year');
|
||||
|
||||
expect(cards.length).toBeGreaterThan(0);
|
||||
expect(years).toHaveLength(cards.length);
|
||||
expect(years.every((y) => /^\(\d{4}\)$/.test(y.textContent!.trim()))).toBe(
|
||||
true,
|
||||
);
|
||||
});
|
||||
|
||||
it('is not what the ellipsis eats', async () => {
|
||||
const el = await fixture<LitElement>('cover-grid');
|
||||
|
||||
sized(el);
|
||||
await settle(el);
|
||||
|
||||
const year = shadowAll(el, '.album-year')[0]!;
|
||||
const title = shadowAll(el, '.album-title')[0]!;
|
||||
|
||||
// The title is the box that gives way...
|
||||
expect(title.scrollWidth).toBeGreaterThan(title.clientWidth);
|
||||
// ...and the year keeps every pixel it asked for.
|
||||
expect(year.clientWidth).toBeGreaterThan(0);
|
||||
expect(year.scrollWidth).toBeLessThanOrEqual(year.clientWidth + 1);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,149 @@
|
||||
/**
|
||||
* 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 tracklist’s 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',
|
||||
]);
|
||||
});
|
||||
});
|
||||
@@ -198,6 +198,61 @@ 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', () => {
|
||||
beforeEach(() => {
|
||||
sync([track(1)], 0);
|
||||
|
||||
Reference in New Issue
Block a user