Skip to content

Commit

Permalink
[RAM] Slack api allowed channels (elastic#167150)
Browse files Browse the repository at this point in the history
## Summary

We add a lot of issue with our slack API by trying to fetch the
channels, so we decided to only use channel ID and create an allowed
list with channel IDs.

Here the new design:

# Connector Creation
#### Creation new connector
<img width="789" alt="image"
src="https://github.com/elastic/kibana/assets/189600/058667a0-5c93-4647-9f72-999008c410de">

#### Editing new connector
<img width="788" alt="image"
src="https://github.com/elastic/kibana/assets/189600/be2bfcb4-6fdd-4036-82a2-e5d2d50933f6">

#### Editing old slack api connector
<img width="788" alt="image"
src="https://github.com/elastic/kibana/assets/189600/aae32391-c69d-4947-8f02-4207112e2588">

# Slack Action Form
#### Creation of new slack api action in rule form
<img width="582" alt="image"
src="https://github.com/elastic/kibana/assets/189600/1389b6de-b549-44ff-88f6-f4cc9475198a">

#### Editing an old slack api action
<img width="560" alt="image"
src="https://github.com/elastic/kibana/assets/189600/a208ca97-1fe3-4301-9a2c-96807db846bf">

<img width="570" alt="image"
src="https://github.com/elastic/kibana/assets/189600/9fe019e7-4833-480b-9149-49aa618f6d74">




### Checklist

Delete any items that are not applicable to this PR.

- [ ] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

---------

Co-authored-by: Lisa Cawley <lcawley@elastic.co>
  • Loading branch information
XavierM and lcawl authored Oct 3, 2023
1 parent 853d9e6 commit 2256a7a
Show file tree
Hide file tree
Showing 25 changed files with 1,151 additions and 361 deletions.
26 changes: 21 additions & 5 deletions x-pack/plugins/stack_connectors/common/slack_api/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,30 @@ export const SlackApiSecretsSchema = schema.object({
token: schema.string({ minLength: 1 }),
});

export const SlackApiConfigSchema = schema.object({}, { defaultValue: {} });
export const SlackApiConfigSchema = schema.object({
allowedChannels: schema.maybe(
schema.arrayOf(
schema.object({
id: schema.string({ minLength: 1 }),
name: schema.string({ minLength: 1 }),
}),
{ maxSize: 25 }
)
),
});

export const ValidChannelIdSubActionParamsSchema = schema.object({
channelId: schema.maybe(schema.string()),
});

export const GetChannelsParamsSchema = schema.object({
subAction: schema.literal('getChannels'),
export const ValidChannelIdParamsSchema = schema.object({
subAction: schema.literal('validChannelId'),
subActionParams: ValidChannelIdSubActionParamsSchema,
});

export const PostMessageSubActionParamsSchema = schema.object({
channels: schema.arrayOf(schema.string(), { maxSize: 1 }),
channels: schema.maybe(schema.arrayOf(schema.string(), { maxSize: 1 })),
channelIds: schema.maybe(schema.arrayOf(schema.string(), { maxSize: 1 })),
text: schema.string({ minLength: 1 }),
});

Expand All @@ -28,6 +44,6 @@ export const PostMessageParamsSchema = schema.object({
});

export const SlackApiParamsSchema = schema.oneOf([
GetChannelsParamsSchema,
ValidChannelIdParamsSchema,
PostMessageParamsSchema,
]);
19 changes: 15 additions & 4 deletions x-pack/plugins/stack_connectors/common/slack_api/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,15 @@ import {
SlackApiSecretsSchema,
SlackApiParamsSchema,
SlackApiConfigSchema,
ValidChannelIdSubActionParamsSchema,
} from './schema';

export type SlackApiSecrets = TypeOf<typeof SlackApiSecretsSchema>;
export type SlackApiConfig = TypeOf<typeof SlackApiConfigSchema>;

export type PostMessageParams = TypeOf<typeof PostMessageParamsSchema>;
export type PostMessageSubActionParams = TypeOf<typeof PostMessageSubActionParamsSchema>;
export type ValidChannelIdSubActionParams = TypeOf<typeof ValidChannelIdSubActionParamsSchema>;
export type SlackApiParams = TypeOf<typeof SlackApiParamsSchema>;
export type SlackApiConnectorType = ConnectorType<
SlackApiConfig,
Expand Down Expand Up @@ -55,25 +57,34 @@ export interface SlackAPiResponse {
};
}

export interface ChannelsResponse {
export interface ChannelResponse {
id: string;
name: string;
is_channel: boolean;
is_archived: boolean;
is_private: boolean;
}
export interface GetChannelsResponse extends SlackAPiResponse {
channels?: ChannelsResponse[];

export interface ValidChannelResponse extends SlackAPiResponse {
channel?: ChannelResponse;
}

export interface PostMessageResponse extends SlackAPiResponse {
channel?: string;
}

export interface ValidChannelRouteResponse {
validChannels: Array<{ id: string; name: string }>;
invalidChannels: string[];
}

export interface SlackApiService {
getChannels: () => Promise<ConnectorTypeExecutorResult<GetChannelsResponse | void>>;
validChannelId: (
channelId: string
) => Promise<ConnectorTypeExecutorResult<ValidChannelResponse | void>>;
postMessage: ({
channels,
channelIds,
text,
}: PostMessageSubActionParams) => Promise<ConnectorTypeExecutorResult<unknown>>;
}
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,6 @@ describe('ActionForm - Slack API Connector', () => {
</IntlProvider>
);

expect(await screen.findByText('Channel is required.')).toBeInTheDocument();
expect(await screen.findByText('Channel ID is required.')).toBeInTheDocument();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,20 @@ describe('Slack action params validation', () => {
});
});

test('should succeed when action params include valid message and channels and channel ids', async () => {
const actionParams = {
subAction: 'postMessage',
subActionParams: { channels: ['general'], channelIds: ['general'], text: 'some text' },
};

expect(await connectorTypeModel.validateParams(actionParams)).toEqual({
errors: {
text: [],
channels: [],
},
});
});

test('should fail when channels field is missing in action params', async () => {
const actionParams = {
subAction: 'postMessage',
Expand All @@ -53,7 +67,7 @@ describe('Slack action params validation', () => {
expect(await connectorTypeModel.validateParams(actionParams)).toEqual({
errors: {
text: [],
channels: ['Channel is required.'],
channels: ['Channel ID is required.'],
},
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,17 @@ import { SLACK_API_CONNECTOR_ID } from '../../../common/slack_api/constants';
import { SlackActionParams } from '../types';
import { subtype } from '../slack/slack';

const isChannelValid = (channels?: string[], channelIds?: string[]) => {
if (
(channels === undefined && !channelIds?.length) ||
(channelIds === undefined && !channels?.length) ||
(!channelIds?.length && !channels?.length)
) {
return false;
}
return true;
};

export const getConnectorType = (): ConnectorTypeModel<
SlackApiConfig,
SlackApiSecrets,
Expand All @@ -50,7 +61,12 @@ export const getConnectorType = (): ConnectorTypeModel<
if (!actionParams.subActionParams.text) {
errors.text.push(MESSAGE_REQUIRED);
}
if (!actionParams.subActionParams.channels?.length) {
if (
!isChannelValid(
actionParams.subActionParams.channels,
actionParams.subActionParams.channelIds
)
) {
errors.channels.push(CHANNEL_REQUIRED);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,15 @@
*/

import React from 'react';
import { act, render, fireEvent, screen } from '@testing-library/react';
import { act, render, screen, waitFor } from '@testing-library/react';
import { useKibana } from '@kbn/triggers-actions-ui-plugin/public';

import { ConnectorFormTestProvider, waitForComponentToUpdate } from '../lib/test_utils';
import SlackActionFields from './slack_connectors';
import { useFetchChannels } from './use_fetch_channels';
import { SlackActionFieldsComponents as SlackActionFields } from './slack_connectors';
import { useValidChannels } from './use_valid_channels';

jest.mock('@kbn/triggers-actions-ui-plugin/public/common/lib/kibana');
jest.mock('./use_fetch_channels');
jest.mock('./use_valid_channels');

(useKibana as jest.Mock).mockImplementation(() => ({
services: {
Expand All @@ -32,15 +32,19 @@ jest.mock('./use_fetch_channels');
},
}));

(useFetchChannels as jest.Mock).mockImplementation(() => ({
channels: [],
isLoading: false,
}));
const useValidChannelsMock = useValidChannels as jest.Mock;

describe('SlackActionFields renders', () => {
const onSubmit = jest.fn();
beforeEach(() => {
useValidChannelsMock.mockClear();
onSubmit.mockClear();
jest.clearAllMocks();
useValidChannelsMock.mockImplementation(() => ({
channels: [],
isLoading: false,
resetChannelsToValidate: jest.fn(),
}));
});

it('all connector fields is rendered for web_api type', async () => {
Expand Down Expand Up @@ -77,27 +81,68 @@ describe('SlackActionFields renders', () => {
isDeprecated: false,
};

// Simulate that user just type a channel ID
useValidChannelsMock.mockImplementation(() => ({
channels: ['my-channel'],
isLoading: false,
resetChannelsToValidate: jest.fn(),
}));

render(
<ConnectorFormTestProvider connector={actionConnector} onSubmit={onSubmit}>
<SlackActionFields readOnly={false} isEdit={false} registerPreSubmitValidator={() => {}} />
</ConnectorFormTestProvider>
);
await waitForComponentToUpdate();
await act(async () => {
fireEvent.click(screen.getByTestId('form-test-provide-submit'));
act(() => {
screen.getByTestId('form-test-provide-submit').click();
});
expect(onSubmit).toBeCalledTimes(1);
expect(onSubmit).toBeCalledWith({
data: {
secrets: {
token: 'some token',

await waitFor(() => {
expect(onSubmit).toBeCalledTimes(1);
expect(onSubmit).toBeCalledWith({
data: {
secrets: {
token: 'some token',
},
config: {
allowedChannels: ['my-channel'],
},
id: 'test',
actionTypeId: '.slack',
name: 'slack',
isDeprecated: false,
},
id: 'test',
actionTypeId: '.slack',
name: 'slack',
isDeprecated: false,
isValid: true,
});
});
});

it('connector validation should failed when allowedChannels is empty', async () => {
const actionConnector = {
secrets: {
token: 'some token',
},
isValid: true,
id: 'test',
actionTypeId: '.slack',
name: 'slack',
config: {},
isDeprecated: false,
};

render(
<ConnectorFormTestProvider connector={actionConnector} onSubmit={onSubmit}>
<SlackActionFields readOnly={false} isEdit={false} registerPreSubmitValidator={() => {}} />
</ConnectorFormTestProvider>
);
await waitForComponentToUpdate();
act(() => {
screen.getByTestId('form-test-provide-submit').click();
});

await waitFor(() => {
expect(onSubmit).toBeCalledTimes(1);
expect(onSubmit).toBeCalledWith(expect.objectContaining({ isValid: false }));
});
});
});
Loading

0 comments on commit 2256a7a

Please sign in to comment.