-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
correctly passing saved filters and query to all response handlers #32749
Conversation
This comment has been minimized.
This comment has been minimized.
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.
Tested with new pipeline and everything works. I noticed that we weren't excluding disabled saved filters, so I pushed an update for that.
Ideally I'd like to do a functional test for this too; I started looking into it, but there's very little in place so far for vega. I'll try to find time to add this if possible.
💔 Build Failed |
e00c184
to
116763a
Compare
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! Added a functional test.
Not sure what's up with the build failures - there are a lot of them but they look unrelated. (Cannot find module 'csstype'.
)
I rebased on master and pushed the latest to see if that helps.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
💔 Build Failed |
seems a related test is failing:
|
116763a
to
dd101c2
Compare
@ppisljar Yeah that was the test that I added -- cross-browser rendering differences make screenshot comparison in functional tests hard to do. I tried a different approach that hopefully will work this time around. |
💚 Build Succeeded |
retest (just to be safe) |
💚 Build Succeeded |
Pinging @elastic/kibana-app |
Summary
resolves #29836
Summarize your PR. If it involves visual changes include a screenshot or gif.
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](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#release-notes-process)[ ] This includes a feature addition or change that requires a release note and was labeled appropriately