From 7c16b16ee098b3ab6b11b74e1b00531e298dac73 Mon Sep 17 00:00:00 2001 From: Jamie Kyle <113370520+jamiebuilds-signal@users.noreply.github.com> Date: Mon, 21 Aug 2023 10:09:54 -0700 Subject: [PATCH] Revert to previous method of rendering calling notifications --- .../CallingNotification.stories.tsx | 81 +++++++++++++++---- .../conversation/CallingNotification.tsx | 30 +++---- ts/state/selectors/message.ts | 60 +++++++------- ts/test-both/util/callingNotification_test.ts | 20 ++--- ts/util/callingNotification.ts | 24 +++--- 5 files changed, 131 insertions(+), 84 deletions(-) diff --git a/ts/components/conversation/CallingNotification.stories.tsx b/ts/components/conversation/CallingNotification.stories.tsx index e1c2b6dbda72..2a85a651c7a9 100644 --- a/ts/components/conversation/CallingNotification.stories.tsx +++ b/ts/components/conversation/CallingNotification.stories.tsx @@ -21,7 +21,6 @@ import { DirectCallStatus, } from '../../types/CallDisposition'; import type { ConversationType } from '../../state/ducks/conversations'; -import { CallExternalState } from '../../util/callingNotification'; const i18n = setupI18n('en', enMessages); @@ -35,7 +34,9 @@ const getCommonProps = (options: { direction?: CallDirection; status?: CallStatus; callCreator?: ConversationType | null; - callExternalState?: CallExternalState; + groupCallEnded: boolean | null; + deviceCount: number; + maxDevices: number; }): PropsType => { const { mode, @@ -48,7 +49,9 @@ const getCommonProps = (options: { serviceId: generateAci(), isMe: direction === CallDirection.Outgoing, }), - callExternalState = CallExternalState.Active, + groupCallEnded, + deviceCount, + maxDevices, } = options; const conversation = @@ -71,15 +74,10 @@ const getCommonProps = (options: { status, }, callCreator, - callExternalState, - maxDevices: mode === CallMode.Group ? 15 : 0, - deviceCount: - // eslint-disable-next-line no-nested-ternary - mode === CallMode.Group - ? callExternalState === CallExternalState.Full - ? 15 - : 13 - : Infinity, + activeConversationId: null, + groupCallEnded, + maxDevices, + deviceCount, }; }; @@ -103,6 +101,9 @@ export function AcceptedIncomingAudioCall(): JSX.Element { type: CallType.Audio, direction: CallDirection.Incoming, status: DirectCallStatus.Accepted, + groupCallEnded: null, + deviceCount: 0, + maxDevices: Infinity, })} /> ); @@ -116,7 +117,9 @@ export function AcceptedIncomingVideoCall(): JSX.Element { type: CallType.Video, direction: CallDirection.Incoming, status: DirectCallStatus.Accepted, - callExternalState: CallExternalState.Ended, + groupCallEnded: null, + deviceCount: 0, + maxDevices: Infinity, })} /> ); @@ -130,6 +133,9 @@ export function DeclinedIncomingAudioCall(): JSX.Element { type: CallType.Audio, direction: CallDirection.Incoming, status: DirectCallStatus.Declined, + groupCallEnded: null, + deviceCount: 0, + maxDevices: Infinity, })} /> ); @@ -143,6 +149,9 @@ export function DeclinedIncomingVideoCall(): JSX.Element { type: CallType.Video, direction: CallDirection.Incoming, status: DirectCallStatus.Declined, + groupCallEnded: null, + deviceCount: 0, + maxDevices: Infinity, })} /> ); @@ -156,6 +165,9 @@ export function AcceptedOutgoingAudioCall(): JSX.Element { type: CallType.Audio, direction: CallDirection.Outgoing, status: DirectCallStatus.Accepted, + groupCallEnded: null, + deviceCount: 0, + maxDevices: Infinity, })} /> ); @@ -169,6 +181,9 @@ export function AcceptedOutgoingVideoCall(): JSX.Element { type: CallType.Video, direction: CallDirection.Outgoing, status: DirectCallStatus.Accepted, + groupCallEnded: null, + deviceCount: 0, + maxDevices: Infinity, })} /> ); @@ -182,6 +197,9 @@ export function DeclinedOutgoingAudioCall(): JSX.Element { type: CallType.Audio, direction: CallDirection.Outgoing, status: DirectCallStatus.Declined, + groupCallEnded: null, + deviceCount: 0, + maxDevices: Infinity, })} /> ); @@ -195,6 +213,9 @@ export function DeclinedOutgoingVideoCall(): JSX.Element { type: CallType.Video, direction: CallDirection.Outgoing, status: DirectCallStatus.Declined, + groupCallEnded: null, + deviceCount: 0, + maxDevices: Infinity, })} /> ); @@ -209,7 +230,9 @@ export function TwoIncomingDirectCallsBackToBack(): JSX.Element { type: CallType.Video, direction: CallDirection.Incoming, status: DirectCallStatus.Declined, - callExternalState: CallExternalState.Ended, + groupCallEnded: null, + deviceCount: 0, + maxDevices: Infinity, })} isNextItemCallingNotification /> @@ -219,6 +242,9 @@ export function TwoIncomingDirectCallsBackToBack(): JSX.Element { type: CallType.Audio, direction: CallDirection.Incoming, status: DirectCallStatus.Declined, + groupCallEnded: null, + deviceCount: 0, + maxDevices: Infinity, })} /> @@ -238,7 +264,9 @@ export function TwoOutgoingDirectCallsBackToBack(): JSX.Element { type: CallType.Video, direction: CallDirection.Outgoing, status: DirectCallStatus.Declined, - callExternalState: CallExternalState.Ended, + groupCallEnded: null, + deviceCount: 0, + maxDevices: Infinity, })} isNextItemCallingNotification /> @@ -248,6 +276,9 @@ export function TwoOutgoingDirectCallsBackToBack(): JSX.Element { type: CallType.Audio, direction: CallDirection.Outgoing, status: DirectCallStatus.Declined, + groupCallEnded: null, + deviceCount: 0, + maxDevices: Infinity, })} /> @@ -267,6 +298,9 @@ export function GroupCallByUnknown(): JSX.Element { direction: CallDirection.Incoming, status: GroupCallStatus.Accepted, callCreator: null, + groupCallEnded: false, + deviceCount: 1, + maxDevices: 8, })} /> ); @@ -280,6 +314,9 @@ export function GroupCallByYou(): JSX.Element { type: CallType.Group, direction: CallDirection.Outgoing, status: GroupCallStatus.Accepted, + groupCallEnded: false, + deviceCount: 1, + maxDevices: 8, })} /> ); @@ -293,6 +330,9 @@ export function GroupCallBySomeone(): JSX.Element { type: CallType.Group, direction: CallDirection.Incoming, status: GroupCallStatus.GenericGroupCall, + groupCallEnded: false, + deviceCount: 1, + maxDevices: 8, })} /> ); @@ -309,6 +349,9 @@ export function GroupCallStartedBySomeoneWithALongName(): JSX.Element { callCreator: getDefaultConversation({ name: '😤🪐🦆'.repeat(50), }), + groupCallEnded: false, + deviceCount: 1, + maxDevices: 8, })} /> ); @@ -326,7 +369,9 @@ export function GroupCallActiveCallFull(): JSX.Element { type: CallType.Group, direction: CallDirection.Incoming, status: GroupCallStatus.GenericGroupCall, - callExternalState: CallExternalState.Full, + groupCallEnded: false, + deviceCount: 8, + maxDevices: 8, })} /> ); @@ -344,7 +389,9 @@ export function GroupCallEnded(): JSX.Element { type: CallType.Group, direction: CallDirection.Incoming, status: GroupCallStatus.GenericGroupCall, - callExternalState: CallExternalState.Ended, + groupCallEnded: true, + deviceCount: 0, + maxDevices: Infinity, })} /> ); diff --git a/ts/components/conversation/CallingNotification.tsx b/ts/components/conversation/CallingNotification.tsx index bf368064ab29..dbc60eca2013 100644 --- a/ts/components/conversation/CallingNotification.tsx +++ b/ts/components/conversation/CallingNotification.tsx @@ -12,7 +12,6 @@ import type { LocalizerType } from '../../types/Util'; import { CallMode } from '../../types/Calling'; import type { CallingNotificationType } from '../../util/callingNotification'; import { - CallExternalState, getCallingIcon, getCallingNotificationText, } from '../../util/callingNotification'; @@ -110,10 +109,7 @@ function renderCallingNotificationButton( direction === CallDirection.Incoming ? i18n('icu:calling__call-back') : i18n('icu:calling__call-again'); - if ( - props.callExternalState === CallExternalState.Joined || - props.callExternalState === CallExternalState.InOtherCall - ) { + if (props.activeConversationId != null) { disabledTooltipText = i18n('icu:calling__in-another-call-tooltip'); onClick = noop; } else { @@ -127,17 +123,19 @@ function renderCallingNotificationButton( break; } case CallMode.Group: { - if (props.callExternalState === CallExternalState.Ended) { + if (props.groupCallEnded) { return null; } - if (props.callExternalState === CallExternalState.Joined) { - buttonText = i18n('icu:calling__return'); - onClick = returnToActiveCall; - } else if (props.callExternalState === CallExternalState.InOtherCall) { - buttonText = i18n('icu:calling__join'); - disabledTooltipText = i18n('icu:calling__in-another-call-tooltip'); - onClick = noop; - } else if (props.callExternalState === CallExternalState.Full) { + if (props.activeConversationId != null) { + if (props.activeConversationId === conversationId) { + buttonText = i18n('icu:calling__return'); + onClick = returnToActiveCall; + } else { + buttonText = i18n('icu:calling__join'); + disabledTooltipText = i18n('icu:calling__in-another-call-tooltip'); + onClick = noop; + } + } else if (props.deviceCount > props.maxDevices) { buttonText = i18n('icu:calling__call-is-full'); disabledTooltipText = i18n( 'icu:calling__call-notification__button__call-full-tooltip', @@ -146,13 +144,11 @@ function renderCallingNotificationButton( } ); onClick = noop; - } else if (props.callExternalState === CallExternalState.Active) { + } else { buttonText = i18n('icu:calling__join'); onClick = () => { startCallingLobby({ conversationId, isVideoCall: true }); }; - } else { - throw missingCaseError(props.callExternalState); } break; } diff --git a/ts/state/selectors/message.ts b/ts/state/selectors/message.ts index 11490e908b81..c291e9fa4110 100644 --- a/ts/state/selectors/message.ts +++ b/ts/state/selectors/message.ts @@ -64,7 +64,6 @@ import { isVoiceMessage, canBeDownloaded } from '../../types/Attachment'; import { ReadStatus } from '../../messages/MessageReadStatus'; import type { CallingNotificationType } from '../../util/callingNotification'; -import { CallExternalState } from '../../util/callingNotification'; import { getRecipients } from '../../util/getRecipients'; import { getOwn } from '../../util/getOwn'; import { isNotNil } from '../../util/isNotNil'; @@ -134,6 +133,7 @@ import { getMessageSentTimestamp } from '../../util/getMessageSentTimestamp'; import type { CallHistorySelectorType } from './callHistory'; import { CallMode } from '../../types/Calling'; import { CallDirection } from '../../types/CallDisposition'; +import { getCallIdFromEra } from '../../util/callDisposition'; export { isIncoming, isOutgoing, isStory }; @@ -1317,10 +1317,11 @@ export type GetPropsForCallHistoryOptions = Pick< | 'ourConversationId' >; -const emptyCallNotification = { +const emptyCallNotification: CallingNotificationType = { callHistory: null, callCreator: null, - callExternalState: CallExternalState.Ended, + activeConversationId: null, + groupCallEnded: null, maxDevices: Infinity, deviceCount: 0, }; @@ -1346,6 +1347,8 @@ export function getPropsForCallHistory( return emptyCallNotification; } + const activeConversationId = activeCall?.conversationId ?? null; + const conversation = conversationSelector(callHistory.peerId); strictAssert( conversation != null, @@ -1359,38 +1362,39 @@ export function getPropsForCallHistory( callCreator = conversationSelector(ourConversationId); } - const call = callSelector(callHistory.callId); - - let deviceCount = 0; - let maxDevices = Infinity; - if ( - call?.callMode === CallMode.Group && - call.peekInfo?.deviceCount != null && - call.peekInfo?.maxDevices != null - ) { - deviceCount = call.peekInfo.deviceCount; - maxDevices = call.peekInfo.maxDevices; + if (callHistory.mode === CallMode.Direct) { + return { + callHistory, + callCreator, + activeConversationId, + groupCallEnded: false, + deviceCount: 0, + maxDevices: Infinity, + }; } - let callExternalState: CallExternalState; - if (call == null || deviceCount === 0) { - callExternalState = CallExternalState.Ended; - } else if (activeCall != null) { - if (activeCall.conversationId === call.conversationId) { - callExternalState = CallExternalState.Joined; - } else { - callExternalState = CallExternalState.InOtherCall; - } - } else if (deviceCount >= maxDevices) { - callExternalState = CallExternalState.Full; - } else { - callExternalState = CallExternalState.Active; + // This could be a later call in the conversation + const conversationCall = callSelector(conversation.id); + + if (conversationCall != null) { + strictAssert( + conversationCall?.callMode === CallMode.Group, + 'getPropsForCallHistory: Call was expected to be a group call' + ); } + const conversationCallId = + conversationCall?.peekInfo?.eraId != null && + getCallIdFromEra(conversationCall.peekInfo.eraId); + + const deviceCount = conversationCall?.peekInfo?.deviceCount ?? 0; + const maxDevices = conversationCall?.peekInfo?.maxDevices ?? Infinity; + return { callHistory, callCreator, - callExternalState, + activeConversationId, + groupCallEnded: callId !== conversationCallId || deviceCount === 0, deviceCount, maxDevices, }; diff --git a/ts/test-both/util/callingNotification_test.ts b/ts/test-both/util/callingNotification_test.ts index 107a43073bf2..1d68802b9308 100644 --- a/ts/test-both/util/callingNotification_test.ts +++ b/ts/test-both/util/callingNotification_test.ts @@ -2,10 +2,7 @@ // SPDX-License-Identifier: AGPL-3.0-only import { assert } from 'chai'; -import { - CallExternalState, - getCallingNotificationText, -} from '../../util/callingNotification'; +import { getCallingNotificationText } from '../../util/callingNotification'; import { CallMode } from '../../types/Calling'; import { setupI18n } from '../../util/setupI18n'; import enMessages from '../../../_locales/en/messages.json'; @@ -42,7 +39,8 @@ describe('calling notification helpers', () => { status: GroupCallStatus.Missed, }, callCreator, - callExternalState: CallExternalState.Ended, + activeConversationId: null, + groupCallEnded: true, deviceCount: 1, maxDevices: 23, }, @@ -70,7 +68,8 @@ describe('calling notification helpers', () => { status: GroupCallStatus.Ringing, }, callCreator, - callExternalState: CallExternalState.Active, + activeConversationId: null, + groupCallEnded: false, deviceCount: 1, maxDevices: 23, }, @@ -99,7 +98,8 @@ describe('calling notification helpers', () => { status: GroupCallStatus.Ringing, }, callCreator, - callExternalState: CallExternalState.Active, + activeConversationId: null, + groupCallEnded: false, deviceCount: 1, maxDevices: 23, }, @@ -127,7 +127,8 @@ describe('calling notification helpers', () => { status: GroupCallStatus.Ringing, }, callCreator, - callExternalState: CallExternalState.Active, + activeConversationId: null, + groupCallEnded: false, deviceCount: 1, maxDevices: 23, }, @@ -152,7 +153,8 @@ describe('calling notification helpers', () => { status: GroupCallStatus.Ringing, }, callCreator: null, - callExternalState: CallExternalState.Active, + activeConversationId: null, + groupCallEnded: false, deviceCount: 1, maxDevices: 23, }, diff --git a/ts/util/callingNotification.ts b/ts/util/callingNotification.ts index 4d979b60ec74..effdb4b47bb0 100644 --- a/ts/util/callingNotification.ts +++ b/ts/util/callingNotification.ts @@ -12,20 +12,14 @@ import { CallType, } from '../types/CallDisposition'; import type { ConversationType } from '../state/ducks/conversations'; - -export enum CallExternalState { - Active, - Full, - Joined, - Ended, - InOtherCall, -} +import { strictAssert } from './assert'; export type CallingNotificationType = Readonly<{ // In some older calls, we don't have a call id, this hardens against that. callHistory: CallHistoryDetails | null; callCreator: ConversationType | null; - callExternalState: CallExternalState; + activeConversationId: string | null; + groupCallEnded: boolean | null; deviceCount: number; maxDevices: number; }>; @@ -90,11 +84,11 @@ function getDirectCallNotificationText( } function getGroupCallNotificationText( - callExternalState: CallExternalState, + groupCallEnded: boolean, creator: ConversationType | null, i18n: LocalizerType ): string { - if (callExternalState === CallExternalState.Ended) { + if (groupCallEnded) { return i18n('icu:calling__call-notification__ended'); } if (creator == null) { @@ -112,7 +106,7 @@ export function getCallingNotificationText( callingNotification: CallingNotificationType, i18n: LocalizerType ): string | null { - const { callHistory, callCreator, callExternalState } = callingNotification; + const { callHistory, callCreator, groupCallEnded } = callingNotification; if (callHistory == null) { return null; } @@ -126,7 +120,11 @@ export function getCallingNotificationText( ); } if (callHistory.mode === CallMode.Group) { - return getGroupCallNotificationText(callExternalState, callCreator, i18n); + strictAssert( + groupCallEnded != null, + 'getCallingNotificationText: groupCallEnded shouldnt be null for a group call' + ); + return getGroupCallNotificationText(groupCallEnded, callCreator, i18n); } throw missingCaseError(callHistory.mode); }