Migrate Coordinate Maps to NP #64668

kindsun · 2020-04-28T15:19:54Z

Updates Coordinate Maps dependencies and migrates it to the New Platform. This PR also removes the file containing the mapConfig declaration originally located in src/legacy/core_plugins/tile_map/index.ts. This necessitated updates to the new Maps app which also relies heavily on mapConfig.

I originally planned to rename the plugin to coordinate_maps as part of this PR but will handle that separately at a later point. There are also several config values under tilemap, so ideally these + any other renaming would be handled together.

Related issue: #64810

elasticmachine · 2020-04-28T15:19:56Z

Pinging @elastic/kibana-gis (Team:Geo)

… loaded yet in NP

…nate-maps

thomasneirynck

I only have a few nits. Tested and works. Really great..

thomasneirynck · 2020-04-29T22:22:27Z

.i18nrc.json

@@ -42,7 +42,7 @@
      "src/plugins/telemetry",
      "src/plugins/telemetry_management_section"
    ],
-    "tileMap": "src/legacy/core_plugins/tile_map",
+    "tileMap": "src/plugins/tile_map",


src/plugins/maps_legacy/public/map/service_settings.js

src/plugins/maps_legacy/public/plugin.ts

x-pack/plugins/maps/public/plugin.ts

flash1293 · 2020-05-05T14:29:50Z

@thomasneirynck Did you test with the kibana_dashboard_only_user role? This file is only hit when your user has that role associated.

thomasneirynck · 2020-05-05T14:42:21Z

@flash1293 thx for headsup! I tested the removal of that import with the kibana_dashboard_only_user role (and read priviliges on index) and works fine.

kindsun · 2020-05-05T15:00:32Z

Thanks @flash1293 for the user role info! I've also tested this locally and found no issues with dashboards loading leaflet as a user with this role.

Just to give you a heads up, it looks like this PR flagged the kibana-app team due to my removal of another leaflet import in src/legacy/core_plugins/kibana/public/kibana.js. This import was actually the source of the bulk of the issues preventing leaflet plugins from loading correctly in NP. The legacy maps plugin structure has changed substantially since that import was originally added. All legacy maps plugins now leverage the maps_legacy plugin which loads leaflet and the needed plugins. Other leaflet imports are no longer necessary, and in fact in a post-webpackShim world, actually cause problems. Let me know if you need any clarification, happy to do a deeper dive!

…l way (tests have no access to the window object)

…nate-maps

thomasneirynck

👏

flash1293 · 2020-05-06T16:16:01Z

src/plugins/vis_type_vega/public/vega_view/vega_map_view.js

@@ -102,6 +102,7 @@ export class VegaMapView extends VegaBaseView {
    // let maxBounds = null;
    // if (mapConfig.maxBounds) {
    //   const b = mapConfig.maxBounds;
+    // eslint-disable-next-line no-undef


Is this necessary? Seems like this commented out code can be removed anyway.

It was on the chance someone ever uncommented it. I don't know the history of why it was commented out and left in. In general I think retaining commented code is unnecessary (that's what git is for, seeing code history) but I didn't touch it otherwise since it wasn't the focus of this PR

flash1293

I tried to test this but I'm always running into "Elastic Kibana did not load properly. Check the server output for more information.". Not sure whether it's related to this PR or my environment broke somehow, I will re-setup tomorrow and test again. Bootstrapping didn't help.

I'm going to approve because the Kibana App changes LGTM, but it's only code review at this point.

If you are certain that broken Kibana is related to my environment and not the PR feel free to merge if it's blocking you somehow and I will verify after the fact tomorrow.

kindsun · 2020-05-06T16:31:20Z

I tried to test this but I'm always running into "Elastic Kibana did not load properly. Check the server output for more information.". Not sure whether it's related to this PR or my environment broke somehow, I will re-setup tomorrow and test again.

Thanks for the review! @flash1293 This sounds likely to be an environmental issue since tests are passing and neither myself nor @thomasneirynck have seen any issues locally. I'm fairly confident we should be in good shape. Per your comment we do also need these changes, specifically those related to map config, for 2 other PRs. I don't believe we'll have any problems, but definitely feel free to follow up if you see any further issues!

kindsun · 2020-05-06T17:26:23Z

@elasticmachine merge upstream

…nate-maps

kibanamachine · 2020-05-06T21:04:19Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 46ad010

History

💚 Build #46041 succeeded f4af673
💔 Build #46017 failed 098d419
💚 Build #45800 succeeded 098d419
💔 Build #45771 failed 9ef70a2
💔 Build #45682 failed 83e467d

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

* master: (42 commits) fix SavedObjectMigrationMap type (elastic#65569) [Uptime] Improve cert flaky test (elastic#65458) [Uptime] Fix monitor list result runtime type, ip can be null (elastic#65532) [APM] Agent configuration: Bug makes it possible to create invalid configurations (elastic#65508) [APM] Remove link from active page in the breadcrumb (elastic#65473) [SIEM] Fixes test flakiness (elastic#65510) [ESLint] update @kbn/eslint/no-restricted-paths rule to allow imports mocks from folder (elastic#65471) Migrate test plugins ⇒ NP (kbn_tp_run_pipeline) (elastic#64780) move core provier to NP. allows to run tests on every page (elastic#64929) Extended alerting documentation with information about using Kibana keystore and action types for preconfigured connectors (elastic#65201) [functional tests] add some missing awaits (elastic#65566) Fixed create new connector from alert flyout form throw an error messages in external plugins. (elastic#65539) [SIEM] [Cases] External services not getting all comments bug fix (elastic#65307) Migrate Coordinate Maps to NP (elastic#64668) Updating Canvas location in nav (elastic#65519) [SIEM][Lists] Fixes up contracts to work outside of requests [Lens] Remove "inside only" option for treemap labels (elastic#65363) [Uptime] Add TLS alert functional test (elastic#65303) Fix z-index of kbnLoadingIndicator (elastic#65521) Fixed indice assertion to loop through expected keys (elastic#64684) ...

kindsun added Feature:Coordinate Map [Deprecated-Use Team:Presentation]Team:Geo Former Team Label for Geo Team. Now use Team:Presentation v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Feature:NP Migration v7.8.0 labels Apr 28, 2020

kindsun force-pushed the np-migrate-coordinate-maps branch 2 times, most recently from b87d8ae to 99efbbf Compare April 28, 2020 15:50

Aaron Caldwell added 5 commits April 28, 2020 19:51

Use NP format service

8f9984e

Use NP data query service

9848713

Mostly file movement, a few updates. Some work towards using NP config

b66704f

Import leaflet draw to avoid timing issue that prevents it from being…

6a4c970

… loaded yet in NP

Publish configs for maps_legacy and tile_maps using NP format

151ddc9

kindsun force-pushed the np-migrate-coordinate-maps branch from 99efbbf to 151ddc9 Compare April 29, 2020 01:52

Aaron Caldwell added 3 commits April 29, 2020 07:46

Switch new maps over to using NP map config

47aec38

Consume mapsLegacy config on Maps server

a53a452

Type fixes. Schema updates

5561154

kindsun marked this pull request as ready for review April 29, 2020 17:12

kindsun requested review from a team as code owners April 29, 2020 17:12

kindsun requested a review from thomasneirynck April 29, 2020 17:12

kindsun mentioned this pull request Apr 29, 2020

[Maps] Decouple maps and maps_legacy config dependencies #64810

Closed

Aaron Caldwell added 3 commits April 29, 2020 13:33

Add new plugin path to i18n

d3bae1c

Mock configs for karma test service settings

1855529

Merge remote-tracking branch 'upstream/master' into np-migrate-coordi…

bb3404c

…nate-maps

thomasneirynck reviewed Apr 29, 2020

View reviewed changes

kindsun mentioned this pull request Apr 30, 2020

Migrate Region Maps to NP #64272

Merged

Aaron Caldwell added 2 commits April 30, 2020 07:24

Fix jest tests

05b672b

Update test files to snake case

c0154b3

Remove one more unneeded leaflet ref in dashboard

61b1c01

Aaron Caldwell added 2 commits May 5, 2020 10:16

Uno mas

3d7a38e

Import leaflet for vega tests since it can't be initialized the norma…

83e467d

…l way (tests have no access to the window object)

kindsun added v7.9.0 and removed v7.8.0 labels May 5, 2020

Aaron Caldwell added 3 commits May 5, 2020 16:46

Update browser tests service settings instantiation to include configs

8f861ce

Merge remote-tracking branch 'upstream/master' into np-migrate-coordi…

9ef70a2

…nate-maps

Get rid of unused module.exports

098d419

kindsun requested a review from thomasneirynck May 6, 2020 13:57

thomasneirynck approved these changes May 6, 2020

View reviewed changes

flash1293 reviewed May 6, 2020

View reviewed changes

flash1293 approved these changes May 6, 2020

View reviewed changes

kindsun mentioned this pull request May 6, 2020

Timeline for removing legacy maps plugins #65520

Closed

Merge branch 'master' into np-migrate-coordinate-maps

f4af673

kindsun mentioned this pull request May 6, 2020

Complete transition of Kibana Legacy Maps to NP #60325

Closed

4 tasks

Merge remote-tracking branch 'upstream/master' into np-migrate-coordi…

46ad010

…nate-maps

kindsun merged commit c00b36e into elastic:master May 6, 2020

kindsun deleted the np-migrate-coordinate-maps branch May 6, 2020 21:12

kindsun mentioned this pull request May 6, 2020

[7.x] Migrate Coordinate Maps to NP (#64668) #65581

Merged

kindsun pushed a commit to kindsun/kibana that referenced this pull request May 6, 2020

Migrate Coordinate Maps to NP (elastic#64668)

614c7e1

kindsun pushed a commit that referenced this pull request May 7, 2020

[7.x] Migrate Coordinate Maps to NP (#64668) (#65581)

8955488

thomasneirynck mentioned this pull request May 29, 2020

[Maps][File upload] Migrate Maps team apps to new platform #33514

Closed

26 tasks

kindsun mentioned this pull request Jun 23, 2020

Remove injected vars from maps legacy #69744

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate Coordinate Maps to NP #64668

Migrate Coordinate Maps to NP #64668

kindsun commented Apr 28, 2020 •

edited

Loading

elasticmachine commented Apr 28, 2020

thomasneirynck left a comment

thomasneirynck Apr 29, 2020

flash1293 commented May 5, 2020

thomasneirynck commented May 5, 2020

kindsun commented May 5, 2020 •

edited

Loading

thomasneirynck left a comment

flash1293 May 6, 2020

kindsun May 6, 2020

flash1293 left a comment •

edited

Loading

kindsun commented May 6, 2020

kindsun commented May 6, 2020

kibanamachine commented May 6, 2020

Migrate Coordinate Maps to NP #64668

Migrate Coordinate Maps to NP #64668

Conversation

kindsun commented Apr 28, 2020 • edited Loading

elasticmachine commented Apr 28, 2020

thomasneirynck left a comment

Choose a reason for hiding this comment

thomasneirynck Apr 29, 2020

Choose a reason for hiding this comment

flash1293 commented May 5, 2020

thomasneirynck commented May 5, 2020

kindsun commented May 5, 2020 • edited Loading

thomasneirynck left a comment

Choose a reason for hiding this comment

flash1293 May 6, 2020

Choose a reason for hiding this comment

kindsun May 6, 2020

Choose a reason for hiding this comment

flash1293 left a comment • edited Loading

Choose a reason for hiding this comment

kindsun commented May 6, 2020

kindsun commented May 6, 2020

kibanamachine commented May 6, 2020

💚 Build Succeeded

History

kindsun commented Apr 28, 2020 •

edited

Loading

kindsun commented May 5, 2020 •

edited

Loading

flash1293 left a comment •

edited

Loading