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

ymao1 · 2020-11-05T15:47:40Z

Resolves #81943

Summary

On connector forms, placeholders were used in URL fields. Removed the inconsistent placeholders and updated the URL validation message to be more specific. Screenshots attached of the validation messages:

JIRA Connector (requires https://)

IBM Resilient Connector (requires https://)

ServiceNow Connector (requires https://)

elasticmachine · 2020-11-05T17:54:50Z

Pinging @elastic/kibana-alerting-services (Team:Alerting Services)

ymao1 · 2020-11-05T17:55:25Z

@gchaps Do the updated URL validation messages look ok?

YulNaumenko · 2020-11-05T19:29:30Z

UX improvement is great, but I think it make sense to split the validation to the Valid URL and Valid URL protocol. Because currently the message is URL is invalid. URL must start with http:// or https:// will appear in all cases. What if we have just invalid URL but the correct protocol?

gchaps · 2020-11-05T21:52:23Z

I'd remove "URL is invalid" and just go with the line "URL must start with http//." This is a sentence, so should end with a period.

…ns/placeholders

ymao1 · 2020-11-06T13:59:38Z

I think it make sense to split the validation to the Valid URL and Valid URL protocol. Because currently the message is URL is invalid. URL must start with http:// or https:// will appear in all cases. What if we have just invalid URL but the correct protocol?

Good call on splitting up the validation! Pushed changes to do this and updated the screenshots.

I did notice that the URL validation doesn't actually fail on invalid URLs:

If I enter a URL with spaces like http://i have a space, the URL parser auto-replaces the spaces with %20
If I enter a URL with backslashes like http://google\.com, the URL parser parses it to http://goog but doesn't fail the validation.

gchaps · 2020-11-06T14:44:57Z

For the validation messages that simply say "URL is invalid" can we give some advice on what is needed to make it valid?

ymao1 · 2020-11-06T15:24:33Z

For the validation messages that simply say "URL is invalid" can we give some advice on what is needed to make it valid?

Maybe we can create a separate issue to improve the URL validation and update the message? So far, I've only see the "URL is invalid" message for inputs that don't have an http or https protocol.

YulNaumenko

LGTM

YulNaumenko · 2020-11-06T22:05:05Z

...ers_actions_ui/public/application/components/builtin_action_types/slack/slack_connectors.tsx

@@ -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"


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.

Added URL and protocol validation to the slack webhook url

…ns/placeholders

ymao1 · 2020-11-11T16:07:49Z

@elasticmachine merge upstream

ymao1 · 2020-11-11T16:27:23Z

UX improvement is great, but I think it make sense to split the validation to the Valid URL and Valid URL protocol. Because currently the message is URL is invalid. URL must start with http:// or https:// will appear in all cases. What if we have just invalid URL but the correct protocol?

@YulNaumenko I started making an issue for this but looked into why the current validation is not failing URLs with spaces or backslashes and it looks like the link you sent is Google's rules for invalid URLs and not a broad definition of invalid URLs. Spaces and backslashes are allowed generally in URLs, which is why when we use the Typescript URL object for validation, it does not fail. Do you think this is still an issue worth investigating more? We would likely have to write our own URL validator for more custom behavior. Have we had issues with users specifying invalid URLs in connectors?

YulNaumenko · 2020-11-12T16:55:13Z

UX improvement is great, but I think it make sense to split the validation to the Valid URL and Valid URL protocol. Because currently the message is URL is invalid. URL must start with http:// or https:// will appear in all cases. What if we have just invalid URL but the correct protocol?

@YulNaumenko I started making an issue for this but looked into why the current validation is not failing URLs with spaces or backslashes and it looks like the link you sent is Google's rules for invalid URLs and not a broad definition of invalid URLs. Spaces and backslashes are allowed generally in URLs, which is why when we use the Typescript URL object for validation, it does not fail. Do you think this is still an issue worth investigating more? We would likely have to write our own URL validator for more custom behavior. Have we had issues with users specifying invalid URLs in connectors?

I think we can skip this validation till we get the real issue. 👍

gmmorris

LGTM 👍

ymao1 · 2020-11-12T18:20:37Z

@elasticmachine merge upstream

kibanamachine · 2020-11-12T20:06:37Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: e831103

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`triggersActionsUi`	1.4MB	1.4MB	-207.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`triggersActionsUi`	140.0KB	141.9KB	+1.9KB

History

💚 Build #87381 succeeded a8c2cd0
💚 Build #86758 succeeded 1e24dbf
💔 Build #86734 failed 4884b26
💚 Build #86396 succeeded 70a5a43
💚 Build #86148 succeeded c00eef4

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

…onnector forms (elastic#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 <[email protected]>

…onnector forms (#82734) (#83336) * 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 <[email protected]> Co-authored-by: Kibana Machine <[email protected]>

* master: [Security Solution][Detections] Adds framework for replacing API schemas (elastic#82462) [Enterprise Search] Share Loading component (elastic#83246) Consolidates Jest configuration files and scripts (elastic#82671) APM header changes (elastic#82870) [Security Solutions] Adds a default for indicator match custom query of *:* (elastic#81727) [Security Solution] Note 10k object paging limit on Endpoint list (elastic#82889) [packerCache] fix gulp usage, don't archive node_modules (elastic#83327) Upgrade Node.js to version 12 (elastic#61587) [Actions] Removing placeholders and updating validation messages on connector forms (elastic#82734) [Fleet] Rename ingest_manager_api_integration tests fleet_api_integration (elastic#83011) [DOCS] Updates Discover docs (elastic#82773) [ML] Data frame analytics: Adds map view (elastic#81666) enables actions scoped within the stack to register at Basic license (elastic#82931)

Removing placeholders. Updating validation messages

c00eef4

ymao1 changed the title ~~Removing placeholders. Updating validation messages~~ [Actions] Removing placeholders and updating validation messages on connector forms Nov 5, 2020

ymao1 self-assigned this Nov 5, 2020

ymao1 added Feature:Actions release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v7.11.0 v8.0.0 labels Nov 5, 2020

ymao1 marked this pull request as ready for review November 5, 2020 17:54

ymao1 requested a review from a team as a code owner November 5, 2020 17:54

ymao1 added 2 commits November 6, 2020 08:12

Merge branch 'master' of https://github.com/elastic/kibana into actio…

c20e5e8

…ns/placeholders

Splitting out url and protocol validation

70a5a43

YulNaumenko approved these changes Nov 6, 2020

View reviewed changes

ymao1 added 4 commits November 9, 2020 07:42

Merge branch 'master' of https://github.com/elastic/kibana into actio…

76d2ee7

…ns/placeholders

Adding url validation for slack webhook urls

4884b26

Fixing test

dea3c34

Merge branch 'master' of https://github.com/elastic/kibana into actio…

1e24dbf

…ns/placeholders

gmmorris self-requested a review November 10, 2020 14:13

Merge branch 'master' into actions/placeholders

a8c2cd0

gmmorris approved these changes Nov 12, 2020

View reviewed changes

Merge branch 'master' into actions/placeholders

e831103

ymao1 merged commit 3412843 into elastic:master Nov 12, 2020

ymao1 mentioned this pull request Nov 12, 2020

[7.x] [Actions] Removing placeholders and updating validation messages on connector forms (#82734) #83336

Merged

mikecote added release_note:enhancement and removed release_note:skip Skip the PR/issue when compiling release notes labels Dec 16, 2020

ymao1 deleted the actions/placeholders branch February 4, 2021 15:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

ymao1 commented Nov 5, 2020 •

edited

Loading

elasticmachine commented Nov 5, 2020

ymao1 commented Nov 5, 2020

YulNaumenko commented Nov 5, 2020

gchaps commented Nov 5, 2020

ymao1 commented Nov 6, 2020

gchaps commented Nov 6, 2020

ymao1 commented Nov 6, 2020

YulNaumenko left a comment

YulNaumenko Nov 6, 2020

ymao1 Nov 9, 2020

ymao1 commented Nov 11, 2020

ymao1 commented Nov 11, 2020

YulNaumenko commented Nov 12, 2020 •

edited

Loading

gmmorris left a comment

ymao1 commented Nov 12, 2020

kibanamachine commented Nov 12, 2020

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

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

Conversation

ymao1 commented Nov 5, 2020 • edited Loading

Summary

elasticmachine commented Nov 5, 2020

ymao1 commented Nov 5, 2020

YulNaumenko commented Nov 5, 2020

gchaps commented Nov 5, 2020

ymao1 commented Nov 6, 2020

gchaps commented Nov 6, 2020

ymao1 commented Nov 6, 2020

YulNaumenko left a comment

Choose a reason for hiding this comment

YulNaumenko Nov 6, 2020

Choose a reason for hiding this comment

ymao1 Nov 9, 2020

Choose a reason for hiding this comment

ymao1 commented Nov 11, 2020

ymao1 commented Nov 11, 2020

YulNaumenko commented Nov 12, 2020 • edited Loading

gmmorris left a comment

Choose a reason for hiding this comment

ymao1 commented Nov 12, 2020

kibanamachine commented Nov 12, 2020

💚 Build Succeeded

Metrics [docs]

Async chunks

Page load bundle

History

ymao1 commented Nov 5, 2020 •

edited

Loading

YulNaumenko commented Nov 12, 2020 •

edited

Loading