Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MM-58259] [MM-58260] Move the /call end command to host controls #747

Merged
merged 14 commits into from
May 28, 2024
2 changes: 1 addition & 1 deletion e2e/page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ export default class PlaywrightDevPage {
await expect(this.page.locator('#calls-widget')).toBeVisible();
}

async endCall() {
async slashCallEnd() {
await this.sendSlashCommand('/call end');
let modal = this.page.locator('#calls-end-call-modal');
if (await modal.isVisible()) {
Expand Down
12 changes: 12 additions & 0 deletions e2e/tests/channel_toast.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,4 +54,16 @@ test.describe('channel toast', () => {

await userPage.leaveCall();
});

test('does not reappear after leaving call (call ends)', async () => {
const user0Page = await startCall(userStorages[0]);

await expect(user0Page.page.locator('#calls-channel-toast')).toBeHidden();

await user0Page.leaveCall();

await user0Page.wait(3000);

await expect(user0Page.page.locator('#calls-channel-toast')).toBeHidden();
});
});
2 changes: 1 addition & 1 deletion e2e/tests/host_controls.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ test.beforeEach(async ({page, context}) => {
});
test.afterEach(async ({page, context}) => {
const devPage = new PlaywrightDevPage(page);
await devPage.endCall();
await devPage.slashCallEnd();
});

test.describe('host controls', () => {
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
52 changes: 52 additions & 0 deletions e2e/tests/slash_commands.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import {chromium, expect, test} from '@playwright/test';
import {readFile} from 'fs/promises';

import PlaywrightDevPage from '../page';
import {getUserIdxForTest, getUsernamesForTest, getUserStoragesForTest, joinCall, startCall} from '../utils';

const userStorages = getUserStoragesForTest();
const usernames = getUsernamesForTest();

test.beforeEach(async ({page, context}) => {
const devPage = new PlaywrightDevPage(page);
await devPage.goto();
});

test.describe('slash commands', () => {
const userIdx = getUserIdxForTest();
test.use({storageState: getUserStoragesForTest()[0]});

test('end call', async ({page}) => {
const devPage = new PlaywrightDevPage(page);
await devPage.startCall();

await expect(page.locator('#calls-widget')).toBeVisible();
await expect(page.getByTestId('calls-widget-loading-overlay')).toBeHidden();

await devPage.slashCallEnd();

// /call end cleans up all indicators of a running call
await expect(page.locator('#calls-widget')).toBeHidden();
await expect(page.locator('#calls-channel-toast')).toBeHidden();
await expect(page.locator(`#sidebarItem_calls${userIdx}`).getByTestId('calls-sidebar-active-call-icon')).toBeHidden();
});

test('end call as second host', async () => {
const user0Page = await startCall(userStorages[0]);
const user1Page = await joinCall(userStorages[1]);

await expect(user0Page.page.locator('#calls-widget')).toBeVisible();
await expect(user0Page.page.getByTestId('calls-widget-loading-overlay')).toBeHidden();

await user0Page.sendSlashCommand(`/call host @${usernames[1]}`);
await user0Page.wait(1000);
await user0Page.expectHostToBe(usernames[1]);

await user1Page.slashCallEnd();

// /call end cleans up all indicators of a running call
await expect(user0Page.page.locator('#calls-widget')).toBeHidden();
await expect(user0Page.page.locator('#calls-channel-toast')).toBeHidden();
await expect(user0Page.page.locator(`#sidebarItem_calls${userIdx}`).getByTestId('calls-sidebar-active-call-icon')).toBeHidden();
});
});
75 changes: 0 additions & 75 deletions server/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"fmt"
"net/http"
"path/filepath"
"time"

"golang.org/x/time/rate"

Expand Down Expand Up @@ -212,80 +211,6 @@ func (p *Plugin) handleGetAllCallChannelStates(w http.ResponseWriter, r *http.Re
}
}

func (p *Plugin) handleEndCall(w http.ResponseWriter, r *http.Request) {
var res httpResponse
defer p.httpAudit("handleEndCall", &res, w, r)

userID := r.Header.Get("Mattermost-User-Id")
channelID := mux.Vars(r)["channel_id"]

isAdmin := p.API.HasPermissionTo(userID, model.PermissionManageSystem)

state, err := p.lockCallReturnState(channelID)
if err != nil {
res.Err = fmt.Errorf("failed to lock call: %w", err).Error()
res.Code = http.StatusInternalServerError
return
}
defer p.unlockCall(channelID)

if state == nil {
res.Err = "no call ongoing"
res.Code = http.StatusBadRequest
return
}

if !isAdmin && state.Call.OwnerID != userID {
res.Err = "no permissions to end the call"
res.Code = http.StatusForbidden
return
}

if state.Call.EndAt == 0 {
setCallEnded(&state.Call)
}

if err := p.store.UpdateCall(&state.Call); err != nil {
res.Err = fmt.Errorf("failed to update call: %w", err).Error()
res.Code = http.StatusInternalServerError
return
}

p.publishWebSocketEvent(wsEventCallEnd, map[string]interface{}{}, &WebSocketBroadcast{ChannelID: channelID, ReliableClusterSend: true})

callID := state.Call.ID
nodeID := state.Call.Props.NodeID

go func() {
// We wait a few seconds for the call to end cleanly. If this doesn't
// happen we force end it.
time.Sleep(5 * time.Second)

call, err := p.store.GetCall(callID, db.GetCallOpts{})
if err != nil {
p.LogError("failed to get call", "err", err.Error())
}

sessions, err := p.store.GetCallSessions(callID, db.GetCallSessionOpts{})
if err != nil {
p.LogError("failed to get call sessions", "err", err.Error())
}

for _, session := range sessions {
if err := p.closeRTCSession(session.UserID, session.ID, channelID, nodeID, callID); err != nil {
p.LogError(err.Error())
}
}

if err := p.cleanCallState(call); err != nil {
p.LogError(err.Error())
}
}()

res.Code = http.StatusOK
res.Msg = "success"
}

func (p *Plugin) handleDismissNotification(w http.ResponseWriter, r *http.Request) {
var res httpResponse
defer p.httpAudit("handleDismissNotification", &res, w, r)
Expand Down
5 changes: 4 additions & 1 deletion server/api_router.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,10 +93,12 @@ func (p *Plugin) newAPIRouter() *mux.Router {
// router.HandleFunc("/channels/{channel_id:[a-z0-9]{26}}", p.handlePostCallsChannel).Methods("POST")

// Calls
router.HandleFunc("/calls/{channel_id:[a-z0-9]{26}}/end", p.handleEndCall).Methods("POST")
router.HandleFunc("/calls/{channel_id:[a-z0-9]{26}}/dismiss-notification", p.handleDismissNotification).Methods("POST")
router.HandleFunc("/calls/{call_id:[a-z0-9]{26}}/recording/{action}", p.handleRecordingAction).Methods("POST")

// Deprecated for hostCtrlRounder /end, but needed for mobile backward compatibility (pre 2.18)
router.HandleFunc("/calls/{call_id:[a-z0-9]{26}}/end", p.handleEnd).Methods("POST")

// Host Controls
hostCtrlRouter := router.PathPrefix("/calls/{call_id:[a-z0-9]{26}}/host").Subrouter()
hostCtrlRouter.HandleFunc("/make", p.handleMakeHost).Methods("POST")
Expand All @@ -105,6 +107,7 @@ func (p *Plugin) newAPIRouter() *mux.Router {
hostCtrlRouter.HandleFunc("/lower-hand", p.handleLowerHand).Methods("POST")
hostCtrlRouter.HandleFunc("/remove", p.handleRemoveSession).Methods("POST")
hostCtrlRouter.HandleFunc("/mute-others", p.handleMuteOthers).Methods("POST")
hostCtrlRouter.HandleFunc("/end", p.handleEnd).Methods("POST")

// Bot
botRouter := router.PathPrefix("/bot").Subrouter()
Expand Down
61 changes: 61 additions & 0 deletions server/host_controls.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import (
"fmt"
"time"

"github.com/mattermost/mattermost-plugin-calls/server/db"

"github.com/mattermost/mattermost/server/public/model"
"github.com/pkg/errors"
)
Expand Down Expand Up @@ -255,3 +257,62 @@ func (p *Plugin) hostRemoveSession(requesterID, channelID, sessionID string) err

return nil
}

func (p *Plugin) hostEnd(requesterID, channelID string) error {
state, err := p.lockCallReturnState(channelID)
if err != nil {
return fmt.Errorf("failed to lock call: %w", err)
}
defer p.unlockCall(channelID)

if state == nil {
return ErrNoCallOngoing
}

if requesterID != state.Call.GetHostID() {
if isAdmin := p.API.HasPermissionTo(requesterID, model.PermissionManageSystem); !isAdmin {
return ErrNoPermissions
}
}

if state.Call.EndAt == 0 {
setCallEnded(&state.Call)
}

if err := p.store.UpdateCall(&state.Call); err != nil {
return fmt.Errorf("failed to update call: %w", err)
}

p.publishWebSocketEvent(wsEventCallEnd, map[string]interface{}{}, &WebSocketBroadcast{ChannelID: channelID, ReliableClusterSend: true})

callID := state.Call.ID
nodeID := state.Call.Props.NodeID

go func() {
// We wait a few seconds for the call to end cleanly. If this doesn't
// happen we force end it.
time.Sleep(5 * time.Second)

call, err := p.store.GetCall(callID, db.GetCallOpts{})
if err != nil {
p.LogError("failed to get call", "err", err.Error())
}

sessions, err := p.store.GetCallSessions(callID, db.GetCallSessionOpts{})
if err != nil {
p.LogError("failed to get call sessions", "err", err.Error())
}

for _, session := range sessions {
if err := p.closeRTCSession(session.UserID, session.ID, channelID, nodeID, callID); err != nil {
p.LogError(err.Error())
}
}

if err := p.cleanCallState(call); err != nil {
p.LogError(err.Error())
}
}()

return nil
}
16 changes: 16 additions & 0 deletions server/host_controls_api.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,22 @@ func (p *Plugin) handleRemoveSession(w http.ResponseWriter, r *http.Request) {
res.Msg = "success"
}

func (p *Plugin) handleEnd(w http.ResponseWriter, r *http.Request) {
var res httpResponse
defer p.httpAudit("handleEnd", &res, w, r)

userID := r.Header.Get("Mattermost-User-Id")
callID := mux.Vars(r)["call_id"]

if err := p.hostEnd(userID, callID); err != nil {
p.handleHostControlsError(err, &res, "handleEnd")
return
}

res.Code = http.StatusOK
res.Msg = "success"
}

func (p *Plugin) handleHostControlsError(err error, res *httpResponse, handlerName string) {
p.LogError(handlerName, "err", err.Error())

Expand Down
3 changes: 2 additions & 1 deletion webapp/src/action_types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ export const RINGING_FOR_CALL = pluginId + '_ringing_for_call';
export const DID_RING_FOR_CALL = pluginId + '_did_ring_for_call';
export const DID_NOTIFY_FOR_CALL = pluginId + '_did_notify_for_call';

// CALL_END is sent when the `/call end` command is used
// CALL_END is sent when the `/call end` command is used,
// and when the last session in the call leaves the call
export const CALL_END = pluginId + '_call_end';

export const RECEIVED_CALLS_CONFIG = pluginId + '_received_calls_config';
Expand Down
16 changes: 11 additions & 5 deletions webapp/src/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,10 @@ import RestClient from 'src/rest_client';
import {
callDismissedNotification,
calls,
channelHasCall,
hostChangeAtForCurrentCall,
idForCurrentCall,
incomingCalls,
ringingEnabled,
numSessionsInCallInChannel,
ringingForCall,
} from 'src/selectors';
import * as Telemetry from 'src/types/telemetry';
Expand All @@ -49,6 +48,7 @@ import {modals, notificationSounds, openPricingModal} from 'src/webapp_globals';

import {
ADD_INCOMING_CALL,
CALL_END,
CALL_HOST,
CALL_LIVE_CAPTIONS_STATE,
CALL_REC_PROMPT_DISMISSED,
Expand Down Expand Up @@ -221,7 +221,7 @@ export const requestOnPremTrialLicense = async (users: number, termsAccepted: bo

export const endCall = (channelID: string) => {
return RestClient.fetch(
`${getPluginPath()}/calls/${channelID}/end`,
`${getPluginPath()}/calls/${channelID}/host/end`,
{method: 'post'},
);
};
Expand Down Expand Up @@ -402,8 +402,14 @@ export const userLeft = (channelID: string, userID: string, sessionID: string) =
},
});

if (ringingEnabled(getState()) && !channelHasCall(getState(), channelID)) {
await dispatch(removeIncomingCallNotification(callID));
if (numSessionsInCallInChannel(getState(), channelID) === 0) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noting this because it's a big change. This is what we do in mobile and it seems to simplify things.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I implicitly thought we would handle this case server-side and send the unique call ID because this does not protect us against a race, as we still use the channel ID to figure out the locally stored call ID. It's not a huge deal, but it's something to note.

dispatch({
type: CALL_END,
data: {
channelID,
callID,
},
});
}
};
};
Expand Down
4 changes: 2 additions & 2 deletions webapp/src/components/channel_call_toast.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,13 @@ const ChannelCallToast = () => {
const callID = useSelector(callInCurrentChannel)?.ID || '';
const [onDismiss, onJoin] = useDismissJoin(currChannelID, callID);

const hasCall = (currChannelID !== connectedID && profiles.length > 0);
const hasCall = (call && currChannelID !== connectedID);

if (!hasCall || dismissed || limitRestricted) {
return null;
}

const timestampFn = () => callStartedTimestampFn(intl, call?.startAt);
const timestampFn = () => callStartedTimestampFn(intl, call.startAt);

return (
<div
Expand Down
4 changes: 2 additions & 2 deletions webapp/src/components/channel_header_button.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@ import {
callsShowButton,
channelIDForCurrentCall,
clientConnecting,
currentChannelHasCall,
isCloudProfessionalOrEnterpriseOrTrial,
isCloudStarter,
isLimitRestricted,
maxParticipants,
profilesInCallInCurrentChannel,
} from 'src/selectors';
import {getUserIdFromDM, isDMChannel} from 'src/utils';
import styled, {css} from 'styled-components';
Expand All @@ -28,7 +28,7 @@ const ChannelHeaderButton = () => {
const isDeactivatedDM = isDMChannel(channel) && otherUser?.delete_at > 0;
const show = useSelector((state: GlobalState) => callsShowButton(state, channel?.id));
const inCall = useSelector(channelIDForCurrentCall) === channel?.id;
const hasCall = useSelector(profilesInCallInCurrentChannel).length > 0;
const hasCall = useSelector(currentChannelHasCall);
const isAdmin = useSelector(isCurrentUserSystemAdmin);
const cloudStarter = useSelector(isCloudStarter);
const isCloudPaid = useSelector(isCloudProfessionalOrEnterpriseOrTrial);
Expand Down
Loading
Loading