Small-fix batch: eleven issues from the desktop backlog #83
No files matched your search
@@ -81,6 +81,7 @@ func (q *Queue) emitTracksModified(
|
|||||||
Index: index,
|
Index: index,
|
||||||
Positions: positions,
|
Positions: positions,
|
||||||
CurrentIndex: q.currentIndex,
|
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) {
|
func TestEmit_RemoveTracksReportsPositions(t *testing.T) {
|
||||||
t.Parallel()
|
t.Parallel()
|
||||||
|
|
||||||
|
|||||||
@@ -156,12 +156,21 @@ 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.
|
||||||
@@ -455,6 +464,8 @@ 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(
|
||||||
@@ -505,6 +516,8 @@ 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(
|
||||||
@@ -563,6 +576,8 @@ 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(
|
||||||
@@ -613,6 +628,8 @@ 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(
|
||||||
@@ -680,6 +697,8 @@ 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(
|
||||||
@@ -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.
|
// 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.
|
||||||
|
|||||||
@@ -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) {
|
func TestSetQueue_WithStartIndex(t *testing.T) {
|
||||||
t.Parallel()
|
t.Parallel()
|
||||||
|
|
||||||
|
|||||||
@@ -57,6 +57,12 @@ 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;
|
||||||
@@ -198,6 +204,7 @@ class QueueStore {
|
|||||||
}
|
}
|
||||||
|
|
||||||
this.state.currentIndex = delta.currentIndex;
|
this.state.currentIndex = delta.currentIndex;
|
||||||
|
this.state.source = delta.source ?? EMPTY_QUEUE_SOURCE;
|
||||||
}
|
}
|
||||||
|
|
||||||
// ===================================================================
|
// ===================================================================
|
||||||
|
|||||||
@@ -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', () => {
|
describe('queue store: mode deltas', () => {
|
||||||
beforeEach(() => {
|
beforeEach(() => {
|
||||||
sync([track(1)], 0);
|
sync([track(1)], 0);
|
||||||
|
|||||||
Reference in new issue
Block a user