From ea679bb5ef73e55adc9e998092556c37ac59f0b4 Mon Sep 17 00:00:00 2001 From: "D. Ror" Date: Mon, 8 Jul 2024 17:39:57 -0400 Subject: [PATCH] Prevent cross-entry recording interference (#3157) --- .../Pronunciations/AudioRecorder.tsx | 23 +++-- src/components/Pronunciations/Recorder.ts | 30 ++++-- .../Pronunciations/RecorderIcon.tsx | 31 ++++-- .../tests/AudioRecorder.test.tsx | 53 +--------- .../tests/RecorderIcon.test.tsx | 99 +++++++++++++++++++ 5 files changed, 162 insertions(+), 74 deletions(-) create mode 100644 src/components/Pronunciations/tests/RecorderIcon.test.tsx diff --git a/src/components/Pronunciations/AudioRecorder.tsx b/src/components/Pronunciations/AudioRecorder.tsx index 33dbd12a9d..fcb8ad58ce 100644 --- a/src/components/Pronunciations/AudioRecorder.tsx +++ b/src/components/Pronunciations/AudioRecorder.tsx @@ -2,10 +2,8 @@ import { ReactElement, useContext } from "react"; import { useTranslation } from "react-i18next"; import { toast } from "react-toastify"; -import Recorder from "components/Pronunciations/Recorder"; import RecorderContext from "components/Pronunciations/RecorderContext"; import RecorderIcon from "components/Pronunciations/RecorderIcon"; -import { getFileNameForWord } from "components/Pronunciations/utilities"; import { useAppSelector } from "rootRedux/hooks"; import { type StoreState } from "rootRedux/types"; import { FileWithSpeakerId } from "types/word"; @@ -26,31 +24,32 @@ export default function AudioRecorder(props: RecorderProps): ReactElement { const { t } = useTranslation(); async function startRecording(): Promise { + const recordingId = recorder.getRecordingId(); + if (recordingId && recordingId !== props.id) { + // Prevent interfering with an active recording on a different entry. + return; + } + // Prevent starting a recording before a previous one is finished. await stopRecording(); - recorder.startRecording(); + recorder.startRecording(props.id); } - async function stopRecording(): Promise { + async function stopRecording(): Promise { // Prevent triggering this function if no recording is active. - if (!recorder.isRecording()) { + if (recorder.getRecordingId() === undefined) { return; } if (props.onClick) { props.onClick(); } - const blob = await recorder.stopRecording(); - if (!blob) { + const file = await recorder.stopRecording(); + if (!file) { toast.error(t("pronunciations.noMicAccess")); return; } - const fileName = getFileNameForWord(props.id); - const file = new File([blob], fileName, { - lastModified: Date.now(), - type: Recorder.blobType, - }); if (!props.noSpeaker) { (file as FileWithSpeakerId).speakerId = speakerId; } diff --git a/src/components/Pronunciations/Recorder.ts b/src/components/Pronunciations/Recorder.ts index b044d118d3..38fe5b50d6 100644 --- a/src/components/Pronunciations/Recorder.ts +++ b/src/components/Pronunciations/Recorder.ts @@ -1,8 +1,11 @@ import RecordRTC from "recordrtc"; +import { getFileNameForWord } from "components/Pronunciations/utilities"; + export default class Recorder { private toast: (text: string) => void; private recordRTC?: RecordRTC; + private id?: string; static blobType: RecordRTC.Options["type"] = "audio"; @@ -14,21 +17,36 @@ export default class Recorder { .catch((err) => this.onError(err)); } - isRecording(): boolean { - return this.recordRTC?.getState() === "recording"; + /** Checks if the recorder state is `"recording"`. + * If so, returns the `id` used with `startRecording()`. + * If not, returns `undefined`. */ + getRecordingId(): string | undefined { + return this.recordRTC?.getState() === "recording" + ? this.id ?? "" + : undefined; } - startRecording(): void { + startRecording(id: string): void { this.recordRTC?.reset(); + this.id = id; this.recordRTC?.startRecording(); } - stopRecording(): Promise { - return new Promise((resolve) => { + stopRecording(): Promise { + return new Promise((resolve) => { const rec = this.recordRTC; if (rec) { - rec.stopRecording(() => resolve(rec.getBlob())); + rec.stopRecording(() => { + const fileName = getFileNameForWord(this.id ?? ""); + const file = new File([rec.getBlob()], fileName, { + lastModified: Date.now(), + type: Recorder.blobType, + }); + this.id = undefined; + resolve(file); + }); } else { + this.id = undefined; resolve(undefined); } }); diff --git a/src/components/Pronunciations/RecorderIcon.tsx b/src/components/Pronunciations/RecorderIcon.tsx index 0ef7babf4f..7ecd2422d3 100644 --- a/src/components/Pronunciations/RecorderIcon.tsx +++ b/src/components/Pronunciations/RecorderIcon.tsx @@ -25,20 +25,37 @@ interface RecorderIconProps { export default function RecorderIcon(props: RecorderIconProps): ReactElement { const isRecording = useAppSelector( (state: StoreState) => - state.pronunciationsState.status === PronunciationsStatus.Recording && - state.pronunciationsState.wordId === props.id + state.pronunciationsState.status === PronunciationsStatus.Recording ); + const recordingId = useAppSelector( + (state: StoreState) => state.pronunciationsState.wordId + ); + const isRecordingThis = isRecording && recordingId === props.id; const dispatch = useAppDispatch(); const { t } = useTranslation(); function toggleIsRecordingToTrue(): void { - dispatch(recording(props.id)); - props.startRecording(); + if (!isRecording) { + // Only start a recording if there's not another on in progress. + dispatch(recording(props.id)); + props.startRecording(); + } else { + // This triggers if user clicks-and-holds on one entry's record icon, + // drags the mouse outside that icon before releasing, + // then clicks-and-holds a different entry's record icon. + if (recordingId !== props.id) { + console.error( + "Tried to record for an entry before finishing a recording on another entry." + ); + } + } } function toggleIsRecordingToFalse(): void { - props.stopRecording(); - dispatch(resetPronunciations()); + if (isRecordingThis) { + props.stopRecording(); + dispatch(resetPronunciations()); + } } function handleTouchStart(): void { @@ -82,7 +99,7 @@ export default function RecorderIcon(props: RecorderIconProps): ReactElement { color: (t) => props.disabled ? t.palette.grey[400] - : isRecording + : isRecordingThis ? themeColors.recordActive : themeColors.recordIdle, }} diff --git a/src/components/Pronunciations/tests/AudioRecorder.test.tsx b/src/components/Pronunciations/tests/AudioRecorder.test.tsx index 0d6827d169..3f1e1b2cfb 100644 --- a/src/components/Pronunciations/tests/AudioRecorder.test.tsx +++ b/src/components/Pronunciations/tests/AudioRecorder.test.tsx @@ -5,10 +5,7 @@ import configureMockStore from "redux-mock-store"; import { defaultState } from "components/App/DefaultState"; import AudioRecorder from "components/Pronunciations/AudioRecorder"; -import RecorderIcon, { - recordButtonId, - recordIconId, -} from "components/Pronunciations/RecorderIcon"; +import { recordIconId } from "components/Pronunciations/RecorderIcon"; import { PronunciationsStatus } from "components/Pronunciations/Redux/PronunciationsReduxTypes"; import { type StoreState } from "rootRedux/types"; import theme, { themeColors } from "types/theme"; @@ -27,50 +24,8 @@ function mockRecordingState(wordId: string): Partial { }; } -beforeAll(() => { - act(() => { - testRenderer = create( - - - - - - - - ); - }); -}); - -describe("Pronunciations", () => { - test("pointerDown and pointerUp", () => { - const mockStartRecording = jest.fn(); - const mockStopRecording = jest.fn(); - act(() => { - testRenderer = create( - - - - - - - - ); - }); - - expect(mockStartRecording).not.toHaveBeenCalled(); - testRenderer.root.findByProps({ id: recordButtonId }).props.onPointerDown(); - expect(mockStartRecording).toHaveBeenCalled(); - - expect(mockStopRecording).not.toHaveBeenCalled(); - testRenderer.root.findByProps({ id: recordButtonId }).props.onPointerUp(); - expect(mockStopRecording).toHaveBeenCalled(); - }); - - test("default style is idle", () => { +describe("AudioRecorder", () => { + test("default icon style is idle", () => { act(() => { testRenderer = create( @@ -86,7 +41,7 @@ describe("Pronunciations", () => { expect(icon.props.sx.color({})).toEqual(themeColors.recordIdle); }); - test("style depends on pronunciations state", () => { + test("icon style depends on pronunciations state", () => { const wordId = "1"; const mockStore2 = configureMockStore()(mockRecordingState(wordId)); act(() => { diff --git a/src/components/Pronunciations/tests/RecorderIcon.test.tsx b/src/components/Pronunciations/tests/RecorderIcon.test.tsx new file mode 100644 index 0000000000..91d308c8ce --- /dev/null +++ b/src/components/Pronunciations/tests/RecorderIcon.test.tsx @@ -0,0 +1,99 @@ +import { StyledEngineProvider, ThemeProvider } from "@mui/material/styles"; +import { Provider } from "react-redux"; +import { + ReactTestInstance, + ReactTestRenderer, + act, + create, +} from "react-test-renderer"; +import configureMockStore from "redux-mock-store"; + +import { defaultState } from "components/App/DefaultState"; +import RecorderIcon, { + recordButtonId, +} from "components/Pronunciations/RecorderIcon"; +import { PronunciationsStatus } from "components/Pronunciations/Redux/PronunciationsReduxTypes"; +import { type StoreState } from "rootRedux/types"; +import theme from "types/theme"; + +let testRenderer: ReactTestRenderer; +let testButton: ReactTestInstance; + +function mockRecordingState(wordId: string): Partial { + return { + ...defaultState, + pronunciationsState: { + fileName: "", + status: PronunciationsStatus.Recording, + wordId, + }, + }; +} + +const mockWordId = "1234567890"; + +const mockStartRecording = jest.fn(); +const mockStopRecording = jest.fn(); + +const renderRecorderIcon = async (wordId = ""): Promise => { + await act(async () => { + testRenderer = create( + + + + + + + + ); + }); + testButton = testRenderer.root.findByProps({ id: recordButtonId }); +}; + +beforeEach(() => { + jest.resetAllMocks(); +}); + +describe("RecorderIcon", () => { + test("pointerDown records if no recording active", async () => { + await renderRecorderIcon(); + expect(mockStartRecording).not.toHaveBeenCalled(); + await act(async () => { + testButton.props.onPointerDown(); + }); + expect(mockStartRecording).toHaveBeenCalled(); + }); + + test("pointerUp stops recording", async () => { + await renderRecorderIcon(mockWordId); + expect(mockStopRecording).not.toHaveBeenCalled(); + await act(async () => { + testButton.props.onPointerUp(); + }); + expect(mockStopRecording).toHaveBeenCalled(); + }); + + test("pointerUp does nothing if no recording active", async () => { + await renderRecorderIcon(); + await act(async () => { + testButton.props.onPointerUp(); + }); + expect(mockStopRecording).not.toHaveBeenCalled(); + }); + + test("pointerUp does nothing if different word id", async () => { + await renderRecorderIcon("different-id"); + await act(async () => { + testButton.props.onPointerUp(); + }); + expect(mockStopRecording).not.toHaveBeenCalled(); + }); +});