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.

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

[Search][a11y] Add validation to extraction rules form #202980

Merged

Conversation

navarone-feekery
Copy link
Contributor

@navarone-feekery navarone-feekery commented Dec 4, 2024

Closes: #199154

This adds more validations to the Crawler extraction rules form.

The original issue of the error being at the top of the page is not easily fixable, as it's a catch-all server error display. Ideally, we shouldn't have server errors occurring at all, so it makes sense to me to just add a front-end validation to the inputs in this field.

These validations cover the following previously-missed scenarios:

  1. When a user has not added any rules
  2. When rule is for a specific URL and the URL pattern field is empty, or doesn't begin with /
  3. When the value for "Source" is empty (covers both HTML element and URL selectors)
  4. When "Content" is "A fixed value" and the value field is empty
in.mov

@navarone-feekery navarone-feekery added release_note:skip Skip the PR/issue when compiling release notes Team:Search backport:prev-major Backport to (8.x, 8.17, 8.16) the previous major branch and other branches in development labels Dec 4, 2024
@navarone-feekery navarone-feekery requested a review from a team as a code owner December 4, 2024 16:19
Copy link
Member

@jedrazb jedrazb left a comment

Choose a reason for hiding this comment

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

Neat! Good improvement! Since you are touching this code can you verify that:

  • if we enter invalid rule, the form is correctly marked with aria-invalid prop
  • if there is an error, the error is annotated with role=alert or any aria live prop that would trigger VO to tell user that there is an issue (optional)
  • linking react hook form reference

LGTM!

@navarone-feekery
Copy link
Contributor Author

@elasticmachine merge upstream

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

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
enterpriseSearch 2.6MB 2.6MB +1.0KB

History

@navarone-feekery navarone-feekery merged commit 9865da3 into elastic:main Dec 27, 2024
8 checks passed
@navarone-feekery navarone-feekery deleted the a11y/199154-add-validations branch December 27, 2024 11:22
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.16, 8.17, 8.x

https://github.com/elastic/kibana/actions/runs/12515457584

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Dec 27, 2024
## Closes: elastic#199154

This adds more validations to the Crawler extraction rules form.

The original issue of the error being at the top of the page is not
easily fixable, as it's a catch-all server error display. Ideally, we
shouldn't have server errors occurring at all, so it makes sense to me
to just add a front-end validation to the inputs in this field.

These validations cover the following previously-missed scenarios:

1. When a user has not added any rules
2. When rule is for a specific URL and the URL pattern field is empty,
or doesn't begin with `/`
3. When the value for "Source" is empty (covers both HTML element and
URL selectors)
4. When "Content" is "A fixed value" and the value field is empty

(cherry picked from commit 9865da3)
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Dec 27, 2024
## Closes: elastic#199154

This adds more validations to the Crawler extraction rules form.

The original issue of the error being at the top of the page is not
easily fixable, as it's a catch-all server error display. Ideally, we
shouldn't have server errors occurring at all, so it makes sense to me
to just add a front-end validation to the inputs in this field.

These validations cover the following previously-missed scenarios:

1. When a user has not added any rules
2. When rule is for a specific URL and the URL pattern field is empty,
or doesn't begin with `/`
3. When the value for "Source" is empty (covers both HTML element and
URL selectors)
4. When "Content" is "A fixed value" and the value field is empty

(cherry picked from commit 9865da3)
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Dec 27, 2024
## Closes: elastic#199154

This adds more validations to the Crawler extraction rules form.

The original issue of the error being at the top of the page is not
easily fixable, as it's a catch-all server error display. Ideally, we
shouldn't have server errors occurring at all, so it makes sense to me
to just add a front-end validation to the inputs in this field.

These validations cover the following previously-missed scenarios:

1. When a user has not added any rules
2. When rule is for a specific URL and the URL pattern field is empty,
or doesn't begin with `/`
3. When the value for "Source" is empty (covers both HTML element and
URL selectors)
4. When "Content" is "A fixed value" and the value field is empty

(cherry picked from commit 9865da3)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.16
8.17
8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Dec 27, 2024
… (#205192)

# Backport

This will backport the following commits from `main` to `8.17`:
- [[Search][a11y] Add validation to extraction rules form
(#202980)](#202980)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Navarone
Feekery","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-12-27T11:22:19Z","message":"[Search][a11y]
Add validation to extraction rules form (#202980)\n\n## Closes:
https://github.com/elastic/kibana/issues/199154\r\n\r\nThis adds more
validations to the Crawler extraction rules form.\r\n\r\nThe original
issue of the error being at the top of the page is not\r\neasily
fixable, as it's a catch-all server error display. Ideally,
we\r\nshouldn't have server errors occurring at all, so it makes sense
to me\r\nto just add a front-end validation to the inputs in this
field.\r\n\r\nThese validations cover the following previously-missed
scenarios:\r\n\r\n1. When a user has not added any rules\r\n2. When rule
is for a specific URL and the URL pattern field is empty,\r\nor doesn't
begin with `/`\r\n3. When the value for \"Source\" is empty (covers both
HTML element and\r\nURL selectors)\r\n4. When \"Content\" is \"A fixed
value\" and the value field is
empty","sha":"9865da3844674e560e9f87c07ce65ad025afb12f","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","Team:Search","backport:prev-major"],"title":"[Search][a11y]
Add validation to extraction rules
form","number":202980,"url":"https://github.com/elastic/kibana/pull/202980","mergeCommit":{"message":"[Search][a11y]
Add validation to extraction rules form (#202980)\n\n## Closes:
https://github.com/elastic/kibana/issues/199154\r\n\r\nThis adds more
validations to the Crawler extraction rules form.\r\n\r\nThe original
issue of the error being at the top of the page is not\r\neasily
fixable, as it's a catch-all server error display. Ideally,
we\r\nshouldn't have server errors occurring at all, so it makes sense
to me\r\nto just add a front-end validation to the inputs in this
field.\r\n\r\nThese validations cover the following previously-missed
scenarios:\r\n\r\n1. When a user has not added any rules\r\n2. When rule
is for a specific URL and the URL pattern field is empty,\r\nor doesn't
begin with `/`\r\n3. When the value for \"Source\" is empty (covers both
HTML element and\r\nURL selectors)\r\n4. When \"Content\" is \"A fixed
value\" and the value field is
empty","sha":"9865da3844674e560e9f87c07ce65ad025afb12f"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/202980","number":202980,"mergeCommit":{"message":"[Search][a11y]
Add validation to extraction rules form (#202980)\n\n## Closes:
https://github.com/elastic/kibana/issues/199154\r\n\r\nThis adds more
validations to the Crawler extraction rules form.\r\n\r\nThe original
issue of the error being at the top of the page is not\r\neasily
fixable, as it's a catch-all server error display. Ideally,
we\r\nshouldn't have server errors occurring at all, so it makes sense
to me\r\nto just add a front-end validation to the inputs in this
field.\r\n\r\nThese validations cover the following previously-missed
scenarios:\r\n\r\n1. When a user has not added any rules\r\n2. When rule
is for a specific URL and the URL pattern field is empty,\r\nor doesn't
begin with `/`\r\n3. When the value for \"Source\" is empty (covers both
HTML element and\r\nURL selectors)\r\n4. When \"Content\" is \"A fixed
value\" and the value field is
empty","sha":"9865da3844674e560e9f87c07ce65ad025afb12f"}}]}] BACKPORT-->

Co-authored-by: Navarone Feekery <[email protected]>
kibanamachine added a commit that referenced this pull request Dec 27, 2024
… (#205191)

# Backport

This will backport the following commits from `main` to `8.16`:
- [[Search][a11y] Add validation to extraction rules form
(#202980)](#202980)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Navarone
Feekery","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-12-27T11:22:19Z","message":"[Search][a11y]
Add validation to extraction rules form (#202980)\n\n## Closes:
https://github.com/elastic/kibana/issues/199154\r\n\r\nThis adds more
validations to the Crawler extraction rules form.\r\n\r\nThe original
issue of the error being at the top of the page is not\r\neasily
fixable, as it's a catch-all server error display. Ideally,
we\r\nshouldn't have server errors occurring at all, so it makes sense
to me\r\nto just add a front-end validation to the inputs in this
field.\r\n\r\nThese validations cover the following previously-missed
scenarios:\r\n\r\n1. When a user has not added any rules\r\n2. When rule
is for a specific URL and the URL pattern field is empty,\r\nor doesn't
begin with `/`\r\n3. When the value for \"Source\" is empty (covers both
HTML element and\r\nURL selectors)\r\n4. When \"Content\" is \"A fixed
value\" and the value field is
empty","sha":"9865da3844674e560e9f87c07ce65ad025afb12f","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","Team:Search","backport:prev-major"],"title":"[Search][a11y]
Add validation to extraction rules
form","number":202980,"url":"https://github.com/elastic/kibana/pull/202980","mergeCommit":{"message":"[Search][a11y]
Add validation to extraction rules form (#202980)\n\n## Closes:
https://github.com/elastic/kibana/issues/199154\r\n\r\nThis adds more
validations to the Crawler extraction rules form.\r\n\r\nThe original
issue of the error being at the top of the page is not\r\neasily
fixable, as it's a catch-all server error display. Ideally,
we\r\nshouldn't have server errors occurring at all, so it makes sense
to me\r\nto just add a front-end validation to the inputs in this
field.\r\n\r\nThese validations cover the following previously-missed
scenarios:\r\n\r\n1. When a user has not added any rules\r\n2. When rule
is for a specific URL and the URL pattern field is empty,\r\nor doesn't
begin with `/`\r\n3. When the value for \"Source\" is empty (covers both
HTML element and\r\nURL selectors)\r\n4. When \"Content\" is \"A fixed
value\" and the value field is
empty","sha":"9865da3844674e560e9f87c07ce65ad025afb12f"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/202980","number":202980,"mergeCommit":{"message":"[Search][a11y]
Add validation to extraction rules form (#202980)\n\n## Closes:
https://github.com/elastic/kibana/issues/199154\r\n\r\nThis adds more
validations to the Crawler extraction rules form.\r\n\r\nThe original
issue of the error being at the top of the page is not\r\neasily
fixable, as it's a catch-all server error display. Ideally,
we\r\nshouldn't have server errors occurring at all, so it makes sense
to me\r\nto just add a front-end validation to the inputs in this
field.\r\n\r\nThese validations cover the following previously-missed
scenarios:\r\n\r\n1. When a user has not added any rules\r\n2. When rule
is for a specific URL and the URL pattern field is empty,\r\nor doesn't
begin with `/`\r\n3. When the value for \"Source\" is empty (covers both
HTML element and\r\nURL selectors)\r\n4. When \"Content\" is \"A fixed
value\" and the value field is
empty","sha":"9865da3844674e560e9f87c07ce65ad025afb12f"}}]}] BACKPORT-->

Co-authored-by: Navarone Feekery <[email protected]>
kibanamachine added a commit that referenced this pull request Dec 27, 2024
#205193)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Search][a11y] Add validation to extraction rules form
(#202980)](#202980)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Navarone
Feekery","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-12-27T11:22:19Z","message":"[Search][a11y]
Add validation to extraction rules form (#202980)\n\n## Closes:
https://github.com/elastic/kibana/issues/199154\r\n\r\nThis adds more
validations to the Crawler extraction rules form.\r\n\r\nThe original
issue of the error being at the top of the page is not\r\neasily
fixable, as it's a catch-all server error display. Ideally,
we\r\nshouldn't have server errors occurring at all, so it makes sense
to me\r\nto just add a front-end validation to the inputs in this
field.\r\n\r\nThese validations cover the following previously-missed
scenarios:\r\n\r\n1. When a user has not added any rules\r\n2. When rule
is for a specific URL and the URL pattern field is empty,\r\nor doesn't
begin with `/`\r\n3. When the value for \"Source\" is empty (covers both
HTML element and\r\nURL selectors)\r\n4. When \"Content\" is \"A fixed
value\" and the value field is
empty","sha":"9865da3844674e560e9f87c07ce65ad025afb12f","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","Team:Search","backport:prev-major"],"title":"[Search][a11y]
Add validation to extraction rules
form","number":202980,"url":"https://github.com/elastic/kibana/pull/202980","mergeCommit":{"message":"[Search][a11y]
Add validation to extraction rules form (#202980)\n\n## Closes:
https://github.com/elastic/kibana/issues/199154\r\n\r\nThis adds more
validations to the Crawler extraction rules form.\r\n\r\nThe original
issue of the error being at the top of the page is not\r\neasily
fixable, as it's a catch-all server error display. Ideally,
we\r\nshouldn't have server errors occurring at all, so it makes sense
to me\r\nto just add a front-end validation to the inputs in this
field.\r\n\r\nThese validations cover the following previously-missed
scenarios:\r\n\r\n1. When a user has not added any rules\r\n2. When rule
is for a specific URL and the URL pattern field is empty,\r\nor doesn't
begin with `/`\r\n3. When the value for \"Source\" is empty (covers both
HTML element and\r\nURL selectors)\r\n4. When \"Content\" is \"A fixed
value\" and the value field is
empty","sha":"9865da3844674e560e9f87c07ce65ad025afb12f"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/202980","number":202980,"mergeCommit":{"message":"[Search][a11y]
Add validation to extraction rules form (#202980)\n\n## Closes:
https://github.com/elastic/kibana/issues/199154\r\n\r\nThis adds more
validations to the Crawler extraction rules form.\r\n\r\nThe original
issue of the error being at the top of the page is not\r\neasily
fixable, as it's a catch-all server error display. Ideally,
we\r\nshouldn't have server errors occurring at all, so it makes sense
to me\r\nto just add a front-end validation to the inputs in this
field.\r\n\r\nThese validations cover the following previously-missed
scenarios:\r\n\r\n1. When a user has not added any rules\r\n2. When rule
is for a specific URL and the URL pattern field is empty,\r\nor doesn't
begin with `/`\r\n3. When the value for \"Source\" is empty (covers both
HTML element and\r\nURL selectors)\r\n4. When \"Content\" is \"A fixed
value\" and the value field is
empty","sha":"9865da3844674e560e9f87c07ce65ad025afb12f"}}]}] BACKPORT-->

Co-authored-by: Navarone Feekery <[email protected]>
stratoula pushed a commit to stratoula/kibana that referenced this pull request Jan 2, 2025
## Closes: elastic#199154

This adds more validations to the Crawler extraction rules form.

The original issue of the error being at the top of the page is not
easily fixable, as it's a catch-all server error display. Ideally, we
shouldn't have server errors occurring at all, so it makes sense to me
to just add a front-end validation to the inputs in this field.

These validations cover the following previously-missed scenarios:

1. When a user has not added any rules
2. When rule is for a specific URL and the URL pattern field is empty,
or doesn't begin with `/`
3. When the value for "Source" is empty (covers both HTML element and
URL selectors)
4. When "Content" is "A fixed value" and the value field is empty
benakansara pushed a commit to benakansara/kibana that referenced this pull request Jan 2, 2025
## Closes: elastic#199154

This adds more validations to the Crawler extraction rules form.

The original issue of the error being at the top of the page is not
easily fixable, as it's a catch-all server error display. Ideally, we
shouldn't have server errors occurring at all, so it makes sense to me
to just add a front-end validation to the inputs in this field.

These validations cover the following previously-missed scenarios:

1. When a user has not added any rules
2. When rule is for a specific URL and the URL pattern field is empty,
or doesn't begin with `/`
3. When the value for "Source" is empty (covers both HTML element and
URL selectors)
4. When "Content" is "A fixed value" and the value field is empty
cqliu1 pushed a commit to cqliu1/kibana that referenced this pull request Jan 2, 2025
## Closes: elastic#199154

This adds more validations to the Crawler extraction rules form.

The original issue of the error being at the top of the page is not
easily fixable, as it's a catch-all server error display. Ideally, we
shouldn't have server errors occurring at all, so it makes sense to me
to just add a front-end validation to the inputs in this field.

These validations cover the following previously-missed scenarios:

1. When a user has not added any rules
2. When rule is for a specific URL and the URL pattern field is empty,
or doesn't begin with `/`
3. When the value for "Source" is empty (covers both HTML element and
URL selectors)
4. When "Content" is "A fixed value" and the value field is empty
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:prev-major Backport to (8.x, 8.17, 8.16) the previous major branch and other branches in development release_note:skip Skip the PR/issue when compiling release notes Team:Search v8.16.3 v8.17.1 v8.18.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Search:WebCrawlers:ViewCrawler:Manage Domains page]Incorrect error placement after Saving Extraction rules
4 participants