Unverified Commit 3690eb87 authored by Kyriakos Z's avatar Kyriakos Z Committed by GitHub
Browse files

MM-40418: fix leave/archive channel (#9495)



* MM-40418: fix leave/archive channel

Leaving or Archiving a channel while having a thread that belongs in
that channel open in the RHS crashes the app.

This commit fixes the crash, and also closes the RHS upon aforementioned actions.

* Fixes tests

* Some refactor and test additions

* Adds case on leave_channel_spec when CRT is ON

* Apply suggestions from code review
Co-authored-by: default avatarJelena Gilliam <52937121+jgilliam17@users.noreply.github.com>

* Minor refactor

* Minor refactor, and unit tests added

* Fixes an issue when kicking a user of a channel

* Handle channel deleted by other user

* Minor refactor
Co-authored-by: default avatarJelena Gilliam <52937121+jgilliam17@users.noreply.github.com>
parent 5b0e9faf
......@@ -8,6 +8,7 @@ import {
joinChannel,
markChannelAsRead,
unfavoriteChannel,
deleteChannel as deleteChannelRedux,
} from 'mattermost-redux/actions/channels';
import * as PostActions from 'mattermost-redux/actions/posts';
import {TeamTypes} from 'mattermost-redux/action_types';
......@@ -34,11 +35,13 @@ import {makeAddLastViewAtToProfiles} from 'mattermost-redux/selectors/entities/u
import {getChannelByName} from 'mattermost-redux/utils/channel_utils';
import EventEmitter from 'mattermost-redux/utils/event_emitter';
import {closeRightHandSide} from 'actions/views/rhs';
import {openDirectChannelToUserId} from 'actions/channel_actions.jsx';
import {loadCustomStatusEmojisForPostList} from 'actions/emoji_actions';
import {getLastViewedChannelName} from 'selectors/local_storage';
import {getLastPostsApiTimeForChannel} from 'selectors/views/channel';
import {getSocketStatus} from 'selectors/views/websocket';
import {getSelectedPost, getSelectedPostId} from 'selectors/rhs';
import {browserHistory} from 'utils/browser_history';
import {Constants, ActionTypes, EventTypes, PostRequestTypes} from 'utils/constants';
......@@ -147,6 +150,11 @@ export function leaveChannel(channelId) {
if (!prevChannel || !getMyChannelMemberships(state)[prevChannel.id]) {
LocalStorageStore.removePreviousChannelName(currentUserId, currentTeam.id, state);
}
const selectedPost = getSelectedPost(state);
const selectedPostId = getSelectedPostId(state);
if (selectedPostId && selectedPost.exists === false) {
dispatch(closeRightHandSide());
}
if (getMyChannels(getState()).filter((c) => c.type === Constants.OPEN_CHANNEL || c.type === Constants.PRIVATE_CHANNEL).length === 0) {
LocalStorageStore.removePreviousChannelName(currentUserId, currentTeam.id, state);
......@@ -456,3 +464,21 @@ export function updateToastStatus(status) {
});
};
}
export function deleteChannel(channelId) {
return async (dispatch, getState) => {
const res = await dispatch(deleteChannelRedux(channelId));
if (res.error) {
return {data: false};
}
const state = getState();
const selectedPost = getSelectedPost(state);
const selectedPostId = getSelectedPostId(state);
if (selectedPostId && !selectedPost.exists) {
dispatch(closeRightHandSide());
}
return {data: true};
};
}
......@@ -11,6 +11,7 @@ import * as PostActions from 'mattermost-redux/actions/posts';
import {browserHistory} from 'utils/browser_history';
import * as Actions from 'actions/views/channel';
import {closeRightHandSide} from 'actions/views/rhs';
import {ActionTypes, PostRequestTypes} from 'utils/constants';
const mockStore = configureStore([thunk]);
......@@ -38,6 +39,11 @@ jest.mock('mattermost-redux/actions/channels', () => ({
leaveChannel: jest.fn(() => ({type: ''})),
}));
jest.mock('actions/views/rhs', () => ({
...jest.requireActual('actions/views/rhs'),
closeRightHandSide: jest.fn(() => ({type: ''})),
}));
jest.mock('mattermost-redux/actions/posts');
jest.mock('selectors/local_storage', () => ({
......@@ -93,6 +99,7 @@ describe('channel view actions', () => {
},
posts: {
postsInChannel: {},
posts: {},
},
channelCategories: {
byId: {},
......@@ -103,6 +110,9 @@ describe('channel view actions', () => {
loadingPosts: {},
postVisibility: {current_channel_id: 60},
},
rhs: {
selectedPostId: '',
},
},
};
......@@ -129,6 +139,22 @@ describe('channel view actions', () => {
await store.dispatch(Actions.leaveChannel('channelid1'));
expect(browserHistory.push).toHaveBeenCalledWith(`/${team1.name}`);
expect(leaveChannel).toHaveBeenCalledWith('channelid1');
expect(closeRightHandSide).not.toHaveBeenCalled();
});
test('leave a channel successfully with a thread open', async () => {
store = mockStore({
...initialState,
views: {
...initialState.views,
rhs: {
selectedPostId: '1',
},
},
});
await store.dispatch(Actions.leaveChannel('channelid1'));
expect(browserHistory.push).toHaveBeenCalledWith(`/${team1.name}`);
expect(leaveChannel).toHaveBeenCalledWith('channelid1');
expect(closeRightHandSide).toHaveBeenCalled();
});
test('leave the last channel successfully', async () => {
store = mockStore({
......
......@@ -952,9 +952,15 @@ export function handleUserRemovedEvent(msg) {
}
}
const channel = getChannel(state, msg.data.channel_id);
dispatch({
type: ChannelTypes.LEAVE_CHANNEL,
data: {id: msg.data.channel_id, user_id: msg.broadcast.user_id},
data: {
id: msg.data.channel_id,
user_id: msg.broadcast.user_id,
team_id: channel?.team_id,
},
});
if (msg.data.channel_id === currentChannel.id) {
......
......@@ -4,12 +4,13 @@
import {ActionCreatorsMapObject, bindActionCreators, Dispatch} from 'redux';
import {connect} from 'react-redux';
import {deleteChannel} from 'mattermost-redux/actions/channels';
import {getCurrentTeam} from 'mattermost-redux/selectors/entities/teams';
import {getConfig} from 'mattermost-redux/selectors/entities/general';
import {GlobalState} from 'mattermost-redux/types/store';
import {ActionFunc} from 'mattermost-redux/types/actions';
import {deleteChannel} from 'actions/views/channel';
import DeleteChannelModal from './delete_channel_modal';
function mapStateToProps(state: GlobalState) {
......
......@@ -27,30 +27,38 @@ import {
} from 'actions/views/rhs';
import {getIsRhsExpanded} from 'selectors/rhs';
import {allAtMentions} from '../../utils/text_formatting';
import {matchUserMentionTriggersWithMessageMentions} from '../../utils/post_utils';
import {allAtMentions} from 'utils/text_formatting';
import {matchUserMentionTriggersWithMessageMentions} from 'utils/post_utils';
import RhsHeaderPost from './rhs_header_post';
type OwnProps = Pick<ComponentProps<typeof RhsHeaderPost>, 'rootPostId'>
function mapStateToProps(state: GlobalState, {rootPostId}: OwnProps) {
let isFollowingThread = false;
const collapsedThreads = isCollapsedThreadsEnabled(state);
const root = getPost(state, rootPostId);
const currentUserMentionKeys = getCurrentUserMentionKeys(state);
const rootMessageMentionKeys = allAtMentions(root.message);
const thread = getThreadOrSynthetic(state, root);
const isMentionedInRootPost = thread.reply_count === 0 &&
matchUserMentionTriggersWithMessageMentions(currentUserMentionKeys, rootMessageMentionKeys);
if (root && collapsedThreads) {
const thread = getThreadOrSynthetic(state, root);
isFollowingThread = thread.is_following;
if (isFollowingThread === null && thread.reply_count === 0) {
const currentUserMentionKeys = getCurrentUserMentionKeys(state);
const rootMessageMentionKeys = allAtMentions(root.message);
isFollowingThread = matchUserMentionTriggersWithMessageMentions(currentUserMentionKeys, rootMessageMentionKeys);
}
}
return {
isExpanded: getIsRhsExpanded(state),
relativeTeamUrl: getCurrentRelativeTeamUrl(state),
currentTeamId: getCurrentTeamId(state),
currentUserId: getCurrentUserId(state),
isCollapsedThreadsEnabled: isCollapsedThreadsEnabled(state),
isFollowingThread: isCollapsedThreadsEnabled(state) && root && thread.is_following,
isMentionedInRootPost,
isCollapsedThreadsEnabled: collapsedThreads,
isFollowingThread,
};
}
......
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.
import React from 'react';
import {mount} from 'enzyme';
import {Preferences} from 'mattermost-redux/constants';
import {TestHelper} from 'utils/test_helper';
import {mockStore} from 'tests/test_store';
import FollowButton from 'components/threading/common/follow_button';
import RhsHeaderPost from './index';
describe('rhs_header_post', () => {
const initialState = {
entities: {
users: {
currentUserId: '12',
profiles: {
12: {
username: 'jessica.hyde',
notify_props: {
mention_keys: 'jessicahyde,Jessica Hyde',
},
},
},
},
teams: {
teams: {},
currentTeamId: '22',
},
general: {
config: {
FeatureFlagCollapsedThreads: 'true',
CollapsedThreads: 'default_off',
},
},
preferences: {
myPreferences: {
[`${Preferences.CATEGORY_DISPLAY_SETTINGS}--${Preferences.COLLAPSED_REPLY_THREADS}`]: {
value: 'on',
},
},
},
posts: {
posts: {
42: {
id: 42,
message: 'where is @jessica.hyde?',
},
43: {
id: 43,
message: 'not a mention',
},
},
},
threads: {
threads: {
42: {
id: 42,
reply_count: 0,
is_following: null,
},
43: {
id: 43,
reply_count: 0,
is_following: null,
},
},
},
},
views: {
rhs: {
isSidebarExpanded: false,
},
},
};
const baseProps = {
channel: TestHelper.getChannelMock(),
currentChannelId: '32',
rootPostId: '42',
showMentions: jest.fn(),
showFlaggedPosts: jest.fn(),
showPinnedPosts: jest.fn(),
closeRightHandSide: jest.fn(),
toggleRhsExpanded: jest.fn(),
setThreadFollow: jest.fn(),
};
test('should not crash when no root', () => {
const {mountOptions} = mockStore(initialState);
const wrapper = mount(
<RhsHeaderPost
{...baseProps}
rootPostId='41'
/>, mountOptions);
expect(wrapper.exists()).toBe(true);
expect(wrapper.find(FollowButton).prop('isFollowing')).toBe(false);
});
test('should not show following when no replies and not mentioned', () => {
const {mountOptions} = mockStore(initialState);
const wrapper = mount(
<RhsHeaderPost
{...baseProps}
rootPostId='43'
/>, mountOptions);
expect(wrapper.exists()).toBe(true);
expect(wrapper.find(FollowButton).prop('isFollowing')).toBe(false);
});
test('should show following when no replies but user is mentioned', () => {
const {mountOptions} = mockStore(initialState);
const wrapper = mount(
<RhsHeaderPost
{...baseProps}
rootPostId='42'
/>, mountOptions);
expect(wrapper.exists()).toBe(true);
expect(wrapper.find(FollowButton).prop('isFollowing')).toBe(true);
});
});
......@@ -24,7 +24,6 @@ interface RhsHeaderPostProps {
channel: Channel;
isCollapsedThreadsEnabled: boolean;
isFollowingThread?: boolean;
isMentionedInRootPost?: boolean;
currentTeamId: string;
currentUserId: string;
setRhsExpanded: (b: boolean) => void;
......@@ -70,19 +69,13 @@ export default class RhsHeaderPost extends React.PureComponent<RhsHeaderPostProp
}
handleFollowChange = () => {
const {currentTeamId, currentUserId, rootPostId, isFollowingThread, isMentionedInRootPost} = this.props;
let followingThread: boolean;
if (isFollowingThread === null) {
followingThread = !isMentionedInRootPost;
} else {
followingThread = !isFollowingThread;
}
this.props.setThreadFollow(currentUserId, currentTeamId, rootPostId, followingThread);
const {currentTeamId, currentUserId, rootPostId, isFollowingThread} = this.props;
this.props.setThreadFollow(currentUserId, currentTeamId, rootPostId, !isFollowingThread);
}
render() {
let back;
const {isMentionedInRootPost, isFollowingThread} = this.props;
const {isFollowingThread} = this.props;
const closeSidebarTooltip = (
<Tooltip id='closeSidebarTooltip'>
<FormattedMessage
......@@ -190,7 +183,7 @@ export default class RhsHeaderPost extends React.PureComponent<RhsHeaderPostProp
{this.props.isCollapsedThreadsEnabled ? (
<FollowButton
className='sidebar--right__follow__thread'
isFollowing={isFollowingThread ?? isMentionedInRootPost}
isFollowing={isFollowingThread}
onClick={this.handleFollowChange}
/>
) : null}
......
......@@ -47,22 +47,32 @@ describe('Archived channels', () => {
// # Post a message in the channel
cy.postMessage('Test archive reply');
cy.clickPostCommentIcon();
cy.getLastPostId().then((id) => {
cy.clickPostCommentIcon(id);
// * RHS should be visible
cy.get('#rhsContainer').should('be.visible');
// * RHS should be visible
cy.get('#rhsContainer').should('be.visible');
// * RHS text box should be visible
cy.get('#reply_textbox').should('be.visible');
// * RHS text box should be visible
cy.get('#reply_textbox').should('be.visible');
// # Archive the channel
cy.uiArchiveChannel();
// # Archive the channel
cy.uiArchiveChannel();
// * Post text box should not be visible
cy.get('#post_textbox').should('not.exist');
// * Post text box should not be visible
cy.get('#post_textbox').should('not.exist');
// * RHS should not be visible
cy.get('#rhsContainer').should('not.exist');
// * RHS text box should not be visible
cy.get('#reply_textbox').should('not.exist');
// # Open RHS
cy.clickPostCommentIcon(id);
cy.get('#rhsContainer').should('be.visible');
// * RHS text box should not be visible
cy.get('#reply_textbox').should('not.exist');
});
});
it('MM-T1722 Can click reply arrow on a post from archived channel, from saved posts list', () => {
......
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.
// ***************************************************************
// - [#] indicates a test step (e.g. # Go to a page)
// - [*] indicates an assertion (e.g. * Check the title)
// - Use element ID when selecting an element. Create one if none.
// ***************************************************************
// Stage: @prod
// Group: @channel
import * as TIMEOUTS from '../../fixtures/timeouts';
describe('Leave channel', () => {
let testTeam;
let testUser;
let testChannel;
before(() => {
cy.apiUpdateConfig({
ServiceSettings: {
ThreadAutoFollow: true,
CollapsedThreads: 'default_off',
},
});
// # Login as test user and visit created channel
cy.apiInitSetup().then(({team, user}) => {
testUser = user;
testTeam = team;
cy.apiLogin(testUser);
cy.apiCreateChannel(testTeam.id, 'channel', 'channel').then(({channel}) => {
testChannel = channel;
});
});
});
beforeEach(() => {
cy.visit(`/${testTeam.name}/channels/${testChannel.name}`);
});
it('MM-T4429_1 Leave a channel while RHS is open', () => {
// * Post text box should be visible
cy.get('#post_textbox').should('be.visible');
// # Post a message in the channel
cy.postMessage('Test leave channel while RHS open');
cy.getLastPostId().then((id) => {
// # Open RHS
cy.clickPostCommentIcon(id);
// # Post a reply message
cy.postMessageAs({sender: testUser, message: 'another reply!', channelId: testChannel.id, rootId: id});
// * RHS should be visible
cy.get('#rhsContainer').should('be.visible');
// * RHS text box should be visible
cy.get('#reply_textbox').should('be.visible');
// # Archive the channel
cy.uiLeaveChannel();
cy.wait(TIMEOUTS.TWO_SEC); // eslint-disable-line cypress/no-unnecessary-waiting
// * RHS should not be visible
cy.get('#rhsContainer').should('not.exist');
// * Assert that user is redirected to townsquare
cy.url().should('include', '/channels/town-square');
cy.get('#channelHeaderTitle').should('be.visible').and('contain', 'Town Square');
});
});
it('MM-T4429_2 Leave a channel while RHS is open and CRT on', () => {
// * Post text box should be visible
cy.get('#post_textbox').should('be.visible');
// # Set CRT to on
cy.uiChangeCRTDisplaySetting('ON');
// # Post a message in the channel
cy.postMessage('Test leave channel while RHS open');
cy.getLastPostId().then((id) => {
// # Open RHS
cy.clickPostCommentIcon(id);
// # Post a reply message
cy.postMessageAs({sender: testUser, message: 'another reply!', channelId: testChannel.id, rootId: id});
// * RHS should be visible
cy.get('#rhsContainer').should('be.visible');
// * RHS text box should be visible
cy.get('#reply_textbox').should('be.visible');
// # Archive the channel
cy.uiLeaveChannel();
cy.wait(TIMEOUTS.TWO_SEC); // eslint-disable-line cypress/no-unnecessary-waiting
// * RHS should not be visible
cy.get('#rhsContainer').should('not.exist');
// * Assert that user is redirected to townsquare
cy.url().should('include', '/channels/town-square');
cy.get('#channelHeaderTitle').should('be.visible').and('contain', 'Town Square');
});
});
});
......@@ -71,6 +71,10 @@ function handleLeaveChannel(state: ThreadsState['counts'] = {}, action: GenericA
const teamId = action.data.team_id;
if (!teamId || !state[teamId]) {
return state;
}
const {unreadMentions, unreadThreads} = extra.threadsToDelete.reduce((curr, item: UserThread) => {
curr.unreadMentions += item.unread_mentions;
curr.unreadThreads = item.unread_replies > 0 ? curr.unreadThreads + 1 : curr.unreadThreads;
......@@ -113,6 +117,7 @@ export function countsIncludingDirectReducer(state: ThreadsState['counts'] = {},
}
case TeamTypes.LEAVE_TEAM:
return handleLeaveTeam(state, action);
case ChannelTypes.RECEIVED_CHANNEL_DELETED:
case ChannelTypes.LEAVE_CHANNEL:
return handleLeaveChannel(state, action, extra);
case ThreadTypes.RECEIVED_THREADS:
......@@ -140,6 +145,7 @@ export function countsReducer(state: ThreadsState['counts'] = {}, action: Generi
return handleLeaveTeam(state, action);
case UserTypes.LOGOUT_SUCCESS:
return {};
case ChannelTypes.RECEIVED_CHANNEL_DELETED:
case ChannelTypes.LEAVE_CHANNEL:
return handleLeaveChannel(state, action, extra);
case TeamTypes.RECEIVED_MY_TEAM_UNREADS: {
......
......@@ -112,6 +112,7 @@ export const threadsReducer = (state: ThreadsState['threads'] = {}, action: Gene
}
case UserTypes.LOGOUT_SUCCESS:
return {};
case ChannelTypes.RECEIVED_CHANNEL_DELETED:
case ChannelTypes.LEAVE_CHANNEL: {
if (!extra.threadsToDelete || extra.threadsToDelete.length === 0) {
return state;
......@@ -169,7 +170,10 @@ function reducer(state: ThreadsState = initialState, action: GenericAction): Thr
};
// acting as a 'middleware'
if (action.type === ChannelTypes.LEAVE_CHANNEL) {
if (
action.type === ChannelTypes.LEAVE_CHANNEL ||
action.type === ChannelTypes.RECEIVED_CHANNEL_DELETED
) {
if (!action.data.viewArchivedChannels) {
extra.threadsToDelete = getThreadsOfChannel(state.threads, action.data.id);
}
......
......@@ -174,6 +174,7 @@ export const threadsInTeamReducer = (state: ThreadsState['threadsInTeam'] = {},
return handleLeaveTeam(state, action);
case UserTypes.LOGOUT_SUCCESS:
return {};
case ChannelTypes.RECEIVED_CHANNEL_DELETED:
case ChannelTypes.LEAVE_CHANNEL:
return handleLeaveChannel(state, action, extra);
}
......@@ -242,6 +243,7 @@ export const unreadThreadsInTeamReducer = (state: ThreadsState['unreadThreadsInT
return handleLeaveTeam(state, action);
case UserTypes.LOGOUT_SUCCESS:
return {};
case ChannelTypes.RECEIVED_CHANNEL_DELETED:
case ChannelTypes.LEAVE_CHANNEL:
return handleLeaveChannel(state, action, extra);
case ThreadTypes.FOLLOW_CHANGED_THREAD:
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment