Prevent cross-entry recording interference (#3157)

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<void> {
+    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<void> {
+  async function stopRecording(): Promise<string | undefined> {
     // 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;
     }

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<Blob | undefined> {
-    return new Promise<Blob | undefined>((resolve) => {
+  stopRecording(): Promise<File | undefined> {
+    return new Promise<File | undefined>((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);
       }
     });

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,
           }}

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<StoreState> {
   };
 }
 
-beforeAll(() => {
-  act(() => {
-    testRenderer = create(
-      <StyledEngineProvider injectFirst>
-        <ThemeProvider theme={theme}>
-          <Provider store={mockStore}>
-            <AudioRecorder id="2" uploadAudio={jest.fn()} />
-          </Provider>
-        </ThemeProvider>
-      </StyledEngineProvider>
-    );
-  });
-});
-
-describe("Pronunciations", () => {
-  test("pointerDown and pointerUp", () => {
-    const mockStartRecording = jest.fn();
-    const mockStopRecording = jest.fn();
-    act(() => {
-      testRenderer = create(
-        <StyledEngineProvider injectFirst>
-          <ThemeProvider theme={theme}>
-            <Provider store={mockStore}>
-              <RecorderIcon
-                id={"mockId"}
-                startRecording={mockStartRecording}
-                stopRecording={mockStopRecording}
-              />
-            </Provider>
-          </ThemeProvider>
-        </StyledEngineProvider>
-      );
-    });
-
-    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(
         <ThemeProvider theme={theme}>
@@ -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(() => {

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<StoreState> {
+  return {
+    ...defaultState,
+    pronunciationsState: {
+      fileName: "",
+      status: PronunciationsStatus.Recording,
+      wordId,
+    },
+  };
+}
+
+const mockWordId = "1234567890";
+
+const mockStartRecording = jest.fn();
+const mockStopRecording = jest.fn();
+
+const renderRecorderIcon = async (wordId = ""): Promise<void> => {
+  await act(async () => {
+    testRenderer = create(
+      <StyledEngineProvider injectFirst>
+        <ThemeProvider theme={theme}>
+          <Provider
+            store={configureMockStore()(
+              wordId ? mockRecordingState(wordId) : defaultState
+            )}
+          >
+            <RecorderIcon
+              id={mockWordId}
+              startRecording={mockStartRecording}
+              stopRecording={mockStopRecording}
+            />
+          </Provider>
+        </ThemeProvider>
+      </StyledEngineProvider>
+    );
+  });
+  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();
+  });
+});