Skip to content

Commit

Permalink
[Actions] Removing placeholders and updating validation messages on c…
Browse files Browse the repository at this point in the history
…onnector forms (#82734)

* Removing placeholders. Updating validation messages

* Splitting out url and protocol validation

* Adding url validation for slack webhook urls

* Fixing test

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
ymao1 and kibanamachine authored Nov 12, 2020
1 parent fed9a4f commit 3412843
Show file tree
Hide file tree
Showing 13 changed files with 102 additions and 17 deletions.
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
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
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
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
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
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
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
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
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
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
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
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"
value={webhookUrl || ''}
data-test-subj="slackWebhookUrlInput"
onChange={(e) => {
Expand Down
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

0 comments on commit 3412843

Please sign in to comment.