From bc67d421abe9013e25d1edb1a58c95227ad7c754 Mon Sep 17 00:00:00 2001 From: Jamie Kyle <113370520+jamiebuilds-signal@users.noreply.github.com> Date: Wed, 20 Sep 2023 07:00:01 -0700 Subject: [PATCH] Track acceptedTime during call, fix call screen duration --- ts/components/CallManager.tsx | 2 - ts/components/CallScreen.tsx | 8 +- ts/services/calling.ts | 7 ++ ts/state/ducks/calling.ts | 10 +- ts/state/smart/CallManager.tsx | 9 +- ts/test-electron/state/ducks/calling_test.ts | 115 ++++++++++-------- .../state/selectors/calling_test.ts | 1 + ts/types/Calling.ts | 4 +- 8 files changed, 92 insertions(+), 64 deletions(-) diff --git a/ts/components/CallManager.tsx b/ts/components/CallManager.tsx index 6453eb7c9f1..543141c81c0 100644 --- a/ts/components/CallManager.tsx +++ b/ts/components/CallManager.tsx @@ -149,7 +149,6 @@ function ActiveCallManager({ conversation, hasLocalAudio, hasLocalVideo, - joinedAt, peekedParticipants, pip, presentingSourcesAvailable, @@ -327,7 +326,6 @@ function ActiveCallManager({ groupMembers={groupMembers} hangUpActiveCall={hangUpActiveCall} i18n={i18n} - joinedAt={joinedAt} me={me} openSystemPreferencesAction={openSystemPreferencesAction} setGroupCallVideoRequest={setGroupCallVideoRequestForConversation} diff --git a/ts/components/CallScreen.tsx b/ts/components/CallScreen.tsx index 41d8a63ddab..60fa0a3d747 100644 --- a/ts/components/CallScreen.tsx +++ b/ts/components/CallScreen.tsx @@ -57,7 +57,6 @@ export type PropsType = { groupMembers?: Array>; hangUpActiveCall: (reason: string) => void; i18n: LocalizerType; - joinedAt?: number; me: ConversationType; openSystemPreferencesAction: () => unknown; setGroupCallVideoRequest: ( @@ -82,7 +81,7 @@ export type PropsType = { type DirectCallHeaderMessagePropsType = { i18n: LocalizerType; callState: CallState; - joinedAt?: number; + joinedAt: number | null; }; export const isInSpeakerView = ( @@ -104,7 +103,7 @@ function DirectCallHeaderMessage({ >(); useEffect(() => { - if (!joinedAt) { + if (joinedAt == null) { return noop; } // It's really jumpy with a value of 500ms. @@ -136,7 +135,6 @@ export function CallScreen({ groupMembers, hangUpActiveCall, i18n, - joinedAt, me, openSystemPreferencesAction, setGroupCallVideoRequest, @@ -289,7 +287,7 @@ export function CallScreen({ ); headerTitle = isRinging ? undefined : conversation.title; diff --git a/ts/services/calling.ts b/ts/services/calling.ts index 06d6accc0f4..3b455776b91 100644 --- a/ts/services/calling.ts +++ b/ts/services/calling.ts @@ -2101,8 +2101,14 @@ export class CallingClass { return; } + let acceptedTime: number | null = null; + // eslint-disable-next-line no-param-reassign call.handleStateChanged = async () => { + if (call.state === CallState.Accepted) { + acceptedTime = acceptedTime ?? Date.now(); + } + if (call.state === CallState.Ended) { this.stopDeviceReselectionTimer(); this.lastMediaDeviceSettings = undefined; @@ -2121,6 +2127,7 @@ export class CallingClass { conversationId: conversation.id, callState: call.state, callEndedReason: call.endedReason, + acceptedTime, }); }; diff --git a/ts/state/ducks/calling.ts b/ts/state/ducks/calling.ts index b052bff55e0..917bca6c876 100644 --- a/ts/state/ducks/calling.ts +++ b/ts/state/ducks/calling.ts @@ -121,7 +121,7 @@ export type ActiveCallStateType = { hasLocalVideo: boolean; localAudioLevel: number; viewMode: CallViewMode; - joinedAt?: number; + joinedAt: number | null; outgoingRing: boolean; pip: boolean; presentingSource?: PresentedSource; @@ -150,7 +150,7 @@ export type AcceptCallType = ReadonlyDeep<{ export type CallStateChangeType = ReadonlyDeep<{ conversationId: string; - acceptedTime?: number; + acceptedTime: number | null; callState: CallState; callEndedReason?: CallEndedReason; }>; @@ -1647,6 +1647,7 @@ export function reducer( settingsDialogOpen: false, showParticipantsList: false, outgoingRing, + joinedAt: null, }, }; } @@ -1675,6 +1676,7 @@ export function reducer( settingsDialogOpen: false, showParticipantsList: false, outgoingRing: true, + joinedAt: null, }, }; } @@ -1698,6 +1700,7 @@ export function reducer( settingsDialogOpen: false, showParticipantsList: false, outgoingRing: false, + joinedAt: null, }, }; } @@ -1852,6 +1855,7 @@ export function reducer( settingsDialogOpen: false, showParticipantsList: false, outgoingRing: true, + joinedAt: null, }, }; } @@ -1882,7 +1886,7 @@ export function reducer( ) { activeCallState = { ...state.activeCallState, - joinedAt: action.payload.acceptedTime, + joinedAt: action.payload.acceptedTime ?? null, }; } else { ({ activeCallState } = state); diff --git a/ts/state/smart/CallManager.tsx b/ts/state/smart/CallManager.tsx index 508736568c7..40423ebff3a 100644 --- a/ts/state/smart/CallManager.tsx +++ b/ts/state/smart/CallManager.tsx @@ -14,7 +14,10 @@ import type { ConversationType } from '../ducks/conversations'; import { getIncomingCall } from '../selectors/calling'; import { isGroupCallOutboundRingEnabled } from '../../util/isGroupCallOutboundRingEnabled'; import type { + ActiveCallBaseType, ActiveCallType, + ActiveDirectCallType, + ActiveGroupCallType, GroupCallRemoteParticipantType, } from '../../types/Calling'; import { isAciString } from '../../util/isAciString'; @@ -138,7 +141,7 @@ const mapStateToActiveCallProp = ( return convoForAci ? conversationSelector(convoForAci.id) : undefined; }); - const baseResult = { + const baseResult: ActiveCallBaseType = { conversation, hasLocalAudio: activeCallState.hasLocalAudio, hasLocalVideo: activeCallState.hasLocalVideo, @@ -185,7 +188,7 @@ const mapStateToActiveCallProp = ( serviceId: conversation.serviceId, }, ], - }; + } satisfies ActiveDirectCallType; case CallMode.Group: { const conversationsWithSafetyNumberChanges: Array = []; const groupMembers: Array = []; @@ -282,7 +285,7 @@ const mapStateToActiveCallProp = ( peekedParticipants, remoteParticipants, remoteAudioLevels: call.remoteAudioLevels || new Map(), - }; + } satisfies ActiveGroupCallType; } default: throw missingCaseError(call); diff --git a/ts/test-electron/state/ducks/calling_test.ts b/ts/test-electron/state/ducks/calling_test.ts index d63fcdbf6ca..e686d0726ab 100644 --- a/ts/test-electron/state/ducks/calling_test.ts +++ b/ts/test-electron/state/ducks/calling_test.ts @@ -4,12 +4,16 @@ import { assert } from 'chai'; import * as sinon from 'sinon'; import { cloneDeep, noop } from 'lodash'; +import type { PeekInfo } from '@signalapp/ringrtc'; import type { StateType as RootStateType } from '../../../state/reducer'; import { reducer as rootReducer } from '../../../state/reducer'; import { noopAction } from '../../../state/ducks/noop'; import type { + ActiveCallStateType, CallingStateType, + DirectCallStateType, GroupCallStateChangeActionType, + GroupCallStateType, } from '../../../state/ducks/calling'; import { actions, @@ -33,25 +37,30 @@ import type { UnwrapPromise } from '../../../types/Util'; const ACI_1 = generateAci(); +type CallingStateTypeWithActiveCall = CallingStateType & { + activeCallState: ActiveCallStateType; +}; + describe('calling duck', () => { + const directCallState: DirectCallStateType = { + callMode: CallMode.Direct, + conversationId: 'fake-direct-call-conversation-id', + callState: CallState.Accepted, + isIncoming: false, + isVideoCall: false, + hasRemoteVideo: false, + }; const stateWithDirectCall: CallingStateType = { ...getEmptyState(), callsByConversation: { - 'fake-direct-call-conversation-id': { - callMode: CallMode.Direct as CallMode.Direct, - conversationId: 'fake-direct-call-conversation-id', - callState: CallState.Accepted, - isIncoming: false, - isVideoCall: false, - hasRemoteVideo: false, - }, + [directCallState.conversationId]: directCallState, }, }; - const stateWithActiveDirectCall = { + const stateWithActiveDirectCall: CallingStateTypeWithActiveCall = { ...stateWithDirectCall, activeCallState: { - conversationId: 'fake-direct-call-conversation-id', + conversationId: directCallState.conversationId, hasLocalAudio: true, hasLocalVideo: false, localAudioLevel: 0, @@ -61,20 +70,21 @@ describe('calling duck', () => { outgoingRing: true, pip: false, settingsDialogOpen: false, + joinedAt: null, }, }; - const stateWithIncomingDirectCall = { + const stateWithIncomingDirectCall: CallingStateType = { ...getEmptyState(), callsByConversation: { 'fake-direct-call-conversation-id': { - callMode: CallMode.Direct as CallMode.Direct, + callMode: CallMode.Direct, conversationId: 'fake-direct-call-conversation-id', callState: CallState.Ringing, isIncoming: true, isVideoCall: false, hasRemoteVideo: false, - }, + } satisfies DirectCallStateType, }, }; @@ -83,11 +93,11 @@ describe('calling duck', () => { const remoteAci = generateAci(); const ringerAci = generateAci(); - const stateWithGroupCall = { + const stateWithGroupCall: CallingStateType = { ...getEmptyState(), callsByConversation: { 'fake-group-call-conversation-id': { - callMode: CallMode.Group as CallMode.Group, + callMode: CallMode.Group, conversationId: 'fake-group-call-conversation-id', connectionState: GroupCallConnectionState.Connected, joinState: GroupCallJoinState.NotJoined, @@ -109,11 +119,11 @@ describe('calling duck', () => { videoAspectRatio: 4 / 3, }, ], - }, + } satisfies GroupCallStateType, }, }; - const stateWithIncomingGroupCall = { + const stateWithIncomingGroupCall: CallingStateType = { ...stateWithGroupCall, callsByConversation: { ...stateWithGroupCall.callsByConversation, @@ -127,7 +137,7 @@ describe('calling duck', () => { }, }; - const stateWithActiveGroupCall = { + const stateWithActiveGroupCall: CallingStateTypeWithActiveCall = { ...stateWithGroupCall, activeCallState: { conversationId: 'fake-group-call-conversation-id', @@ -140,18 +150,20 @@ describe('calling duck', () => { outgoingRing: false, pip: false, settingsDialogOpen: false, + joinedAt: null, }, }; - const stateWithActivePresentationViewGroupCall = { - ...stateWithGroupCall, - activeCallState: { - ...stateWithActiveGroupCall.activeCallState, - viewMode: CallViewMode.Presentation, - }, - }; + const stateWithActivePresentationViewGroupCall: CallingStateTypeWithActiveCall = + { + ...stateWithGroupCall, + activeCallState: { + ...stateWithActiveGroupCall.activeCallState, + viewMode: CallViewMode.Presentation, + }, + }; - const stateWithActiveSpeakerViewGroupCall = { + const stateWithActiveSpeakerViewGroupCall: CallingStateTypeWithActiveCall = { ...stateWithGroupCall, activeCallState: { ...stateWithActiveGroupCall.activeCallState, @@ -240,20 +252,18 @@ describe('calling duck', () => { isSharingScreen: true, }; - const state = { + const state: CallingStateTypeWithActiveCall = { ...stateWithActiveDirectCall, }; const nextState = reducer(state, remoteSharingScreenChange(payload)); - const expectedState = { + const expectedState: CallingStateTypeWithActiveCall = { ...stateWithActiveDirectCall, callsByConversation: { - 'fake-direct-call-conversation-id': { - ...stateWithActiveDirectCall.callsByConversation[ - 'fake-direct-call-conversation-id' - ], + [directCallState.conversationId]: { + ...directCallState, isSharingScreen: true, - }, + } satisfies DirectCallStateType, }, }; @@ -276,7 +286,7 @@ describe('calling duck', () => { id: 'window:786', name: 'Application', }; - const getState = () => ({ + const getState = (): RootStateType => ({ ...getEmptyRootState(), calling: { ...stateWithActiveGroupCall, @@ -301,7 +311,7 @@ describe('calling duck', () => { id: 'window:786', name: 'Application', }; - const getState = () => ({ + const getState = (): RootStateType => ({ ...getEmptyRootState(), calling: { ...stateWithActiveGroupCall, @@ -325,7 +335,7 @@ describe('calling duck', () => { name: 'Application', }; - const getState = () => ({ + const getState = (): RootStateType => ({ ...getEmptyRootState(), calling: { ...stateWithActiveGroupCall, @@ -352,7 +362,7 @@ describe('calling duck', () => { const dispatch = sinon.spy(); const { setPresenting } = actions; - const getState = () => ({ + const getState = (): RootStateType => ({ ...getEmptyRootState(), calling: { ...stateWithActiveGroupCall, @@ -386,7 +396,7 @@ describe('calling duck', () => { }); describe('accepting a direct call', () => { - const getState = () => ({ + const getState = (): RootStateType => ({ ...getEmptyRootState(), calling: stateWithIncomingDirectCall, }); @@ -472,12 +482,13 @@ describe('calling duck', () => { outgoingRing: false, pip: false, settingsDialogOpen: false, - }); + joinedAt: null, + } satisfies ActiveCallStateType); }); }); describe('accepting a group call', () => { - const getState = () => ({ + const getState = (): RootStateType => ({ ...getEmptyRootState(), calling: stateWithIncomingGroupCall, }); @@ -565,7 +576,8 @@ describe('calling duck', () => { outgoingRing: false, pip: false, settingsDialogOpen: false, - }); + joinedAt: null, + } satisfies ActiveCallStateType); }); }); }); @@ -677,7 +689,7 @@ describe('calling duck', () => { }); describe('declining a direct call', () => { - const getState = () => ({ + const getState = (): RootStateType => ({ ...getEmptyRootState(), calling: stateWithIncomingDirectCall, }); @@ -735,7 +747,7 @@ describe('calling duck', () => { }); describe('declining a group call', () => { - const getState = () => ({ + const getState = (): RootStateType => ({ ...getEmptyRootState(), calling: stateWithIncomingGroupCall, }); @@ -1157,7 +1169,8 @@ describe('calling duck', () => { outgoingRing: false, pip: false, settingsDialogOpen: false, - }); + joinedAt: null, + } satisfies ActiveCallStateType); }); it('if the call is active, updates the active call state', () => { @@ -1225,7 +1238,7 @@ describe('calling duck', () => { }); it('stops ringing if someone enters the call', () => { - const state = { + const state: CallingStateType = { ...stateWithActiveGroupCall, activeCallState: { ...stateWithActiveGroupCall.activeCallState, @@ -1328,7 +1341,7 @@ describe('calling duck', () => { }); it('closes the PiP', () => { - const state = { + const state: CallingStateType = { ...stateWithActiveDirectCall, activeCallState: { ...stateWithActiveDirectCall.activeCallState, @@ -1576,9 +1589,11 @@ describe('calling duck', () => { noop, () => { const callingState = cloneDeep(stateWithGroupCall); - callingState.callsByConversation[ + const call = callingState.callsByConversation[ 'fake-group-call-conversation-id' - ].peekInfo.deviceCount = 8; + ] as GroupCallStateType; + const peekInfo = call.peekInfo as unknown as PeekInfo; + peekInfo.deviceCount = 8; return { ...rootState, calling: callingState }; }, null @@ -1686,7 +1701,8 @@ describe('calling duck', () => { pip: false, settingsDialogOpen: false, outgoingRing: true, - }); + joinedAt: null, + } satisfies ActiveCallStateType); }); it('saves a group call and makes it active', async () => { @@ -1972,6 +1988,7 @@ describe('calling duck', () => { pip: false, settingsDialogOpen: false, outgoingRing: true, + joinedAt: null, }); }); diff --git a/ts/test-electron/state/selectors/calling_test.ts b/ts/test-electron/state/selectors/calling_test.ts index cfe2d675425..6948fa4f757 100644 --- a/ts/test-electron/state/selectors/calling_test.ts +++ b/ts/test-electron/state/selectors/calling_test.ts @@ -72,6 +72,7 @@ describe('state/selectors/calling', () => { outgoingRing: true, pip: false, settingsDialogOpen: false, + joinedAt: null, }, }; diff --git a/ts/types/Calling.ts b/ts/types/Calling.ts index ececa6425cd..095b2a8aeac 100644 --- a/ts/types/Calling.ts +++ b/ts/types/Calling.ts @@ -32,14 +32,14 @@ export type PresentedSource = { name: string; }; -type ActiveCallBaseType = { +export type ActiveCallBaseType = { conversation: ConversationType; hasLocalAudio: boolean; hasLocalVideo: boolean; localAudioLevel: number; viewMode: CallViewMode; isSharingScreen?: boolean; - joinedAt?: number; + joinedAt: number | null; outgoingRing: boolean; pip: boolean; presentingSource?: PresentedSource;