From 1aa9860743c82778c3f1cf22e3ee747419c24589 Mon Sep 17 00:00:00 2001 From: David Baker Date: Mon, 15 May 2023 09:37:40 +0100 Subject: [PATCH] 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 --- .../structures/LegacyCallEventGrouper.ts | 24 ++-- .../views/messages/LegacyCallEvent.tsx | 52 ++++----- src/i18n/strings/en_EN.json | 4 +- .../structures/LegacyCallEventGrouper-test.ts | 21 +++- .../views/messages/LegacyCallEvent-test.tsx | 105 ++++++++++++++++++ 5 files changed, 163 insertions(+), 43 deletions(-) create mode 100644 test/components/views/messages/LegacyCallEvent-test.tsx diff --git a/src/components/structures/LegacyCallEventGrouper.ts b/src/components/structures/LegacyCallEventGrouper.ts index 354116009b1..c87f08bfffc 100644 --- a/src/components/structures/LegacyCallEventGrouper.ts +++ b/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 = new Set(); 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 { - return ![...this.events].some((event) => event.sender?.userId === MatrixClientPeg.get().getUserId()); + public get callWasMissed(): boolean { + 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; - else if (this.reject) this.state = CallState.Ended; - else if (this.hangup) this.state = CallState.Ended; - else if (this.invite && this.call) this.state = CallState.Connecting; + if (this.reject) { + this.state = CallState.Ended; + } else if (this.hangup) { + this.state = CallState.Ended; + } else if (this.invite && this.call) { + this.state = CallState.Connecting; + } } this.emit(LegacyCallEventGrouperEvent.StateChanged, this.state); }; diff --git a/src/components/views/messages/LegacyCallEvent.tsx b/src/components/views/messages/LegacyCallEvent.tsx index 6d7bc3db8f6..07337c78749 100644 --- a/src/components/views/messages/LegacyCallEvent.tsx +++ b/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, { - LegacyCallEventGrouperEvent, - CustomCallState, -} from "../../structures/LegacyCallEventGrouper"; +import LegacyCallEventGrouper, { LegacyCallEventGrouperEvent } 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 ); } - private renderContent(state: CallState | CustomCallState): JSX.Element { - if (state === CallState.Ringing) { + private renderContent(): JSX.Element { + 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 ); } - 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 {this.props.timestamp} ); + } else if (hangupReason === CallErrorCode.AnsweredElsewhere) { + return ( +
+ {_t("Answered elsewhere")} + {this.props.timestamp} +
+ ); + } else if (this.props.callEventGrouper.callWasMissed) { + return ( +
+ {_t("Missed call")} + {this.renderCallBackButton(_t("Call back"))} + {this.props.timestamp} +
+ ); } 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 {this.props.timestamp} ); - } else if (hangupReason === CallErrorCode.AnsweredElsewhere) { - return ( -
- {_t("Answered elsewhere")} - {this.props.timestamp} -
- ); } let reason; @@ -234,7 +239,7 @@ export default class LegacyCallEvent extends React.PureComponent ); } - if (state === CallState.Connected) { + if (this.state.callState === CallState.Connected) { return (
@@ -242,7 +247,7 @@ export default class LegacyCallEvent extends React.PureComponent
); } - if (state === CallState.Connecting) { + if (this.state.callState === CallState.Connecting) { return (
{_t("Connecting")} @@ -250,15 +255,6 @@ export default class LegacyCallEvent extends React.PureComponent
); } - if (state === CustomCallState.Missed) { - return ( -
- {_t("Missed call")} - {this.renderCallBackButton(_t("Call back"))} - {this.props.timestamp} -
- ); - } return (
@@ -275,12 +271,12 @@ export default class LegacyCallEvent extends React.PureComponent 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, }); diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 59e30ff0cf4..52e01c03037 100644 --- a/src/i18n/strings/en_EN.json +++ b/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", diff --git a/test/components/structures/LegacyCallEventGrouper-test.ts b/test/components/structures/LegacyCallEventGrouper-test.ts index 7cc29bce342..1fb34555874 100644 --- a/test/components/structures/LegacyCallEventGrouper-test.ts +++ b/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", () => { diff --git a/test/components/views/messages/LegacyCallEvent-test.tsx b/test/components/views/messages/LegacyCallEvent-test.tsx new file mode 100644 index 00000000000..3b665897ec6 --- /dev/null +++ b/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; + let callEventGrouper: Record; + + beforeEach(() => { + callInviteEvent = { + sender: { + userId: THEIR_USER_ID, + }, + }; + + callEventGrouper = { + addListener: jest.fn(), + removeListener: jest.fn(), + invite: jest.fn().mockReturnValue(callInviteEvent), + }; + }); + + const renderEvent = () => { + render( + , + ); + }; + + 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"); + }); +});