[MM-36041] Ensure post options and app commands always have the bindings for its channel (#5563)

* Ensure post options always have the bindings for its channel

* Mimic webapp model

* Address feedback

* Add return to validate bindings

* Address feedback

* Use empty bindings constant to avoid rerenderings

Co-authored-by: Michael Kochell <6913320+mickmister@users.noreply.github.com>
This commit is contained in:
Daniel Espino García 2021-09-24 16:32:58 +02:00 committed by GitHub
parent edfd743699
commit dd36545079
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
17 changed files with 283 additions and 43 deletions

View file

@ -1,7 +1,8 @@
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.
import {fetchAppBindings} from '@mm-redux/actions/apps';
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 {ActionResult, DispatchFunc, GetStateFunc} from '@mm-redux/types/actions';
@ -10,9 +11,17 @@ import {appsEnabled} from '@utils/apps';
export function handleRefreshAppsBindings() {
return (dispatch: DispatchFunc, getState: GetStateFunc): ActionResult => {
const state = getState();
if (appsEnabled(state)) {
dispatch(fetchAppBindings(getCurrentUserId(state), getCurrentChannelId(state)));
if (!appsEnabled(state)) {
return {data: true};
}
dispatch(fetchAppBindings(getCurrentUserId(state), getCurrentChannelId(state)));
const threadChannelID = getThreadAppsBindingsChannelId(state);
if (threadChannelID) {
dispatch(fetchThreadAppBindings(getCurrentUserId(state), threadChannelID));
}
return {data: true};
};
}

View file

@ -33,7 +33,11 @@ describe('AppCommandParser', () => {
...reduxTestState,
entities: {
...reduxTestState.entities,
apps: {bindings},
apps: {
bindings,
threadBindings: bindings,
threadBindingsForms: {},
},
},
} as any;
const testStore = await mockStore(initialState);

View file

@ -4,6 +4,7 @@
/* eslint-disable max-lines */
import {
AppsTypes,
AppCallRequest,
AppBinding,
AppField,
@ -49,6 +50,9 @@ import {
getChannelSuggestions,
getUserSuggestions,
inTextMentionSuggestions,
getAppCommandForm,
getAppRHSCommandForm,
makeRHSAppBindingSelector,
} from './app_command_parser_dependencies';
export interface Store {
@ -85,6 +89,7 @@ interface Intl {
}
const getCommandBindings = makeAppBindingsSelector(AppBindingLocations.COMMAND);
const getRHSCommandBindings = makeRHSAppBindingSelector(AppBindingLocations.COMMAND);
export class ParsedCommand {
state = ParseState.Start;
@ -572,8 +577,6 @@ export class AppCommandParser {
private rootPostID?: string;
private intl: Intl;
forms: {[location: string]: AppForm} = {};
constructor(store: Store|null, intl: Intl, channelID: string, teamID = '', rootPostID = '') {
this.store = store || getStore() as Store;
this.channelID = channelID;
@ -926,8 +929,11 @@ export class AppCommandParser {
// getCommandBindings returns the commands in the redux store.
// They are grouped by app id since each app has one base command
private getCommandBindings = (): AppBinding[] => {
const bindings = getCommandBindings(this.store.getState());
return bindings;
const state = this.store.getState();
if (this.rootPostID) {
return getRHSCommandBindings(state);
}
return getCommandBindings(state);
}
// getChannel gets the channel in which the user is typing the command
@ -937,9 +943,6 @@ export class AppCommandParser {
}
public setChannelContext = (channelID: string, teamID = '', rootPostID?: string) => {
if (this.channelID !== channelID || this.rootPostID !== rootPostID || this.teamID !== teamID) {
this.forms = {};
}
this.channelID = channelID;
this.rootPostID = rootPostID;
this.teamID = teamID;
@ -1033,15 +1036,21 @@ export class AppCommandParser {
public getForm = async (location: string, binding: AppBinding): Promise<{form?: AppForm; error?: string} | undefined> => {
const rootID = this.rootPostID || '';
const key = `${this.channelID}-${rootID}-${location}`;
const form = this.forms[key];
const form = this.rootPostID ? getAppRHSCommandForm(this.store.getState(), key) : getAppCommandForm(this.store.getState(), key);
if (form) {
return {form};
}
this.forms = {};
const fetched = await this.fetchForm(binding);
if (fetched?.form) {
this.forms[key] = fetched.form;
let actionType: string = AppsTypes.RECEIVED_APP_COMMAND_FORM;
if (this.rootPostID) {
actionType = AppsTypes.RECEIVED_APP_RHS_COMMAND_FORM;
}
this.store.dispatch({
data: {form: fetched.form, location: key},
type: actionType,
});
}
return fetched;
}

View file

@ -37,6 +37,8 @@ export type {
DoAppCallResult,
} from 'types/actions/apps';
export {AppsTypes} from '@mm-redux/action_types';
export type {AutocompleteSuggestion};
export type {
@ -65,7 +67,7 @@ export {
COMMAND_SUGGESTION_USER,
} from '@mm-redux/constants/apps';
export {makeAppBindingsSelector} from '@mm-redux/selectors/entities/apps';
export {makeAppBindingsSelector, makeRHSAppBindingSelector, getAppCommandForm, getAppRHSCommandForm} from '@mm-redux/selectors/entities/apps';
export {getPost} from '@mm-redux/selectors/entities/posts';
export {getChannel as selectChannel, getCurrentChannel, getChannelByName as selectChannelByName} from '@mm-redux/selectors/entities/channels';

View file

@ -27,7 +27,12 @@ const makeStore = async (bindings: AppBinding[]) => {
...reduxTestState,
entities: {
...reduxTestState.entities,
apps: {bindings},
apps: {
bindings,
bindingsForms: {},
threadBindings: bindings,
threadBindingsForms: {},
},
},
} as any;
const testStore = await mockStore(initialState);

View file

@ -4,4 +4,9 @@ import keyMirror from '@mm-redux/utils/key_mirror';
export default keyMirror({
RECEIVED_APP_BINDINGS: null,
RECEIVED_THREAD_APP_BINDINGS: null,
CLEAR_APP_BINDINGS: null,
CLEAR_THREAD_APP_BINDINGS: null,
RECEIVED_APP_COMMAND_FORM: null,
RECEIVED_APP_RHS_COMMAND_FORM: null,
});

View file

@ -16,6 +16,30 @@ export function fetchAppBindings(userID: string, channelID: string): ActionFunc
return dispatch(bindClientFunc({
clientFunc: () => Client4.getAppsBindings(userID, channelID, teamID),
onSuccess: AppsTypes.RECEIVED_APP_BINDINGS,
onFailure: AppsTypes.CLEAR_APP_BINDINGS,
}));
};
}
export function fetchThreadAppBindings(userID: string, channelID: string): ActionFunc {
return async (dispatch: DispatchFunc, getState: GetStateFunc) => {
const channel = getChannel(getState(), channelID);
const teamID = channel?.team_id || '';
return dispatch(bindClientFunc({
clientFunc: async () => {
const bindings = await Client4.getAppsBindings(userID, channelID, teamID);
return {bindings, channelID};
},
onSuccess: AppsTypes.RECEIVED_THREAD_APP_BINDINGS,
onRequest: AppsTypes.CLEAR_THREAD_APP_BINDINGS,
}));
};
}
export function clearThreadAppBindings() {
return {
type: AppsTypes.CLEAR_THREAD_APP_BINDINGS,
data: true,
};
}

View file

@ -5,7 +5,7 @@ import {combineReducers} from 'redux';
import {AppsTypes} from '@mm-redux/action_types';
import {GenericAction} from '@mm-redux/types/actions';
import {AppBinding, AppsState} from '@mm-redux/types/apps';
import {AppBinding, AppCommandFormMap, AppsState} from '@mm-redux/types/apps';
import {validateBindings} from '@utils/apps';
export function bindings(state: AppBinding[] = [], action: GenericAction): AppBinding[] {
@ -17,6 +17,92 @@ export function bindings(state: AppBinding[] = [], action: GenericAction): AppBi
}
return newBindings;
}
case AppsTypes.CLEAR_APP_BINDINGS:
if (state.length > 0) {
return [];
}
return state;
default:
return state;
}
}
export function bindingsForms(state: AppCommandFormMap = {}, action: GenericAction): AppCommandFormMap {
switch (action.type) {
case AppsTypes.RECEIVED_APP_BINDINGS:
if (Object.keys(state).length) {
return {};
}
return state;
case AppsTypes.RECEIVED_APP_COMMAND_FORM: {
const {form, location} = action.data;
const newState = {
...state,
[location]: form,
};
return newState;
}
case AppsTypes.CLEAR_APP_BINDINGS: {
if (Object.keys(state).length) {
return {};
}
return state;
}
default:
return state;
}
}
export function threadBindings(state: AppBinding[] = [], action: GenericAction): AppBinding[] {
switch (action.type) {
case AppsTypes.RECEIVED_THREAD_APP_BINDINGS: {
return validateBindings(action.data.bindings) || [];
}
case AppsTypes.CLEAR_THREAD_APP_BINDINGS:
if (state.length > 0) {
return [];
}
return state;
default:
return state;
}
}
export function threadBindingsChannelId(state = '', action: GenericAction): string {
switch (action.type) {
case AppsTypes.RECEIVED_THREAD_APP_BINDINGS: {
return action.data.channelID || '';
}
case AppsTypes.CLEAR_THREAD_APP_BINDINGS:
if (state.length > 0) {
return '';
}
return state;
default:
return state;
}
}
export function threadBindingsForms(state: AppCommandFormMap = {}, action: GenericAction): AppCommandFormMap {
switch (action.type) {
case AppsTypes.RECEIVED_THREAD_APP_BINDINGS:
if (Object.keys(state).length) {
return {};
}
return state;
case AppsTypes.RECEIVED_APP_RHS_COMMAND_FORM: {
const {form, location} = action.data;
const newState = {
...state,
[location]: form,
};
return newState;
}
case AppsTypes.CLEAR_THREAD_APP_BINDINGS:
if (Object.keys(state).length) {
return {};
}
return state;
default:
return state;
}
@ -24,4 +110,8 @@ export function bindings(state: AppBinding[] = [], action: GenericAction): AppBi
export default (combineReducers({
bindings,
bindingsForms,
threadBindings,
threadBindingsForms,
threadBindingsChannelId,
}) as (b: AppsState, a: GenericAction) => AppsState);

View file

@ -7,16 +7,8 @@ import {AppBinding} from '@mm-redux/types/apps';
import {GlobalState} from '@mm-redux/types/store';
import {appsEnabled} from '@utils/apps';
export function getAppsBindings(state: GlobalState, location?: string): AppBinding[] {
if (!state.entities.apps.bindings) {
return [];
}
if (location) {
const bindings = state.entities.apps.bindings.filter((b) => b.location === location);
return bindings.reduce((accum: AppBinding[], current: AppBinding) => accum.concat(current.bindings || []), []);
}
return state.entities.apps.bindings;
export function getThreadAppsBindingsChannelId(state: GlobalState): string {
return state.entities.apps.threadBindingsChannelId;
}
export const makeAppBindingsSelector = (location: string) => {
@ -33,3 +25,26 @@ export const makeAppBindingsSelector = (location: string) => {
},
);
};
export const makeRHSAppBindingSelector = (location: string) => {
return createSelector(
(state: GlobalState) => state.entities.apps.threadBindings,
(state: GlobalState) => appsEnabled(state),
(bindings: AppBinding[], areAppsEnabled: boolean) => {
if (!areAppsEnabled || !bindings) {
return [];
}
const headerBindings = bindings.filter((b) => b.location === location);
return headerBindings.reduce((accum: AppBinding[], current: AppBinding) => accum.concat(current.bindings || []), []);
},
);
};
export const getAppCommandForm = (state: GlobalState, location: string) => {
return state.entities.apps.bindingsForms[location];
};
export const getAppRHSCommandForm = (state: GlobalState, location: string) => {
return state.entities.apps.threadBindingsForms[location];
};

View file

@ -18,6 +18,10 @@ export type AppModalState = {
export type AppsState = {
bindings: AppBinding[];
bindingsForms: AppCommandFormMap;
threadBindings: AppBinding[];
threadBindingsForms: AppCommandFormMap;
threadBindingsChannelId: string;
};
export type AppBinding = {
@ -219,3 +223,5 @@ export type FormResponseData = {
export type AppLookupResponse = {
items: AppSelectOption[];
}
export type AppCommandFormMap = {[location: string]: AppForm}

View file

@ -20,7 +20,7 @@ import ChannelInfoRow from '../channel_info_row';
type Props = {
bindings: AppBinding[];
theme: Theme;
currentChannel: Channel;
currentChannel?: Channel;
appsEnabled: boolean;
intl: typeof intlShape;
currentTeamId: string;
@ -32,11 +32,15 @@ type Props = {
}
const Bindings: React.FC<Props> = injectIntl((props: Props) => {
if (!props.appsEnabled) {
const {bindings, currentChannel, appsEnabled, ...optionProps} = props;
if (!appsEnabled) {
return null;
}
if (!currentChannel) {
return null;
}
const {bindings, ...optionProps} = props;
if (bindings.length === 0) {
return null;
}
@ -45,6 +49,7 @@ const Bindings: React.FC<Props> = injectIntl((props: Props) => {
<Option
key={b.app_id + b.location}
binding={b}
currentChannel={currentChannel}
{...optionProps}
/>
));

View file

@ -7,20 +7,24 @@ import {bindActionCreators, Dispatch, ActionCreatorsMapObject} from 'redux';
import {doAppCall, postEphemeralCallResponseForChannel} from '@actions/apps';
import {handleGotoLocation} from '@mm-redux/actions/integrations';
import {AppBindingLocations} from '@mm-redux/constants/apps';
import {getAppsBindings} from '@mm-redux/selectors/entities/apps';
import {makeAppBindingsSelector} from '@mm-redux/selectors/entities/apps';
import {getCurrentChannel} from '@mm-redux/selectors/entities/channels';
import {getCurrentTeamId} from '@mm-redux/selectors/entities/teams';
import {GenericAction, ActionFunc} from '@mm-redux/types/actions';
import {AppBinding} from '@mm-redux/types/apps';
import {GlobalState} from '@mm-redux/types/store';
import {DoAppCall, PostEphemeralCallResponseForChannel} from '@mm-types/actions/apps';
import {appsEnabled} from '@utils/apps';
import Bindings from './bindings';
const getChannelHeaderBindings = makeAppBindingsSelector(AppBindingLocations.CHANNEL_HEADER_ICON);
const emptyBindingsList: AppBinding[] = [];
function mapStateToProps(state: GlobalState) {
const apps = appsEnabled(state);
const currentChannel = getCurrentChannel(state) || {};
const bindings = apps ? getAppsBindings(state, AppBindingLocations.CHANNEL_HEADER_ICON) : [];
const currentChannel = getCurrentChannel(state);
const bindings = apps ? getChannelHeaderBindings(state) : emptyBindingsList;
return {
bindings,

View file

@ -1,25 +1,26 @@
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.
import React from 'react';
import React, {useState, useEffect} from 'react';
import {intlShape, injectIntl} from 'react-intl';
import {Alert} from 'react-native';
import {DoAppCall, PostEphemeralCallResponseForPost} from 'types/actions/apps';
import {showAppForm} from '@actions/navigation';
import {AppCallResponseTypes, AppCallTypes, AppExpandLevels} from '@mm-redux/constants/apps';
import {Client4} from '@client/rest';
import {AppBindingLocations, AppCallResponseTypes, AppCallTypes, AppExpandLevels} from '@mm-redux/constants/apps';
import {ActionResult} from '@mm-redux/types/actions';
import {AppBinding, AppCallResponse} from '@mm-redux/types/apps';
import {Post} from '@mm-redux/types/posts';
import {Theme} from '@mm-redux/types/theme';
import {UserProfile} from '@mm-redux/types/users';
import {isSystemMessage} from '@mm-redux/utils/post_utils';
import {DoAppCall, PostEphemeralCallResponseForPost} from '@mm-types/actions/apps';
import {createCallContext, createCallRequest} from '@utils/apps';
import PostOption from '../post_option';
type Props = {
bindings: AppBinding[];
bindings: AppBinding[] | null;
theme: Theme;
post: Post;
currentUser: UserProfile;
@ -34,13 +35,38 @@ type Props = {
};
}
const fetchBindings = (userId: string, channelId: string, teamId: string, setState: React.Dispatch<React.SetStateAction<AppBinding[] | null>>) => {
Client4.getAppsBindings(userId, channelId, teamId).then(
(allBindings) => {
const headerBindings = allBindings.filter((b) => b.location === AppBindingLocations.POST_MENU_ITEM);
const postMenuBindings = headerBindings.reduce((accum: AppBinding[], current: AppBinding) => accum.concat(current.bindings || []), []);
setState(postMenuBindings);
},
() => {/* Do nothing */},
).catch(() => {/* Do nothing */});
};
const Bindings = injectIntl((props: Props) => {
const [bindings, setBindings] = useState(props.bindings);
useEffect(() => {
if (bindings) {
return;
}
setBindings([]);
if (!props.appsEnabled) {
return;
}
fetchBindings(props.currentUser.id, props.post.channel_id, props.teamID, setBindings);
}, []);
if (!props.appsEnabled) {
return null;
}
const {bindings, post, ...optionProps} = props;
if (bindings.length === 0) {
const {post, ...optionProps} = props;
if (!bindings || bindings.length === 0) {
return null;
}

View file

@ -7,12 +7,13 @@ import {bindActionCreators, Dispatch, ActionCreatorsMapObject} from 'redux';
import {doAppCall, postEphemeralCallResponseForPost} from '@actions/apps';
import {handleGotoLocation} from '@mm-redux/actions/integrations';
import {AppBindingLocations} from '@mm-redux/constants/apps';
import {getAppsBindings} from '@mm-redux/selectors/entities/apps';
import {getChannel} from '@mm-redux/selectors/entities/channels';
import {getThreadAppsBindingsChannelId, makeAppBindingsSelector, makeRHSAppBindingSelector} from '@mm-redux/selectors/entities/apps';
import {getChannel, getCurrentChannelId} from '@mm-redux/selectors/entities/channels';
import {getTheme} from '@mm-redux/selectors/entities/preferences';
import {getCurrentTeamId} from '@mm-redux/selectors/entities/teams';
import {getCurrentUser} from '@mm-redux/selectors/entities/users';
import {GenericAction, ActionFunc} from '@mm-redux/types/actions';
import {AppBinding} from '@mm-redux/types/apps';
import {Post} from '@mm-redux/types/posts';
import {GlobalState} from '@mm-redux/types/store';
import {DoAppCall, PostEphemeralCallResponseForPost} from '@mm-types/actions/apps';
@ -24,9 +25,24 @@ type OwnProps = {
post: Post;
}
const emptyBindings: AppBinding[] = [];
const getPostMenuBindings = makeAppBindingsSelector(AppBindingLocations.POST_MENU_ITEM);
const getThreadPostMenuBindings = makeRHSAppBindingSelector(AppBindingLocations.POST_MENU_ITEM);
function mapStateToProps(state: GlobalState, props: OwnProps) {
const apps = appsEnabled(state);
const bindings = apps ? getAppsBindings(state, AppBindingLocations.POST_MENU_ITEM) : [];
let bindings: AppBinding[] | null = emptyBindings;
if (apps) {
if (props.post.channel_id === getCurrentChannelId(state)) {
bindings = getPostMenuBindings(state);
} else if (props.post.channel_id === getThreadAppsBindingsChannelId(state)) {
bindings = getThreadPostMenuBindings(state);
} else {
bindings = null;
}
}
const currentUser = getCurrentUser(state);
const teamID = getChannel(state, props.post.channel_id)?.team_id || getCurrentTeamId(state);

View file

@ -4,10 +4,11 @@
import {connect} from 'react-redux';
import {bindActionCreators} from 'redux';
import {fetchThreadAppBindings, clearThreadAppBindings} from '@mm-redux/actions/apps';
import {selectPost} from '@mm-redux/actions/posts';
import {setThreadFollow, updateThreadRead} from '@mm-redux/actions/threads';
import {getChannel, getMyCurrentChannelMembership} from '@mm-redux/selectors/entities/channels';
import {getCurrentUserId} from '@mm-redux/selectors/entities/common';
import {getCurrentChannelId, getCurrentUserId} from '@mm-redux/selectors/entities/common';
import {makeGetPostIdsForThread} from '@mm-redux/selectors/entities/posts';
import {getTheme, isCollapsedThreadsEnabled} from '@mm-redux/selectors/entities/preferences';
import {getThread} from '@mm-redux/selectors/entities/threads';
@ -18,6 +19,7 @@ function makeMapStateToProps() {
const getPostIdsForThread = makeGetPostIdsForThread();
return function mapStateToProps(state, ownProps) {
const channel = getChannel(state, ownProps.channelId);
const collapsedThreadsEnabled = isCollapsedThreadsEnabled(state);
return {
channelId: ownProps.channelId,
@ -31,6 +33,7 @@ function makeMapStateToProps() {
theme: getTheme(state),
thread: collapsedThreadsEnabled ? getThread(state, ownProps.rootId, true) : null,
threadLoadingStatus: state.requests.posts.getPostThread,
shouldFetchBindings: ownProps.channelId !== getCurrentChannelId(state),
};
};
}
@ -41,6 +44,8 @@ function mapDispatchToProps(dispatch) {
selectPost,
setThreadFollow,
updateThreadRead,
fetchThreadAppBindings,
clearThreadAppBindings,
}, dispatch),
};
}

View file

@ -17,6 +17,8 @@ describe('thread', () => {
actions: {
selectPost: jest.fn(),
setThreadFollow: jest.fn(),
fetchThreadAppBindings: jest.fn(),
clearThreadAppBindings: jest.fn(),
},
channelId: 'channel_id',
channelType: General.OPEN_CHANNEL,
@ -27,6 +29,9 @@ describe('thread', () => {
postIds: ['root_id', 'post_id_1', 'post_id_2'],
channelIsArchived: false,
threadLoadingStatus: {status: RequestStatus.STARTED},
teamId: 'team_id',
userId: 'user_id',
postBindings: [],
};
test('should match snapshot, has root post', () => {

View file

@ -20,8 +20,11 @@ export default class ThreadBase extends PureComponent {
selectPost: PropTypes.func.isRequired,
setThreadFollow: PropTypes.func.isRequired,
updateThreadRead: PropTypes.func,
fetchThreadAppBindings: PropTypes.func.isRequired,
clearThreadAppBindings: PropTypes.func.isRequired,
}).isRequired,
componentId: PropTypes.string,
channelId: PropTypes.string,
channelType: PropTypes.string,
collapsedThreadsEnabled: PropTypes.bool,
currentUserId: PropTypes.string,
@ -33,6 +36,7 @@ export default class ThreadBase extends PureComponent {
channelIsArchived: PropTypes.bool,
thread: PropTypes.object,
threadLoadingStatus: PropTypes.object,
shouldFetchBindings: PropTypes.bool,
};
static defaultProps = {
@ -115,6 +119,9 @@ export default class ThreadBase extends PureComponent {
this.markThreadRead();
this.removeTypingAnimation = this.registerTypingAnimation(this.bottomPaddingAnimation);
EventEmitter.on(TYPING_VISIBLE, this.runTypingAnimations);
if (this.props.shouldFetchBindings) {
this.props.actions.fetchThreadAppBindings(this.props.currentUserId, this.props.channelId);
}
}
componentWillReceiveProps(nextProps) {
@ -140,6 +147,9 @@ export default class ThreadBase extends PureComponent {
this.props.actions.selectPost('');
this.removeTypingAnimation();
EventEmitter.off(TYPING_VISIBLE, this.runTypingAnimations);
if (this.props.shouldFetchBindings) {
this.props.actions.clearThreadAppBindings();
}
}
handleThreadFollow() {