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 Queries] "Save query" button is grayed out/disabled even after changing filters #176427

Closed
lukasolson opened this issue Feb 7, 2024 · 3 comments · Fixed by #170599
Closed
Labels
bug Fixes for quality problems that affect the customer experience Feature:Filters Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL.

Comments

@lukasolson
Copy link
Member

Kibana version: 8.13.0-SNAPSHOT

Describe the bug:

After creating a saved query, making changes to the meta information in a filter does not make the state "dirty", so the "Save query" button is still disabled.

Steps to reproduce:

  1. Create a saved query with a filter
  2. Edit the filter to exclude results

The "Save query" button is disabled.

Expected behavior:

The "Save query" button is enabled since the filter has changed.

Screenshots (if relevant):

Untitled.mov

Any additional context:

The same occurs if you disable/re-enable, or edit the filter to add a custom label.

@lukasolson lukasolson added bug Fixes for quality problems that affect the customer experience Feature:Filters Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. labels Feb 7, 2024
@elasticmachine
Copy link
Contributor

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

@davismcphee
Copy link
Contributor

@lukasolson Maybe you could verify when you get a chance to review again, but I believe this should be fixed by #170599.

@lukasolson
Copy link
Member Author

Oh, nice! Yeah it looks like it does.

CoenWarmer pushed a commit to CoenWarmer/kibana that referenced this issue Feb 15, 2024
## Summary

This PR introduces a number of changes and improvements to saved query
management:
- Add server side pagination (5 queries per page) and search
functionality to the "Load query" list, which improves UX and
performance by no longer requesting all queries at once.
- Redesign the "Load query" list to improve the UX and a11y, making it
possible for keyboard users to effectively navigate the list and
load/delete queries.
- Add an "Active" badge to the "Load query" list to indicate which list
entry represents the currently loaded query, and hoist the entry to the
top of the first page for better visibility when no search term exists.
- Deprecate the saved query `/_all` endpoint and update it to return
only the first 100 queries instead of loading them all into memory at
once.
- Add a new `titleKeyword` field to the saved query SO, which allows
sorting queries alphabetically by title when displaying them in the
"Load query" list.
- Improve the performance of the "has saved queries" check when Unified
Search is mounted to no longer request actual queries, and instead just
request the count.
- Update the saved query duplicate title check to no longer rely on
fetching all queries at once, and instead asynchronously check for
duplicates by title on save.
- Add server side duplicate title validation to the create and update
saved query endpoints.
- Various small fixes and cleanups throughout saved query management.


https://github.com/elastic/kibana/assets/25592674/43328aea-0f7b-4b7a-a5fb-e33ed822f317

Resolves elastic#172044.
Resolves elastic#176427.

## Testing

To generate saved queries for testing, run the script below and replace
`{KIBANA_REQUEST_COOKIE}` with the cookie header value from an API
request of a Kibana user with an active session:
```shell
for i in {1..100}; do curl 'http://localhost:5601/internal/saved_query/_create' \
  -H 'Accept: */*' \
  -H 'Accept-Language: en-US,en;q=0.9,az;q=0.8,es;q=0.7' \
  -H 'Cache-Control: no-cache' \
  -H 'Connection: keep-alive' \
  -H 'Content-Type: application/json' \
  -H 'Cookie: {KIBANA_REQUEST_COOKIE}' \
  -H 'elastic-api-version: 1' \
  -H 'kbn-build-number: 9007199254740991' \
  -H 'kbn-version: 8.13.0' \
  -H 'x-elastic-internal-origin: Kibana' \
  --data-raw '{"title":"query '"$(echo $(($i - 1)) | tr '[0-9]' '[a-j]')"'","description":"","query":{"query":"bytes > 500","language":"kuery"},"filters":[]}' \
  --compressed; done
```

### Checklist

- [x] 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
- [x] Any UI touched in this PR is usable by keyboard only (learn more
about [keyboard accessibility](https://webaim.org/techniques/keyboard/))
- [x] 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)
- [x] 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))
- [x] 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)

---------

Co-authored-by: kibanamachine <[email protected]>
Co-authored-by: Stratoula Kalafateli <[email protected]>
fkanout pushed a commit to fkanout/kibana that referenced this issue Mar 4, 2024
## Summary

This PR introduces a number of changes and improvements to saved query
management:
- Add server side pagination (5 queries per page) and search
functionality to the "Load query" list, which improves UX and
performance by no longer requesting all queries at once.
- Redesign the "Load query" list to improve the UX and a11y, making it
possible for keyboard users to effectively navigate the list and
load/delete queries.
- Add an "Active" badge to the "Load query" list to indicate which list
entry represents the currently loaded query, and hoist the entry to the
top of the first page for better visibility when no search term exists.
- Deprecate the saved query `/_all` endpoint and update it to return
only the first 100 queries instead of loading them all into memory at
once.
- Add a new `titleKeyword` field to the saved query SO, which allows
sorting queries alphabetically by title when displaying them in the
"Load query" list.
- Improve the performance of the "has saved queries" check when Unified
Search is mounted to no longer request actual queries, and instead just
request the count.
- Update the saved query duplicate title check to no longer rely on
fetching all queries at once, and instead asynchronously check for
duplicates by title on save.
- Add server side duplicate title validation to the create and update
saved query endpoints.
- Various small fixes and cleanups throughout saved query management.


https://github.com/elastic/kibana/assets/25592674/43328aea-0f7b-4b7a-a5fb-e33ed822f317

Resolves elastic#172044.
Resolves elastic#176427.

## Testing

To generate saved queries for testing, run the script below and replace
`{KIBANA_REQUEST_COOKIE}` with the cookie header value from an API
request of a Kibana user with an active session:
```shell
for i in {1..100}; do curl 'http://localhost:5601/internal/saved_query/_create' \
  -H 'Accept: */*' \
  -H 'Accept-Language: en-US,en;q=0.9,az;q=0.8,es;q=0.7' \
  -H 'Cache-Control: no-cache' \
  -H 'Connection: keep-alive' \
  -H 'Content-Type: application/json' \
  -H 'Cookie: {KIBANA_REQUEST_COOKIE}' \
  -H 'elastic-api-version: 1' \
  -H 'kbn-build-number: 9007199254740991' \
  -H 'kbn-version: 8.13.0' \
  -H 'x-elastic-internal-origin: Kibana' \
  --data-raw '{"title":"query '"$(echo $(($i - 1)) | tr '[0-9]' '[a-j]')"'","description":"","query":{"query":"bytes > 500","language":"kuery"},"filters":[]}' \
  --compressed; done
```

### Checklist

- [x] 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
- [x] Any UI touched in this PR is usable by keyboard only (learn more
about [keyboard accessibility](https://webaim.org/techniques/keyboard/))
- [x] 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)
- [x] 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))
- [x] 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)

---------

Co-authored-by: kibanamachine <[email protected]>
Co-authored-by: Stratoula Kalafateli <[email protected]>
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:Filters Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants