From 0a4ec73ee3690ff65397f057da5632e957067035 Mon Sep 17 00:00:00 2001 From: Michael Kochell <6913320+mickmister@users.noreply.github.com> Date: Mon, 28 Mar 2022 13:45:29 -0400 Subject: [PATCH] [MM-34233] Only refresh App bindings if apps plugin is enabled (#5673) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Daniel Espino García --- app/actions/views/channel.js | 1 - app/actions/views/channel.test.js | 3 ++ app/actions/websocket/apps.ts | 30 ++++++++------- app/actions/websocket/index.ts | 12 +++++- app/actions/websocket/websocket.test.js | 38 +++++++++++++------ .../app_command_parser.test.ts | 1 + .../app_slash_suggestion.test.tsx | 1 + .../slash_suggestion.test.tsx | 5 ++- app/constants/websocket.ts | 2 + app/mm-redux/action_types/apps.ts | 4 +- app/mm-redux/actions/apps.ts | 25 ++++++++++-- app/mm-redux/reducers/entities/apps.ts | 22 +++++++++-- app/mm-redux/types/apps.ts | 1 + app/utils/apps.ts | 10 ++++- 14 files changed, 119 insertions(+), 36 deletions(-) diff --git a/app/actions/views/channel.js b/app/actions/views/channel.js index 8d77c1f59..a9a02648c 100644 --- a/app/actions/views/channel.js +++ b/app/actions/views/channel.js @@ -232,7 +232,6 @@ export function handleSelectChannel(channelId) { dispatch(batchActions(actions, 'BATCH_SWITCH_CHANNEL')); if (appsEnabled(state)) { - //TODO improve sync method dispatch(fetchAppBindings(currentUserId, channelId)); } console.log('channel switch to', channel?.display_name, channelId, (Date.now() - dt), 'ms'); //eslint-disable-line diff --git a/app/actions/views/channel.test.js b/app/actions/views/channel.test.js index b03a7da4d..2c3693d15 100644 --- a/app/actions/views/channel.test.js +++ b/app/actions/views/channel.test.js @@ -166,6 +166,9 @@ describe('Actions.Views.Channel', () => { [currentTeamId]: {}, }, }, + apps: { + pluginEnabled: true, + }, general: { config: { EnableLegacySidebar: 'true', diff --git a/app/actions/websocket/apps.ts b/app/actions/websocket/apps.ts index 755728ad4..d35dcef97 100644 --- a/app/actions/websocket/apps.ts +++ b/app/actions/websocket/apps.ts @@ -1,27 +1,29 @@ // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. // See LICENSE.txt for license information. -import {fetchAppBindings, fetchThreadAppBindings} from '@mm-redux/actions/apps'; -import {getThreadAppsBindingsChannelId} from '@mm-redux/selectors/entities/apps'; -import {getCurrentChannelId} from '@mm-redux/selectors/entities/common'; -import {getCurrentUserId} from '@mm-redux/selectors/entities/users'; +import AppsTypes from '@mm-redux/action_types/apps'; +import {refreshAppBindings} from '@mm-redux/actions/apps'; import {ActionResult, DispatchFunc, GetStateFunc} from '@mm-redux/types/actions'; import {appsEnabled} from '@utils/apps'; export function handleRefreshAppsBindings() { return (dispatch: DispatchFunc, getState: GetStateFunc): ActionResult => { - const state = getState(); - if (!appsEnabled(state)) { - return {data: true}; - } - - dispatch(fetchAppBindings(getCurrentUserId(state), getCurrentChannelId(state))); - - const threadChannelID = getThreadAppsBindingsChannelId(state); - if (threadChannelID) { - dispatch(fetchThreadAppBindings(getCurrentUserId(state), threadChannelID)); + if (appsEnabled(getState())) { + dispatch(refreshAppBindings()); } return {data: true}; }; } + +export function handleAppsPluginEnabled() { + return { + type: AppsTypes.APPS_PLUGIN_ENABLED, + }; +} + +export function handleAppsPluginDisabled() { + return { + type: AppsTypes.APPS_PLUGIN_DISABLED, + }; +} diff --git a/app/actions/websocket/index.ts b/app/actions/websocket/index.ts index de39eee4c..b17cc6522 100644 --- a/app/actions/websocket/index.ts +++ b/app/actions/websocket/index.ts @@ -7,6 +7,7 @@ import {loadMe} from '@actions/views/user'; import {Client4} from '@client/rest'; import {WebsocketEvents} from '@constants'; import {ChannelTypes, GeneralTypes, PreferenceTypes, TeamTypes, UserTypes, RoleTypes} from '@mm-redux/action_types'; +import {refreshAppBindings} from '@mm-redux/actions/apps'; import {getThreads} from '@mm-redux/actions/threads'; import {getProfilesByIds, getStatusesByIds} from '@mm-redux/actions/users'; import {General} from '@mm-redux/constants'; @@ -37,11 +38,12 @@ import { handleCallScreenOn, handleCallScreenOff, handleCallUserRaiseHand, handleCallUserUnraiseHand, } from '@mmproducts/calls/store/actions/websockets'; +import {appsConfiguredAsEnabled} from '@utils/apps'; import {getChannelSinceValue} from '@utils/channels'; import {semverFromServerVersion} from '@utils/general'; import websocketClient from '@websocket'; -import {handleRefreshAppsBindings} from './apps'; +import {handleAppsPluginDisabled, handleAppsPluginEnabled, handleRefreshAppsBindings} from './apps'; import {handleSidebarCategoryCreated, handleSidebarCategoryDeleted, handleSidebarCategoryOrderUpdated, handleSidebarCategoryUpdated} from './categories'; import { handleChannelConvertedEvent, @@ -158,6 +160,10 @@ export function doReconnect(now: number) { setChannelRetryFailed(false), ], 'BATCH_WS_SUCCESS')); + if (appsConfiguredAsEnabled(state)) { + dispatch(refreshAppBindings()); + } + try { const {data: me}: any = await dispatch(loadMe(null, null, true)); @@ -430,6 +436,10 @@ function handleEvent(msg: WebSocketMessage) { return dispatch(handleThreadFollowChanged(msg)); case WebsocketEvents.APPS_FRAMEWORK_REFRESH_BINDINGS: return dispatch(handleRefreshAppsBindings()); + case WebsocketEvents.APPS_FRAMEWORK_PLUGIN_ENABLED: + return dispatch(handleAppsPluginEnabled()); + case WebsocketEvents.APPS_FRAMEWORK_PLUGIN_DISABLED: + return dispatch(handleAppsPluginDisabled()); case WebsocketEvents.SIDEBAR_CATEGORY_CREATED: return dispatch(handleSidebarCategoryCreated(msg)); case WebsocketEvents.SIDEBAR_CATEGORY_UPDATED: diff --git a/app/actions/websocket/websocket.test.js b/app/actions/websocket/websocket.test.js index 99c50d5af..3e14b6c39 100644 --- a/app/actions/websocket/websocket.test.js +++ b/app/actions/websocket/websocket.test.js @@ -84,6 +84,7 @@ describe('Actions.Websocket doReconnect', () => { const channel2 = TestHelper.fakeChannelWithId(team.id); const cMember1 = TestHelper.fakeChannelMember(me.id, channel1.id); const cMember2 = TestHelper.fakeChannelMember(me.id, channel2.id); + const post1 = TestHelper.fakePost(channel1.id); const currentTeamId = team.id; const currentUserId = me.id; @@ -92,7 +93,9 @@ describe('Actions.Websocket doReconnect', () => { const initialState = { entities: { general: { - config: {}, + config: { + FeatureFlagAppsEnabled: 'false', + }, }, teams: { currentTeamId, @@ -106,7 +109,7 @@ describe('Actions.Websocket doReconnect', () => { channels: { currentChannelId, channels: { - currentChannelId: channel1, + currentChannelId: channel1.id, }, }, users: { @@ -119,8 +122,20 @@ describe('Actions.Websocket doReconnect', () => { myPreferences: {}, }, posts: { - posts: {}, - postsInChannel: {}, + posts: { + [post1.id]: post1, + }, + postsInChannel: { + [channel1.id]: [ + { + recent: true, + order: [post1.id], + }, + ], + }, + }, + apps: { + pluginEnabled: true, }, }, websocket: { @@ -128,6 +143,13 @@ describe('Actions.Websocket doReconnect', () => { lastConnectAt: 0, lastDisconnectAt: 0, }, + views: { + channel: { + lastGetPosts: { + [channel1.id]: 1, + }, + }, + }, }; beforeAll(async () => { @@ -170,14 +192,12 @@ describe('Actions.Websocket doReconnect', () => { await TestHelper.tearDown(); }); - it('handle doReconnect', async () => { + it('handle doReconnect base case', async () => { const state = {...initialState}; const testStore = await mockStore(state); const timestamp = 1000; const expectedActions = [ 'BATCH_WS_SUCCESS', - ]; - const expectedMissingActions = [ 'BATCH_WS_RECONNECT', ]; @@ -197,7 +217,6 @@ describe('Actions.Websocket doReconnect', () => { await TestHelper.wait(300); const actionTypes = testStore.getActions().map((a) => a.type); expect(actionTypes).toEqual(expectedActions); - expect(actionTypes).not.toEqual(expect.arrayContaining(expectedMissingActions)); }); it('handle doReconnect after the current channel was archived or the user left it', async () => { @@ -260,8 +279,6 @@ describe('Actions.Websocket doReconnect', () => { const timestamp = 1000; const expectedActions = [ 'BATCH_WS_SUCCESS', - ]; - const expectedMissingActions = [ 'BATCH_WS_RECONNECT', ]; @@ -282,7 +299,6 @@ describe('Actions.Websocket doReconnect', () => { const actions = testStore.getActions().map((a) => a.type); expect(actions).toEqual(expect.arrayContaining(expectedActions)); - expect(actions).not.toEqual(expect.arrayContaining(expectedMissingActions)); }); it('handle doReconnect after the current channel was archived and setting is off', async () => { diff --git a/app/components/autocomplete/slash_suggestion/app_command_parser/app_command_parser.test.ts b/app/components/autocomplete/slash_suggestion/app_command_parser/app_command_parser.test.ts index 2a04f16b1..f2469cf87 100644 --- a/app/components/autocomplete/slash_suggestion/app_command_parser/app_command_parser.test.ts +++ b/app/components/autocomplete/slash_suggestion/app_command_parser/app_command_parser.test.ts @@ -36,6 +36,7 @@ describe('AppCommandParser', () => { bindings, threadBindings: bindings, threadBindingsForms: {}, + pluginEnabled: true, }, }, } as any; diff --git a/app/components/autocomplete/slash_suggestion/app_slash_suggestion/app_slash_suggestion.test.tsx b/app/components/autocomplete/slash_suggestion/app_slash_suggestion/app_slash_suggestion.test.tsx index 9e71499ea..e424a15f9 100644 --- a/app/components/autocomplete/slash_suggestion/app_slash_suggestion/app_slash_suggestion.test.tsx +++ b/app/components/autocomplete/slash_suggestion/app_slash_suggestion/app_slash_suggestion.test.tsx @@ -32,6 +32,7 @@ const makeStore = async (bindings: AppBinding[]) => { bindingsForms: {}, threadBindings: bindings, threadBindingsForms: {}, + pluginEnabled: true, }, }, } as any; diff --git a/app/components/autocomplete/slash_suggestion/slash_suggestion.test.tsx b/app/components/autocomplete/slash_suggestion/slash_suggestion.test.tsx index 19a7cfa64..a86d1a0fe 100644 --- a/app/components/autocomplete/slash_suggestion/slash_suggestion.test.tsx +++ b/app/components/autocomplete/slash_suggestion/slash_suggestion.test.tsx @@ -28,7 +28,10 @@ const makeStore = async (bindings: AppBinding[]) => { ...reduxTestState, entities: { ...reduxTestState.entities, - apps: {bindings}, + apps: { + bindings, + pluginEnabled: true, + }, }, } as any; const testStore = await mockStore(initialState); diff --git a/app/constants/websocket.ts b/app/constants/websocket.ts index 9b3ae6217..9be370735 100644 --- a/app/constants/websocket.ts +++ b/app/constants/websocket.ts @@ -47,6 +47,8 @@ const WebsocketEvents = { THREAD_FOLLOW_CHANGED: 'thread_follow_changed', THREAD_READ_CHANGED: 'thread_read_changed', APPS_FRAMEWORK_REFRESH_BINDINGS: 'custom_com.mattermost.apps_refresh_bindings', + APPS_FRAMEWORK_PLUGIN_ENABLED: 'custom_com.mattermost.apps_plugin_enabled', + APPS_FRAMEWORK_PLUGIN_DISABLED: 'custom_com.mattermost.apps_plugin_disabled', SIDEBAR_CATEGORY_CREATED: 'sidebar_category_created', SIDEBAR_CATEGORY_UPDATED: 'sidebar_category_updated', SIDEBAR_CATEGORY_DELETED: 'sidebar_category_deleted', diff --git a/app/mm-redux/action_types/apps.ts b/app/mm-redux/action_types/apps.ts index 32bedb4d6..82919e80e 100644 --- a/app/mm-redux/action_types/apps.ts +++ b/app/mm-redux/action_types/apps.ts @@ -5,8 +5,10 @@ import keyMirror from '@mm-redux/utils/key_mirror'; export default keyMirror({ RECEIVED_APP_BINDINGS: null, RECEIVED_THREAD_APP_BINDINGS: null, - CLEAR_APP_BINDINGS: null, + FAILED_TO_FETCH_APP_BINDINGS: null, CLEAR_THREAD_APP_BINDINGS: null, RECEIVED_APP_COMMAND_FORM: null, RECEIVED_APP_RHS_COMMAND_FORM: null, + APPS_PLUGIN_ENABLED: null, + APPS_PLUGIN_DISABLED: null, }); diff --git a/app/mm-redux/actions/apps.ts b/app/mm-redux/actions/apps.ts index c3159c371..ebcc0f3b3 100644 --- a/app/mm-redux/actions/apps.ts +++ b/app/mm-redux/actions/apps.ts @@ -3,20 +3,38 @@ import {Client4} from '@client/rest'; import {AppsTypes} from '@mm-redux/action_types'; +import {getThreadAppsBindingsChannelId} from '@mm-redux/selectors/entities/apps'; import {getChannel} from '@mm-redux/selectors/entities/channels'; +import {getCurrentChannelId, getCurrentTeamId, getCurrentUserId} from '@mm-redux/selectors/entities/common'; import {ActionFunc, DispatchFunc, GetStateFunc} from '@mm-redux/types/actions'; import {bindClientFunc} from './helpers'; +export function refreshAppBindings(): ActionFunc { + return (dispatch: DispatchFunc, getState: GetStateFunc) => { + const state = getState(); + + dispatch(fetchAppBindings(getCurrentUserId(state), getCurrentChannelId(state))); + + const threadChannelID = getThreadAppsBindingsChannelId(state); + if (threadChannelID) { + dispatch(fetchThreadAppBindings(getCurrentUserId(state), threadChannelID)); + } + + return {data: true}; + }; +} + export function fetchAppBindings(userID: string, channelID: string): ActionFunc { return async (dispatch: DispatchFunc, getState: GetStateFunc) => { - const channel = getChannel(getState(), channelID); - const teamID = channel?.team_id || ''; + const state = getState(); + const channel = getChannel(state, channelID); + const teamID = channel?.team_id || getCurrentTeamId(state); return dispatch(bindClientFunc({ clientFunc: () => Client4.getAppsBindings(userID, channelID, teamID), onSuccess: AppsTypes.RECEIVED_APP_BINDINGS, - onFailure: AppsTypes.CLEAR_APP_BINDINGS, + onFailure: AppsTypes.FAILED_TO_FETCH_APP_BINDINGS, })); }; } @@ -33,6 +51,7 @@ export function fetchThreadAppBindings(userID: string, channelID: string): Actio }, onSuccess: AppsTypes.RECEIVED_THREAD_APP_BINDINGS, onRequest: AppsTypes.CLEAR_THREAD_APP_BINDINGS, + onFailure: AppsTypes.FAILED_TO_FETCH_APP_BINDINGS, })); }; } diff --git a/app/mm-redux/reducers/entities/apps.ts b/app/mm-redux/reducers/entities/apps.ts index f2eddd34b..6a190c08b 100644 --- a/app/mm-redux/reducers/entities/apps.ts +++ b/app/mm-redux/reducers/entities/apps.ts @@ -17,7 +17,8 @@ export function bindings(state: AppBinding[] = [], action: GenericAction): AppBi } return newBindings; } - case AppsTypes.CLEAR_APP_BINDINGS: + case AppsTypes.FAILED_TO_FETCH_APP_BINDINGS: + case AppsTypes.APPS_PLUGIN_DISABLED: if (state.length > 0) { return []; } @@ -42,12 +43,11 @@ export function bindingsForms(state: AppCommandFormMap = {}, action: GenericActi }; return newState; } - case AppsTypes.CLEAR_APP_BINDINGS: { + case AppsTypes.FAILED_TO_FETCH_APP_BINDINGS: if (Object.keys(state).length) { return {}; } return state; - } default: return state; } @@ -108,10 +108,26 @@ export function threadBindingsForms(state: AppCommandFormMap = {}, action: Gener } } +export function pluginEnabled(state = true, action: GenericAction): boolean { + switch (action.type) { + case AppsTypes.APPS_PLUGIN_ENABLED: + return true; + case AppsTypes.APPS_PLUGIN_DISABLED: + return false; + case AppsTypes.RECEIVED_APP_BINDINGS: + return true; + case AppsTypes.FAILED_TO_FETCH_APP_BINDINGS: + return false; + default: + return state; + } +} + export default (combineReducers({ bindings, bindingsForms, threadBindings, threadBindingsForms, threadBindingsChannelId, + pluginEnabled, }) as (b: AppsState, a: GenericAction) => AppsState); diff --git a/app/mm-redux/types/apps.ts b/app/mm-redux/types/apps.ts index 9cb504567..2670bd5e5 100644 --- a/app/mm-redux/types/apps.ts +++ b/app/mm-redux/types/apps.ts @@ -21,6 +21,7 @@ export type AppsState = { threadBindings: AppBinding[]; threadBindingsForms: AppCommandFormMap; threadBindingsChannelId: string; + pluginEnabled: boolean; }; export type AppBinding = { diff --git a/app/utils/apps.ts b/app/utils/apps.ts index b123d59f8..a360ac9ee 100644 --- a/app/utils/apps.ts +++ b/app/utils/apps.ts @@ -6,11 +6,19 @@ import {AppBinding, AppCall, AppCallRequest, AppCallResponse, AppCallValues, App import {Config} from '@mm-redux/types/config'; import {GlobalState} from '@mm-redux/types/store'; -export function appsEnabled(state: GlobalState): boolean { // eslint-disable-line @typescript-eslint/no-unused-vars +export function appsConfiguredAsEnabled(state: GlobalState): boolean { // eslint-disable-line @typescript-eslint/no-unused-vars const enabled = getConfig(state)?.['FeatureFlagAppsEnabled' as keyof Partial]; return enabled === 'true'; } +export function appsPluginIsEnabled(state: GlobalState): boolean { + return state.entities.apps.pluginEnabled; +} + +export function appsEnabled(state: GlobalState): boolean { + return appsConfiguredAsEnabled(state) && appsPluginIsEnabled(state); +} + export function cleanBinding(binding: AppBinding, topLocation: string): AppBinding { return cleanBindingRec(binding, topLocation, 0); }