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

Pin across all apps on filter triggers unsaved changes badge #172025

Closed
bhavyarm opened this issue Nov 27, 2023 · 2 comments · Fixed by #172063
Closed

Pin across all apps on filter triggers unsaved changes badge #172025

bhavyarm opened this issue Nov 27, 2023 · 2 comments · Fixed by #172063
Assignees
Labels
bug Fixes for quality problems that affect the customer experience Feature:Discover Discover Application Feature:Filters Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL.

Comments

@bhavyarm
Copy link
Contributor

bhavyarm commented Nov 27, 2023

Kibana version: 8.12.0 snapshot

Elasticsearch version: 8.12.0 snapshot

Browser version: chrome latest

Browser OS version: OS X

Original install method (e.g. download page, yum, from source, etc.): from staging

Describe the bug: Pin across all apps on a filter triggers unsaved changes badge. But clicking on revert changes doesn't unpin this filter.

Please note other actions on the filter like enable/disable/include results works fine with triggering unsaved badges in the UI and reverting those changes.

Should unsaved changes badge triggered by user pinning a filter?

unpin.mp4
@bhavyarm bhavyarm added bug Fixes for quality problems that affect the customer experience Feature:Filters Feature:Discover Discover Application Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. labels Nov 27, 2023
@elasticmachine
Copy link
Contributor

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

@kertal
Copy link
Member

kertal commented Nov 28, 2023

thx @bhavyarm for reporting! Good catch and good question

Should unsaved changes badge triggered by user pinning a filter?

we will discuss this

@jughosta jughosta self-assigned this Nov 30, 2023
jughosta added a commit that referenced this issue Dec 8, 2023
…172063)

- Closes #172025

## Summary

This PR changes filters diffing logic for the "Unsaved changes" badge
and fixes "Revert changes" action of it. What to expect when testing:

- If user pins a filter and the order of filters does not change, then
the badge should not appear.
- If user has 2 filters for example and pins the second one, then this
action would change filters order and the badge would appear.
- If user presses "Revert changes" action, then newly pinned filters
would become unpinned.


### Checklist

- [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
kibanamachine pushed a commit to kibanamachine/kibana that referenced this issue Dec 8, 2023
…lastic#172063)

- Closes elastic#172025

## Summary

This PR changes filters diffing logic for the "Unsaved changes" badge
and fixes "Revert changes" action of it. What to expect when testing:

- If user pins a filter and the order of filters does not change, then
the badge should not appear.
- If user has 2 filters for example and pins the second one, then this
action would change filters order and the badge would appear.
- If user presses "Revert changes" action, then newly pinned filters
would become unpinned.

### Checklist

- [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

(cherry picked from commit c89c990)
kibanamachine referenced this issue Dec 8, 2023
…lters (#172063) (#172999)

# Backport

This will backport the following commits from `main` to `8.12`:
- [[Discover] Address reverting unsaved changes issue to unpin filters
(#172063)](#172063)

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

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

<!--BACKPORT [{"author":{"name":"Julia
Rechkunova","email":"[email protected]"},"sourceCommit":{"committedDate":"2023-12-08T21:45:31Z","message":"[Discover]
Address reverting unsaved changes issue to unpin filters (#172063)\n\n-
Closes https://github.com/elastic/kibana/issues/172025\r\n\r\n##
Summary\r\n\r\nThis PR changes filters diffing logic for the \"Unsaved
changes\" badge\r\nand fixes \"Revert changes\" action of it. What to
expect when testing:\r\n\r\n- If user pins a filter and the order of
filters does not change, then\r\nthe badge should not appear.\r\n- If
user has 2 filters for example and pins the second one, then
this\r\naction would change filters order and the badge would
appear.\r\n- If user presses \"Revert changes\" action, then newly
pinned filters\r\nwould become unpinned.\r\n\r\n\r\n###
Checklist\r\n\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","sha":"c89c990f30ea1e01bc29438aa48995fa2477119c","branchLabelMapping":{"^v8.13.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Feature:Discover","release_note:skip","Team:DataDiscovery","backport:prev-minor","v8.13.0"],"number":172063,"url":"https://github.com/elastic/kibana/pull/172063","mergeCommit":{"message":"[Discover]
Address reverting unsaved changes issue to unpin filters (#172063)\n\n-
Closes https://github.com/elastic/kibana/issues/172025\r\n\r\n##
Summary\r\n\r\nThis PR changes filters diffing logic for the \"Unsaved
changes\" badge\r\nand fixes \"Revert changes\" action of it. What to
expect when testing:\r\n\r\n- If user pins a filter and the order of
filters does not change, then\r\nthe badge should not appear.\r\n- If
user has 2 filters for example and pins the second one, then
this\r\naction would change filters order and the badge would
appear.\r\n- If user presses \"Revert changes\" action, then newly
pinned filters\r\nwould become unpinned.\r\n\r\n\r\n###
Checklist\r\n\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","sha":"c89c990f30ea1e01bc29438aa48995fa2477119c"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v8.13.0","labelRegex":"^v8.13.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/172063","number":172063,"mergeCommit":{"message":"[Discover]
Address reverting unsaved changes issue to unpin filters (#172063)\n\n-
Closes https://github.com/elastic/kibana/issues/172025\r\n\r\n##
Summary\r\n\r\nThis PR changes filters diffing logic for the \"Unsaved
changes\" badge\r\nand fixes \"Revert changes\" action of it. What to
expect when testing:\r\n\r\n- If user pins a filter and the order of
filters does not change, then\r\nthe badge should not appear.\r\n- If
user has 2 filters for example and pins the second one, then
this\r\naction would change filters order and the badge would
appear.\r\n- If user presses \"Revert changes\" action, then newly
pinned filters\r\nwould become unpinned.\r\n\r\n\r\n###
Checklist\r\n\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","sha":"c89c990f30ea1e01bc29438aa48995fa2477119c"}}]}]
BACKPORT-->

Co-authored-by: Julia Rechkunova <[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:Discover Discover Application 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.

4 participants