From b6489be2cd7409b781dbab926796351e71d6c347 Mon Sep 17 00:00:00 2001 From: mignot Date: Tue, 29 Sep 2026 00:15:17 +0200 Subject: [PATCH 1/2] feat(genre-tree)!: page genre playlist tracks lazily Play fetches {me/}genre-playlists/{uuid}/tracks/ page by page instead of the playlist detail; the track list auto-loads near the end and the sidebar infinite-scrolls. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 13 + apps/playground/src/App.tsx | 6 - .../src/genre-tree/GenreTreeView.test.tsx | 14 +- .../app-kit/src/genre-tree/GenreTreeView.tsx | 26 +- .../src/genre-tree/TrackListContext.test.tsx | 300 ++++++++++++++---- .../src/genre-tree/TrackListContext.tsx | 90 +++++- .../api/genre-playlists/endpoints.ts | 1 + .../genre-playlists/genre-playlists.test.ts | 2 + packages/app-kit/src/genre-tree/index.test.ts | 2 +- packages/app-kit/src/genre-tree/index.ts | 1 - .../src/genre-tree/models/TrackList.ts | 10 +- .../src/genre-tree/models/TrackListOrigin.ts | 15 +- .../playlist-tree/TreePerRoot.test.tsx | 40 +-- .../genre-tree/playlist-tree/TreePerRoot.tsx | 22 +- .../playlist-tree/TreeWheel.test.tsx | 44 +-- .../genre-tree/playlist-tree/TreeWheel.tsx | 22 +- .../TreeWheelRadialPopCore.test.tsx | 44 +-- .../playlist-tree/TreeWheelRadialPopCore.tsx | 21 +- .../criteria-playlist.test.ts | 32 +- .../schemas/criteria-playlist/detailed.ts | 16 +- .../schemas/criteria-playlist/simple.ts | 4 +- .../without-playlist.test.ts | 17 +- .../track-playlist-rel/without-playlist.ts | 5 + .../src/genre-tree/schemas/track/base.ts | 2 - .../TrackListSidebar.test.tsx | 77 ++++- .../track-list-sidebar/TrackListSidebar.tsx | 21 +- .../src/genre-tree/useGenrePlaylist.test.ts | 39 +-- .../src/genre-tree/useGenrePlaylist.ts | 24 +- 28 files changed, 526 insertions(+), 384 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ca8e100..910c896 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,19 @@ easy to spot when bumping. ## [Unreleased] +### Breaking + +- **genre-tree**: playing a genre playlist now pages its tracks from `GET {me/}genre-playlists/{uuid}/tracks/` (100 per page) instead of fetching the playlist detail, so the backend must serve that endpoint. `playNewTrackListFromGenrePlaylist(genrePlaylist, scope)` takes the tree's list item and returns a Promise. +- **genre-tree**: `GenreTreeView` drops the `criteriaPlaylistDetailedSchema` prop and its track generic; tracks are parsed with `TrackListProvider`'s `schema`. +- **genre-tree**: `CriteriaPlaylistDetailedSchema` is metadata-only (no `trackPlaylistRelations`). `makeCriteriaPlaylistDetailedSchema`, `CriteriaPlaylistDetailedLike` and `useFetchGenrePlaylistDetailed` are removed. +- **genre-tree**: `TrackBaseSchema` no longer has `playlists`. + +### Added + +- **genre-tree**: `TrackList` carries `total` and `nextPage`; `useTrackList().loadMore()` appends the next page, and the provider loads it automatically when the selected track is within 10 of the end. +- **genre-tree**: `TrackListSidebar` shows the playlist's total track count and loads more tracks when scrolled to the bottom. +- **genre-tree**: `genrePlaylistEndpoints.tracks(uuid)` and `makeTrackPlaylistRelPageSchema(trackSchema)`. + ## [7.0.2] - 2026-09-28 ### Fixed diff --git a/apps/playground/src/App.tsx b/apps/playground/src/App.tsx index 98a2581..977fcda 100644 --- a/apps/playground/src/App.tsx +++ b/apps/playground/src/App.tsx @@ -14,7 +14,6 @@ import { libraryQueryKeys, YoutubeTrackDetailed, YoutubeTrackDetailedSchema, - makeCriteriaPlaylistDetailedSchema, } from "@behindthemusictree/app-kit"; import { Button, RingLoader, Skeleton } from "@behindthemusictree/ui"; import GenreCreationPopup from "./GenreCreationPopup"; @@ -47,10 +46,6 @@ function useLoadTrack(): (trackId: string) => Promise { ); } -const criteriaPlaylistDetailedSchema = makeCriteriaPlaylistDetailedSchema( - YoutubeTrackDetailedSchema, -); - function ReferenceGenreTree() { const { showPopup, hidePopup } = usePopup(); @@ -88,7 +83,6 @@ function ReferenceGenreTree() { handleGenreCreationAction={showCriteriaCreationPopup} handleGenreRenameAction={showGenreRenamePopup} getBackendBaseUrl={getBackendBaseUrl} - criteriaPlaylistDetailedSchema={criteriaPlaylistDetailedSchema} /> ); } diff --git a/packages/app-kit/src/genre-tree/GenreTreeView.test.tsx b/packages/app-kit/src/genre-tree/GenreTreeView.test.tsx index 809a9d1..ff6f4d0 100644 --- a/packages/app-kit/src/genre-tree/GenreTreeView.test.tsx +++ b/packages/app-kit/src/genre-tree/GenreTreeView.test.tsx @@ -64,11 +64,8 @@ vi.mock("@behindthemusictree/genre-tree-view", async (importOriginal) => ({ })); import { GenreTreeView, type GenreTreeViewProps } from "./GenreTreeView"; -import type { TrackBase } from "./schemas/track/base"; -import type { CriteriaPlaylistDetailedLike } from "./models/TrackListOrigin"; const getBackendBaseUrl = () => "https://backend.example.com"; -const schema = z.custom>(); // requestAnimationFrame isn't driven by fake timers in jsdom — stub it onto a manually-flushable // queue so tests can step through GenreTreeWheelHandoff's two nested rAFs deterministically. @@ -102,13 +99,12 @@ function makePlaylist(overrides: Record = {}) { }; } -function renderView(overrides: Partial> = {}) { - const props: GenreTreeViewProps = { +function renderView(overrides: Partial = {}) { + const props: GenreTreeViewProps = { scope: "me", handleGenreCreationAction: vi.fn(), handleGenreRenameAction: vi.fn(), getBackendBaseUrl, - criteriaPlaylistDetailedSchema: schema, ...overrides, }; render(); @@ -487,12 +483,11 @@ describe("GenreTreeView", () => { data: undefined, isPending: true, }); - const props: GenreTreeViewProps = { + const props: GenreTreeViewProps = { scope: "me", handleGenreCreationAction: vi.fn(), handleGenreRenameAction: vi.fn(), getBackendBaseUrl, - criteriaPlaylistDetailedSchema: schema, }; const { rerender } = render(); @@ -533,12 +528,11 @@ describe("GenreTreeView", () => { data: undefined, isPending: true, }); - const props: GenreTreeViewProps = { + const props: GenreTreeViewProps = { scope: "me", handleGenreCreationAction: vi.fn(), handleGenreRenameAction: vi.fn(), getBackendBaseUrl, - criteriaPlaylistDetailedSchema: schema, }; const { rerender } = render(); diff --git a/packages/app-kit/src/genre-tree/GenreTreeView.tsx b/packages/app-kit/src/genre-tree/GenreTreeView.tsx index fdcd9ca..8ae43f5 100644 --- a/packages/app-kit/src/genre-tree/GenreTreeView.tsx +++ b/packages/app-kit/src/genre-tree/GenreTreeView.tsx @@ -17,9 +17,7 @@ import type { import { CriteriaPlaylistSimple } from "./schemas/criteria-playlist/simple"; import { CriteriaMinimum } from "./schemas/criteria/minimum"; -import { TrackBase } from "./schemas/track/base"; import { CriteriaOverview } from "./schemas/criteria/overview"; -import { CriteriaPlaylistDetailedLike } from "./models/TrackListOrigin"; import { Scope } from "../transport/lib/scope"; import { useListFullGenrePlaylists } from "./useGenrePlaylist"; import { usePrefetchGenreOverview } from "./useGenre"; @@ -39,15 +37,11 @@ export type { GenreTreeViewMode } from "@behindthemusictree/genre-tree-view"; const HOVER_PREFETCH_DELAY_MS = 100; -export type GenreTreeViewProps< - T extends TrackBase, - O extends CriteriaOverview = CriteriaOverview, -> = { +export type GenreTreeViewProps = { scope: Scope; handleGenreCreationAction: (parent: CriteriaMinimum | null) => void; handleGenreRenameAction: (genre: CriteriaMinimum) => void; getBackendBaseUrl: () => string; - criteriaPlaylistDetailedSchema: z.ZodType>; additionalActions?: (node: GenreTreeNode) => GenreTreeAction[]; /** Controlled view mode. When provided, the internal Stacked/Wheel toggle is not rendered — the consumer owns that UI. */ viewMode?: GenreTreeViewMode; @@ -60,21 +54,17 @@ export type GenreTreeViewProps< renderGenreDetailExtras?: (overview: O) => ReactNode; }; -export function GenreTreeView< - T extends TrackBase, - O extends CriteriaOverview = CriteriaOverview, ->({ +export function GenreTreeView({ scope, handleGenreCreationAction, handleGenreRenameAction, getBackendBaseUrl, - criteriaPlaylistDetailedSchema, additionalActions, viewMode: controlledViewMode, readOnly = false, criteriaOverviewSchema, renderGenreDetailExtras, -}: GenreTreeViewProps) { +}: GenreTreeViewProps) { const [reparentingGenreUuid, setReparentingGenreUuid] = useState< string | null >(null); @@ -310,9 +300,6 @@ export function GenreTreeView< handleGenreCreationAction={handleGenreCreationAction} handleGenreRenameAction={handleGenreRenameAction} getBackendBaseUrl={getBackendBaseUrl} - criteriaPlaylistDetailedSchema={ - criteriaPlaylistDetailedSchema - } additionalActions={additionalActions} onNodeClick={handleNodeClick} onNodeHover={handleNodeHover} @@ -340,9 +327,6 @@ export function GenreTreeView< handleGenreCreationAction={handleGenreCreationAction} handleGenreRenameAction={handleGenreRenameAction} getBackendBaseUrl={getBackendBaseUrl} - criteriaPlaylistDetailedSchema={ - criteriaPlaylistDetailedSchema - } additionalActions={additionalActions} onNodeClick={handleNodeClick} onNodeHover={handleNodeHover} @@ -366,7 +350,6 @@ export function GenreTreeView< handleGenreCreationAction={handleGenreCreationAction} handleGenreRenameAction={handleGenreRenameAction} getBackendBaseUrl={getBackendBaseUrl} - criteriaPlaylistDetailedSchema={criteriaPlaylistDetailedSchema} additionalActions={additionalActions} onNodeClick={handleNodeClick} onNodeHover={handleNodeHover} @@ -395,9 +378,6 @@ export function GenreTreeView< handleGenreCreationAction={handleGenreCreationAction} handleGenreRenameAction={handleGenreRenameAction} getBackendBaseUrl={getBackendBaseUrl} - criteriaPlaylistDetailedSchema={ - criteriaPlaylistDetailedSchema - } additionalActions={additionalActions} onNodeClick={handleNodeClick} onNodeHover={handleNodeHover} diff --git a/packages/app-kit/src/genre-tree/TrackListContext.test.tsx b/packages/app-kit/src/genre-tree/TrackListContext.test.tsx index 45c18b5..189d4bf 100644 --- a/packages/app-kit/src/genre-tree/TrackListContext.test.tsx +++ b/packages/app-kit/src/genre-tree/TrackListContext.test.tsx @@ -50,7 +50,6 @@ function makeTrack(uuid: string, title: string, overrides: Partial = genre: { uuid: "g1", name: "Jazz" } as unknown as TrackBase["genre"], rating: null, language: null, - playlists: [], playCount: 0, createdOn: "2024-01-01T00:00:00.000Z", updatedOn: null, @@ -58,6 +57,43 @@ function makeTrack(uuid: string, title: string, overrides: Partial = } as TrackBase; } +type RenderedTrackList = { current: ReturnType }; + +function makePage(tracks: TrackBase[], { page = 1, next = false, total = tracks.length } = {}) { + return { + overallTotal: total, + next: next ? `https://backend.example.com/genre-playlists/p1/tracks/?page=${page + 1}` : null, + previous: null, + results: tracks.map((track, index) => ({ position: index + 1, track })), + page, + pageSize: 100, + totalPages: 1, + }; +} + +function makeTracks(count: number, prefix = "t") { + return Array.from({ length: count }, (_, index) => makeTrack(`${prefix}${index}`, `Track ${index}`)); +} + +function deferred() { + let resolve!: (value: T) => void; + let reject!: (error: unknown) => void; + const promise = new Promise((res, rej) => { + resolve = res; + reject = rej; + }); + return { promise, resolve, reject }; +} + +const genrePlaylist = { uuid: "p1", name: "My Playlist" }; + +async function playGenre(result: RenderedTrackList, page: ReturnType, scope: "me" | "reference" = "me") { + fetchMock.mockResolvedValueOnce(page); + await act(async () => { + await result.current.playNewTrackListFromGenrePlaylist(genrePlaylist, scope); + }); +} + function wrapper({ children }: { children: ReactNode }) { return ( { }); describe("playNewTrackListFromGenrePlaylist", () => { - it("sorts tracks by position, selects the first, shows the sidebar, and loads it", () => { + it("fetches the first reference page, selects the first track, shows the sidebar, and loads it", async () => { const { result } = renderHook(() => useTrackList(), { wrapper }); - const trackA = makeTrack("a", "Track A"); - const trackB = makeTrack("b", "Track B"); - const genrePlaylist = { - uuid: "p1", - name: "My Playlist", - trackPlaylistRelations: [ - { track: trackB, position: 2 }, - { track: trackA, position: 1 }, - ], - }; + const [trackA, trackB] = makeTracks(2); - act(() => { - result.current.playNewTrackListFromGenrePlaylist(genrePlaylist, "reference"); - }); + await playGenre(result, makePage([trackA, trackB], { next: true, total: 250 }), "reference"); + expect(fetchMock).toHaveBeenCalledWith("genre-playlists/p1/tracks/", true, false, {}, { page: 1, pageSize: 100 }); expect(result.current.trackList?.tracks).toEqual([trackA, trackB]); + expect(result.current.trackList?.total).toBe(250); + expect(result.current.trackList?.nextPage).toBe(2); expect(result.current.selectedTrack).toEqual(trackA); expect(showTrackListSidebarMock).toHaveBeenCalled(); - expect(loadTrackForPlayerMock).toHaveBeenCalledWith("a"); + expect(loadTrackForPlayerMock).toHaveBeenCalledWith("t0"); + }); + + it("fetches the me-scoped endpoint with auth and has no next page on the last page", async () => { + const { result } = renderHook(() => useTrackList(), { wrapper }); + + await playGenre(result, makePage(makeTracks(1)), "me"); + + expect(fetchMock).toHaveBeenCalledWith("me/genre-playlists/p1/tracks/", true, true, {}, { page: 1, pageSize: 100 }); + expect(result.current.trackList?.nextPage).toBeNull(); }); - it("warns and does nothing when the playlist has no tracks", () => { + it("warns and does nothing when the playlist has no tracks", async () => { const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); const { result } = renderHook(() => useTrackList(), { wrapper }); - const genrePlaylist = { uuid: "p1", name: "Empty", trackPlaylistRelations: [] }; - act(() => { - result.current.playNewTrackListFromGenrePlaylist(genrePlaylist, "me"); - }); + await playGenre(result, makePage([])); expect(warnSpy).toHaveBeenCalledWith("No tracks found in genre playlist"); expect(result.current.trackList).toBeNull(); @@ -149,27 +183,196 @@ describe("TrackListContext", () => { warnSpy.mockRestore(); }); + + it("drops a response that arrives after a newer play request", async () => { + const { result } = renderHook(() => useTrackList(), { wrapper }); + const slow = deferred(); + fetchMock.mockReturnValueOnce(slow.promise); + let first!: Promise; + act(() => { + first = result.current.playNewTrackListFromGenrePlaylist({ uuid: "old", name: "Old" }, "me"); + }); + + await playGenre(result, makePage(makeTracks(1, "new"))); + await act(async () => { + slow.resolve(makePage(makeTracks(1, "old"))); + await first; + }); + + expect(result.current.trackList?.origin.uuid).toBe("p1"); + expect(result.current.selectedTrack?.uuid).toBe("new0"); + expect(loadTrackForPlayerMock).toHaveBeenCalledTimes(1); + }); + + it("rejects when the page fails schema validation", async () => { + const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + const { result } = renderHook(() => useTrackList(), { wrapper }); + fetchMock.mockResolvedValueOnce({ results: "nope" }); + + await act(async () => { + await expect(result.current.playNewTrackListFromGenrePlaylist(genrePlaylist, "me")).rejects.toBeDefined(); + }); + + expect(result.current.trackList).toBeNull(); + consoleErrorSpy.mockRestore(); + }); + }); + + describe("loadMore", () => { + it("does nothing without a track list", async () => { + const { result } = renderHook(() => useTrackList(), { wrapper }); + + await act(async () => { + await result.current.loadMore(); + }); + + expect(fetchMock).not.toHaveBeenCalled(); + }); + + it("does nothing when there is no next page", async () => { + const { result } = renderHook(() => useTrackList(), { wrapper }); + act(() => { + result.current.playNewTrackListFromTrackUuid(makeTrack("a", "A"), "me"); + }); + + await act(async () => { + await result.current.loadMore(); + }); + + expect(fetchMock).not.toHaveBeenCalled(); + }); + + it("appends the next page, skipping tracks already loaded, and updates total and next page", async () => { + const { result } = renderHook(() => useTrackList(), { wrapper }); + const firstPage = makeTracks(20); + await playGenre(result, makePage(firstPage, { next: true, total: 40 })); + const secondPage = [firstPage[19], ...makeTracks(3, "u")]; + fetchMock.mockResolvedValueOnce(makePage(secondPage, { page: 2, total: 41 })); + + await act(async () => { + await result.current.loadMore(); + }); + + expect(fetchMock).toHaveBeenLastCalledWith("me/genre-playlists/p1/tracks/", true, true, {}, { page: 2, pageSize: 100 }); + expect(result.current.trackList?.tracks.map((track) => track.uuid)).toEqual([ + ...firstPage.map((track) => track.uuid), + "u0", + "u1", + "u2", + ]); + expect(result.current.trackList?.total).toBe(41); + expect(result.current.trackList?.nextPage).toBeNull(); + }); + + it("fetches a page only once while a load is in flight", async () => { + const { result } = renderHook(() => useTrackList(), { wrapper }); + await playGenre(result, makePage(makeTracks(20), { next: true })); + const pending = deferred(); + fetchMock.mockReturnValueOnce(pending.promise); + + let first!: Promise; + await act(async () => { + first = result.current.loadMore(); + await result.current.loadMore(); + }); + await act(async () => { + pending.resolve(makePage(makeTracks(1, "u"), { page: 2 })); + await first; + }); + + expect(fetchMock).toHaveBeenCalledTimes(2); + expect(result.current.trackList?.tracks).toHaveLength(21); + }); + + it("discards a page that lands after a different list started playing", async () => { + const { result } = renderHook(() => useTrackList(), { wrapper }); + await playGenre(result, makePage(makeTracks(20), { next: true })); + const pending = deferred(); + fetchMock.mockReturnValueOnce(pending.promise); + let load!: Promise; + act(() => { + load = result.current.loadMore(); + }); + + const other = makeTrack("x", "X"); + act(() => { + result.current.playNewTrackListFromTrackUuid(other, "me"); + }); + await act(async () => { + pending.resolve(makePage(makeTracks(1, "u"), { page: 2 })); + await load; + }); + + expect(result.current.trackList?.tracks).toEqual([other]); + }); + + it("releases the in-flight guard when the fetch fails", async () => { + const { result } = renderHook(() => useTrackList(), { wrapper }); + await playGenre(result, makePage(makeTracks(20), { next: true })); + fetchMock.mockRejectedValueOnce(new Error("offline")); + + await act(async () => { + await expect(result.current.loadMore()).rejects.toThrow("offline"); + }); + fetchMock.mockResolvedValueOnce(makePage(makeTracks(1, "u"), { page: 2 })); + await act(async () => { + await result.current.loadMore(); + }); + + expect(result.current.trackList?.tracks).toHaveLength(21); + }); + }); + + describe("auto-load", () => { + it("loads the next page once the selected track is within 10 of the end", async () => { + const { result } = renderHook(() => useTrackList(), { wrapper }); + await playGenre(result, makePage(makeTracks(15), { next: true })); + expect(fetchMock).toHaveBeenCalledTimes(1); + + fetchMock.mockResolvedValueOnce(makePage(makeTracks(1, "u"), { page: 2 })); + await act(async () => { + result.current.toTrackAtPosition(5); + }); + + expect(fetchMock).toHaveBeenCalledTimes(2); + expect(result.current.trackList?.tracks).toHaveLength(16); + }); + + it("ignores a selected track that isn't in the list", async () => { + const { result } = renderHook(() => useTrackList(), { wrapper }); + await playGenre(result, makePage(makeTracks(15), { next: true })); + + act(() => { + result.current.setSelectedTrack(makeTrack("elsewhere", "Elsewhere")); + }); + + expect(fetchMock).toHaveBeenCalledTimes(1); + }); + + it("logs when the automatic load fails", async () => { + const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + const { result } = renderHook(() => useTrackList(), { wrapper }); + const error = new Error("offline"); + fetchMock.mockResolvedValueOnce(makePage(makeTracks(5), { next: true })).mockRejectedValueOnce(error); + + await act(async () => { + await result.current.playNewTrackListFromGenrePlaylist(genrePlaylist, "me"); + }); + + await vi.waitFor(() => + expect(consoleErrorSpy).toHaveBeenCalledWith("Failed to load more genre playlist tracks:", error), + ); + consoleErrorSpy.mockRestore(); + }); }); describe("toTrackAtPosition", () => { - it("selects the track at a valid position", () => { + it("selects the track at a valid position", async () => { const { result } = renderHook(() => useTrackList(), { wrapper }); const trackA = makeTrack("a", "Track A"); const trackB = makeTrack("b", "Track B"); - act(() => { - result.current.playNewTrackListFromGenrePlaylist( - { - uuid: "p1", - name: "P", - trackPlaylistRelations: [ - { track: trackA, position: 0 }, - { track: trackB, position: 1 }, - ], - }, - "me", - ); - }); + await playGenre(result, makePage([trackA, trackB])); act(() => { result.current.toTrackAtPosition(1); @@ -245,7 +448,7 @@ describe("TrackListContext", () => { expect(result.current.trackList?.tracks).toEqual([original]); }); - it("updates matching tracks in a genre playlist and keeps others unchanged", () => { + it("updates matching tracks in a genre playlist and keeps others unchanged", async () => { const trackA = makeTrack("a", "Track A"); const trackB = makeTrack("b", "Track B"); const updatedB = makeTrack("b", "Track B Updated"); @@ -253,35 +456,20 @@ describe("TrackListContext", () => { const { result } = renderHook(() => useTrackList(), { wrapper }); - act(() => { - result.current.playNewTrackListFromGenrePlaylist( - { - uuid: "p1", - name: "P", - trackPlaylistRelations: [ - { track: trackA, position: 0 }, - { track: trackB, position: 1 }, - ], - }, - "me", - ); - }); + await playGenre(result, makePage([trackA, trackB], { next: true, total: 7 })); expect(result.current.trackList?.tracks).toEqual([trackA, updatedB]); + expect(result.current.trackList?.total).toBe(7); + expect(result.current.trackList?.nextPage).toBe(2); }); - it("leaves the genre playlist list untouched when nothing changed", () => { + it("leaves the genre playlist list untouched when nothing changed", async () => { const trackA = makeTrack("a", "Track A"); useQueryWithParseMock.mockReturnValue({ data: { results: [] } }); const { result } = renderHook(() => useTrackList(), { wrapper }); - act(() => { - result.current.playNewTrackListFromGenrePlaylist( - { uuid: "p1", name: "P", trackPlaylistRelations: [{ track: trackA, position: 0 }] }, - "me", - ); - }); + await playGenre(result, makePage([trackA])); expect(result.current.trackList?.tracks).toEqual([trackA]); }); diff --git a/packages/app-kit/src/genre-tree/TrackListContext.tsx b/packages/app-kit/src/genre-tree/TrackListContext.tsx index a9230b6..b42946d 100644 --- a/packages/app-kit/src/genre-tree/TrackListContext.tsx +++ b/packages/app-kit/src/genre-tree/TrackListContext.tsx @@ -8,24 +8,29 @@ * `(track, scope)` signature. */ -import { createContext, useState, useContext, ReactNode, useCallback, useMemo } from "react"; +import { createContext, useState, useContext, ReactNode, useCallback, useEffect, useMemo, useRef } from "react"; import { z } from "zod"; import { useFetchWrapper } from "../transport/useFetchWrapper"; import { useSession } from "../auth/SessionContext"; import { useQueryWithParse } from "../transport/lib/use-query-with-parse"; import { PaginatedResponseSchema } from "../transport/lib/paginated-response"; +import { parseWithLog } from "../transport/lib/parse-with-log"; import { TrackBase } from "./schemas/track/base"; +import { CriteriaPlaylistMinimum } from "./schemas/criteria-playlist/minimum"; +import { makeTrackPlaylistRelPageSchema } from "./schemas/track-playlist-rel/without-playlist"; +import { genrePlaylistEndpoints } from "./api/genre-playlists"; import TrackList, { TrackListFromTrack, TrackListFromCriteriaPlaylist } from "./models/TrackList"; -import { - TrackListOriginFromTrack, - TrackListOriginFromCriteriaPlaylist, - CriteriaPlaylistDetailedLike, -} from "./models/TrackListOrigin"; +import { TrackListOriginFromTrack, TrackListOriginFromCriteriaPlaylist } from "./models/TrackListOrigin"; import { TrackListOriginType } from "./models/TrackListOriginType"; import { Scope } from "../transport/lib/scope"; import { usePlayer } from "../player/PlayerContext"; import { useTrackListSidebarVisibility } from "./TrackListSidebarVisibilityContext"; +const GENRE_PLAYLIST_PAGE_SIZE = 100; +// Load the next page once the selected track is this close to the end of what's loaded, so +// auto-advance and next-track never run out. +const AUTO_LOAD_REMAINING_TRACKS = 10; + export function useListTracks( scope: Scope | null, getBackendBaseUrl: () => string, @@ -56,7 +61,10 @@ interface TrackListContextType { setSelectedTrack: (track: T | null) => void; toTrackAtPosition: (position: number) => void; playNewTrackListFromTrackUuid: (track: T, scope: Scope) => void; - playNewTrackListFromGenrePlaylist: (genrePlaylist: CriteriaPlaylistDetailedLike, scope: Scope) => void; + /** Fetches the playlist's first tracks page and plays its first track. Rejects on fetch/parse failure. */ + playNewTrackListFromGenrePlaylist: (genrePlaylist: CriteriaPlaylistMinimum, scope: Scope) => Promise; + /** Appends the next tracks page of the current genre-playlist list; no-op when fully loaded or already loading. */ + loadMore: () => Promise; } const TrackListContext = createContext | undefined>(undefined); @@ -82,6 +90,26 @@ export function TrackListProvider({ const { showTrackListSidebar } = useTrackListSidebarVisibility(); const scope = trackList?.origin?.scope ?? null; const { data: tracksResponse } = useListTracks(scope, getBackendBaseUrl, schema, listEndpoint, listQueryKey); + const { fetch } = useFetchWrapper(getBackendBaseUrl); + const pageSchema = useMemo(() => makeTrackPlaylistRelPageSchema(schema), [schema]); + // Identifies the latest play request / in-flight page load, so late responses for a list that's + // no longer current are dropped. + const latestPlayRequestRef = useRef(0); + const loadingOriginRef = useRef(null); + + const fetchTracksPage = useCallback( + async (uuid: string, scope: Scope, page: number) => { + const endpoint = genrePlaylistEndpoints[scope].tracks(uuid); + const response = await fetch(endpoint, true, scope === "me", {}, { page, pageSize: GENRE_PLAYLIST_PAGE_SIZE }); + const parsed = parseWithLog(pageSchema, response, "fetchGenrePlaylistTracksPage"); + return { + tracks: parsed.results.map((rel) => rel.track as T), + total: parsed.overallTotal, + nextPage: parsed.next ? parsed.page + 1 : null, + }; + }, + [fetch, pageSchema], + ); // Create a memoized track list that updates when tracks changes const currentTrackList = useMemo(() => { @@ -103,7 +131,7 @@ export function TrackListProvider({ } // If the current track list is from a genre playlist, update tracks with fresh data else if (trackList.origin.type === TrackListOriginType.GENRE_PLAYLIST) { - const origin = trackList.origin as TrackListOriginFromCriteriaPlaylist; + const origin = trackList.origin as TrackListOriginFromCriteriaPlaylist; // Update all tracks in the playlist with fresh data const updatedTracks = trackList.tracks.map((originalTrack) => { @@ -115,7 +143,7 @@ export function TrackListProvider({ const hasUpdates = updatedTracks.some((updatedTrack, index) => updatedTrack !== trackList.tracks[index]); if (hasUpdates) { - return new TrackListFromCriteriaPlaylist(updatedTracks, origin); + return new TrackListFromCriteriaPlaylist(updatedTracks, origin, trackList.total, trackList.nextPage); } } @@ -145,10 +173,10 @@ export function TrackListProvider({ ); const playNewTrackListFromGenrePlaylist = useCallback( - (genrePlaylist: CriteriaPlaylistDetailedLike, scope: Scope) => { - const tracks = genrePlaylist.trackPlaylistRelations - .sort((a, b) => a.position - b.position) - .map((rel) => rel.track); + async (genrePlaylist: CriteriaPlaylistMinimum, scope: Scope) => { + const request = ++latestPlayRequestRef.current; + const { tracks, total, nextPage } = await fetchTracksPage(genrePlaylist.uuid, scope, 1); + if (request !== latestPlayRequestRef.current) return; if (tracks.length === 0) { console.warn("No tracks found in genre playlist"); @@ -156,16 +184,42 @@ export function TrackListProvider({ } const origin = new TrackListOriginFromCriteriaPlaylist(genrePlaylist, scope); - const newTrackList = new TrackListFromCriteriaPlaylist(tracks, origin); - - setTrackList(newTrackList); + setTrackList(new TrackListFromCriteriaPlaylist(tracks, origin, total, nextPage)); setSelectedTrack(tracks[0]); showTrackListSidebar(); loadTrackForPlayer(tracks[0].uuid); }, - [showTrackListSidebar, loadTrackForPlayer], + [fetchTracksPage, showTrackListSidebar, loadTrackForPlayer], ); + const loadMore = useCallback(async () => { + if (!trackList || trackList.nextPage === null) return; + const origin = trackList.origin as TrackListOriginFromCriteriaPlaylist; + if (loadingOriginRef.current === origin) return; + + loadingOriginRef.current = origin; + try { + const page = await fetchTracksPage(origin.uuid, origin.scope, trackList.nextPage); + setTrackList((prev) => { + if (prev?.origin !== origin) return prev; + // Positions can shift between page fetches if the playlist changes; never list a track twice. + const loaded = new Set(prev.tracks.map((track) => track.uuid)); + const newTracks = page.tracks.filter((track) => !loaded.has(track.uuid)); + return new TrackListFromCriteriaPlaylist([...prev.tracks, ...newTracks], origin, page.total, page.nextPage); + }); + } finally { + if (loadingOriginRef.current === origin) loadingOriginRef.current = null; + } + }, [trackList, fetchTracksPage]); + + useEffect(() => { + if (!currentTrackList || currentTrackList.nextPage === null || !selectedTrack) return; + const index = currentTrackList.tracks.findIndex((track) => track.uuid === selectedTrack.uuid); + if (index !== -1 && currentTrackList.tracks.length - index <= AUTO_LOAD_REMAINING_TRACKS) { + loadMore().catch((error) => console.error("Failed to load more genre playlist tracks:", error)); + } + }, [currentTrackList, selectedTrack, loadMore]); + const value = useMemo( () => ({ trackList: currentTrackList, @@ -174,6 +228,7 @@ export function TrackListProvider({ toTrackAtPosition, playNewTrackListFromTrackUuid, playNewTrackListFromGenrePlaylist, + loadMore, }), [ currentTrackList, @@ -181,6 +236,7 @@ export function TrackListProvider({ toTrackAtPosition, playNewTrackListFromTrackUuid, playNewTrackListFromGenrePlaylist, + loadMore, ], ); diff --git a/packages/app-kit/src/genre-tree/api/genre-playlists/endpoints.ts b/packages/app-kit/src/genre-tree/api/genre-playlists/endpoints.ts index 22effdc..bcb654a 100644 --- a/packages/app-kit/src/genre-tree/api/genre-playlists/endpoints.ts +++ b/packages/app-kit/src/genre-tree/api/genre-playlists/endpoints.ts @@ -7,6 +7,7 @@ const makeGenrePlaylistEndpoints = (prefix: string) => ({ list: () => `${prefix}genre-playlists/`, detail: (uuid: string) => `${prefix}genre-playlists/${uuid}/`, + tracks: (uuid: string) => `${prefix}genre-playlists/${uuid}/tracks/`, create: () => `${prefix}genre-playlists/`, update: (uuid: string) => `${prefix}genre-playlists/${uuid}/`, delete: (uuid: string) => `${prefix}genre-playlists/${uuid}/`, diff --git a/packages/app-kit/src/genre-tree/api/genre-playlists/genre-playlists.test.ts b/packages/app-kit/src/genre-tree/api/genre-playlists/genre-playlists.test.ts index 13c2d77..01558cc 100644 --- a/packages/app-kit/src/genre-tree/api/genre-playlists/genre-playlists.test.ts +++ b/packages/app-kit/src/genre-tree/api/genre-playlists/genre-playlists.test.ts @@ -10,6 +10,7 @@ describe("genrePlaylistEndpoints", () => { it("builds 'me' scope URLs with the 'me/' prefix", () => { expect(genrePlaylistEndpoints.me.list()).toBe("me/genre-playlists/"); expect(genrePlaylistEndpoints.me.detail(uuid)).toBe(`me/genre-playlists/${uuid}/`); + expect(genrePlaylistEndpoints.me.tracks(uuid)).toBe(`me/genre-playlists/${uuid}/tracks/`); expect(genrePlaylistEndpoints.me.create()).toBe("me/genre-playlists/"); expect(genrePlaylistEndpoints.me.update(uuid)).toBe(`me/genre-playlists/${uuid}/`); expect(genrePlaylistEndpoints.me.delete(uuid)).toBe(`me/genre-playlists/${uuid}/`); @@ -18,6 +19,7 @@ describe("genrePlaylistEndpoints", () => { it("builds 'reference' scope URLs with no prefix", () => { expect(genrePlaylistEndpoints.reference.list()).toBe("genre-playlists/"); expect(genrePlaylistEndpoints.reference.detail(uuid)).toBe(`genre-playlists/${uuid}/`); + expect(genrePlaylistEndpoints.reference.tracks(uuid)).toBe(`genre-playlists/${uuid}/tracks/`); }); }); diff --git a/packages/app-kit/src/genre-tree/index.test.ts b/packages/app-kit/src/genre-tree/index.test.ts index 917b8a5..81c1635 100644 --- a/packages/app-kit/src/genre-tree/index.test.ts +++ b/packages/app-kit/src/genre-tree/index.test.ts @@ -40,7 +40,7 @@ describe("genre-tree barrel", () => { expect(genreTree.MbArtistDetailedSchema).toBeTypeOf("object"); expect(genreTree.MbRecordingDetailedSchema).toBeTypeOf("object"); expect(genreTree.CriteriaMinimumSchema).toBeTypeOf("object"); - expect(genreTree.CriteriaPlaylistDetailedBaseSchema).toBeTypeOf("object"); + expect(genreTree.CriteriaPlaylistDetailedSchema).toBeTypeOf("object"); expect(genreTree.CriteriaPlaylistSimpleSchema).toBeTypeOf("object"); expect(genreTree.YoutubeTrackDetailedSchema).toBeTypeOf("object"); expect(genreTree.TrackBaseSchema).toBeTypeOf("object"); diff --git a/packages/app-kit/src/genre-tree/index.ts b/packages/app-kit/src/genre-tree/index.ts index 197cfb0..bd29f6d 100644 --- a/packages/app-kit/src/genre-tree/index.ts +++ b/packages/app-kit/src/genre-tree/index.ts @@ -36,7 +36,6 @@ export { TrackListOriginFromTrack, TrackListOriginFromCriteriaPlaylist, } from "./models/TrackListOrigin"; -export type { CriteriaPlaylistDetailedLike } from "./models/TrackListOrigin"; export * from "./models/TrackListOriginType"; // Schemas / domain types diff --git a/packages/app-kit/src/genre-tree/models/TrackList.ts b/packages/app-kit/src/genre-tree/models/TrackList.ts index bfb2e49..f36d98f 100644 --- a/packages/app-kit/src/genre-tree/models/TrackList.ts +++ b/packages/app-kit/src/genre-tree/models/TrackList.ts @@ -5,6 +5,10 @@ export default class TrackList { constructor( public tracks: T[], public origin: TrackListOrigin, + /** Tracks in the whole origin, loaded or not. */ + public total: number = tracks.length, + /** Next page to fetch, or null once every track is loaded. */ + public nextPage: number | null = null, ) {} } @@ -20,8 +24,10 @@ export class TrackListFromTrack extends TrackLi export class TrackListFromCriteriaPlaylist extends TrackList { constructor( public tracks: T[], - public origin: TrackListOriginFromCriteriaPlaylist, + public origin: TrackListOriginFromCriteriaPlaylist, + total: number, + nextPage: number | null, ) { - super(tracks, origin); + super(tracks, origin, total, nextPage); } } diff --git a/packages/app-kit/src/genre-tree/models/TrackListOrigin.ts b/packages/app-kit/src/genre-tree/models/TrackListOrigin.ts index a6229d2..190d43b 100644 --- a/packages/app-kit/src/genre-tree/models/TrackListOrigin.ts +++ b/packages/app-kit/src/genre-tree/models/TrackListOrigin.ts @@ -6,19 +6,10 @@ * workspace — are kept. */ import { TrackBase } from "../schemas/track/base"; +import { CriteriaPlaylistMinimum } from "../schemas/criteria-playlist/minimum"; import { Scope } from "../../transport/lib/scope"; import { TrackListOriginType } from "./TrackListOriginType"; -// Structural shape of a parsed criteria-playlist-detailed response, generic over its track type — -// see `../schemas/criteria-playlist/detailed.ts`'s `makeCriteriaPlaylistDetailedSchema`. Kept -// minimal (only the fields this model touches) so this module doesn't need to import any one -// consumer's concrete track schema. -export interface CriteriaPlaylistDetailedLike { - uuid: string; - name: string; - trackPlaylistRelations: { track: T; position: number }[]; -} - export default class TrackListOrigin { constructor( public type: TrackListOriginType, @@ -39,8 +30,8 @@ export class TrackListOriginFromTrack extends T } } -export class TrackListOriginFromCriteriaPlaylist extends TrackListOrigin { - constructor(public criteriaPlaylist: CriteriaPlaylistDetailedLike, scope: Scope) { +export class TrackListOriginFromCriteriaPlaylist extends TrackListOrigin { + constructor(public criteriaPlaylist: CriteriaPlaylistMinimum, scope: Scope) { super(TrackListOriginType.GENRE_PLAYLIST, criteriaPlaylist.name, criteriaPlaylist.uuid, scope); } } diff --git a/packages/app-kit/src/genre-tree/playlist-tree/TreePerRoot.test.tsx b/packages/app-kit/src/genre-tree/playlist-tree/TreePerRoot.test.tsx index efe0421..4c4aa17 100644 --- a/packages/app-kit/src/genre-tree/playlist-tree/TreePerRoot.test.tsx +++ b/packages/app-kit/src/genre-tree/playlist-tree/TreePerRoot.test.tsx @@ -1,6 +1,5 @@ import { render } from "@testing-library/react"; import { describe, expect, it, vi, beforeEach } from "vitest"; -import { z } from "zod"; import GenrePlaylistTreePerRoot from "./TreePerRoot"; import { TrackListOriginType } from "../models/TrackListOriginType"; @@ -12,7 +11,6 @@ const { setIsPlaying, playNewTrackListFromGenrePlaylist, updateGenreMutate, - fetchGenrePlaylistDetailed, handleGenreCreationAction, handleGenreRenameAction, showPopup, @@ -20,7 +18,6 @@ const { setIsPlaying: vi.fn(), playNewTrackListFromGenrePlaylist: vi.fn(), updateGenreMutate: vi.fn(), - fetchGenrePlaylistDetailed: vi.fn(), handleGenreCreationAction: vi.fn(), handleGenreRenameAction: vi.fn(), showPopup: vi.fn(), @@ -43,10 +40,6 @@ vi.mock("../useGenre", () => ({ useUpdateGenre: () => ({ mutate: updateGenreMutate }), })); -vi.mock("../useGenrePlaylist", () => ({ - useFetchGenrePlaylistDetailed: () => ({ mutate: fetchGenrePlaylistDetailed }), -})); - vi.mock("../../player/PlayerContext", () => ({ usePlayer: () => ({ isPlaying, setIsPlaying }), })); @@ -90,7 +83,6 @@ function renderTree(nodes: CriteriaPlaylistSimple[] = [genrePlaylist]) { handleGenreCreationAction={handleGenreCreationAction} handleGenreRenameAction={handleGenreRenameAction} getBackendBaseUrl={() => "https://api.example.com"} - criteriaPlaylistDetailedSchema={z.any()} />, ); } @@ -112,34 +104,29 @@ describe("GenrePlaylistTreePerRoot", () => { capturedProps!.onPlayPause!(playlistUuid); expect(setIsPlaying).toHaveBeenCalledWith(false); - expect(fetchGenrePlaylistDetailed).not.toHaveBeenCalled(); + expect(playNewTrackListFromGenrePlaylist).not.toHaveBeenCalled(); }); - it("fetches and plays the genre playlist when it isn't already playing", () => { + it("plays the genre playlist when it isn't already playing", () => { + playNewTrackListFromGenrePlaylist.mockResolvedValue(undefined); renderTree(); capturedProps!.onPlayPause!(playlistUuid); - expect(fetchGenrePlaylistDetailed).toHaveBeenCalledWith( - playlistUuid, - expect.objectContaining({ onSuccess: expect.any(Function), onError: expect.any(Function) }), - ); - - const detailedPlaylist = { uuid: playlistUuid }; - const { onSuccess } = fetchGenrePlaylistDetailed.mock.calls[0][1]; - onSuccess(detailedPlaylist); - expect(playNewTrackListFromGenrePlaylist).toHaveBeenCalledWith(detailedPlaylist, "reference"); + expect(playNewTrackListFromGenrePlaylist).toHaveBeenCalledWith(genrePlaylist, "reference"); }); - it("logs an error when fetching the detailed genre playlist fails", () => { + it("logs an error and shows a popup when loading the tracks fails", async () => { const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + const error = new Error("network error"); + playNewTrackListFromGenrePlaylist.mockRejectedValueOnce(error); renderTree(); capturedProps!.onPlayPause!(playlistUuid); - const { onError } = fetchGenrePlaylistDetailed.mock.calls[0][1]; - onError(new Error("network error")); - expect(consoleErrorSpy).toHaveBeenCalledWith("Failed to fetch detailed genre playlist:", expect.any(Error)); + await vi.waitFor(() => + expect(consoleErrorSpy).toHaveBeenCalledWith("Failed to load genre playlist tracks:", error), + ); expect(showPopup).toHaveBeenCalledWith( expect.objectContaining({ props: expect.objectContaining({ errorCode: ErrorCode.CLIENT_UNKNOWN }) }), ); @@ -152,7 +139,7 @@ describe("GenrePlaylistTreePerRoot", () => { capturedProps!.onPlayPause!(playlistUuid); - expect(fetchGenrePlaylistDetailed).not.toHaveBeenCalled(); + expect(playNewTrackListFromGenrePlaylist).not.toHaveBeenCalled(); }); it("does nothing when the node has no matching genre-playlist", () => { @@ -160,7 +147,7 @@ describe("GenrePlaylistTreePerRoot", () => { capturedProps!.onPlayPause!("unknown-node-id"); - expect(fetchGenrePlaylistDetailed).not.toHaveBeenCalled(); + expect(playNewTrackListFromGenrePlaylist).not.toHaveBeenCalled(); expect(setIsPlaying).not.toHaveBeenCalled(); }); }); @@ -238,7 +225,6 @@ describe("GenrePlaylistTreePerRoot", () => { handleGenreCreationAction={handleGenreCreationAction} handleGenreRenameAction={handleGenreRenameAction} getBackendBaseUrl={() => "https://api.example.com"} - criteriaPlaylistDetailedSchema={z.any()} readOnly />, ); @@ -274,7 +260,6 @@ describe("GenrePlaylistTreePerRoot", () => { handleGenreCreationAction={handleGenreCreationAction} handleGenreRenameAction={handleGenreRenameAction} getBackendBaseUrl={() => "https://api.example.com"} - criteriaPlaylistDetailedSchema={z.any()} />, ); @@ -296,7 +281,6 @@ describe("GenrePlaylistTreePerRoot", () => { handleGenreCreationAction={handleGenreCreationAction} handleGenreRenameAction={handleGenreRenameAction} getBackendBaseUrl={() => "https://api.example.com"} - criteriaPlaylistDetailedSchema={z.any()} />, ); diff --git a/packages/app-kit/src/genre-tree/playlist-tree/TreePerRoot.tsx b/packages/app-kit/src/genre-tree/playlist-tree/TreePerRoot.tsx index 6266627..0c72df0 100644 --- a/packages/app-kit/src/genre-tree/playlist-tree/TreePerRoot.tsx +++ b/packages/app-kit/src/genre-tree/playlist-tree/TreePerRoot.tsx @@ -1,7 +1,6 @@ "use client"; import { useCallback, useMemo, type ReactNode } from "react"; -import { z } from "zod"; import { GenreTree, getGenreTreeColor, @@ -11,14 +10,12 @@ import { import { useTrackList } from "../TrackListContext"; import { useUpdateGenre } from "../useGenre"; -import { useFetchGenrePlaylistDetailed } from "../useGenrePlaylist"; import { usePlayer } from "../../player/PlayerContext"; import { InternalErrorPopup, usePopup } from "../../popup"; import { ErrorCode } from "../../transport/app-errors/app-error-codes"; import { TrackListOriginType } from "../models/TrackListOriginType"; import { TrackBase } from "../schemas/track/base"; -import { CriteriaPlaylistDetailedLike } from "../models/TrackListOrigin"; import { CriteriaPlaylistSimple } from "../schemas/criteria-playlist/simple"; import { CriteriaMinimum } from "../schemas/criteria/minimum"; @@ -34,7 +31,6 @@ export type GenrePlaylistTreePerRootProps = { handleGenreCreationAction: (parent: CriteriaMinimum | null) => void; handleGenreRenameAction: (genre: CriteriaMinimum) => void; getBackendBaseUrl: () => string; - criteriaPlaylistDetailedSchema: z.ZodType>; additionalActions?: (node: GenreTreeNode) => GenreTreeAction[]; onNodeClick?: (node: GenreTreeNode) => void; onNodeHover?: (node: GenreTreeNode) => void; @@ -57,7 +53,6 @@ export default function GenrePlaylistTreePerRoot({ handleGenreCreationAction, handleGenreRenameAction, getBackendBaseUrl, - criteriaPlaylistDetailedSchema, additionalActions, onNodeClick, onNodeHover, @@ -69,11 +64,6 @@ export default function GenrePlaylistTreePerRoot({ const { isPlaying, setIsPlaying } = usePlayer(); const { trackList, playNewTrackListFromGenrePlaylist } = useTrackList(); const { mutate: updateGenreMutate } = useUpdateGenre(scope, getBackendBaseUrl); - const { mutate: fetchGenrePlaylistDetailed } = useFetchGenrePlaylistDetailed( - scope, - getBackendBaseUrl, - criteriaPlaylistDetailedSchema, - ); const { showPopup } = usePopup(); const nodes: GenreTreeNode[] = useMemo( @@ -110,14 +100,9 @@ export default function GenrePlaylistTreePerRoot({ return; } - fetchGenrePlaylistDetailed(genrePlaylist.uuid, { - onSuccess: (detailedPlaylist) => { - playNewTrackListFromGenrePlaylist(detailedPlaylist, scope); - }, - onError: (error) => { - console.error("Failed to fetch detailed genre playlist:", error); - showPopup(); - }, + playNewTrackListFromGenrePlaylist(genrePlaylist, scope).catch((error) => { + console.error("Failed to load genre playlist tracks:", error); + showPopup(); }); }, [ @@ -126,7 +111,6 @@ export default function GenrePlaylistTreePerRoot({ isPlaying, setIsPlaying, playNewTrackListFromGenrePlaylist, - fetchGenrePlaylistDetailed, scope, showPopup, ], diff --git a/packages/app-kit/src/genre-tree/playlist-tree/TreeWheel.test.tsx b/packages/app-kit/src/genre-tree/playlist-tree/TreeWheel.test.tsx index b540079..0c003bd 100644 --- a/packages/app-kit/src/genre-tree/playlist-tree/TreeWheel.test.tsx +++ b/packages/app-kit/src/genre-tree/playlist-tree/TreeWheel.test.tsx @@ -1,20 +1,19 @@ import { describe, it, expect, beforeEach, vi } from "vitest"; import { render } from "@testing-library/react"; -import { z } from "zod"; const { genreTreeWheelPropsMock, usePlayerMock, useTrackListMock, updateGenreMutateMock, - fetchGenrePlaylistDetailedMutateMock, + playGenreMock, showPopupMock, } = vi.hoisted(() => ({ genreTreeWheelPropsMock: vi.fn(), usePlayerMock: vi.fn(), useTrackListMock: vi.fn(), updateGenreMutateMock: vi.fn(), - fetchGenrePlaylistDetailedMutateMock: vi.fn(), + playGenreMock: vi.fn(), showPopupMock: vi.fn(), })); @@ -38,21 +37,15 @@ vi.mock("../useGenre", () => ({ useUpdateGenre: () => ({ mutate: updateGenreMutateMock }), })); -vi.mock("../useGenrePlaylist", () => ({ - useFetchGenrePlaylistDetailed: () => ({ mutate: fetchGenrePlaylistDetailedMutateMock }), -})); - vi.mock("../../player/PlayerContext", () => ({ usePlayer: () => usePlayerMock(), })); import GenrePlaylistTreeWheel, { type GenrePlaylistTreeWheelProps } from "./TreeWheel"; import type { TrackBase } from "../schemas/track/base"; -import type { CriteriaPlaylistDetailedLike } from "../models/TrackListOrigin"; import { ErrorCode } from "../../transport/app-errors/app-error-codes"; const getBackendBaseUrl = () => "https://backend.example.com"; -const schema = z.custom>(); function makeGenrePlaylist(overrides: Record = {}) { return { @@ -77,7 +70,6 @@ function renderWheel(overrides: Partial> handleGenreCreationAction: vi.fn(), handleGenreRenameAction: vi.fn(), getBackendBaseUrl, - criteriaPlaylistDetailedSchema: schema, ...overrides, }; render(); @@ -88,7 +80,8 @@ describe("GenrePlaylistTreeWheel", () => { beforeEach(() => { vi.clearAllMocks(); usePlayerMock.mockReturnValue({ isPlaying: false, setIsPlaying: vi.fn() }); - useTrackListMock.mockReturnValue({ trackList: null, playNewTrackListFromGenrePlaylist: vi.fn() }); + useTrackListMock.mockReturnValue({ trackList: null, playNewTrackListFromGenrePlaylist: playGenreMock }); + playGenreMock.mockResolvedValue(undefined); }); it("maps genre playlists to tree nodes", () => { @@ -144,7 +137,7 @@ describe("GenrePlaylistTreeWheel", () => { genreTreeWheelPropsMock.mock.calls[0][0].onPlayPause("missing"); expect(setIsPlaying).not.toHaveBeenCalled(); - expect(fetchGenrePlaylistDetailedMutateMock).not.toHaveBeenCalled(); + expect(playGenreMock).not.toHaveBeenCalled(); }); it("toggles isPlaying when the playlist is already the current track list", () => { @@ -159,7 +152,7 @@ describe("GenrePlaylistTreeWheel", () => { genreTreeWheelPropsMock.mock.calls[0][0].onPlayPause("gp1"); expect(setIsPlaying).toHaveBeenCalledWith(true); - expect(fetchGenrePlaylistDetailedMutateMock).not.toHaveBeenCalled(); + expect(playGenreMock).not.toHaveBeenCalled(); }); it("does nothing when the playlist has no tracks", () => { @@ -167,37 +160,26 @@ describe("GenrePlaylistTreeWheel", () => { genreTreeWheelPropsMock.mock.calls[0][0].onPlayPause("gp1"); - expect(fetchGenrePlaylistDetailedMutateMock).not.toHaveBeenCalled(); + expect(playGenreMock).not.toHaveBeenCalled(); }); - it("fetches the detailed playlist and plays it on success", () => { - const playNewTrackListFromGenrePlaylist = vi.fn(); - useTrackListMock.mockReturnValue({ trackList: null, playNewTrackListFromGenrePlaylist }); + it("plays the node's genre playlist", () => { renderWheel(); genreTreeWheelPropsMock.mock.calls[0][0].onPlayPause("gp1"); - expect(fetchGenrePlaylistDetailedMutateMock).toHaveBeenCalledWith( - "gp1", - expect.objectContaining({ onSuccess: expect.any(Function), onError: expect.any(Function) }), - ); - - const { onSuccess } = fetchGenrePlaylistDetailedMutateMock.mock.calls[0][1]; - const detailedPlaylist = { uuid: "gp1", name: "Jazz", trackPlaylistRelations: [] }; - onSuccess(detailedPlaylist); - - expect(playNewTrackListFromGenrePlaylist).toHaveBeenCalledWith(detailedPlaylist, "me"); + expect(playGenreMock).toHaveBeenCalledWith(expect.objectContaining({ uuid: "gp1" }), "me"); }); - it("logs an error via onError", () => { + it("logs an error when loading the tracks fails", async () => { const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + const error = new Error("boom"); + playGenreMock.mockRejectedValueOnce(error); renderWheel(); genreTreeWheelPropsMock.mock.calls[0][0].onPlayPause("gp1"); - const { onError } = fetchGenrePlaylistDetailedMutateMock.mock.calls[0][1]; - onError(new Error("boom")); - expect(errorSpy).toHaveBeenCalledWith("Failed to fetch detailed genre playlist:", expect.any(Error)); + await vi.waitFor(() => expect(errorSpy).toHaveBeenCalledWith("Failed to load genre playlist tracks:", error)); expect(showPopupMock).toHaveBeenCalledWith( expect.objectContaining({ props: expect.objectContaining({ errorCode: ErrorCode.CLIENT_UNKNOWN }) }), ); diff --git a/packages/app-kit/src/genre-tree/playlist-tree/TreeWheel.tsx b/packages/app-kit/src/genre-tree/playlist-tree/TreeWheel.tsx index 35d723c..9ecb256 100644 --- a/packages/app-kit/src/genre-tree/playlist-tree/TreeWheel.tsx +++ b/packages/app-kit/src/genre-tree/playlist-tree/TreeWheel.tsx @@ -1,7 +1,6 @@ "use client"; import { useCallback, useMemo, type ReactNode } from "react"; -import { z } from "zod"; import { GenreTreeWheel, type GenreTreeAction, @@ -10,14 +9,12 @@ import { import { useTrackList } from "../TrackListContext"; import { useUpdateGenre } from "../useGenre"; -import { useFetchGenrePlaylistDetailed } from "../useGenrePlaylist"; import { usePlayer } from "../../player/PlayerContext"; import { InternalErrorPopup, usePopup } from "../../popup"; import { ErrorCode } from "../../transport/app-errors/app-error-codes"; import { TrackListOriginType } from "../models/TrackListOriginType"; import { TrackBase } from "../schemas/track/base"; -import { CriteriaPlaylistDetailedLike } from "../models/TrackListOrigin"; import { CriteriaPlaylistSimple } from "../schemas/criteria-playlist/simple"; import { CriteriaMinimum } from "../schemas/criteria/minimum"; @@ -32,7 +29,6 @@ export type GenrePlaylistTreeWheelProps = { handleGenreCreationAction: (parent: CriteriaMinimum | null) => void; handleGenreRenameAction: (genre: CriteriaMinimum) => void; getBackendBaseUrl: () => string; - criteriaPlaylistDetailedSchema: z.ZodType>; additionalActions?: (node: GenreTreeNode) => GenreTreeAction[]; onNodeClick?: (node: GenreTreeNode) => void; onNodeHover?: (node: GenreTreeNode) => void; @@ -57,7 +53,6 @@ export default function GenrePlaylistTreeWheel({ handleGenreCreationAction, handleGenreRenameAction, getBackendBaseUrl, - criteriaPlaylistDetailedSchema, additionalActions, onNodeClick, onNodeHover, @@ -70,11 +65,6 @@ export default function GenrePlaylistTreeWheel({ const { isPlaying, setIsPlaying } = usePlayer(); const { trackList, playNewTrackListFromGenrePlaylist } = useTrackList(); const { mutate: updateGenreMutate } = useUpdateGenre(scope, getBackendBaseUrl); - const { mutate: fetchGenrePlaylistDetailed } = useFetchGenrePlaylistDetailed( - scope, - getBackendBaseUrl, - criteriaPlaylistDetailedSchema, - ); const { showPopup } = usePopup(); const nodes: GenreTreeNode[] = useMemo( @@ -111,14 +101,9 @@ export default function GenrePlaylistTreeWheel({ return; } - fetchGenrePlaylistDetailed(genrePlaylist.uuid, { - onSuccess: (detailedPlaylist) => { - playNewTrackListFromGenrePlaylist(detailedPlaylist, scope); - }, - onError: (error) => { - console.error("Failed to fetch detailed genre playlist:", error); - showPopup(); - }, + playNewTrackListFromGenrePlaylist(genrePlaylist, scope).catch((error) => { + console.error("Failed to load genre playlist tracks:", error); + showPopup(); }); }, [ @@ -127,7 +112,6 @@ export default function GenrePlaylistTreeWheel({ isPlaying, setIsPlaying, playNewTrackListFromGenrePlaylist, - fetchGenrePlaylistDetailed, scope, showPopup, ], diff --git a/packages/app-kit/src/genre-tree/playlist-tree/TreeWheelRadialPopCore.test.tsx b/packages/app-kit/src/genre-tree/playlist-tree/TreeWheelRadialPopCore.test.tsx index 2cdcf53..4a8f41e 100644 --- a/packages/app-kit/src/genre-tree/playlist-tree/TreeWheelRadialPopCore.test.tsx +++ b/packages/app-kit/src/genre-tree/playlist-tree/TreeWheelRadialPopCore.test.tsx @@ -1,6 +1,5 @@ import { describe, it, expect, beforeEach, vi } from "vitest"; import { render } from "@testing-library/react"; -import { z } from "zod"; const { genreTreeWheelRadialPopCorePropsMock, @@ -8,14 +7,14 @@ const { usePlayerMock, useTrackListMock, updateGenreMutateMock, - fetchGenrePlaylistDetailedMutateMock, + playGenreMock, } = vi.hoisted(() => ({ genreTreeWheelRadialPopCorePropsMock: vi.fn(), genreTreeOutlinePropsMock: vi.fn(), usePlayerMock: vi.fn(), useTrackListMock: vi.fn(), updateGenreMutateMock: vi.fn(), - fetchGenrePlaylistDetailedMutateMock: vi.fn(), + playGenreMock: vi.fn(), })); vi.mock("@behindthemusictree/genre-tree-view", () => ({ @@ -37,10 +36,6 @@ vi.mock("../useGenre", () => ({ useUpdateGenre: () => ({ mutate: updateGenreMutateMock }), })); -vi.mock("../useGenrePlaylist", () => ({ - useFetchGenrePlaylistDetailed: () => ({ mutate: fetchGenrePlaylistDetailedMutateMock }), -})); - vi.mock("../../player/PlayerContext", () => ({ usePlayer: () => usePlayerMock(), })); @@ -49,10 +44,8 @@ import GenrePlaylistTreeWheelRadialPopCore, { type GenrePlaylistTreeWheelRadialPopCoreProps, } from "./TreeWheelRadialPopCore"; import type { TrackBase } from "../schemas/track/base"; -import type { CriteriaPlaylistDetailedLike } from "../models/TrackListOrigin"; const getBackendBaseUrl = () => "https://backend.example.com"; -const schema = z.custom>(); function makeGenrePlaylist(overrides: Record = {}) { return { @@ -79,7 +72,6 @@ function renderWheelRadialPopCore(overrides: Partial); @@ -90,7 +82,8 @@ describe("GenrePlaylistTreeWheelRadialPopCore", () => { beforeEach(() => { vi.clearAllMocks(); usePlayerMock.mockReturnValue({ isPlaying: false, setIsPlaying: vi.fn() }); - useTrackListMock.mockReturnValue({ trackList: null, playNewTrackListFromGenrePlaylist: vi.fn() }); + useTrackListMock.mockReturnValue({ trackList: null, playNewTrackListFromGenrePlaylist: playGenreMock }); + playGenreMock.mockResolvedValue(undefined); }); describe("outline", () => { @@ -172,7 +165,7 @@ describe("GenrePlaylistTreeWheelRadialPopCore", () => { genreTreeWheelRadialPopCorePropsMock.mock.calls[0][0].onPlayPause("missing"); expect(setIsPlaying).not.toHaveBeenCalled(); - expect(fetchGenrePlaylistDetailedMutateMock).not.toHaveBeenCalled(); + expect(playGenreMock).not.toHaveBeenCalled(); }); it("toggles isPlaying when the playlist is already the current track list", () => { @@ -187,7 +180,7 @@ describe("GenrePlaylistTreeWheelRadialPopCore", () => { genreTreeWheelRadialPopCorePropsMock.mock.calls[0][0].onPlayPause("gp1"); expect(setIsPlaying).toHaveBeenCalledWith(true); - expect(fetchGenrePlaylistDetailedMutateMock).not.toHaveBeenCalled(); + expect(playGenreMock).not.toHaveBeenCalled(); }); it("does nothing when the playlist has no tracks", () => { @@ -195,37 +188,26 @@ describe("GenrePlaylistTreeWheelRadialPopCore", () => { genreTreeWheelRadialPopCorePropsMock.mock.calls[0][0].onPlayPause("gp1"); - expect(fetchGenrePlaylistDetailedMutateMock).not.toHaveBeenCalled(); + expect(playGenreMock).not.toHaveBeenCalled(); }); - it("fetches the detailed playlist and plays it on success", () => { - const playNewTrackListFromGenrePlaylist = vi.fn(); - useTrackListMock.mockReturnValue({ trackList: null, playNewTrackListFromGenrePlaylist }); + it("plays the node's genre playlist", () => { renderWheelRadialPopCore(); genreTreeWheelRadialPopCorePropsMock.mock.calls[0][0].onPlayPause("gp1"); - expect(fetchGenrePlaylistDetailedMutateMock).toHaveBeenCalledWith( - "gp1", - expect.objectContaining({ onSuccess: expect.any(Function), onError: expect.any(Function) }), - ); - - const { onSuccess } = fetchGenrePlaylistDetailedMutateMock.mock.calls[0][1]; - const detailedPlaylist = { uuid: "gp1", name: "Jazz", trackPlaylistRelations: [] }; - onSuccess(detailedPlaylist); - - expect(playNewTrackListFromGenrePlaylist).toHaveBeenCalledWith(detailedPlaylist, "me"); + expect(playGenreMock).toHaveBeenCalledWith(expect.objectContaining({ uuid: "gp1" }), "me"); }); - it("logs an error via onError", () => { + it("logs an error when loading the tracks fails", async () => { const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + const error = new Error("boom"); + playGenreMock.mockRejectedValueOnce(error); renderWheelRadialPopCore(); genreTreeWheelRadialPopCorePropsMock.mock.calls[0][0].onPlayPause("gp1"); - const { onError } = fetchGenrePlaylistDetailedMutateMock.mock.calls[0][1]; - onError(new Error("boom")); - expect(errorSpy).toHaveBeenCalledWith("Failed to fetch detailed genre playlist:", expect.any(Error)); + await vi.waitFor(() => expect(errorSpy).toHaveBeenCalledWith("Failed to load genre playlist tracks:", error)); errorSpy.mockRestore(); }); }); diff --git a/packages/app-kit/src/genre-tree/playlist-tree/TreeWheelRadialPopCore.tsx b/packages/app-kit/src/genre-tree/playlist-tree/TreeWheelRadialPopCore.tsx index a017ac6..32cb357 100644 --- a/packages/app-kit/src/genre-tree/playlist-tree/TreeWheelRadialPopCore.tsx +++ b/packages/app-kit/src/genre-tree/playlist-tree/TreeWheelRadialPopCore.tsx @@ -1,7 +1,6 @@ "use client"; import { useCallback, useMemo, type ReactNode } from "react"; -import { z } from "zod"; import { GenreTreeOutline, GenreTreeWheelRadialPopCore, @@ -11,12 +10,10 @@ import { import { useTrackList } from "../TrackListContext"; import { useUpdateGenre } from "../useGenre"; -import { useFetchGenrePlaylistDetailed } from "../useGenrePlaylist"; import { usePlayer } from "../../player/PlayerContext"; import { TrackListOriginType } from "../models/TrackListOriginType"; import { TrackBase } from "../schemas/track/base"; -import { CriteriaPlaylistDetailedLike } from "../models/TrackListOrigin"; import { CriteriaPlaylistSimple } from "../schemas/criteria-playlist/simple"; import { CriteriaMinimum } from "../schemas/criteria/minimum"; @@ -31,7 +28,6 @@ export type GenrePlaylistTreeWheelRadialPopCoreProps = { handleGenreCreationAction: (parent: CriteriaMinimum | null) => void; handleGenreRenameAction: (genre: CriteriaMinimum) => void; getBackendBaseUrl: () => string; - criteriaPlaylistDetailedSchema: z.ZodType>; additionalActions?: (node: GenreTreeNode) => GenreTreeAction[]; onNodeClick?: (node: GenreTreeNode) => void; onNodeHover?: (node: GenreTreeNode) => void; @@ -59,7 +55,6 @@ export default function GenrePlaylistTreeWheelRadialPopCore handleGenreCreationAction, handleGenreRenameAction, getBackendBaseUrl, - criteriaPlaylistDetailedSchema, additionalActions, onNodeClick, onNodeHover, @@ -73,11 +68,6 @@ export default function GenrePlaylistTreeWheelRadialPopCore const { isPlaying, setIsPlaying } = usePlayer(); const { trackList, playNewTrackListFromGenrePlaylist } = useTrackList(); const { mutate: updateGenreMutate } = useUpdateGenre(scope, getBackendBaseUrl); - const { mutate: fetchGenrePlaylistDetailed } = useFetchGenrePlaylistDetailed( - scope, - getBackendBaseUrl, - criteriaPlaylistDetailedSchema, - ); const nodes: GenreTreeNode[] = useMemo( () => @@ -113,16 +103,11 @@ export default function GenrePlaylistTreeWheelRadialPopCore return; } - fetchGenrePlaylistDetailed(genrePlaylist.uuid, { - onSuccess: (detailedPlaylist) => { - playNewTrackListFromGenrePlaylist(detailedPlaylist, scope); - }, - onError: (error) => { - console.error("Failed to fetch detailed genre playlist:", error); - }, + playNewTrackListFromGenrePlaylist(genrePlaylist, scope).catch((error) => { + console.error("Failed to load genre playlist tracks:", error); }); }, - [genrePlaylists, trackList, isPlaying, setIsPlaying, playNewTrackListFromGenrePlaylist, fetchGenrePlaylistDetailed, scope], + [genrePlaylists, trackList, isPlaying, setIsPlaying, playNewTrackListFromGenrePlaylist, scope], ); const handleAddChild = useCallback( diff --git a/packages/app-kit/src/genre-tree/schemas/criteria-playlist/criteria-playlist.test.ts b/packages/app-kit/src/genre-tree/schemas/criteria-playlist/criteria-playlist.test.ts index 98497a9..b0c56a8 100644 --- a/packages/app-kit/src/genre-tree/schemas/criteria-playlist/criteria-playlist.test.ts +++ b/packages/app-kit/src/genre-tree/schemas/criteria-playlist/criteria-playlist.test.ts @@ -1,12 +1,8 @@ import { describe, it, expect } from "vitest"; -import { z } from "zod"; -import { CriteriaPlaylistDetailedBaseSchema, makeCriteriaPlaylistDetailedSchema } from "./detailed"; +import { CriteriaPlaylistDetailedSchema } from "./detailed"; import { CriteriaPlaylistSimpleSchema } from "./simple"; -const trackSchema = z.object({ uuid: z.string().uuid() }); -const CriteriaPlaylistDetailedSchema = makeCriteriaPlaylistDetailedSchema(trackSchema); - const uuid = "b1e6a1c8-0e3d-4d3d-9d2e-2f6c1a2b3c4d"; const validCriteriaPlaylistBase = { @@ -22,38 +18,20 @@ const validCriteriaPlaylistBase = { updatedOn: null, }; -describe("CriteriaPlaylistDetailedBaseSchema", () => { +describe("CriteriaPlaylistDetailedSchema", () => { it("parses a valid base shape", () => { - expect(() => CriteriaPlaylistDetailedBaseSchema.parse(validCriteriaPlaylistBase)).not.toThrow(); + expect(() => CriteriaPlaylistDetailedSchema.parse(validCriteriaPlaylistBase)).not.toThrow(); }); it("rejects a shape missing a required field", () => { const { name: _name, ...invalid } = validCriteriaPlaylistBase; - expect(() => CriteriaPlaylistDetailedBaseSchema.parse(invalid)).toThrow(); + expect(() => CriteriaPlaylistDetailedSchema.parse(invalid)).toThrow(); }); it("accepts null/omitted duration fields", () => { const { durationInSec: _durationInSec, durationStrInHourMinSec: _durationStrInHourMinSec, ...rest } = validCriteriaPlaylistBase; - expect(() => CriteriaPlaylistDetailedBaseSchema.parse(rest)).not.toThrow(); - }); -}); - -describe("makeCriteriaPlaylistDetailedSchema", () => { - it("parses a valid detailed shape with trackPlaylistRelations", () => { - const valid = { - ...validCriteriaPlaylistBase, - trackPlaylistRelations: [{ track: { uuid }, position: 0 }], - }; - expect(() => CriteriaPlaylistDetailedSchema.parse(valid)).not.toThrow(); - }); - - it("rejects a shape with an invalid trackPlaylistRelations entry", () => { - const invalid = { - ...validCriteriaPlaylistBase, - trackPlaylistRelations: [{ track: { uuid }, position: -1 }], - }; - expect(() => CriteriaPlaylistDetailedSchema.parse(invalid)).toThrow(); + expect(() => CriteriaPlaylistDetailedSchema.parse(rest)).not.toThrow(); }); }); diff --git a/packages/app-kit/src/genre-tree/schemas/criteria-playlist/detailed.ts b/packages/app-kit/src/genre-tree/schemas/criteria-playlist/detailed.ts index 0c26134..dd11f54 100644 --- a/packages/app-kit/src/genre-tree/schemas/criteria-playlist/detailed.ts +++ b/packages/app-kit/src/genre-tree/schemas/criteria-playlist/detailed.ts @@ -1,14 +1,11 @@ import { z } from "zod"; import { UuidResourceSchema } from "../uuid-resource"; -import { makeTrackPlaylistRelSchema } from "../track-playlist-rel/without-playlist"; import { CriteriaMinimumSchema } from "../criteria/minimum"; import { CriteriaPlaylistMinimumSchema } from "./minimum"; -// Fields shared by every criteria playlist shape, regardless of track kind. `trackPlaylistRelations` -// is deliberately excluded here — it's added by `makeCriteriaPlaylistDetailedSchema` below, since -// its track shape varies per consumer. -export const CriteriaPlaylistDetailedBaseSchema = UuidResourceSchema.extend({ +// Metadata only — tracks are paged separately via `genrePlaylistEndpoints.*.tracks(uuid)`. +export const CriteriaPlaylistDetailedSchema = UuidResourceSchema.extend({ name: z.string(), tracksCount: z.number(), durationInSec: z.number().min(0).nullable().optional(), @@ -21,11 +18,4 @@ export const CriteriaPlaylistDetailedBaseSchema = UuidResourceSchema.extend({ updatedOn: z.string().nullable(), }); -export const makeCriteriaPlaylistDetailedSchema = (trackSchema: T) => - CriteriaPlaylistDetailedBaseSchema.extend({ - trackPlaylistRelations: z.array(makeTrackPlaylistRelSchema(trackSchema)), - }); - -export type CriteriaPlaylistDetailed = z.infer< - ReturnType> ->; +export type CriteriaPlaylistDetailed = z.infer; diff --git a/packages/app-kit/src/genre-tree/schemas/criteria-playlist/simple.ts b/packages/app-kit/src/genre-tree/schemas/criteria-playlist/simple.ts index dcb399d..67dbf35 100644 --- a/packages/app-kit/src/genre-tree/schemas/criteria-playlist/simple.ts +++ b/packages/app-kit/src/genre-tree/schemas/criteria-playlist/simple.ts @@ -1,7 +1,7 @@ import { z } from "zod"; -import { CriteriaPlaylistDetailedBaseSchema } from "./detailed"; +import { CriteriaPlaylistDetailedSchema } from "./detailed"; -export const CriteriaPlaylistSimpleSchema = CriteriaPlaylistDetailedBaseSchema.pick({ +export const CriteriaPlaylistSimpleSchema = CriteriaPlaylistDetailedSchema.pick({ uuid: true, name: true, criteria: true, diff --git a/packages/app-kit/src/genre-tree/schemas/track-playlist-rel/without-playlist.test.ts b/packages/app-kit/src/genre-tree/schemas/track-playlist-rel/without-playlist.test.ts index ecd09c1..a8a6f82 100644 --- a/packages/app-kit/src/genre-tree/schemas/track-playlist-rel/without-playlist.test.ts +++ b/packages/app-kit/src/genre-tree/schemas/track-playlist-rel/without-playlist.test.ts @@ -1,7 +1,7 @@ import { describe, it, expect } from "vitest"; import { z } from "zod"; -import { makeTrackPlaylistRelSchema } from "./without-playlist"; +import { makeTrackPlaylistRelPageSchema, makeTrackPlaylistRelSchema } from "./without-playlist"; const trackSchema = z.object({ uuid: z.string().uuid() }); const TrackPlaylistRelSchema = makeTrackPlaylistRelSchema(trackSchema); @@ -17,3 +17,18 @@ describe("makeTrackPlaylistRelSchema", () => { expect(() => TrackPlaylistRelSchema.parse(invalid)).toThrow(); }); }); + +describe("makeTrackPlaylistRelPageSchema", () => { + it("parses a paginated page of track/position pairs", () => { + const page = { + overallTotal: 1, + next: null, + previous: null, + results: [{ track: { uuid: "b1e6a1c8-0e3d-4d3d-9d2e-2f6c1a2b3c4d" }, position: 1 }], + page: 1, + pageSize: 100, + totalPages: 1, + }; + expect(makeTrackPlaylistRelPageSchema(trackSchema).parse(page).results[0].position).toBe(1); + }); +}); diff --git a/packages/app-kit/src/genre-tree/schemas/track-playlist-rel/without-playlist.ts b/packages/app-kit/src/genre-tree/schemas/track-playlist-rel/without-playlist.ts index d362487..92c9452 100644 --- a/packages/app-kit/src/genre-tree/schemas/track-playlist-rel/without-playlist.ts +++ b/packages/app-kit/src/genre-tree/schemas/track-playlist-rel/without-playlist.ts @@ -1,5 +1,7 @@ import { z } from "zod"; +import { PaginatedResponseSchema } from "../../../transport/lib/paginated-response"; + export const makeTrackPlaylistRelSchema = (trackSchema: T) => z.object({ track: trackSchema, @@ -7,3 +9,6 @@ export const makeTrackPlaylistRelSchema = (trackSchema: }); export type TrackPlaylistRel = z.infer>>; + +export const makeTrackPlaylistRelPageSchema = (trackSchema: T) => + PaginatedResponseSchema(makeTrackPlaylistRelSchema(trackSchema)); diff --git a/packages/app-kit/src/genre-tree/schemas/track/base.ts b/packages/app-kit/src/genre-tree/schemas/track/base.ts index 4c2395f..df85edb 100644 --- a/packages/app-kit/src/genre-tree/schemas/track/base.ts +++ b/packages/app-kit/src/genre-tree/schemas/track/base.ts @@ -3,7 +3,6 @@ import { z } from "zod"; import { ArtistMinimumSchema } from "../artist-minimum"; import { AlbumMinimumSchema } from "../album-minimum"; import { CriteriaMinimumSchema } from "../criteria/minimum"; -import { CriteriaPlaylistMinimumSchema } from "../criteria-playlist/minimum"; import { UuidResourceSchema } from "../uuid-resource"; // Fields shared by every track kind (uploaded, youtube). Playback-specific fields @@ -16,7 +15,6 @@ export const TrackBaseSchema = UuidResourceSchema.extend({ genre: CriteriaMinimumSchema, rating: z.number().min(0).max(10).nullable().optional(), language: z.string().nullable().optional(), - playlists: z.array(CriteriaPlaylistMinimumSchema), playCount: z.number().min(0), createdOn: z.string().datetime(), updatedOn: z.string().datetime().nullable().optional(), diff --git a/packages/app-kit/src/genre-tree/track-list-sidebar/TrackListSidebar.test.tsx b/packages/app-kit/src/genre-tree/track-list-sidebar/TrackListSidebar.test.tsx index 99fbcde..c916b91 100644 --- a/packages/app-kit/src/genre-tree/track-list-sidebar/TrackListSidebar.test.tsx +++ b/packages/app-kit/src/genre-tree/track-list-sidebar/TrackListSidebar.test.tsx @@ -1,4 +1,4 @@ -import { describe, it, expect, vi, beforeEach } from "vitest"; +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; import { render, screen, fireEvent } from "@testing-library/react"; const { useTrackListMock, useTrackListSidebarVisibilityMock } = vi.hoisted(() => ({ @@ -24,6 +24,8 @@ function makeTrackList(overrides: Record = {}) { { uuid: "t1", title: "Song One" }, { uuid: "t2", title: "Song Two" }, ], + total: 2, + nextPage: null, ...overrides, }; } @@ -68,6 +70,7 @@ describe("TrackListSidebar", () => { trackList: makeTrackList({ origin: { label: "Single Track", type: TrackListOriginType.TRACK }, tracks: [{ uuid: "t1", title: "Only Song" }], + total: 1, }), }); @@ -77,6 +80,78 @@ describe("TrackListSidebar", () => { expect(screen.getByText(/1 track /)).toBeInTheDocument(); }); + it("shows the playlist total rather than the loaded count", () => { + useTrackListMock.mockReturnValue({ trackList: makeTrackList({ total: 250 }) }); + + render(); + + expect(screen.getByText(/250 tracks/)).toBeInTheDocument(); + }); + + describe("load-more sentinel", () => { + let observerCallback: IntersectionObserverCallback; + const observe = vi.fn(); + const disconnect = vi.fn(); + + beforeEach(() => { + vi.stubGlobal( + "IntersectionObserver", + vi.fn(function (this: unknown, callback: IntersectionObserverCallback) { + observerCallback = callback; + return { observe, disconnect }; + }), + ); + }); + + afterEach(() => { + vi.unstubAllGlobals(); + }); + + const intersect = (isIntersecting: boolean) => + observerCallback([{ isIntersecting } as IntersectionObserverEntry], {} as IntersectionObserver); + + it("does not observe when there is no next page", () => { + useTrackListMock.mockReturnValue({ trackList: makeTrackList(), loadMore: vi.fn() }); + + render(); + + expect(observe).not.toHaveBeenCalled(); + }); + + it("calls loadMore when the sentinel scrolls into view, and disconnects on unmount", () => { + const loadMore = vi.fn().mockResolvedValue(undefined); + useTrackListMock.mockReturnValue({ trackList: makeTrackList({ nextPage: 2 }), loadMore }); + + const { unmount } = render(); + expect(observe).toHaveBeenCalledTimes(1); + + intersect(false); + expect(loadMore).not.toHaveBeenCalled(); + intersect(true); + expect(loadMore).toHaveBeenCalledTimes(1); + + unmount(); + expect(disconnect).toHaveBeenCalled(); + }); + + it("logs when loadMore rejects", async () => { + const error = new Error("boom"); + const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + useTrackListMock.mockReturnValue({ + trackList: makeTrackList({ nextPage: 2 }), + loadMore: vi.fn().mockRejectedValue(error), + }); + + render(); + intersect(true); + + await vi.waitFor(() => + expect(consoleErrorSpy).toHaveBeenCalledWith("Failed to load more genre playlist tracks:", error), + ); + consoleErrorSpy.mockRestore(); + }); + }); + it("calls hideTrackListSidebar when the close control is clicked", () => { useTrackListMock.mockReturnValue({ trackList: makeTrackList() }); diff --git a/packages/app-kit/src/genre-tree/track-list-sidebar/TrackListSidebar.tsx b/packages/app-kit/src/genre-tree/track-list-sidebar/TrackListSidebar.tsx index f2b412f..72c3a65 100644 --- a/packages/app-kit/src/genre-tree/track-list-sidebar/TrackListSidebar.tsx +++ b/packages/app-kit/src/genre-tree/track-list-sidebar/TrackListSidebar.tsx @@ -1,6 +1,6 @@ "use client"; -import { ReactNode } from "react"; +import { ReactNode, useEffect, useRef } from "react"; import TrackItem from "./TrackItem"; import { useTrackList } from "../TrackListContext"; @@ -21,7 +21,21 @@ export default function TrackListSidebar({ renderActions, layout = "fixed", }: TrackListSidebarProps) { - const { trackList } = useTrackList(); + const { trackList, loadMore } = useTrackList(); + const loadMoreSentinelRef = useRef(null); + const hasMore = !!trackList && trackList.nextPage !== null; + + useEffect(() => { + const sentinel = loadMoreSentinelRef.current; + if (!hasMore || !sentinel) return; + const observer = new IntersectionObserver((entries) => { + if (entries.some((entry) => entry.isIntersecting)) { + loadMore().catch((error) => console.error("Failed to load more genre playlist tracks:", error)); + } + }); + observer.observe(sentinel); + return () => observer.disconnect(); + }, [hasMore, loadMore]); const { hideTrackListSidebar } = useTrackListSidebarVisibility(); const positionClasses = @@ -45,7 +59,7 @@ export default function TrackListSidebar({ {trackList && trackList.origin.type === TrackListOriginType.GENRE_PLAYLIST ? "• Genre playlist • " : "• track playlist • "} - {trackList.tracks.length + " track" + (trackList.tracks.length > 1 ? "s •" : " •")} + {trackList.total + " track" + (trackList.total > 1 ? "s •" : " •")}
({ ))} + {hasMore &&
) : null; diff --git a/packages/app-kit/src/genre-tree/useGenrePlaylist.test.ts b/packages/app-kit/src/genre-tree/useGenrePlaylist.test.ts index 862c0b2..ce5caa3 100644 --- a/packages/app-kit/src/genre-tree/useGenrePlaylist.test.ts +++ b/packages/app-kit/src/genre-tree/useGenrePlaylist.test.ts @@ -1,14 +1,12 @@ import { describe, it, expect, beforeEach, vi } from "vitest"; import { renderHook } from "@testing-library/react"; -const { fetchMock, useSessionMock, useQueryWithParseMock, useMutationMock, invalidateQueriesMock, parseWithLogMock } = +const { fetchMock, useSessionMock, useQueryWithParseMock, invalidateQueriesMock } = vi.hoisted(() => ({ fetchMock: vi.fn(), useSessionMock: vi.fn(), useQueryWithParseMock: vi.fn(), - useMutationMock: vi.fn(), invalidateQueriesMock: vi.fn(), - parseWithLogMock: vi.fn(), })); vi.mock("@tanstack/react-query", async (importOriginal) => { @@ -16,7 +14,6 @@ vi.mock("@tanstack/react-query", async (importOriginal) => { return { ...actual, useQueryClient: () => ({ invalidateQueries: invalidateQueriesMock }), - useMutation: (options: unknown) => useMutationMock(options), }; }); @@ -32,15 +29,10 @@ vi.mock("../transport/lib/use-query-with-parse", () => ({ useQueryWithParse: (options: unknown) => useQueryWithParseMock(options), })); -vi.mock("../transport/lib/parse-with-log", () => ({ - parseWithLog: (...args: unknown[]) => parseWithLogMock(...args), -})); - import { useListGenrePlaylists, useListFullGenrePlaylists, useFetchGenrePlaylist, - useFetchGenrePlaylistDetailed, useInvalidateAllGenrePlaylistQueries, } from "./useGenrePlaylist"; import { CriteriaPlaylistSimpleSchema } from "./schemas/criteria-playlist/simple"; @@ -209,35 +201,6 @@ describe("useGenrePlaylist", () => { }); }); - describe("useFetchGenrePlaylistDetailed", () => { - it("mutationFn fetches the reference detail endpoint and parses the response", async () => { - fetchMock.mockResolvedValue({ uuid: "gp1" }); - parseWithLogMock.mockReturnValue({ uuid: "gp1" }); - renderHook(() => useFetchGenrePlaylistDetailed("reference", getBackendBaseUrl, CriteriaPlaylistSimpleSchema)); - const { mutationFn } = useMutationMock.mock.calls[0][0]; - - const result = await mutationFn("gp1"); - - expect(fetchMock).toHaveBeenCalledWith("genre-playlists/gp1/", true, false); - expect(parseWithLogMock).toHaveBeenCalledWith( - CriteriaPlaylistSimpleSchema, - { uuid: "gp1" }, - "useFetchGenrePlaylistDetailed", - ); - expect(result).toEqual({ uuid: "gp1" }); - }); - - it("mutationFn fetches the me detail endpoint", async () => { - fetchMock.mockResolvedValue({ uuid: "gp1" }); - renderHook(() => useFetchGenrePlaylistDetailed("me", getBackendBaseUrl, CriteriaPlaylistSimpleSchema)); - const { mutationFn } = useMutationMock.mock.calls[0][0]; - - await mutationFn("gp1"); - - expect(fetchMock).toHaveBeenCalledWith("me/genre-playlists/gp1/", true, true); - }); - }); - describe("useInvalidateAllGenrePlaylistQueries", () => { it("returns a function that invalidates the me and reference genre playlist query keys", () => { const { result } = renderHook(() => useInvalidateAllGenrePlaylistQueries()); diff --git a/packages/app-kit/src/genre-tree/useGenrePlaylist.ts b/packages/app-kit/src/genre-tree/useGenrePlaylist.ts index 26d0180..e0100a1 100644 --- a/packages/app-kit/src/genre-tree/useGenrePlaylist.ts +++ b/packages/app-kit/src/genre-tree/useGenrePlaylist.ts @@ -1,9 +1,8 @@ "use client"; import { z } from "zod"; -import { useQueryClient, useMutation } from "@tanstack/react-query"; +import { useQueryClient } from "@tanstack/react-query"; import { useFetchWrapper } from "../transport/useFetchWrapper"; -import { parseWithLog } from "../transport/lib/parse-with-log"; import { useQueryWithParse } from "../transport/lib/use-query-with-parse"; import { useSession } from "../auth/SessionContext"; @@ -118,27 +117,6 @@ export const useFetchGenrePlaylist = ( }); }; -export const useFetchGenrePlaylistDetailed = ( - scope: Scope, - getBackendBaseUrl: () => string, - criteriaPlaylistDetailedSchema: S, -) => { - const { fetch } = useFetchWrapper(getBackendBaseUrl); - - return useMutation, Error, string>({ - mutationFn: async (uuid: string) => { - const endpoint = - scope === "reference" ? genrePlaylistEndpoints.reference.detail(uuid) : genrePlaylistEndpoints.me.detail(uuid); - const response = await fetch(endpoint, true, scope === "me"); - return parseWithLog( - criteriaPlaylistDetailedSchema, - response, - "useFetchGenrePlaylistDetailed", - ) as z.infer; - }, - }); -}; - export const useInvalidateAllGenrePlaylistQueries = () => { const queryClient = useQueryClient(); return () => { From dfc6f187e7ea5fbe70f61b4226f6061aeaf1a5a7 Mon Sep 17 00:00:00 2001 From: mignot Date: Tue, 29 Sep 2026 00:20:30 +0200 Subject: [PATCH 2/2] fix(genre-tree): drop superseded play results and stale page refetches Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + .../src/genre-tree/TrackListContext.test.tsx | 58 +++++++++++++++++++ .../src/genre-tree/TrackListContext.tsx | 19 ++++-- 3 files changed, 74 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 910c896..756105a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ easy to spot when bumping. - **genre-tree**: `GenreTreeView` drops the `criteriaPlaylistDetailedSchema` prop and its track generic; tracks are parsed with `TrackListProvider`'s `schema`. - **genre-tree**: `CriteriaPlaylistDetailedSchema` is metadata-only (no `trackPlaylistRelations`). `makeCriteriaPlaylistDetailedSchema`, `CriteriaPlaylistDetailedLike` and `useFetchGenrePlaylistDetailed` are removed. - **genre-tree**: `TrackBaseSchema` no longer has `playlists`. +- **genre-tree**: `TrackListSidebar` uses `IntersectionObserver` when the list has more pages; consumer tests rendering it under jsdom must stub it. ### Added diff --git a/packages/app-kit/src/genre-tree/TrackListContext.test.tsx b/packages/app-kit/src/genre-tree/TrackListContext.test.tsx index 189d4bf..70a6d6a 100644 --- a/packages/app-kit/src/genre-tree/TrackListContext.test.tsx +++ b/packages/app-kit/src/genre-tree/TrackListContext.test.tsx @@ -204,6 +204,46 @@ describe("TrackListContext", () => { expect(loadTrackForPlayerMock).toHaveBeenCalledTimes(1); }); + it("drops a response that arrives after a single track started playing", async () => { + const { result } = renderHook(() => useTrackList(), { wrapper }); + const slow = deferred(); + fetchMock.mockReturnValueOnce(slow.promise); + let genrePlay!: Promise; + act(() => { + genrePlay = result.current.playNewTrackListFromGenrePlaylist(genrePlaylist, "me"); + }); + + const single = makeTrack("x", "X"); + act(() => { + result.current.playNewTrackListFromTrackUuid(single, "me"); + }); + await act(async () => { + slow.resolve(makePage(makeTracks(1))); + await genrePlay; + }); + + expect(result.current.trackList?.tracks).toEqual([single]); + expect(loadTrackForPlayerMock).toHaveBeenCalledTimes(1); + }); + + it("swallows the failure of a request superseded by a newer play", async () => { + const { result } = renderHook(() => useTrackList(), { wrapper }); + const slow = deferred(); + fetchMock.mockReturnValueOnce(slow.promise); + let first!: Promise; + act(() => { + first = result.current.playNewTrackListFromGenrePlaylist({ uuid: "old", name: "Old" }, "me"); + }); + + await playGenre(result, makePage(makeTracks(1, "new"))); + await act(async () => { + slow.reject(new Error("offline")); + await expect(first).resolves.toBeUndefined(); + }); + + expect(result.current.selectedTrack?.uuid).toBe("new0"); + }); + it("rejects when the page fails schema validation", async () => { const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); const { result } = renderHook(() => useTrackList(), { wrapper }); @@ -284,6 +324,24 @@ describe("TrackListContext", () => { expect(result.current.trackList?.tracks).toHaveLength(21); }); + it("ignores a page refetched by a stale loadMore after it was already appended", async () => { + const { result } = renderHook(() => useTrackList(), { wrapper }); + await playGenre(result, makePage(makeTracks(20), { next: true })); + const staleLoadMore = result.current.loadMore; + fetchMock.mockResolvedValueOnce(makePage(makeTracks(1, "u"), { page: 2, next: true })); + await act(async () => { + await result.current.loadMore(); + }); + + fetchMock.mockResolvedValueOnce(makePage(makeTracks(1, "v"), { page: 2 })); + await act(async () => { + await staleLoadMore(); + }); + + expect(result.current.trackList?.tracks).toHaveLength(21); + expect(result.current.trackList?.nextPage).toBe(3); + }); + it("discards a page that lands after a different list started playing", async () => { const { result } = renderHook(() => useTrackList(), { wrapper }); await playGenre(result, makePage(makeTracks(20), { next: true })); diff --git a/packages/app-kit/src/genre-tree/TrackListContext.tsx b/packages/app-kit/src/genre-tree/TrackListContext.tsx index b42946d..ccc78c5 100644 --- a/packages/app-kit/src/genre-tree/TrackListContext.tsx +++ b/packages/app-kit/src/genre-tree/TrackListContext.tsx @@ -161,6 +161,7 @@ export function TrackListProvider({ const playNewTrackListFromTrackUuid = useCallback( (track: T, scope: Scope) => { + latestPlayRequestRef.current++; const origin = new TrackListOriginFromTrack(track, scope); const newTrackList = new TrackListFromTrack([track], origin); @@ -175,8 +176,15 @@ export function TrackListProvider({ const playNewTrackListFromGenrePlaylist = useCallback( async (genrePlaylist: CriteriaPlaylistMinimum, scope: Scope) => { const request = ++latestPlayRequestRef.current; - const { tracks, total, nextPage } = await fetchTracksPage(genrePlaylist.uuid, scope, 1); + let page: Awaited>; + try { + page = await fetchTracksPage(genrePlaylist.uuid, scope, 1); + } catch (error) { + if (request !== latestPlayRequestRef.current) return; + throw error; + } if (request !== latestPlayRequestRef.current) return; + const { tracks, total, nextPage } = page; if (tracks.length === 0) { console.warn("No tracks found in genre playlist"); @@ -197,12 +205,15 @@ export function TrackListProvider({ const origin = trackList.origin as TrackListOriginFromCriteriaPlaylist; if (loadingOriginRef.current === origin) return; + const requestedPage = trackList.nextPage; loadingOriginRef.current = origin; try { - const page = await fetchTracksPage(origin.uuid, origin.scope, trackList.nextPage); + const page = await fetchTracksPage(origin.uuid, origin.scope, requestedPage); setTrackList((prev) => { - if (prev?.origin !== origin) return prev; - // Positions can shift between page fetches if the playlist changes; never list a track twice. + // A stale loadMore closure can refetch a page that was already appended. + if (prev?.origin !== origin || prev.nextPage !== requestedPage) return prev; + // ponytail: page-number paging skips a track when an earlier one is removed server-side + // between fetches; switch to cursor paging if that matters. Insertions only duplicate, filtered here. const loaded = new Set(prev.tracks.map((track) => track.uuid)); const newTracks = page.tracks.filter((track) => !loaded.has(track.uuid)); return new TrackListFromCriteriaPlaylist([...prev.tracks, ...newTracks], origin, page.total, page.nextPage);