[Visualize] fix tile_map visualization#154392
Conversation
| this.bucketAggs.get(name) || this.metricAggs.get(name) | ||
| ), | ||
| get: (name: string) => { | ||
| const agg = this.bucketAggs.get(name) || this.metricAggs.get(name) || this.legacyAggs.get(name); |
There was a problem hiding this comment.
map.get can return undefined. Updating method to handle this case. This required lots of updates in tests and some updated types in lens
…-ref HEAD~1..HEAD --fix'
|
Pinging @elastic/kibana-presentation (Team:Presentation) |
|
Pinging @elastic/kibana-visualizations @elastic/kibana-visualizations-external (Team:Visualizations) |
|
Pinging @elastic/kibana-data-discovery (Team:DataDiscovery) |
|
@elasticmachine merge upstream |
drewdaemon
left a comment
There was a problem hiding this comment.
I'm not sure I see the utility of adding a new "legacy" agg category. Do we have a specific plan to remove this agg? If so, I think a reference to a Github issue in a comment would cut it. Any thoughts there?
I also left a suggestion regarding the change to the Agg types registry get API.
If the aggregation is put into BucketAggs, then it will be returned from The aggregation is registered in the Maps plugin along with the registration of the legacy tile_map and regionmap visualizations. That way, if they are ever removed, the aggregation registration can also be removed at the same time. |
|
@elasticmachine merge upstream |
nickpeihl
left a comment
There was a problem hiding this comment.
lgtm! code review and tested legacy tile map vis
thanks for adding the functional test!
drewdaemon
left a comment
There was a problem hiding this comment.
After some further discussion, I think the aggs changes make sense. Thank you!
💛 Build succeeded, but was flaky
Failed CI StepsMetrics [docs]Module Count
Async chunks
Page load bundle
Unknown metric groupsESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: |
Fixes elastic#154375 ### background elastic#105326 replaced tile_map visualization implementation with a new implementation that is a wrapper around MapEmbeddable. elastic#152952 removed geohash_grid aggregation. This causes a regression where existing tile_map visualizations no longer work. Even though geohash_grid aggregation is no longer used, the AggType is still needed so that new tile_map visualization wrapper can access aggregation configuration state. This PR adds back geohash_grid AggType in `legacyAggs` for this purpose. PR also adds a functional test to better prevent regressions with tile_map ### Test * install web logs sample data * install legacy tile_map saved objects from ``` {"attributes":{"fieldFormatMap":"{\"hour_of_day\":{}}","runtimeFieldMap":"{\"hour_of_day\":{\"type\":\"long\",\"script\":{\"source\":\"emit(doc['timestamp'].value.getHour());\"}}}","timeFieldName":"timestamp","title":"kibana_sample_data_logs"},"coreMigrationVersion":"7.17.5","id":"90943e30-9a47-11e8-b64d-95841ca0b247","migrationVersion":{"index-pattern":"7.11.0"},"references":[],"type":"index-pattern","updated_at":"2022-08-17T20:25:52.585Z","version":"WzEzMDQsMV0="} {"attributes":{"description":"","kibanaSavedObjectMeta":{"searchSourceJSON":"{\"query\":{\"language\":\"kuery\",\"query\":\"\"},\"filter\":[],\"indexRefName\":\"kibanaSavedObjectMeta.searchSourceJSON.index\"}"},"title":"region_map","uiStateJSON":"{}","version":1,"visState":"{\"aggs\":[{\"enabled\":true,\"id\":\"1\",\"params\":{},\"schema\":\"metric\",\"type\":\"count\"},{\"enabled\":true,\"id\":\"2\",\"params\":{\"field\":\"geo.dest\",\"missingBucket\":false,\"missingBucketLabel\":\"Missing\",\"order\":\"desc\",\"orderBy\":\"1\",\"otherBucket\":false,\"otherBucketLabel\":\"Other\",\"size\":5},\"schema\":\"segment\",\"type\":\"terms\"}],\"params\":{\"addTooltip\":true,\"colorSchema\":\"Yellow to Red\",\"emsHotLink\":\"https://maps.elastic.co/v7.16?locale=en#file/world_countries\",\"isDisplayWarning\":true,\"legendPosition\":\"bottomright\",\"mapCenter\":[0,0],\"mapZoom\":2,\"outlineWeight\":1,\"selectedJoinField\":{\"description\":\"ISO 3166-1 alpha-2 code\",\"name\":\"iso2\",\"type\":\"id\"},\"selectedLayer\":{\"attribution\":\"<a rel=\\\"noreferrer noopener\\\" href=\\\"http://www.naturalearthdata.com/about/terms-of-use\\\">Made with NaturalEarth</a> | <a rel=\\\"noreferrer noopener\\\" href=\\\"https://www.openstreetmap.org/copyright\\\">OpenStreetMap contributors</a> | <a rel=\\\"noreferrer noopener\\\" href=\\\"https://www.elastic.co/elastic-maps-service\\\">Elastic Maps Service</a>\",\"created_at\":\"2020-10-28T16:16:08.720286\",\"fields\":[{\"description\":\"ISO 3166-1 alpha-2 code\",\"name\":\"iso2\",\"type\":\"id\"},{\"description\":\"ISO 3166-1 alpha-3 code\",\"name\":\"iso3\",\"type\":\"id\"},{\"description\":\"ISO 3166-1 numeric code\",\"name\":\"iso_numeric\",\"type\":\"id\"},{\"description\":\"name\",\"name\":\"name\",\"type\":\"property\"}],\"format\":\"topojson\",\"id\":\"world_countries\",\"isEMS\":true,\"layerId\":\"elastic_maps_service.World Countries\",\"meta\":{\"feature_collection_path\":\"data\"},\"name\":\"World Countries\",\"origin\":\"elastic_maps_service\"},\"showAllShapes\":true,\"wms\":{\"enabled\":false,\"options\":{\"attribution\":\"\",\"format\":\"image/png\",\"layers\":\"\",\"styles\":\"\",\"transparent\":true,\"version\":\"\"},\"selectedTmsLayer\":{\"attribution\":\"<a rel=\\\"noreferrer noopener\\\" href=\\\"https://www.openstreetmap.org/copyright\\\">OpenStreetMap contributors</a> | <a rel=\\\"noreferrer noopener\\\" href=\\\"https://openmaptiles.org\\\">OpenMapTiles</a> | <a rel=\\\"noreferrer noopener\\\" href=\\\"https://www.elastic.co/elastic-maps-service\\\">Elastic Maps Service</a>\",\"id\":\"road_map\",\"maxZoom\":20,\"minZoom\":0,\"origin\":\"elastic_maps_service\"},\"url\":\"\"}},\"title\":\"region_map\",\"type\":\"region_map\"}"},"coreMigrationVersion":"7.17.5","id":"64a5b9f0-1e6b-11ed-833b-a105e9534fa9","migrationVersion":{"visualization":"7.17.0"},"references":[{"id":"90943e30-9a47-11e8-b64d-95841ca0b247","name":"kibanaSavedObjectMeta.searchSourceJSON.index","type":"index-pattern"}],"type":"visualization","updated_at":"2022-08-17T20:30:50.288Z","version":"WzE0MDIsMV0="} {"attributes":{"description":"","kibanaSavedObjectMeta":{"searchSourceJSON":"{\"query\":{\"query\":\"\",\"language\":\"kuery\"},\"filter\":[],\"indexRefName\":\"kibanaSavedObjectMeta.searchSourceJSON.index\"}"},"title":"tile_map","uiStateJSON":"{\"mapZoom\":2,\"mapCenter\":[13.64385981167601,-135.97675761558068]}","version":1,"visState":"{\"title\":\"tile_map\",\"type\":\"tile_map\",\"aggs\":[{\"id\":\"1\",\"enabled\":true,\"type\":\"count\",\"params\":{},\"schema\":\"metric\"},{\"id\":\"2\",\"enabled\":true,\"type\":\"geohash_grid\",\"params\":{\"field\":\"geo.coordinates\",\"autoPrecision\":true,\"precision\":2,\"useGeocentroid\":true,\"isFilteredByCollar\":true},\"schema\":\"segment\"}],\"params\":{\"colorSchema\":\"Yellow to Red\",\"mapType\":\"Scaled Circle Markers\",\"isDesaturated\":true,\"addTooltip\":true,\"heatClusterSize\":1.5,\"legendPosition\":\"bottomright\",\"mapZoom\":2,\"mapCenter\":[0,0],\"wms\":{\"enabled\":false,\"url\":\"\",\"options\":{\"version\":\"\",\"layers\":\"\",\"format\":\"image/png\",\"transparent\":true,\"attribution\":\"\",\"styles\":\"\"},\"selectedTmsLayer\":{\"origin\":\"elastic_maps_service\",\"id\":\"road_map\",\"minZoom\":0,\"maxZoom\":20,\"attribution\":\"<a rel=\\\"noreferrer noopener\\\" href=\\\"https://www.openstreetmap.org/copyright\\\">OpenStreetMap contributors</a> | <a rel=\\\"noreferrer noopener\\\" href=\\\"https://openmaptiles.org\\\">OpenMapTiles</a> | <a rel=\\\"noreferrer noopener\\\" href=\\\"https://www.elastic.co/elastic-maps-service\\\">Elastic Maps Service</a>\"}}}}"},"coreMigrationVersion":"7.17.5","id":"4b073190-1e6b-11ed-833b-a105e9534fa9","migrationVersion":{"visualization":"7.17.0"},"references":[{"id":"90943e30-9a47-11e8-b64d-95841ca0b247","name":"kibanaSavedObjectMeta.searchSourceJSON.index","type":"index-pattern"}],"type":"visualization","updated_at":"2022-08-17T20:29:28.769Z","version":"WzEzNDksMV0="} {"attributes":{"description":"","hits":0,"kibanaSavedObjectMeta":{"searchSourceJSON":"{\"query\":{\"query\":\"\",\"language\":\"kuery\"},\"filter\":[]}"},"optionsJSON":"{\"useMargins\":true,\"syncColors\":false,\"hidePanelTitles\":false}","panelsJSON":"[{\"version\":\"7.17.5\",\"type\":\"visualization\",\"gridData\":{\"x\":24,\"y\":0,\"w\":24,\"h\":15,\"i\":\"ad02c99b-8bae-42e0-8a43-d27762f1e607\"},\"panelIndex\":\"ad02c99b-8bae-42e0-8a43-d27762f1e607\",\"embeddableConfig\":{\"enhancements\":{}},\"panelRefName\":\"panel_ad02c99b-8bae-42e0-8a43-d27762f1e607\"},{\"version\":\"7.17.5\",\"type\":\"visualization\",\"gridData\":{\"x\":0,\"y\":0,\"w\":24,\"h\":15,\"i\":\"8412ffc8-b94c-4bbd-aa55-fa670f3fb4ee\"},\"panelIndex\":\"8412ffc8-b94c-4bbd-aa55-fa670f3fb4ee\",\"embeddableConfig\":{\"enhancements\":{}},\"panelRefName\":\"panel_8412ffc8-b94c-4bbd-aa55-fa670f3fb4ee\"}]","timeRestore":false,"title":"dash with legacy map visualizations","version":1},"coreMigrationVersion":"7.17.5","id":"97437d70-1e6b-11ed-833b-a105e9534fa9","migrationVersion":{"dashboard":"7.17.3"},"references":[{"id":"64a5b9f0-1e6b-11ed-833b-a105e9534fa9","name":"ad02c99b-8bae-42e0-8a43-d27762f1e607:panel_ad02c99b-8bae-42e0-8a43-d27762f1e607","type":"visualization"},{"id":"4b073190-1e6b-11ed-833b-a105e9534fa9","name":"8412ffc8-b94c-4bbd-aa55-fa670f3fb4ee:panel_8412ffc8-b94c-4bbd-aa55-fa670f3fb4ee","type":"visualization"}],"type":"dashboard","updated_at":"2022-08-17T20:31:36.656Z","version":"WzE0MzEsMV0="} {"excludedObjects":[],"excludedObjectsCount":0,"exportedCount":4,"missingRefCount":0,"missingReferences":[]} ``` * Open dashboard and verify tile_map renders --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Fixes #154375
background
#105326 replaced tile_map visualization implementation with a new implementation that is a wrapper around MapEmbeddable.
#152952 removed geohash_grid aggregation. This causes a regression where existing tile_map visualizations no longer work. Even though geohash_grid aggregation is no longer used, the AggType is still needed so that new tile_map visualization wrapper can access aggregation configuration state.
This PR adds back geohash_grid AggType in
legacyAggsfor this purpose. PR also adds a functional test to better prevent regressions with tile_mapTest