diff --git a/webapp/channels/src/actions/new_post.ts b/webapp/channels/src/actions/new_post.ts index 76162ec30a..4b2d31c92a 100644 --- a/webapp/channels/src/actions/new_post.ts +++ b/webapp/channels/src/actions/new_post.ts @@ -43,7 +43,9 @@ export function completePostReceive(post: Post, websocketMessageProps: NewPostMe return async (dispatch, getState) => { const state = getState(); const rootPost = PostSelectors.getPost(state, post.root_id); - if (post.root_id && !rootPost) { + const isPostFromCurrentChannel = post.channel_id === getCurrentChannelId(state); + + if (post.root_id && !rootPost && isPostFromCurrentChannel) { const result = await dispatch(PostActions.getPostThread(post.root_id)); if ('error' in result) { @@ -52,7 +54,7 @@ export function completePostReceive(post: Post, websocketMessageProps: NewPostMe } const actions: AnyAction[] = []; - if (post.channel_id === getCurrentChannelId(getState())) { + if (isPostFromCurrentChannel) { actions.push({ type: ActionTypes.INCREASE_POST_VISIBILITY, data: post.channel_id, diff --git a/webapp/channels/src/actions/status_actions.ts b/webapp/channels/src/actions/status_actions.ts index d1197deaa2..82e024285d 100644 --- a/webapp/channels/src/actions/status_actions.ts +++ b/webapp/channels/src/actions/status_actions.ts @@ -18,32 +18,34 @@ import type {GlobalState} from 'types/store'; export function loadStatusesForChannelAndSidebar(): ActionFunc { return (dispatch, getState) => { const state = getState(); - const statusesToLoad: Record = {}; const channelId = getCurrentChannelId(state); const postsInChannel = getPostsInCurrentChannel(state); + const userIds = new Set(); + if (postsInChannel) { const posts = postsInChannel.slice(0, state.views.channel.postVisibility[channelId] || 0); for (const post of posts) { if (post.user_id) { - statusesToLoad[post.user_id] = true; + userIds.add(post.user_id); } } } - const dmPrefs = getDirectShowPreferences(state); - - for (const pref of dmPrefs) { - if (pref.value === 'true') { - statusesToLoad[pref.name] = true; + const directShowPreferences = getDirectShowPreferences(state); + for (const directShowPreference of directShowPreferences) { + if (directShowPreference.value === 'true') { + // This is the other user's id in the DM + userIds.add(directShowPreference.name); } } const currentUserId = getCurrentUserId(state); - statusesToLoad[currentUserId] = true; + userIds.add(currentUserId); + + dispatch(loadStatusesByIds(Array.from(userIds))); - dispatch(loadStatusesByIds(Object.keys(statusesToLoad))); return {data: true}; }; } diff --git a/webapp/channels/src/actions/websocket_actions.jsx b/webapp/channels/src/actions/websocket_actions.jsx index 081ab1ef65..05e37e6f18 100644 --- a/webapp/channels/src/actions/websocket_actions.jsx +++ b/webapp/channels/src/actions/websocket_actions.jsx @@ -40,7 +40,7 @@ import { getPosts, getPostThread, getMentionsAndStatusesForPosts, - getThreadsForPosts, + getPostThreads, postDeleted, receivedNewPost, receivedPost, @@ -739,7 +739,7 @@ export function handleNewPostEvents(queue) { myDispatch(batchActions(actions)); // Load the posts' threads - myDispatch(getThreadsForPosts(posts)); + myDispatch(getPostThreads(posts)); // And any other data needed for them getMentionsAndStatusesForPosts(posts, myDispatch, myGetState); diff --git a/webapp/channels/src/actions/websocket_actions.test.jsx b/webapp/channels/src/actions/websocket_actions.test.jsx index 83617defdf..8f48186615 100644 --- a/webapp/channels/src/actions/websocket_actions.test.jsx +++ b/webapp/channels/src/actions/websocket_actions.test.jsx @@ -5,7 +5,7 @@ import {UserTypes, CloudTypes} from 'mattermost-redux/action_types'; import {getGroup} from 'mattermost-redux/actions/groups'; import { getMentionsAndStatusesForPosts, - getThreadsForPosts, + getPostThreads, receivedNewPost, } from 'mattermost-redux/actions/posts'; import {getUser} from 'mattermost-redux/actions/users'; @@ -40,7 +40,7 @@ import { jest.mock('mattermost-redux/actions/posts', () => ({ ...jest.requireActual('mattermost-redux/actions/posts'), - getThreadsForPosts: jest.fn(() => ({type: 'GET_THREADS_FOR_POSTS'})), + getPostThreads: jest.fn(() => ({type: 'GET_THREADS_FOR_POSTS'})), getMentionsAndStatusesForPosts: jest.fn(), })); @@ -619,7 +619,7 @@ describe('handleNewPostEvents', () => { type: 'GET_THREADS_FOR_POSTS', }, ]); - expect(getThreadsForPosts).toHaveBeenCalledWith(posts); + expect(getPostThreads).toHaveBeenCalledWith(posts); expect(getMentionsAndStatusesForPosts).toHaveBeenCalledWith(posts, expect.anything(), expect.anything()); }); }); diff --git a/webapp/channels/src/components/channel_layout/channel_controller.tsx b/webapp/channels/src/components/channel_layout/channel_controller.tsx index 8fd6fac33f..a2440771de 100644 --- a/webapp/channels/src/components/channel_layout/channel_controller.tsx +++ b/webapp/channels/src/components/channel_layout/channel_controller.tsx @@ -44,12 +44,13 @@ export default function ChannelController(props: Props) { }, []); useEffect(() => { - let loadStatusesIntervalId: ReturnType; + let loadStatusesIntervalId: NodeJS.Timeout; if (enabledUserStatuses) { loadStatusesIntervalId = setInterval(() => { dispatch(loadStatusesForChannelAndSidebar()); }, Constants.STATUS_INTERVAL); } + return () => { clearInterval(loadStatusesIntervalId); }; diff --git a/webapp/channels/src/components/team_controller/actions/index.ts b/webapp/channels/src/components/team_controller/actions/index.ts index b6f9e949a2..ff63cf1001 100644 --- a/webapp/channels/src/components/team_controller/actions/index.ts +++ b/webapp/channels/src/components/team_controller/actions/index.ts @@ -10,6 +10,7 @@ import {logError} from 'mattermost-redux/actions/errors'; import {getGroups, getAllGroupsAssociatedToChannelsInTeam, getAllGroupsAssociatedToTeam, getGroupsByUserIdPaginated} from 'mattermost-redux/actions/groups'; import {forceLogoutIfNecessary} from 'mattermost-redux/actions/helpers'; import {getTeamByName, selectTeam} from 'mattermost-redux/actions/teams'; +import {getIsUserStatusesConfigEnabled} from 'mattermost-redux/selectors/entities/common'; import {getLicense} from 'mattermost-redux/selectors/entities/general'; import {isCustomGroupsEnabled} from 'mattermost-redux/selectors/entities/preferences'; import {getCurrentUser} from 'mattermost-redux/selectors/entities/users'; @@ -38,7 +39,13 @@ export function initializeTeam(team: Team): ActionFuncAsync { return {error: error as ServerError}; } - dispatch(loadStatusesForChannelAndSidebar()); + const enabledUserStatuses = getIsUserStatusesConfigEnabled(state); + + if (enabledUserStatuses) { + // This is the first time in the pool of user statuses that we request, + // subsequent requests will be done via setInterval at channel_controller.tsx + dispatch(loadStatusesForChannelAndSidebar()); + } const license = getLicense(state); const customGroupEnabled = isCustomGroupsEnabled(state); diff --git a/webapp/channels/src/packages/mattermost-redux/src/actions/posts.test.ts b/webapp/channels/src/packages/mattermost-redux/src/actions/posts.test.ts index 325880d0b2..d8852c681e 100644 --- a/webapp/channels/src/packages/mattermost-redux/src/actions/posts.test.ts +++ b/webapp/channels/src/packages/mattermost-redux/src/actions/posts.test.ts @@ -17,6 +17,8 @@ import {Client4} from 'mattermost-redux/client'; import type {GetStateFunc} from 'mattermost-redux/types/actions'; import {getPreferenceKey} from 'mattermost-redux/utils/preference_utils'; +import mockStore from 'tests/test_store'; + import TestHelper from '../../test/test_helper'; import configureStore from '../../test/test_store'; import {Preferences, Posts, RequestStatus} from '../constants'; @@ -1560,103 +1562,6 @@ describe('Actions.Posts', () => { }); }); - describe('getThreadsForPosts', () => { - beforeAll(() => { - TestHelper.initBasic(Client4); - }); - - afterAll(() => { - TestHelper.tearDown(); - }); - - let channelId = ''; - let post1 = TestHelper.getPostMock(); - let post2 = TestHelper.getPostMock(); - let post3 = TestHelper.getPostMock(); - let comment = TestHelper.getPostMock(); - - beforeEach(async () => { - store = configureStore(); - - channelId = TestHelper.basicChannel!.id; - post1 = TestHelper.getPostMock({id: TestHelper.generateId(), channel_id: channelId, message: ''}); - post2 = TestHelper.getPostMock({id: TestHelper.generateId(), channel_id: channelId, message: ''}); - comment = TestHelper.getPostMock({id: TestHelper.generateId(), root_id: post1.id, channel_id: channelId, message: ''}); - post3 = TestHelper.getPostMock({id: TestHelper.generateId(), channel_id: channelId, message: ''}); - - store.dispatch(Actions.receivedPostsInChannel({ - order: [post2.id, post3.id], - posts: {[post2.id]: post2, [post3.id]: post3}, - } as PostList, channelId)); - - const threadList = { - order: [post1.id], - posts: { - [post1.id]: post1, - [comment.id]: comment, - }, - }; - - nock(Client4.getBaseRoute()). - get(`/posts/${post1.id}/thread?skipFetchThreads=false&collapsedThreads=false&collapsedThreadsExtended=false&direction=down&perPage=60`). - reply(200, threadList); - }); - - it('handlesNull', async () => { - const ret = await store.dispatch(Actions.getThreadsForPosts(null as any)); - expect(ret).toEqual({data: true}); - - const state: GlobalState = store.getState(); - - const getRequest = state.requests.posts.getPostThread; - if (getRequest.status === RequestStatus.FAILURE) { - throw new Error(JSON.stringify(getRequest.error)); - } - - const { - postsInChannel, - postsInThread, - } = state.entities.posts; - - expect(postsInChannel[channelId]).toBeTruthy(); - expect(postsInChannel[channelId][0].order).toEqual([post2.id, post3.id]); - expect(!postsInThread[post1.id]).toBeTruthy(); - - const found = postsInChannel[channelId].find((block) => block.order.indexOf(comment.id) !== -1); - - // should not have found comment in postsInChannel - expect(!found).toBeTruthy(); - }); - - it('pullsUpTheThreadOfAMissingPost', async () => { - await store.dispatch(Actions.getThreadsForPosts([comment])); - - const state: GlobalState = store.getState(); - - const getRequest = state.requests.posts.getPostThread; - if (getRequest.status === RequestStatus.FAILURE) { - throw new Error(JSON.stringify(getRequest.error)); - } - - const { - posts, - postsInChannel, - postsInThread, - } = state.entities.posts; - - expect(posts).toBeTruthy(); - expect(postsInChannel[channelId][0].order).toEqual([post2.id, post3.id]); - expect(posts[post1.id]).toBeTruthy(); - expect(postsInThread[post1.id]).toBeTruthy(); - expect(postsInThread[post1.id]).toEqual([comment.id]); - - const found = postsInChannel[channelId].find((block) => block.order.indexOf(comment.id) !== -1); - - // should not have found comment in postsInChannel - expect(!found).toBeTruthy(); - }); - }); - describe('receivedPostsBefore', () => { it('Should return default false for oldest key if param does not exist', () => { const posts = {} as PostList; @@ -1709,3 +1614,74 @@ describe('Actions.Posts', () => { }); }); }); + +describe('getPostThreads', () => { + beforeAll(() => { + TestHelper.initBasic(Client4); + }); + + afterAll(() => { + TestHelper.tearDown(); + }); + + const channelId = 'currentChannelId'; + const initialState = { + entities: { + channels: { + currentChannelId: channelId, + }, + users: { + currentUserId: 'currentUserId', + statuses: {}, + }, + general: { + config: {}, + }, + posts: { + posts: {}, + }, + preferences: { + myPreferences: {}, + }, + }, + }; + + const post1 = TestHelper.getPostMock({id: TestHelper.generateId(), channel_id: channelId, message: '', user_id: 'currentUserId'}); + const comment = TestHelper.getPostMock({id: TestHelper.generateId(), root_id: post1.id, channel_id: channelId, message: '', user_id: 'currentUserId'}); + + it('handles null', async () => { + const testStore = await mockStore(initialState); + const ret = await testStore.dispatch(Actions.getPostThreads(null as any)); + expect(ret).toEqual({data: true}); + + expect(testStore.getActions()).toEqual([]); + }); + + it('pulls up the thread of missing root post in the same channel', async () => { + const testStore = await mockStore(initialState); + nock(Client4.getBaseRoute()). + get(`/posts/${post1.id}/thread?skipFetchThreads=false&collapsedThreads=false&collapsedThreadsExtended=false&direction=down&perPage=60`). + reply(200, { + order: [post1.id], + posts: { + [post1.id]: post1, + [comment.id]: comment, + }, + }); + + await testStore.dispatch(Actions.getPostThreads([comment])); + + expect(testStore.getActions()[1].payload[0].type).toEqual('RECEIVED_POSTS'); + expect(testStore.getActions()[1].payload[0].data.posts).toEqual({ + [post1.id]: post1, + [comment.id]: comment, + }); + + expect(testStore.getActions()[1].payload[1].type).toEqual('RECEIVED_POSTS_IN_THREAD'); + expect(testStore.getActions()[1].payload[1].rootId).toEqual(post1.id); + expect(testStore.getActions()[1].payload[1].data.posts).toEqual({ + [post1.id]: post1, + [comment.id]: comment, + }); + }); +}); diff --git a/webapp/channels/src/packages/mattermost-redux/src/actions/posts.ts b/webapp/channels/src/packages/mattermost-redux/src/actions/posts.ts index d0c35430c4..894cc39d6b 100644 --- a/webapp/channels/src/packages/mattermost-redux/src/actions/posts.ts +++ b/webapp/channels/src/packages/mattermost-redux/src/actions/posts.ts @@ -955,34 +955,46 @@ export function getPostsAround(channelId: string, postId: string, perPage = Post }; } -// getThreadsForPosts is intended for an array of posts that have been batched -// (see the actions/websocket_actions/handleNewPostEvents function in the webapp) -export function getThreadsForPosts(posts: Post[], fetchThreads = true): ThunkActionFunc { - const rootsSet = new Set(); +/** + * getPostThreads is intended for an array of posts that have been batched + * (see the actions/websocket_actions/handleNewPostEvents function in the webapp) +* */ +export function getPostThreads(posts: Post[], fetchThreads = true): ThunkActionFunc { return (dispatch, getState) => { if (!Array.isArray(posts) || !posts.length) { return {data: true}; } const state = getState(); - const promises: Array> = []; + const currentChannelId = getCurrentChannelId(state); - posts.forEach((post) => { + const getPostThreadPromises: Array>> = []; + + const rootPostIds = new Set(); + + for (const post of posts) { if (!post.root_id) { - return; + continue; } + const rootPost = PostSelectors.getPost(state, post.root_id); - - if (!rootPost) { - rootsSet.add(post.root_id); + if (rootPost) { + continue; } - }); - rootsSet.forEach((rootId) => { - promises.push(dispatch(getPostThread(rootId, fetchThreads))); - }); + if (rootPostIds.has(post.root_id)) { + continue; + } - return Promise.all(promises); + // At this point, we know that this post is a thread/reply and its root post is not in the store + rootPostIds.add(post.root_id); + + if (post.channel_id === currentChannelId) { + getPostThreadPromises.push(dispatch(getPostThread(post.root_id, fetchThreads))); + } + } + + return Promise.all(getPostThreadPromises); }; }