[Region Map] Shim new Platform #40966

gospodarsky · 2019-07-12T11:22:40Z

Summary

First step of migrating to the new Platform

What was done in this PR:

Have a new platform-style plugin definition in typescript, with a setup method returning the public contract (see data plugin as an example). This needs to be exported from the top-level /server and/or /public directory, e.g. export foo = new Plugin.setup()
Be broken into logical "services", each with a service definition class (see data plugin as an example)
Register the visualization via the "visualizations" plugin's types service.
Have all static exports done from the top level plugin definition

elasticmachine · 2019-07-12T12:08:26Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-07-12T14:37:01Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-07-15T10:15:39Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-07-16T09:24:17Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-07-22T15:16:26Z

Pinging @elastic/kibana-gis

elasticmachine · 2019-07-23T14:31:17Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

alexwizp · 2019-07-24T13:18:25Z

src/legacy/core_plugins/region_map/public/region_map_vis_params.js

 import { toastNotifications } from 'ui/notify';
 import regionMapVisParamsTemplate from './region_map_vis_params.html';
 import { mapToLayerWithId } from './util';
-import '../../tile_map/public/editors/wms_options';
+
+// TODO: reference to TILE_MAP plugin should be removed
 import { ORIGIN } from '../../../../legacy/core_plugins/tile_map/common/origin';


@lukeelmers @nreese
In this PR I've marked all places where we do an import from tile_map plugin. Not sure that we need to fix it now because in future we are going to create one common plugin for tile_map and region_map

I agree this is probably outside the scope of this PR.

We could open another PR that exports it (and any other stuff imported from it) from tile_map/public/index if you think it will make it easier to remember later.

But I don't think it's critical to address right now unless @nreese feels otherwise.

elasticmachine · 2019-07-24T15:29:24Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

src/legacy/core_plugins/region_map/public/plugin.ts

lukeelmers · 2019-07-24T21:16:29Z

src/legacy/core_plugins/region_map/public/region_map_vis_params.js

 import { toastNotifications } from 'ui/notify';
 import regionMapVisParamsTemplate from './region_map_vis_params.html';
 import { mapToLayerWithId } from './util';
-import '../../tile_map/public/editors/wms_options';
+
+// TODO: reference to TILE_MAP plugin should be removed
 import { ORIGIN } from '../../../../legacy/core_plugins/tile_map/common/origin';


I agree this is probably outside the scope of this PR.

We could open another PR that exports it (and any other stuff imported from it) from tile_map/public/index if you think it will make it easier to remember later.

But I don't think it's critical to address right now unless @nreese feels otherwise.

lukeelmers · 2019-07-24T21:17:45Z

src/legacy/core_plugins/region_map/public/region_map_visualization.js

+import { TileMapTooltipFormatter } from './tooltip_formatter';
+
+// TODO: reference to TILE_MAP plugin should be removed
+import { BaseMapsVisualizationProvider } from '../../tile_map/public/base_maps_visualization';


This is another thing we could potentially export directly from tile_map/public

We could open another PR that exports it (and any other stuff imported from it) from tile_map/public/index if you think it will make it easier to remember later.

Yes, that can be addressed in another PR. Maybe tile map and region map should be in the same plugin if region map needs a bunch of stuff from tile map but that can be considered in another PR as well

@nreese @thomasneirynck Yeah our idea in the new platform migration plans was to have a single vis_type_maps plugin that could register both of the maps visualizations since they are sharing some code. But y'all would probably be better judges of the feasibility of that. In the interim we can just keep them shimmed separately like this.

elasticmachine · 2019-07-25T09:41:17Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

thomasneirynck

thx @Avinar-24

I tested this and looks good to me, but not comfortable enough to speak to the actual changes wrt new platform.

src/legacy/core_plugins/region_map/public/choropleth_layer.js

thomasneirynck · 2019-07-25T14:05:25Z

src/legacy/core_plugins/tile_map/public/__tests__/coordinate_maps_visualization.js

      setupDOM('512px', '512px');

      imageComparator = new ImageComparator();
      vis = new Vis(indexPattern, {
-        type: 'region_map'
+        type: 'tile_map'


Thanks for fixing. This was nasty. This just used to work because it was using the vis-object only for the configuration params when calling new CoordinateMapsVisualization(domNode, vis).

elasticmachine · 2019-07-25T16:05:35Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-07-25T19:18:51Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-07-26T10:56:02Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

lukeelmers

LGTM in terms of new platform prep!

Edit: let's wait for a final approval from @thomasneirynck or @nreese though

lukeelmers · 2019-07-26T17:36:51Z

src/legacy/core_plugins/region_map/public/region_map_visualization.js

+import { TileMapTooltipFormatter } from './tooltip_formatter';
+
+// TODO: reference to TILE_MAP plugin should be removed
+import { BaseMapsVisualizationProvider } from '../../tile_map/public/base_maps_visualization';


@nreese @thomasneirynck Yeah our idea in the new platform migration plans was to have a single vis_type_maps plugin that could register both of the maps visualizations since they are sharing some code. But y'all would probably be better judges of the feasibility of that. In the interim we can just keep them shimmed separately like this.

thomasneirynck

lgtm

@lukeelmers @Avinar-24 I like the idea of single plugin registering both region and coordinate map visualizations. +1 on separate PR for that.

* Migrate Region Map to New Platform * Replace Vis Factory * fix plugin name - work in progress * code cleanup * fix tests for tile_map * fix PR comments * fix PR comments

Migrate Region Map to New Platform

801d26c

gospodarsky added Feature:Region Map release_note:enhancement labels Jul 12, 2019

Artyom Gospodarsky added 2 commits July 12, 2019 15:12

Replace Vis Factory

058e9e4

Merge remote-tracking branch 'upstream/master' into region-map-migration

20c631c

Merge remote-tracking branch 'upstream/master' into region-map-migration

1612276

Merge remote-tracking branch 'upstream/master' into region-map-migration

236c8a7

lukeelmers added the [Deprecated-Use Team:Presentation]Team:Geo Former Team Label for Geo Team. Now use Team:Presentation label Jul 22, 2019

lukeelmers added the Feature:New Platform label Jul 22, 2019

lukeelmers mentioned this pull request Jul 22, 2019

App Architecture roadmap for New Platform migration #31968

Closed

thomasneirynck self-requested a review July 22, 2019 16:55

Merge branch 'master' into region-map-migration

3497117

alexwizp changed the title ~~[Region Map] Shim new platform~~ [WIP][Region Map] Shim new platform Jul 23, 2019

fix plugin name - work in progress

599beab

alexwizp added the WIP Work in progress label Jul 23, 2019

Merge remote-tracking branch 'upstream/master' into region-map-migration

f97f2bc

alexwizp self-assigned this Jul 24, 2019

alexwizp added v7.4.0 v8.0.0 release_note:skip Skip the PR/issue when compiling release notes and removed WIP Work in progress release_note:enhancement labels Jul 24, 2019

alexwizp changed the title ~~[WIP][Region Map] Shim new platform~~ [Tile Map] Shim new Platform Jul 24, 2019

alexwizp reviewed Jul 24, 2019

View reviewed changes

alexwizp added 2 commits July 24, 2019 16:32

code cleanup

47cffc4

fix tests for tile_map

4f16437

elastic deleted a comment from elasticmachine Jul 24, 2019

lukeelmers reviewed Jul 24, 2019

View reviewed changes

alexwizp added 2 commits July 25, 2019 11:38

Merge branch 'master' into region-map-migration

1ca9a1a

fix PR comments

6269e30

thomasneirynck reviewed Jul 25, 2019

View reviewed changes

fix PR comments

f44e4db

alexwizp approved these changes Jul 25, 2019

View reviewed changes

alexwizp requested review from thomasneirynck and lukeelmers July 25, 2019 15:07

lukeelmers added Feature:NP Migration and removed Feature:New Platform labels Jul 25, 2019

Merge remote-tracking branch 'upstream/master' into region-map-migration

f836a75

Merge remote-tracking branch 'upstream/master' into region-map-migration

3d32c5d

lukeelmers approved these changes Jul 26, 2019

View reviewed changes

thomasneirynck approved these changes Jul 26, 2019

View reviewed changes

alexwizp merged commit f84f07b into elastic:master Jul 26, 2019

alexwizp mentioned this pull request Jul 26, 2019

[7.x] [Region Map] Shim new Platform (#40966) #42091

Merged

thomasneirynck mentioned this pull request Nov 25, 2019

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

Closed

26 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Region Map] Shim new Platform #40966

[Region Map] Shim new Platform #40966

gospodarsky commented Jul 12, 2019 •

edited by alexwizp

Loading

elasticmachine commented Jul 12, 2019

elasticmachine commented Jul 12, 2019

elasticmachine commented Jul 15, 2019

elasticmachine commented Jul 16, 2019

elasticmachine commented Jul 22, 2019

elasticmachine commented Jul 23, 2019

alexwizp Jul 24, 2019 •

edited

Loading

lukeelmers Jul 24, 2019

elasticmachine commented Jul 24, 2019

lukeelmers Jul 24, 2019

lukeelmers Jul 24, 2019

nreese Jul 25, 2019

lukeelmers Jul 26, 2019

elasticmachine commented Jul 25, 2019

thomasneirynck left a comment

thomasneirynck Jul 25, 2019

elasticmachine commented Jul 25, 2019

elasticmachine commented Jul 25, 2019

elasticmachine commented Jul 26, 2019

lukeelmers left a comment •

edited

Loading

lukeelmers Jul 26, 2019

thomasneirynck left a comment

[Region Map] Shim new Platform #40966

[Region Map] Shim new Platform #40966

Conversation

gospodarsky commented Jul 12, 2019 • edited by alexwizp Loading

Summary

elasticmachine commented Jul 12, 2019

💔 Build Failed

elasticmachine commented Jul 12, 2019

💔 Build Failed

elasticmachine commented Jul 15, 2019

💔 Build Failed

elasticmachine commented Jul 16, 2019

💔 Build Failed

elasticmachine commented Jul 22, 2019

elasticmachine commented Jul 23, 2019

💔 Build Failed

alexwizp Jul 24, 2019 • edited Loading

Choose a reason for hiding this comment

lukeelmers Jul 24, 2019

Choose a reason for hiding this comment

elasticmachine commented Jul 24, 2019

💚 Build Succeeded

lukeelmers Jul 24, 2019

Choose a reason for hiding this comment

lukeelmers Jul 24, 2019

Choose a reason for hiding this comment

nreese Jul 25, 2019

Choose a reason for hiding this comment

lukeelmers Jul 26, 2019

Choose a reason for hiding this comment

elasticmachine commented Jul 25, 2019

💚 Build Succeeded

thomasneirynck left a comment

Choose a reason for hiding this comment

thomasneirynck Jul 25, 2019

Choose a reason for hiding this comment

elasticmachine commented Jul 25, 2019

💔 Build Failed

elasticmachine commented Jul 25, 2019

💔 Build Failed

elasticmachine commented Jul 26, 2019

💚 Build Succeeded

lukeelmers left a comment • edited Loading

Choose a reason for hiding this comment

lukeelmers Jul 26, 2019

Choose a reason for hiding this comment

thomasneirynck left a comment

Choose a reason for hiding this comment

gospodarsky commented Jul 12, 2019 •

edited by alexwizp

Loading

alexwizp Jul 24, 2019 •

edited

Loading

lukeelmers left a comment •

edited

Loading