[MM-58002] Review start call button functionality in profile popover (#…

…26867) * Review start call button functionality in profile popover * Address feedback * Use published @mattermost/calls-common package * Fix import * Lint fix
mattermost · May 8, 2024 · 5d4ad44 · 5d4ad44
1 parent be2ffbc
commit 5d4ad44
Show file tree

Hide file tree

Showing 10 changed files with 73 additions and 84 deletions.
diff --git a/webapp/channels/package.json b/webapp/channels/package.json
@@ -103,6 +103,7 @@
   "devDependencies": {
     "@deanwhillier/jest-matchmedia-mock": "1.2.0",
     "@hot-loader/react-dom": "17.0.2",
+    "@mattermost/calls-common": "0.27.0",
     "@mattermost/eslint-plugin": "*",
     "@redux-devtools/extension": "3.2.3",
     "@stylistic/stylelint-plugin": "2.1.0",

diff --git a/webapp/channels/src/components/profile_popover/index.test.tsx b/webapp/channels/src/components/profile_popover/index.test.tsx
@@ -14,6 +14,7 @@ import {General, Permissions} from 'mattermost-redux/constants';
 
 import {renderWithContext} from 'tests/react_testing_utils';
 import {TestHelper} from 'utils/test_helper';
+import {getDirectChannelName} from 'utils/utils';
 
 import type {GlobalState} from 'types/store';
 
@@ -52,6 +53,10 @@ function getBasePropsAndState(): [Props, DeepPartial<GlobalState>] {
     const currentUser = TestHelper.getUserMock({id: 'currentUser', roles: 'role'});
     const currentTeam = TestHelper.getTeamMock({id: 'currentTeam'});
     const channel = TestHelper.getChannelMock({id: 'channelId', team_id: currentTeam.id, type: General.OPEN_CHANNEL});
+    const dmChannel = {
+        id: 'dmChannelId',
+        name: getDirectChannelName(user.id, currentUser.id),
+    };
 
     const state: DeepPartial<GlobalState> = {
         entities: {
@@ -84,9 +89,11 @@ function getBasePropsAndState(): [Props, DeepPartial<GlobalState>] {
             channels: {
                 channels: {
                     [channel.id]: channel,
+                    [dmChannel.id]: dmChannel,
                 },
                 myMembers: {
                     [channel.id]: {},
+                    [dmChannel.id]: {},
                 },
             },
             general: {
@@ -140,7 +147,7 @@ function getBasePropsAndState(): [Props, DeepPartial<GlobalState>] {
         // eslint-disable-next-line @typescript-eslint/ban-ts-comment
         // @ts-ignore
         'plugins-com.mattermost.calls': {
-            profiles: {},
+            sessions: {},
         },
     };
     const props: Props = {
@@ -302,16 +309,16 @@ describe('components/ProfilePopover', () => {
         expect(screen.queryByLabelText('Start Call')).not.toBeInTheDocument();
     });
 
-    test('should disable start call button when user is in another call', async () => {
+    test('should disable start call button when call is ongoing in the DM', async () => {
         const [props, initialState] = getBasePropsAndState();
-        (initialState as any)['plugins-com.mattermost.calls'].profiles = {fakeChannel: {currentUser: {id: 'currentUser'}}};
+        (initialState as any)['plugins-com.mattermost.calls'].sessions = {dmChannelId: {currentUser: {user_id: 'currentUser'}}};
 
         renderWithPluginReducers(<ProfilePopover {...props}/>, initialState);
-        const button = (await screen.findByLabelText('Start Call')).closest('button');
+        const button = (await screen.findByLabelText('Call with user is ongoing')).closest('button');
         expect(button?.getAttribute('aria-disabled')).toBe('true');
     });
 
-    test('should not show the start call button when isCallsDefaultEnabledOnAllChannels, isCallsCanBeDisabledOnSpecificChannels is false and callsChannelState.enabled is false', async () => {
+    test('should not show the start call button when callsChannelState.enabled is false', async () => {
         (Client4.getCallsChannelState as jest.Mock).mockImplementationOnce(async () => ({enabled: false}));
         const [props, initialState] = getBasePropsAndState();
 

diff --git a/webapp/channels/src/components/profile_popover/index.tsx b/webapp/channels/src/components/profile_popover/index.tsx
@@ -310,7 +310,6 @@ const ProfilePopover = ({
                         username={user.username}
                     />
                     <ProfilePopoverActions
-                        channelId={channelId}
                         currentUserId={currentUserId}
                         fullname={fullname}
                         handleCloseModals={handleCloseModals}

diff --git a/webapp/channels/src/components/profile_popover/profile_popover_actions/call_button.test.tsx b/webapp/channels/src/components/profile_popover/profile_popover_actions/call_button.test.tsx
@@ -1,53 +1,53 @@
 // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
 // See LICENSE.txt for license information.
 
-import {checkUserInCall} from './call_button';
+import {isUserInCall} from './call_button';
 
-describe('checkUserInCall', () => {
+describe('isUserInCall', () => {
     test('missing state', () => {
-        expect(checkUserInCall({
+        expect(isUserInCall({
             'plugins-com.mattermost.calls': {},
-        } as any, 'userA')).toBe(false);
+        } as any, 'userA', 'channelID')).toBe(false);
     });
 
     test('call state missing', () => {
-        expect(checkUserInCall({
+        expect(isUserInCall({
             'plugins-com.mattermost.calls': {
-                profiles: {
+                sessions: {
                     channelID: null,
                 },
             },
-        } as any, 'userA')).toBe(false);
+        } as any, 'userA', 'channelID')).toBe(false);
     });
 
     test('user not in call', () => {
-        expect(checkUserInCall({
+        expect(isUserInCall({
             'plugins-com.mattermost.calls': {
-                profiles: {
+                sessions: {
                     channelID: {
                         sessionB: {
-                            id: 'userB',
+                            user_id: 'userB',
                         },
                     },
                 },
             },
-        } as any, 'userA')).toBe(false);
+        } as any, 'userA', 'channelID')).toBe(false);
     });
 
     test('user in call', () => {
-        expect(checkUserInCall({
+        expect(isUserInCall({
             'plugins-com.mattermost.calls': {
-                profiles: {
+                sessions: {
                     channelID: {
                         sessionB: {
-                            id: 'userB',
+                            user_id: 'userB',
                         },
                         sessionA: {
-                            id: 'userA',
+                            user_id: 'userA',
                         },
                     },
                 },
             },
-        } as any, 'userA')).toBe(true);
+        } as any, 'userA', 'channelID')).toBe(true);
     });
 });
diff --git a/webapp/channels/src/components/profile_popover/profile_popover_actions/call_button.tsx b/webapp/channels/src/components/profile_popover/profile_popover_actions/call_button.tsx
@@ -10,9 +10,8 @@ import {PhoneInTalkIcon} from '@mattermost/compass-icons/components';
 
 import {Client4} from 'mattermost-redux/client';
 import {getChannelByName} from 'mattermost-redux/selectors/entities/channels';
-import {getCallsConfig, getProfilesInCalls} from 'mattermost-redux/selectors/entities/common';
 
-import {isCallsEnabled as getIsCallsEnabled} from 'selectors/calls';
+import {isCallsEnabled as getIsCallsEnabled, getSessionsInCalls} from 'selectors/calls';
 
 import OverlayTrigger from 'components/overlay_trigger';
 import ProfilePopoverCallButton from 'components/profile_popover_call_button';
@@ -25,7 +24,6 @@ import type {GlobalState} from 'types/store';
 type Props = {
     userId: string;
     currentUserId: string;
-    channelId?: string;
     fullname: string;
     username: string;
 }
@@ -35,12 +33,12 @@ type ChannelCallsState = {
     id: string;
 };
 
-export function checkUserInCall(state: GlobalState, userId: string) {
-    for (const profilesMap of Object.values(getProfilesInCalls(state))) {
-        for (const profile of Object.values(profilesMap || {})) {
-            if (profile?.id === userId) {
-                return true;
-            }
+export function isUserInCall(state: GlobalState, userId: string, channelId: string) {
+    const sessionsInCall = getSessionsInCalls(state)[channelId] || {};
+
+    for (const session of Object.values(sessionsInCall)) {
+        if (session.user_id === userId) {
+            return true;
         }
     }
 
@@ -50,22 +48,22 @@ export function checkUserInCall(state: GlobalState, userId: string) {
 const CallButton = ({
     userId,
     currentUserId,
-    channelId,
     fullname,
     username,
 }: Props) => {
     const {formatMessage} = useIntl();
 
     const isCallsEnabled = useSelector((state: GlobalState) => getIsCallsEnabled(state));
-    const isUserInCall = useSelector((state: GlobalState) => (isCallsEnabled ? checkUserInCall(state, userId) : undefined));
-    const isCurrentUserInCall = useSelector((state: GlobalState) => (isCallsEnabled ? checkUserInCall(state, currentUserId) : undefined));
-    const callsConfig = useSelector((state: GlobalState) => (isCallsEnabled ? getCallsConfig(state) : undefined));
-    const isCallsDefaultEnabledOnAllChannels = callsConfig?.DefaultEnabled;
-    const isCallsCanBeDisabledOnSpecificChannels = callsConfig?.AllowEnableCalls;
     const dmChannel = useSelector((state: GlobalState) => getChannelByName(state, getDirectChannelName(currentUserId, userId)));
 
+    const hasDMCall = useSelector((state: GlobalState) => {
+        if (isCallsEnabled && dmChannel) {
+            return isUserInCall(state, currentUserId, dmChannel.id) || isUserInCall(state, userId, dmChannel.id);
+        }
+        return false;
+    });
+
     const [callsDMChannelState, setCallsDMChannelState] = useState<ChannelCallsState>();
-    const [callsChannelState, setCallsChannelState] = useState<ChannelCallsState>();
 
     const getCallsChannelState = useCallback((channelId: string): Promise<ChannelCallsState> => {
         let data: Promise<ChannelCallsState>;
@@ -84,26 +82,17 @@ const CallButton = ({
                 setCallsDMChannelState(data);
             });
         }
-
-        if (isCallsEnabled && channelId) {
-            getCallsChannelState(channelId).then((data) => {
-                setCallsChannelState(data);
-            });
-        }
     }, []);
 
-    if (
-        !isCallsEnabled ||
-        callsDMChannelState?.enabled === false ||
-        (!isCallsDefaultEnabledOnAllChannels && !isCallsCanBeDisabledOnSpecificChannels && callsChannelState?.enabled === false)
-    ) {
+    if (!isCallsEnabled || callsDMChannelState?.enabled === false) {
         return null;
     }
 
-    const disabled = isUserInCall || isCurrentUserInCall;
-    const startCallMessage = isUserInCall ? formatMessage({
-        id: 'user_profile.call.userBusy',
-        defaultMessage: '{user} is in another call',
+    // We disable the button if there's already a call ongoing with the user.
+    const disabled = hasDMCall;
+    const startCallMessage = hasDMCall ? formatMessage({
+        id: 'user_profile.call.ongoing',
+        defaultMessage: 'Call with {user} is ongoing',
     }, {user: fullname || username},
     ) : formatMessage({
         id: 'webapp.mattermost.feature.start_call',

diff --git a/webapp/channels/src/components/profile_popover/profile_popover_actions/index.tsx b/webapp/channels/src/components/profile_popover/profile_popover_actions/index.tsx
@@ -13,7 +13,6 @@ import CallButton from './call_button';
 type Props = {
     user: UserProfile;
     fullname: string;
-    channelId?: string;
     currentUserId: string;
     haveOverrideProp: boolean;
     handleShowDirectChannel: (e: React.MouseEvent<HTMLButtonElement>) => void;
@@ -31,7 +30,6 @@ const ProfilePopoverActions = ({
     returnFocus,
     hide,
     fullname,
-    channelId,
 }: Props) => {
     const {formatMessage} = useIntl();
 
@@ -69,7 +67,6 @@ const ProfilePopoverActions = ({
                     hide={hide}
                 />
                 <CallButton
-                    channelId={channelId}
                     currentUserId={currentUserId}
                     fullname={fullname}
                     userId={user.id}

diff --git a/webapp/channels/src/i18n/en.json b/webapp/channels/src/i18n/en.json
@@ -5258,7 +5258,7 @@
   "user_profile.account.post_was_created": "This post was created by an integration from @{username}",
   "user_profile.add_user_to_channel": "Add to a Channel",
   "user_profile.add_user_to_channel.icon": "Add User to Channel Icon",
-  "user_profile.call.userBusy": "{user} is in another call",
+  "user_profile.call.ongoing": "Call with {user} is ongoing",
   "user_profile.custom_status": "Status",
   "user_profile.custom_status.set_status": "Set a status",
   "user_profile.send.dm": "Message",

diff --git a/webapp/channels/src/packages/mattermost-redux/src/selectors/entities/common.ts b/webapp/channels/src/packages/mattermost-redux/src/selectors/entities/common.ts
@@ -9,19 +9,6 @@ import type {RelationOneToOne, IDMappedObjects} from '@mattermost/types/utilitie
 
 import {createSelector} from 'mattermost-redux/selectors/create_selector';
 
-const CALLS_PLUGIN = 'plugins-com.mattermost.calls';
-
-type CallsConfig = {
-    ICEServers: string[];
-    ICEServersConfigs: RTCIceServer[];
-    AllowEnableCalls: boolean;
-    DefaultEnabled: boolean;
-    MaxCallParticipants: number;
-    NeedsTURNCredentials: boolean;
-    AllowScreenSharing: boolean;
-    sku_short_name: string;
-}
-
 // Channels
 
 export function getCurrentChannelId(state: GlobalState): string {
@@ -69,20 +56,6 @@ export function getUsers(state: GlobalState): IDMappedObjects<UserProfile> {
     return state.entities.users.profiles;
 }
 
-// Calls
-
-export function getProfilesInCalls(state: GlobalState): Record<string, Record<string, UserProfile>> {
-    // eslint-disable-next-line @typescript-eslint/ban-ts-comment
-    // @ts-ignore
-    return state[CALLS_PLUGIN].profiles || {};
-}
-
-export function getCallsConfig(state: GlobalState): CallsConfig {
-    // eslint-disable-next-line @typescript-eslint/ban-ts-comment
-    // @ts-ignore
-    return state[CALLS_PLUGIN].callsConfig;
-}
-
 // Config
 export const getIsUserStatusesConfigEnabled: (a: GlobalState) => boolean = createSelector(
     'getIsUserStatusesConfigEnabled',

diff --git a/webapp/channels/src/selectors/calls.ts b/webapp/channels/src/selectors/calls.ts
@@ -3,10 +3,14 @@
 
 import semver from 'semver';
 
+import type {CallsConfig, UserSessionState} from '@mattermost/calls-common/lib/types';
+
 import {suitePluginIds} from 'utils/constants';
 
 import type {GlobalState} from 'types/store';
 
+const CALLS_PLUGIN = 'plugins-com.mattermost.calls';
+
 export function isCallsEnabled(state: GlobalState, minVersion = '0.4.2') {
     return Boolean(state.plugins.plugins[suitePluginIds.calls] &&
         semver.gte(String(semver.clean(state.plugins.plugins[suitePluginIds.calls].version || '0.0.0')), minVersion));
@@ -18,3 +22,15 @@ export function isCallsRingingEnabledOnServer(state: GlobalState) {
     // @ts-ignore
     return Boolean(state[`plugins-${suitePluginIds.calls}`]?.callsConfig?.EnableRinging);
 }
+
+export function getSessionsInCalls(state: GlobalState): Record<string, Record<string, UserSessionState>> {
+    // eslint-disable-next-line @typescript-eslint/ban-ts-comment
+    // @ts-ignore
+    return state[CALLS_PLUGIN].sessions || {};
+}
+
+export function getCallsConfig(state: GlobalState): CallsConfig {
+    // eslint-disable-next-line @typescript-eslint/ban-ts-comment
+    // @ts-ignore
+    return state[CALLS_PLUGIN].callsConfig;
+}
diff --git a/webapp/package-lock.json b/webapp/package-lock.json