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

Uiactions to navigate to visualize or maps #74121

Merged
merged 38 commits into from
Aug 19, 2020

Conversation

stratoula
Copy link
Contributor

@stratoula stratoula commented Aug 3, 2020

Summary

Closes #70178.

Changes the way that discover navigates to Visualize or Maps apps. We use two triggers:

  • VISUALIZE_FIELD for handling the navigation to visualize editor (except geo field)
  • VISUALIZE_GEO_FIELDfor handling the navigation to tilemaps or maps app

Both tilemaps and maps app register to VISUALIZE_GEO_FIELD. It is checked whether the maps app is enabled and the tilemaps is hidden or not. If it is not hidden and the maps app is enabled the user has two options to visualize a geo field. Otherwise, the Visualize button will navigate the user to the plugin that is compatible.

Update: As tilemaps are going to be deprecated we decided to not register any actions for legacy mapping visualizations.
This means that if the maps app is not enabled, the visualize button for geo fields won't appear on Discover.

Except for that, the previous functionality is the same. We had functional tests to check the navigation from discover to visualize and from discover to the maps app. I added a new one to check the navigation to tilemaps.

I have also run a flaky test server https://kibana-ci.elastic.co/job/kibana+flaky-test-suite-runner/741/

Checklist

@stratoula stratoula changed the title Uiactions navigate to visualize Uiactions navigate to visualize or maps Aug 4, 2020
@stratoula stratoula changed the title Uiactions navigate to visualize or maps Uiactions to navigate to visualize or maps Aug 4, 2020
@stratoula stratoula marked this pull request as ready for review August 4, 2020 13:17
@stratoula stratoula requested a review from a team August 4, 2020 13:17
@stratoula stratoula requested review from a team as code owners August 4, 2020 13:17
@stratoula stratoula added the Team:Visualizations Visualization editors, elastic-charts and infrastructure label Aug 4, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app (Team:KibanaApp)

Comment on lines 76 to 80
declare module '../../../../src/plugins/ui_actions/public' {
export interface ActionContextMapping {
[ACTION_VISUALIZE_GEO_FIELD]: VisualizeFieldContext;
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

@stratoula could we then so far move that type declaraction into the plugin where it's defined?

src/plugins/visualize/common/constants.ts Outdated Show resolved Hide resolved
@stratoula
Copy link
Contributor Author

@elasticmachine merge upstream

@kertal kertal self-requested a review August 19, 2020 08:29
Copy link
Member

@kertal kertal left a comment

Choose a reason for hiding this comment

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

Code LGTM 👍 it's much better this way, thx for this cleanup and improvement! Tested locally in Firefox, Chrome, Safari, also with a role with only Discover enabled. No regressions detected.

@kibanamachine
Copy link
Contributor

💛 Build succeeded, but was flaky


Test Failures

Plugin Functional Tests.test/plugin_functional/test_suites/core/route·ts.core route timeouts idle socket should timeout if servers response is too slow

Link to Jenkins

Standard Out

Failed Tests Reporter:
  - Test has not failed recently on tracked branches

[00:00:00]       │
[00:00:00]         └-: core
[00:00:00]           └-> "before all" hook
[00:00:00]           └-: route
[00:00:00]             └-> "before all" hook
[00:00:00]             └-: timeouts
[00:00:00]               └-> "before all" hook
[00:00:00]               └-: idle socket
[00:00:00]                 └-> "before all" hook
[00:00:00]                 └-> should timeout if payload sending has too long of an idle period
[00:00:00]                   └-> "before each" hook: global before each
[00:00:00]                   └- ✓ pass  (52ms) "core route timeouts idle socket should timeout if payload sending has too long of an idle period"
[00:00:00]                 └-> should not timeout if payload sending does not have too long of an idle period
[00:00:00]                   └-> "before each" hook: global before each
[00:00:00]                   └- ✓ pass  (223ms) "core route timeouts idle socket should not timeout if payload sending does not have too long of an idle period"
[00:00:00]                 └-> should timeout if servers response is too slow
[00:00:00]                   └-> "before each" hook: global before each
[00:00:00]                   │ info Taking screenshot "/dev/shm/workspace/parallel/3/kibana/test/functional/screenshots/failure/core route timeouts idle socket should timeout if servers response is too slow.png"
[00:00:01]                   │ info Current URL is: data:/,
[00:00:01]                   │ info Saving page source to: /dev/shm/workspace/parallel/3/kibana/test/plugin_functional/failure_debug/html/core route timeouts idle socket should timeout if servers response is too slow.html
[00:00:01]                   └- ✖ fail: core route timeouts idle socket should timeout if servers response is too slow
[00:00:01]                   │      Error: expected 'read ECONNRESET' to equal 'socket hang up'
[00:00:01]                   │       at Assertion.assert (packages/kbn-expect/expect.js:100:11)
[00:00:01]                   │       at Assertion.be.Assertion.equal (packages/kbn-expect/expect.js:227:8)
[00:00:01]                   │       at Assertion.be (packages/kbn-expect/expect.js:69:22)
[00:00:01]                   │       at result.then.err (test/plugin_functional/test_suites/core/route.ts:147:38)
[00:00:01]                   │       at process._tickCallback (internal/process/next_tick.js:68:7)
[00:00:01]                   │ 
[00:00:01]                   │ 

Stack Trace

Error: expected 'read ECONNRESET' to equal 'socket hang up'
    at Assertion.assert (packages/kbn-expect/expect.js:100:11)
    at Assertion.be.Assertion.equal (packages/kbn-expect/expect.js:227:8)
    at Assertion.be (packages/kbn-expect/expect.js:69:22)
    at result.then.err (test/plugin_functional/test_suites/core/route.ts:147:38)
    at process._tickCallback (internal/process/next_tick.js:68:7)

Build metrics

@kbn/optimizer bundle module count

id value diff baseline
discover 258 -7 265
maps 695 +2 693
uiActions 34 +2 32
visualize 315 +4 311
total +1

async chunks size

id value diff baseline
discover 430.7KB -5.5KB 436.3KB
maps 3.3MB -7.4KB 3.3MB
visualize 693.8KB +1.0B 693.8KB
total -12.9KB

page load bundle size

id value diff baseline
discover 229.3KB +373.0B 229.0KB
maps 296.4KB +17.5KB 278.9KB
uiActions 218.6KB +3.7KB 214.9KB
visualize 41.3KB +10.3KB 31.0KB
total +31.9KB

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@stratoula stratoula merged commit b6c4757 into elastic:master Aug 19, 2020
stratoula added a commit to stratoula/kibana that referenced this pull request Aug 19, 2020
* Navigate from discover to visualize with registering into a trigger

* Implement the VISUALIZE_FIELD action

* Implementation of the maps app trigger actions with the isCompatible functionality

* clean up discover code and tile map action implementation

* Add typeIsHidden on mocks

* Retrieve filters and query from url state

* functional test for oss and tile map

* include geoshape

* fix functional tests

* fix types

* remove unecessary dependencies

* minor fixes

* Remove tilemaps actios as it is going tobe deprecated

* Add useEffect on discover details and move the map action to a separate folder

* Retrieve map tooltips info from context

* Retrieve query and filters from QueryService

* Building urls with urlGenerators

* replace with constants, fetch initialLayers as array

* remove irrelevant comments

* nice improvements

* Return contextualFields for both triggers

* Add getHref on actions, move capabilities to isCompatible method per action and other fixes

* fix type

* Fix type incompatibility after merging with master

* fixes on maps plugin file after merge

* remove unecessary declarations

* nice improvements

* Refactor maps services code to be inline with master

Co-authored-by: Elastic Machine <[email protected]>
stratoula added a commit that referenced this pull request Aug 19, 2020
* Navigate from discover to visualize with registering into a trigger

* Implement the VISUALIZE_FIELD action

* Implementation of the maps app trigger actions with the isCompatible functionality

* clean up discover code and tile map action implementation

* Add typeIsHidden on mocks

* Retrieve filters and query from url state

* functional test for oss and tile map

* include geoshape

* fix functional tests

* fix types

* remove unecessary dependencies

* minor fixes

* Remove tilemaps actios as it is going tobe deprecated

* Add useEffect on discover details and move the map action to a separate folder

* Retrieve map tooltips info from context

* Retrieve query and filters from QueryService

* Building urls with urlGenerators

* replace with constants, fetch initialLayers as array

* remove irrelevant comments

* nice improvements

* Return contextualFields for both triggers

* Add getHref on actions, move capabilities to isCompatible method per action and other fixes

* fix type

* Fix type incompatibility after merging with master

* fixes on maps plugin file after merge

* remove unecessary declarations

* nice improvements

* Refactor maps services code to be inline with master

Co-authored-by: Elastic Machine <[email protected]>

Co-authored-by: Elastic Machine <[email protected]>
gmmorris added a commit to gmmorris/kibana that referenced this pull request Aug 19, 2020
* master: (112 commits)
  [Ingest Manager] Fix agent config rollout rate limit to use constants (elastic#75364)
  Update Node.js to version 10.22.0 (elastic#75254)
  [ML] Anomaly Explorer / Single Metric Viewer: Fix error reporting for annotations. (elastic#74953)
  [Discover] Fix histogram cloud tests (elastic#75268)
  Uiactions to navigate to visualize or maps (elastic#74121)
  Use prefix search invis editor field/agg combo box (elastic#75290)
  Fix docs in trigger alerting UI (elastic#75363)
  [SIEM] Fixes search bar Cypress test (elastic#74833)
  Add libnss3.so to Dockerfile template (reporting) (elastic#75370)
  [Discover] Create field_button and add popovers to sidebar (elastic#73226)
  [Reporting] Network Policy: Do not throw from the intercept handler (elastic#75105)
  [Reporting] Increase capture.timeouts.openUrl to 1 minute (elastic#75207)
  Allow routes to specify the idle socket timeout in addition to the payload timeout (elastic#73730)
  [src/dev/build] remove node-version from snapshots (elastic#75303)
  [ENDPOINT] Reintroduced tabs to endpoint management and migrated pages to use common security components (elastic#74886)
  [Canvas] Remove dependency on legacy expressions APIs (elastic#74885)
  Skip failing test in CI (elastic#75266)
  [Task Manager] time out work when it overruns in poller (elastic#74980)
  [Drilldowns] misc improvements & fixes (elastic#75276)
  Small README note on bumping memory for builds (elastic#75247)
  ...
gmmorris added a commit to gmmorris/kibana that referenced this pull request Aug 19, 2020
* master: (30 commits)
  [code coverage] always download node before team assignment (elastic#75424)
  [Form lib] Allow new "defaultValue" to be provided when resetting the… (elastic#75302)
  [Logs UI] Add "View in machine learning" links in the anomaly explorer (elastic#74555)
  skip flaky suite (elastic#75440)
  skip flaky suite (elastic#75386)
  [Saved objects] Add support for version on create & bulkCreate when overwriting a document (elastic#75172)
  [Functional]Table Vis increase sleep time in order filter to be applied (elastic#75138)
  MOAR RAM (elastic#75423)
  [Visualize] Horizontal Bar Percentiles Overlapping (elastic#75315)
  [ML] DF Analytics / Transforms: Fix job row actions menu invalid DOM nesting warning (elastic#74499)
  [ML] Inference models management (elastic#74978)
  [Monitoring] Migrate karma tests (elastic#75301)
  [Index template] Add filters to simulate preview (elastic#74497)
  Bump and consolidate dependencies (elastic#75360)
  [Ingest Manager] Fix agent config rollout rate limit to use constants (elastic#75364)
  Update Node.js to version 10.22.0 (elastic#75254)
  [ML] Anomaly Explorer / Single Metric Viewer: Fix error reporting for annotations. (elastic#74953)
  [Discover] Fix histogram cloud tests (elastic#75268)
  Uiactions to navigate to visualize or maps (elastic#74121)
  Use prefix search invis editor field/agg combo box (elastic#75290)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Discover Discover Application Feature:Vis Editor Visualization editor issues release_note:enhancement Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.10.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Using uiActions to navigate from discover to visualize
9 participants