From bd7afae6f031f7a994d1dff7b4635482729e844d Mon Sep 17 00:00:00 2001 From: Elias Nahum Date: Tue, 8 Sep 2020 13:09:46 -0300 Subject: [PATCH] Always load the thread & modify thread selector (#4771) * Always load the thread & modify thread selector * review feedback --- app/actions/views/channel.js | 14 +------------ app/mm-redux/selectors/entities/posts.ts | 17 +++++++-------- .../channel_post_list/channel_post_list.js | 4 ++-- .../channel_post_list.test.js | 2 +- .../channel/channel_post_list/index.js | 21 +++++++++---------- app/screens/flagged_posts/flagged_posts.js | 4 ++-- .../flagged_posts/flagged_posts.test.js | 2 +- app/screens/flagged_posts/index.js | 8 +++---- .../__snapshots__/long_post.test.js.snap | 4 ++-- app/screens/long_post/index.js | 6 +++--- app/screens/long_post/long_post.js | 5 ++--- app/screens/long_post/long_post.test.js | 2 +- app/screens/permalink/index.js | 13 ++++-------- app/screens/permalink/permalink.js | 3 +-- app/screens/permalink/permalink.test.js | 1 - app/screens/pinned_posts/index.js | 8 +++---- app/screens/pinned_posts/pinned_posts.js | 4 ++-- app/screens/pinned_posts/pinned_posts.test.js | 2 +- app/screens/recent_mentions/index.js | 8 +++---- .../recent_mentions/recent_mentions.js | 6 ++---- .../recent_mentions/recent_mentions.test.js | 2 +- app/screens/search/index.js | 14 ++++++------- app/screens/search/search.js | 5 ++--- 23 files changed, 64 insertions(+), 91 deletions(-) diff --git a/app/actions/views/channel.js b/app/actions/views/channel.js index 6bc68b8b5..ee419268f 100644 --- a/app/actions/views/channel.js +++ b/app/actions/views/channel.js @@ -31,7 +31,7 @@ import {getChannelByName as selectChannelByName, getChannelsIdForTeam} from '@mm import EventEmitter from '@mm-redux/utils/event_emitter'; import {lastChannelIdForTeam, loadSidebarDirectMessagesProfiles} from '@actions/helpers/channels'; -import {getPosts, getPostsBefore, getPostsSince, getPostThread, loadUnreadChannelPosts} from '@actions/views/post'; +import {getPosts, getPostsBefore, getPostsSince, loadUnreadChannelPosts} from '@actions/views/post'; import {INSERT_TO_COMMENT, INSERT_TO_DRAFT} from '@constants/post_draft'; import {getChannelReachable} from '@selectors/channel'; import telemetry from '@telemetry'; @@ -111,18 +111,6 @@ export function fetchPostActionWithRetry(action, maxTries = MAX_RETRIES) { }; } -export function loadThreadIfNecessary(rootId) { - return (dispatch, getState) => { - const state = getState(); - const {posts, postsInThread} = state.entities.posts; - const threadPosts = postsInThread[rootId]; - - if (!posts[rootId] || !threadPosts) { - dispatch(getPostThread(rootId)); - } - }; -} - export function selectInitialChannel(teamId) { return (dispatch, getState) => { const state = getState(); diff --git a/app/mm-redux/selectors/entities/posts.ts b/app/mm-redux/selectors/entities/posts.ts index 39457df03..3ebc71b9e 100644 --- a/app/mm-redux/selectors/entities/posts.ts +++ b/app/mm-redux/selectors/entities/posts.ts @@ -69,23 +69,20 @@ export const getPostsInCurrentChannel: (a: GlobalState) => Array) => Array<$ID> { return createIdsSelector( getAllPosts, - (state: GlobalState, rootId: string) => state.entities.posts.postsInThread[rootId] || [], - (state: GlobalState, rootId) => state.entities.posts.posts[rootId], - (posts, postsForThread, rootPost) => { + (state: GlobalState, rootId: string) => state.entities.posts.posts[rootId], + (posts, rootPost) => { const thread: Post[] = []; if (rootPost) { thread.push(rootPost); - } - postsForThread.forEach((id) => { - const post = posts[id]; - if (post) { - thread.push(post); + const postsArray = Object.values(posts).filter((p) => p.root_id === rootPost.id); + if (postsArray.length) { + thread.push(...postsArray); } - }); - thread.sort(comparePosts); + thread.sort(comparePosts); + } return thread.map((post) => post.id); }, diff --git a/app/screens/channel/channel_post_list/channel_post_list.js b/app/screens/channel/channel_post_list/channel_post_list.js index e994ebe73..458d0a7cd 100644 --- a/app/screens/channel/channel_post_list/channel_post_list.js +++ b/app/screens/channel/channel_post_list/channel_post_list.js @@ -30,7 +30,7 @@ export default class ChannelPostList extends PureComponent { static propTypes = { actions: PropTypes.shape({ loadPostsIfNecessaryWithRetry: PropTypes.func.isRequired, - loadThreadIfNecessary: PropTypes.func.isRequired, + getPostThread: PropTypes.func.isRequired, increasePostVisibility: PropTypes.func.isRequired, selectPost: PropTypes.func.isRequired, recordLoadTime: PropTypes.func.isRequired, @@ -106,7 +106,7 @@ export default class ChannelPostList extends PureComponent { const rootId = (post.root_id || post.id); Keyboard.dismiss(); - actions.loadThreadIfNecessary(rootId); + actions.getPostThread(rootId); actions.selectPost(rootId); const screen = 'Thread'; diff --git a/app/screens/channel/channel_post_list/channel_post_list.test.js b/app/screens/channel/channel_post_list/channel_post_list.test.js index 44d488824..efac19727 100644 --- a/app/screens/channel/channel_post_list/channel_post_list.test.js +++ b/app/screens/channel/channel_post_list/channel_post_list.test.js @@ -12,7 +12,7 @@ describe('ChannelPostList', () => { const baseProps = { actions: { loadPostsIfNecessaryWithRetry: jest.fn(), - loadThreadIfNecessary: jest.fn(), + getPostThread: jest.fn(), increasePostVisibility: jest.fn(), selectPost: jest.fn(), recordLoadTime: jest.fn(), diff --git a/app/screens/channel/channel_post_list/index.js b/app/screens/channel/channel_post_list/index.js index ee88eecad..b917f8775 100644 --- a/app/screens/channel/channel_post_list/index.js +++ b/app/screens/channel/channel_post_list/index.js @@ -4,21 +4,20 @@ import {bindActionCreators} from 'redux'; import {connect} from 'react-redux'; +import { + loadPostsIfNecessaryWithRetry, + increasePostVisibility, + refreshChannelWithRetry, +} from '@actions/views/channel'; +import {getPostThread} from '@actions/views/post'; +import {recordLoadTime} from 'app/actions/views/root'; +import {Types} from '@constants'; import {selectPost} from '@mm-redux/actions/posts'; import {getPostIdsInCurrentChannel} from '@mm-redux/selectors/entities/posts'; import {getCurrentChannelId} from '@mm-redux/selectors/entities/channels'; import {getCurrentUserId} from '@mm-redux/selectors/entities/users'; import {getTheme} from '@mm-redux/selectors/entities/preferences'; - -import { - loadPostsIfNecessaryWithRetry, - loadThreadIfNecessary, - increasePostVisibility, - refreshChannelWithRetry, -} from 'app/actions/views/channel'; -import {recordLoadTime} from 'app/actions/views/root'; -import {Types} from 'app/constants'; -import {isLandscape} from 'app/selectors/device'; +import {isLandscape} from '@selectors/device'; import ChannelPostList from './channel_post_list'; @@ -44,7 +43,7 @@ function mapDispatchToProps(dispatch) { return { actions: bindActionCreators({ loadPostsIfNecessaryWithRetry, - loadThreadIfNecessary, + getPostThread, increasePostVisibility, selectPost, recordLoadTime, diff --git a/app/screens/flagged_posts/flagged_posts.js b/app/screens/flagged_posts/flagged_posts.js index 7aa46243e..9a12ee282 100644 --- a/app/screens/flagged_posts/flagged_posts.js +++ b/app/screens/flagged_posts/flagged_posts.js @@ -36,7 +36,7 @@ export default class FlaggedPosts extends PureComponent { actions: PropTypes.shape({ clearSearch: PropTypes.func.isRequired, loadChannelsByTeamName: PropTypes.func.isRequired, - loadThreadIfNecessary: PropTypes.func.isRequired, + getPostThread: PropTypes.func.isRequired, getFlaggedPosts: PropTypes.func.isRequired, selectFocusedPostId: PropTypes.func.isRequired, selectPost: PropTypes.func.isRequired, @@ -103,7 +103,7 @@ export default class FlaggedPosts extends PureComponent { }; Keyboard.dismiss(); - actions.loadThreadIfNecessary(rootId); + actions.getPostThread(rootId); actions.selectPost(rootId); goToScreen(screen, title, passProps); }; diff --git a/app/screens/flagged_posts/flagged_posts.test.js b/app/screens/flagged_posts/flagged_posts.test.js index 39d7bc0aa..275d0b612 100644 --- a/app/screens/flagged_posts/flagged_posts.test.js +++ b/app/screens/flagged_posts/flagged_posts.test.js @@ -15,7 +15,7 @@ describe('FlaggedPosts', () => { actions: { clearSearch: jest.fn(), loadChannelsByTeamName: jest.fn(), - loadThreadIfNecessary: jest.fn(), + getPostThread: jest.fn(), getFlaggedPosts: jest.fn(), selectFocusedPostId: jest.fn(), selectPost: jest.fn(), diff --git a/app/screens/flagged_posts/index.js b/app/screens/flagged_posts/index.js index 2aae50935..fce9448ee 100644 --- a/app/screens/flagged_posts/index.js +++ b/app/screens/flagged_posts/index.js @@ -4,12 +4,12 @@ import {bindActionCreators} from 'redux'; import {connect} from 'react-redux'; +import {loadChannelsByTeamName} from '@actions/views/channel'; +import {getPostThread} from '@actions/views/post'; import {selectFocusedPostId, selectPost} from '@mm-redux/actions/posts'; import {clearSearch, getFlaggedPosts} from '@mm-redux/actions/search'; import {getTheme} from '@mm-redux/selectors/entities/preferences'; - -import {loadChannelsByTeamName, loadThreadIfNecessary} from 'app/actions/views/channel'; -import {makePreparePostIdsForSearchPosts} from 'app/selectors/post_list'; +import {makePreparePostIdsForSearchPosts} from '@selectors/post_list'; import FlaggedPosts from './flagged_posts'; @@ -30,7 +30,7 @@ function mapDispatchToProps(dispatch) { actions: bindActionCreators({ clearSearch, loadChannelsByTeamName, - loadThreadIfNecessary, + getPostThread, getFlaggedPosts, selectFocusedPostId, selectPost, diff --git a/app/screens/long_post/__snapshots__/long_post.test.js.snap b/app/screens/long_post/__snapshots__/long_post.test.js.snap index b8844613e..8ff6e8827 100644 --- a/app/screens/long_post/__snapshots__/long_post.test.js.snap +++ b/app/screens/long_post/__snapshots__/long_post.test.js.snap @@ -35,7 +35,7 @@ LongPost { "navigationEventListener": undefined, "props": Object { "actions": Object { - "loadThreadIfNecessary": [MockFunction], + "getPostThread": [MockFunction], "selectPost": [MockFunction], }, "intl": Object { @@ -147,7 +147,7 @@ LongPost { "_element": ({ describe('LongPost', () => { const baseProps = { actions: { - loadThreadIfNecessary: jest.fn(), + getPostThread: jest.fn(), selectPost: jest.fn(), }, postId: 'post-id', diff --git a/app/screens/permalink/index.js b/app/screens/permalink/index.js index fa6525372..f54da2e0b 100644 --- a/app/screens/permalink/index.js +++ b/app/screens/permalink/index.js @@ -4,6 +4,9 @@ import {bindActionCreators} from 'redux'; import {connect} from 'react-redux'; +import {handleSelectChannel} from '@actions/views/channel'; +import {getPostsAround, getPostThread} from '@actions/views/post'; +import {handleTeamChange} from '@actions/views/select_team'; import {getChannel as getChannelAction, joinChannel} from '@mm-redux/actions/channels'; import {selectPost} from '@mm-redux/actions/posts'; import {makeGetChannel, getMyChannelMemberships} from '@mm-redux/selectors/entities/channels'; @@ -11,14 +14,7 @@ import {makeGetPostIdsAroundPost, getPost} from '@mm-redux/selectors/entities/po import {getTheme} from '@mm-redux/selectors/entities/preferences'; import {getCurrentTeamId} from '@mm-redux/selectors/entities/teams'; import {getCurrentUserId} from '@mm-redux/selectors/entities/users'; - -import { - handleSelectChannel, - loadThreadIfNecessary, -} from 'app/actions/views/channel'; -import {getPostsAround, getPostThread} from 'app/actions/views/post'; -import {handleTeamChange} from 'app/actions/views/select_team'; -import {isLandscape} from 'app/selectors/device'; +import {isLandscape} from '@selectors/device'; import Permalink from './permalink'; @@ -66,7 +62,6 @@ function mapDispatchToProps(dispatch) { handleSelectChannel, handleTeamChange, joinChannel, - loadThreadIfNecessary, selectPost, }, dispatch), }; diff --git a/app/screens/permalink/permalink.js b/app/screens/permalink/permalink.js index 2e63f5cfc..8bc8b9a32 100644 --- a/app/screens/permalink/permalink.js +++ b/app/screens/permalink/permalink.js @@ -60,7 +60,6 @@ export default class Permalink extends PureComponent { handleSelectChannel: PropTypes.func.isRequired, handleTeamChange: PropTypes.func.isRequired, joinChannel: PropTypes.func.isRequired, - loadThreadIfNecessary: PropTypes.func.isRequired, selectPost: PropTypes.func.isRequired, }).isRequired, channelId: PropTypes.string, @@ -147,7 +146,7 @@ export default class Permalink extends PureComponent { rootId, }; - actions.loadThreadIfNecessary(rootId); + actions.getPostThread(rootId); actions.selectPost(rootId); goToScreen(screen, title, passProps); diff --git a/app/screens/permalink/permalink.test.js b/app/screens/permalink/permalink.test.js index b1f0be07c..70810a09b 100644 --- a/app/screens/permalink/permalink.test.js +++ b/app/screens/permalink/permalink.test.js @@ -18,7 +18,6 @@ describe('Permalink', () => { handleSelectChannel: jest.fn(), handleTeamChange: jest.fn(), joinChannel: jest.fn(), - loadThreadIfNecessary: jest.fn(), selectPost: jest.fn(), }; diff --git a/app/screens/pinned_posts/index.js b/app/screens/pinned_posts/index.js index ebea713ec..b3f792504 100644 --- a/app/screens/pinned_posts/index.js +++ b/app/screens/pinned_posts/index.js @@ -4,12 +4,12 @@ import {bindActionCreators} from 'redux'; import {connect} from 'react-redux'; +import {loadChannelsByTeamName} from '@actions/views/channel'; +import {getPostThread} from '@actions/views/post'; import {selectFocusedPostId, selectPost} from '@mm-redux/actions/posts'; import {clearSearch, getPinnedPosts} from '@mm-redux/actions/search'; import {getTheme} from '@mm-redux/selectors/entities/preferences'; - -import {loadChannelsByTeamName, loadThreadIfNecessary} from 'app/actions/views/channel'; -import {makePreparePostIdsForSearchPosts} from 'app/selectors/post_list'; +import {makePreparePostIdsForSearchPosts} from '@selectors/post_list'; import PinnedPosts from './pinned_posts'; @@ -32,7 +32,7 @@ function mapDispatchToProps(dispatch) { actions: bindActionCreators({ clearSearch, loadChannelsByTeamName, - loadThreadIfNecessary, + getPostThread, getPinnedPosts, selectFocusedPostId, selectPost, diff --git a/app/screens/pinned_posts/pinned_posts.js b/app/screens/pinned_posts/pinned_posts.js index 00cc488aa..897267b5e 100644 --- a/app/screens/pinned_posts/pinned_posts.js +++ b/app/screens/pinned_posts/pinned_posts.js @@ -36,7 +36,7 @@ export default class PinnedPosts extends PureComponent { actions: PropTypes.shape({ clearSearch: PropTypes.func.isRequired, loadChannelsByTeamName: PropTypes.func.isRequired, - loadThreadIfNecessary: PropTypes.func.isRequired, + getPostThread: PropTypes.func.isRequired, getPinnedPosts: PropTypes.func.isRequired, selectFocusedPostId: PropTypes.func.isRequired, selectPost: PropTypes.func.isRequired, @@ -104,7 +104,7 @@ export default class PinnedPosts extends PureComponent { rootId, }; Keyboard.dismiss(); - actions.loadThreadIfNecessary(rootId); + actions.getPostThread(rootId); actions.selectPost(rootId); goToScreen(screen, title, passProps); }; diff --git a/app/screens/pinned_posts/pinned_posts.test.js b/app/screens/pinned_posts/pinned_posts.test.js index acd3d7f87..834f574c3 100644 --- a/app/screens/pinned_posts/pinned_posts.test.js +++ b/app/screens/pinned_posts/pinned_posts.test.js @@ -14,7 +14,7 @@ describe('PinnedPosts', () => { actions: { clearSearch: jest.fn(), loadChannelsByTeamName: jest.fn(), - loadThreadIfNecessary: jest.fn(), + getPostThread: jest.fn(), getPinnedPosts: jest.fn(), selectFocusedPostId: jest.fn(), selectPost: jest.fn(), diff --git a/app/screens/recent_mentions/index.js b/app/screens/recent_mentions/index.js index 5b8311992..ceb011abd 100644 --- a/app/screens/recent_mentions/index.js +++ b/app/screens/recent_mentions/index.js @@ -4,12 +4,12 @@ import {bindActionCreators} from 'redux'; import {connect} from 'react-redux'; +import {loadChannelsByTeamName} from '@actions/views/channel'; +import {getPostThread} from '@actions/views/post'; import {selectFocusedPostId, selectPost} from '@mm-redux/actions/posts'; import {clearSearch, getRecentMentions} from '@mm-redux/actions/search'; import {getTheme} from '@mm-redux/selectors/entities/preferences'; - -import {loadChannelsByTeamName, loadThreadIfNecessary} from 'app/actions/views/channel'; -import {makePreparePostIdsForSearchPosts} from 'app/selectors/post_list'; +import {makePreparePostIdsForSearchPosts} from '@selectors/post_list'; import RecentMentions from './recent_mentions'; @@ -30,7 +30,7 @@ function mapDispatchToProps(dispatch) { actions: bindActionCreators({ clearSearch, loadChannelsByTeamName, - loadThreadIfNecessary, + getPostThread, getRecentMentions, selectFocusedPostId, selectPost, diff --git a/app/screens/recent_mentions/recent_mentions.js b/app/screens/recent_mentions/recent_mentions.js index 4861bd21d..c55d4d64b 100644 --- a/app/screens/recent_mentions/recent_mentions.js +++ b/app/screens/recent_mentions/recent_mentions.js @@ -36,13 +36,11 @@ export default class RecentMentions extends PureComponent { actions: PropTypes.shape({ clearSearch: PropTypes.func.isRequired, loadChannelsByTeamName: PropTypes.func.isRequired, - loadThreadIfNecessary: PropTypes.func.isRequired, + getPostThread: PropTypes.func.isRequired, getRecentMentions: PropTypes.func.isRequired, selectFocusedPostId: PropTypes.func.isRequired, selectPost: PropTypes.func.isRequired, }).isRequired, - didFail: PropTypes.bool, - isLoading: PropTypes.bool, postIds: PropTypes.array, theme: PropTypes.object.isRequired, }; @@ -99,7 +97,7 @@ export default class RecentMentions extends PureComponent { }; Keyboard.dismiss(); - actions.loadThreadIfNecessary(rootId); + actions.getPostThread(rootId); actions.selectPost(rootId); goToScreen(screen, title, passProps); }; diff --git a/app/screens/recent_mentions/recent_mentions.test.js b/app/screens/recent_mentions/recent_mentions.test.js index cf82df5b7..04a472275 100644 --- a/app/screens/recent_mentions/recent_mentions.test.js +++ b/app/screens/recent_mentions/recent_mentions.test.js @@ -15,7 +15,7 @@ describe('RecentMentions', () => { actions: { clearSearch: jest.fn(), loadChannelsByTeamName: jest.fn(), - loadThreadIfNecessary: jest.fn(), + getPostThread: jest.fn(), getRecentMentions: jest.fn(), selectFocusedPostId: jest.fn(), selectPost: jest.fn(), diff --git a/app/screens/search/index.js b/app/screens/search/index.js index 8ddbc13e5..a2f38035f 100644 --- a/app/screens/search/index.js +++ b/app/screens/search/index.js @@ -4,6 +4,9 @@ import {bindActionCreators} from 'redux'; import {connect} from 'react-redux'; +import {loadChannelsByTeamName} from '@actions/views/channel'; +import {getPostThread} from '@actions/views/post'; +import {handleSearchDraftChanged} from '@actions/views/search'; import {selectFocusedPostId, selectPost} from '@mm-redux/actions/posts'; import {clearSearch, removeSearchTerms, searchPostsWithParams, getMorePostsForSearch} from '@mm-redux/actions/search'; import {getCurrentChannelId, filterPostIds} from '@mm-redux/selectors/entities/channels'; @@ -14,12 +17,9 @@ import {isTimezoneEnabled} from '@mm-redux/selectors/entities/timezone'; import {isMinimumServerVersion} from '@mm-redux/utils/helpers'; import {getUserCurrentTimezone} from '@mm-redux/utils/timezone_utils'; import {getCurrentUser} from '@mm-redux/selectors/entities/users'; - -import {loadChannelsByTeamName, loadThreadIfNecessary} from 'app/actions/views/channel'; -import {handleSearchDraftChanged} from 'app/actions/views/search'; -import {isLandscape} from 'app/selectors/device'; -import {makePreparePostIdsForSearchPosts} from 'app/selectors/post_list'; -import {getDeviceUtcOffset, getUtcOffsetForTimeZone} from 'app/utils/timezone'; +import {isLandscape} from '@selectors/device'; +import {makePreparePostIdsForSearchPosts} from '@selectors/post_list'; +import {getDeviceUtcOffset, getUtcOffsetForTimeZone} from '@utils/timezone'; import Search from './search'; @@ -76,7 +76,7 @@ function mapDispatchToProps(dispatch) { clearSearch, handleSearchDraftChanged, loadChannelsByTeamName, - loadThreadIfNecessary, + getPostThread, removeSearchTerms, selectFocusedPostId, searchPostsWithParams, diff --git a/app/screens/search/search.js b/app/screens/search/search.js index a0f0dd423..384922029 100644 --- a/app/screens/search/search.js +++ b/app/screens/search/search.js @@ -60,14 +60,13 @@ export default class Search extends PureComponent { clearSearch: PropTypes.func.isRequired, handleSearchDraftChanged: PropTypes.func.isRequired, loadChannelsByTeamName: PropTypes.func.isRequired, - loadThreadIfNecessary: PropTypes.func.isRequired, + getPostThread: PropTypes.func.isRequired, removeSearchTerms: PropTypes.func.isRequired, searchPostsWithParams: PropTypes.func.isRequired, getMorePostsForSearch: PropTypes.func.isRequired, selectFocusedPostId: PropTypes.func.isRequired, selectPost: PropTypes.func.isRequired, }).isRequired, - componentId: PropTypes.string.isRequired, currentTeamId: PropTypes.string.isRequired, initialValue: PropTypes.string, isLandscape: PropTypes.bool.isRequired, @@ -222,7 +221,7 @@ export default class Search extends PureComponent { const rootId = (post.root_id || post.id); Keyboard.dismiss(); - actions.loadThreadIfNecessary(rootId); + actions.getPostThread(rootId); actions.selectPost(rootId); const screen = 'Thread';