-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
Reset dirty saved query on reload #43927
Reset dirty saved query on reload #43927
Conversation
Pinging @elastic/kibana-app |
💚 Build Succeeded |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code LGTM, thanks for the change
💚 Build Succeeded |
@@ -145,6 +145,7 @@ export const SavedQueryManagementComponent: FunctionComponent<Props> = ({ | |||
savedQuery={savedQuery} | |||
isSelected={!!loadedSavedQuery && loadedSavedQuery.id === savedQuery.id} | |||
onSelect={savedQueryToSelect => { | |||
onClearSavedQuery(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately this causes two search requests to elasticsearch to fire, one after the clear and one after the load. It think it would be better to handle this in the app controller. In Discover for example, we can change this line to be $scope.savedQuery = { ...savedQuery };
and that will fix the problem.
The issue is that $scope.savedQuery === savedQuery in this particular case, so even though we're assigning $scope.savedQuery the watcher here won't fire because there was no change. By creating a copy in onSavedQueryUpdated
we signal to angular that a change has happened and the watcher needs to fire. The same thing can be done in Visualize and Dashboard.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
…r, Visualize and Dashboard
💚 Build Succeeded |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
* Clears changes a loaded saved query before loading another one * Adds general functional test to ensure changes to a saved query are discarded on reloading it * Moves resetting a dirty saved query to the app controllers in Discover, Visualize and Dashboard
Summary
This PR ensures any changes made to a saved query are discarded when it is reloaded.
Addresses 43887
Previous behavior:
When a saved query is reloaded using the Saved Query management popover and changes have been made to the saved query, the changes persist.
New behavior:
Any changes made to the loaded saved query when reloaded are discarded.
Checklist
Use
strikethroughsto remove checklist items you don't feel are applicable to this PR.- [ ] This was checked for cross-browser compatibility, including a check against IE11- [ ] Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support- [ ] Documentation was added for features that require explanation or tutorials- [ ] This was checked for keyboard-only and screenreader accessibilityFor maintainers
- [ ] This was checked for breaking API changes and was labeled appropriately- [ ] This includes a feature addition or change that requires a release note and was labeled appropriately