Jelajahi Sumber

fix: rely on effects for synchronising paused state between all clients

This fixed a bug where master could not pause the currently playing song
without being overridden.
Fela Maslen 4 tahun lalu
induk
melakukan
8657b2f56f

+ 18 - 26
gmus-web/src/effects/effects.spec.ts

@@ -196,38 +196,30 @@ describe(globalEffects.name, () => {
   });
 
   describe(ActionTypeLocal.PlayPaused, () => {
-    const statePriorMaster: GlobalState = {
-      ...initialState,
-      player: {
-        songId: 123,
-        playing: true,
-        currentTime: 83,
-        seekTime: 5,
-        master: 'some-master-client',
-        activeClients: [],
-        queue: [],
-      },
-      myClientName: 'some-master-client',
-    };
-
     const action = playPaused();
 
-    describe('when the client is master', () => {
-      it('should return null', () => {
-        expect.assertions(1);
-        expect(globalEffects(statePriorMaster, action)).toBeNull();
-      });
-    });
-
-    describe('when the client is a slave', () => {
-      const stateSlave: GlobalState = {
-        ...statePriorMaster,
-        myClientName: 'some-slave-client',
+    describe.each`
+      currentClient | myClientName
+      ${'master'}   | ${'some-master-client'}
+      ${'a slave'}  | ${'my client'}
+    `('when the current client is $currentClient', ({ myClientName }) => {
+      const statePrior: GlobalState = {
+        ...initialState,
+        player: {
+          songId: 123,
+          playing: true,
+          currentTime: 83,
+          seekTime: 5,
+          master: 'some-master-client',
+          activeClients: [],
+          queue: [],
+        },
+        myClientName,
       };
 
       it('should return a StateSet action informing other clients of the updated playing state', () => {
         expect.assertions(1);
-        const result = globalEffects(stateSlave, action);
+        const result = globalEffects(statePrior, action);
 
         expect(result).toStrictEqual<ActionStateSetRemote>({
           type: ActionTypeRemote.StateSet,

+ 0 - 3
gmus-web/src/effects/effects.ts

@@ -93,9 +93,6 @@ export function globalEffects(prevState: GlobalState, action: LocalAction): Remo
       };
 
     case ActionTypeLocal.PlayPaused:
-      if (isMaster(prevState)) {
-        return null;
-      }
       return {
         type: ActionTypeRemote.StateSet,
         payload: {

+ 0 - 80
gmus-web/src/reducer/reducer.spec.ts

@@ -6,7 +6,6 @@ import {
   ActionTypeRemote,
   masterSet,
   nameSet,
-  playPaused,
   seeked,
   songInfoFetched,
   stateSet,
@@ -432,85 +431,6 @@ describe(globalReducer.name, () => {
     });
   });
 
-  describe(ActionTypeLocal.PlayPaused, () => {
-    const action = playPaused();
-
-    describe('when the current client is master', () => {
-      const stateMaster: GlobalState = {
-        ...initialState,
-        myClientName: 'my-client',
-        player: {
-          ...nullPlayer,
-          master: 'my-client',
-        },
-      };
-
-      describe('when playing', () => {
-        const statePlaying: GlobalState = {
-          ...stateMaster,
-          player: {
-            ...stateMaster.player,
-            songId: 123,
-            playing: true,
-          },
-        };
-
-        it('should set playing=false', () => {
-          expect.assertions(1);
-          const result = globalReducer(statePlaying, action);
-
-          expect(result.player).toStrictEqual<MusicPlayer>({
-            ...stateMaster.player,
-            songId: 123,
-            playing: false,
-          });
-        });
-      });
-
-      describe('when not playing', () => {
-        const statePaused: GlobalState = {
-          ...stateMaster,
-          player: {
-            ...stateMaster.player,
-            songId: 123,
-            playing: false,
-          },
-        };
-
-        it('should set playing=true', () => {
-          expect.assertions(1);
-          const result = globalReducer(statePaused, action);
-
-          expect(result.player).toStrictEqual<MusicPlayer>({
-            ...stateMaster.player,
-            songId: 123,
-            playing: true,
-          });
-        });
-      });
-    });
-
-    describe('when the current client is a slave', () => {
-      const stateSlave: GlobalState = {
-        ...initialState,
-        myClientName: 'my-client',
-        player: {
-          ...initialState.player,
-          songId: 123,
-          playing: true,
-          master: 'some-master-client',
-        },
-      };
-
-      it('should not update the state optimistically', () => {
-        expect.assertions(1);
-        const result = globalReducer(stateSlave, action);
-
-        expect(result.player).toBe(stateSlave.player);
-      });
-    });
-  });
-
   describe(ActionTypeLocal.SongInfoFetched, () => {
     const song: Song = {
       id: 123,

+ 0 - 6
gmus-web/src/reducer/reducer.ts

@@ -122,12 +122,6 @@ export function globalReducer(state: GlobalState, action: AnyAction): GlobalStat
         },
       };
 
-    case ActionTypeLocal.PlayPaused:
-      if (!isMaster(state)) {
-        return state;
-      }
-      return { ...state, player: { ...state.player, playing: !state.player.playing } };
-
     case ActionTypeLocal.SongInfoFetched:
       return onSongFetched(state, action);