MM-60200 Fix for Plugin Dialog with wrong channel ID. (#28122)
* save arguments to state for later usage * add test * feat: Add tests for submitInteractiveDialog with channel and thread context * fix: Add missing properties to DialogSubmission in integration_actions.test.ts * feat: Add channel_id to DialogSubmission objects in test file * refactor: Move selectedThreadIdInTeam to views.threads state * add channel id to state * add unit test * add unit test * update submitInteractiveDialog * update tests for changes * remove log line * refactor: Enhance submitInteractiveDialog proxy action with improved error handling * add userID to submit data to plugin * remove log message * remove unnecessary default state * lint fixes * update unit test * fixes from code review * fixes from code review * fixes from code review * fix test, add userID to expected --------- Co-authored-by: Mattermost Build <build@mattermost.com> Co-authored-by: Caleb Roseland <caleb@calebroseland.com>
Этот коммит содержится в:
коммит произвёл
GitHub
родитель
0cacee570a
Коммит
ee0361894f
@@ -220,6 +220,10 @@ export function executeCommand(message: string, args: CommandArgs): ActionFuncAs
|
|||||||
}
|
}
|
||||||
|
|
||||||
if (data.trigger_id) {
|
if (data.trigger_id) {
|
||||||
|
const dialogArguments = {
|
||||||
|
channel_id: args.channel_id,
|
||||||
|
};
|
||||||
|
dispatch({type: IntegrationTypes.RECEIVED_DIALOG_ARGUMENTS, data: dialogArguments});
|
||||||
dispatch({type: IntegrationTypes.RECEIVED_DIALOG_TRIGGER_ID, data: data.trigger_id});
|
dispatch({type: IntegrationTypes.RECEIVED_DIALOG_TRIGGER_ID, data: data.trigger_id});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -3,6 +3,7 @@
|
|||||||
|
|
||||||
import type {IncomingWebhook, OutgoingWebhook, Command, OAuthApp} from '@mattermost/types/integrations';
|
import type {IncomingWebhook, OutgoingWebhook, Command, OAuthApp} from '@mattermost/types/integrations';
|
||||||
|
|
||||||
|
import * as IntegrationActions from 'mattermost-redux/actions/integrations';
|
||||||
import {getProfilesByIds} from 'mattermost-redux/actions/users';
|
import {getProfilesByIds} from 'mattermost-redux/actions/users';
|
||||||
|
|
||||||
import * as Actions from 'actions/integration_actions';
|
import * as Actions from 'actions/integration_actions';
|
||||||
@@ -15,6 +16,12 @@ jest.mock('mattermost-redux/actions/users', () => ({
|
|||||||
}),
|
}),
|
||||||
}));
|
}));
|
||||||
|
|
||||||
|
jest.mock('mattermost-redux/actions/integrations', () => ({
|
||||||
|
submitInteractiveDialog: jest.fn(() => {
|
||||||
|
return {type: 'MOCK_SUBMIT_DIALOG', data: {errors: {}}};
|
||||||
|
}),
|
||||||
|
}));
|
||||||
|
|
||||||
interface CustomMatchers<R = unknown> {
|
interface CustomMatchers<R = unknown> {
|
||||||
arrayContainingExactly(stringArray: string[]): R;
|
arrayContainingExactly(stringArray: string[]): R;
|
||||||
}
|
}
|
||||||
@@ -37,6 +44,10 @@ describe('actions/integration_actions', () => {
|
|||||||
currentUserId: 'current_user_id',
|
currentUserId: 'current_user_id',
|
||||||
profiles: {current_user_id: {id: 'current_user_id', username: 'current_user'}, user_id3: {id: 'user_id3', username: 'user3'}, user_id4: {id: 'user_id4', username: 'user4'}},
|
profiles: {current_user_id: {id: 'current_user_id', username: 'current_user'}, user_id3: {id: 'user_id3', username: 'user3'}, user_id4: {id: 'user_id4', username: 'user4'}},
|
||||||
},
|
},
|
||||||
|
channels: {
|
||||||
|
currentChannelId: 'current_channel_id',
|
||||||
|
},
|
||||||
|
integrations: {},
|
||||||
},
|
},
|
||||||
};
|
};
|
||||||
|
|
||||||
@@ -119,4 +130,62 @@ describe('actions/integration_actions', () => {
|
|||||||
expect(getProfilesByIds).not.toHaveBeenCalled();
|
expect(getProfilesByIds).not.toHaveBeenCalled();
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe('submitInteractiveDialog', () => {
|
||||||
|
test('submitInteractiveDialog with current channel', async () => {
|
||||||
|
const testState = {
|
||||||
|
...initialState,
|
||||||
|
entities: {
|
||||||
|
...initialState.entities,
|
||||||
|
integrations: {
|
||||||
|
...initialState.entities.integrations,
|
||||||
|
dialogArguments: {
|
||||||
|
channel_id: 'dialog_channel_id',
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
};
|
||||||
|
const testStore = mockStore(testState);
|
||||||
|
const submission = {
|
||||||
|
callback_id: 'callback_id',
|
||||||
|
state: 'state',
|
||||||
|
submission: {
|
||||||
|
name: 'value',
|
||||||
|
},
|
||||||
|
user_id: 'current_user_id',
|
||||||
|
team_id: 'team_id1',
|
||||||
|
channel_id: '',
|
||||||
|
cancelled: false,
|
||||||
|
};
|
||||||
|
|
||||||
|
const expectedSubmission = {
|
||||||
|
...submission,
|
||||||
|
channel_id: 'dialog_channel_id',
|
||||||
|
};
|
||||||
|
|
||||||
|
await testStore.dispatch(Actions.submitInteractiveDialog(submission));
|
||||||
|
|
||||||
|
expect(IntegrationActions.submitInteractiveDialog).toHaveBeenCalledWith(expectedSubmission);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('submitInteractiveDialog with currentChannel context', async () => {
|
||||||
|
const testStore = mockStore(initialState);
|
||||||
|
|
||||||
|
const submission = {
|
||||||
|
callback_id: 'callback_id',
|
||||||
|
state: 'state',
|
||||||
|
submission: {
|
||||||
|
name: 'value',
|
||||||
|
},
|
||||||
|
user_id: 'current_user_id',
|
||||||
|
team_id: 'team_id1',
|
||||||
|
channel_id: 'current_channel_id',
|
||||||
|
cancelled: false,
|
||||||
|
};
|
||||||
|
|
||||||
|
await testStore.dispatch(Actions.submitInteractiveDialog(submission));
|
||||||
|
|
||||||
|
expect(IntegrationActions.submitInteractiveDialog).toHaveBeenCalledWith(submission);
|
||||||
|
});
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -1,11 +1,12 @@
|
|||||||
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
|
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
|
||||||
// See LICENSE.txt for license information.
|
// See LICENSE.txt for license information.
|
||||||
|
|
||||||
import type {IncomingWebhook, IncomingWebhooksWithCount, OutgoingWebhook, Command, OAuthApp, OutgoingOAuthConnection} from '@mattermost/types/integrations';
|
import type {IncomingWebhook, IncomingWebhooksWithCount, OutgoingWebhook, Command, OAuthApp, OutgoingOAuthConnection, DialogSubmission, SubmitDialogResponse} from '@mattermost/types/integrations';
|
||||||
|
|
||||||
import * as IntegrationActions from 'mattermost-redux/actions/integrations';
|
import * as IntegrationActions from 'mattermost-redux/actions/integrations';
|
||||||
import {getProfilesByIds} from 'mattermost-redux/actions/users';
|
import {getProfilesByIds} from 'mattermost-redux/actions/users';
|
||||||
import {appsEnabled} from 'mattermost-redux/selectors/entities/apps';
|
import {appsEnabled} from 'mattermost-redux/selectors/entities/apps';
|
||||||
|
import {getDialogArguments} from 'mattermost-redux/selectors/entities/integrations';
|
||||||
import {getUser} from 'mattermost-redux/selectors/entities/users';
|
import {getUser} from 'mattermost-redux/selectors/entities/users';
|
||||||
|
|
||||||
import type {ActionFuncAsync} from 'types/store';
|
import type {ActionFuncAsync} from 'types/store';
|
||||||
@@ -171,3 +172,29 @@ export function loadProfilesForOutgoingOAuthConnections(connections: OutgoingOAu
|
|||||||
return {data: null};
|
return {data: null};
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Proxy action for submitting an interactive dialog
|
||||||
|
* This enhances the base Redux action by checking for dialog arguments in the state
|
||||||
|
* before falling back to the current channel ID
|
||||||
|
*/
|
||||||
|
export function submitInteractiveDialog(submission: DialogSubmission): ActionFuncAsync<SubmitDialogResponse> {
|
||||||
|
return async (dispatch, getState) => {
|
||||||
|
const state = getState();
|
||||||
|
|
||||||
|
// Get dialog arguments from state if available
|
||||||
|
const dialogArguments = getDialogArguments(state);
|
||||||
|
|
||||||
|
// Use channel_id from dialog arguments if available
|
||||||
|
if (dialogArguments && dialogArguments.channel_id) {
|
||||||
|
submission.channel_id = dialogArguments.channel_id;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Dispatch the base action with our enhanced submission
|
||||||
|
const {data, error} = await dispatch(IntegrationActions.submitInteractiveDialog(submission));
|
||||||
|
if (error) {
|
||||||
|
return {error};
|
||||||
|
}
|
||||||
|
return {data};
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|||||||
@@ -6,8 +6,7 @@ import {connect} from 'react-redux';
|
|||||||
import {bindActionCreators} from 'redux';
|
import {bindActionCreators} from 'redux';
|
||||||
import type {Dispatch} from 'redux';
|
import type {Dispatch} from 'redux';
|
||||||
|
|
||||||
import {submitInteractiveDialog} from 'mattermost-redux/actions/integrations';
|
import {submitInteractiveDialog} from 'actions/integration_actions';
|
||||||
|
|
||||||
import {getEmojiMap} from 'selectors/emojis';
|
import {getEmojiMap} from 'selectors/emojis';
|
||||||
|
|
||||||
import type {GlobalState} from 'types/store';
|
import type {GlobalState} from 'types/store';
|
||||||
|
|||||||
@@ -27,5 +27,6 @@ export default keyMirror({
|
|||||||
DELETED_OUTGOING_OAUTH_CONNECTION: null,
|
DELETED_OUTGOING_OAUTH_CONNECTION: null,
|
||||||
|
|
||||||
RECEIVED_DIALOG_TRIGGER_ID: null,
|
RECEIVED_DIALOG_TRIGGER_ID: null,
|
||||||
|
RECEIVED_DIALOG_ARGUMENTS: null,
|
||||||
RECEIVED_DIALOG: null,
|
RECEIVED_DIALOG: null,
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -731,7 +731,7 @@ describe('Actions.Integrations', () => {
|
|||||||
expect(oauthApps[created.id].client_secret !== created.client_secret).toBeTruthy();
|
expect(oauthApps[created.id].client_secret !== created.client_secret).toBeTruthy();
|
||||||
});
|
});
|
||||||
|
|
||||||
it('submitInteractiveDialog', async () => {
|
it('submitInteractiveDialogError', async () => {
|
||||||
nock(Client4.getBaseRoute()).
|
nock(Client4.getBaseRoute()).
|
||||||
post('/actions/dialogs/submit').
|
post('/actions/dialogs/submit').
|
||||||
reply(200, {errors: {name: 'some error'}});
|
reply(200, {errors: {name: 'some error'}});
|
||||||
@@ -752,4 +752,68 @@ describe('Actions.Integrations', () => {
|
|||||||
expect(data.errors).toBeTruthy();
|
expect(data.errors).toBeTruthy();
|
||||||
expect(data.errors.name).toEqual('some error');
|
expect(data.errors.name).toEqual('some error');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('submitInteractiveDialog uses submission data', async () => {
|
||||||
|
const submit: DialogSubmission = {
|
||||||
|
callback_id: 'callback_id',
|
||||||
|
channel_id: 'submission_channel_id',
|
||||||
|
state: 'state',
|
||||||
|
submission: {
|
||||||
|
field1: 'value1',
|
||||||
|
},
|
||||||
|
cancelled: false,
|
||||||
|
team_id: 'submission_team_id',
|
||||||
|
user_id: TestHelper.generateId(),
|
||||||
|
};
|
||||||
|
|
||||||
|
nock(Client4.getBaseRoute()).
|
||||||
|
post('/actions/dialogs/submit', submit).
|
||||||
|
reply(200, OK_RESPONSE);
|
||||||
|
|
||||||
|
const {data} = await store.dispatch(Actions.submitInteractiveDialog(submit));
|
||||||
|
expect(data).toEqual(OK_RESPONSE);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('submitInteractiveDialog uses state information', async () => {
|
||||||
|
store = configureStore({
|
||||||
|
entities: {
|
||||||
|
users: {
|
||||||
|
currentUserId: 'currentUserID',
|
||||||
|
},
|
||||||
|
teams: {
|
||||||
|
currentTeamId: 'currentTeamID',
|
||||||
|
},
|
||||||
|
channels: {
|
||||||
|
currentChannelId: 'dialog_channel_id',
|
||||||
|
},
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
const submit: DialogSubmission = {
|
||||||
|
callback_id: 'callback_id',
|
||||||
|
channel_id: '',
|
||||||
|
state: 'state',
|
||||||
|
submission: {
|
||||||
|
field1: 'value1',
|
||||||
|
field2: 'value2',
|
||||||
|
},
|
||||||
|
cancelled: false,
|
||||||
|
team_id: '',
|
||||||
|
user_id: TestHelper.generateId(),
|
||||||
|
};
|
||||||
|
|
||||||
|
const expectedRequest = {
|
||||||
|
...submit,
|
||||||
|
channel_id: 'dialog_channel_id',
|
||||||
|
team_id: 'currentTeamID',
|
||||||
|
user_id: 'currentUserID',
|
||||||
|
};
|
||||||
|
|
||||||
|
nock(Client4.getBaseRoute()).
|
||||||
|
post('/actions/dialogs/submit', expectedRequest).
|
||||||
|
reply(200, OK_RESPONSE);
|
||||||
|
|
||||||
|
const {data} = await store.dispatch(Actions.submitInteractiveDialog(submit));
|
||||||
|
expect(data).toEqual(OK_RESPONSE);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -497,8 +497,11 @@ export function deleteOutgoingOAuthConnection(id: string): ActionFuncAsync<boole
|
|||||||
export function submitInteractiveDialog(submission: DialogSubmission): ActionFuncAsync<SubmitDialogResponse> {
|
export function submitInteractiveDialog(submission: DialogSubmission): ActionFuncAsync<SubmitDialogResponse> {
|
||||||
return async (dispatch, getState) => {
|
return async (dispatch, getState) => {
|
||||||
const state = getState();
|
const state = getState();
|
||||||
submission.channel_id = getCurrentChannelId(state);
|
|
||||||
|
// Use the current channel as fallback
|
||||||
|
submission.channel_id ||= getCurrentChannelId(state);
|
||||||
submission.team_id = getCurrentTeamId(state);
|
submission.team_id = getCurrentTeamId(state);
|
||||||
|
submission.user_id = getCurrentUserId(state);
|
||||||
|
|
||||||
let data;
|
let data;
|
||||||
try {
|
try {
|
||||||
|
|||||||
@@ -1349,6 +1349,43 @@ describe('Actions.Posts', () => {
|
|||||||
expect(data).toEqual({});
|
expect(data).toEqual({});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('doPostActionWithCookie with trigger_id', async () => {
|
||||||
|
const postId = 'posth67ja7ntdkek6g13dp3wka';
|
||||||
|
const actionId = 'action7ja7ntdkek6g13dp3wka';
|
||||||
|
const triggerId = 'trigger7ja7ntdkek6g13dp3wka';
|
||||||
|
const channelId = 'channel7ja7ntdkek6g13dp3wka';
|
||||||
|
|
||||||
|
// Setup post in state
|
||||||
|
store = configureStore({
|
||||||
|
entities: {
|
||||||
|
posts: {
|
||||||
|
posts: {
|
||||||
|
[postId]: {id: postId, channel_id: channelId},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
integrations: {
|
||||||
|
dialogArguments: {},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
nock(Client4.getBaseRoute()).
|
||||||
|
post(`/posts/${postId}/actions/${actionId}`).
|
||||||
|
reply(200, {trigger_id: triggerId});
|
||||||
|
|
||||||
|
const {data} = await store.dispatch(Actions.doPostActionWithCookie(postId, actionId, '', 'option'));
|
||||||
|
|
||||||
|
// Verify the trigger_id was received and stored in state
|
||||||
|
const state = store.getState();
|
||||||
|
expect(data).toBeTruthy();
|
||||||
|
expect(data).toEqual({trigger_id: triggerId});
|
||||||
|
expect(data).toBeTruthy();
|
||||||
|
|
||||||
|
expect(state.entities.integrations.dialogArguments).toBeTruthy();
|
||||||
|
expect(state.entities.integrations.dialogTriggerId).toEqual(triggerId);
|
||||||
|
expect(state.entities.integrations.dialogArguments.channel_id).toEqual(channelId);
|
||||||
|
});
|
||||||
|
|
||||||
it('addMessageIntoHistory', async () => {
|
it('addMessageIntoHistory', async () => {
|
||||||
const {dispatch, getState} = store;
|
const {dispatch, getState} = store;
|
||||||
|
|
||||||
|
|||||||
@@ -1243,6 +1243,13 @@ export function doPostActionWithCookie(postId: string, actionId: string, actionC
|
|||||||
type: IntegrationTypes.RECEIVED_DIALOG_TRIGGER_ID,
|
type: IntegrationTypes.RECEIVED_DIALOG_TRIGGER_ID,
|
||||||
data: data.trigger_id,
|
data: data.trigger_id,
|
||||||
});
|
});
|
||||||
|
const state = getState();
|
||||||
|
const post = PostSelectors.getPost(state, postId);
|
||||||
|
dispatch({
|
||||||
|
type: IntegrationTypes.RECEIVED_DIALOG_ARGUMENTS,
|
||||||
|
data: {
|
||||||
|
channel_id: post.channel_id,
|
||||||
|
}});
|
||||||
}
|
}
|
||||||
|
|
||||||
return {data};
|
return {data};
|
||||||
|
|||||||
@@ -3,7 +3,7 @@
|
|||||||
|
|
||||||
import {combineReducers} from 'redux';
|
import {combineReducers} from 'redux';
|
||||||
|
|
||||||
import type {Command, IncomingWebhook, OutgoingWebhook, OAuthApp, OutgoingOAuthConnection} from '@mattermost/types/integrations';
|
import type {Command, IncomingWebhook, OutgoingWebhook, OAuthApp, OutgoingOAuthConnection, DialogArgs} from '@mattermost/types/integrations';
|
||||||
import type {IDMappedObjects} from '@mattermost/types/utilities';
|
import type {IDMappedObjects} from '@mattermost/types/utilities';
|
||||||
|
|
||||||
import type {MMReduxAction} from 'mattermost-redux/action_types';
|
import type {MMReduxAction} from 'mattermost-redux/action_types';
|
||||||
@@ -296,6 +296,15 @@ function appsBotIDs(state: string[] = [], action: MMReduxAction) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function dialogArguments(state: DialogArgs | null = null, action: MMReduxAction) {
|
||||||
|
switch (action.type) {
|
||||||
|
case IntegrationTypes.RECEIVED_DIALOG_ARGUMENTS:
|
||||||
|
return action.data;
|
||||||
|
default:
|
||||||
|
return state;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
function dialogTriggerId(state = '', action: MMReduxAction) {
|
function dialogTriggerId(state = '', action: MMReduxAction) {
|
||||||
switch (action.type) {
|
switch (action.type) {
|
||||||
case IntegrationTypes.RECEIVED_DIALOG_TRIGGER_ID:
|
case IntegrationTypes.RECEIVED_DIALOG_TRIGGER_ID:
|
||||||
@@ -343,6 +352,9 @@ export default combineReducers({
|
|||||||
// object to represent built-in slash commands
|
// object to represent built-in slash commands
|
||||||
systemCommands,
|
systemCommands,
|
||||||
|
|
||||||
|
// object containing arguments for interactive dialog
|
||||||
|
dialogArguments,
|
||||||
|
|
||||||
// trigger ID for interactive dialogs
|
// trigger ID for interactive dialogs
|
||||||
dialogTriggerId,
|
dialogTriggerId,
|
||||||
|
|
||||||
|
|||||||
@@ -34,6 +34,10 @@ export function getOutgoingOAuthConnections(state: GlobalState) {
|
|||||||
return state.entities.integrations.outgoingOAuthConnections;
|
return state.entities.integrations.outgoingOAuthConnections;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
export function getDialogArguments(state: GlobalState) {
|
||||||
|
return state.entities.integrations.dialogArguments;
|
||||||
|
}
|
||||||
|
|
||||||
export const getFilteredIncomingHooks: (state: GlobalState) => IncomingWebhook[] = createSelector(
|
export const getFilteredIncomingHooks: (state: GlobalState) => IncomingWebhook[] = createSelector(
|
||||||
'getFilteredIncomingHooks',
|
'getFilteredIncomingHooks',
|
||||||
getCurrentTeamId,
|
getCurrentTeamId,
|
||||||
|
|||||||
@@ -69,6 +69,10 @@ export type CommandArgs = {
|
|||||||
root_id?: string;
|
root_id?: string;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
export type DialogArgs = {
|
||||||
|
channel_id: string;
|
||||||
|
}
|
||||||
|
|
||||||
export type CommandResponse = {
|
export type CommandResponse = {
|
||||||
response_type: string;
|
response_type: string;
|
||||||
text: string;
|
text: string;
|
||||||
@@ -134,6 +138,7 @@ export type IntegrationsState = {
|
|||||||
appsBotIDs: string[];
|
appsBotIDs: string[];
|
||||||
systemCommands: IDMappedObjects<Command>;
|
systemCommands: IDMappedObjects<Command>;
|
||||||
commands: IDMappedObjects<Command>;
|
commands: IDMappedObjects<Command>;
|
||||||
|
dialogArguments?: DialogArgs;
|
||||||
dialogTriggerId: string;
|
dialogTriggerId: string;
|
||||||
dialog?: {
|
dialog?: {
|
||||||
url: string;
|
url: string;
|
||||||
|
|||||||
Ссылка в новой задаче
Block a user