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] clean up feature editing name space to avoid conflicts with layer settings editing #102516

Merged
merged 4 commits into from
Jun 18, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion x-pack/plugins/maps/common/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ export const SOURCE_DATA_REQUEST_ID = 'source';
export const SOURCE_META_DATA_REQUEST_ID = `${SOURCE_DATA_REQUEST_ID}_${META_DATA_REQUEST_ID_SUFFIX}`;
export const SOURCE_FORMATTERS_DATA_REQUEST_ID = `${SOURCE_DATA_REQUEST_ID}_${FORMATTERS_DATA_REQUEST_ID_SUFFIX}`;
export const SOURCE_BOUNDS_DATA_REQUEST_ID = `${SOURCE_DATA_REQUEST_ID}_bounds`;
export const IS_EDITABLE_REQUEST_ID = 'isEditable';
export const SUPPORTS_FEATURE_EDITING_REQUEST_ID = 'SUPPORTS_FEATURE_EDITING_REQUEST_ID';

export const MIN_ZOOM = 0;
export const MAX_ZOOM = 24;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import {
FIELD_ORIGIN,
KBN_TOO_MANY_FEATURES_IMAGE_ID,
FieldFormatter,
IS_EDITABLE_REQUEST_ID,
SUPPORTS_FEATURE_EDITING_REQUEST_ID,
} from '../../../../common/constants';
import { JoinTooltipProperty } from '../../tooltips/join_tooltip_property';
import { DataRequestAbortError } from '../../util/data_request';
Expand Down Expand Up @@ -177,10 +177,10 @@ export class VectorLayer extends AbstractLayer implements IVectorLayer {
}

supportsFeatureEditing(): boolean {
const dataRequest = this.getDataRequest(IS_EDITABLE_REQUEST_ID);
const data = dataRequest?.getData() as { isEditable: boolean } | undefined;
const dataRequest = this.getDataRequest(SUPPORTS_FEATURE_EDITING_REQUEST_ID);
const data = dataRequest?.getData() as { supportsFeatureEditing: boolean } | undefined;

return data ? data.isEditable : false;
return data ? data.supportsFeatureEditing : false;
}

hasJoins() {
Expand Down Expand Up @@ -678,7 +678,7 @@ export class VectorLayer extends AbstractLayer implements IVectorLayer {
syncContext,
source,
});
await this._syncIsEditable({ syncContext });
await this._syncSupportsFeatureEditing({ syncContext, source });
nreese marked this conversation as resolved.
Show resolved Hide resolved
if (
!sourceResult.featureCollection ||
!sourceResult.featureCollection.features.length ||
Expand All @@ -696,21 +696,27 @@ export class VectorLayer extends AbstractLayer implements IVectorLayer {
}
}

async _syncIsEditable({ syncContext }: { syncContext: DataRequestContext }) {
async _syncSupportsFeatureEditing({
syncContext,
source,
}: {
syncContext: DataRequestContext;
source: IVectorSource;
}) {
if (syncContext.dataFilters.isReadOnly) {
return;
}
const { startLoading, stopLoading, onLoadError } = syncContext;
const dataRequestId = IS_EDITABLE_REQUEST_ID;
const dataRequestId = SUPPORTS_FEATURE_EDITING_REQUEST_ID;
const requestToken = Symbol(`layer-${this.getId()}-${dataRequestId}`);
const prevDataRequest = this.getDataRequest(dataRequestId);
if (prevDataRequest) {
return;
}
try {
startLoading(dataRequestId, requestToken);
const isEditable = await this.getSource().loadIsEditable();
stopLoading(dataRequestId, requestToken, { isEditable });
const supportsFeatureEditing = await source.supportsFeatureEditing();
stopLoading(dataRequestId, requestToken, { supportsFeatureEditing });
} catch (error) {
onLoadError(dataRequestId, requestToken, error.message);
throw error;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,7 @@ export class ESSearchSource extends AbstractESSource implements ITiledSingleLaye
return !!(scalingType === SCALING_TYPES.TOP_HITS && topHitsSplitField);
}

async loadIsEditable(): Promise<boolean> {
async supportsFeatureEditing(): Promise<boolean> {
if (!getMapAppConfig().enableDrawingFeature) {
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ export class MVTSingleLayerVectorSource
return tooltips;
}

async loadIsEditable(): Promise<boolean> {
async supportsFeatureEditing(): Promise<boolean> {
return false;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ export interface IVectorSource extends ISource {
getSupportedShapeTypes(): Promise<VECTOR_SHAPE_TYPE[]>;
isBoundsAware(): boolean;
getSourceTooltipContent(sourceDataRequest?: DataRequest): SourceTooltipConfig;
loadIsEditable(): Promise<boolean>;
supportsFeatureEditing(): Promise<boolean>;
addFeature(geometry: Geometry | Position[]): Promise<void>;
}

Expand Down Expand Up @@ -160,7 +160,7 @@ export class AbstractVectorSource extends AbstractSource implements IVectorSourc
throw new Error('Should implement VectorSource#addFeature');
}

async loadIsEditable(): Promise<boolean> {
async supportsFeatureEditing(): Promise<boolean> {
return false;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,15 @@ export interface Props {
interface State {
isPopoverOpen: boolean;
supportsFeatureEditing: boolean;
canEditFeatures: boolean;
isFeatureEditingEnabled: boolean;
}

export class TOCEntryActionsPopover extends Component<Props, State> {
state: State = { isPopoverOpen: false, supportsFeatureEditing: false, canEditFeatures: false };
state: State = {
isPopoverOpen: false,
supportsFeatureEditing: false,
isFeatureEditingEnabled: false,
};
private _isMounted = false;

componentDidMount() {
Expand All @@ -57,26 +61,26 @@ export class TOCEntryActionsPopover extends Component<Props, State> {
}

componentDidUpdate() {
this._checkLayerEditable();
this._loadFeatureEditing();
}

async _checkLayerEditable() {
async _loadFeatureEditing() {
if (!(this.props.layer instanceof VectorLayer)) {
return;
}
nreese marked this conversation as resolved.
Show resolved Hide resolved
const supportsFeatureEditing = this.props.layer.supportsFeatureEditing();
const canEditFeatures = await this._getCanEditFeatures();
const isFeatureEditingEnabled = await this._getIsFeatureEditingEnabled();
if (
!this._isMounted ||
(supportsFeatureEditing === this.state.supportsFeatureEditing &&
canEditFeatures === this.state.canEditFeatures)
isFeatureEditingEnabled === this.state.isFeatureEditingEnabled)
) {
return;
}
this.setState({ supportsFeatureEditing, canEditFeatures });
this.setState({ supportsFeatureEditing, isFeatureEditingEnabled });
}

async _getCanEditFeatures(): Promise<boolean> {
async _getIsFeatureEditingEnabled(): Promise<boolean> {
const vectorLayer = this.props.layer as VectorLayer;
const layerSource = await this.props.layer.getSource();
if (!(layerSource instanceof ESSearchSource)) {
Expand Down Expand Up @@ -160,13 +164,13 @@ export class TOCEntryActionsPopover extends Component<Props, State> {
name: EDIT_FEATURES_LABEL,
icon: <EuiIcon type="vector" size="m" />,
'data-test-subj': 'editLayerButton',
toolTipContent: this.state.canEditFeatures
toolTipContent: this.state.isFeatureEditingEnabled
? null
: i18n.translate('xpack.maps.layerTocActions.editLayerTooltip', {
defaultMessage:
'Edit features only supported for document layers without clustering, joins, or time filtering',
}),
disabled: !this.state.canEditFeatures,
disabled: !this.state.isFeatureEditingEnabled,
onClick: async () => {
this._closePopover();
const supportedShapeTypes = await (this.props.layer.getSource() as ESSearchSource).getSupportedShapeTypes();
Expand Down