Move on to the next episode through the SyncPlay group

packages/app/src/App/App.js

@@ -38,7 +38,7 @@ import {AchievementsProvider, useAchievements} from '../context/AchievementsCont
 import {achievementIconPath} from '../views/Settings/achievements/achievementIcons';
 import {threadIsPhoto} from '../utils/achievementsModel';
 import {ServerMessagesProvider, useServerMessages} from '../context/ServerMessagesContext';
-import {SyncPlayProvider, useSyncPlay} from '../context/SyncPlayContext';
+import {SyncPlayProvider, followsQueueUpdate, useSyncPlay} from '../context/SyncPlayContext';
 import {useVersionCheck} from '../hooks/useVersionCheck';
 import UpdateNotification from '../components/UpdateNotification';
 import {ItemMenuProvider} from '../components/ItemContextMenu';
@@ -168,7 +168,7 @@ const AppContent = (props) => {
 	const {streamNotification, dismissStreamNotification, pluginInfo} = useSeerr();
 	const {pendingPopups, markPopupsRead} = useServerMessages();
 	const themeMusic = useThemeMusic();
-	const {openDialog: openSyncPlay, closeDialog: closeSyncPlay, isDialogOpen: syncPlayDialogOpen, playQueueUpdate: syncPlayQueueUpdate, isInGroup: isSyncPlayInGroup, setNewQueue: syncPlaySetNewQueue, getGroupPositionTicks: getSyncPlayPositionTicks} = useSyncPlay();
+	const {openDialog: openSyncPlay, closeDialog: closeSyncPlay, isDialogOpen: syncPlayDialogOpen, playQueueUpdate: syncPlayQueueUpdate, isInGroup: isSyncPlayInGroup, setNewQueue: syncPlaySetNewQueue, moveGroupTo: syncPlayMoveGroupTo, getGroupPositionTicks: getSyncPlayPositionTicks} = useSyncPlay();
 	const handledSyncPlayQueueRef = useRef(null);
 
 	const [remoteMessage, setRemoteMessage] = useState(null);
@@ -994,7 +994,7 @@ const AppContent = (props) => {
 		const update = syncPlayQueueUpdate;
 		if (!update || update === handledSyncPlayQueueRef.current) return;
 		handledSyncPlayQueueRef.current = update;
-		if (!update.startsPlayback && playingItem?.Id === update.item.Id) return;
+		if (!followsQueueUpdate(update, playingItem?.Id)) return;
 		playSyncPlayItem(update.item);
 	}, [syncPlayQueueUpdate, playingItem, playSyncPlayItem]);
 
@@ -1010,6 +1010,11 @@ const AppContent = (props) => {
 	// one the viewer picked by hand from the episode browser passes `resume` so an episode they
 	// were part way through carries on from where it stopped.
 	const handlePlayNext = useCallback((item, options) => {
+		// In a group the group moves on, and its queue update brings this player along.
+		if (isSyncPlayInGroup && item.MediaType !== 'Audio' && item.Type !== 'TvChannel') {
+			syncPlayMoveGroupTo(item.Id);
+			return;
+		}
 		setPlayingItem(item);
 		setPlaybackOptions(prev => {
 			if (prev?.audioPlaylist?.some(t => t.Id === item.Id)) {
@@ -1024,7 +1029,7 @@ const AppContent = (props) => {
 			return null;
 		});
 		setIsResume(options?.resume === true);
-	}, []);
+	}, [isSyncPlayInGroup, syncPlayMoveGroupTo]);
 
 	const handlePlayerEnd = useCallback(() => {
 		// Episodes played on from inside the player (next episode, the episode browser) leave the

packages/app/src/context/SyncPlayContext.js

@@ -7,7 +7,11 @@ import {api} from '../services/jellyfinApi';
 const SyncPlayContext = createContext(null);
 
 // Queue entries carry the GUID with dashes, item DTOs without them.
-const sameItemId = (a, b) => !!a && !!b && String(a).replace(/-/g, '').toLowerCase() === String(b).replace(/-/g, '').toLowerCase();
+const sameItemId = (a, b) => !!a && !!b && syncPlayService.itemIdKey(a) === syncPlayService.itemIdKey(b);
+
+// Only a new item, or one other than what's playing, moves the player. With nothing playing it stays put.
+export const followsQueueUpdate = (update, playingItemId) =>
+	update.startsPlayback || (Boolean(playingItemId) && !sameItemId(playingItemId, update.item.Id));
 
 export const useSyncPlay = () => useContext(SyncPlayContext);
 
@@ -156,6 +160,17 @@ export const SyncPlayProvider = ({children}) => {
 	// time since.
 	const getGroupPositionTicks = useCallback(() => syncPlayService.getGroupPositionTicks(), []);
 
+	// Moves the whole group to an item, by next or previous when it's the neighbouring entry.
+	const moveGroupTo = useCallback((itemId) => {
+		const entries = playQueue?.Playlist || [];
+		const index = playQueue?.PlayingItemIndex ?? -1;
+		const entry = entries.find((e) => sameItemId(e.ItemId, itemId));
+		if (!entry) syncPlayService.setNewQueue([itemId]);
+		else if (index >= 0 && entry === entries[index + 1]) syncPlayService.nextItem();
+		else if (index > 0 && entry === entries[index - 1]) syncPlayService.previousItem();
+		else syncPlayService.setPlaylistItem(entry.PlaylistItemId);
+	}, [playQueue]);
+
 	const openDialog = useCallback(() => {
 		if (settings.syncplayEnabled === false) return;
 		setIsDialogOpen(true);
@@ -177,6 +192,7 @@ export const SyncPlayProvider = ({children}) => {
 		playQueueUpdate,
 		refreshGroups,
 		getGroupPositionTicks,
+		moveGroupTo,
 		getGroup: syncPlayService.getGroup,
 		createGroup: handleCreateGroup,
 		joinGroup: handleJoinGroup,

packages/app/src/context/SyncPlayContext.test.js

@@ -0,0 +1,88 @@
+import {act, renderHook} from '@testing-library/react';
+
+import {SyncPlayProvider, followsQueueUpdate, useSyncPlay} from './SyncPlayContext';
+
+// The CLI ships a second copy of React, so the components' JSX goes through the copy under test.
+jest.mock('react/jsx-dev-runtime', () => {
+	const React = require('react');
+	return {
+		jsxDEV: (type, {children, ...props}, key) => {
+			const config = key === undefined ? props : {...props, key};
+			return children === undefined ? React.createElement(type, config) : React.createElement(type, config, children);
+		}
+	};
+});
+
+let mockEmit;
+const mockQueueRequests = {nextItem: jest.fn(), previousItem: jest.fn(), setPlaylistItem: jest.fn(), setNewQueue: jest.fn()};
+jest.mock('../services/syncPlay', () => ({
+	itemIdKey: jest.requireActual('../services/syncPlay').itemIdKey,
+	start: () => {},
+	stop: () => {},
+	addListener: (listener) => {
+		mockEmit = listener;
+		return () => {};
+	},
+	nextItem: (...args) => mockQueueRequests.nextItem(...args),
+	previousItem: (...args) => mockQueueRequests.previousItem(...args),
+	setPlaylistItem: (...args) => mockQueueRequests.setPlaylistItem(...args),
+	setNewQueue: (...args) => mockQueueRequests.setNewQueue(...args)
+}));
+jest.mock('./AuthContext', () => ({useAuth: () => ({isAuthenticated: true, serverType: 'jellyfin'})}));
+jest.mock('./SettingsContext', () => ({useSettings: () => ({settings: {}})}));
+jest.mock('../services/jellyfinApi', () => ({api: {getItem: () => new Promise(() => {})}}));
+
+const renderSyncPlay = () => renderHook(() => useSyncPlay(), {wrapper: SyncPlayProvider});
+
+describe('followsQueueUpdate', () => {
+	const update = (startsPlayback) => ({item: {Id: 'aaaaaaaa-0000-0000-0000-000000000001'}, startsPlayback});
+
+	test('plays a new item whatever this device is doing', () => {
+		expect(followsQueueUpdate(update(true), null)).toBe(true);
+		expect(followsQueueUpdate(update(true), 'aaaaaaaa000000000000000000000001')).toBe(true);
+	});
+
+	test('a reorder or a removal leaves this device alone when nothing is playing', () => {
+		expect(followsQueueUpdate(update(false), null)).toBe(false);
+		expect(followsQueueUpdate(update(false), undefined)).toBe(false);
+	});
+
+	test('nor when the player already shows that item, however its id is spelled', () => {
+		expect(followsQueueUpdate(update(false), 'AAAAAAAA000000000000000000000001')).toBe(false);
+	});
+
+	test('but moves a player that shows something else', () => {
+		expect(followsQueueUpdate(update(false), 'bbbbbbbb000000000000000000000002')).toBe(true);
+	});
+});
+
+describe('moveGroupTo', () => {
+	const entry = (n) => ({PlaylistItemId: `p${n}`, ItemId: `aaaaaaaa00000000000000000000000${n}`});
+	const queue = (playing) => ({PlayingItemIndex: playing, Playlist: [entry(1), entry(2), entry(3), entry(4)]});
+	const inGroupAt = (playing) => {
+		const {result} = renderSyncPlay();
+		act(() => mockEmit('playQueue', queue(playing)));
+		return result;
+	};
+
+	beforeEach(() => Object.values(mockQueueRequests).forEach((fn) => fn.mockReset()));
+
+	test('asks the group for its next or previous entry, however the id is spelled', () => {
+		const result = inGroupAt(1);
+		result.current.moveGroupTo('aaaaaaaa-0000-0000-0000-000000000003');
+		expect(mockQueueRequests.nextItem).toHaveBeenCalledTimes(1);
+		result.current.moveGroupTo('aaaaaaaa000000000000000000000001');
+		expect(mockQueueRequests.previousItem).toHaveBeenCalledTimes(1);
+	});
+
+	test('jumps to an entry further along', () => {
+		inGroupAt(1).current.moveGroupTo('aaaaaaaa000000000000000000000004');
+		expect(mockQueueRequests.setPlaylistItem).toHaveBeenCalledWith('p4');
+	});
+
+	test('starts a new queue for anything not in it, even from the last entry', () => {
+		inGroupAt(3).current.moveGroupTo('bbbbbbbb000000000000000000000009');
+		expect(mockQueueRequests.setNewQueue).toHaveBeenCalledWith(['bbbbbbbb000000000000000000000009']);
+		expect(mockQueueRequests.nextItem).not.toHaveBeenCalled();
+	});
+});

packages/app/src/services/syncPlay.js

@@ -443,8 +443,17 @@ const handleGroupUpdate = (data) => {
 	}
 };
 
+// Ids come with and without dashes.
+export const itemIdKey = (id) => String(id || '').replace(/-/g, '').toLowerCase();
+
 const handlePlaybackCommand = (data) => {
 	if (!data) return;
+	// A command for another queue entry would land on the wrong item. An all-zero id names none.
+	const commandItem = itemIdKey(data.PlaylistItemId);
+	if (/[1-9a-f]/.test(commandItem) && currentPlaylistItemId && commandItem !== itemIdKey(currentPlaylistItemId)) {
+		syncLog('[SyncPlay] dropped', data.Command, 'for item', data.PlaylistItemId);
+		return;
+	}
 	syncLog('[SyncPlay] command', data.Command, 'at', data.PositionTicks != null ? data.PositionTicks / 10000000 : null, 's, when', data.When, 'delay', getDelayToWhen(data.When), 'ms'); // eslint-disable-line no-use-before-define
 	// A command says where the group was at its own time, which for a late
 	// one or an echo is not now.

packages/app/src/services/syncPlay.test.js

@@ -222,6 +222,17 @@ describe('SyncPlay group position', () => {
 		expect(service.getGroupPositionTicks()).toBe(frozen);
 	});
 
+	test('a command for another queue entry is dropped', () => {
+		const itemCommand = (PlaylistItemId) => socket.onmessage({
+			data: JSON.stringify({MessageType: 'SyncPlayCommand', Data: {Command: 'Seek', PositionTicks: 300 * SECOND, When: new Date().toISOString(), PlaylistItemId}})
+		});
+		groupUpdate('PlayQueue', {Playlist: [{PlaylistItemId: 'bbbb-0002'}], PlayingItemIndex: 0, StartPositionTicks: 0, IsPlaying: false});
+		itemCommand('aaaa-0001');
+		expect(service.getGroupPositionTicks()).toBe(0);
+		itemCommand('BBBB0002');
+		expect(service.getGroupPositionTicks()).toBe(300 * SECOND);
+	});
+
 	test('is forgotten on leaving', () => {
 		command('Unpause', 100 * SECOND, Date.now());
 		groupUpdate('GroupLeft', {GroupId: 'g1'});