From 85df3e42c187f163ec9450a70b1ea9ede3965047 Mon Sep 17 00:00:00 2001
From: Frank Stellmacher <171614930+Psychotoxical@users.noreply.github.com>
Date: Tue, 12 May 2026 14:22:38 +0200
Subject: [PATCH] =?UTF-8?q?refactor(player):=20E.12=20=E2=80=94=20extract?=
 =?UTF-8?q?=20skip-to-1=E2=98=85=20helper=20(#575)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

`applySkipStarOnManualNext` — the helper that records a manual `next()`
into the per-track skip counter and auto-rates the track 1★ once the
configured threshold crosses — moves into `src/store/skipStarRating.ts`.
File-private with one call site; no caller-side changes outside
playerStore's own import.

10 focused tests pin each early-return branch (manual=false, null
track, threshold not crossed, recordSkipStarManualAdvance returning
null, already-rated via override / queue / passed-track) plus the
happy path that calls setRating(1) + the state update for the queue,
currentTrack, and override map. Promise rejections are verified to
be swallowed.

playerStore 3291 → 3263 LOC.
---
 src/store/playerStore.ts         |  30 +------
 src/store/skipStarRating.test.ts | 132 +++++++++++++++++++++++++++++++
 src/store/skipStarRating.ts      |  37 +++++++++
 3 files changed, 170 insertions(+), 29 deletions(-)
 create mode 100644 src/store/skipStarRating.test.ts
 create mode 100644 src/store/skipStarRating.ts

diff --git a/src/store/playerStore.ts b/src/store/playerStore.ts
index 1aa559b5..883238fb 100644
--- a/src/store/playerStore.ts
+++ b/src/store/playerStore.ts
@@ -74,6 +74,7 @@ import {
   getWaveformRefreshGen,
 } from './waveformRefreshGen';
 import { touchHotCacheOnPlayback } from './hotCacheTouch';
+import { applySkipStarOnManualNext } from './skipStarRating';
 
 // Re-export the playback-progress public surface so existing call sites
 // (PlayerBar, FullscreenPlayer, WaveformSeek, LyricsPane, MobilePlayerView,
@@ -533,35 +534,6 @@ function scheduleSeekFallbackRetry(trackId: string, seconds: number) {
 // Guard against rapid double-click play/pause sending two state transitions
 // to the Rust backend before it has finished the previous one.
 let togglePlayLock = false;
-/**
- * Skip → 1★: counts in `authStore.skipStarManualSkipCountsByKey` (persisted).
- * Only user-initiated `next()` increments. Natural track end (incl. gapless) clears the count;
- * threshold reached clears count and sets 1★ if still unrated.
- */
-function applySkipStarOnManualNext(skippedTrack: Track | null, manual: boolean): void {
-  if (!manual || !skippedTrack) return;
-  const id = skippedTrack.id;
-  const adv = useAuthStore.getState().recordSkipStarManualAdvance(id);
-  if (!adv?.crossedThreshold) return;
-  const live = usePlayerStore.getState();
-  const fromQueue = live.queue.find(t => t.id === id);
-  const cur =
-    live.userRatingOverrides[id] ??
-    fromQueue?.userRating ??
-    skippedTrack.userRating ??
-    0;
-  if (cur >= 1) return;
-  setRating(id, 1)
-    .then(() => {
-      usePlayerStore.setState(s => ({
-        queue: s.queue.map(t => (t.id === id ? { ...t, userRating: 1 } : t)),
-        currentTrack: s.currentTrack?.id === id ? { ...s.currentTrack, userRating: 1 } : s.currentTrack,
-        userRatingOverrides: { ...s.userRatingOverrides, [id]: 1 },
-      }));
-    })
-    .catch(() => {});
-}
-
 // ── HTML5 Radio Player ────────────────────────────────────────────────────────
 // Internet radio streams are played via a native <audio> element instead of
 // the Rust/Symphonia engine.  This gives us browser-native reconnect logic,
diff --git a/src/store/skipStarRating.test.ts b/src/store/skipStarRating.test.ts
new file mode 100644
index 00000000..d9dc19f5
--- /dev/null
+++ b/src/store/skipStarRating.test.ts
@@ -0,0 +1,132 @@
+/**
+ * Skip → 1★ helper: drive each early-return branch + the happy path through
+ * the threshold-crossing flow that calls `setRating` and updates the
+ * playerStore. Hoisted mocks replace `setRating`, the auth-store helper, and
+ * the player-store state surface so the test can drive every input
+ * independently.
+ */
+import { beforeEach, describe, expect, it, vi } from 'vitest';
+import type { Track } from './playerStore';
+
+const { setRatingMock, recordSkipStarMock, playerSetStateMock, playerStateGet } = vi.hoisted(() => {
+  const playerState = {
+    queue: [] as Track[],
+    currentTrack: null as Track | null,
+    userRatingOverrides: {} as Record<string, number>,
+  };
+  return {
+    setRatingMock: vi.fn(async () => undefined),
+    recordSkipStarMock: vi.fn(),
+    playerSetStateMock: vi.fn((updater: (s: typeof playerState) => Partial<typeof playerState>) => {
+      Object.assign(playerState, updater(playerState));
+    }),
+    playerStateGet: () => playerState,
+  };
+});
+
+vi.mock('../api/subsonic', () => ({ setRating: setRatingMock }));
+vi.mock('./authStore', () => ({
+  useAuthStore: { getState: () => ({ recordSkipStarManualAdvance: recordSkipStarMock }) },
+}));
+vi.mock('./playerStore', () => ({
+  usePlayerStore: {
+    getState: playerStateGet,
+    setState: playerSetStateMock,
+  },
+}));
+
+import { applySkipStarOnManualNext } from './skipStarRating';
+
+function track(id: string, overrides: Partial<Track> = {}): Track {
+  return {
+    id, title: id, artist: 'A', album: 'X', albumId: 'X', duration: 100, ...overrides,
+  };
+}
+
+beforeEach(() => {
+  setRatingMock.mockClear();
+  recordSkipStarMock.mockReset();
+  playerSetStateMock.mockClear();
+  const s = playerStateGet();
+  s.queue = [];
+  s.currentTrack = null;
+  s.userRatingOverrides = {};
+});
+
+describe('applySkipStarOnManualNext', () => {
+  it('is a no-op when manual=false (gapless / natural advance)', () => {
+    applySkipStarOnManualNext(track('t1'), false);
+    expect(recordSkipStarMock).not.toHaveBeenCalled();
+    expect(setRatingMock).not.toHaveBeenCalled();
+  });
+
+  it('is a no-op when skippedTrack is null', () => {
+    applySkipStarOnManualNext(null, true);
+    expect(recordSkipStarMock).not.toHaveBeenCalled();
+  });
+
+  it('records the manual advance but does not rate when threshold not crossed', () => {
+    recordSkipStarMock.mockReturnValueOnce({ crossedThreshold: false });
+    applySkipStarOnManualNext(track('t1'), true);
+    expect(recordSkipStarMock).toHaveBeenCalledWith('t1');
+    expect(setRatingMock).not.toHaveBeenCalled();
+  });
+
+  it('handles a null return from recordSkipStarManualAdvance gracefully', () => {
+    recordSkipStarMock.mockReturnValueOnce(null);
+    expect(() => applySkipStarOnManualNext(track('t1'), true)).not.toThrow();
+    expect(setRatingMock).not.toHaveBeenCalled();
+  });
+
+  it("skips rating when the track is already rated via the override map", () => {
+    recordSkipStarMock.mockReturnValueOnce({ crossedThreshold: true });
+    playerStateGet().userRatingOverrides = { t1: 3 };
+    applySkipStarOnManualNext(track('t1'), true);
+    expect(setRatingMock).not.toHaveBeenCalled();
+  });
+
+  it('skips rating when the queue entry is already rated', () => {
+    recordSkipStarMock.mockReturnValueOnce({ crossedThreshold: true });
+    playerStateGet().queue = [track('t1', { userRating: 4 })];
+    applySkipStarOnManualNext(track('t1'), true);
+    expect(setRatingMock).not.toHaveBeenCalled();
+  });
+
+  it('skips rating when the passed track is already rated', () => {
+    recordSkipStarMock.mockReturnValueOnce({ crossedThreshold: true });
+    applySkipStarOnManualNext(track('t1', { userRating: 2 }), true);
+    expect(setRatingMock).not.toHaveBeenCalled();
+  });
+
+  it('calls setRating(1) when threshold crosses and the track is unrated', async () => {
+    recordSkipStarMock.mockReturnValueOnce({ crossedThreshold: true });
+    applySkipStarOnManualNext(track('t1'), true);
+    expect(setRatingMock).toHaveBeenCalledWith('t1', 1);
+    await Promise.resolve();
+    expect(playerSetStateMock).toHaveBeenCalledTimes(1);
+    const updated = playerStateGet();
+    expect(updated.userRatingOverrides).toEqual({ t1: 1 });
+  });
+
+  it('updates queue + currentTrack when the skipped track is the current one', async () => {
+    recordSkipStarMock.mockReturnValueOnce({ crossedThreshold: true });
+    const s = playerStateGet();
+    s.queue = [track('t1'), track('t2')];
+    s.currentTrack = s.queue[0];
+    applySkipStarOnManualNext(track('t1'), true);
+    await Promise.resolve();
+    const updated = playerStateGet();
+    expect(updated.queue[0].userRating).toBe(1);
+    expect(updated.queue[1].userRating).toBeUndefined();
+    expect(updated.currentTrack?.userRating).toBe(1);
+  });
+
+  it('swallows setRating rejections silently', async () => {
+    recordSkipStarMock.mockReturnValueOnce({ crossedThreshold: true });
+    setRatingMock.mockRejectedValueOnce(new Error('network down'));
+    expect(() => applySkipStarOnManualNext(track('t1'), true)).not.toThrow();
+    // Drain the rejected microtask
+    await Promise.resolve();
+    await Promise.resolve();
+  });
+});
diff --git a/src/store/skipStarRating.ts b/src/store/skipStarRating.ts
new file mode 100644
index 00000000..445f9f7d
--- /dev/null
+++ b/src/store/skipStarRating.ts
@@ -0,0 +1,37 @@
+import { setRating } from '../api/subsonic';
+import { useAuthStore } from './authStore';
+import { usePlayerStore, type Track } from './playerStore';
+
+/**
+ * Skip → 1★ behaviour: every user-initiated `next()` on an unrated track
+ * counts in `authStore.skipStarManualSkipCountsByKey` (persisted). Once the
+ * configured threshold is crossed, the track is auto-rated 1★ — both on the
+ * Subsonic server and in local Zustand state (queue + currentTrack + the
+ * override map that QueuePanel reads).
+ *
+ * Natural track end (incl. gapless advance) does NOT count; it clears the
+ * threshold counter elsewhere. Already-rated tracks are skipped silently.
+ */
+export function applySkipStarOnManualNext(skippedTrack: Track | null, manual: boolean): void {
+  if (!manual || !skippedTrack) return;
+  const id = skippedTrack.id;
+  const adv = useAuthStore.getState().recordSkipStarManualAdvance(id);
+  if (!adv?.crossedThreshold) return;
+  const live = usePlayerStore.getState();
+  const fromQueue = live.queue.find(t => t.id === id);
+  const cur =
+    live.userRatingOverrides[id] ??
+    fromQueue?.userRating ??
+    skippedTrack.userRating ??
+    0;
+  if (cur >= 1) return;
+  setRating(id, 1)
+    .then(() => {
+      usePlayerStore.setState(s => ({
+        queue: s.queue.map(t => (t.id === id ? { ...t, userRating: 1 } : t)),
+        currentTrack: s.currentTrack?.id === id ? { ...s.currentTrack, userRating: 1 } : s.currentTrack,
+        userRatingOverrides: { ...s.userRatingOverrides, [id]: 1 },
+      }));
+    })
+    .catch(() => {});
+}