From 5d78720c2b48609c61b4ab4893aad528db9201bd Mon Sep 17 00:00:00 2001 From: Mattermost Build Date: Wed, 11 Aug 2021 13:19:22 +0200 Subject: [PATCH] MM-37485 MM-37602 MM-37610 MM-37684 MM-37719 MM-37522 MM-37720 App crash, Sync issue fixes (#5612) (#5615) Summary Fixes app crash on opening channel header Fixes app crash on opening permalink from permalink screen Fixes showing duplicates channels while searching in "Jump to..." Fixes auto following the thread on opening the thread view when there are unreads Fixes updation of threads state (Read/Unread/Deleted) of already loaded threads Fixes counter showing as blank due to threads sync issue Doesn't show Follow Thread/Message when CRT is not enabled Ticket Link https://mattermost.atlassian.net/browse/MM-37485 https://mattermost.atlassian.net/browse/MM-37602 https://mattermost.atlassian.net/browse/MM-37610 https://mattermost.atlassian.net/browse/MM-37684 https://mattermost.atlassian.net/browse/MM-37719 https://mattermost.atlassian.net/browse/MM-37522 https://mattermost.atlassian.net/browse/MM-37720 Device Information This PR was tested on: iOS 14.5 (cherry picked from commit fc929891eafb55096a7823a589f30a9193af0d80) Co-authored-by: Anurag Shivarathri --- .../global_threads/global_threads.tsx | 20 ++++--------------- .../list/__snapshots__/list.test.js.snap | 2 +- .../sidebars/main/channels_list/list/list.js | 2 +- app/mm-redux/actions/threads.ts | 16 +++++++++++++-- app/mm-redux/reducers/entities/threads.ts | 3 ++- app/mm-redux/utils/channel_utils.ts | 5 ++++- app/screens/post_options/post_options.js | 4 ++-- app/screens/thread/thread_base.js | 10 +++++++--- app/store/middlewares/helpers.js | 8 +++++--- app/store/middlewares/middleware.test.js | 18 ++++++++++++++++- 10 files changed, 57 insertions(+), 31 deletions(-) diff --git a/app/components/global_threads/global_threads.tsx b/app/components/global_threads/global_threads.tsx index d8391bd07..e2c24594c 100644 --- a/app/components/global_threads/global_threads.tsx +++ b/app/components/global_threads/global_threads.tsx @@ -54,8 +54,8 @@ function GlobalThreadsList({actions, allThreadIds, intl, teamId, theme, threadCo React.useEffect(() => { // Loads on mount, Loads on team change scrollToTop(); - loadThreads('', ids[0]); - }, [teamId]); + loadThreads('', '', viewingUnreads); + }, [teamId, viewingUnreads]); // Prevent from being called when an active request is pending. const loadMoreThreads = async () => { @@ -79,18 +79,6 @@ function GlobalThreadsList({actions, allThreadIds, intl, teamId, theme, threadCo } }; - const handleViewAllThreads = () => { - scrollToTop(); - loadThreads('', allThreadIds[0], false); - actions.handleViewingGlobalThreadsAll(); - }; - - const handleViewUnreadThreads = () => { - scrollToTop(); - loadThreads('', unreadThreadIds[0], true); - actions.handleViewingGlobalThreadsUnreads(); - }; - const markAllAsRead = () => { Alert.alert( intl.formatMessage({ @@ -131,8 +119,8 @@ function GlobalThreadsList({actions, allThreadIds, intl, teamId, theme, threadCo theme={theme} threadIds={ids} viewingUnreads={viewingUnreads} - viewAllThreads={handleViewAllThreads} - viewUnreadThreads={handleViewUnreadThreads} + viewAllThreads={actions.handleViewingGlobalThreadsAll} + viewUnreadThreads={actions.handleViewingGlobalThreadsUnreads} /> ); } diff --git a/app/components/sidebars/main/channels_list/list/__snapshots__/list.test.js.snap b/app/components/sidebars/main/channels_list/list/__snapshots__/list.test.js.snap index 36a61bd83..dd74d8a10 100644 --- a/app/components/sidebars/main/channels_list/list/__snapshots__/list.test.js.snap +++ b/app/components/sidebars/main/channels_list/list/__snapshots__/list.test.js.snap @@ -84,7 +84,7 @@ exports[`ChannelsList List should match snapshot with collapsed threads enabled Array [ undefined, Object { - "marginTop": 70, + "marginTop": 64, }, ] } diff --git a/app/components/sidebars/main/channels_list/list/list.js b/app/components/sidebars/main/channels_list/list/list.js index cc9e1dd26..02bde1955 100644 --- a/app/components/sidebars/main/channels_list/list/list.js +++ b/app/components/sidebars/main/channels_list/list/list.js @@ -406,7 +406,7 @@ export default class List extends PureComponent { const paddingBottom = this.listContentPadding(); const indicatorStyle = [styles.above]; if (collapsedThreadsEnabled) { - indicatorStyle.push({marginTop: 70}); + indicatorStyle.push({marginTop: 64}); } return ( diff --git a/app/mm-redux/actions/threads.ts b/app/mm-redux/actions/threads.ts index 3cd687927..2e9e5b333 100644 --- a/app/mm-redux/actions/threads.ts +++ b/app/mm-redux/actions/threads.ts @@ -29,6 +29,8 @@ export function getThreads(userId: string, teamId: string, before = '', after = } if (userThreadList) { + const currentUserId = getCurrentUserId(getState()); + const data = { threads: [] as UserThread[], participants: [] as UserThread['participants'], @@ -43,10 +45,15 @@ export function getThreads(userId: string, teamId: string, before = '', after = is_following: true, }); - // participants, participantIds + // data.participantIds - Get Missing Profiles + // data.participants - Received Profile List thread.participants?.forEach((participant) => { data.participantIds.push(participant.id); - data.participants.push(participant); + + // Exclude current user + if (participant.id !== currentUserId) { + data.participants.push(participant); + } }); // posts @@ -63,6 +70,11 @@ export function getThreads(userId: string, teamId: string, before = '', after = ...userThreadList, threads: data.threads, team_id: teamId, + removeOldThreads: + !before && !after && // When loading on mount + perPage >= ViewTypes.CRT_CHUNK_SIZE && // Exclude on load where we get 5 threads + !since && // Exclude reconnect + !unread, // Exclude when user is loading unreads or on switching to "All your threads" it will take time to fetch from server }, }, ]; diff --git a/app/mm-redux/reducers/entities/threads.ts b/app/mm-redux/reducers/entities/threads.ts index 4b01864dc..87393d163 100644 --- a/app/mm-redux/reducers/entities/threads.ts +++ b/app/mm-redux/reducers/entities/threads.ts @@ -106,7 +106,8 @@ export const threadsReducer = (state: ThreadsState['threads'] = {}, action: Gene export const threadsInTeamReducer = (state: ThreadsState['threadsInTeam'] = {}, action: GenericAction) => { switch (action.type) { case ThreadTypes.RECEIVED_THREADS: { - const nextSet = new Set(state[action.data.team_id]); + const threads = action.data.removeOldThreads ? [] : state[action.data.team_id]; + const nextSet = new Set(threads); action.data.threads.forEach((thread: UserThread) => { nextSet.add(thread.id); diff --git a/app/mm-redux/utils/channel_utils.ts b/app/mm-redux/utils/channel_utils.ts index 2b011f1e6..f95cf9b50 100644 --- a/app/mm-redux/utils/channel_utils.ts +++ b/app/mm-redux/utils/channel_utils.ts @@ -44,7 +44,10 @@ export function buildDisplayableChannelListWithUnreadSection(usersState: UsersSt const missingDirectChannels = createMissingDirectChannels(currentUserId, myChannels, myPreferences); const channels = buildChannels(usersState, myChannels, missingDirectChannels, teammateNameDisplay, locale); const unreadChannels = [...buildChannelsWithMentions(channels, myMembers, locale), ...buildUnreadChannels(channels, myMembers, locale)]; - const notUnreadChannels = channels.filter((channel: Channel) => !isUnreadChannel(myMembers, channel, collapsedThreadsEnabled)); + + // collapsedThreadsEnabled is set to "false" as we are filtering not just based on root posts + const notUnreadChannels = channels.filter((channel: Channel) => !isUnreadChannel(myMembers, channel, false)); + const favoriteChannels = buildFavoriteChannels(notUnreadChannels, myPreferences, locale); const notFavoriteChannels = buildNotFavoriteChannels(notUnreadChannels, myPreferences); const directAndGroupChannels = buildDirectAndGroupChannels(notFavoriteChannels, myMembers, config, myPreferences, currentUserId, profiles, lastPosts, collapsedThreadsEnabled); diff --git a/app/screens/post_options/post_options.js b/app/screens/post_options/post_options.js index bbdf16043..ff6e88265 100644 --- a/app/screens/post_options/post_options.js +++ b/app/screens/post_options/post_options.js @@ -112,13 +112,13 @@ export default class PostOptions extends PureComponent { getFollowThreadOption = () => { const {location, thread} = this.props; - if (location !== CHANNEL) { + if (location !== CHANNEL || !thread) { return null; } const key = 'follow'; let icon; let message; - if (thread?.is_following) { + if (thread.is_following) { icon = 'message-minus-outline'; if (thread?.participants?.length) { message = {id: t('threads.unfollowThread'), defaultMessage: 'Unfollow Thread'}; diff --git a/app/screens/thread/thread_base.js b/app/screens/thread/thread_base.js index 0b5b9556a..34e930864 100644 --- a/app/screens/thread/thread_base.js +++ b/app/screens/thread/thread_base.js @@ -52,6 +52,9 @@ export default class ThreadBase extends PureComponent { const options = {}; if (props.collapsedThreadsEnabled) { + // Without unique id, it breaks navigation from permalink view. + this.threadFollowId = Math.floor(Math.random() * 0x10000000000).toString(16); + let titleText; if (channelType === General.DM_CHANNEL) { titleText = formatMessage({id: 'mobile.routes.thread_dm', defaultMessage: 'Direct Message Thread'}); @@ -67,9 +70,9 @@ export default class ThreadBase extends PureComponent { }, rightButtons: [ { - id: '1', + id: 1, component: { - id: 'ThreadFollow', + id: this.threadFollowId, name: 'ThreadFollow', passProps: { active: thread?.is_following, @@ -129,7 +132,7 @@ export default class ThreadBase extends PureComponent { } if (this.props.thread?.is_following !== nextProps.thread?.is_following) { - Navigation.updateProps('ThreadFollow', {active: nextProps.thread?.is_following}); + Navigation.updateProps(this.threadFollowId, {active: nextProps.thread?.is_following}); } } @@ -148,6 +151,7 @@ export default class ThreadBase extends PureComponent { if ( this.props.collapsedThreadsEnabled && this.props.thread && + this.props.thread.is_following && ( hasNewPost || this.props.thread.last_viewed_at < this.props.thread.last_reply_at || diff --git a/app/store/middlewares/helpers.js b/app/store/middlewares/helpers.js index a669e2ce1..9b40f3f14 100644 --- a/app/store/middlewares/helpers.js +++ b/app/store/middlewares/helpers.js @@ -1,6 +1,7 @@ // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. // See LICENSE.txt for license information. +import {Posts} from '@mm-redux/constants'; import {isCollapsedThreadsEnabled} from '@mm-redux/selectors/entities/preferences'; export function getLastChannelForTeam(payload) { @@ -104,7 +105,7 @@ export function cleanUpState(payload, keepCurrent = false) { // Keep the last 60 threads in each team nextEntities.threads = { ...nextEntities.threads, - ...cleanUpThreadsInTeam(payload.entities.threads?.threads, payload.entities.threads?.threadsInTeam, keepCurrent ? lastTeamId : ''), + ...cleanUpThreadsInTeam(payload.entities.posts?.posts, payload.entities.threads?.threads, payload.entities.threads?.threadsInTeam, keepCurrent ? lastTeamId : ''), }; postIdsToKeep.push(...getAllFromThreadsInTeam(nextEntities.threads?.threadsInTeam)); @@ -274,7 +275,7 @@ export function cleanUpPostsInChannel(postsInChannel, lastChannelForTeam, curren return nextPostsInChannel; } -export function cleanUpThreadsInTeam(threads, threadsInTeam, currentTeamId, threadsCountPerTeam = 60) { +export function cleanUpThreadsInTeam(posts, threads, threadsInTeam, currentTeamId, threadsCountPerTeam = 60) { const newThreads = {}; const newThreadsInTeam = {}; if (threads && threadsInTeam) { @@ -292,7 +293,8 @@ export function cleanUpThreadsInTeam(threads, threadsInTeam, currentTeamId, thre newThreadsInTeam[teamId] = []; const retainedThreads = currentTeamId === teamId ? mappedThreads : mappedThreads.slice(0, threadsCountPerTeam); retainedThreads.forEach((thread) => { - if (thread) { + const post = posts?.[thread.id]; + if (post && post.state !== Posts.POST_DELETED) { newThreadsInTeam[teamId].push(thread.id); newThreads[thread.id] = thread; } diff --git a/app/store/middlewares/middleware.test.js b/app/store/middlewares/middleware.test.js index 6dceef01a..148d97e9a 100644 --- a/app/store/middlewares/middleware.test.js +++ b/app/store/middlewares/middleware.test.js @@ -10,6 +10,7 @@ import DeviceInfo from 'react-native-device-info'; import {REHYDRATE} from 'redux-persist'; import {ViewTypes} from '@constants'; +import {Posts} from '@mm-redux/constants'; import initialState from '@store/initial_state'; import { @@ -734,6 +735,16 @@ describe('cleanUpThreadsInTeam', () => { team1: ['thread1', 'thread2', 'thread3'], team2: ['thread3', 'thread4', 'thread5'], team3: ['thread1', 'thread2', 'thread3', 'thread4'], + team4: ['thread7'], + }; + const posts = { + thread1: {}, + thread2: {}, + thread3: {}, + thread4: {}, + thread5: {}, + thread6: {}, + thread7: {state: Posts.POST_DELETED}, }; const threads = { thread1: {id: 'thread1', last_reply_at: 100}, @@ -742,9 +753,10 @@ describe('cleanUpThreadsInTeam', () => { thread4: {id: 'thread4', last_reply_at: 97}, thread5: {id: 'thread5', last_reply_at: 96}, thread6: {id: 'thread6', last_reply_at: 95}, + thread7: {id: 'thread7', last_reply_at: 100}, }; - const {threads: nextThreads, threadsInTeam: nextThreadsInTeam} = cleanUpThreadsInTeam(threads, threadsInTeam, 'team3', 2); + const {threads: nextThreads, threadsInTeam: nextThreadsInTeam} = cleanUpThreadsInTeam(posts, threads, threadsInTeam, 'team3', 2); test('should only keep limited threads per team', () => { expect(nextThreadsInTeam.team1).toEqual(['thread1', 'thread2']); @@ -762,6 +774,10 @@ describe('cleanUpThreadsInTeam', () => { test('Should exclude passed teamId', () => { expect(nextThreadsInTeam.team3).toEqual(threadsInTeam.team3); }); + + test('Should not include deleted posts', () => { + expect(nextThreadsInTeam.team4).toEqual([]); + }); }); describe('getAllFromPostsInChannel', () => {