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

[Saved Search] Saved search schema does not allow empty sort arrays #156724

Closed
davismcphee opened this issue May 4, 2023 · 1 comment · Fixed by #156769
Closed

[Saved Search] Saved search schema does not allow empty sort arrays #156724

davismcphee opened this issue May 4, 2023 · 1 comment · Fixed by #156769
Assignees
Labels
bug Fixes for quality problems that affect the customer experience Feature:Saved Objects impact:critical This issue should be addressed immediately due to a critical level of impact on the product. loe:small Small Level of Effort Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. v8.8.0

Comments

@davismcphee
Copy link
Contributor

Issue discovered in elastic/integrations#6088.

The changes to the saved search schema introduced in #154016 cause validation failures when attempting to create a saved search with an empty sort array. Creating a saved search with an empty sort array is a valid scenario and should be allowed by the schema.

@davismcphee davismcphee added bug Fixes for quality problems that affect the customer experience loe:small Small Level of Effort Feature:Saved Objects impact:critical This issue should be addressed immediately due to a critical level of impact on the product. Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. v8.8.0 labels May 4, 2023
@davismcphee davismcphee self-assigned this May 4, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-data-discovery (Team:DataDiscovery)

jughosta pushed a commit that referenced this issue May 5, 2023
#156769)

## Summary

This PR removes the `minSize` restriction added in #154016 from the
saved search schema `sort` property, which was causing AWS integrations
to fail.

Before:

![broken](https://user-images.githubusercontent.com/25592674/236355150-112e103b-00ef-4f0d-baa5-35de6b944152.gif)

After:

![working](https://user-images.githubusercontent.com/25592674/236355167-1185279b-84ef-4570-8762-90944c839c4d.gif)

Fixes #156724.

### Checklist

- [ ] ~Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)~
- [ ]
~[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials~
- [x] [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
- [ ] ~Any UI touched in this PR is usable by keyboard only (learn more
about [keyboard
accessibility](https://webaim.org/techniques/keyboard/))~
- [ ] ~Any UI touched in this PR does not create any new axe failures
(run axe in browser:
[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),
[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))~
- [ ] ~If a plugin configuration key changed, check if it needs to be
allowlisted in the cloud and added to the [docker
list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)~
- [ ] ~This renders correctly on smaller devices using a responsive
layout. (You can test this [in your
browser](https://www.browserstack.com/guide/responsive-testing-on-local-server))~
- [ ] ~This was checked for [cross-browser
compatibility](https://www.elastic.co/support/matrix#matrix_browsers)~

### For maintainers

- [ ] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
kibanamachine pushed a commit to kibanamachine/kibana that referenced this issue May 5, 2023
elastic#156769)

## Summary

This PR removes the `minSize` restriction added in elastic#154016 from the
saved search schema `sort` property, which was causing AWS integrations
to fail.

Before:

![broken](https://user-images.githubusercontent.com/25592674/236355150-112e103b-00ef-4f0d-baa5-35de6b944152.gif)

After:

![working](https://user-images.githubusercontent.com/25592674/236355167-1185279b-84ef-4570-8762-90944c839c4d.gif)

Fixes elastic#156724.

### Checklist

- [ ] ~Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)~
- [ ]
~[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials~
- [x] [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
- [ ] ~Any UI touched in this PR is usable by keyboard only (learn more
about [keyboard
accessibility](https://webaim.org/techniques/keyboard/))~
- [ ] ~Any UI touched in this PR does not create any new axe failures
(run axe in browser:
[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),
[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))~
- [ ] ~If a plugin configuration key changed, check if it needs to be
allowlisted in the cloud and added to the [docker
list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)~
- [ ] ~This renders correctly on smaller devices using a responsive
layout. (You can test this [in your
browser](https://www.browserstack.com/guide/responsive-testing-on-local-server))~
- [ ] ~This was checked for [cross-browser
compatibility](https://www.elastic.co/support/matrix#matrix_browsers)~

### For maintainers

- [ ] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

(cherry picked from commit 2f53207)
kibanamachine added a commit that referenced this issue May 5, 2023
… arrays (#156769) (#156789)

# Backport

This will backport the following commits from `main` to `8.8`:
- [[Saved Search] Update saved search schema to allow empty `sort`
arrays (#156769)](#156769)

<!--- Backport version: 8.9.7 -->

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

<!--BACKPORT [{"author":{"name":"Davis
McPhee","email":"davis.mcphee@elastic.co"},"sourceCommit":{"committedDate":"2023-05-05T07:16:32Z","message":"[Saved
Search] Update saved search schema to allow empty `sort` arrays
(#156769)\n\n## Summary\r\n\r\nThis PR removes the `minSize` restriction
added in #154016 from the\r\nsaved search schema `sort` property, which
was causing AWS integrations\r\nto
fail.\r\n\r\nBefore:\r\n\r\n![broken](https://user-images.githubusercontent.com/25592674/236355150-112e103b-00ef-4f0d-baa5-35de6b944152.gif)\r\n\r\nAfter:\r\n\r\n![working](https://user-images.githubusercontent.com/25592674/236355167-1185279b-84ef-4570-8762-90944c839c4d.gif)\r\n\r\nFixes
#156724.\r\n\r\n### Checklist\r\n\r\n- [ ] ~Any text added follows
[EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)~\r\n-
[
]\r\n~[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas
added for features that require explanation or tutorials~\r\n- [x] [Unit
or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [ ] ~Any UI
touched in this PR is usable by keyboard only (learn more\r\nabout
[keyboard\r\naccessibility](https://webaim.org/techniques/keyboard/))~\r\n-
[ ] ~Any UI touched in this PR does not create any new axe
failures\r\n(run axe in
browser:\r\n[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),\r\n[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))~\r\n-
[ ] ~If a plugin configuration key changed, check if it needs to
be\r\nallowlisted in the cloud and added to the
[docker\r\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)~\r\n-
[ ] ~This renders correctly on smaller devices using a
responsive\r\nlayout. (You can test this [in
your\r\nbrowser](https://www.browserstack.com/guide/responsive-testing-on-local-server))~\r\n-
[ ] ~This was checked for
[cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)~\r\n\r\n###
For maintainers\r\n\r\n- [ ] This was checked for breaking API changes
and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"2f53207727d321363a1664e2a952301fd5b20f7e","branchLabelMapping":{"^v8.9.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Feature:Saved
Objects","release_note:skip","Team:DataDiscovery","v8.8.0","v8.9.0"],"number":156769,"url":"https://github.com/elastic/kibana/pull/156769","mergeCommit":{"message":"[Saved
Search] Update saved search schema to allow empty `sort` arrays
(#156769)\n\n## Summary\r\n\r\nThis PR removes the `minSize` restriction
added in #154016 from the\r\nsaved search schema `sort` property, which
was causing AWS integrations\r\nto
fail.\r\n\r\nBefore:\r\n\r\n![broken](https://user-images.githubusercontent.com/25592674/236355150-112e103b-00ef-4f0d-baa5-35de6b944152.gif)\r\n\r\nAfter:\r\n\r\n![working](https://user-images.githubusercontent.com/25592674/236355167-1185279b-84ef-4570-8762-90944c839c4d.gif)\r\n\r\nFixes
#156724.\r\n\r\n### Checklist\r\n\r\n- [ ] ~Any text added follows
[EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)~\r\n-
[
]\r\n~[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas
added for features that require explanation or tutorials~\r\n- [x] [Unit
or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [ ] ~Any UI
touched in this PR is usable by keyboard only (learn more\r\nabout
[keyboard\r\naccessibility](https://webaim.org/techniques/keyboard/))~\r\n-
[ ] ~Any UI touched in this PR does not create any new axe
failures\r\n(run axe in
browser:\r\n[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),\r\n[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))~\r\n-
[ ] ~If a plugin configuration key changed, check if it needs to
be\r\nallowlisted in the cloud and added to the
[docker\r\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)~\r\n-
[ ] ~This renders correctly on smaller devices using a
responsive\r\nlayout. (You can test this [in
your\r\nbrowser](https://www.browserstack.com/guide/responsive-testing-on-local-server))~\r\n-
[ ] ~This was checked for
[cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)~\r\n\r\n###
For maintainers\r\n\r\n- [ ] This was checked for breaking API changes
and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"2f53207727d321363a1664e2a952301fd5b20f7e"}},"sourceBranch":"main","suggestedTargetBranches":["8.8"],"targetPullRequestStates":[{"branch":"8.8","label":"v8.8.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.9.0","labelRegex":"^v8.9.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/156769","number":156769,"mergeCommit":{"message":"[Saved
Search] Update saved search schema to allow empty `sort` arrays
(#156769)\n\n## Summary\r\n\r\nThis PR removes the `minSize` restriction
added in #154016 from the\r\nsaved search schema `sort` property, which
was causing AWS integrations\r\nto
fail.\r\n\r\nBefore:\r\n\r\n![broken](https://user-images.githubusercontent.com/25592674/236355150-112e103b-00ef-4f0d-baa5-35de6b944152.gif)\r\n\r\nAfter:\r\n\r\n![working](https://user-images.githubusercontent.com/25592674/236355167-1185279b-84ef-4570-8762-90944c839c4d.gif)\r\n\r\nFixes
#156724.\r\n\r\n### Checklist\r\n\r\n- [ ] ~Any text added follows
[EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)~\r\n-
[
]\r\n~[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas
added for features that require explanation or tutorials~\r\n- [x] [Unit
or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [ ] ~Any UI
touched in this PR is usable by keyboard only (learn more\r\nabout
[keyboard\r\naccessibility](https://webaim.org/techniques/keyboard/))~\r\n-
[ ] ~Any UI touched in this PR does not create any new axe
failures\r\n(run axe in
browser:\r\n[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),\r\n[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))~\r\n-
[ ] ~If a plugin configuration key changed, check if it needs to
be\r\nallowlisted in the cloud and added to the
[docker\r\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)~\r\n-
[ ] ~This renders correctly on smaller devices using a
responsive\r\nlayout. (You can test this [in
your\r\nbrowser](https://www.browserstack.com/guide/responsive-testing-on-local-server))~\r\n-
[ ] ~This was checked for
[cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)~\r\n\r\n###
For maintainers\r\n\r\n- [ ] This was checked for breaking API changes
and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"2f53207727d321363a1664e2a952301fd5b20f7e"}}]}]
BACKPORT-->

Co-authored-by: Davis McPhee <davis.mcphee@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience Feature:Saved Objects impact:critical This issue should be addressed immediately due to a critical level of impact on the product. loe:small Small Level of Effort Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. v8.8.0
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants