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

visualize should apply saved query and filters #12789

Merged
merged 1 commit into from
Jul 13, 2017

Conversation

ppisljar
Copy link
Member

closes #12784

@ppisljar ppisljar added Feature:Visualizations Generic visualization features (in case no more specific feature label is available) review v6.0.0 labels Jul 12, 2017
@thomasneirynck
Copy link
Contributor

jenkins, test this

@thomasneirynck
Copy link
Contributor

thomasneirynck commented Jul 12, 2017

CI failed twice on same test: https://kibana-ci.elastic.co/job/elastic+kibana+pull-request+multijob-selenium/5969/console
https://kibana-ci.elastic.co/job/elastic+kibana+pull-request+multijob-selenium/5964/console

 ✖ fail: "management scripted fields creating and using Lucence expression scripted fields should visualize scripted field in vertical bar chart"
02:27:59.746        │        [POST http://localhost:9515/session/1ea43470387cdc99c9c4f06e7ce6a316/element / {"using":"css selector","value":"[data-test-subj=\"paginateControlsPageSizeSelect\"] option[label=\"All\"]"}] no such element: Unable to locate element: {"method":"css selector","selector":"[data-test-subj="paginateControlsPageSizeSelect"] option[label="All"]"}
02:27:59.746        │         (Session info: chrome=59.0.3071.115)
02:27:59.746        │         (Driver info: chromedriver=2.29.461571 (8a88bbe0775e2a23afda0ceaf2ef7ee74e822cc5),platform=Linux 3.13.0-108-generic x86_64)
02:27:59.746        │         at Server._post (test/functional/services/remote/verbose_remote_logging.js:15:21)
02:27:59.747        │         at runRequest (node_modules/leadfoot/Session.js:92:40)
02:27:59.747        │         at node_modules/leadfoot/Session.js:113:39
02:27:59.747        │         at new Promise (node_modules/dojo/Promise.js:172:17)
02:27:59.747        │         at Session._post (node_modules/leadfoot/Session.js:67:10)
02:27:59.747        │         at Session.find (node_modules/leadfoot/Session.js:1304:15)
02:27:59.747        │         at Command.<anonymous> (node_modules/leadfoot/Command.js:42:36)
02:27:59.747        │         at node_modules/dojo/Promise.js:156:41
02:27:59.747        │         at run (node_modules/dojo/Promise.js:51:33)
02:27:59.750        │         at node_modules/dojo/nextTick.js:35:17
02:27:59.750        │         at _combinedTickCallback (internal/process/next_tick.js:73:7)
02:27:59.750        │         at process._tickDomainCallback (internal/process/next_tick.js:128:9)
02:27:59.751        │         at Command.find (node_modules/leadfoot/Command.js:23:10)
02:27:59.751        │         at Command.prototype.(anonymous function) [as findByCssSelector] (node_modules/leadfoot/lib/strategies.js:29:16)
02:27:59.751        │         at test/functional/page_objects/settings_page.js:270:10
02:27:59.751        │         at undefined.next (native)
02:27:59.751        │         at step (test/functional/page_objects/settings_page.js:1:1)
02:27:59.751        │         at test/functional/page_objects/settings_page.js:1:1
02:27:59.751        │         at test/functional/page_objects/settings_page.js:1:1
02:27:59.751        │         at SettingsPage.setPageSize (test/functional/page_objects/settings_page.js:268:29)
02:27:59.751        │         at Context.<anonymous> (test/functional/apps/management/_scripted_fields.js:101:36)
02:27:59.751        │         at undefined.next (native)
02:27:59.751        │         at step (test/functional/apps/management/_scripted_fields.js:14:1)
02:27:59.751        │         at test/functional/apps/management/_scripted_fields.js:14:1

@thomasneirynck
Copy link
Contributor

jenkins, test this

@thomasneirynck
Copy link
Contributor

failing on new test this time

fail: "dashboard app dashboard tab add new visualization link adds a new visualization"
08:17:28.300        │        [POST http://localhost:9515/session/581ffccb64199d5d6c322c00cc7129d1/element / {"using":"name","value":"visTitle"}] no such element: Unable to locate element: {"method":"name","selector":"visTitle"}
08:17:28.300        │         (Session info: chrome=59.0.3071.115)
08:17:28.300        │         (Driver info: chromedriver=2.29.461571 (8a88bbe0775e2a23afda0ceaf2ef7ee74e822cc5),platform=Linux 3.13.0-108-generic x86_64)
08:17:28.300        │         at Server._post (test/functional/services/remote/verbose_remote_logging.js:15:21)
08:17:28.300        │         at runRequest (node_modules/leadfoot/Session.js:92:40)
08:17:28.300        │         at node_modules/leadfoot/Session.js:113:39
08:17:28.300        │         at new Promise (node_modules/dojo/Promise.js:172:17)
08:17:28.300        │         at Session._post (node_modules/leadfoot/Session.js:67:10)
08:17:28.300        │         at Session.find (node_modules/leadfoot/Session.js:1304:15)
08:17:28.300        │         at Command.<anonymous> (node_modules/leadfoot/Command.js:42:36)
08:17:28.300        │         at node_modules/dojo/Promise.js:156:41
08:17:28.300        │         at run (node_modules/dojo/Promise.js:51:33)
08:17:28.300        │         at node_modules/dojo/nextTick.js:35:17
08:17:28.300        │         at _combinedTickCallback (internal/process/next_tick.js:73:7)
08:17:28.300        │         at process._tickDomainCallback (internal/process/next_tick.js:128:9)
08:17:28.300        │         at Command.find (node_modules/leadfoot/Command.js:23:10)
08:17:28.300        │         at Command.prototype.(anonymous function) [as findByName] (node_modules/leadfoot/lib/strategies.js:29:16)
08:17:28.300        │         at testSubjects.click.then.then (test/functional/page_objects/visualize_page.js:418:10)

https://kibana-ci.elastic.co/job/elastic+kibana+pull-request+multijob-selenium/5988/console

searchSource.set('filter', appState.filters);
if (!appState.linked) searchSource.set('query', appState.query);
searchSource.set('filter', _.union(searchSource.initialState.filter, appState.filters));
if (!appState.linked) searchSource.set('query', _.union([searchSource.initialState.query], [appState.query]));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should be merging this with a union of arrays. The problem is that we get other bugs:

  • saving twice, will then save the visualization with an array in the searchSourceJSON.
    image
  • a side-effect is that the query-string of the visualization can no longer be restored correctly in the UI control
    image
  • Another issue is, when the query is undefined, it gets merged into the object as null, creating malformed queries
  • This in turn causes errors with the query, breaking both dashboards and visualizations.

image

@thomasneirynck
Copy link
Contributor

thomasneirynck commented Jul 12, 2017

The test failures may be related to my comments above, but don't know 100% for sure. How did this even work pre-refactor?!? I can't really find an example of where the merging of these queries is governed.

@ppisljar ppisljar merged commit 3876ff0 into elastic:master Jul 13, 2017
@ppisljar ppisljar deleted the fix/savedQuery branch July 13, 2017 16:51
@ppisljar ppisljar restored the fix/savedQuery branch September 26, 2018 10:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Visualizations Generic visualization features (in case no more specific feature label is available) review v6.0.0-beta1 v6.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Visualization queries aren't applied on dashboard
2 participants