From ee6457c3dfaa903d979e2fcc4bf33113f07da1fb Mon Sep 17 00:00:00 2001 From: Ben Cooke Date: Wed, 8 Nov 2023 17:23:32 -0500 Subject: [PATCH] [MM-55009] Remove pre-fetch preference and set new pre fetch limits (#25251) * remove pre-fetch preference and set new pre fetch limits * i18n * adding unit test --------- Co-authored-by: Mattermost Build --- .../components/data_prefetch/actions.test.tsx | 59 ++++++++++ .../src/components/data_prefetch/actions.ts | 71 ++++++++++++ .../data_prefetch/data_prefetch.test.tsx | 2 - .../data_prefetch/data_prefetch.tsx | 13 +-- .../src/components/data_prefetch/index.ts | 57 +--------- .../user_settings/advanced/index.ts | 5 +- .../advanced/user_settings_advanced.test.tsx | 2 - .../advanced/user_settings_advanced.tsx | 102 ------------------ webapp/channels/src/i18n/en.json | 2 - .../src/constants/preferences.ts | 1 - webapp/channels/src/utils/constants.tsx | 1 - 11 files changed, 136 insertions(+), 179 deletions(-) create mode 100644 webapp/channels/src/components/data_prefetch/actions.test.tsx diff --git a/webapp/channels/src/components/data_prefetch/actions.test.tsx b/webapp/channels/src/components/data_prefetch/actions.test.tsx new file mode 100644 index 0000000000..f7d38e9b13 --- /dev/null +++ b/webapp/channels/src/components/data_prefetch/actions.test.tsx @@ -0,0 +1,59 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import type {Channel, ChannelMembership} from '@mattermost/types/channels'; +import type {RelationOneToOne} from '@mattermost/types/utilities'; + +import {TestHelper} from 'utils/test_helper'; + +import {prefetchQueue} from './actions'; + +describe('DataPrefetchActions', () => { + const unreadChannels: Channel[] = []; + const channelMemberships: RelationOneToOne = {}; + for (let i = 0; i < 25; i++) { + unreadChannels.push(TestHelper.getChannelMock({id: 'channel' + i})); + channelMemberships['channel' + i] = TestHelper.getChannelMembershipMock({channel_id: 'channel' + i}); + } + + const mentionChannels: Channel[] = []; + for (let i = 25; i < 50; i++) { + mentionChannels.push(TestHelper.getChannelMock({id: 'channel' + i})); + channelMemberships['channel' + i] = TestHelper.getChannelMembershipMock({channel_id: 'channel' + i, mention_count_root: 1}); + } + + it('prefetchQueue', () => { + // Unread channels only + expect(prefetchQueue(unreadChannels, channelMemberships, true)).toEqual({1: [], 2: [], 3: []}); + + const unreadChannels9 = unreadChannels.slice(0, 9); + let unreadQueue = prefetchQueue(unreadChannels9, channelMemberships, true); + expect(unreadQueue['1'].length).toBe(0); + expect(unreadQueue['2'].length).toBe(9); + + const unreadChannels10 = unreadChannels.slice(0, 10); + unreadQueue = prefetchQueue(unreadChannels10, channelMemberships, true); + expect(unreadQueue['1'].length).toBe(0); + expect(unreadQueue['2'].length).toBe(0); + + // Mention channels only + expect(prefetchQueue(mentionChannels, channelMemberships, true)).toEqual({1: [], 2: [], 3: []}); + + const mentionChannels9 = mentionChannels.slice(0, 9); + let mentionQueue = prefetchQueue(mentionChannels9, channelMemberships, true); + expect(mentionQueue['1'].length).toBe(9); + expect(unreadQueue['2'].length).toBe(0); + + const mentionChannels10 = mentionChannels.slice(0, 10); + mentionQueue = prefetchQueue(mentionChannels10, channelMemberships, true); + expect(mentionQueue['1'].length).toBe(10); + expect(unreadQueue['2'].length).toBe(0); + + // Mixing unread and mention channels + expect(prefetchQueue([...unreadChannels, ...mentionChannels], channelMemberships, true)).toEqual({1: [], 2: [], 3: []}); + + const mixedQueue = prefetchQueue([...unreadChannels9, ...mentionChannels10], channelMemberships, true); + expect(mixedQueue['1'].length).toBe(10); + expect(mixedQueue['2'].length).toBe(0); + }); +}); diff --git a/webapp/channels/src/components/data_prefetch/actions.ts b/webapp/channels/src/components/data_prefetch/actions.ts index 81fcc511dd..a1bbe678e1 100644 --- a/webapp/channels/src/components/data_prefetch/actions.ts +++ b/webapp/channels/src/components/data_prefetch/actions.ts @@ -3,7 +3,12 @@ import type {Dispatch} from 'redux'; +import type {Channel, ChannelMembership} from '@mattermost/types/channels'; +import type {RelationOneToOne} from '@mattermost/types/utilities'; + import {getChannelIdsForCurrentTeam} from 'mattermost-redux/selectors/entities/channels'; +import {isChannelMuted} from 'mattermost-redux/utils/channel_utils'; +import {memoizeResult} from 'mattermost-redux/utils/helpers'; import {trackEvent} from 'actions/telemetry_actions'; @@ -31,3 +36,69 @@ export function trackPreloadedChannels(prefetchQueueObj: Record20 unread. Don't prefetch anything. +export const prefetchQueue = memoizeResult(( + unreadChannels: Channel[], + memberships: RelationOneToOne, + collapsedThreads: boolean, +) => { + const unreadChannelsCount = unreadChannels.length; + let result: { + 1: string[]; + 2: string[]; + 3: string[]; + } = { + [Priority.high]: [], // 1 being high priority requests + [Priority.medium]: [], + [Priority.low]: [], //TODO: add chanenls such as fav. + }; + if (!unreadChannelsCount || unreadChannelsCount > PrefetchLimits.unreadMax) { + return result; + } + for (const channel of unreadChannels) { + const channelId = channel.id; + const membership = memberships[channelId]; + + if (unreadChannelsCount >= PrefetchLimits.mentionMax && result[Priority.high].length >= PrefetchLimits.mentionMax) { + break; + } + + // TODO We check for muted channels 3 times here: getUnreadChannels checks it, this checks it, and the mark_unread + // check below is equivalent to checking if its muted. + if (membership && !isChannelMuted(membership)) { + if (collapsedThreads ? membership.mention_count_root : membership.mention_count) { + result = { + ...result, + [Priority.high]: [...result[Priority.high], channelId], + }; + } else if ( + membership.notify_props && + membership.notify_props.mark_unread !== 'mention' && + unreadChannelsCount < PrefetchLimits.mentionMax + ) { + result = { + ...result, + [Priority.medium]: [...result[Priority.medium], channelId], + }; + } + } + } + return result; +}); diff --git a/webapp/channels/src/components/data_prefetch/data_prefetch.test.tsx b/webapp/channels/src/components/data_prefetch/data_prefetch.test.tsx index 04100ed248..222a62e189 100644 --- a/webapp/channels/src/components/data_prefetch/data_prefetch.test.tsx +++ b/webapp/channels/src/components/data_prefetch/data_prefetch.test.tsx @@ -68,8 +68,6 @@ describe('/components/data_prefetch', () => { last_post_at: 1235, last_root_post_at: 1235, })], - disableWebappPrefetchAllowed: false, - dataPrefetchEnabled: true, }; beforeEach(() => { diff --git a/webapp/channels/src/components/data_prefetch/data_prefetch.tsx b/webapp/channels/src/components/data_prefetch/data_prefetch.tsx index fe6e064178..d09959d1e0 100644 --- a/webapp/channels/src/components/data_prefetch/data_prefetch.tsx +++ b/webapp/channels/src/components/data_prefetch/data_prefetch.tsx @@ -22,8 +22,6 @@ type Props = { unreadChannels: Channel[]; - disableWebappPrefetchAllowed: boolean; - dataPrefetchEnabled: boolean; actions: { prefetchChannelPosts: (channelId: string, delay?: number) => Promise; trackPreloadedChannels: (prefetchQueueObj: Record) => void; @@ -55,20 +53,15 @@ export default class DataPrefetch extends React.PureComponent { private prefetchTimeout?: number; async componentDidUpdate(prevProps: Props) { - const {currentChannelId, prefetchQueueObj, sidebarLoaded, disableWebappPrefetchAllowed, dataPrefetchEnabled} = this.props; - const enablePrefetch = (!disableWebappPrefetchAllowed) || (disableWebappPrefetchAllowed && dataPrefetchEnabled); + const {currentChannelId, prefetchQueueObj, sidebarLoaded} = this.props; if (currentChannelId && sidebarLoaded && (!prevProps.currentChannelId || !prevProps.sidebarLoaded)) { queue.add(async () => this.prefetchPosts(currentChannelId)); await loadProfilesForSidebar(); - if (enablePrefetch) { - this.prefetchData(); - } + this.prefetchData(); } else if (prevProps.prefetchQueueObj !== prefetchQueueObj) { clearTimeout(this.prefetchTimeout); await queue.clear(); - if (enablePrefetch) { - this.prefetchData(); - } + this.prefetchData(); } if (currentChannelId && sidebarLoaded && (!prevProps.currentChannelId || !prevProps.sidebarLoaded)) { diff --git a/webapp/channels/src/components/data_prefetch/index.ts b/webapp/channels/src/components/data_prefetch/index.ts index e8fc8ad9f0..fa5bac9aca 100644 --- a/webapp/channels/src/components/data_prefetch/index.ts +++ b/webapp/channels/src/components/data_prefetch/index.ts @@ -5,24 +5,18 @@ import {connect} from 'react-redux'; import {bindActionCreators} from 'redux'; import type {ActionCreatorsMapObject, Dispatch} from 'redux'; -import type {Channel, ChannelMembership} from '@mattermost/types/channels'; import type {PostList} from '@mattermost/types/posts'; -import type {RelationOneToOne} from '@mattermost/types/utilities'; -import {Preferences} from 'mattermost-redux/constants'; import {getCurrentChannelId, getUnreadChannels} from 'mattermost-redux/selectors/entities/channels'; import {getMyChannelMemberships} from 'mattermost-redux/selectors/entities/common'; -import {isPerformanceDebuggingEnabled} from 'mattermost-redux/selectors/entities/general'; -import {getBool, isCollapsedThreadsEnabled} from 'mattermost-redux/selectors/entities/preferences'; -import {isChannelMuted} from 'mattermost-redux/utils/channel_utils'; -import {memoizeResult} from 'mattermost-redux/utils/helpers'; +import {isCollapsedThreadsEnabled} from 'mattermost-redux/selectors/entities/preferences'; import {prefetchChannelPosts} from 'actions/views/channel'; import {getCategoriesForCurrentTeam} from 'selectors/views/channel_sidebar'; import type {GlobalState} from 'types/store'; -import {trackPreloadedChannels} from './actions'; +import {prefetchQueue, trackPreloadedChannels} from './actions'; import DataPrefetch from './data_prefetch'; type Actions = { @@ -30,50 +24,6 @@ type Actions = { trackPreloadedChannels: (prefetchQueueObj: Record) => void; }; -enum Priority { - high = 1, - medium, - low -} - -// function to return a queue obj with priotiy as key and array of channelIds as values. -// high priority has channels with mentions -// medium priority has channels with unreads -const prefetchQueue = memoizeResult(( - unreadChannels: Channel[], - memberships: RelationOneToOne, - collapsedThreads: boolean, -) => { - return unreadChannels.reduce((acc: Record, channel: Channel) => { - const channelId = channel.id; - const membership = memberships[channelId]; - - // TODO We check for muted channels 3 times here: getUnreadChannels checks it, this checks it, and the mark_unread - // check below is equivalent to checking if its muted. - if (membership && !isChannelMuted(membership)) { - if (collapsedThreads ? membership.mention_count_root : membership.mention_count) { - return { - ...acc, - [Priority.high]: [...acc[Priority.high], channelId], - }; - } else if ( - membership.notify_props && - membership.notify_props.mark_unread !== 'mention' - ) { - return { - ...acc, - [Priority.medium]: [...acc[Priority.medium], channelId], - }; - } - } - return acc; - }, { - [Priority.high]: [], // 1 being high priority requests - [Priority.medium]: [], - [Priority.low]: [], //TODO: add chanenls such as fav. - }); -}); - function isSidebarLoaded(state: GlobalState) { return getCategoriesForCurrentTeam(state).length > 0; } @@ -84,7 +34,6 @@ function mapStateToProps(state: GlobalState) { const unreadChannels = getUnreadChannels(state, lastUnreadChannel); const prefetchQueueObj = prefetchQueue(unreadChannels, memberships, isCollapsedThreadsEnabled(state)); const prefetchRequestStatus = state.views.channel.channelPrefetchStatus; - const disableWebappPrefetchAllowed = isPerformanceDebuggingEnabled(state); return { currentChannelId: getCurrentChannelId(state), @@ -92,8 +41,6 @@ function mapStateToProps(state: GlobalState) { prefetchRequestStatus, sidebarLoaded: isSidebarLoaded(state), unreadChannels, - disableWebappPrefetchAllowed, - dataPrefetchEnabled: getBool(state, Preferences.CATEGORY_ADVANCED_SETTINGS, Preferences.ADVANCED_DATA_PREFETCH, true), }; } diff --git a/webapp/channels/src/components/user_settings/advanced/index.ts b/webapp/channels/src/components/user_settings/advanced/index.ts index 49bf659e9b..4ea3a91b8d 100644 --- a/webapp/channels/src/components/user_settings/advanced/index.ts +++ b/webapp/channels/src/components/user_settings/advanced/index.ts @@ -7,7 +7,7 @@ import type {ActionCreatorsMapObject, Dispatch} from 'redux'; import {savePreferences} from 'mattermost-redux/actions/preferences'; import {updateUserActive, revokeAllSessionsForUser} from 'mattermost-redux/actions/users'; -import {getConfig, isPerformanceDebuggingEnabled} from 'mattermost-redux/selectors/entities/general'; +import {getConfig} from 'mattermost-redux/selectors/entities/general'; import {get, getUnreadScrollPositionPreference, makeGetCategory, syncedDraftsAreAllowed} from 'mattermost-redux/selectors/entities/preferences'; import {getCurrentUser} from 'mattermost-redux/selectors/entities/users'; import type {ActionFunc} from 'mattermost-redux/types/actions'; @@ -27,7 +27,6 @@ function makeMapStateToProps() { const enablePreviewFeatures = config.EnablePreviewFeatures === 'true'; const enableUserDeactivation = config.EnableUserDeactivation === 'true'; - const disableWebappPrefetchAllowed = isPerformanceDebuggingEnabled(state); const enableJoinLeaveMessage = config.EnableJoinLeaveMessageByDefault === 'true'; return { @@ -42,8 +41,6 @@ function makeMapStateToProps() { enablePreviewFeatures, enableUserDeactivation, syncedDraftsAreAllowed: syncedDraftsAreAllowed(state), - disableWebappPrefetchAllowed, - dataPrefetchEnabled: get(state, Preferences.CATEGORY_ADVANCED_SETTINGS, 'data_prefetch', 'true'), }; }; } diff --git a/webapp/channels/src/components/user_settings/advanced/user_settings_advanced.test.tsx b/webapp/channels/src/components/user_settings/advanced/user_settings_advanced.test.tsx index 79dbec275f..945e60b65a 100644 --- a/webapp/channels/src/components/user_settings/advanced/user_settings_advanced.test.tsx +++ b/webapp/channels/src/components/user_settings/advanced/user_settings_advanced.test.tsx @@ -47,8 +47,6 @@ describe('components/user_settings/display/UserSettingsDisplay', () => { enablePreviewFeatures: false, enableUserDeactivation: false, syncedDraftsAreAllowed: true, - disableWebappPrefetchAllowed: false, - dataPrefetchEnabled: 'true', }; test('should have called handleSubmit', async () => { diff --git a/webapp/channels/src/components/user_settings/advanced/user_settings_advanced.tsx b/webapp/channels/src/components/user_settings/advanced/user_settings_advanced.tsx index f80749c494..91539d0172 100644 --- a/webapp/channels/src/components/user_settings/advanced/user_settings_advanced.tsx +++ b/webapp/channels/src/components/user_settings/advanced/user_settings_advanced.tsx @@ -36,7 +36,6 @@ type Settings = { formatting: Props['formatting']; join_leave: Props['joinLeave']; sync_drafts: Props['syncDrafts']; - data_prefetch: Props['dataPrefetchEnabled']; }; export type Props = { @@ -55,8 +54,6 @@ export type Props = { enablePreviewFeatures: boolean; enableUserDeactivation: boolean; syncedDraftsAreAllowed: boolean; - disableWebappPrefetchAllowed: boolean; - dataPrefetchEnabled: string; actions: { savePreferences: (userId: string, preferences: PreferenceType[]) => Promise; updateUserActive: (userId: string, active: boolean) => Promise; @@ -90,7 +87,6 @@ export default class AdvancedSettingsDisplay extends React.PureComponent { - const active = this.props.activeSection === AdvancedSections.DATA_PREFETCH; - let max = null; - if (active) { - max = ( - - } - inputs={[ -
- - - -
- -
-
-
- -
-
-
- -
-
, - ]} - setting={AdvancedSections.DATA_PREFETCH} - submit={this.handleSubmit.bind(this, ['data_prefetch'])} - saving={this.state.isSaving} - serverError={this.state.serverError} - updateSection={this.handleUpdateSection} - /> - ); - } - - return ( - - } - describe={this.renderOnOffLabel(this.state.settings.data_prefetch)} - section={AdvancedSections.DATA_PREFETCH} - updateSection={this.handleUpdateSection} - max={max} - /> - ); - }; - renderFeatureLabel(feature: string): ReactNode { switch (feature) { case 'MARKDOWN_PREVIEW': @@ -986,15 +895,6 @@ export default class AdvancedSettingsDisplay extends React.PureComponent; - } - } - return (
@@ -1053,8 +953,6 @@ export default class AdvancedSettingsDisplay extends React.PureComponent {makeConfirmationModal}
diff --git a/webapp/channels/src/i18n/en.json b/webapp/channels/src/i18n/en.json index 75f2c7ac1c..cc9920a467 100644 --- a/webapp/channels/src/i18n/en.json +++ b/webapp/channels/src/i18n/en.json @@ -5220,8 +5220,6 @@ "user_profile.send.dm.yourself": "Send yourself a message", "user.settings.advance.confirmDeactivateAccountTitle": "Confirm Deactivation", "user.settings.advance.confirmDeactivateDesc": "Are you sure you want to deactivate your account? This can only be reversed by your System Administrator.", - "user.settings.advance.dataPrefetch.Desc": "When disabled, messages and user information will be fetched on each channel load instead of being pre-fetched on startup. Disabling prefetch is recommended for users with a high unread channel count in order to improve application performance.", - "user.settings.advance.dataPrefetch.Title": "Allow Mattermost to prefetch channel posts", "user.settings.advance.deactivate_member_modal.deactivateButton": "Yes, deactivate my account", "user.settings.advance.deactivateAccountTitle": "Deactivate Account", "user.settings.advance.deactivateDesc": "Deactivating your account removes your ability to log in to this server and disables all email and mobile notifications. To reactivate your account, contact your System Administrator.", diff --git a/webapp/channels/src/packages/mattermost-redux/src/constants/preferences.ts b/webapp/channels/src/packages/mattermost-redux/src/constants/preferences.ts index d3f3e52597..d4efe1e522 100644 --- a/webapp/channels/src/packages/mattermost-redux/src/constants/preferences.ts +++ b/webapp/channels/src/packages/mattermost-redux/src/constants/preferences.ts @@ -55,7 +55,6 @@ const Preferences = { ADVANCED_CODE_BLOCK_ON_CTRL_ENTER: 'code_block_ctrl_enter', ADVANCED_SEND_ON_CTRL_ENTER: 'send_on_ctrl_enter', ADVANCED_SYNC_DRAFTS: 'sync_drafts', - ADVANCED_DATA_PREFETCH: 'data_prefetch', CATEGORY_WHATS_NEW_MODAL: 'whats_new_modal', HAS_SEEN_SIDEBAR_WHATS_NEW_MODAL: 'has_seen_sidebar_whats_new_modal', diff --git a/webapp/channels/src/utils/constants.tsx b/webapp/channels/src/utils/constants.tsx index fb2560d066..3c2c4f8557 100644 --- a/webapp/channels/src/utils/constants.tsx +++ b/webapp/channels/src/utils/constants.tsx @@ -1014,7 +1014,6 @@ export const AdvancedSections = { PREVIEW_FEATURES: 'advancedPreviewFeatures', PERFORMANCE_DEBUGGING: 'performanceDebugging', SYNC_DRAFTS: 'syncDrafts', - DATA_PREFETCH: 'dataPrefetch', }; export const RHSStates = {