From 45ccd50f8c763a0729df0adb15162cb9f1213690 Mon Sep 17 00:00:00 2001 From: Harshil Sharma <18575143+harshilsharma63@users.noreply.github.com> Date: Thu, 5 Oct 2023 11:10:43 +0530 Subject: [PATCH] MM-54581 Fixed a bug where converted GM showed up in incorrect team as well when someone else converted it (#24633) * Fixed a bug where converted GM showed up in incorrect team as well when someone else converted it * Created new redux action * Fixed test * Unexported fjunction * More tests * Using old state instead new state as data exists in old, not new state --- .../src/actions/websocket_actions.jsx | 15 ++++- .../src/actions/websocket_actions.test.jsx | 59 +++++++++++++++++-- .../src/action_types/channels.ts | 2 + .../reducers/entities/channel_categories.ts | 32 ++++++++++ 4 files changed, 103 insertions(+), 5 deletions(-) diff --git a/webapp/channels/src/actions/websocket_actions.jsx b/webapp/channels/src/actions/websocket_actions.jsx index a2b77f59c8..5cf8740a21 100644 --- a/webapp/channels/src/actions/websocket_actions.jsx +++ b/webapp/channels/src/actions/websocket_actions.jsx @@ -622,9 +622,22 @@ export function handleChannelUpdatedEvent(msg) { return (doDispatch, doGetState) => { const channel = JSON.parse(msg.data.channel); - doDispatch({type: ChannelTypes.RECEIVED_CHANNEL, data: channel}); + const actions = [{type: ChannelTypes.RECEIVED_CHANNEL, data: channel}]; + // handling the case of GM converted to private channel. const state = doGetState(); + const existingChannel = getChannel(state, channel.id); + + // if the updated channel exists in store + if (existingChannel) { + // and it was a GM, converted to a private channel + if (existingChannel.type === General.GM_CHANNEL && channel.type === General.PRIVATE_CHANNEL) { + actions.push({type: ChannelTypes.GM_CONVERTED_TO_CHANNEL, data: channel}); + } + } + + doDispatch(batchActions(actions)); + if (channel.id === getCurrentChannelId(state)) { // using channel's team_id to ensure we always redirect to current channel even if channel's team changes. getHistory().replace(`${getRelativeTeamUrl(state, channel.team_id)}/channels/${channel.name}`); diff --git a/webapp/channels/src/actions/websocket_actions.test.jsx b/webapp/channels/src/actions/websocket_actions.test.jsx index 25d54b1d6e..8b50d69d25 100644 --- a/webapp/channels/src/actions/websocket_actions.test.jsx +++ b/webapp/channels/src/actions/websocket_actions.test.jsx @@ -1,7 +1,7 @@ // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. // See LICENSE.txt for license information. -import {ChannelTypes, UserTypes, CloudTypes} from 'mattermost-redux/action_types'; +import {UserTypes, CloudTypes} from 'mattermost-redux/action_types'; import {getGroup} from 'mattermost-redux/actions/groups'; import { getMentionsAndStatusesForPosts, @@ -636,6 +636,11 @@ describe('handleChannelUpdatedEvent', () => { entities: { channels: { currentChannelId: 'channel', + channels: { + channel: { + id: 'channel', + }, + }, }, teams: { currentTeamId: 'team', @@ -656,10 +661,56 @@ describe('handleChannelUpdatedEvent', () => { const msg = {data: {channel: JSON.stringify(channel)}}; testStore.dispatch(handleChannelUpdatedEvent(msg)); + expect(testStore.getActions()).toEqual([{ + type: 'BATCHING_REDUCER.BATCH', + meta: {batch: true}, + payload: [ + { + type: 'RECEIVED_CHANNEL', + data: { + id: 'channel', + team_id: 'team', + }, + }, + ], + }]); + }); - expect(testStore.getActions()).toEqual([ - {type: ChannelTypes.RECEIVED_CHANNEL, data: channel}, - ]); + test('when GM is converted to private channel', () => { + const state = initialState; + state.entities.channels.channels.channel.type = Constants.GM_CHANNEL; + + const testStore = configureStore(state); + const channel = { + id: 'channel', + team_id: 'team', + type: Constants.PRIVATE_CHANNEL, + }; + + const msg = {data: {channel: JSON.stringify(channel)}}; + testStore.dispatch(handleChannelUpdatedEvent(msg)); + expect(testStore.getActions()).toEqual([{ + type: 'BATCHING_REDUCER.BATCH', + meta: {batch: true}, + payload: [ + { + type: 'RECEIVED_CHANNEL', + data: { + id: 'channel', + team_id: 'team', + type: 'P', + }, + }, + { + type: 'GM_CONVERTED_TO_CHANNEL', + data: { + id: 'channel', + team_id: 'team', + type: 'P', + }, + }, + ], + }]); }); test('should not change URL when current channel is updated', () => { diff --git a/webapp/channels/src/packages/mattermost-redux/src/action_types/channels.ts b/webapp/channels/src/packages/mattermost-redux/src/action_types/channels.ts index b3120b761b..658ee8eeed 100644 --- a/webapp/channels/src/packages/mattermost-redux/src/action_types/channels.ts +++ b/webapp/channels/src/packages/mattermost-redux/src/action_types/channels.ts @@ -95,4 +95,6 @@ export default keyMirror({ DECREMENT_PINNED_POST_COUNT: null, INCREMENT_FILE_COUNT: null, + + GM_CONVERTED_TO_CHANNEL: null, }); diff --git a/webapp/channels/src/packages/mattermost-redux/src/reducers/entities/channel_categories.ts b/webapp/channels/src/packages/mattermost-redux/src/reducers/entities/channel_categories.ts index 4787696010..e338df06b8 100644 --- a/webapp/channels/src/packages/mattermost-redux/src/reducers/entities/channel_categories.ts +++ b/webapp/channels/src/packages/mattermost-redux/src/reducers/entities/channel_categories.ts @@ -4,6 +4,7 @@ import {combineReducers} from 'redux'; import type {ChannelCategory} from '@mattermost/types/channel_categories'; +import type {Channel} from '@mattermost/types/channels'; import type {Team} from '@mattermost/types/teams'; import type {IDMappedObjects, RelationOneToOne} from '@mattermost/types/utilities'; @@ -92,7 +93,38 @@ export function byId(state: IDMappedObjects = {}, action: Gener return changed ? nextState : state; } + case ChannelTypes.GM_CONVERTED_TO_CHANNEL: { + // For GM to Private channel conversion feature + // In the case when someone converts your GM to a private channel and moves it to a team + // you're not currently on, we need to remove the channel from "direct messages" category + // and add it to "channels" category of target team. Even though the server sends a websocket event about updated category data, + // it does so only for the team the channel got moved into, and not every team a user is part of, for performance reasons. + // For every other team, we update the state here. Everything is correct on server side, but we update state here + // to avoid re-fetching all categories again for all teams. + const receivedChannel = action.data as Channel; + const newState: IDMappedObjects = {}; + const categoryIDs = Object.keys(state); + + categoryIDs.forEach((categoryID) => { + if (categoryID.startsWith('channels_') && state[categoryID].team_id === receivedChannel.team_id && state[categoryID].channel_ids.indexOf(receivedChannel.id) < 0) { + // We don't need to worry about adding the channel in the right order as this is only + // an intermediate step, meant to handle the edge case of missing the upcoming "update category" + // websocket message, triggered on conversion of GM to private channel. + newState[categoryID] = { + ...state[categoryID], + channel_ids: [...state[categoryID].channel_ids, receivedChannel.id], + }; + } else { + newState[categoryID] = { + ...state[categoryID], + channel_ids: state[categoryID].channel_ids.filter((channelID) => channelID !== receivedChannel.id), + }; + } + }); + + return newState; + } case UserTypes.LOGOUT_SUCCESS: return {}; default: