Fix the state shown for call in rooms (#10833)

* Fix the state shown for call in rooms

We split out a separate state for 'missed' separate to 'ended' which
caused confusion as the state got set to this when it shouldn't have,
so calls that wouldn't have been shown as missed were.

Remove the csutom call state and just have the missed state as a
variant of the ended state. Re-order the if clauses so they hit the
right ones. Also don't pass the callState variable into renderContent()
which is a class method and so has access to the same info anyway.

* Fix test

* i18n (reorder only)

* Fix test

* Fix types

* Add test for legacy call event tile

src/components/structures/LegacyCallEventGrouper.ts

@@ -37,10 +37,6 @@ const CONNECTING_STATES = [
 
 const SUPPORTED_STATES = [CallState.Connected, CallState.Ringing, CallState.Ended];
 
-export enum CustomCallState {
-    Missed = "missed",
-}
-
 const isCallEventType = (eventType: string): boolean =>
     eventType.startsWith("m.call.") || eventType.startsWith("org.matrix.call.");
 
@@ -73,7 +69,7 @@ export function buildLegacyCallEventGroupers(
 export default class LegacyCallEventGrouper extends EventEmitter {
     private events: Set<MatrixEvent> = new Set<MatrixEvent>();
     private call: MatrixCall | null = null;
-    public state?: CallState | CustomCallState;
+    public state?: CallState;
 
     public constructor() {
         super();
@@ -130,8 +126,11 @@ export default class LegacyCallEventGrouper extends EventEmitter {
     /**
      * Returns true if there are only events from the other side - we missed the call
      */
-    private get callWasMissed(): boolean {
+    public get callWasMissed(): boolean {
-        return ![...this.events].some((event) => event.sender?.userId === MatrixClientPeg.get().getUserId());
+        return (
+            this.state === CallState.Ended &&
+            ![...this.events].some((event) => event.sender?.userId === MatrixClientPeg.get().getUserId())
+        );
     }
 
     private get callId(): string | undefined {
@@ -188,10 +187,13 @@ export default class LegacyCallEventGrouper extends EventEmitter {
         } else if (this.call && SUPPORTED_STATES.includes(this.call.state)) {
             this.state = this.call.state;
         } else {
-            if (this.callWasMissed) this.state = CustomCallState.Missed;
+            if (this.reject) {
-            else if (this.reject) this.state = CallState.Ended;
+                this.state = CallState.Ended;
-            else if (this.hangup) this.state = CallState.Ended;
+            } else if (this.hangup) {
-            else if (this.invite && this.call) this.state = CallState.Connecting;
+                this.state = CallState.Ended;
+            } else if (this.invite && this.call) {
+                this.state = CallState.Connecting;
+            }
         }
         this.emit(LegacyCallEventGrouperEvent.StateChanged, this.state);
     };

src/components/views/messages/LegacyCallEvent.tsx

@@ -21,10 +21,7 @@ import classNames from "classnames";
 
 import { _t } from "../../../languageHandler";
 import MemberAvatar from "../avatars/MemberAvatar";
-import LegacyCallEventGrouper, {
+import LegacyCallEventGrouper, { LegacyCallEventGrouperEvent } from "../../structures/LegacyCallEventGrouper";
-    LegacyCallEventGrouperEvent,
-    CustomCallState,
-} from "../../structures/LegacyCallEventGrouper";
 import AccessibleButton from "../elements/AccessibleButton";
 import InfoTooltip, { InfoTooltipKind } from "../elements/InfoTooltip";
 import AccessibleTooltipButton from "../elements/AccessibleTooltipButton";
@@ -40,7 +37,7 @@ interface IProps {
 }
 
 interface IState {
-    callState: CallState | CustomCallState;
+    callState?: CallState;
     silenced: boolean;
     narrow: boolean;
     length: number;
@@ -125,8 +122,8 @@ export default class LegacyCallEvent extends React.PureComponent<IProps, IState>
         );
     }
 
-    private renderContent(state: CallState | CustomCallState): JSX.Element {
+    private renderContent(): JSX.Element {
-        if (state === CallState.Ringing) {
+        if (this.state.callState === CallState.Ringing) {
             let silenceIcon;
             if (!this.state.narrow) {
                 silenceIcon = this.renderSilenceIcon();
@@ -153,7 +150,7 @@ export default class LegacyCallEvent extends React.PureComponent<IProps, IState>
                 </div>
             );
         }
-        if (state === CallState.Ended) {
+        if (this.state.callState === CallState.Ended) {
             const hangupReason = this.props.callEventGrouper.hangupReason;
             const gotRejected = this.props.callEventGrouper.gotRejected;
 
@@ -165,6 +162,21 @@ export default class LegacyCallEvent extends React.PureComponent<IProps, IState>
                         {this.props.timestamp}
                     </div>
                 );
+            } else if (hangupReason === CallErrorCode.AnsweredElsewhere) {
+                return (
+                    <div className="mx_LegacyCallEvent_content">
+                        {_t("Answered elsewhere")}
+                        {this.props.timestamp}
+                    </div>
+                );
+            } else if (this.props.callEventGrouper.callWasMissed) {
+                return (
+                    <div className="mx_LegacyCallEvent_content">
+                        {_t("Missed call")}
+                        {this.renderCallBackButton(_t("Call back"))}
+                        {this.props.timestamp}
+                    </div>
+                );
             } else if (!hangupReason || [CallErrorCode.UserHangup, "user hangup"].includes(hangupReason)) {
                 // workaround for https://github.com/vector-im/element-web/issues/5178
                 // it seems Android randomly sets a reason of "user hangup" which is
@@ -191,13 +203,6 @@ export default class LegacyCallEvent extends React.PureComponent<IProps, IState>
                         {this.props.timestamp}
                     </div>
                 );
-            } else if (hangupReason === CallErrorCode.AnsweredElsewhere) {
-                return (
-                    <div className="mx_LegacyCallEvent_content">
-                        {_t("Answered elsewhere")}
-                        {this.props.timestamp}
-                    </div>
-                );
             }
 
             let reason;
@@ -234,7 +239,7 @@ export default class LegacyCallEvent extends React.PureComponent<IProps, IState>
                 </div>
             );
         }
-        if (state === CallState.Connected) {
+        if (this.state.callState === CallState.Connected) {
             return (
                 <div className="mx_LegacyCallEvent_content">
                     <Clock seconds={this.state.length} aria-live="off" />
@@ -242,7 +247,7 @@ export default class LegacyCallEvent extends React.PureComponent<IProps, IState>
                 </div>
             );
         }
-        if (state === CallState.Connecting) {
+        if (this.state.callState === CallState.Connecting) {
             return (
                 <div className="mx_LegacyCallEvent_content">
                     {_t("Connecting")}
@@ -250,15 +255,6 @@ export default class LegacyCallEvent extends React.PureComponent<IProps, IState>
                 </div>
             );
         }
-        if (state === CustomCallState.Missed) {
-            return (
-                <div className="mx_LegacyCallEvent_content">
-                    {_t("Missed call")}
-                    {this.renderCallBackButton(_t("Call back"))}
-                    {this.props.timestamp}
-                </div>
-            );
-        }
 
         return (
             <div className="mx_LegacyCallEvent_content">
@@ -275,12 +271,12 @@ export default class LegacyCallEvent extends React.PureComponent<IProps, IState>
         const callType = isVoice ? _t("Voice call") : _t("Video call");
         const callState = this.state.callState;
         const hangupReason = this.props.callEventGrouper.hangupReason;
-        const content = this.renderContent(callState);
+        const content = this.renderContent();
         const className = classNames("mx_LegacyCallEvent", {
             mx_LegacyCallEvent_voice: isVoice,
             mx_LegacyCallEvent_video: !isVoice,
             mx_LegacyCallEvent_narrow: this.state.narrow,
-            mx_LegacyCallEvent_missed: callState === CustomCallState.Missed,
+            mx_LegacyCallEvent_missed: this.props.callEventGrouper.callWasMissed,
             mx_LegacyCallEvent_noAnswer: callState === CallState.Ended && hangupReason === CallErrorCode.InviteTimeout,
             mx_LegacyCallEvent_rejected: callState === CallState.Ended && this.props.callEventGrouper.gotRejected,
         });

src/i18n/strings/en_EN.json

@@ -2402,15 +2402,15 @@
     "Go": "Go",
     "Call declined": "Call declined",
     "Call back": "Call back",
-    "No answer": "No answer",
     "Answered elsewhere": "Answered elsewhere",
+    "Missed call": "Missed call",
+    "No answer": "No answer",
     "Could not connect media": "Could not connect media",
     "Connection failed": "Connection failed",
     "Their device couldn't start the camera or microphone": "Their device couldn't start the camera or microphone",
     "An unknown error occurred": "An unknown error occurred",
     "Unknown failure: %(reason)s": "Unknown failure: %(reason)s",
     "Retry": "Retry",
-    "Missed call": "Missed call",
     "The call is in an unknown state!": "The call is in an unknown state!",
     "Error processing audio message": "Error processing audio message",
     "View live location": "View live location",

test/components/structures/LegacyCallEventGrouper-test.ts

@@ -20,7 +20,7 @@ import { CallState } from "matrix-js-sdk/src/webrtc/call";
 
 import { stubClient } from "../../test-utils";
 import { MatrixClientPeg } from "../../../src/MatrixClientPeg";
-import LegacyCallEventGrouper, { CustomCallState } from "../../../src/components/structures/LegacyCallEventGrouper";
+import LegacyCallEventGrouper from "../../../src/components/structures/LegacyCallEventGrouper";
 
 const MY_USER_ID = "@me:here";
 const THEIR_USER_ID = "@they:here";
@@ -39,6 +39,9 @@ describe("LegacyCallEventGrouper", () => {
     it("detects a missed call", () => {
         const grouper = new LegacyCallEventGrouper();
 
+        // This assumes that the other party aborted the call by sending a hangup,
+        // which is the usual case. Another possible test would be for the edge
+        // case where there is only an expired invite event.
         grouper.add({
             getContent: () => {
                 return {
@@ -52,8 +55,22 @@ describe("LegacyCallEventGrouper", () => {
                 userId: THEIR_USER_ID,
             },
         } as unknown as MatrixEvent);
+        grouper.add({
+            getContent: () => {
+                return {
+                    call_id: "callId",
+                };
+            },
+            getType: () => {
+                return EventType.CallHangup;
+            },
+            sender: {
+                userId: THEIR_USER_ID,
+            },
+        } as unknown as MatrixEvent);
 
-        expect(grouper.state).toBe(CustomCallState.Missed);
+        expect(grouper.state).toBe(CallState.Ended);
+        expect(grouper.callWasMissed).toBe(true);
     });
 
     it("detects an ended call", () => {

test/components/views/messages/LegacyCallEvent-test.tsx

@@ -0,0 +1,105 @@
+/*
+Copyright 2023 The Matrix.org Foundation C.I.C.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+import React from "react";
+import { render, screen } from "@testing-library/react";
+import { CallErrorCode, CallState } from "matrix-js-sdk/src/webrtc/call";
+import { MatrixEvent } from "matrix-js-sdk/src/models/event";
+
+import LegacyCallEvent from "../../../../src/components/views/messages/LegacyCallEvent";
+import LegacyCallEventGrouper from "../../../../src/components/structures/LegacyCallEventGrouper";
+
+const THEIR_USER_ID = "@them:here";
+
+describe("LegacyCallEvent", () => {
+    let callInviteEvent: Record<string, any>;
+    let callEventGrouper: Record<string, any>;
+
+    beforeEach(() => {
+        callInviteEvent = {
+            sender: {
+                userId: THEIR_USER_ID,
+            },
+        };
+
+        callEventGrouper = {
+            addListener: jest.fn(),
+            removeListener: jest.fn(),
+            invite: jest.fn().mockReturnValue(callInviteEvent),
+        };
+    });
+
+    const renderEvent = () => {
+        render(
+            <LegacyCallEvent
+                mxEvent={callInviteEvent as unknown as MatrixEvent}
+                callEventGrouper={callEventGrouper as unknown as LegacyCallEventGrouper}
+            />,
+        );
+    };
+
+    it("shows if the call was ended", () => {
+        callEventGrouper.state = CallState.Ended;
+        callEventGrouper.gotRejected = jest.fn().mockReturnValue(true);
+
+        renderEvent();
+
+        screen.getByText("Call declined");
+    });
+
+    it("shows if the call was answered elsewhere", () => {
+        callEventGrouper.state = CallState.Ended;
+        callEventGrouper.hangupReason = CallErrorCode.AnsweredElsewhere;
+
+        renderEvent();
+
+        screen.getByText("Answered elsewhere");
+    });
+
+    it("shows if the call was missed", () => {
+        callEventGrouper.state = CallState.Ended;
+        callEventGrouper.callWasMissed = jest.fn().mockReturnValue(true);
+
+        renderEvent();
+
+        screen.getByText("Missed call");
+    });
+
+    it("shows if the call ended cleanly", () => {
+        callEventGrouper.state = CallState.Ended;
+        callEventGrouper.hangupReason = CallErrorCode.UserHangup;
+
+        renderEvent();
+
+        screen.getByText("Call ended");
+    });
+
+    it("shows if the call is connecting", () => {
+        callEventGrouper.state = CallState.Connecting;
+
+        renderEvent();
+
+        screen.getByText("Connecting");
+    });
+
+    it("shows timer if the call is connected", () => {
+        callEventGrouper.state = CallState.Connected;
+
+        renderEvent();
+
+        screen.getByText("00:00");
+    });
+});