From 3ba75afa08911f50642b24e9091e70670f41d4dc Mon Sep 17 00:00:00 2001 From: Harrison Healey Date: Tue, 2 May 2023 10:11:15 -0400 Subject: [PATCH] MM-52173 Add setting to delay channel autocomplete (#22952) * Add unit tests for ChannelMentionProvider.handleCompleteWord * Add a minimum length before the ChannelMentionProvider triggers * MM-52173 Move delayed autcomplete behind a setting * Fix e2e test type check * Update Textbox tests * Add new setting to server telemetry --- .../support/server/default_config.ts | 1 + server/config/client.go | 1 + server/model/config.go | 5 + .../platform/services/telemetry/telemetry.go | 1 + .../__snapshots__/textbox.test.tsx.snap | 4 + .../admin_console/admin_definition.jsx | 9 + .../channel_mention_provider.test.tsx | 214 ++++++++++++++++++ .../suggestion/channel_mention_provider.tsx | 21 +- .../channels/src/components/textbox.test.tsx | 1 + .../channels/src/components/textbox/index.ts | 3 +- .../src/components/textbox/textbox.tsx | 13 +- webapp/channels/src/i18n/en.json | 2 + webapp/platform/types/src/config.ts | 2 + 13 files changed, 271 insertions(+), 6 deletions(-) create mode 100644 webapp/channels/src/components/suggestion/channel_mention_provider.test.tsx diff --git a/e2e-tests/playwright/support/server/default_config.ts b/e2e-tests/playwright/support/server/default_config.ts index c62f808707..4bcfdd5798 100644 --- a/e2e-tests/playwright/support/server/default_config.ts +++ b/e2e-tests/playwright/support/server/default_config.ts @@ -532,6 +532,7 @@ const defaultServerConfig: AdminConfig = { EnableAppBar: false, PatchPluginsReactDOM: false, DisableRefetchingOnBrowserFocus: false, + DelayChannelAutocomplete: false, }, AnalyticsSettings: { MaxUsersForStatistics: 2500, diff --git a/server/config/client.go b/server/config/client.go index 0de8694999..a4541dd32e 100644 --- a/server/config/client.go +++ b/server/config/client.go @@ -136,6 +136,7 @@ func GenerateClientConfig(c *model.Config, telemetryID string, license *model.Li props["InsightsEnabled"] = strconv.FormatBool(c.FeatureFlags.InsightsEnabled) props["PostPriority"] = strconv.FormatBool(*c.ServiceSettings.PostPriority) props["AllowSyncedDrafts"] = strconv.FormatBool(*c.ServiceSettings.AllowSyncedDrafts) + props["DelayChannelAutocomplete"] = strconv.FormatBool(*c.ExperimentalSettings.DelayChannelAutocomplete) if license != nil { props["ExperimentalEnableAuthenticationTransfer"] = strconv.FormatBool(*c.ServiceSettings.ExperimentalEnableAuthenticationTransfer) diff --git a/server/model/config.go b/server/model/config.go index 262b8548ba..7bd69d00db 100644 --- a/server/model/config.go +++ b/server/model/config.go @@ -973,6 +973,7 @@ type ExperimentalSettings struct { EnableAppBar *bool `access:"experimental_features"` PatchPluginsReactDOM *bool `access:"experimental_features"` DisableRefetchingOnBrowserFocus *bool `access:"experimental_features"` + DelayChannelAutocomplete *bool `access:"experimental_features"` } func (s *ExperimentalSettings) SetDefaults() { @@ -1015,6 +1016,10 @@ func (s *ExperimentalSettings) SetDefaults() { if s.DisableRefetchingOnBrowserFocus == nil { s.DisableRefetchingOnBrowserFocus = NewBool(false) } + + if s.DelayChannelAutocomplete == nil { + s.DelayChannelAutocomplete = NewBool(false) + } } type AnalyticsSettings struct { diff --git a/server/platform/services/telemetry/telemetry.go b/server/platform/services/telemetry/telemetry.go index 31ea427505..6982904d15 100644 --- a/server/platform/services/telemetry/telemetry.go +++ b/server/platform/services/telemetry/telemetry.go @@ -760,6 +760,7 @@ func (ts *TelemetryService) trackConfig() { "enable_app_bar": *cfg.ExperimentalSettings.EnableAppBar, "patch_plugins_react_dom": *cfg.ExperimentalSettings.PatchPluginsReactDOM, "disable_refetching_on_browser_focus": *cfg.ExperimentalSettings.DisableRefetchingOnBrowserFocus, + "delay_channel_autocomplete": *cfg.ExperimentalSettings.DelayChannelAutocomplete, }) ts.SendTelemetry(TrackConfigAnalytics, map[string]any{ diff --git a/webapp/channels/src/components/__snapshots__/textbox.test.tsx.snap b/webapp/channels/src/components/__snapshots__/textbox.test.tsx.snap index 9cc5396955..a039205a4c 100644 --- a/webapp/channels/src/components/__snapshots__/textbox.test.tsx.snap +++ b/webapp/channels/src/components/__snapshots__/textbox.test.tsx.snap @@ -62,6 +62,7 @@ exports[`components/TextBox should match snapshot with additional, optional prop }, ChannelMentionProvider { "autocompleteChannels": [MockFunction], + "delayChannelAutocomplete": false, "disableDispatches": false, "forceDispatch": false, "lastCompletedWord": "", @@ -174,6 +175,7 @@ exports[`components/TextBox should match snapshot with required props 1`] = ` }, ChannelMentionProvider { "autocompleteChannels": [MockFunction], + "delayChannelAutocomplete": false, "disableDispatches": false, "forceDispatch": false, "lastCompletedWord": "", @@ -267,6 +269,7 @@ exports[`components/TextBox should throw error when new property is too long 1`] }, ChannelMentionProvider { "autocompleteChannels": [MockFunction], + "delayChannelAutocomplete": false, "disableDispatches": false, "forceDispatch": false, "lastCompletedWord": "", @@ -360,6 +363,7 @@ exports[`components/TextBox should throw error when value is too long 1`] = ` }, ChannelMentionProvider { "autocompleteChannels": [MockFunction], + "delayChannelAutocomplete": false, "disableDispatches": false, "forceDispatch": false, "lastCompletedWord": "", diff --git a/webapp/channels/src/components/admin_console/admin_definition.jsx b/webapp/channels/src/components/admin_console/admin_definition.jsx index abab875ec2..2ce6a1c404 100644 --- a/webapp/channels/src/components/admin_console/admin_definition.jsx +++ b/webapp/channels/src/components/admin_console/admin_definition.jsx @@ -6896,6 +6896,15 @@ const AdminDefinition = { help_text_default: 'When true, Mattermost will not refetch channels and channel members when the browser regains focus. This may result in improved performance for users with many channels and channel members.', isDisabled: it.not(it.userHasWritePermissionOnResource(RESOURCE_KEYS.EXPERIMENTAL.FEATURES)), }, + { + type: Constants.SettingsTypes.TYPE_BOOL, + key: 'ExperimentalSettings.DelayChannelAutocomplete', + label: t('admin.experimental.delayChannelAutocomplete.title'), + label_default: 'Delay Channel Autocomplete:', + help_text: t('admin.experimental.delayChannelAutocomplete.desc'), + help_text_default: 'When true, the autocomplete for channel links (such as ~town-square) will only trigger after typing a tilde followed by a couple letters. When false, the autocomplete will appear as soon as the user types a tilde.', + isDisabled: it.not(it.userHasWritePermissionOnResource(RESOURCE_KEYS.EXPERIMENTAL.FEATURES)), + }, ], }, }, diff --git a/webapp/channels/src/components/suggestion/channel_mention_provider.test.tsx b/webapp/channels/src/components/suggestion/channel_mention_provider.test.tsx new file mode 100644 index 0000000000..83f31582b0 --- /dev/null +++ b/webapp/channels/src/components/suggestion/channel_mention_provider.test.tsx @@ -0,0 +1,214 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +jest.mock('mattermost-redux/selectors/entities/channels', () => ({ + getMyChannels: jest.fn(() => []), + getMyChannelMemberships: jest.fn(() => {}), +})); + +jest.mock('stores/redux_store'); + +import ChannelMentionProvider from './channel_mention_provider'; + +describe('ChannelMentionProvider.handlePretextChanged', () => { + const autocompleteChannels = jest.fn(); + const resultsCallback = jest.fn(); + + let provider: ChannelMentionProvider; + beforeEach(() => { + provider = new ChannelMentionProvider(autocompleteChannels, false); + }); + + describe('basic cases', () => { + test('should not match empty string', () => { + const matched = provider.handlePretextChanged('', resultsCallback); + + expect(matched).toBe(false); + expect(autocompleteChannels).not.toHaveBeenCalled(); + expect(resultsCallback).not.toHaveBeenCalled(); + }); + + test('should not match plain text', () => { + const matched = provider.handlePretextChanged('this is a test', resultsCallback); + + expect(matched).toBe(false); + expect(autocompleteChannels).not.toHaveBeenCalled(); + expect(resultsCallback).not.toHaveBeenCalled(); + }); + + test('should match a partial channel\'s name', () => { + const matched = provider.handlePretextChanged('~town-sq', resultsCallback); + + expect(matched).toBe(true); + expect(autocompleteChannels).toHaveBeenCalledWith('town-sq', expect.anything(), expect.anything()); + expect(resultsCallback).toHaveBeenCalled(); + }); + + test('should match a channel\'s name', () => { + const matched = provider.handlePretextChanged('~town-square', resultsCallback); + + expect(matched).toBe(true); + expect(autocompleteChannels).toHaveBeenCalledWith('town-square', expect.anything(), expect.anything()); + expect(resultsCallback).toHaveBeenCalled(); + }); + + test('should match a channel\'s partial display name', () => { + const matched = provider.handlePretextChanged('~Town Sq', resultsCallback); + + expect(matched).toBe(true); + expect(autocompleteChannels).toHaveBeenCalledWith('town sq', expect.anything(), expect.anything()); + expect(resultsCallback).toHaveBeenCalled(); + }); + + test('should match a channel\'s display name', () => { + const matched = provider.handlePretextChanged('~Town Square', resultsCallback); + + expect(matched).toBe(true); + expect(autocompleteChannels).toHaveBeenCalledWith('town square', expect.anything(), expect.anything()); + expect(resultsCallback).toHaveBeenCalled(); + }); + + test('should match part of the text', () => { + const matched = provider.handlePretextChanged('this is ~town-squ', resultsCallback); + + expect(matched).toBe(true); + expect(autocompleteChannels).toHaveBeenCalledWith('town-squ', expect.anything(), expect.anything()); + expect(resultsCallback).toHaveBeenCalled(); + }); + + test('should only match at the end of the text', () => { + const matched = provider.handlePretextChanged('this is ~town-square, not ~off-topic', resultsCallback); + + expect(matched).toBe(true); + expect(autocompleteChannels).toHaveBeenCalledWith('off-topic', expect.anything(), expect.anything()); + expect(resultsCallback).toHaveBeenCalled(); + }); + + test('should lower case search term', () => { + const matched = provider.handlePretextChanged('this is ~town SQUARE ', resultsCallback); + + expect(matched).toBe(true); + expect(autocompleteChannels).toHaveBeenCalledWith('town square ', expect.anything(), expect.anything()); + expect(resultsCallback).toHaveBeenCalled(); + }); + }); + + describe('strikethrough text', () => { + test('should not match the start of strikethrough text', () => { + const matched = provider.handlePretextChanged('~~', resultsCallback); + + expect(matched).toBe(false); + expect(autocompleteChannels).not.toHaveBeenCalled(); + expect(resultsCallback).not.toHaveBeenCalled(); + }); + + test('should not match the middle of strikethrough text', () => { + const matched = provider.handlePretextChanged('~~town square', resultsCallback); + + expect(matched).toBe(false); + expect(autocompleteChannels).not.toHaveBeenCalled(); + expect(resultsCallback).not.toHaveBeenCalled(); + }); + + test('should not match the end of strikethrough text', () => { + const matched = provider.handlePretextChanged('~~this is a test~~', resultsCallback); + + expect(matched).toBe(false); + expect(autocompleteChannels).not.toHaveBeenCalled(); + expect(resultsCallback).not.toHaveBeenCalled(); + }); + }); + + describe('matching text after completing a result', () => { + test('should not continue to match a link that was just completed', () => { + provider.handleCompleteWord('~town-square'); + + const matched = provider.handlePretextChanged('This is ~town-square', resultsCallback); + + expect(matched).toBe(false); + expect(autocompleteChannels).not.toHaveBeenCalled(); + expect(resultsCallback).not.toHaveBeenCalled(); + }); + + test('should not continue to match a link that was completed, even after typing more text', () => { + provider.handleCompleteWord('~town-square'); + + const matched = provider.handlePretextChanged('This is ~town-square and a test', resultsCallback); + + expect(matched).toBe(false); + expect(autocompleteChannels).not.toHaveBeenCalled(); + expect(resultsCallback).not.toHaveBeenCalled(); + }); + + test('should start matching input again after another link is started', () => { + provider.handleCompleteWord('~town-square'); + + const matched = provider.handlePretextChanged('This is ~town-square and not ~off', resultsCallback); + + expect(matched).toBe(true); + expect(autocompleteChannels).toHaveBeenCalledWith('off', expect.anything(), expect.anything()); + expect(resultsCallback).toHaveBeenCalled(); + }); + }); + + test('should not continue to match after receiving no results until another possible link starts', () => { + autocompleteChannels.mockImplementationOnce((prefix, success) => { + success([]); + }); + + let matched = provider.handlePretextChanged('This is ~no-results', resultsCallback); + + expect(matched).toBe(true); + expect(autocompleteChannels).toHaveBeenCalledWith('no-results', expect.anything(), expect.anything()); + expect(resultsCallback).toHaveBeenCalledTimes(2); + + autocompleteChannels.mockReset(); + resultsCallback.mockReset(); + + matched = provider.handlePretextChanged('This is ~no-results in a test', resultsCallback); + + expect(matched).toBe(false); + expect(autocompleteChannels).not.toHaveBeenCalled(); + expect(resultsCallback).not.toHaveBeenCalled(); + + matched = provider.handlePretextChanged('This is ~no-results in a test using ~town', resultsCallback); + + expect(matched).toBe(true); + expect(autocompleteChannels).toHaveBeenCalledWith('town', expect.anything(), expect.anything()); + expect(resultsCallback).toHaveBeenCalled(); + }); + + describe('delayed autocomplete', () => { + test('with the setting enabled, should not match a link shorter than the minimum length', () => { + provider.setProps({delayChannelAutocomplete: true}); + + let matched = provider.handlePretextChanged('~', resultsCallback); + + expect(matched).toBe(false); + expect(autocompleteChannels).not.toHaveBeenCalled(); + expect(resultsCallback).not.toHaveBeenCalled(); + + matched = provider.handlePretextChanged('~t', resultsCallback); + + expect(matched).toBe(false); + expect(autocompleteChannels).not.toHaveBeenCalled(); + expect(resultsCallback).not.toHaveBeenCalled(); + + matched = provider.handlePretextChanged('~to', resultsCallback); + + expect(matched).toBe(true); + expect(autocompleteChannels).toHaveBeenCalledWith('to', expect.anything(), expect.anything()); + expect(resultsCallback).toHaveBeenCalled(); + }); + + test('with the setting disabled, should match a link shorter than the minimum length', () => { + provider.setProps({delayChannelAutocomplete: false}); + + const matched = provider.handlePretextChanged('~', resultsCallback); + + expect(matched).toBe(true); + expect(autocompleteChannels).toHaveBeenCalledWith('', expect.anything(), expect.anything()); + expect(resultsCallback).toHaveBeenCalled(); + }); + }); +}); diff --git a/webapp/channels/src/components/suggestion/channel_mention_provider.tsx b/webapp/channels/src/components/suggestion/channel_mention_provider.tsx index 01c4b47335..445f8ccfc9 100644 --- a/webapp/channels/src/components/suggestion/channel_mention_provider.tsx +++ b/webapp/channels/src/components/suggestion/channel_mention_provider.tsx @@ -17,6 +17,8 @@ import {Constants} from 'utils/constants'; import Provider from './provider'; import Suggestion from './suggestion.jsx'; +export const MIN_CHANNEL_LINK_LENGTH = 2; + export type Results = { matchedPretext: string; terms: string[]; @@ -81,12 +83,14 @@ export class ChannelMentionSuggestion extends Suggestion { } export default class ChannelMentionProvider extends Provider { - lastPrefixTrimmed: string; - lastPrefixWithNoResults: string; - lastCompletedWord: string; + private lastPrefixTrimmed: string; + private lastPrefixWithNoResults: string; + private lastCompletedWord: string; triggerCharacter: string; + private delayChannelAutocomplete: boolean; autocompleteChannels: (term: string, success: (channels: Channel[]) => void, error: () => void) => Promise; - constructor(channelSearchFunc: (term: string, success: (channels: Channel[]) => void, error: () => void) => Promise) { + + constructor(channelSearchFunc: (term: string, success: (channels: Channel[]) => void, error: () => void) => Promise, delayChannelAutocomplete: boolean) { super(); this.lastPrefixTrimmed = ''; @@ -95,6 +99,11 @@ export default class ChannelMentionProvider extends Provider { this.triggerCharacter = '~'; this.autocompleteChannels = channelSearchFunc; + this.delayChannelAutocomplete = delayChannelAutocomplete; + } + + setProps(props: {delayChannelAutocomplete: boolean}) { + this.delayChannelAutocomplete = props.delayChannelAutocomplete; } handlePretextChanged(pretext: string, resultCallback: ResultsCallback) { @@ -114,6 +123,10 @@ export default class ChannelMentionProvider extends Provider { const prefix = captured[2]; + if (this.delayChannelAutocomplete && prefix.length < MIN_CHANNEL_LINK_LENGTH) { + return false; + } + if (this.lastPrefixTrimmed && prefix.trim() === this.lastPrefixTrimmed) { // Don't keep searching if the user keeps typing spaces return true; diff --git a/webapp/channels/src/components/textbox.test.tsx b/webapp/channels/src/components/textbox.test.tsx index eaaaac0b87..8f3d75e0e5 100644 --- a/webapp/channels/src/components/textbox.test.tsx +++ b/webapp/channels/src/components/textbox.test.tsx @@ -16,6 +16,7 @@ describe('components/TextBox', () => { {id: 'id1'}, {id: 'id2'}, ], + delayChannelAutocomplete: false, autocompleteGroups: [ {id: 'gid1'}, {id: 'gid2'}, diff --git a/webapp/channels/src/components/textbox/index.ts b/webapp/channels/src/components/textbox/index.ts index 95d657cb39..bb78620d5f 100644 --- a/webapp/channels/src/components/textbox/index.ts +++ b/webapp/channels/src/components/textbox/index.ts @@ -5,7 +5,7 @@ import {ActionCreatorsMapObject, bindActionCreators, Dispatch} from 'redux'; import {connect} from 'react-redux'; import {getAssociatedGroupsForReference} from 'mattermost-redux/selectors/entities/groups'; -import {getLicense} from 'mattermost-redux/selectors/entities/general'; +import {getConfig, getLicense} from 'mattermost-redux/selectors/entities/general'; import {getCurrentTeamId} from 'mattermost-redux/selectors/entities/teams'; import {makeGetProfilesForThread} from 'mattermost-redux/selectors/entities/posts'; @@ -47,6 +47,7 @@ const makeMapStateToProps = () => { currentTeamId: teamId, autocompleteGroups, priorityProfiles: getProfilesForThread(state, ownProps.rootId ?? ''), + delayChannelAutocomplete: getConfig(state).DelayChannelAutocomplete === 'true', }; }; }; diff --git a/webapp/channels/src/components/textbox/textbox.tsx b/webapp/channels/src/components/textbox/textbox.tsx index 8030eb5e88..8cf131c2c1 100644 --- a/webapp/channels/src/components/textbox/textbox.tsx +++ b/webapp/channels/src/components/textbox/textbox.tsx @@ -61,6 +61,7 @@ export type Props = { currentTeamId: string; preview?: boolean; autocompleteGroups: Array<{ id: string }> | null; + delayChannelAutocomplete: boolean; actions: { autocompleteUsersInChannel: (prefix: string, channelId: string) => Promise; autocompleteChannels: (term: string, success: (channels: Channel[]) => void, error: () => void) => Promise; @@ -113,7 +114,7 @@ export default class Textbox extends React.PureComponent { searchAssociatedGroupsForReference: (prefix: string) => this.props.actions.searchAssociatedGroupsForReference(prefix, this.props.currentTeamId, this.props.channelId), priorityProfiles: this.props.priorityProfiles, }), - new ChannelMentionProvider(props.actions.autocompleteChannels), + new ChannelMentionProvider(props.actions.autocompleteChannels, props.delayChannelAutocomplete), new EmoticonProvider(), ); @@ -180,6 +181,16 @@ export default class Textbox extends React.PureComponent { } } + if (this.props.delayChannelAutocomplete !== prevProps.delayChannelAutocomplete) { + for (const provider of this.suggestionProviders) { + if (provider instanceof ChannelMentionProvider) { + provider.setProps({ + delayChannelAutocomplete: this.props.delayChannelAutocomplete, + }); + } + } + } + if (prevProps.value !== this.props.value) { this.checkMessageLength(this.props.value); } diff --git a/webapp/channels/src/i18n/en.json b/webapp/channels/src/i18n/en.json index ace9ab4b66..914c3a12d3 100644 --- a/webapp/channels/src/i18n/en.json +++ b/webapp/channels/src/i18n/en.json @@ -895,6 +895,8 @@ "admin.experimental.collapsedThreads.title": "Collapsed Reply Threads", "admin.experimental.defaultTheme.desc": "Set a default theme that applies to all new users on the system.", "admin.experimental.defaultTheme.title": "Default Theme:", + "admin.experimental.delayChannelAutocomplete.desc": "When true, the autocomplete for channel links (such as ~town-square) will only trigger after typing a tilde followed by a couple letters. When false, the autocomplete will appear as soon as the user types a tilde.", + "admin.experimental.delayChannelAutocomplete.title": "Delay Channel Autocomplete:", "admin.experimental.disableRefetchingOnBrowserFocus.desc": "When true, Mattermost will not refetch channels and channel members when the browser regains focus. This may result in improved performance for users with many channels and channel members.", "admin.experimental.disableRefetchingOnBrowserFocus.title": "Disable data refetching on browser refocus:", "admin.experimental.emailBatchingBufferSize.desc": "Specify the maximum number of notifications batched into a single email.", diff --git a/webapp/platform/types/src/config.ts b/webapp/platform/types/src/config.ts index dfd5e21127..de54b12b14 100644 --- a/webapp/platform/types/src/config.ts +++ b/webapp/platform/types/src/config.ts @@ -196,6 +196,7 @@ export type ClientConfig = { PostPriority: string; ReduceOnBoardingTaskList: string; PostAcknowledgements: string; + DelayChannelAutocomplete: 'true' | 'false'; }; export type License = { @@ -730,6 +731,7 @@ export type ExperimentalSettings = { EnableAppBar: boolean; PatchPluginsReactDOM: boolean; DisableRefetchingOnBrowserFocus: boolean; + DelayChannelAutocomplete: boolean; }; export type AnalyticsSettings = {