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.

Sign up for GitHub

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
Merged

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

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d055541
[Maps][File upload] fix layer in preview mode shows different results…
nreese Apr 14, 2021
3108702
i18n clean up
nreese Apr 14, 2021
0396f00
Merge branch 'master' into add_document_layer
kibanamachine Apr 15, 2021
bbc07a6
review feedback
nreese Apr 15, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 26 additions & 11 deletions x-pack/plugins/file_upload/public/components/json_upload_and_parse.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { getIndexPatternService } from '../kibana_services';
import { GeoJsonUploadForm, OnFileSelectParameters } from './geojson_upload_form';
import { ImportCompleteView } from './import_complete_view';
import { ES_FIELD_TYPES } from '../../../../../src/plugins/data/public';
import { FileUploadComponentProps } from '../lazy_load_bundle';
import type { FileUploadComponentProps, FileUploadGeoResults } from '../lazy_load_bundle';
import { ImportResults } from '../importer';
import { GeoJsonImporter } from '../importer/geojson_importer';
import { Settings } from '../../common';
Expand Down Expand Up @@ -93,7 +93,7 @@ export class JsonUploadAndParse extends Component<FileUploadComponentProps, Stat
phase: PHASE.COMPLETE,
failedPermissionCheck: true,
});
this.props.onIndexingError();
this.props.onUploadError();
return;
}

Expand Down Expand Up @@ -136,7 +136,7 @@ export class JsonUploadAndParse extends Component<FileUploadComponentProps, Stat
phase: PHASE.COMPLETE,
importResults: initializeImportResp,
});
this.props.onIndexingError();
this.props.onUploadError();
return;
}

Expand Down Expand Up @@ -170,7 +170,7 @@ export class JsonUploadAndParse extends Component<FileUploadComponentProps, Stat
}),
phase: PHASE.COMPLETE,
});
this.props.onIndexingError();
this.props.onUploadError();
return;
}

Expand All @@ -185,13 +185,31 @@ export class JsonUploadAndParse extends Component<FileUploadComponentProps, Stat
}),
});
let indexPattern;
let results: FileUploadGeoResults | undefined;
try {
indexPattern = await getIndexPatternService().createAndSave(
{
title: this.state.indexName,
},
true
);
if (!indexPattern.id) {
throw new Error('Index pattern id not provided');
}
const geoField = indexPattern.fields.find((field) =>
[ES_FIELD_TYPES.GEO_POINT as string, ES_FIELD_TYPES.GEO_SHAPE as string].includes(
field.type
)
);
if (!geoField) {
throw new Error('geo field not created in index pattern');
}
results = {
indexPatternId: indexPattern.id,
geoFieldName: geoField.name,
geoFieldType: geoField.type as ES_FIELD_TYPES.GEO_POINT | ES_FIELD_TYPES.GEO_SHAPE,
docCount: importResults.docCount !== undefined ? importResults.docCount : 0,
nreese marked this conversation as resolved.
Show resolved Hide resolved
};
} catch (error) {
if (this._isMounted) {
this.setState({
Expand All @@ -200,7 +218,7 @@ export class JsonUploadAndParse extends Component<FileUploadComponentProps, Stat
}),
phase: PHASE.COMPLETE,
});
this.props.onIndexingError();
this.props.onUploadError();
}
return;
}
Expand All @@ -220,16 +238,13 @@ export class JsonUploadAndParse extends Component<FileUploadComponentProps, Stat
phase: PHASE.COMPLETE,
importStatus: '',
});
this.props.onIndexingComplete({
indexDataResp: importResults,
indexPattern,
});
this.props.onUploadComplete(results!);
};

_onFileSelect = ({ features, importer, indexName, previewCoverage }: OnFileSelectParameters) => {
this._geojsonImporter = importer;

this.props.onFileUpload(
this.props.onFileSelect(
{
type: 'FeatureCollection',
features,
Expand All @@ -245,7 +260,7 @@ export class JsonUploadAndParse extends Component<FileUploadComponentProps, Stat
this._geojsonImporter = undefined;
}

this.props.onFileRemove();
this.props.onFileClear();
};

_onGeoFieldTypeSelect = (geoFieldType: ES_FIELD_TYPES.GEO_POINT | ES_FIELD_TYPES.GEO_SHAPE) => {
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/file_upload/public/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,4 @@ export * from '../common';
export * from './importer/types';

export { FileUploadPluginStart } from './plugin';
export { FileUploadComponentProps } from './lazy_load_bundle';
export { FileUploadComponentProps, FileUploadGeoResults } from './lazy_load_bundle';
22 changes: 13 additions & 9 deletions x-pack/plugins/file_upload/public/lazy_load_bundle/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,21 +7,25 @@

import React from 'react';
import { FeatureCollection } from 'geojson';
import { IndexPattern } from 'src/plugins/data/public';
import { HttpStart } from 'src/core/public';
import { IImporter, ImportFactoryOptions, ImportResults } from '../importer';
import { IImporter, ImportFactoryOptions } from '../importer';
import { getHttp } from '../kibana_services';
import { ES_FIELD_TYPES } from '../../../../../src/plugins/data/public';

export interface FileUploadGeoResults {
indexPatternId: string;
geoFieldName: string;
geoFieldType: ES_FIELD_TYPES.GEO_POINT | ES_FIELD_TYPES.GEO_SHAPE;
docCount: number;
}

export interface FileUploadComponentProps {
isIndexingTriggered: boolean;
onFileUpload: (geojsonFile: FeatureCollection, name: string, previewCoverage: number) => void;
onFileRemove: () => void;
onFileSelect: (geojsonFile: FeatureCollection, name: string, previewCoverage: number) => void;
onFileClear: () => void;
onIndexReady: (indexReady: boolean) => void;
onIndexingComplete: (results: {
indexDataResp: ImportResults;
indexPattern: IndexPattern;
}) => void;
onIndexingError: () => void;
onUploadComplete: (results: FileUploadGeoResults) => void;
onUploadError: () => void;
}

let loadModulesPromise: Promise<LazyLoadedFileUploadModules>;
Expand Down
22 changes: 17 additions & 5 deletions x-pack/plugins/maps/public/classes/layers/file_upload_wizard/config.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import { i18n } from '@kbn/i18n';
import React from 'react';
import { LayerWizard, RenderWizardArguments } from '../../layers/layer_wizard_registry';
import { ClientFileCreateSourceEditor, INDEX_SETUP_STEP_ID, INDEXING_STEP_ID } from './wizard';
import { ClientFileCreateSourceEditor, UPLOAD_STEPS } from './wizard';
import { getFileUpload } from '../../../kibana_services';

export const uploadLayerWizardConfig: LayerWizard = {
Expand All @@ -30,17 +30,29 @@ export const uploadLayerWizardConfig: LayerWizard = {
icon: 'importAction',
prerequisiteSteps: [
{
id: INDEX_SETUP_STEP_ID,
label: i18n.translate('xpack.maps.fileUploadWizard.importFileSetupLabel', {
id: UPLOAD_STEPS.CONFIGURE_UPLOAD,
label: i18n.translate('xpack.maps.fileUploadWizard.configureUploadLabel', {
defaultMessage: 'Import file',
}),
},
{
id: INDEXING_STEP_ID,
label: i18n.translate('xpack.maps.fileUploadWizard.indexingLabel', {
id: UPLOAD_STEPS.UPLOAD,
label: i18n.translate('xpack.maps.fileUploadWizard.uploadLabel', {
defaultMessage: 'Importing file',
}),
},
{
id: UPLOAD_STEPS.CONFIGURE_DOCUMENT_LAYER,
label: i18n.translate('xpack.maps.fileUploadWizard.configureDocumentLayerLabel', {
defaultMessage: 'Add as document layer',
}),
},
{
id: UPLOAD_STEPS.ADD_DOCUMENT_LAYER,
label: i18n.translate('xpack.maps.fileUploadWizard.addDocumentLayerLabel', {
defaultMessage: 'Adding document layer',
}),
},
nreese marked this conversation as resolved.
Show resolved Hide resolved
],
renderWizard: (renderWizardArguments: RenderWizardArguments) => {
return <ClientFileCreateSourceEditor {...renderWizardArguments} />;
Expand Down
101 changes: 52 additions & 49 deletions x-pack/plugins/maps/public/classes/layers/file_upload_wizard/wizard.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,21 +9,21 @@ import { i18n } from '@kbn/i18n';
import React, { Component } from 'react';
import { FeatureCollection } from 'geojson';
import { EuiPanel } from '@elastic/eui';
import { IndexPattern, IFieldType } from 'src/plugins/data/public';
import {
ES_GEO_FIELD_TYPE,
DEFAULT_MAX_RESULT_WINDOW,
SCALING_TYPES,
} from '../../../../common/constants';
import { DEFAULT_MAX_RESULT_WINDOW, SCALING_TYPES } from '../../../../common/constants';
import { getFileUpload } from '../../../kibana_services';
import { GeoJsonFileSource } from '../../sources/geojson_file_source';
import { VectorLayer } from '../../layers/vector_layer';
import { createDefaultLayerDescriptor } from '../../sources/es_search_source';
import { RenderWizardArguments } from '../../layers/layer_wizard_registry';
import { FileUploadComponentProps, ImportResults } from '../../../../../file_upload/public';

export const INDEX_SETUP_STEP_ID = 'INDEX_SETUP_STEP_ID';
export const INDEXING_STEP_ID = 'INDEXING_STEP_ID';
import { FileUploadComponentProps, FileUploadGeoResults } from '../../../../../file_upload/public';
import { ES_FIELD_TYPES } from '../../../../../../../src/plugins/data/public';

export enum UPLOAD_STEPS {
CONFIGURE_UPLOAD = 'CONFIGURE_UPLOAD',
UPLOAD = 'UPLOAD',
CONFIGURE_DOCUMENT_LAYER = 'CONFIGURE_DOCUMENT_LAYER',
ADD_DOCUMENT_LAYER = 'ADD_DOCUMENT_LAYER',
}

enum INDEXING_STAGE {
READY = 'READY',
Expand All @@ -35,6 +35,7 @@ enum INDEXING_STAGE {
interface State {
indexingStage: INDEXING_STAGE | null;
fileUploadComponent: React.ComponentType<FileUploadComponentProps> | null;
results?: FileUploadGeoResults;
}

export class ClientFileCreateSourceEditor extends Component<RenderWizardArguments, State> {
Expand All @@ -56,22 +57,48 @@ export class ClientFileCreateSourceEditor extends Component<RenderWizardArgument

componentDidUpdate() {
if (
this.props.currentStepId === INDEXING_STEP_ID &&
this.props.currentStepId === UPLOAD_STEPS.UPLOAD &&
this.state.indexingStage === INDEXING_STAGE.READY
) {
this.setState({ indexingStage: INDEXING_STAGE.TRIGGERED });
this.props.startStepLoading();
return;
}

if (
this.props.currentStepId === UPLOAD_STEPS.ADD_DOCUMENT_LAYER &&
this.state.indexingStage === INDEXING_STAGE.SUCCESS &&
this.state.results
) {
this._addDocumentLayer(this.state.results);
}
}

_addDocumentLayer(results: FileUploadGeoResults) {
const esSearchSourceConfig = {
indexPatternId: results.indexPatternId,
geoField: results.geoFieldName,
// Only turn on bounds filter for large doc counts
filterByMapBounds: results.docCount > DEFAULT_MAX_RESULT_WINDOW,
scalingType:
results.geoFieldType === ES_FIELD_TYPES.GEO_POINT
? SCALING_TYPES.CLUSTERS
: SCALING_TYPES.LIMIT,
};
this.props.previewLayers([
createDefaultLayerDescriptor(esSearchSourceConfig, this.props.mapColors),
]);
this.props.advanceToNextStep();
}

async _loadFileUploadComponent() {
const fileUploadComponent = await getFileUpload().getFileUploadComponent();
if (this._isMounted) {
this.setState({ fileUploadComponent });
}
}

_onFileUpload = (geojsonFile: FeatureCollection, name: string, previewCoverage: number) => {
_onFileSelect = (geojsonFile: FeatureCollection, name: string, previewCoverage: number) => {
if (!this._isMounted) {
return;
}
Expand Down Expand Up @@ -103,41 +130,22 @@ export class ClientFileCreateSourceEditor extends Component<RenderWizardArgument
this.props.previewLayers([layerDescriptor]);
};

_onIndexingComplete = (results: { indexDataResp: ImportResults; indexPattern: IndexPattern }) => {
_onFileClear = () => {
this.props.previewLayers([]);
};

_onUploadComplete = (results: FileUploadGeoResults) => {
if (!this._isMounted) {
return;
}

this.setState({ results });
this.setState({ indexingStage: INDEXING_STAGE.SUCCESS });
this.props.advanceToNextStep();

const geoField = results.indexPattern.fields.find((field: IFieldType) =>
[ES_GEO_FIELD_TYPE.GEO_POINT as string, ES_GEO_FIELD_TYPE.GEO_SHAPE as string].includes(
field.type
)
);
if (!results.indexPattern.id || !geoField) {
this.setState({ indexingStage: INDEXING_STAGE.ERROR });
this.props.previewLayers([]);
} else {
const esSearchSourceConfig = {
indexPatternId: results.indexPattern.id,
geoField: geoField.name,
// Only turn on bounds filter for large doc counts
// @ts-ignore
filterByMapBounds: results.indexDataResp.docCount > DEFAULT_MAX_RESULT_WINDOW,
scalingType:
geoField.type === ES_GEO_FIELD_TYPE.GEO_POINT
? SCALING_TYPES.CLUSTERS
: SCALING_TYPES.LIMIT,
};
this.setState({ indexingStage: INDEXING_STAGE.SUCCESS });
this.props.previewLayers([
createDefaultLayerDescriptor(esSearchSourceConfig, this.props.mapColors),
]);
}
this.props.enableNextBtn();
};

_onIndexingError = () => {
_onUploadError = () => {
if (!this._isMounted) {
return;
}
Expand All @@ -161,11 +169,6 @@ export class ClientFileCreateSourceEditor extends Component<RenderWizardArgument
}
};

// Called on file upload screen when upload file is changed or removed
_onFileRemove = () => {
this.props.previewLayers([]);
};

render() {
if (!this.state.fileUploadComponent) {
return null;
Expand All @@ -176,11 +179,11 @@ export class ClientFileCreateSourceEditor extends Component<RenderWizardArgument
<EuiPanel>
<FileUpload
isIndexingTriggered={this.state.indexingStage === INDEXING_STAGE.TRIGGERED}
onFileUpload={this._onFileUpload}
onFileRemove={this._onFileRemove}
onFileSelect={this._onFileSelect}
onFileClear={this._onFileClear}
onIndexReady={this._onIndexReady}
onIndexingComplete={this._onIndexingComplete}
onIndexingError={this._onIndexingError}
onUploadComplete={this._onUploadComplete}
onUploadError={this._onUploadError}
/>
</EuiPanel>
);
Expand Down
15 changes: 8 additions & 7 deletions x-pack/plugins/maps/public/connected_components/add_layer_panel/view.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,13 +68,14 @@ export class AddLayerPanel extends Component<Props, State> {
};

_onWizardSelect = (layerWizard: LayerWizard) => {
const layerSteps = [
...(layerWizard.prerequisiteSteps ? layerWizard.prerequisiteSteps : []),
{
id: ADD_LAYER_STEP_ID,
label: ADD_LAYER_STEP_LABEL,
},
];
const layerSteps = layerWizard.prerequisiteSteps
? layerWizard.prerequisiteSteps
: [
{
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.

this.setState({
...INITIAL_STATE,
layerWizard,
Expand Down
2 changes: 0 additions & 2 deletions x-pack/plugins/translations/translations/ja-JP.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12442,8 +12442,6 @@
"xpack.maps.featureRegistry.mapsFeatureName": "マップ",
"xpack.maps.fields.percentileMedianLabek": "中間",
"xpack.maps.fileUploadWizard.description": "Elasticsearch で GeoJSON データにインデックスします",
"xpack.maps.fileUploadWizard.importFileSetupLabel": "ファイルのインポート",
"xpack.maps.fileUploadWizard.indexingLabel": "ファイルをインポートしています",
"xpack.maps.fileUploadWizard.title": "GeoJSONをアップロード",
"xpack.maps.filterEditor.applyGlobalQueryCheckboxLabel": "レイヤーデータにグローバルフィルターを適用",
"xpack.maps.filterEditor.applyGlobalTimeCheckboxLabel": "グローバル時刻をレイヤーデータに適用",
Expand Down
2 changes: 0 additions & 2 deletions x-pack/plugins/translations/translations/zh-CN.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12609,8 +12609,6 @@
"xpack.maps.featureRegistry.mapsFeatureName": "Maps",
"xpack.maps.fields.percentileMedianLabek": "中值",
"xpack.maps.fileUploadWizard.description": "在 Elasticsearch 中索引 GeoJSON 数据",
"xpack.maps.fileUploadWizard.importFileSetupLabel": "导入文件",
"xpack.maps.fileUploadWizard.indexingLabel": "正在导入文件",
"xpack.maps.fileUploadWizard.title": "上传 GeoJSON",
"xpack.maps.filterEditor.applyGlobalQueryCheckboxLabel": "将全局筛选应用到图层数据",
"xpack.maps.filterEditor.applyGlobalTimeCheckboxLabel": "将全局时间应用于图层数据",
Expand Down