Don't ring large groups

This commit is contained in:
Evan Hahn 2021-09-02 17:34:38 -05:00 committed by GitHub
parent 1f45bce0a2
commit 3e18a8a337
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 144 additions and 44 deletions

View File

@ -70,7 +70,6 @@ const createProps = (storyProps: Partial<PropsType> = {}): PropsType => ({
i18n, i18n,
isGroupCallOutboundRingEnabled: true, isGroupCallOutboundRingEnabled: true,
keyChangeOk: action('key-change-ok'), keyChangeOk: action('key-change-ok'),
maxGroupCallRingSize: 16,
me: { me: {
...getDefaultConversation({ ...getDefaultConversation({
color: select( color: select(

View File

@ -81,7 +81,6 @@ export type PropsType = {
declineCall: (_: DeclineCallType) => void; declineCall: (_: DeclineCallType) => void;
i18n: LocalizerType; i18n: LocalizerType;
isGroupCallOutboundRingEnabled: boolean; isGroupCallOutboundRingEnabled: boolean;
maxGroupCallRingSize: number;
me: MeType; me: MeType;
notifyForCall: (title: string, isVideoCall: boolean) => unknown; notifyForCall: (title: string, isVideoCall: boolean) => unknown;
openSystemPreferencesAction: () => unknown; openSystemPreferencesAction: () => unknown;
@ -116,7 +115,6 @@ const ActiveCallManager: React.FC<ActiveCallManagerPropsType> = ({
keyChangeOk, keyChangeOk,
getGroupCallVideoFrameSource, getGroupCallVideoFrameSource,
getPresentingSources, getPresentingSources,
maxGroupCallRingSize,
me, me,
openSystemPreferencesAction, openSystemPreferencesAction,
renderDeviceSelection, renderDeviceSelection,
@ -234,7 +232,6 @@ const ActiveCallManager: React.FC<ActiveCallManagerPropsType> = ({
isGroupCall={activeCall.callMode === CallMode.Group} isGroupCall={activeCall.callMode === CallMode.Group}
isGroupCallOutboundRingEnabled={isGroupCallOutboundRingEnabled} isGroupCallOutboundRingEnabled={isGroupCallOutboundRingEnabled}
isCallFull={isCallFull} isCallFull={isCallFull}
maxGroupCallRingSize={maxGroupCallRingSize}
me={me} me={me}
onCallCanceled={cancelActiveCall} onCallCanceled={cancelActiveCall}
onJoinCall={joinActiveCall} onJoinCall={joinActiveCall}

View File

@ -57,7 +57,6 @@ const createProps = (overrideProps: Partial<PropsType> = {}): PropsType => {
isGroupCall, isGroupCall,
isGroupCallOutboundRingEnabled: true, isGroupCallOutboundRingEnabled: true,
isCallFull: boolean('isCallFull', overrideProps.isCallFull || false), isCallFull: boolean('isCallFull', overrideProps.isCallFull || false),
maxGroupCallRingSize: overrideProps.maxGroupCallRingSize || 16,
me: overrideProps.me || { me: overrideProps.me || {
color: AvatarColors[0], color: AvatarColors[0],
id: generateUuid(), id: generateUuid(),

View File

@ -20,6 +20,7 @@ import {
import { AvatarColorType } from '../types/Colors'; import { AvatarColorType } from '../types/Colors';
import { LocalizerType } from '../types/Util'; import { LocalizerType } from '../types/Util';
import { ConversationType } from '../state/ducks/conversations'; import { ConversationType } from '../state/ducks/conversations';
import { isConversationTooBigToRing } from '../conversations/isConversationTooBigToRing';
export type PropsType = { export type PropsType = {
availableCameras: Array<MediaDeviceInfo>; availableCameras: Array<MediaDeviceInfo>;
@ -29,6 +30,7 @@ export type PropsType = {
| 'avatarPath' | 'avatarPath'
| 'color' | 'color'
| 'isMe' | 'isMe'
| 'memberships'
| 'name' | 'name'
| 'phoneNumber' | 'phoneNumber'
| 'profileName' | 'profileName'
@ -44,7 +46,6 @@ export type PropsType = {
isGroupCall: boolean; isGroupCall: boolean;
isGroupCallOutboundRingEnabled: boolean; isGroupCallOutboundRingEnabled: boolean;
isCallFull?: boolean; isCallFull?: boolean;
maxGroupCallRingSize: number;
me: { me: {
avatarPath?: string; avatarPath?: string;
id: string; id: string;
@ -74,7 +75,6 @@ export const CallingLobby = ({
isGroupCall = false, isGroupCall = false,
isGroupCallOutboundRingEnabled, isGroupCallOutboundRingEnabled,
isCallFull = false, isCallFull = false,
maxGroupCallRingSize,
me, me,
onCallCanceled, onCallCanceled,
onJoinCall, onJoinCall,
@ -150,21 +150,30 @@ export const CallingLobby = ({
? CallingButtonType.AUDIO_ON ? CallingButtonType.AUDIO_ON
: CallingButtonType.AUDIO_OFF; : CallingButtonType.AUDIO_OFF;
const isGroupTooLargeToRing = isConversationTooBigToRing(conversation);
const isRingButtonVisible: boolean = const isRingButtonVisible: boolean =
isGroupCall && isGroupCall &&
isGroupCallOutboundRingEnabled && isGroupCallOutboundRingEnabled &&
peekedParticipants.length === 0 && peekedParticipants.length === 0 &&
(groupMembers || []).length > 1; (groupMembers || []).length > 1;
const preCallInfoRingMode: RingMode = let preCallInfoRingMode: RingMode;
isGroupCall && !outgoingRing ? RingMode.WillNotRing : RingMode.WillRing; if (isGroupCall) {
preCallInfoRingMode =
outgoingRing && !isGroupTooLargeToRing
? RingMode.WillRing
: RingMode.WillNotRing;
} else {
preCallInfoRingMode = RingMode.WillRing;
}
let ringButtonType: let ringButtonType:
| CallingButtonType.RING_DISABLED | CallingButtonType.RING_DISABLED
| CallingButtonType.RING_ON | CallingButtonType.RING_ON
| CallingButtonType.RING_OFF; | CallingButtonType.RING_OFF;
if (isRingButtonVisible) { if (isRingButtonVisible) {
if ((groupMembers || []).length > maxGroupCallRingSize) { if (isGroupTooLargeToRing) {
ringButtonType = CallingButtonType.RING_DISABLED; ringButtonType = CallingButtonType.RING_DISABLED;
} else if (outgoingRing) { } else if (outgoingRing) {
ringButtonType = CallingButtonType.RING_ON; ringButtonType = CallingButtonType.RING_ON;

View File

@ -0,0 +1,14 @@
// Copyright 2021 Signal Messenger, LLC
// SPDX-License-Identifier: AGPL-3.0-only
import { parseIntWithFallback } from '../util/parseIntWithFallback';
import { getValue } from '../RemoteConfig';
import type { ConversationType } from '../state/ducks/conversations';
const getMaxGroupCallRingSize = (): number =>
parseIntWithFallback(getValue('global.calling.maxGroupCallRingSize'), 16);
export const isConversationTooBigToRing = (
conversation: Readonly<Pick<ConversationType, 'memberships'>>
): boolean =>
(conversation.memberships?.length || 0) >= getMaxGroupCallRingSize();

View File

@ -3,7 +3,6 @@
import { isNumber } from 'lodash'; import { isNumber } from 'lodash';
import { parseIntOrThrow } from '../util/parseIntOrThrow'; import { parseIntOrThrow } from '../util/parseIntOrThrow';
import { parseIntWithFallback } from '../util/parseIntWithFallback';
import { getValue, ConfigKeyType } from '../RemoteConfig'; import { getValue, ConfigKeyType } from '../RemoteConfig';
function makeGetter(configKey: ConfigKeyType): (fallback?: number) => number { function makeGetter(configKey: ConfigKeyType): (fallback?: number) => number {
@ -29,6 +28,3 @@ export const getGroupSizeRecommendedLimit = makeGetter(
export const getGroupSizeHardLimit = makeGetter( export const getGroupSizeHardLimit = makeGetter(
'global.groupsv2.groupSizeHardLimit' 'global.groupsv2.groupSizeHardLimit'
); );
export const getMaxGroupCallRingSize = (): number =>
parseIntWithFallback(getValue('global.calling.maxGroupCallRingSize'), 16);

View File

@ -40,6 +40,7 @@ import {
GroupCallPeekInfoType, GroupCallPeekInfoType,
} from '../state/ducks/calling'; } from '../state/ducks/calling';
import { getConversationCallMode } from '../state/ducks/conversations'; import { getConversationCallMode } from '../state/ducks/conversations';
import { isConversationTooBigToRing } from '../conversations/isConversationTooBigToRing';
import { import {
AudioDevice, AudioDevice,
AvailableIODevicesType, AvailableIODevicesType,
@ -378,6 +379,9 @@ export class CallingClass {
this.uxActions.showCallLobby({ this.uxActions.showCallLobby({
callMode: CallMode.Group, callMode: CallMode.Group,
conversationId: conversationProps.id, conversationId: conversationProps.id,
isConversationTooBigToRing: isConversationTooBigToRing(
conversationProps
),
...this.formatGroupCallForRedux(groupCall), ...this.formatGroupCallForRedux(groupCall),
}); });
break; break;

View File

@ -11,6 +11,7 @@ import {
import { has, omit } from 'lodash'; import { has, omit } from 'lodash';
import { getOwn } from '../../util/getOwn'; import { getOwn } from '../../util/getOwn';
import { getPlatform } from '../selectors/user'; import { getPlatform } from '../selectors/user';
import { isConversationTooBigToRing } from '../../conversations/isConversationTooBigToRing';
import { missingCaseError } from '../../util/missingCaseError'; import { missingCaseError } from '../../util/missingCaseError';
import { calling } from '../../services/calling'; import { calling } from '../../services/calling';
import { StateType as RootStateType } from '../reducer'; import { StateType as RootStateType } from '../reducer';
@ -30,6 +31,7 @@ import { callingTones } from '../../util/callingTones';
import { requestCameraPermissions } from '../../util/callingPermissions'; import { requestCameraPermissions } from '../../util/callingPermissions';
import { sleep } from '../../util/sleep'; import { sleep } from '../../util/sleep';
import { LatestQueue } from '../../util/LatestQueue'; import { LatestQueue } from '../../util/LatestQueue';
import type { ConversationChangedActionType } from './conversations';
// State // State
@ -228,6 +230,7 @@ export type ShowCallLobbyType =
joinState: GroupCallJoinState; joinState: GroupCallJoinState;
hasLocalAudio: boolean; hasLocalAudio: boolean;
hasLocalVideo: boolean; hasLocalVideo: boolean;
isConversationTooBigToRing: boolean;
peekInfo?: GroupCallPeekInfoType; peekInfo?: GroupCallPeekInfoType;
remoteParticipants: Array<GroupCallParticipantInfoType>; remoteParticipants: Array<GroupCallParticipantInfoType>;
}; };
@ -465,6 +468,7 @@ export type CallingActionType =
| CallStateChangeFulfilledActionType | CallStateChangeFulfilledActionType
| ChangeIODeviceFulfilledActionType | ChangeIODeviceFulfilledActionType
| CloseNeedPermissionScreenActionType | CloseNeedPermissionScreenActionType
| ConversationChangedActionType
| DeclineCallActionType | DeclineCallActionType
| GroupCallStateChangeActionType | GroupCallStateChangeActionType
| HangUpActionType | HangUpActionType
@ -1069,9 +1073,22 @@ function startCall(
}); });
break; break;
case CallMode.Group: { case CallMode.Group: {
const outgoingRing = Boolean( let outgoingRing: boolean;
getState().calling.activeCallState?.outgoingRing
); const state = getState();
const { activeCallState } = state.calling;
if (activeCallState?.outgoingRing) {
const conversation = getOwn(
state.conversations.conversationLookup,
activeCallState.conversationId
);
outgoingRing = Boolean(
conversation && !isConversationTooBigToRing(conversation)
);
} else {
outgoingRing = false;
}
calling.joinGroupCall( calling.joinGroupCall(
payload.conversationId, payload.conversationId,
payload.hasLocalAudio, payload.hasLocalAudio,
@ -1220,6 +1237,7 @@ export function reducer(
// We expect to be in this state briefly. The Calling service should update the // We expect to be in this state briefly. The Calling service should update the
// call state shortly. // call state shortly.
const existingCall = getGroupCall(conversationId, state); const existingCall = getGroupCall(conversationId, state);
const ringState = getGroupCallRingState(existingCall);
call = { call = {
callMode: CallMode.Group, callMode: CallMode.Group,
conversationId, conversationId,
@ -1232,10 +1250,13 @@ export function reducer(
deviceCount: action.payload.remoteParticipants.length, deviceCount: action.payload.remoteParticipants.length,
}, },
remoteParticipants: action.payload.remoteParticipants, remoteParticipants: action.payload.remoteParticipants,
...getGroupCallRingState(existingCall), ...ringState,
}; };
outgoingRing = outgoingRing =
!call.peekInfo.uuids.length && !call.remoteParticipants.length; !ringState.ringId &&
!call.peekInfo.uuids.length &&
!call.remoteParticipants.length &&
!action.payload.isConversationTooBigToRing;
break; break;
} }
default: default:
@ -1352,6 +1373,25 @@ export function reducer(
}; };
} }
if (action.type === 'CONVERSATION_CHANGED') {
const activeCall = getActiveCall(state);
const { activeCallState } = state;
if (
!activeCallState?.outgoingRing ||
activeCallState.conversationId !== action.payload.id ||
activeCall?.callMode !== CallMode.Group ||
activeCall.joinState !== GroupCallJoinState.NotJoined ||
!isConversationTooBigToRing(action.payload.data)
) {
return state;
}
return {
...state,
activeCallState: { ...activeCallState, outgoingRing: false },
};
}
if (action.type === DECLINE_DIRECT_CALL) { if (action.type === DECLINE_DIRECT_CALL) {
return removeConversationFromState(state, action.payload.conversationId); return removeConversationFromState(state, action.payload.conversationId);
} }

View File

@ -443,7 +443,7 @@ type ConversationAddedActionType = {
data: ConversationType; data: ConversationType;
}; };
}; };
type ConversationChangedActionType = { export type ConversationChangedActionType = {
type: 'CONVERSATION_CHANGED'; type: 'CONVERSATION_CHANGED';
payload: { payload: {
id: string; id: string;

View File

@ -12,7 +12,6 @@ import { getMe, getConversationSelector } from '../selectors/conversations';
import { getActiveCall } from '../ducks/calling'; import { getActiveCall } from '../ducks/calling';
import { ConversationType } from '../ducks/conversations'; import { ConversationType } from '../ducks/conversations';
import { getIncomingCall } from '../selectors/calling'; import { getIncomingCall } from '../selectors/calling';
import { getMaxGroupCallRingSize } from '../../groups/limits';
import { isGroupCallOutboundRingEnabled } from '../../util/isGroupCallOutboundRingEnabled'; import { isGroupCallOutboundRingEnabled } from '../../util/isGroupCallOutboundRingEnabled';
import { import {
ActiveCallType, ActiveCallType,
@ -297,7 +296,6 @@ const mapStateToProps = (state: StateType) => ({
i18n: getIntl(state), i18n: getIntl(state),
isGroupCallOutboundRingEnabled: isGroupCallOutboundRingEnabled(), isGroupCallOutboundRingEnabled: isGroupCallOutboundRingEnabled(),
incomingCall: mapStateToIncomingCallProp(state), incomingCall: mapStateToIncomingCallProp(state),
maxGroupCallRingSize: getMaxGroupCallRingSize(),
me: { me: {
...getMe(state), ...getMe(state),
// `getMe` returns a `ConversationType` which might not have a UUID, at least // `getMe` returns a `ConversationType` which might not have a UUID, at least

View File

@ -0,0 +1,61 @@
// Copyright 2021 Signal Messenger, LLC
// SPDX-License-Identifier: AGPL-3.0-only
import { assert } from 'chai';
import * as sinon from 'sinon';
import { times } from 'lodash';
import { v4 as uuid } from 'uuid';
import * as remoteConfig from '../../RemoteConfig';
import { isConversationTooBigToRing } from '../../conversations/isConversationTooBigToRing';
describe('isConversationTooBigToRing', () => {
let sinonSandbox: sinon.SinonSandbox;
let getMaxGroupCallRingSizeStub: sinon.SinonStub;
beforeEach(() => {
sinonSandbox = sinon.createSandbox();
const getValueStub = sinonSandbox.stub(remoteConfig, 'getValue');
getMaxGroupCallRingSizeStub = getValueStub.withArgs(
'global.calling.maxGroupCallRingSize'
);
});
const fakeMemberships = (count: number) =>
times(count, () => ({ conversationId: uuid(), isAdmin: false }));
afterEach(() => {
sinonSandbox.restore();
});
it('returns false if there are no memberships (i.e., for a direct conversation)', () => {
assert.isFalse(isConversationTooBigToRing({}));
assert.isFalse(isConversationTooBigToRing({ memberships: [] }));
});
const textMaximum = (max: number): void => {
for (let count = 1; count < max; count += 1) {
const memberships = fakeMemberships(count);
assert.isFalse(isConversationTooBigToRing({ memberships }));
}
for (let count = max; count < max + 5; count += 1) {
const memberships = fakeMemberships(count);
assert.isTrue(isConversationTooBigToRing({ memberships }));
}
};
it('returns whether there are 16 or more people in the group, if there is nothing in remote config', () => {
textMaximum(16);
});
it('returns whether there are 16 or more people in the group, if the remote config value is bogus', () => {
getMaxGroupCallRingSizeStub.returns('uh oh');
textMaximum(16);
});
it('returns whether there are 9 or more people in the group, if the remote config value is 9', () => {
getMaxGroupCallRingSizeStub.returns('9');
textMaximum(9);
});
});

View File

@ -8,14 +8,12 @@ import * as remoteConfig from '../../RemoteConfig';
import { import {
getGroupSizeRecommendedLimit, getGroupSizeRecommendedLimit,
getGroupSizeHardLimit, getGroupSizeHardLimit,
getMaxGroupCallRingSize,
} from '../../groups/limits'; } from '../../groups/limits';
describe('group limit utilities', () => { describe('group limit utilities', () => {
let sinonSandbox: sinon.SinonSandbox; let sinonSandbox: sinon.SinonSandbox;
let getRecommendedLimitStub: sinon.SinonStub; let getRecommendedLimitStub: sinon.SinonStub;
let getHardLimitStub: sinon.SinonStub; let getHardLimitStub: sinon.SinonStub;
let getMaxGroupCallRingSizeStub: sinon.SinonStub;
beforeEach(() => { beforeEach(() => {
sinonSandbox = sinon.createSandbox(); sinonSandbox = sinon.createSandbox();
@ -27,9 +25,6 @@ describe('group limit utilities', () => {
getHardLimitStub = getValueStub.withArgs( getHardLimitStub = getValueStub.withArgs(
'global.groupsv2.groupSizeHardLimit' 'global.groupsv2.groupSizeHardLimit'
); );
getMaxGroupCallRingSizeStub = getValueStub.withArgs(
'global.calling.maxGroupCallRingSize'
);
}); });
afterEach(() => { afterEach(() => {
@ -69,21 +64,4 @@ describe('group limit utilities', () => {
assert.strictEqual(getGroupSizeHardLimit(), 123); assert.strictEqual(getGroupSizeHardLimit(), 123);
}); });
}); });
describe('getMaxGroupCallRingSize', () => {
it('returns 16 if the value in remote config is not defined', () => {
getMaxGroupCallRingSizeStub.returns(undefined);
assert.strictEqual(getMaxGroupCallRingSize(), 16);
});
it('returns 16 if the value in remote config is not a parseable integer', () => {
getMaxGroupCallRingSizeStub.returns('uh oh');
assert.strictEqual(getMaxGroupCallRingSize(), 16);
});
it('returns the value in remote config, parsed as an integer', () => {
getMaxGroupCallRingSizeStub.returns('123');
assert.strictEqual(getMaxGroupCallRingSize(), 123);
});
});
}); });

View File

@ -1639,6 +1639,7 @@ describe('calling duck', () => {
conversationId: 'fake-conversation-id', conversationId: 'fake-conversation-id',
hasLocalAudio: true, hasLocalAudio: true,
hasLocalVideo: true, hasLocalVideo: true,
isConversationTooBigToRing: false,
connectionState: GroupCallConnectionState.Connected, connectionState: GroupCallConnectionState.Connected,
joinState: GroupCallJoinState.NotJoined, joinState: GroupCallJoinState.NotJoined,
peekInfo: { peekInfo: {
@ -1701,6 +1702,7 @@ describe('calling duck', () => {
conversationId: 'fake-group-call-conversation-id', conversationId: 'fake-group-call-conversation-id',
hasLocalAudio: true, hasLocalAudio: true,
hasLocalVideo: true, hasLocalVideo: true,
isConversationTooBigToRing: false,
connectionState: GroupCallConnectionState.Connected, connectionState: GroupCallConnectionState.Connected,
joinState: GroupCallJoinState.NotJoined, joinState: GroupCallJoinState.NotJoined,
peekInfo: undefined, peekInfo: undefined,
@ -1725,6 +1727,7 @@ describe('calling duck', () => {
conversationId: 'fake-group-call-conversation-id', conversationId: 'fake-group-call-conversation-id',
hasLocalAudio: true, hasLocalAudio: true,
hasLocalVideo: true, hasLocalVideo: true,
isConversationTooBigToRing: false,
connectionState: GroupCallConnectionState.Connected, connectionState: GroupCallConnectionState.Connected,
joinState: GroupCallJoinState.NotJoined, joinState: GroupCallJoinState.NotJoined,
peekInfo: undefined, peekInfo: undefined,
@ -1761,6 +1764,7 @@ describe('calling duck', () => {
conversationId: 'fake-group-call-conversation-id', conversationId: 'fake-group-call-conversation-id',
hasLocalAudio: true, hasLocalAudio: true,
hasLocalVideo: true, hasLocalVideo: true,
isConversationTooBigToRing: false,
connectionState: GroupCallConnectionState.Connected, connectionState: GroupCallConnectionState.Connected,
joinState: GroupCallJoinState.NotJoined, joinState: GroupCallJoinState.NotJoined,
peekInfo: { peekInfo: {
@ -1814,6 +1818,7 @@ describe('calling duck', () => {
conversationId: 'fake-group-call-conversation-id', conversationId: 'fake-group-call-conversation-id',
hasLocalAudio: true, hasLocalAudio: true,
hasLocalVideo: true, hasLocalVideo: true,
isConversationTooBigToRing: false,
connectionState: GroupCallConnectionState.Connected, connectionState: GroupCallConnectionState.Connected,
joinState: GroupCallJoinState.NotJoined, joinState: GroupCallJoinState.NotJoined,
peekInfo: undefined, peekInfo: undefined,