From 07a1e9a00941a53b528b119680c823d99ae19e47 Mon Sep 17 00:00:00 2001 From: Michael Weimann Date: Wed, 19 Oct 2022 18:02:48 +0200 Subject: [PATCH] Stop voice broadcast recording on redaction (#9455) --- .../hooks/useVoiceBroadcastRecording.tsx | 2 - .../models/VoiceBroadcastRecording.ts | 12 +++- .../stores/VoiceBroadcastRecordingsStore.ts | 16 ++++- .../models/VoiceBroadcastRecording-test.ts | 14 +++++ .../VoiceBroadcastRecordingsStore-test.ts | 62 +++++++++++-------- 5 files changed, 76 insertions(+), 30 deletions(-) diff --git a/src/voice-broadcast/hooks/useVoiceBroadcastRecording.tsx b/src/voice-broadcast/hooks/useVoiceBroadcastRecording.tsx index c0db561746..341283c2ad 100644 --- a/src/voice-broadcast/hooks/useVoiceBroadcastRecording.tsx +++ b/src/voice-broadcast/hooks/useVoiceBroadcastRecording.tsx @@ -20,7 +20,6 @@ import { VoiceBroadcastInfoState, VoiceBroadcastRecording, VoiceBroadcastRecordingEvent, - VoiceBroadcastRecordingsStore, } from ".."; import QuestionDialog from "../../components/views/dialogs/QuestionDialog"; import { useTypedEventEmitter } from "../../hooks/useEventEmitter"; @@ -54,7 +53,6 @@ export const useVoiceBroadcastRecording = (recording: VoiceBroadcastRecording) = if (confirmed) { recording.stop(); - VoiceBroadcastRecordingsStore.instance().clearCurrent(); } }; diff --git a/src/voice-broadcast/models/VoiceBroadcastRecording.ts b/src/voice-broadcast/models/VoiceBroadcastRecording.ts index 96b62a670f..f7faa0876e 100644 --- a/src/voice-broadcast/models/VoiceBroadcastRecording.ts +++ b/src/voice-broadcast/models/VoiceBroadcastRecording.ts @@ -15,7 +15,7 @@ limitations under the License. */ import { logger } from "matrix-js-sdk/src/logger"; -import { MatrixClient, MatrixEvent, RelationType } from "matrix-js-sdk/src/matrix"; +import { MatrixClient, MatrixEvent, MatrixEventEvent, RelationType } from "matrix-js-sdk/src/matrix"; import { TypedEventEmitter } from "matrix-js-sdk/src/models/typed-event-emitter"; import { @@ -67,6 +67,7 @@ export class VoiceBroadcastRecording }) ? VoiceBroadcastInfoState.Started : VoiceBroadcastInfoState.Stopped; // TODO Michael W: add listening for updates + this.infoEvent.on(MatrixEventEvent.BeforeRedaction, this.onBeforeRedaction); this.dispatcherRef = dis.register(this.onAction); } @@ -99,10 +100,19 @@ export class VoiceBroadcastRecording this.recorder.stop(); } + this.infoEvent.off(MatrixEventEvent.BeforeRedaction, this.onBeforeRedaction); this.removeAllListeners(); dis.unregister(this.dispatcherRef); } + private onBeforeRedaction = () => { + if (this.getState() !== VoiceBroadcastInfoState.Stopped) { + this.setState(VoiceBroadcastInfoState.Stopped); + // destroy cleans up everything + this.destroy(); + } + }; + private onAction = (payload: ActionPayload) => { if (payload.action !== "call_state") return; diff --git a/src/voice-broadcast/stores/VoiceBroadcastRecordingsStore.ts b/src/voice-broadcast/stores/VoiceBroadcastRecordingsStore.ts index cc12b474e8..b5c78a1b0e 100644 --- a/src/voice-broadcast/stores/VoiceBroadcastRecordingsStore.ts +++ b/src/voice-broadcast/stores/VoiceBroadcastRecordingsStore.ts @@ -17,7 +17,7 @@ limitations under the License. import { MatrixClient, MatrixEvent } from "matrix-js-sdk/src/matrix"; import { TypedEventEmitter } from "matrix-js-sdk/src/models/typed-event-emitter"; -import { VoiceBroadcastRecording } from ".."; +import { VoiceBroadcastInfoState, VoiceBroadcastRecording, VoiceBroadcastRecordingEvent } from ".."; export enum VoiceBroadcastRecordingsStoreEvent { CurrentChanged = "current_changed", @@ -41,7 +41,12 @@ export class VoiceBroadcastRecordingsStore extends TypedEventEmitter { + if (state === VoiceBroadcastInfoState.Stopped) { + this.clearCurrent(); + } + }; + private static readonly cachedInstance = new VoiceBroadcastRecordingsStore(); /** diff --git a/test/voice-broadcast/models/VoiceBroadcastRecording-test.ts b/test/voice-broadcast/models/VoiceBroadcastRecording-test.ts index 25a325aba7..049c03c5a4 100644 --- a/test/voice-broadcast/models/VoiceBroadcastRecording-test.ts +++ b/test/voice-broadcast/models/VoiceBroadcastRecording-test.ts @@ -20,6 +20,7 @@ import { EventType, MatrixClient, MatrixEvent, + MatrixEventEvent, MsgType, RelationType, Room, @@ -81,6 +82,7 @@ describe("VoiceBroadcastRecording", () => { const setUpVoiceBroadcastRecording = () => { voiceBroadcastRecording = new VoiceBroadcastRecording(infoEvent, client); voiceBroadcastRecording.on(VoiceBroadcastRecordingEvent.StateChanged, onStateChanged); + jest.spyOn(voiceBroadcastRecording, "destroy"); jest.spyOn(voiceBroadcastRecording, "removeAllListeners"); }; @@ -214,6 +216,18 @@ describe("VoiceBroadcastRecording", () => { expect(voiceBroadcastRecorder.start).toHaveBeenCalled(); }); + describe("and the info event is redacted", () => { + beforeEach(() => { + infoEvent.emit(MatrixEventEvent.BeforeRedaction, null, null); + }); + + itShouldBeInState(VoiceBroadcastInfoState.Stopped); + + it("should destroy the recording", () => { + expect(voiceBroadcastRecording.destroy).toHaveBeenCalled(); + }); + }); + describe("and receiving a call action", () => { beforeEach(() => { dis.dispatch({ diff --git a/test/voice-broadcast/stores/VoiceBroadcastRecordingsStore-test.ts b/test/voice-broadcast/stores/VoiceBroadcastRecordingsStore-test.ts index 56b90b73ae..3edb74592e 100644 --- a/test/voice-broadcast/stores/VoiceBroadcastRecordingsStore-test.ts +++ b/test/voice-broadcast/stores/VoiceBroadcastRecordingsStore-test.ts @@ -18,28 +18,22 @@ import { mocked } from "jest-mock"; import { MatrixClient, MatrixEvent, Room } from "matrix-js-sdk/src/matrix"; import { - VoiceBroadcastInfoEventType, VoiceBroadcastRecordingsStore, VoiceBroadcastRecordingsStoreEvent, VoiceBroadcastRecording, + VoiceBroadcastInfoState, } from "../../../src/voice-broadcast"; -import { mkEvent, mkStubRoom, stubClient } from "../../test-utils"; - -jest.mock("../../../src/voice-broadcast/models/VoiceBroadcastRecording.ts", () => ({ - VoiceBroadcastRecording: jest.fn().mockImplementation( - ( - infoEvent: MatrixEvent, - client: MatrixClient, - ) => ({ infoEvent, client }), - ), -})); +import { mkStubRoom, stubClient } from "../../test-utils"; +import { mkVoiceBroadcastInfoStateEvent } from "../utils/test-utils"; describe("VoiceBroadcastRecordingsStore", () => { const roomId = "!room:example.com"; let client: MatrixClient; let room: Room; let infoEvent: MatrixEvent; + let otherInfoEvent: MatrixEvent; let recording: VoiceBroadcastRecording; + let otherRecording: VoiceBroadcastRecording; let recordings: VoiceBroadcastRecordingsStore; let onCurrentChanged: (recording: VoiceBroadcastRecording) => void; @@ -51,22 +45,17 @@ describe("VoiceBroadcastRecordingsStore", () => { return room; } }); - infoEvent = mkEvent({ - event: true, - type: VoiceBroadcastInfoEventType, - user: client.getUserId(), - room: roomId, - content: {}, - }); - recording = { - infoEvent, - } as unknown as VoiceBroadcastRecording; + infoEvent = mkVoiceBroadcastInfoStateEvent(roomId, VoiceBroadcastInfoState.Started, client.getUserId()); + otherInfoEvent = mkVoiceBroadcastInfoStateEvent(roomId, VoiceBroadcastInfoState.Started, client.getUserId()); + recording = new VoiceBroadcastRecording(infoEvent, client); + otherRecording = new VoiceBroadcastRecording(otherInfoEvent, client); recordings = new VoiceBroadcastRecordingsStore(); onCurrentChanged = jest.fn(); recordings.on(VoiceBroadcastRecordingsStoreEvent.CurrentChanged, onCurrentChanged); }); afterEach(() => { + recording.destroy(); recordings.off(VoiceBroadcastRecordingsStoreEvent.CurrentChanged, onCurrentChanged); }); @@ -110,6 +99,32 @@ describe("VoiceBroadcastRecordingsStore", () => { it("should emit a current changed event", () => { expect(onCurrentChanged).toHaveBeenCalledWith(null); }); + + it("and calling it again should work", () => { + recordings.clearCurrent(); + expect(recordings.getCurrent()).toBeNull(); + }); + }); + + describe("and setting another recording and stopping the previous recording", () => { + beforeEach(() => { + recordings.setCurrent(otherRecording); + recording.stop(); + }); + + it("should keep the current recording", () => { + expect(recordings.getCurrent()).toBe(otherRecording); + }); + }); + + describe("and the recording stops", () => { + beforeEach(() => { + recording.stop(); + }); + + it("should clear the current recording", () => { + expect(recordings.getCurrent()).toBeNull(); + }); }); }); @@ -133,10 +148,7 @@ describe("VoiceBroadcastRecordingsStore", () => { }); it("should return the recording", () => { - expect(returnedRecording).toEqual({ - infoEvent, - client, - }); + expect(returnedRecording.infoEvent).toBe(infoEvent); }); }); });