Skip to content

Commit

Permalink
fix(lobby): Inconsistent state after deny and then approve.
Browse files Browse the repository at this point in the history
Fixes several issues:
- The error on lobby deny is not sticky
- When preJoin is not enabled we were showing conference UI and showing the error, while the participant is denied to enter the meeting.
- There was inconsistent state (after deny we were keeping membersOnly conference state) and when being approved on re-try while being in the meeting, no remote thumbnails are shown although media is flowing.

The scenario is enabling lobby and tryintg to join, denying the first attempt and approving the second one.
  • Loading branch information
damencho committed Oct 24, 2024
1 parent 390431f commit a961969
Show file tree
Hide file tree
Showing 5 changed files with 55 additions and 8 deletions.
14 changes: 14 additions & 0 deletions react/features/base/conference/reducer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@ export interface IConferenceState {
followMeRecorderEnabled?: boolean;
joining?: IJitsiConference;
leaving?: IJitsiConference;
lobbyError?: boolean;
lobbyWaitingForHost?: boolean;
localSubject?: string;
locked?: string;
Expand Down Expand Up @@ -363,13 +364,24 @@ function _conferenceFailed(state: IConferenceState, { conference, error }: {
let membersOnly;
let passwordRequired;
let lobbyWaitingForHost;
let lobbyError;

switch (error.name) {
case JitsiConferenceErrors.AUTHENTICATION_REQUIRED:
authRequired = conference;
break;

/**
* Access denied while waiting in the lobby.
* A conference error when we tried to join into a room with no display name when lobby is enabled in the room.
*/
case JitsiConferenceErrors.CONFERENCE_ACCESS_DENIED:
case JitsiConferenceErrors.DISPLAY_NAME_REQUIRED: {
lobbyError = true;

break;
}

case JitsiConferenceErrors.MEMBERS_ONLY_ERROR: {
membersOnly = conference;

Expand All @@ -393,6 +405,7 @@ function _conferenceFailed(state: IConferenceState, { conference, error }: {
error,
joining: undefined,
leaving: undefined,
lobbyError,
lobbyWaitingForHost,

/**
Expand Down Expand Up @@ -450,6 +463,7 @@ function _conferenceJoined(state: IConferenceState, { conference }: { conference
membersOnly: undefined,
leaving: undefined,

lobbyError: undefined,
lobbyWaitingForHost: undefined,

/**
Expand Down
2 changes: 2 additions & 0 deletions react/features/lobby/actions.any.ts
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,8 @@ export function startKnocking() {
logger.info(`Lobby starting knocking (membersOnly = ${membersOnly})`);

if (!membersOnly) {
// let's hide the notification (the case with denied access and retrying)
dispatch(hideNotification(LOBBY_NOTIFICATION_ID));

// no membersOnly, this means we got lobby screen shown as someone
// tried to join a conference that has lobby enabled without setting display name
Expand Down
24 changes: 19 additions & 5 deletions react/features/lobby/middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import {
import { INotificationProps } from '../notifications/types';
import { open as openParticipantsPane } from '../participants-pane/actions';
import { getParticipantsPaneOpen } from '../participants-pane/functions';
import { PREJOIN_JOINING_IN_PROGRESS } from '../prejoin/actionTypes';
import {
isPrejoinEnabledInConfig,
isPrejoinPageVisible,
Expand Down Expand Up @@ -108,6 +109,14 @@ MiddlewareRegistry.register(store => next => action => {

return result;
}
case PREJOIN_JOINING_IN_PROGRESS: {
if (action.value) {
// let's hide the notification (the case with denied access and retrying) when prejoin is enabled
store.dispatch(hideNotification(LOBBY_NOTIFICATION_ID));
}

break;
}
}

return next(action);
Expand Down Expand Up @@ -270,9 +279,8 @@ function _handleLobbyNotification(store: IStore) {
function _conferenceFailed({ dispatch, getState }: IStore, next: Function, action: AnyAction) {
const { error } = action;
const state = getState();
const { membersOnly } = state['features/base/conference'];
const { lobbyError, membersOnly } = state['features/base/conference'];
const nonFirstFailure = Boolean(membersOnly);
const { isDisplayNameRequiredError } = state['features/lobby'];

if (error.name === JitsiConferenceErrors.MEMBERS_ONLY_ERROR) {
if (typeof error.recoverable === 'undefined') {
Expand All @@ -288,7 +296,7 @@ function _conferenceFailed({ dispatch, getState }: IStore, next: Function, actio

// if there was an error about display name and pre-join is not enabled
if (shouldAutoKnock(state)
|| (isDisplayNameRequiredError && !isPrejoinEnabledInConfig(state))
|| (lobbyError && !isPrejoinEnabledInConfig(state))
|| lobbyWaitingForHost) {
dispatch(startKnocking());
}
Expand All @@ -315,17 +323,23 @@ function _conferenceFailed({ dispatch, getState }: IStore, next: Function, actio
return result;
}

dispatch(hideLobbyScreen());
// if both are available pre-join is with priority (the case when pre-join is enabled)
if (isPrejoinPageVisible(state)) {
dispatch(hideLobbyScreen());
}

if (error.name === JitsiConferenceErrors.CONFERENCE_ACCESS_DENIED) {
dispatch(
showNotification({
appearance: NOTIFICATION_TYPE.ERROR,
hideErrorSupportLink: true,
titleKey: 'lobby.joinRejectedTitle',
uid: LOBBY_NOTIFICATION_ID,
descriptionKey: 'lobby.joinRejectedMessage'
}, NOTIFICATION_TIMEOUT_TYPE.LONG)
}, NOTIFICATION_TIMEOUT_TYPE.STICKY)
);
} else {
dispatch(hideLobbyScreen());
}

return next(action);
Expand Down
1 change: 1 addition & 0 deletions react/features/lobby/reducer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ ReducerRegistry.register<ILobbyState>('features/lobby', (state = DEFAULT_STATE,
case CONFERENCE_LEFT:
return {
...state,
isDisplayNameRequiredError: false,
knocking: false,
passwordJoinFailed: false
};
Expand Down
22 changes: 19 additions & 3 deletions react/features/notifications/middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,22 @@ const getNotifications = (state: IReduxState) => {
return _visible ? notifications : [];
};

/**
* Check whether we need to clear notifications.
*
* @param {Object} state - Global state.
* @returns {boolean} - True if we need to clear any notification.
*/
const shouldClearNotifications = (state: IReduxState): boolean => {
// in case of lobby error, access is denied, the conference is cleared, but we still
// want to show the notification
if (state['features/base/conference'].lobbyError) {
return false;
}

return !getCurrentConference(state);
};

/**
* Middleware that captures actions to display notifications.
*
Expand Down Expand Up @@ -201,9 +217,9 @@ MiddlewareRegistry.register(store => next => action => {
* conference, where we need to clean up the notifications.
*/
StateListenerRegistry.register(
/* selector */ state => getCurrentConference(state),
/* listener */ (conference, { dispatch }) => {
if (!conference) {
/* selector */ state => shouldClearNotifications(state),
/* listener */ (clear, { dispatch }) => {
if (clear) {
dispatch(clearNotifications());
}
}
Expand Down

0 comments on commit a961969

Please sign in to comment.