Using new getKnownUsers api to cleanup the users on leave channel (#4220)

This commit is contained in:
Jesús Espino 2020-05-11 18:19:38 +02:00 committed by Amit Uttam
parent a01bdd6fc4
commit d17e94eae3
No known key found for this signature in database
GPG key ID: B3D4CC224111BC5D
9 changed files with 127 additions and 135 deletions

View file

@ -38,6 +38,12 @@ const mockChanelsRequest = (teamId, channels = []) => {
reply(200, channels); reply(200, channels);
}; };
const mockGetKnownUsersRequest = (userIds = []) => {
nock(Client4.getBaseRoute()).
get('/users/known').
reply(200, userIds);
};
const mockRolesRequest = (rolesToLoad = []) => { const mockRolesRequest = (rolesToLoad = []) => {
nock(Client4.getRolesRoute()). nock(Client4.getRolesRoute()).
post('/names', JSON.stringify(rolesToLoad)). post('/names', JSON.stringify(rolesToLoad)).
@ -901,9 +907,6 @@ describe('Actions.Websocket doReconnect', () => {
describe('Actions.Websocket notVisibleUsersActions', () => { describe('Actions.Websocket notVisibleUsersActions', () => {
configureMockStore([thunk]); configureMockStore([thunk]);
const channel1 = TestHelper.fakeChannelWithId('');
const channel2 = TestHelper.fakeChannelWithId('');
const me = TestHelper.fakeUserWithId(); const me = TestHelper.fakeUserWithId();
const user = TestHelper.fakeUserWithId(); const user = TestHelper.fakeUserWithId();
const user2 = TestHelper.fakeUserWithId(); const user2 = TestHelper.fakeUserWithId();
@ -911,29 +914,17 @@ describe('Actions.Websocket notVisibleUsersActions', () => {
const user4 = TestHelper.fakeUserWithId(); const user4 = TestHelper.fakeUserWithId();
const user5 = TestHelper.fakeUserWithId(); const user5 = TestHelper.fakeUserWithId();
it('should do nothing if the known users and the profiles list are the same', () => { it('should do nothing if the known users and the profiles list are the same', async () => {
const membersInChannel = {
[channel1.id]: {
[user.id]: {channel_id: channel1.id, user_id: user.id},
[user2.id]: {channel_id: channel1.id, user_id: user2.id},
},
[channel2.id]: {
[user3.id]: {channel_id: channel2.id, user_id: user3.id},
},
};
const profiles = { const profiles = {
[me.id]: me, [me.id]: me,
[user.id]: user, [user.id]: user,
[user2.id]: user2, [user2.id]: user2,
[user3.id]: user3, [user3.id]: user3,
}; };
Client4.serverVersion = '5.23.0';
const state = { const state = {
entities: { entities: {
channels: {
membersInChannel,
},
users: { users: {
currentUserId: me.id, currentUserId: me.id,
profiles, profiles,
@ -941,31 +932,21 @@ describe('Actions.Websocket notVisibleUsersActions', () => {
}, },
}; };
const actions = Actions.notVisibleUsersActions(state); mockGetKnownUsersRequest([user.id, user2.id, user3.id]);
const actions = await Actions.notVisibleUsersActions(state);
expect(actions.length).toEqual(0); expect(actions.length).toEqual(0);
}); });
it('should do nothing if there are known users in my memberships but not in the profiles list', () => { it('should do nothing if there are known users in my memberships but not in the profiles list', async () => {
const membersInChannel = {
[channel1.id]: {
[user.id]: {channel_id: channel1.id, user_id: user.id},
[user2.id]: {channel_id: channel1.id, user_id: user2.id},
},
[channel2.id]: {
[user3.id]: {channel_id: channel2.id, user_id: user3.id},
},
};
const profiles = { const profiles = {
[me.id]: me, [me.id]: me,
[user3.id]: user3, [user3.id]: user3,
}; };
Client4.serverVersion = '5.23.0';
const state = { const state = {
entities: { entities: {
channels: {
membersInChannel,
},
users: { users: {
currentUserId: me.id, currentUserId: me.id,
profiles, profiles,
@ -973,20 +954,13 @@ describe('Actions.Websocket notVisibleUsersActions', () => {
}, },
}; };
const actions = Actions.notVisibleUsersActions(state); mockGetKnownUsersRequest([user.id, user2.id, user3.id]);
const actions = await Actions.notVisibleUsersActions(state);
expect(actions.length).toEqual(0); expect(actions.length).toEqual(0);
}); });
it('should remove the users if there are unknown users in the profiles list', async () => { it('should remove the users if there are unknown users in the profiles list', async () => {
const membersInChannel = {
[channel1.id]: {
[user.id]: {channel_id: channel1.id, user_id: user.id},
},
[channel2.id]: {
[user3.id]: {channel_id: channel2.id, user_id: user3.id},
},
};
const profiles = { const profiles = {
[me.id]: me, [me.id]: me,
[user.id]: user, [user.id]: user,
@ -995,12 +969,10 @@ describe('Actions.Websocket notVisibleUsersActions', () => {
[user4.id]: user4, [user4.id]: user4,
[user5.id]: user5, [user5.id]: user5,
}; };
Client4.serverVersion = '5.23.0';
const state = { const state = {
entities: { entities: {
channels: {
membersInChannel,
},
users: { users: {
currentUserId: me.id, currentUserId: me.id,
profiles, profiles,
@ -1008,15 +980,43 @@ describe('Actions.Websocket notVisibleUsersActions', () => {
}, },
}; };
mockGetKnownUsersRequest([user.id, user3.id]);
const expectedAction = [ const expectedAction = [
{type: UserTypes.PROFILE_NO_LONGER_VISIBLE, data: {user_id: user2.id}}, {type: UserTypes.PROFILE_NO_LONGER_VISIBLE, data: {user_id: user2.id}},
{type: UserTypes.PROFILE_NO_LONGER_VISIBLE, data: {user_id: user4.id}}, {type: UserTypes.PROFILE_NO_LONGER_VISIBLE, data: {user_id: user4.id}},
{type: UserTypes.PROFILE_NO_LONGER_VISIBLE, data: {user_id: user5.id}}, {type: UserTypes.PROFILE_NO_LONGER_VISIBLE, data: {user_id: user5.id}},
]; ];
const actions = Actions.notVisibleUsersActions(state); const actions = await Actions.notVisibleUsersActions(state);
expect(actions.length).toEqual(3); expect(actions.length).toEqual(3);
expect(actions).toEqual(expectedAction); expect(actions).toEqual(expectedAction);
}); });
it('should do nothing if the server version is less than 5.23', async () => {
const profiles = {
[me.id]: me,
[user.id]: user,
[user2.id]: user2,
[user3.id]: user3,
[user4.id]: user4,
[user5.id]: user5,
};
Client4.serverVersion = '5.22.0';
const state = {
entities: {
users: {
currentUserId: me.id,
profiles,
},
},
};
mockGetKnownUsersRequest([user.id, user3.id]);
const actions = await Actions.notVisibleUsersActions(state);
expect(actions.length).toEqual(0);
});
}); });
describe('Actions.Websocket handleUserTypingEvent', () => { describe('Actions.Websocket handleUserTypingEvent', () => {
@ -1089,4 +1089,4 @@ describe('Actions.Websocket handleUserTypingEvent', () => {
const actionTypes = testStore.getActions().map((action) => action.type); const actionTypes = testStore.getActions().map((action) => action.type);
expect(actionTypes).toEqual(expectedActionsTypes); expect(actionTypes).toEqual(expectedActionsTypes);
}); });
}); });

View file

@ -16,7 +16,6 @@ import {
getCurrentChannelStats, getCurrentChannelStats,
getChannelMembersInChannels, getChannelMembersInChannels,
isManuallyUnread, isManuallyUnread,
getKnownUsers,
} from '@mm-redux/selectors/entities/channels'; } from '@mm-redux/selectors/entities/channels';
import {getConfig} from '@mm-redux/selectors/entities/general'; import {getConfig} from '@mm-redux/selectors/entities/general';
import {getAllPosts, getPost as selectPost} from '@mm-redux/selectors/entities/posts'; import {getAllPosts, getPost as selectPost} from '@mm-redux/selectors/entities/posts';
@ -518,7 +517,7 @@ function handlePostUnread(msg: WebSocketMessage) {
} }
function handleLeaveTeamEvent(msg: Partial<WebSocketMessage>) { function handleLeaveTeamEvent(msg: Partial<WebSocketMessage>) {
return (dispatch: DispatchFunc, getState: GetStateFunc) => { return async (dispatch: DispatchFunc, getState: GetStateFunc) => {
const state = getState(); const state = getState();
const teams = getTeamsSelector(state); const teams = getTeamsSelector(state);
const currentTeamId = getCurrentTeamId(state); const currentTeamId = getCurrentTeamId(state);
@ -527,7 +526,7 @@ function handleLeaveTeamEvent(msg: Partial<WebSocketMessage>) {
if (currentUser.id === msg.data.user_id) { if (currentUser.id === msg.data.user_id) {
const actions: Array<GenericAction> = [{type: TeamTypes.LEAVE_TEAM, data: teams[msg.data.team_id]}]; const actions: Array<GenericAction> = [{type: TeamTypes.LEAVE_TEAM, data: teams[msg.data.team_id]}];
if (isGuest(currentUser.roles)) { if (isGuest(currentUser.roles)) {
const notVisible = notVisibleUsersActions(state); const notVisible = await notVisibleUsersActions(state);
if (notVisible.length) { if (notVisible.length) {
actions.push(...notVisible); actions.push(...notVisible);
} }
@ -641,17 +640,31 @@ function handleUserRemovedEvent(msg: WebSocketMessage) {
const currentChannelId = getCurrentChannelId(state); const currentChannelId = getCurrentChannelId(state);
const currentTeamId = getCurrentTeamId(state); const currentTeamId = getCurrentTeamId(state);
const currentUser = getCurrentUser(state); const currentUser = getCurrentUser(state);
const actions: Array<GenericAction> = [{ const actions: Array<GenericAction> = [];
type: ChannelTypes.CHANNEL_MEMBER_REMOVED, let channelId;
data: { let userId;
channel_id: msg.broadcast.channel_id,
user_id: msg.data.user_id, if (msg.data.user_id) {
}, userId = msg.data.user_id;
}]; channelId = msg.broadcast.channel_id;
} else if (msg.broadcast.user_id) {
channelId = msg.data.channel_id;
userId = msg.broadcast.user_id;
}
if (userId) {
actions.push({
type: ChannelTypes.CHANNEL_MEMBER_REMOVED,
data: {
channel_id: channelId,
user_id: userId,
},
});
}
const channel = channels[currentChannelId]; const channel = channels[currentChannelId];
if (msg.data.user_id !== currentUser.id) { if (msg.data?.user_id !== currentUser.id) {
const members = getChannelMembersInChannels(state); const members = getChannelMembersInChannels(state);
const isMember = Object.values(members).some((member) => member[msg.data.user_id]); const isMember = Object.values(members).some((member) => member[msg.data.user_id]);
if (channel && isGuest(currentUser.roles) && !isMember) { if (channel && isGuest(currentUser.roles) && !isMember) {
@ -665,6 +678,7 @@ function handleUserRemovedEvent(msg: WebSocketMessage) {
} }
} }
let redirectToDefaultChannel = false;
if (msg.broadcast.user_id === currentUser.id && currentTeamId) { if (msg.broadcast.user_id === currentUser.id && currentTeamId) {
const {data: myData}: any = await dispatch(loadChannelsForTeam(currentTeamId, true)); const {data: myData}: any = await dispatch(loadChannelsForTeam(currentTeamId, true));
@ -689,10 +703,10 @@ function handleUserRemovedEvent(msg: WebSocketMessage) {
if (msg.data.channel_id === currentChannelId) { if (msg.data.channel_id === currentChannelId) {
// emit the event so the client can change his own state // emit the event so the client can change his own state
EventEmitter.emit(General.SWITCH_TO_DEFAULT_CHANNEL, currentTeamId); redirectToDefaultChannel = true;
} }
if (isGuest(currentUser.roles)) { if (isGuest(currentUser.roles)) {
const notVisible = notVisibleUsersActions(state); const notVisible = await notVisibleUsersActions(state);
if (notVisible.length) { if (notVisible.length) {
actions.push(...notVisible); actions.push(...notVisible);
} }
@ -706,6 +720,9 @@ function handleUserRemovedEvent(msg: WebSocketMessage) {
} }
dispatch(batchActions(actions, 'BATCH_WS_USER_REMOVED')); dispatch(batchActions(actions, 'BATCH_WS_USER_REMOVED'));
if (redirectToDefaultChannel) {
EventEmitter.emit(General.SWITCH_TO_DEFAULT_CHANNEL, currentTeamId);
}
} catch { } catch {
// do nothing // do nothing
} }
@ -1126,8 +1143,18 @@ function handleOpenDialogEvent(msg: WebSocketMessage) {
} }
// Helpers // Helpers
export function notVisibleUsersActions(state: GlobalState): Array<GenericAction> { export async function notVisibleUsersActions(state: GlobalState): Promise<Array<GenericAction>> {
const knownUsers = getKnownUsers(state); if (!isMinimumServerVersion(Client4.getServerVersion(), 5, 23)) {
return [];
}
let knownUsers: Set<string>;
try {
const fetchResult = await Client4.getKnownUsers();
knownUsers = new Set(fetchResult);
} catch (err) {
return [];
}
knownUsers.add(getCurrentUserId(state));
const allUsers = Object.keys(getUsers(state)); const allUsers = Object.keys(getUsers(state));
const usersToRemove = new Set(allUsers.filter((x) => !knownUsers.has(x))); const usersToRemove = new Set(allUsers.filter((x) => !knownUsers.has(x)));
@ -1151,4 +1178,4 @@ export function userTyping(state: GlobalState, channelId: string, parentPostId:
websocketClient.userTyping(channelId, parentPostId); websocketClient.userTyping(channelId, parentPostId);
lastTimeTypingSent = t; lastTimeTypingSent = t;
} }
} }

View file

@ -23,7 +23,7 @@ import {bindClientFunc, forceLogoutIfNecessary, debounce} from './helpers';
import {getMyPreferences, makeDirectChannelVisibleIfNecessary, makeGroupMessageVisibleIfNecessary} from './preferences'; import {getMyPreferences, makeDirectChannelVisibleIfNecessary, makeGroupMessageVisibleIfNecessary} from './preferences';
import {Dictionary} from '@mm-redux/types/utilities'; import {Dictionary} from '@mm-redux/types/utilities';
export function checkMfa(loginId: string): ActionFunc { export function checkMfa(loginId: string): ActionFunc {
return async (dispatch: DispatchFunc, getState: GetStateFunc) => { return async (dispatch: DispatchFunc) => {
dispatch({type: UserTypes.CHECK_MFA_REQUEST, data: null}); dispatch({type: UserTypes.CHECK_MFA_REQUEST, data: null});
try { try {
const data = await Client4.checkUserMfa(loginId); const data = await Client4.checkUserMfa(loginId);
@ -985,7 +985,7 @@ export function stopPeriodicStatusUpdates(): ActionFunc {
} }
export function updateMe(user: UserProfile): ActionFunc { export function updateMe(user: UserProfile): ActionFunc {
return async (dispatch: DispatchFunc, getState: GetStateFunc) => { return async (dispatch: DispatchFunc) => {
dispatch({type: UserTypes.UPDATE_ME_REQUEST, data: null}); dispatch({type: UserTypes.UPDATE_ME_REQUEST, data: null});
let data; let data;

View file

@ -448,6 +448,15 @@ export default class Client4 {
); );
} }
getKnownUsers = async () => {
this.trackEvent('api', 'api_get_known_users');
return this.doFetch(
`${this.getUsersRoute()}/known`,
{method: 'get'},
);
}
sendPasswordResetEmail = async (email: string) => { sendPasswordResetEmail = async (email: string) => {
this.trackEvent('api', 'api_users_send_password_reset'); this.trackEvent('api', 'api_users_send_password_reset');

View file

@ -11,6 +11,7 @@ import {Team} from '@mm-redux/types/teams';
function removeMemberFromChannels(state: RelationOneToOne<Channel, UserIDMappedObjects<ChannelMembership>>, action: GenericAction) { function removeMemberFromChannels(state: RelationOneToOne<Channel, UserIDMappedObjects<ChannelMembership>>, action: GenericAction) {
const nextState = {...state}; const nextState = {...state};
Object.keys(state).forEach((channel) => { Object.keys(state).forEach((channel) => {
nextState[channel] = {...nextState[channel]};
delete nextState[channel][action.data.user_id]; delete nextState[channel][action.data.user_id];
}); });
return nextState; return nextState;
@ -371,7 +372,7 @@ function myMembers(state: RelationOneToOne<Channel, ChannelMembership> = {}, act
if (sync) { if (sync) {
current.forEach((member: ChannelMembership) => { current.forEach((member: ChannelMembership) => {
const id = member.channel_id; const id = member.channel_id;
if (channelMembers.find((cm: ChannelMembership) => cm.channel_id === id)) { if (channelMembers.find((cm: ChannelMembership) => cm.channel_id !== id)) {
delete nextState[id]; delete nextState[id];
hasNewValues = true; hasNewValues = true;
} }

View file

@ -135,56 +135,6 @@ describe('Selectors.Channels.getChannelsInCurrentTeam', () => {
}); });
}); });
describe('Selectors.Channels.getKnownUsers', () => {
const channel1 = TestHelper.fakeChannelWithId('');
const channel2 = TestHelper.fakeChannelWithId('');
const me = TestHelper.fakeUserWithId();
const user = TestHelper.fakeUserWithId();
const user2 = TestHelper.fakeUserWithId();
const user3 = TestHelper.fakeUserWithId();
const membersInChannel = {
[channel1.id]: {
[user.id]: {channel_id: channel1.id, user_id: user.id},
[user2.id]: {channel_id: channel1.id, user_id: user2.id},
},
[channel2.id]: {
[user3.id]: {channel_id: channel2.id, user_id: user3.id},
},
};
it('should return all members of all my channels', () => {
const testState = deepFreezeAndThrowOnMutation({
entities: {
users: {
currentUserId: me.id,
},
channels: {
membersInChannel,
},
},
});
assert.deepEqual(Selectors.getKnownUsers(testState), new Set([me.id, user.id, user2.id, user3.id]));
});
it('should return only me if I have no channels', () => {
const testState = deepFreezeAndThrowOnMutation({
entities: {
users: {
currentUserId: me.id,
},
channels: {
membersInChannel: {},
},
},
});
assert.deepEqual(Selectors.getKnownUsers(testState), new Set([me.id]));
});
});
describe('Selectors.Channels.getMyChannels', () => { describe('Selectors.Channels.getMyChannels', () => {
const team1 = TestHelper.fakeTeamWithId(); const team1 = TestHelper.fakeTeamWithId();
const team2 = TestHelper.fakeTeamWithId(); const team2 = TestHelper.fakeTeamWithId();

View file

@ -44,20 +44,6 @@ export function getChannelMembersInChannels(state: GlobalState): RelationOneToOn
return state.entities.channels.membersInChannel; return state.entities.channels.membersInChannel;
} }
export const getKnownUsers: (a: GlobalState) => Set<string> = createSelector(
getChannelMembersInChannels,
getCurrentUserId,
(channelsMemberships: RelationOneToOne<Channel, UserIDMappedObjects<ChannelMembership>>, currentUserId: string): Set<string> => {
const knownUsers: Set<string> = new Set([currentUserId]);
for (const membersInChannel of Object.values(channelsMemberships)) {
for (const member of Object.values(membersInChannel)) {
knownUsers.add(member.user_id);
}
}
return knownUsers;
}
);
function sortChannelsByRecencyOrAlpha(locale: string, lastPosts: RelationOneToOne<Channel, Post>, sorting: SortingType, a: Channel, b: Channel) { function sortChannelsByRecencyOrAlpha(locale: string, lastPosts: RelationOneToOne<Channel, Post>, sorting: SortingType, a: Channel, b: Channel) {
if (sorting === 'recent') { if (sorting === 'recent') {
return sortChannelsByRecency(lastPosts, a, b); return sortChannelsByRecency(lastPosts, a, b);

View file

@ -12,13 +12,16 @@ import {General} from '@mm-redux/constants';
import {getConfig} from '@mm-redux/selectors/entities/general'; import {getConfig} from '@mm-redux/selectors/entities/general';
import {getTeammateNameDisplaySetting, getTheme} from '@mm-redux/selectors/entities/preferences'; import {getTeammateNameDisplaySetting, getTheme} from '@mm-redux/selectors/entities/preferences';
import {getCurrentTeamId} from '@mm-redux/selectors/entities/teams'; import {getCurrentTeamId} from '@mm-redux/selectors/entities/teams';
import {getCurrentUserId, getUsers} from '@mm-redux/selectors/entities/users'; import {getCurrentUserId, getUsers, getCurrentUser} from '@mm-redux/selectors/entities/users';
import {isGuest} from '@utils/users';
import MoreDirectMessages from './more_dms'; import MoreDirectMessages from './more_dms';
function mapStateToProps(state) { function mapStateToProps(state) {
const config = getConfig(state); const config = getConfig(state);
const restrictDirectMessage = config.RestrictDirectMessage === General.RESTRICT_DIRECT_MESSAGE_ANY; const restrictDirectMessage = config.RestrictDirectMessage === General.RESTRICT_DIRECT_MESSAGE_ANY;
const currentUser = getCurrentUser(state);
return { return {
restrictDirectMessage, restrictDirectMessage,
@ -27,6 +30,7 @@ function mapStateToProps(state) {
theme: getTheme(state), theme: getTheme(state),
currentDisplayName: state.views.channel.displayName, currentDisplayName: state.views.channel.displayName,
currentUserId: getCurrentUserId(state), currentUserId: getCurrentUserId(state),
isGuest: isGuest(currentUser),
currentTeamId: getCurrentTeamId(state), currentTeamId: getCurrentTeamId(state),
isLandscape: isLandscape(state), isLandscape: isLandscape(state),
}; };

View file

@ -52,6 +52,7 @@ export default class MoreDirectMessages extends PureComponent {
currentDisplayName: PropTypes.string, currentDisplayName: PropTypes.string,
currentTeamId: PropTypes.string.isRequired, currentTeamId: PropTypes.string.isRequired,
currentUserId: PropTypes.string.isRequired, currentUserId: PropTypes.string.isRequired,
isGuest: PropTypes.object.isRequired,
restrictDirectMessage: PropTypes.bool.isRequired, restrictDirectMessage: PropTypes.bool.isRequired,
teammateNameDisplay: PropTypes.string, teammateNameDisplay: PropTypes.string,
theme: PropTypes.object.isRequired, theme: PropTypes.object.isRequired,
@ -358,6 +359,8 @@ export default class MoreDirectMessages extends PureComponent {
); );
}; };
filterUnknownUsers = (u) => Boolean(this.props.allProfiles[u.id])
renderLoading = () => { renderLoading = () => {
const {theme} = this.props; const {theme} = this.props;
const {loading} = this.state; const {loading} = this.state;
@ -399,7 +402,7 @@ export default class MoreDirectMessages extends PureComponent {
render() { render() {
const {formatMessage} = this.context.intl; const {formatMessage} = this.context.intl;
const {currentUserId, theme, isLandscape} = this.props; const {isGuest, currentUserId, theme, isLandscape} = this.props;
const { const {
loading, loading,
profiles, profiles,
@ -435,7 +438,7 @@ export default class MoreDirectMessages extends PureComponent {
let listType; let listType;
if (term) { if (term) {
const exactMatches = []; const exactMatches = [];
const results = filterProfilesMatchingTerm(searchResults, term).filter((p) => { const filterByTerm = (p) => {
if (selectedCount > 0 && p.id === currentUserId) { if (selectedCount > 0 && p.id === currentUserId) {
return false; return false;
} }
@ -446,11 +449,23 @@ export default class MoreDirectMessages extends PureComponent {
} }
return true; return true;
}); };
let results;
if (isGuest) {
results = filterProfilesMatchingTerm(searchResults, term).filter((u) => filterByTerm(u) && this.filterUnknownUsers(u));
} else {
results = filterProfilesMatchingTerm(searchResults, term).filter(filterByTerm);
}
data = [...exactMatches, ...results]; data = [...exactMatches, ...results];
listType = FLATLIST; listType = FLATLIST;
} else { } else {
data = createProfilesSections(profiles); if (isGuest) {
data = createProfilesSections(profiles.filter(this.filterUnknownUsers));
} else {
data = createProfilesSections(profiles);
}
listType = SECTIONLIST; listType = SECTIONLIST;
} }