From 4f83987840d1759c0972b780025d4aaa6be252b5 Mon Sep 17 00:00:00 2001 From: Deluan Date: Sun, 29 Jun 2025 11:35:10 -0400 Subject: [PATCH] fix(ui): keep the NowPlayingPanel badge in sync. Introduced a new event, EVENT_STREAM_RECONNECTED, to track the last timestamp of stream reconnections. This change updates the activity reducer to handle the new event and modifies the NowPlayingPanel to refresh data based on server and stream status. Signed-off-by: Deluan --- ui/src/actions/serverEvents.js | 6 + ui/src/eventStream.js | 4 +- ui/src/layout/NowPlayingPanel.jsx | 27 +++- ui/src/layout/NowPlayingPanel.test.jsx | 163 +++++++++++++++++++++--- ui/src/reducers/activityReducer.js | 4 + ui/src/reducers/activityReducer.test.js | 15 +++ 6 files changed, 197 insertions(+), 22 deletions(-) diff --git a/ui/src/actions/serverEvents.js b/ui/src/actions/serverEvents.js index d1e55283..99553455 100644 --- a/ui/src/actions/serverEvents.js +++ b/ui/src/actions/serverEvents.js @@ -2,6 +2,7 @@ export const EVENT_SCAN_STATUS = 'scanStatus' export const EVENT_SERVER_START = 'serverStart' export const EVENT_REFRESH_RESOURCE = 'refreshResource' export const EVENT_NOW_PLAYING_COUNT = 'nowPlayingCount' +export const EVENT_STREAM_RECONNECTED = 'streamReconnected' export const processEvent = (type, data) => ({ type, @@ -21,3 +22,8 @@ export const serverDown = () => ({ type: EVENT_SERVER_START, data: {}, }) + +export const streamReconnected = () => ({ + type: EVENT_STREAM_RECONNECTED, + data: {}, +}) diff --git a/ui/src/eventStream.js b/ui/src/eventStream.js index 3d8ddcd1..c91dae87 100644 --- a/ui/src/eventStream.js +++ b/ui/src/eventStream.js @@ -1,6 +1,6 @@ import { baseUrl } from './utils' import throttle from 'lodash.throttle' -import { processEvent, serverDown } from './actions' +import { processEvent, serverDown, streamReconnected } from './actions' import { REST_URL } from './consts' import config from './config' @@ -47,6 +47,8 @@ const connect = async (dispatchFn) => { const stream = await newEventStream() eventStream = stream setupHandlers(stream, dispatchFn) + // Dispatch reconnection event to refresh critical data + dispatchFn(streamReconnected()) return stream } catch (e) { // eslint-disable-next-line no-console diff --git a/ui/src/layout/NowPlayingPanel.jsx b/ui/src/layout/NowPlayingPanel.jsx index 7797c773..4aaee1be 100644 --- a/ui/src/layout/NowPlayingPanel.jsx +++ b/ui/src/layout/NowPlayingPanel.jsx @@ -245,6 +245,12 @@ NowPlayingList.propTypes = { const NowPlayingPanel = () => { const dispatch = useDispatch() const count = useSelector((state) => state.activity.nowPlayingCount) + const streamReconnected = useSelector( + (state) => state.activity.streamReconnected, + ) + const serverUp = useSelector( + (state) => !!state.activity.serverStart.startTime, + ) const translate = useTranslate() const notify = useNotify() const theme = useTheme() @@ -301,23 +307,32 @@ const NowPlayingPanel = () => { [dispatch, notify], ) - // Initialize count and entries on mount + // Initialize count and entries on mount, and refresh on server/stream changes useEffect(() => { - fetchList() - }, [fetchList]) + if (serverUp) fetchList() + }, [fetchList, serverUp, streamReconnected]) // Refresh when count changes from WebSocket events (if panel is open) useEffect(() => { - if (open) fetchList() - }, [count, open, fetchList]) + if (open && serverUp) fetchList() + }, [count, open, fetchList, serverUp]) + // Periodic refresh when panel is open (10 seconds) useInterval( () => { - if (open) fetchList() + if (open && serverUp) fetchList() }, open ? 10000 : null, ) + // Periodic refresh when panel is closed (60 seconds) to keep badge accurate + useInterval( + () => { + if (!open && serverUp) fetchList() + }, + !open ? 60000 : null, + ) + return (
diff --git a/ui/src/layout/NowPlayingPanel.test.jsx b/ui/src/layout/NowPlayingPanel.test.jsx index 6cc332fc..4dd5dac8 100644 --- a/ui/src/layout/NowPlayingPanel.test.jsx +++ b/ui/src/layout/NowPlayingPanel.test.jsx @@ -55,6 +55,21 @@ vi.mock('@material-ui/core/styles/useTheme', () => ({ })) describe('', () => { + const createMockStore = (overrides = {}) => { + const defaultState = { + activity: { + nowPlayingCount: 1, + serverStart: { startTime: Date.now() }, // Server is up by default + streamReconnected: 0, + ...overrides, + }, + } + return createStore( + combineReducers({ activity: activityReducer }), + defaultState, + ) + } + beforeEach(() => { vi.clearAllMocks() mockUseMediaQuery.mockReturnValue(false) // Default to large screen @@ -83,9 +98,7 @@ describe('', () => { }) it('fetches and displays entries when opened', async () => { - const store = createStore(combineReducers({ activity: activityReducer }), { - activity: { nowPlayingCount: 1 }, - }) + const store = createMockStore() render( @@ -108,9 +121,7 @@ describe('', () => { }) it('displays player name after username', async () => { - const store = createStore(combineReducers({ activity: activityReducer }), { - activity: { nowPlayingCount: 1 }, - }) + const store = createMockStore() render( @@ -152,9 +163,7 @@ describe('', () => { }, }) - const store = createStore(combineReducers({ activity: activityReducer }), { - activity: { nowPlayingCount: 1 }, - }) + const store = createMockStore() render( @@ -178,9 +187,7 @@ describe('', () => { 'subsonic-response': { status: 'ok', nowPlaying: { entry: [] } }, }, }) - const store = createStore(combineReducers({ activity: activityReducer }), { - activity: { nowPlayingCount: 0 }, - }) + const store = createMockStore({ nowPlayingCount: 0 }) render( @@ -201,9 +208,7 @@ describe('', () => { it('does not close panel when artist link is clicked on large screens', async () => { mockUseMediaQuery.mockReturnValue(false) // Simulate large screen - const store = createStore(combineReducers({ activity: activityReducer }), { - activity: { nowPlayingCount: 1 }, - }) + const store = createMockStore() render( @@ -231,4 +236,132 @@ describe('', () => { expect(screen.getByRole('presentation')).toBeInTheDocument() expect(screen.getByText('Artist')).toBeInTheDocument() }) + + it('does not fetch on mount when server is down', () => { + const store = createMockStore({ + nowPlayingCount: 1, + serverStart: { startTime: null }, // Server is down + }) + render( + + + , + ) + + // Should not have made initial fetch request due to server being down + expect(subsonic.getNowPlaying).not.toHaveBeenCalled() + }) + + it('does not fetch on stream reconnection when server is down', () => { + const store = createMockStore({ + nowPlayingCount: 1, + serverStart: { startTime: null }, // Server is down + streamReconnected: Date.now(), // Stream reconnected + }) + render( + + + , + ) + + // Should not have made fetch request due to server being down + expect(subsonic.getNowPlaying).not.toHaveBeenCalled() + }) + + it('does not double-fetch on server reconnection', () => { + const initialStore = createMockStore({ + nowPlayingCount: 1, + serverStart: { startTime: null }, // Server initially down + streamReconnected: 0, + }) + const { rerender } = render( + + + , + ) + + // Clear initial (empty) calls + vi.clearAllMocks() + + // Simulate server coming back up with stream reconnection (both state changes happen) + const reconnectedStore = createMockStore({ + nowPlayingCount: 1, + serverStart: { startTime: Date.now() }, // Server back up + streamReconnected: Date.now(), // Stream reconnected + }) + rerender( + + + , + ) + + // Should only make one call despite both serverUp and streamReconnected changing + expect(subsonic.getNowPlaying).toHaveBeenCalledTimes(1) + }) + + it('skips polling when server is down', () => { + vi.useFakeTimers() + + const store = createMockStore({ + nowPlayingCount: 1, + serverStart: { startTime: null }, // Server is down + }) + render( + + + , + ) + + // Clear initial mount fetch + vi.clearAllMocks() + + // Advance time by 70 seconds to trigger polling interval + vi.advanceTimersByTime(70000) + + // Should not have made any additional requests due to server being down + expect(subsonic.getNowPlaying).not.toHaveBeenCalled() + + vi.useRealTimers() + }) + + it('resumes polling when server comes back up', () => { + vi.useFakeTimers() + + const store = createMockStore({ + nowPlayingCount: 1, + serverStart: { startTime: null }, // Server is down + }) + const { rerender } = render( + + + , + ) + + // Clear initial mount fetch + vi.clearAllMocks() + + // Advance time - should not poll when server is down + vi.advanceTimersByTime(70000) + expect(subsonic.getNowPlaying).not.toHaveBeenCalled() + + // Update state to indicate server is back up + const updatedStore = createMockStore({ + nowPlayingCount: 1, + serverStart: { startTime: Date.now() }, // Server is back up + }) + rerender( + + + , + ) + + // Clear the fetch that happens due to initial mount of rerender + vi.clearAllMocks() + + // Advance time again - should now poll since server is up + vi.advanceTimersByTime(70000) + expect(subsonic.getNowPlaying).toHaveBeenCalled() + + vi.useRealTimers() + }) }) diff --git a/ui/src/reducers/activityReducer.js b/ui/src/reducers/activityReducer.js index 874ebb53..8238e395 100644 --- a/ui/src/reducers/activityReducer.js +++ b/ui/src/reducers/activityReducer.js @@ -3,6 +3,7 @@ import { EVENT_SCAN_STATUS, EVENT_SERVER_START, EVENT_NOW_PLAYING_COUNT, + EVENT_STREAM_RECONNECTED, } from '../actions' import config from '../config' @@ -16,6 +17,7 @@ const initialState = { }, serverStart: { version: config.version }, nowPlayingCount: 0, + streamReconnected: 0, // Timestamp of last reconnection } export const activityReducer = (previousState = initialState, payload) => { @@ -44,6 +46,8 @@ export const activityReducer = (previousState = initialState, payload) => { } case EVENT_NOW_PLAYING_COUNT: return { ...previousState, nowPlayingCount: data.count } + case EVENT_STREAM_RECONNECTED: + return { ...previousState, streamReconnected: Date.now() } default: return previousState } diff --git a/ui/src/reducers/activityReducer.test.js b/ui/src/reducers/activityReducer.test.js index 7c1d8b08..c9db38db 100644 --- a/ui/src/reducers/activityReducer.test.js +++ b/ui/src/reducers/activityReducer.test.js @@ -3,6 +3,7 @@ import { EVENT_SCAN_STATUS, EVENT_SERVER_START, EVENT_NOW_PLAYING_COUNT, + EVENT_STREAM_RECONNECTED, } from '../actions' import config from '../config' @@ -17,6 +18,7 @@ describe('activityReducer', () => { }, serverStart: { version: config.version }, nowPlayingCount: 0, + streamReconnected: 0, } it('returns the initial state when no action is specified', () => { @@ -130,4 +132,17 @@ describe('activityReducer', () => { const newState = activityReducer(initialState, action) expect(newState.nowPlayingCount).toEqual(5) }) + + it('handles EVENT_STREAM_RECONNECTED', () => { + const action = { + type: EVENT_STREAM_RECONNECTED, + data: {}, + } + const beforeTimestamp = Date.now() + const newState = activityReducer(initialState, action) + const afterTimestamp = Date.now() + + expect(newState.streamReconnected).toBeGreaterThanOrEqual(beforeTimestamp) + expect(newState.streamReconnected).toBeLessThanOrEqual(afterTimestamp) + }) })