Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Actions] Removing placeholders and updating validation messages on connector forms #82734

Merged
merged 9 commits into from
Nov 12, 2020
Merged

[Actions] Removing placeholders and updating validation messages on connector forms #82734

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
c00eef4
Removing placeholders. Updating validation messages
ymao1 Nov 5, 2020
c20e5e8
Merge branch 'master' of https://github.com/elastic/kibana into actio…
ymao1 Nov 6, 2020
70a5a43
Splitting out url and protocol validation
ymao1 Nov 6, 2020
76d2ee7
Merge branch 'master' of https://github.com/elastic/kibana into actio…
ymao1 Nov 9, 2020
4884b26
Adding url validation for slack webhook urls
ymao1 Nov 9, 2020
dea3c34
Fixing test
ymao1 Nov 9, 2020
1e24dbf
Merge branch 'master' of https://github.com/elastic/kibana into actio…
ymao1 Nov 9, 2020
a8c2cd0
Merge branch 'master' into actions/placeholders
kibanamachine Nov 11, 2020
e831103
Merge branch 'master' into actions/placeholders
kibanamachine Nov 12, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions ...gins/triggers_actions_ui/public/application/components/builtin_action_types/jira/jira.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,12 @@ const validateConnector = (action: JiraActionConnector): ValidationResult => {
errors.apiUrl = [...errors.apiUrl, i18n.API_URL_REQUIRED];
}

if (action.config.apiUrl && !isValidUrl(action.config.apiUrl, 'https:')) {
errors.apiUrl = [...errors.apiUrl, i18n.API_URL_INVALID];
if (action.config.apiUrl) {
if (!isValidUrl(action.config.apiUrl)) {
errors.apiUrl = [...errors.apiUrl, i18n.API_URL_INVALID];
} else if (!isValidUrl(action.config.apiUrl, 'https:')) {
errors.apiUrl = [...errors.apiUrl, i18n.API_URL_REQUIRE_HTTPS];
}
}

if (!action.config.projectKey) {
Expand Down
1 change: 0 additions & 1 deletion ...rs_actions_ui/public/application/components/builtin_action_types/jira/jira_connectors.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,6 @@ const JiraConnectorFields: React.FC<ActionConnectorFieldsProps<JiraActionConnect
readOnly={readOnly}
value={apiUrl || ''} // Needed to prevent uncontrolled input error when value is undefined
data-test-subj="apiUrlFromInput"
placeholder="https://<site-url>"
onChange={(evt) => handleOnChangeActionConfig('apiUrl', evt.target.value)}
onBlur={() => {
if (!apiUrl) {
Expand Down
7 changes: 7 additions & 0 deletions ...iggers_actions_ui/public/application/components/builtin_action_types/jira/translations.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,13 @@ export const API_URL_INVALID = i18n.translate(
}
);

export const API_URL_REQUIRE_HTTPS = i18n.translate(
'xpack.triggersActionsUI.components.builtinActionTypes.jira.requireHttpsApiUrlTextField',
{
defaultMessage: 'URL must start with https://.',
}
);

export const JIRA_PROJECT_KEY_LABEL = i18n.translate(
'xpack.triggersActionsUI.components.builtinActionTypes.jira.projectKey',
{
Expand Down
8 changes: 6 additions & 2 deletions ...ers_actions_ui/public/application/components/builtin_action_types/resilient/resilient.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,12 @@ const validateConnector = (action: ResilientActionConnector): ValidationResult =
errors.apiUrl = [...errors.apiUrl, i18n.API_URL_REQUIRED];
}

if (action.config.apiUrl && !isValidUrl(action.config.apiUrl, 'https:')) {
errors.apiUrl = [...errors.apiUrl, i18n.API_URL_INVALID];
if (action.config.apiUrl) {
if (!isValidUrl(action.config.apiUrl)) {
errors.apiUrl = [...errors.apiUrl, i18n.API_URL_INVALID];
} else if (!isValidUrl(action.config.apiUrl, 'https:')) {
errors.apiUrl = [...errors.apiUrl, i18n.API_URL_REQUIRE_HTTPS];
}
}

if (!action.config.orgId) {
Expand Down
1 change: 0 additions & 1 deletion ..._ui/public/application/components/builtin_action_types/resilient/resilient_connectors.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,6 @@ const ResilientConnectorFields: React.FC<ActionConnectorFieldsProps<ResilientAct
readOnly={readOnly}
value={apiUrl || ''} // Needed to prevent uncontrolled input error when value is undefined
data-test-subj="apiUrlFromInput"
placeholder="https://<site-url>"
onChange={(evt) => handleOnChangeActionConfig('apiUrl', evt.target.value)}
onBlur={() => {
if (!apiUrl) {
Expand Down
7 changes: 7 additions & 0 deletions ...s_actions_ui/public/application/components/builtin_action_types/resilient/translations.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,13 @@ export const API_URL_INVALID = i18n.translate(
}
);

export const API_URL_REQUIRE_HTTPS = i18n.translate(
'xpack.triggersActionsUI.components.builtinActionTypes.resilient.requireHttpsApiUrlTextField',
{
defaultMessage: 'URL must start with https://.',
}
);

export const ORG_ID_LABEL = i18n.translate(
'xpack.triggersActionsUI.components.builtinActionTypes.resilient.orgId',
{
Expand Down
8 changes: 6 additions & 2 deletions ...s_actions_ui/public/application/components/builtin_action_types/servicenow/servicenow.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,12 @@ const validateConnector = (action: ServiceNowActionConnector): ValidationResult
errors.apiUrl = [...errors.apiUrl, i18n.API_URL_REQUIRED];
}

if (action.config.apiUrl && !isValidUrl(action.config.apiUrl, 'https:')) {
errors.apiUrl = [...errors.apiUrl, i18n.API_URL_INVALID];
if (action.config.apiUrl) {
if (!isValidUrl(action.config.apiUrl)) {
errors.apiUrl = [...errors.apiUrl, i18n.API_URL_INVALID];
} else if (!isValidUrl(action.config.apiUrl, 'https:')) {
errors.apiUrl = [...errors.apiUrl, i18n.API_URL_REQUIRE_HTTPS];
}
}

if (!action.secrets.username) {
Expand Down
1 change: 0 additions & 1 deletion ...i/public/application/components/builtin_action_types/servicenow/servicenow_connectors.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,6 @@ const ServiceNowConnectorFields: React.FC<ActionConnectorFieldsProps<
readOnly={readOnly}
value={apiUrl || ''} // Needed to prevent uncontrolled input error when value is undefined
data-test-subj="apiUrlFromInput"
placeholder="https://<site-url>"
onChange={(evt) => handleOnChangeActionConfig('apiUrl', evt.target.value)}
onBlur={() => {
if (!apiUrl) {
Expand Down
7 changes: 7 additions & 0 deletions ..._actions_ui/public/application/components/builtin_action_types/servicenow/translations.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,13 @@ export const API_URL_INVALID = i18n.translate(
}
);

export const API_URL_REQUIRE_HTTPS = i18n.translate(
'xpack.triggersActionsUI.components.builtinActionTypes.servicenow.requireHttpsApiUrlTextField',
{
defaultMessage: 'URL must start with https://.',
}
);

export const AUTHENTICATION_LABEL = i18n.translate(
'xpack.triggersActionsUI.components.builtinActionTypes.servicenow.authenticationLabel',
{
Expand Down
48 changes: 42 additions & 6 deletions ...iggers_actions_ui/public/application/components/builtin_action_types/slack/slack.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,11 @@ describe('slack connector validation', () => {
test('connector validation succeeds when connector config is valid', () => {
const actionConnector = {
secrets: {
webhookUrl: 'http:\\test',
webhookUrl: 'https:\\test',
},
id: 'test',
actionTypeId: '.email',
name: 'email',
actionTypeId: '.slack',
name: 'slack',
config: {},
} as SlackActionConnector;

Expand All @@ -46,12 +46,12 @@ describe('slack connector validation', () => {
});
});

test('connector validation fails when connector config is not valid', () => {
test('connector validation fails when connector config is not valid - no webhook url', () => {
const actionConnector = {
secrets: {},
id: 'test',
actionTypeId: '.email',
name: 'email',
actionTypeId: '.slack',
name: 'slack',
config: {},
} as SlackActionConnector;

Expand All @@ -61,6 +61,42 @@ describe('slack connector validation', () => {
},
});
});

test('connector validation fails when connector config is not valid - invalid webhook protocol', () => {
const actionConnector = {
secrets: {
webhookUrl: 'http:\\test',
},
id: 'test',
actionTypeId: '.slack',
name: 'slack',
config: {},
} as SlackActionConnector;

expect(actionTypeModel.validateConnector(actionConnector)).toEqual({
errors: {
webhookUrl: ['Webhook URL must start with https://.'],
},
});
});

test('connector validation fails when connector config is not valid - invalid webhook url', () => {
const actionConnector = {
secrets: {
webhookUrl: 'h',
},
id: 'test',
actionTypeId: '.slack',
name: 'slack',
config: {},
} as SlackActionConnector;

expect(actionTypeModel.validateConnector(actionConnector)).toEqual({
errors: {
webhookUrl: ['Webhook URL is invalid.'],
},
});
});
});

describe('slack action params validation', () => {
Expand Down
21 changes: 21 additions & 0 deletions ...ns/triggers_actions_ui/public/application/components/builtin_action_types/slack/slack.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { lazy } from 'react';
import { i18n } from '@kbn/i18n';
import { ActionTypeModel, ValidationResult } from '../../../../types';
import { SlackActionParams, SlackSecrets, SlackActionConnector } from '../types';
import { isValidUrl } from '../../../lib/value_validators';

export function getActionType(): ActionTypeModel<unknown, SlackSecrets, SlackActionParams> {
return {
Expand Down Expand Up @@ -39,6 +40,26 @@ export function getActionType(): ActionTypeModel<unknown, SlackSecrets, SlackAct
}
)
);
} else if (action.secrets.webhookUrl) {
if (!isValidUrl(action.secrets.webhookUrl)) {
errors.webhookUrl.push(
i18n.translate(
'xpack.triggersActionsUI.components.builtinActionTypes.slackAction.error.invalidWebhookUrlText',
{
defaultMessage: 'Webhook URL is invalid.',
}
)
);
} else if (!isValidUrl(action.secrets.webhookUrl, 'https:')) {
errors.webhookUrl.push(
i18n.translate(
'xpack.triggersActionsUI.components.builtinActionTypes.slackAction.error.requireHttpsWebhookUrlText',
{
defaultMessage: 'Webhook URL must start with https://.',
}
)
);
}
}
return validationResult;
},
Expand Down
1 change: 0 additions & 1 deletion ..._actions_ui/public/application/components/builtin_action_types/slack/slack_connectors.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ const SlackActionFields: React.FunctionComponent<ActionConnectorFieldsProps<
isInvalid={errors.webhookUrl.length > 0 && webhookUrl !== undefined}
name="webhookUrl"
readOnly={readOnly}
placeholder="Example: https://hooks.slack.com/services"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: For a Slack webhook URL we should not be able to pass non https protocol or invalid URL. But we don't have URL validation on the API level and on the UI level. I think it would be nice to have it here. Anyway, we have a documentation for a Slack configuration.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added URL and protocol validation to the slack webhook url

value={webhookUrl || ''}
data-test-subj="slackWebhookUrlInput"
onChange={(e) => {
Expand Down
1 change: 0 additions & 1 deletion ...ions_ui/public/application/components/builtin_action_types/webhook/webhook_connectors.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,6 @@ const WebhookActionConnectorFields: React.FunctionComponent<ActionConnectorField
fullWidth
readOnly={readOnly}
value={url || ''}
placeholder="https://<site-url> or http://<site-url>"
data-test-subj="webhookUrlText"
onChange={(e) => {
editActionConfig('url', e.target.value);
Expand Down