feat: add to queue when pressing e

gmus-web/src/components/ui/cmus/reducer/fixtures.ts

@@ -32,3 +32,12 @@ export const stateWithActiveArtist: CmusUIState = {
     activeArtist: 'My artist',
   },
 };
+
+export const stateWithActiveSong: CmusUIState = {
+  ...stateLibrary,
+  library: {
+    ...stateLibrary.library,
+    modeWindow: LibraryModeWindow.SongList,
+    activeSongId: 1867,
+  },
+};

gmus-web/src/components/ui/cmus/reducer/keypress.spec.ts

@@ -1,10 +1,10 @@
-import { masterSet, playPaused, stateSet } from '../../../../actions';
+import { masterSet, playPaused, queuePushed, stateSet } from '../../../../actions';
 import { ActionKeyPressed, ActionTypeKeyPressed, Keys } from '../../../../hooks/vim';
 import { Song } from '../../../../types';
 
 import { CmusUIState, LibraryModeWindow, Overlay, View } from '../types';
 
-import { stateDifferentView, stateFromMode, stateLibrary } from './fixtures';
+import { stateDifferentView, stateFromMode, stateLibrary, stateWithActiveSong } from './fixtures';
 import { cmusUIReducer, initialCmusUIState } from './reducer';
 
 describe(ActionTypeKeyPressed, () => {
@@ -67,6 +67,18 @@ describe(ActionTypeKeyPressed, () => {
     });
   });
 
+  describe(Keys.E, () => {
+    const action: ActionKeyPressed = { type: ActionTypeKeyPressed, key: Keys.E };
+
+    describe('when in library view', () => {
+      it('should set global action to add the selected song to the queue', () => {
+        expect.assertions(1);
+        const result = cmusUIReducer(stateWithActiveSong, action);
+        expect(result.globalAction).toStrictEqual(queuePushed(1867));
+      });
+    });
+  });
+
   describe(Keys.J, () => {
     const action: ActionKeyPressed = { type: ActionTypeKeyPressed, key: Keys.J };
 

gmus-web/src/components/ui/cmus/reducer/keypress.ts

@@ -1,4 +1,4 @@
-import { masterSet, playPaused, stateSet } from '../../../../actions';
+import { masterSet, playPaused, queuePushed, stateSet } from '../../../../actions';
 import { ActionKeyPressed, Keys } from '../../../../hooks/vim';
 import { CmusUIState, LibraryModeWindow, Overlay, View } from '../types';
 import { handleScroll } from './scroll';
@@ -106,6 +106,16 @@ export function handleKeyPress(state: CmusUIState, action: ActionKeyPressed): Cm
     case Keys.C:
       return withGlobalAction(state, playPaused());
 
+    case Keys.E:
+      if (
+        state.view === View.Library &&
+        state.library.modeWindow === LibraryModeWindow.SongList &&
+        state.library.activeSongId
+      ) {
+        return withGlobalAction(state, queuePushed(state.library.activeSongId));
+      }
+      return state;
+
     case Keys.J:
       return handleScroll(state, 1);
     case Keys.K:

gmus-web/src/effects/effects.spec.ts

@@ -8,12 +8,11 @@ import {
   queuePushed,
   queueRemoved,
   queueShifted,
-  RemoteAction,
   seeked,
   songInfoFetched,
   stateSet,
 } from '../actions';
-import { globalReducer, GlobalState, initialState } from '../reducer';
+import { GlobalState, initialState } from '../reducer';
 import { Song } from '../types';
 import { MusicPlayer } from '../types/state';
 import { globalEffects } from './effects';
@@ -38,9 +37,8 @@ describe(globalEffects.name, () => {
       };
 
       const action = stateSet(localPlayer);
-      const nextState = globalReducer(prevState, action);
 
-      const result = globalEffects(prevState, action, nextState);
+      const result = globalEffects(prevState, action);
 
       expect(result).toStrictEqual<ActionStateSetRemote>({
         type: ActionTypeRemote.StateSet,
@@ -79,7 +77,7 @@ describe(globalEffects.name, () => {
       it('should create a remote state set action', () => {
         expect.assertions(1);
 
-        const result = globalEffects(state, action, globalReducer(state, action));
+        const result = globalEffects(state, action);
 
         expect(result).toStrictEqual<ActionStateSetRemote>({
           type: ActionTypeRemote.StateSet,
@@ -108,11 +106,7 @@ describe(globalEffects.name, () => {
 
     it('should return a StateSet action informing other clients that we are the new master', () => {
       expect.assertions(1);
-      const result = globalEffects(
-        stateMasterWentAway,
-        action,
-        globalReducer(stateMasterWentAway, action),
-      );
+      const result = globalEffects(stateMasterWentAway, action);
 
       expect(result).toStrictEqual<ActionStateSetRemote>({
         type: ActionTypeRemote.StateSet,
@@ -130,11 +124,7 @@ describe(globalEffects.name, () => {
     describe('when the action specified a particular client', () => {
       it('should return a StateSet action informing the new client to resume playback', () => {
         expect.assertions(1);
-        const result = globalEffects(
-          stateMasterWentAway,
-          masterSet('other-client'),
-          globalReducer(stateMasterWentAway, masterSet('other-client')),
-        );
+        const result = globalEffects(stateMasterWentAway, masterSet('other-client'));
 
         expect(result).toStrictEqual<ActionStateSetRemote>({
           type: ActionTypeRemote.StateSet,
@@ -170,9 +160,7 @@ describe(globalEffects.name, () => {
     describe('when the client is master', () => {
       it('should return null', () => {
         expect.assertions(1);
-        expect(
-          globalEffects(statePriorMaster, action, globalReducer(statePriorMaster, action)),
-        ).toBeNull();
+        expect(globalEffects(statePriorMaster, action)).toBeNull();
       });
     });
 
@@ -184,7 +172,7 @@ describe(globalEffects.name, () => {
 
       it('should return a StateSet action informing other clients of the updated playing state', () => {
         expect.assertions(1);
-        const result = globalEffects(stateSlave, action, globalReducer(stateSlave, action));
+        const result = globalEffects(stateSlave, action);
 
         expect(result).toStrictEqual<ActionStateSetRemote>({
           type: ActionTypeRemote.StateSet,
@@ -220,9 +208,7 @@ describe(globalEffects.name, () => {
     describe('when the client is master', () => {
       it('should return null', () => {
         expect.assertions(1);
-        expect(
-          globalEffects(statePriorMaster, action, globalReducer(statePriorMaster, action)),
-        ).toBeNull();
+        expect(globalEffects(statePriorMaster, action)).toBeNull();
       });
     });
 
@@ -234,7 +220,7 @@ describe(globalEffects.name, () => {
 
       it('should return a StateSet action informing other clients of the changed song', () => {
         expect.assertions(1);
-        const result = globalEffects(stateSlave, action, globalReducer(stateSlave, action));
+        const result = globalEffects(stateSlave, action);
 
         expect(result).toStrictEqual<ActionStateSetRemote>({
           type: ActionTypeRemote.StateSet,
@@ -254,47 +240,118 @@ describe(globalEffects.name, () => {
 
         it('should return null', () => {
           expect.assertions(1);
-          const result = globalEffects(
-            stateSlave,
-            actionNoReplace,
-            globalReducer(stateSlave, actionNoReplace),
-          );
+          const result = globalEffects(stateSlave, actionNoReplace);
           expect(result).toBeNull();
         });
       });
     });
   });
 
-  describe.each`
-    description                     | action
-    ${ActionTypeLocal.QueuePushed}  | ${queuePushed(123)}
-    ${ActionTypeLocal.QueueShifted} | ${queueShifted()}
-    ${ActionTypeLocal.QueueRemoved} | ${queueRemoved(84)}
-    ${ActionTypeLocal.QueueOrdered} | ${queueOrdered(17, -1)}
-  `('$description', ({ action }) => {
-    const statePrior: GlobalState = {
+  describe(ActionTypeLocal.QueuePushed, () => {
+    const action = queuePushed(184);
+
+    it('should add to the end of the queue', () => {
+      expect.assertions(1);
+      const result = globalEffects(
+        {
+          ...initialState,
+          player: { ...initialState.player, queue: [23] },
+        },
+        action,
+      );
+      expect(result).toStrictEqual<ActionStateSetRemote>({
+        type: ActionTypeRemote.StateSet,
+        payload: {
+          ...initialState.player,
+          queue: [23, 184],
+        },
+      });
+    });
+
+    describe('when the song is already in the queue', () => {
+      it('should not modify the queue', () => {
+        expect.assertions(1);
+        const result = globalEffects(
+          {
+            ...initialState,
+            player: { ...initialState.player, queue: [184, 23] },
+          },
+          action,
+        );
+        expect(result).toBeNull();
+      });
+    });
+  });
+
+  describe(ActionTypeLocal.QueueShifted, () => {
+    const action = queueShifted();
+    const stateWithQueue: GlobalState = {
       ...initialState,
-      player: {
-        songId: 123,
-        playing: true,
-        currentTime: 83,
-        seekTime: 5,
-        master: 'some-master-client',
-        queue: [13, 84, 17],
-      },
-      myClientName: 'some-other-client',
+      player: { ...initialState.player, queue: [8843, 23] },
     };
 
-    const nextState = globalReducer(statePrior, action);
+    it('should play the first song on the queue and remove it from the queue', () => {
+      expect.assertions(1);
+      const result = globalEffects(stateWithQueue, action);
+      expect(result).toStrictEqual<ActionStateSetRemote>({
+        type: ActionTypeRemote.StateSet,
+        payload: {
+          ...initialState.player,
+          playing: true,
+          songId: 8843,
+          currentTime: 0,
+          seekTime: 0,
+          queue: [23],
+        },
+      });
+    });
+  });
+
+  describe(ActionTypeLocal.QueueRemoved, () => {
+    const action = queueRemoved(84);
 
-    it('should issue a state set action to update the queue', () => {
+    it('should remove the given song ID from the queue', () => {
       expect.assertions(1);
-      const result = globalEffects(statePrior, action, nextState);
-      expect(result).toStrictEqual<RemoteAction>({
+      const result = globalEffects(
+        {
+          ...initialState,
+          player: { ...initialState.player, queue: [17, 84, 23] },
+        },
+        action,
+      );
+
+      expect(result).toStrictEqual<ActionStateSetRemote>({
+        type: ActionTypeRemote.StateSet,
+        payload: {
+          ...initialState.player,
+          queue: [17, 23],
+        },
+      });
+    });
+  });
+
+  describe(ActionTypeLocal.QueueOrdered, () => {
+    it.each`
+      direction      | delta | expectedResult
+      ${'forwards'}  | ${1}  | ${[17, 23, 84]}
+      ${'backwards'} | ${-1} | ${[84, 17, 23]}
+    `('should reorder ($direction) the given song ID', ({ delta, expectedResult }) => {
+      const action = queueOrdered(84, delta);
+
+      expect.assertions(1);
+      const result = globalEffects(
+        {
+          ...initialState,
+          player: { ...initialState.player, queue: [17, 84, 23] },
+        },
+        action,
+      );
+
+      expect(result).toStrictEqual<ActionStateSetRemote>({
         type: ActionTypeRemote.StateSet,
         payload: {
-          ...statePrior.player,
-          queue: nextState.player.queue,
+          ...initialState.player,
+          queue: expectedResult,
         },
       });
     });

gmus-web/src/effects/effects.ts

@@ -1,12 +1,30 @@
-import { ActionTypeLocal, ActionTypeRemote, LocalAction, RemoteAction } from '../actions';
+import {
+  ActionQueueOrdered,
+  ActionTypeLocal,
+  ActionTypeRemote,
+  LocalAction,
+  RemoteAction,
+} from '../actions';
 import { GlobalState } from '../reducer/types';
 import { isMaster } from '../selectors';
 
-export function globalEffects(
-  prevState: GlobalState,
-  action: LocalAction,
-  nextState: GlobalState,
-): RemoteAction | null {
+const reverseInArray = <T>(array: T[], index: number): T[] => [
+  ...array.slice(0, Math.max(0, index)),
+  ...array.slice(Math.max(0, index), index + 2).reverse(),
+  ...array.slice(index + 2),
+];
+
+function reorderQueue(queue: number[], action: ActionQueueOrdered): number[] {
+  const currentIndex = queue.indexOf(action.payload.songId);
+  if (currentIndex === -1) {
+    return queue;
+  }
+
+  const reverseIndex = action.payload.delta === 1 ? currentIndex : currentIndex - 1;
+  return reverseInArray(queue, reverseIndex);
+}
+
+export function globalEffects(prevState: GlobalState, action: LocalAction): RemoteAction | null {
   switch (action.type) {
     case ActionTypeLocal.StateSet:
       if (!prevState.player.master && !action.payload.master) {
@@ -76,12 +94,40 @@ export function globalEffects(
       };
 
     case ActionTypeLocal.QueuePushed:
-    case ActionTypeLocal.QueueRemoved:
+      if (prevState.player.queue.includes(action.payload)) {
+        return null;
+      }
+      return {
+        type: ActionTypeRemote.StateSet,
+        payload: {
+          ...prevState.player,
+          queue: [...prevState.player.queue, action.payload],
+        },
+      };
     case ActionTypeLocal.QueueShifted:
+      return {
+        type: ActionTypeRemote.StateSet,
+        payload: {
+          ...prevState.player,
+          queue: prevState.player.queue.slice(1),
+          playing: !!prevState.player.queue[0],
+          songId: prevState.player.queue[0],
+          currentTime: 0,
+          seekTime: 0,
+        },
+      };
+    case ActionTypeLocal.QueueRemoved:
+      return {
+        type: ActionTypeRemote.StateSet,
+        payload: {
+          ...prevState.player,
+          queue: prevState.player.queue.filter((id) => id !== action.payload),
+        },
+      };
     case ActionTypeLocal.QueueOrdered:
       return {
         type: ActionTypeRemote.StateSet,
-        payload: { ...prevState.player, queue: nextState.player.queue },
+        payload: { ...prevState.player, queue: reorderQueue(prevState.player.queue, action) },
       };
 
     default:

gmus-web/src/hooks/queue.ts

@@ -55,7 +55,6 @@ export function usePlayQueue(): {
   const firstQueuedSongId = queue[0];
   const onNext = useCallback(() => {
     if (firstQueuedSongId) {
-      dispatch(stateSet({ songId: firstQueuedSongId }));
       dispatch(queueShifted());
     } else if (!loading && songId) {
       onRequestNext(songId);

gmus-web/src/hooks/socket.spec.tsx

@@ -137,7 +137,7 @@ describe(useDispatchWithEffects.name, () => {
         });
 
         expect(globalEffectsSpy).toHaveBeenCalledTimes(1);
-        expect(globalEffectsSpy).toHaveBeenCalledWith(state, someAction, state);
+        expect(globalEffectsSpy).toHaveBeenCalledWith(state, someAction);
 
         expect(socket.send).toHaveBeenCalledTimes(1);
         expect(socket.send).toHaveBeenCalledWith(JSON.stringify(someEffect));

gmus-web/src/hooks/socket.ts

@@ -33,32 +33,28 @@ export function useDispatchWithEffects(
   socket: WebSocket | null,
 ): Dispatch<LocalAction> {
   const [lastAction, setLastAction] = useState<LocalAction | null>(null);
-  const [lastState, setLastState] = useState<GlobalState>(state);
 
   const dispatchWithEffects = useCallback(
     (action: LocalAction): void => {
       if (action.type === ActionTypeLocal.LoggedOut) {
         socket?.close();
       } else {
-        dispatch(action);
         setLastAction(action);
+        dispatch(action);
       }
     },
     [dispatch, socket],
   );
 
-  const actionSerial = useRef<LocalAction | null>(null);
   useEffect(() => {
-    if (lastAction && lastAction !== actionSerial.current) {
-      actionSerial.current = lastAction;
-      const effect = globalEffects(lastState, lastAction, state);
+    if (lastAction) {
+      const effect = globalEffects(state, lastAction);
       setLastAction(null);
       if (effect && socket && socket.readyState === socket.OPEN) {
         socket.send(JSON.stringify(effect));
       }
     }
-    setLastState(state);
-  }, [state, lastState, lastAction, socket]);
+  }, [state, lastAction, socket]);
 
   return dispatchWithEffects;
 }

gmus-web/src/hooks/vim.ts

@@ -14,6 +14,7 @@ export const Keys = {
   '2': '2',
   B: 'b',
   C: 'c',
+  E: 'e',
   J: 'j',
   K: 'k',
   Z: 'z',

gmus-web/src/reducer/reducer.spec.ts

@@ -7,10 +7,6 @@ import {
   masterSet,
   nameSet,
   playPaused,
-  queuePushed,
-  queueShifted,
-  queueRemoved,
-  queueOrdered,
   seeked,
   songInfoFetched,
   stateSet,
@@ -550,74 +546,4 @@ describe(globalReducer.name, () => {
       });
     });
   });
-
-  describe(ActionTypeLocal.QueuePushed, () => {
-    const action = queuePushed(184);
-
-    it('should add to the beginning of the queue', () => {
-      expect.assertions(1);
-      const result = globalReducer(
-        {
-          ...initialState,
-          player: { ...initialState.player, queue: [23] },
-        },
-        action,
-      );
-      expect(result.player.queue).toStrictEqual([184, 23]);
-    });
-  });
-
-  describe(ActionTypeLocal.QueueShifted, () => {
-    const action = queueShifted();
-
-    it('should remove from the beginning of the queue', () => {
-      expect.assertions(1);
-      const result = globalReducer(
-        {
-          ...initialState,
-          player: { ...initialState.player, queue: [8843, 23] },
-        },
-        action,
-      );
-      expect(result.player.queue).toStrictEqual([23]);
-    });
-  });
-
-  describe(ActionTypeLocal.QueueRemoved, () => {
-    const action = queueRemoved(84);
-
-    it('should remove the given song ID from the queue', () => {
-      expect.assertions(1);
-      const result = globalReducer(
-        {
-          ...initialState,
-          player: { ...initialState.player, queue: [17, 84, 23] },
-        },
-        action,
-      );
-
-      expect(result.player.queue).toStrictEqual([17, 23]);
-    });
-  });
-
-  describe(ActionTypeLocal.QueueOrdered, () => {
-    it.each`
-      direction      | delta | expectedResult
-      ${'forwards'}  | ${1}  | ${[17, 23, 84]}
-      ${'backwards'} | ${-1} | ${[84, 17, 23]}
-    `('should reorder ($direction) the given song ID', ({ delta, expectedResult }) => {
-      const action = queueOrdered(84, delta);
-
-      expect.assertions(1);
-      const result = globalReducer(
-        {
-          ...initialState,
-          player: { ...initialState.player, queue: [17, 84, 23] },
-        },
-        action,
-      );
-
-      expect(result.player.queue).toStrictEqual(expectedResult);
-    });
-  });
 });

gmus-web/src/reducer/reducer.ts

@@ -1,5 +1,4 @@
 import {
-  ActionQueueOrdered,
   ActionSongInfoFetched,
   ActionStateSetLocal,
   ActionStateSetRemote,
@@ -77,26 +76,6 @@ function onSongFetched(state: GlobalState, action: ActionSongInfoFetched): Globa
   };
 }
 
-const reverseInArray = <T>(array: T[], index: number): T[] => [
-  ...array.slice(0, Math.max(0, index)),
-  ...array.slice(Math.max(0, index), index + 2).reverse(),
-  ...array.slice(index + 2),
-];
-
-function reorderQueue(state: GlobalState, action: ActionQueueOrdered): GlobalState {
-  const currentIndex = state.player.queue.indexOf(action.payload.songId);
-  if (currentIndex === -1) {
-    return state;
-  }
-
-  const reverseIndex = action.payload.delta === 1 ? currentIndex : currentIndex - 1;
-
-  return {
-    ...state,
-    player: { ...state.player, queue: reverseInArray(state.player.queue, reverseIndex) },
-  };
-}
-
 export function globalReducer(state: GlobalState, action: AnyAction): GlobalState {
   switch (action.type) {
     case ActionTypeRemote.StateSet:
@@ -148,24 +127,6 @@ export function globalReducer(state: GlobalState, action: AnyAction): GlobalStat
     case ActionTypeLocal.SongInfoFetched:
       return onSongFetched(state, action);
 
-    case ActionTypeLocal.QueuePushed:
-      return {
-        ...state,
-        player: { ...state.player, queue: [action.payload, ...state.player.queue] },
-      };
-    case ActionTypeLocal.QueueShifted:
-      return { ...state, player: { ...state.player, queue: state.player.queue.slice(1) } };
-    case ActionTypeLocal.QueueRemoved:
-      return {
-        ...state,
-        player: {
-          ...state.player,
-          queue: state.player.queue.filter((id) => id !== action.payload),
-        },
-      };
-    case ActionTypeLocal.QueueOrdered:
-      return reorderQueue(state, action);
-
     default:
       return state;
   }