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

[Maps][File upload] fix layer in preview mode shows different results after uploading geojson file when feature-count exceeds ES-search limit #97157

Merged
merged 4 commits into from
Apr 15, 2021

Conversation

nreese
Copy link
Contributor

@nreese nreese commented Apr 14, 2021

fixes #40091

This PR updates the file upload flow.

Previously, the preview layer would automatically switched from file preview to the Elasticsearch document layer when the upload process completed. This can be a very visually jaunting process since the preview may show points while the Elasticasearch document layer may show clusters.

This PR updates the file upload flow to only switch from the file preview to the Elasticsearch document layer after the user clicks "Add as document layer" button to avoid any drastic changes until a user has performed an action and knows why the layer is changing.

Screen Shot 2021-04-14 at 11 14 45 AM

… after uploading geojson file when feature-count exceeds ES-search limit
@nreese nreese added release_note:fix [Deprecated-Use Team:Presentation]Team:Geo Former Team Label for Geo Team. Now use Team:Presentation v8.0.0 v7.13.0 labels Apr 14, 2021
@nreese nreese requested a review from kindsun April 14, 2021 17:16
@elasticmachine
Copy link
Contributor

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

Comment on lines 73 to 78
: [
{
id: ADD_LAYER_STEP_ID,
label: ADD_LAYER_STEP_LABEL,
},
];
Copy link
Contributor

Choose a reason for hiding this comment

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

If all layers ultimately end in an "Add layer step", what's the point in overriding this with prereq steps that include an add layer step vs. always including it here as we did before?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did this so ClientFileCreateSourceEditor would know when the final step without needing to know about ADD_LAYER_STEP_ID since ADD_LAYER_STEP_ID seemed like an internal detail in AddLayerPanel.

I will revert this change and instead, pass isOnFinalStep to renderWizard since that is the information that is needed by ClientFileCreateSourceEditor. That way, ADD_LAYER_STEP_ID is the final step even when prerequisiteSteps are provided.

@kindsun
Copy link
Contributor

kindsun commented Apr 15, 2021

First pass looks good. The extra step makes sense to me and it's a nice transition from preview to docs 👍

Copy link
Contributor

@kindsun kindsun left a comment

Choose a reason for hiding this comment

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

Overall I think this is a nicely improved flow. Some minor comments. Worked well in local tests. lgtm!

  • code review
  • tested locally in chrome

@kindsun
Copy link
Contributor

kindsun commented Apr 15, 2021

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
fileUpload 788.7KB 789.1KB +387.0B
maps 2.6MB 2.6MB +570.0B
total +957.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
fileUpload 13.8KB 14.0KB +201.0B
Unknown metric groups

API count

id before after diff
actions - 113 +113
advancedSettings - 18 +18
alerting - 200 +200
apm - 35 +35
apmOss - 38 +38
banners - 9 +9
beatsManagement - 2 +2
bfetch - 58 +58
canvas - 4 +4
cases - 2 +2
charts - 157 +157
cloud - 19 +19
console - 2 +2
core - 2168 +2168
dashboard - 138 +138
dashboardEnhanced - 51 +51
dashboardMode - 11 +11
data - 3436 +3436
dataEnhanced - 52 +52
devTools - 9 +9
discover - 61 +61
discoverEnhanced - 39 +39
embeddable - 426 +426
embeddableEnhanced - 14 +14
encryptedSavedObjects - 20 +20
enterpriseSearch - 2 +2
esUiShared - 84 +84
eventLog - 70 +70
expressions - 1755 +1755
features - 199 +199
fileUpload - 180 +180
fleet - 1075 +1075
globalSearch - 67 +67
home - 91 +91
indexLifecycleManagement - 5 +5
indexManagement - 162 +162
indexPatternFieldEditor - 29 +29
indexPatternManagement - 46 +46
infra - 14 +14
ingestPipelines - 9 +9
inspector - 97 +97
kibanaLegacy - 92 +92
kibanaReact - 232 +232
kibanaUtils - 465 +465
lens - 160 +160
licenseManagement - 3 +3
licensing - 116 +116
lists - 251 +251
management - 35 +35
maps - 182 +182
mapsEms - 69 +69
ml - 343 +343
monitoring - 9 +9
navigation - 29 +29
newsfeed - 17 +17
observability - 176 +176
osquery - 8 +8
presentationUtil - 69 +69
remoteClusters - 4 +4
reporting - 127 +127
rollup - 20 +20
ruleRegistry - 52 +52
runtimeFields - 22 +22
savedObjects - 173 +173
savedObjectsManagement - 91 +91
savedObjectsTagging - 54 +54
savedObjectsTaggingOss - 81 +81
security - 79 +79
securityOss - 11 +11
securitySolution - 99 +99
share - 66 +66
snapshotRestore - 22 +22
spaces - 82 +82
spacesOss - 64 +64
stackAlerts - 4 +4
taskManager - 41 +41
telemetry - 41 +41
telemetryCollectionManager - 24 +24
telemetryCollectionXpack - 1 +1
telemetryManagementSection - 13 +13
timelines - 6 +6
triggersActionsUi - 221 +221
uiActions - 127 +127
uiActionsEnhanced - 185 +185
uptime - 5 +5
urlForwarding - 14 +14
usageCollection - 53 +53
visTypeTimeseries - 7 +7
visualizations - 212 +212
total +15224

API count missing comments

id before after diff
actions - 113 +113
advancedSettings - 17 +17
alerting - 200 +200
apm - 35 +35
apmOss - 38 +38
banners - 9 +9
beatsManagement - 2 +2
bfetch - 47 +47
canvas - 3 +3
cases - 2 +2
charts - 132 +132
cloud - 19 +19
console - 2 +2
core - 989 +989
dashboard - 126 +126
dashboardEnhanced - 50 +50
dashboardMode - 11 +11
data - 2944 +2944
dataEnhanced - 34 +34
devTools - 8 +8
discover - 48 +48
discoverEnhanced - 37 +37
embeddable - 361 +361
embeddableEnhanced - 14 +14
encryptedSavedObjects - 18 +18
enterpriseSearch - 2 +2
esUiShared - 82 +82
eventLog - 70 +70
expressions - 1364 +1364
features - 89 +89
fileUpload - 180 +180
fleet - 985 +985
globalSearch - 13 +13
home - 67 +67
indexLifecycleManagement - 5 +5
indexManagement - 157 +157
indexPatternFieldEditor - 27 +27
indexPatternManagement - 46 +46
infra - 11 +11
ingestPipelines - 9 +9
inspector - 77 +77
kibanaLegacy - 84 +84
kibanaReact - 207 +207
kibanaUtils - 301 +301
lens - 149 +149
licenseManagement - 3 +3
licensing - 40 +40
lists - 234 +234
management - 35 +35
maps - 181 +181
mapsEms - 69 +69
ml - 339 +339
monitoring - 9 +9
navigation - 29 +29
newsfeed - 17 +17
observability - 176 +176
osquery - 8 +8
presentationUtil - 67 +67
remoteClusters - 4 +4
reporting - 126 +126
rollup - 20 +20
ruleRegistry - 52 +52
runtimeFields - 17 +17
savedObjects - 159 +159
savedObjectsManagement - 80 +80
savedObjectsTagging - 50 +50
savedObjectsTaggingOss - 42 +42
security - 38 +38
securityOss - 8 +8
securitySolution - 90 +90
share - 60 +60
snapshotRestore - 22 +22
spaces - 65 +65
spacesOss - 29 +29
stackAlerts - 4 +4
taskManager - 16 +16
telemetry - 37 +37
telemetryCollectionManager - 24 +24
telemetryCollectionXpack - 1 +1
telemetryManagementSection - 12 +12
timelines - 6 +6
triggersActionsUi - 212 +212
uiActions - 88 +88
uiActionsEnhanced - 135 +135
uptime - 5 +5
urlForwarding - 14 +14
usageCollection - 46 +46
visTypeTimeseries - 7 +7
visualizations - 194 +194
total +12054

API count with any type

id before after diff
bfetch - 1 +1
charts - 2 +2
core - 147 +147
dashboard - 1 +1
data - 81 +81
embeddable - 2 +2
esUiShared - 4 +4
expressions - 44 +44
fileUpload - 6 +6
fleet - 16 +16
indexManagement - 12 +12
inspector - 6 +6
kibanaLegacy - 3 +3
kibanaUtils - 3 +3
maps - 2 +2
mapsEms - 1 +1
ml - 14 +14
reporting - 1 +1
ruleRegistry - 1 +1
savedObjects - 3 +3
share - 1 +1
snapshotRestore - 1 +1
triggersActionsUi - 1 +1
visualizations - 11 +11
total +364

Non-exported public API item count

id before after diff
actions - 6 +6
advancedSettings - 1 +1
alerting - 9 +9
apm - 8 +8
bfetch - 2 +2
canvas - 2 +2
cases - 1 +1
charts - 2 +2
core - 29 +29
dashboard - 9 +9
data - 76 +76
dataEnhanced - 2 +2
devTools - 2 +2
discover - 6 +6
discoverEnhanced - 2 +2
embeddable - 3 +3
encryptedSavedObjects - 4 +4
esUiShared - 4 +4
eventLog - 4 +4
expressions - 7 +7
features - 1 +1
fleet - 10 +10
globalSearch - 6 +6
home - 8 +8
indexManagement - 4 +4
indexPatternFieldEditor - 4 +4
indexPatternManagement - 4 +4
infra - 2 +2
ingestPipelines - 4 +4
inspector - 4 +4
kibanaLegacy - 1 +1
kibanaReact - 3 +3
kibanaUtils - 8 +8
lens - 12 +12
licensing - 10 +10
lists - 60 +60
management - 4 +4
maps - 16 +16
ml - 34 +34
navigation - 3 +3
observability - 7 +7
presentationUtil - 4 +4
reporting - 18 +18
ruleRegistry - 4 +4
runtimeFields - 2 +2
savedObjects - 5 +5
savedObjectsManagement - 1 +1
security - 10 +10
securityOss - 3 +3
securitySolution - 7 +7
share - 3 +3
snapshotRestore - 1 +1
spaces - 3 +3
taskManager - 3 +3
telemetry - 3 +3
telemetryCollectionManager - 4 +4
timelines - 1 +1
triggersActionsUi - 28 +28
uiActions - 9 +9
uiActionsEnhanced - 10 +10
uptime - 3 +3
usageCollection - 7 +7
visTypeTimeseries - 2 +2
visualizations - 6 +6
total +521

History

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

@nreese nreese merged commit f972174 into elastic:master Apr 15, 2021
nreese added a commit to nreese/kibana that referenced this pull request Apr 15, 2021
… after uploading geojson file when feature-count exceeds ES-search limit (elastic#97157)

* [Maps][File upload] fix layer in preview mode shows different results after uploading geojson file when feature-count exceeds ES-search limit

* i18n clean up

* review feedback

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
nreese added a commit that referenced this pull request Apr 15, 2021
… after uploading geojson file when feature-count exceeds ES-search limit (#97157) (#97325)

* [Maps][File upload] fix layer in preview mode shows different results after uploading geojson file when feature-count exceeds ES-search limit

* i18n clean up

* review feedback

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Deprecated-Use Team:Presentation]Team:Geo Former Team Label for Geo Team. Now use Team:Presentation release_note:fix v7.13.0 v8.0.0
Projects
None yet
4 participants