[Lens] Make the onDrop operation more atomic (#162125)

## Summary

This move onDrop operation to redux store. Thanks to that instead of 3
updates (update ds, update vis and removeDimension for both) we only
have one and it's nicer to test now.

Fixes https://github.com/elastic/kibana/issues/162283

### Checklist

Delete any items that are not applicable to this PR.

- [ ] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [ ]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [ ] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [ ] Any UI touched in this PR is usable by keyboard only (learn more
about [keyboard accessibility](https://webaim.org/techniques/keyboard/))
- [ ] Any UI touched in this PR does not create any new axe failures
(run axe in browser:
[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),
[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))
- [ ] If a plugin configuration key changed, check if it needs to be
allowlisted in the cloud and added to the [docker
list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)
- [ ] This renders correctly on smaller devices using a responsive
layout. (You can test this [in your
browser](https://www.browserstack.com/guide/responsive-testing-on-local-server))
- [ ] This was checked for [cross-browser
compatibility](https://www.elastic.co/support/matrix#matrix_browsers)


### Risk Matrix

Delete this section if it is not applicable to this PR.

Before closing this PR, invite QA, stakeholders, and other developers to
identify risks that should be tested prior to the change/feature
release.

When forming the risk matrix, consider some of the following examples
and how they may potentially impact the change:

| Risk | Probability | Severity | Mitigation/Notes |

|---------------------------|-------------|----------|-------------------------|
| Multiple Spaces—unexpected behavior in non-default Kibana Space.
| Low | High | Integration tests will verify that all features are still
supported in non-default Kibana Space and when user switches between
spaces. |
| Multiple nodes—Elasticsearch polling might have race conditions
when multiple Kibana nodes are polling for the same tasks. | High | Low
| Tasks are idempotent, so executing them multiple times will not result
in logical error, but will degrade performance. To test for this case we
add plenty of unit tests around this logic and document manual testing
procedure. |
| Code should gracefully handle cases when feature X or plugin Y are
disabled. | Medium | High | Unit tests will verify that any feature flag
or plugin combination still results in our service operational. |
| [See more potential risk
examples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx) |


### For maintainers

- [ ] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
This commit is contained in:
Marta Bondyra 2023-07-20 19:25:13 +02:00 committed by GitHub
parent 84fd8c0254
commit 426ba7984a
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
15 changed files with 999 additions and 1013 deletions

View file

@ -5,13 +5,16 @@
* 2.0.
*/
import { DropType } from '@kbn/dom-drag-drop';
import { isDraggedDataViewField } from '../../../../utils';
import {
isDraggedDataViewField,
reorderElements,
shouldRemoveSource as shouldRemoveSourceChecker,
} from '../../../../utils';
import {
DatasourceDimensionDropHandlerProps,
DragDropOperation,
IndexPatternMap,
isOperation,
StateSetter,
VisualizationDimensionGroupConfig,
DraggedField,
} from '../../../../types';
@ -27,16 +30,10 @@ import {
import { mergeLayer, mergeLayers } from '../../state_helpers';
import { getNewOperation, getField } from './get_drop_props';
import { FormBasedPrivateState, DataViewDragDropOperation } from '../../types';
import { removeColumn } from '../../form_based';
interface DropHandlerProps<T = DataViewDragDropOperation> {
state: FormBasedPrivateState;
setState: StateSetter<
FormBasedPrivateState,
{
isDimensionComplete?: boolean;
forceRender?: boolean;
}
>;
targetLayerDimensionGroups: VisualizationDimensionGroupConfig[];
dropType?: DropType;
source: T;
@ -63,12 +60,12 @@ export function onDrop(props: DatasourceDimensionDropHandlerProps<FormBasedPriva
}
if (!isOperation(source)) {
return false;
return;
}
const sourceDataView = indexPatterns[state.layers[source.layerId].indexPatternId];
const targetDataView = indexPatterns[state.layers[target.layerId].indexPatternId];
if (sourceDataView !== targetDataView) {
return false;
return;
}
const operationProps = {
@ -95,17 +92,18 @@ export function onDrop(props: DatasourceDimensionDropHandlerProps<FormBasedPriva
'replace_compatible',
].includes(dropType)
) {
return onMoveCompatible(operationProps);
return onMoveCompatible(operationProps, shouldRemoveSourceChecker(source, dropType));
}
if (
[
'duplicate_incompatible',
'replace_duplicate_incompatible',
'move_incompatible',
'replace_incompatible',
].includes(dropType)
) {
return onMoveIncompatible(operationProps);
return onMoveIncompatible(operationProps, shouldRemoveSourceChecker(source, dropType));
}
if (dropType === 'swap_compatible') {
return onSwapCompatible(operationProps);
@ -114,7 +112,7 @@ export function onDrop(props: DatasourceDimensionDropHandlerProps<FormBasedPriva
return onSwapIncompatible(operationProps);
}
if (['combine_incompatible', 'combine_compatible'].includes(dropType)) {
return onCombine(operationProps);
return onCombine(operationProps, shouldRemoveSourceChecker(source, dropType));
}
}
@ -122,7 +120,7 @@ const isFieldDropType = (dropType: DropType) =>
['field_add', 'field_replace', 'field_combine'].includes(dropType);
function onFieldDrop(props: DropHandlerProps<DraggedField>, shouldAddField?: boolean) {
const { setState, state, source, target, targetLayerDimensionGroups, indexPatterns } = props;
const { state, source, target, targetLayerDimensionGroups, indexPatterns } = props;
const prioritizedOperation = targetLayerDimensionGroups.find(
(g) => g.groupId === target.groupId
@ -154,7 +152,7 @@ function onFieldDrop(props: DropHandlerProps<DraggedField>, shouldAddField?: boo
(shouldAddField &&
!hasOperationSupportForMultipleFields(indexPattern, targetColumn, undefined, source.field))
) {
return false;
return;
}
const field = shouldAddField ? getField(targetColumn, indexPattern) : source.field;
const initialParams = shouldAddField
@ -179,17 +177,19 @@ function onFieldDrop(props: DropHandlerProps<DraggedField>, shouldAddField?: boo
shouldCombineField: shouldAddField,
initialParams,
});
setState(mergeLayer({ state, layerId: target.layerId, newLayer }));
return true;
return mergeLayer({ state, layerId: target.layerId, newLayer });
}
function onMoveCompatible({
setState,
state,
source,
target,
targetLayerDimensionGroups,
}: DropHandlerProps<DataViewDragDropOperation>) {
function onMoveCompatible(
{
state,
source,
target,
targetLayerDimensionGroups,
indexPatterns,
}: DropHandlerProps<DataViewDragDropOperation>,
shouldRemoveSource?: boolean
) {
let modifiedLayers = copyColumn({
layers: state.layers,
target,
@ -212,55 +212,47 @@ function onMoveCompatible({
},
};
setState(
mergeLayers({
state,
newLayers: modifiedLayers,
})
);
return true;
}
const newState = mergeLayers({
state,
newLayers: modifiedLayers,
});
function onReorder({
setState,
state,
source,
target,
}: DropHandlerProps<DataViewDragDropOperation>) {
function reorderElements(items: string[], targetId: string, sourceId: string) {
const result = items.filter((c) => c !== sourceId);
const targetIndex = items.findIndex((c) => c === sourceId);
const sourceIndex = items.findIndex((c) => c === targetId);
const targetPosition = result.indexOf(targetId);
result.splice(targetIndex < sourceIndex ? targetPosition + 1 : targetPosition, 0, sourceId);
return result;
if (shouldRemoveSource) {
return removeColumn({
layerId: source.layerId,
columnId: source.columnId,
prevState: newState,
indexPatterns,
});
}
setState(
mergeLayer({
state,
layerId: target.layerId,
newLayer: {
columnOrder: reorderElements(
state.layers[target.layerId].columnOrder,
target.columnId,
source.columnId
),
},
})
);
return true;
return newState;
}
function onMoveIncompatible({
setState,
state,
source,
targetLayerDimensionGroups,
target,
indexPatterns,
}: DropHandlerProps<DataViewDragDropOperation>) {
function onReorder({ state, source, target }: DropHandlerProps<DataViewDragDropOperation>) {
return mergeLayer({
state,
layerId: target.layerId,
newLayer: {
columnOrder: reorderElements(
state.layers[target.layerId].columnOrder,
target.columnId,
source.columnId
),
},
});
}
function onMoveIncompatible(
{
state,
source,
targetLayerDimensionGroups,
target,
indexPatterns,
}: DropHandlerProps<DataViewDragDropOperation>,
shouldRemoveSource?: boolean
) {
const targetLayer = state.layers[target.layerId];
const targetColumn = targetLayer.columns[target.columnId] || null;
const sourceLayer = state.layers[source.layerId];
@ -269,9 +261,11 @@ function onMoveIncompatible({
const sourceField = getField(sourceColumn, indexPattern);
const newOperation = getNewOperation(sourceField, target.filterOperations, targetColumn);
if (!newOperation) {
return false;
return;
}
let newState;
if (target.layerId === source.layerId) {
const newLayer = insertOrReplaceColumn({
layer: sourceLayer,
@ -283,14 +277,11 @@ function onMoveIncompatible({
targetGroup: target.groupId,
shouldResetLabel: true,
});
setState(
mergeLayer({
state,
layerId: target.layerId,
newLayer,
})
);
return true;
newState = mergeLayer({
state,
layerId: target.layerId,
newLayer,
});
} else {
const outputTargetLayer = insertOrReplaceColumn({
layer: targetLayer,
@ -302,21 +293,28 @@ function onMoveIncompatible({
targetGroup: target.groupId,
shouldResetLabel: true,
});
setState(
mergeLayers({
state,
newLayers: {
[source.layerId]: sourceLayer,
[target.layerId]: outputTargetLayer,
},
})
);
return true;
newState = mergeLayers({
state,
newLayers: {
[source.layerId]: sourceLayer,
[target.layerId]: outputTargetLayer,
},
});
}
if (shouldRemoveSource) {
return removeColumn({
layerId: source.layerId,
columnId: source.columnId,
prevState: newState,
indexPatterns,
});
}
return newState;
}
function onSwapIncompatible({
setState,
state,
source,
targetLayerDimensionGroups,
@ -336,7 +334,7 @@ function onSwapIncompatible({
const newOperationForTarget = getNewOperation(targetField, source.filterOperations, sourceColumn);
if (!newOperationForSource || !newOperationForTarget) {
return false;
return;
}
const outputTargetLayer = insertOrReplaceColumn({
@ -361,14 +359,11 @@ function onSwapIncompatible({
targetGroup: source.groupId,
shouldResetLabel: true,
});
setState(
mergeLayer({
state,
layerId: target.layerId,
newLayer,
})
);
return true;
return mergeLayer({
state,
layerId: target.layerId,
newLayer,
});
} else {
const outputSourceLayer = insertOrReplaceColumn({
layer: sourceLayer,
@ -380,13 +375,10 @@ function onSwapIncompatible({
targetGroup: source.groupId,
shouldResetLabel: true,
});
setState(
mergeLayers({
state,
newLayers: { [source.layerId]: outputSourceLayer, [target.layerId]: outputTargetLayer },
})
);
return true;
return mergeLayers({
state,
newLayers: { [source.layerId]: outputSourceLayer, [target.layerId]: outputTargetLayer },
});
}
}
@ -402,7 +394,6 @@ const swapColumnOrder = (columnOrder: string[], sourceId: string, targetId: stri
};
function onSwapCompatible({
setState,
state,
source,
targetLayerDimensionGroups,
@ -424,18 +415,14 @@ function onSwapCompatible({
target.columnId
);
setState(
mergeLayer({
state,
layerId: target.layerId,
newLayer: {
columnOrder: updatedColumnOrder,
columns: newColumns,
},
})
);
return true;
return mergeLayer({
state,
layerId: target.layerId,
newLayer: {
columnOrder: updatedColumnOrder,
columns: newColumns,
},
});
} else {
// TODO why not reorderByGroups for both columns? Are they already in that order?
const newTargetLayer = copyColumn({
@ -452,28 +439,26 @@ function onSwapCompatible({
shouldDeleteSource: true,
})[source.layerId];
setState(
mergeLayers({
state,
newLayers: {
[source.layerId]: newSourceLayer,
[target.layerId]: newTargetLayer,
},
})
);
return true;
return mergeLayers({
state,
newLayers: {
[source.layerId]: newSourceLayer,
[target.layerId]: newTargetLayer,
},
});
}
}
function onCombine({
state,
setState,
source,
target,
targetLayerDimensionGroups,
indexPatterns,
}: DropHandlerProps<DataViewDragDropOperation>) {
function onCombine(
{
state,
source,
target,
targetLayerDimensionGroups,
indexPatterns,
}: DropHandlerProps<DataViewDragDropOperation>,
shouldRemoveSource?: boolean
) {
const targetLayer = state.layers[target.layerId];
const targetColumn = targetLayer.columns[target.columnId];
const targetField = getField(targetColumn, target.dataView);
@ -484,7 +469,7 @@ function onCombine({
const sourceField = getField(sourceColumn, indexPattern);
// extract the field from the source column
if (!sourceField || !targetField) {
return false;
return;
}
// pass it to the target column and delete the source column
const initialParams = {
@ -508,8 +493,19 @@ function onCombine({
shouldCombineField: true,
});
setState(
mergeLayers({ state, newLayers: { ...state.layers, [target.layerId]: outputTargetLayer } })
);
return true;
const newState = mergeLayers({
state,
newLayers: { ...state.layers, [target.layerId]: outputTargetLayer },
});
if (shouldRemoveSource) {
return removeColumn({
layerId: source.layerId,
columnId: source.columnId,
prevState: newState,
indexPatterns,
});
}
return newState;
}

View file

@ -6,7 +6,7 @@
*/
import memoizeOne from 'memoize-one';
import { DatasourceDimensionDropProps, IndexPatternMap, OperationMetadata } from '../../../types';
import { DatasourceDimensionProps, IndexPatternMap, OperationMetadata } from '../../../types';
import { OperationType } from '../form_based';
import { memoizedGetAvailableOperationsByMetadata, OperationFieldTuple } from '../operations';
import { FormBasedPrivateState } from '../types';
@ -18,9 +18,12 @@ export interface OperationSupportMatrix {
}
type Props = Pick<
DatasourceDimensionDropProps<FormBasedPrivateState>['target'],
DatasourceDimensionProps<FormBasedPrivateState>,
'layerId' | 'columnId' | 'filterOperations'
> & { state: FormBasedPrivateState; indexPatterns: IndexPatternMap };
> & {
state: FormBasedPrivateState;
indexPatterns: IndexPatternMap;
};
function computeOperationMatrix(
operationsByMetadata: Array<{

View file

@ -135,6 +135,24 @@ function getSortingHint(column: GenericIndexPatternColumn, dataView?: IndexPatte
}
}
export const removeColumn: Datasource<FormBasedPrivateState>['removeColumn'] = ({
prevState,
layerId,
columnId,
indexPatterns,
}) => {
const indexPattern = indexPatterns[prevState.layers[layerId]?.indexPatternId];
return mergeLayer({
state: prevState,
layerId,
newLayer: deleteColumn({
layer: prevState.layers[layerId],
columnId,
indexPattern,
}),
});
};
export function columnToOperation(
column: GenericIndexPatternColumn,
uniqueLabel?: string,
@ -300,18 +318,7 @@ export function getFormBasedDatasource({
return Object.keys(state?.layers);
},
removeColumn({ prevState, layerId, columnId, indexPatterns }) {
const indexPattern = indexPatterns[prevState.layers[layerId]?.indexPatternId];
return mergeLayer({
state: prevState,
layerId,
newLayer: deleteColumn({
layer: prevState.layers[layerId],
columnId,
indexPattern,
}),
});
},
removeColumn,
initializeDimension(
state,

View file

@ -546,7 +546,7 @@ export function getTextBasedDatasource({
const allColumns = currentLayer.allColumns.filter((c) => c.columnId !== target.columnId);
allColumns.push(newColumn);
props.setState({
return {
...props.state,
layers: {
...props.state.layers,
@ -556,11 +556,10 @@ export function getTextBasedDatasource({
allColumns,
},
},
});
};
});
return true;
}
return false;
return undefined;
},
getPublicAPI({ state, layerId, indexPatterns }: PublicAPIProps<TextBasedPrivateState>) {

View file

@ -23,29 +23,17 @@ export interface OnVisDropProps<T> {
group?: VisualizationDimensionGroupConfig;
}
export function shouldRemoveSource(source: DragDropIdentifier, dropType: DropType) {
return (
isOperation(source) &&
(dropType === 'move_compatible' ||
dropType === 'move_incompatible' ||
dropType === 'combine_incompatible' ||
dropType === 'combine_compatible' ||
dropType === 'replace_compatible' ||
dropType === 'replace_incompatible')
);
}
export function onDropForVisualization<T, P = unknown, E = unknown>(
props: OnVisDropProps<T>,
activeVisualization: Visualization<T, P, E>
) {
const { prevState, target, frame, source, group } = props;
const { prevState, target, frame, source, group, dropType } = props;
const { layerId, columnId, groupId } = target;
const previousColumn =
isOperation(source) && group?.requiresPreviousColumnOnDuplicate ? source.columnId : undefined;
const newVisState = activeVisualization.setDimension({
let newVisState = activeVisualization.setDimension({
columnId,
groupId,
layerId,
@ -54,5 +42,21 @@ export function onDropForVisualization<T, P = unknown, E = unknown>(
frame,
});
if (
isOperation(source) &&
(dropType === 'move_compatible' ||
dropType === 'move_incompatible' ||
dropType === 'combine_incompatible' ||
dropType === 'combine_compatible' ||
dropType === 'replace_compatible' ||
dropType === 'replace_incompatible')
)
newVisState = activeVisualization.removeDimension({
layerId: source.layerId,
columnId: source.columnId,
prevState: newVisState,
frame,
});
return newVisState;
}

View file

@ -15,8 +15,13 @@ import {
UPDATE_FILTER_REFERENCES_TRIGGER,
} from '@kbn/unified-search-plugin/public';
import { isEqual } from 'lodash';
import { changeIndexPattern, removeDimension } from '../../../state_management/lens_slice';
import { AddLayerFunction, Visualization } from '../../../types';
import { DragDropIdentifier, DropType } from '@kbn/dom-drag-drop';
import {
changeIndexPattern,
onDimensionDrop,
removeDimension,
} from '../../../state_management/lens_slice';
import { AddLayerFunction, DragDropOperation, Visualization } from '../../../types';
import { LayerPanel } from './layer_panel';
import { generateId } from '../../../id_generator';
import { ConfigPanelWrapperProps } from './types';
@ -149,6 +154,7 @@ export function LayerPanels(
updateVisualizationState({
visualizationId: activeVisualization.id,
newState: newVisState,
dontSyncLinkedDimensions: true, // TODO: to refactor: this is quite brittle, we avoid to sync linked dimensions because we do it with datasourceState update
})
);
dispatchLens(
@ -163,10 +169,13 @@ export function LayerPanels(
[dispatchLens, onUpdateStateCb, visualization.state, datasourceStates, activeVisualization.id]
);
const toggleFullscreen = useMemo(
() => () => {
dispatchLens(setToggleFullscreen());
},
const toggleFullscreen = useCallback(() => {
dispatchLens(setToggleFullscreen());
}, [dispatchLens]);
const handleDimensionDrop = useCallback(
(payload: { source: DragDropIdentifier; target: DragDropOperation; dropType: DropType }) =>
dispatchLens(onDimensionDrop(payload)),
[dispatchLens]
);
@ -288,6 +297,7 @@ export function LayerPanels(
!hidden && (
<LayerPanel
{...props}
onDimensionDrop={handleDimensionDrop}
registerLibraryAnnotationGroup={registerLibraryAnnotationGroupFunction}
dimensionGroups={groups}
activeVisualization={activeVisualization}

View file

@ -64,6 +64,8 @@ const draggingField = {
},
};
const onDimensionDrop = jest.fn();
describe('LayerPanel', () => {
let mockVisualization: jest.Mocked<Visualization>;
let mockVisualization2: jest.Mocked<Visualization>;
@ -107,6 +109,7 @@ describe('LayerPanel', () => {
indexPatternService: createIndexPatternServiceMock(),
getUserMessages: () => [],
displayLayerSettings: true,
onDimensionDrop,
};
}
@ -141,6 +144,8 @@ describe('LayerPanel', () => {
mockDatasource = createMockDatasource('testDatasource');
});
afterEach(() => onDimensionDrop.mockClear());
describe('layer reset and remove', () => {
it('should show the reset button when single layer', async () => {
const { instance } = await mountWithProvider(<LayerPanel {...getDefaultProps()} />);
@ -777,7 +782,7 @@ describe('LayerPanel', () => {
dragDropElement.simulate('dragOver');
dragDropElement.simulate('drop');
expect(mockDatasource.onDrop).toHaveBeenCalledWith(
expect(onDimensionDrop).toHaveBeenCalledWith(
expect.objectContaining({
source: draggingField,
})
@ -827,7 +832,7 @@ describe('LayerPanel', () => {
dragDropElement.simulate('dragOver');
dragDropElement.simulate('drop');
expect(mockDatasource.onDrop).not.toHaveBeenCalled();
expect(onDimensionDrop).not.toHaveBeenCalled();
});
it('should allow drag to move between groups', async () => {
@ -891,13 +896,15 @@ describe('LayerPanel', () => {
dragDropElement.simulate('dragOver');
dragDropElement.simulate('drop');
expect(mockDatasource.onDrop).toHaveBeenCalledWith(
expect(onDimensionDrop).toHaveBeenCalledWith(
expect.objectContaining({
target: expect.objectContaining({ columnId: 'b' }),
source: draggingOperation,
})
);
onDimensionDrop.mockClear();
// Simulate drop on the empty dimension
const updatedDragDropElement = instance
@ -907,7 +914,7 @@ describe('LayerPanel', () => {
updatedDragDropElement.simulate('dragOver');
updatedDragDropElement.simulate('drop');
expect(mockDatasource.onDrop).toHaveBeenCalledWith(
expect(onDimensionDrop).toHaveBeenCalledWith(
expect.objectContaining({
target: expect.objectContaining({ columnId: 'newid' }),
source: draggingOperation,
@ -951,7 +958,7 @@ describe('LayerPanel', () => {
act(() => {
instance.find(DragDrop).at(1).prop('onDrop')!(draggingOperation, 'reorder');
});
expect(mockDatasource.onDrop).toHaveBeenCalledWith(
expect(onDimensionDrop).toHaveBeenCalledWith(
expect.objectContaining({
dropType: 'reorder',
source: draggingOperation,
@ -1002,7 +1009,7 @@ describe('LayerPanel', () => {
act(() => {
instance.find(DragDrop).at(2).prop('onDrop')!(draggingOperation, 'duplicate_compatible');
});
expect(mockDatasource.onDrop).toHaveBeenCalledWith(
expect(onDimensionDrop).toHaveBeenCalledWith(
expect.objectContaining({
target: expect.objectContaining({ columnId: 'newid' }),
dropType: 'duplicate_compatible',
@ -1045,43 +1052,20 @@ describe('LayerPanel', () => {
humanData: { label: 'Label' },
};
mockDatasource.onDrop.mockReturnValue(true);
const updateVisualization = jest.fn();
const mockOnRemoveDimension = jest.fn();
const { instance } = await mountWithProvider(
<ChildDragDropProvider value={createMockedDragDropContext({ dragging: draggingOperation })}>
<LayerPanel
{...getDefaultProps()}
onRemoveDimension={mockOnRemoveDimension}
updateVisualization={updateVisualization}
activeVisualization={mockVis}
/>
<LayerPanel {...getDefaultProps()} activeVisualization={mockVis} />
</ChildDragDropProvider>
);
act(() => {
instance.find(DragDrop).at(3).prop('onDrop')!(draggingOperation, 'replace_compatible');
});
expect(mockDatasource.onDrop).toHaveBeenCalledWith(
expect(onDimensionDrop).toHaveBeenCalledWith(
expect.objectContaining({
dropType: 'replace_compatible',
source: draggingOperation,
})
);
// testing default onDropForVisualization path
expect(mockVis.setDimension).toHaveBeenCalledWith(
expect.objectContaining({
columnId: 'c',
groupId: 'b',
layerId: 'first',
prevState: 'state',
})
);
expect(mockOnRemoveDimension).toHaveBeenCalledWith({
columnId: 'a',
layerId: 'first',
});
expect(updateVisualization).toHaveBeenCalledTimes(1);
});
it('should call onDrop and update visualization when replacing between compatible groups2', async () => {
const mockVis = {
@ -1122,13 +1106,11 @@ describe('LayerPanel', () => {
mockDatasource.onDrop.mockReturnValue(true);
const updateVisualization = jest.fn();
const mockOnRemoveDimension = jest.fn();
const { instance } = await mountWithProvider(
<ChildDragDropProvider value={createMockedDragDropContext({ dragging: draggingOperation })}>
<LayerPanel
{...getDefaultProps()}
onRemoveDimension={mockOnRemoveDimension}
updateVisualization={updateVisualization}
activeVisualization={mockVis}
/>
@ -1138,33 +1120,18 @@ describe('LayerPanel', () => {
instance.find(DragDrop).at(3).prop('onDrop')!(draggingOperation, 'replace_compatible');
});
expect(mockDatasource.onDrop).toHaveBeenCalledWith(
expect(onDimensionDrop).toHaveBeenCalledWith(
expect.objectContaining({
dropType: 'replace_compatible',
source: draggingOperation,
})
);
expect(mockVis.onDrop).toHaveBeenCalledWith(
expect.objectContaining({
dropType: 'replace_compatible',
prevState: 'state',
source: draggingOperation,
target: expect.objectContaining({
columnId: 'c',
groupId: 'b',
id: 'c',
layerId: 'first',
}),
}),
mockVis
})
);
expect(mockVis.setDimension).not.toHaveBeenCalled();
expect(mockOnRemoveDimension).toHaveBeenCalledWith({
columnId: 'a',
layerId: 'first',
});
expect(updateVisualization).toHaveBeenCalledTimes(1);
});
it('should not change visualization state if datasource drop failed', async () => {
@ -1204,13 +1171,11 @@ describe('LayerPanel', () => {
mockDatasource.onDrop.mockReturnValue(false);
const updateVisualization = jest.fn();
const mockOnRemoveDimension = jest.fn();
const { instance } = await mountWithProvider(
<ChildDragDropProvider value={createMockedDragDropContext({ dragging: draggingOperation })}>
<LayerPanel
{...getDefaultProps()}
onRemoveDimension={mockOnRemoveDimension}
updateVisualization={updateVisualization}
activeVisualization={mockVis}
/>
@ -1220,14 +1185,13 @@ describe('LayerPanel', () => {
instance.find(DragDrop).at(3).prop('onDrop')!(draggingOperation, 'replace_compatible');
});
expect(mockDatasource.onDrop).toHaveBeenCalledWith(
expect(onDimensionDrop).toHaveBeenCalledWith(
expect.objectContaining({
dropType: 'replace_compatible',
source: draggingOperation,
})
);
expect(updateVisualization).not.toHaveBeenCalled();
expect(mockOnRemoveDimension).not.toHaveBeenCalled();
});
it("should not remove source if drop type doesn't require it", async () => {
@ -1267,22 +1231,14 @@ describe('LayerPanel', () => {
mockDatasource.onDrop.mockReturnValue(true);
const mockOnRemoveDimension = jest.fn();
const { instance } = await mountWithProvider(
<ChildDragDropProvider value={createMockedDragDropContext({ dragging: draggingOperation })}>
<LayerPanel
{...getDefaultProps()}
onRemoveDimension={mockOnRemoveDimension}
activeVisualization={mockVis}
/>
<LayerPanel {...getDefaultProps()} activeVisualization={mockVis} />
</ChildDragDropProvider>
);
act(() => {
instance.find(DragDrop).at(3).prop('onDrop')!(draggingOperation, 'duplicate_compatible');
});
expect(mockOnRemoveDimension).not.toHaveBeenCalled();
});
});

View file

@ -27,13 +27,13 @@ import { IndexPatternServiceAPI } from '../../../data_views_service/service';
import {
StateSetter,
Visualization,
DragDropOperation,
isOperation,
LayerAction,
VisualizationDimensionGroupConfig,
UserMessagesGetter,
AddLayerFunction,
RegisterLibraryAnnotationGroupFunction,
DragDropOperation,
} from '../../../types';
import { LayerSettings } from './layer_settings';
import { LayerPanelProps, ActiveDimensionState } from './types';
@ -47,7 +47,6 @@ import {
selectResolvedDateRange,
selectDatasourceStates,
} from '../../../state_management';
import { onDropForVisualization, shouldRemoveSource } from './buttons/drop_targets_utils';
import { getSharedActions } from './layer_actions/layer_actions';
import { FlyoutContainer } from '../../../shared_components/flyout_container';
@ -65,6 +64,11 @@ export function LayerPanel(
addLayer: AddLayerFunction;
registerLibraryAnnotationGroup: RegisterLibraryAnnotationGroupFunction;
updateVisualization: StateSetter<unknown>;
onDimensionDrop: (payload: {
source: DragDropIdentifier;
target: DragDropOperation;
dropType: DropType;
}) => void;
updateDatasource: (
datasourceId: string | undefined,
newState: unknown,
@ -118,6 +122,7 @@ export function LayerPanel(
visualizationState,
onChangeIndexPattern,
core,
onDimensionDrop,
} = props;
const isSaveable = useLensSelector((state) => state.lens.isSaveable);
@ -191,8 +196,8 @@ export function LayerPanel(
registerNewRef: registerNewButtonRef,
} = useFocusUpdate(allAccessors);
const onDrop = useMemo(() => {
return (source: DragDropIdentifier, target: DragDropIdentifier, dropType?: DropType) => {
const onDrop = useCallback(
(source: DragDropIdentifier, target: DragDropIdentifier, dropType?: DropType) => {
if (!dropType) {
return;
}
@ -206,65 +211,10 @@ export function LayerPanel(
setNextFocusedButtonId(target.columnId);
}
let hasDropSucceeded = true;
if (layerDatasource) {
hasDropSucceeded = Boolean(
layerDatasource?.onDrop({
state: layerDatasourceState,
setState: (newState: unknown) => {
// we don't sync linked dimension here because that would trigger an onDrop routine within an onDrop routine
updateDatasource(datasourceId, newState, true);
},
source,
target: {
...(target as unknown as DragDropOperation),
filterOperations:
dimensionGroups.find(({ groupId: gId }) => gId === target.groupId)
?.filterOperations || Boolean,
},
targetLayerDimensionGroups: dimensionGroups,
dropType,
indexPatterns: framePublicAPI.dataViews.indexPatterns,
})
);
}
if (hasDropSucceeded) {
activeVisualization.onDrop = activeVisualization.onDrop?.bind(activeVisualization);
updateVisualization(
(activeVisualization.onDrop || onDropForVisualization)?.(
{
prevState: props.visualizationState,
frame: framePublicAPI,
target,
source,
dropType,
group: dimensionGroups.find(({ groupId: gId }) => gId === target.groupId),
},
activeVisualization
)
);
if (isOperation(source) && shouldRemoveSource(source, dropType)) {
props.onRemoveDimension({
columnId: source.columnId,
layerId: source.layerId,
});
}
}
};
}, [
layerDatasource,
setNextFocusedButtonId,
layerDatasourceState,
dimensionGroups,
framePublicAPI,
updateDatasource,
datasourceId,
activeVisualization,
updateVisualization,
props,
]);
onDimensionDrop({ source, target, dropType });
},
[setNextFocusedButtonId, onDimensionDrop]
);
const isDimensionPanelOpen = Boolean(activeId);

View file

@ -42,6 +42,7 @@ export const {
removeOrClearLayer,
cloneLayer,
addLayer,
onDimensionDrop,
setLayerDefaultDimension,
removeDimension,
setIsLoadLibraryVisible,
@ -62,6 +63,8 @@ export const makeConfigureStore = (
'payload.history',
'payload.newState.dataViews',
'lens.activeData',
'payload.source.filterOperations',
'payload.target.filterOperations',
],
ignoredPaths: ['lens.dataViews.indexPatterns'],
},

View file

@ -12,6 +12,7 @@ import { Query } from '@kbn/es-query';
import { History } from 'history';
import { LayerTypes } from '@kbn/expression-xy-plugin/public';
import { EventAnnotationGroupConfig } from '@kbn/event-annotation-common';
import { DragDropIdentifier, DropType } from '@kbn/dom-drag-drop';
import { LensEmbeddableInput } from '..';
import { TableInspectorAdapter } from '../editor_frame_service/types';
import type {
@ -20,6 +21,7 @@ import type {
IndexPattern,
VisualizationMap,
DatasourceMap,
DragDropOperation,
} from '../types';
import { getInitialDatasourceId, getResolvedDateRange, getRemoveOperation } from '../utils';
import type { DataViewsState, LensAppState, LensStoreDeps, VisualizationState } from './types';
@ -156,6 +158,7 @@ export const updateDatasourceState = createAction<{
export const updateVisualizationState = createAction<{
visualizationId: string;
newState: unknown;
dontSyncLinkedDimensions?: boolean;
}>('lens/updateVisualizationState');
export const insertLayer = createAction<{
@ -229,6 +232,11 @@ export const addLayer = createAction<{
extraArg: unknown;
ignoreInitialValues?: boolean;
}>('lens/addLayer');
export const onDimensionDrop = createAction<{
source: DragDropIdentifier;
target: DragDropOperation;
dropType: DropType;
}>('lens/onDimensionDrop');
export const setLayerDefaultDimension = createAction<{
layerId: string;
@ -284,6 +292,7 @@ export const lensActions = {
removeLayers,
removeOrClearLayer,
addLayer,
onDimensionDrop,
cloneLayer,
setLayerDefaultDimension,
updateIndexPatterns,
@ -610,43 +619,28 @@ export const makeLensReducer = (storeDeps: LensStoreDeps) => {
};
}
) => {
const currentState = current(state);
if (payload.clearStagedPreview) {
state.stagedPreview = undefined;
}
const newAppState: LensAppState = {
...currentState,
datasourceStates: {
...currentState.datasourceStates,
[payload.datasourceId]: {
state: payload.newDatasourceState,
isLoading: false,
},
},
stagedPreview: payload.clearStagedPreview ? undefined : currentState.stagedPreview,
state.datasourceStates[payload.datasourceId] = {
state: payload.newDatasourceState,
isLoading: false,
};
if (payload.dontSyncLinkedDimensions) {
return newAppState;
return;
}
const currentState = current(state);
const {
datasourceState: syncedDatasourceState,
visualizationState: syncedVisualizationState,
} = syncLinkedDimensions(newAppState, visualizationMap, datasourceMap, payload.datasourceId);
} = syncLinkedDimensions(currentState, visualizationMap, datasourceMap, payload.datasourceId);
return {
...newAppState,
visualization: {
...newAppState.visualization,
state: syncedVisualizationState,
},
datasourceStates: {
...newAppState.datasourceStates,
[payload.datasourceId]: {
state: syncedDatasourceState,
isLoading: false,
},
},
};
state.visualization.state = syncedVisualizationState;
state.datasourceStates[payload.datasourceId].state = syncedDatasourceState;
},
[updateVisualizationState.type]: (
state,
@ -656,6 +650,7 @@ export const makeLensReducer = (storeDeps: LensStoreDeps) => {
payload: {
visualizationId: string;
newState: unknown;
dontSyncLinkedDimensions?: boolean;
};
}
) => {
@ -675,6 +670,10 @@ export const makeLensReducer = (storeDeps: LensStoreDeps) => {
return;
}
if (payload.dontSyncLinkedDimensions) {
return;
}
// TODO - consolidate into applySyncLinkedDimensions
const {
datasourceState: syncedDatasourceState,
@ -987,6 +986,7 @@ export const makeLensReducer = (storeDeps: LensStoreDeps) => {
state.visualization.state =
typeof updater === 'function' ? updater(current(state.visualization.state)) : updater;
}
layerIds.forEach((layerId) => {
const [layerDatasourceId] =
Object.entries(datasourceMap).find(([datasourceId, datasource]) => {
@ -1080,6 +1080,87 @@ export const makeLensReducer = (storeDeps: LensStoreDeps) => {
state.datasourceStates[state.activeDatasourceId].state = syncedDatasourceState;
state.visualization.state = syncedVisualizationState;
},
[onDimensionDrop.type]: (
state,
{
payload: { source, target, dropType },
}: {
payload: {
source: DragDropIdentifier;
target: DragDropOperation;
dropType: DropType;
};
}
) => {
if (!state.visualization.activeId) {
return state;
}
const activeVisualization = visualizationMap[state.visualization.activeId];
const framePublicAPI = selectFramePublicAPI({ lens: current(state) }, datasourceMap);
const { groups } = activeVisualization.getConfiguration({
layerId: target.layerId,
frame: framePublicAPI,
state: state.visualization.state,
});
const [layerDatasourceId, layerDatasource] =
Object.entries(datasourceMap).find(
([datasourceId, datasource]) =>
state.datasourceStates[datasourceId] &&
datasource
.getLayers(state.datasourceStates[datasourceId].state)
.includes(target.layerId)
) || [];
let newDatasourceState;
if (layerDatasource && layerDatasourceId) {
newDatasourceState = layerDatasource?.onDrop({
state: state.datasourceStates[layerDatasourceId].state,
source,
target: {
...(target as unknown as DragDropOperation),
filterOperations:
groups.find(({ groupId: gId }) => gId === target.groupId)?.filterOperations ||
Boolean,
},
targetLayerDimensionGroups: groups,
dropType,
indexPatterns: framePublicAPI.dataViews.indexPatterns,
});
if (!newDatasourceState) {
return;
}
state.datasourceStates[layerDatasourceId].state = newDatasourceState;
}
activeVisualization.onDrop = activeVisualization.onDrop?.bind(activeVisualization);
const newVisualizationState = (activeVisualization.onDrop || onDropForVisualization)?.(
{
prevState: state.visualization.state,
frame: framePublicAPI,
target,
source,
dropType,
group: groups.find(({ groupId: gId }) => gId === target.groupId),
},
activeVisualization
);
state.visualization.state = newVisualizationState;
if (layerDatasourceId) {
const {
datasourceState: syncedDatasourceState,
visualizationState: syncedVisualizationState,
} = syncLinkedDimensions(current(state), visualizationMap, datasourceMap);
state.datasourceStates[layerDatasourceId].state = syncedDatasourceState;
state.visualization.state = syncedVisualizationState;
}
},
[setLayerDefaultDimension.type]: (
state,
{

View file

@ -379,7 +379,7 @@ export interface Datasource<T = unknown, P = unknown> {
getDropProps: (
props: GetDropPropsArgs<T>
) => { dropTypes: DropType[]; nextLabel?: string } | undefined;
onDrop: (props: DatasourceDimensionDropHandlerProps<T>) => boolean | undefined;
onDrop: (props: DatasourceDimensionDropHandlerProps<T>) => T | undefined;
getCustomWorkspaceRenderer?: (
state: T,
dragging: DraggingIdentifier,
@ -684,24 +684,14 @@ export function isOperation(operationCandidate: unknown): operationCandidate is
);
}
export interface DatasourceDimensionDropProps<T> {
export interface DatasourceDimensionDropHandlerProps<T> {
target: DragDropOperation;
state: T;
setState: StateSetter<
T,
{
isDimensionComplete?: boolean;
forceRender?: boolean;
}
>;
targetLayerDimensionGroups: VisualizationDimensionGroupConfig[];
}
export type DatasourceDimensionDropHandlerProps<S> = DatasourceDimensionDropProps<S> & {
source: DragDropIdentifier;
dropType: DropType;
indexPatterns: IndexPatternMap;
};
}
export type FieldOnlyDataType =
| 'document'

View file

@ -22,7 +22,7 @@ import {
import { emptyTitleText } from '@kbn/visualization-ui-components';
import { RequestAdapter } from '@kbn/inspector-plugin/common';
import { ISearchStart } from '@kbn/data-plugin/public';
import type { DraggingIdentifier } from '@kbn/dom-drag-drop';
import type { DraggingIdentifier, DropType } from '@kbn/dom-drag-drop';
import type { Document } from './persistence/saved_object_store';
import {
Datasource,
@ -389,3 +389,28 @@ export function getUniqueLabelGenerator() {
export function nonNullable<T>(v: T): v is NonNullable<T> {
return v != null;
}
export function reorderElements(items: string[], targetId: string, sourceId: string) {
const result = items.filter((c) => c !== sourceId);
const targetIndex = items.findIndex((c) => c === sourceId);
const sourceIndex = items.findIndex((c) => c === targetId);
const targetPosition = result.indexOf(targetId);
result.splice(targetIndex < sourceIndex ? targetPosition + 1 : targetPosition, 0, sourceId);
return result;
}
export function shouldRemoveSource(
source: unknown,
dropType: DropType
): source is DragDropOperation {
return (
isOperation(source) &&
(dropType === 'move_compatible' ||
dropType === 'move_incompatible' ||
dropType === 'combine_incompatible' ||
dropType === 'combine_compatible' ||
dropType === 'replace_compatible' ||
dropType === 'replace_incompatible')
);
}

View file

@ -362,7 +362,6 @@ export const onAnnotationDrop: Visualization<XYState>['onDrop'] = ({
default:
return prevState;
}
return prevState;
};
export const setAnnotationsDimension: Visualization<XYState>['setDimension'] = ({

View file

@ -108,14 +108,6 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
const trendLineData = await inspector.getTableDataWithId('inspectorTableChooser1');
expect(trendLineData).to.eql(inspectorTrendlineData);
await inspector.close();
await PageObjects.lens.openDimensionEditor(
'lnsMetric_primaryMetricDimensionPanel > lns-dimensionTrigger'
);
await testSubjects.click('lnsMetric_supporting_visualization_none');
await PageObjects.lens.closeDimensionEditor();
await PageObjects.lens.waitForVisualization('mtrVis');
});
it('should enable metric with breakdown', async () => {
@ -136,7 +128,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
extraText: 'Average of bytes 19,755',
value: '19,755',
color: 'rgba(245, 247, 250, 1)',
showingTrendline: false,
showingTrendline: true,
showingBar: false,
},
{
@ -145,7 +137,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
extraText: 'Average of bytes 18,994',
value: '18,994',
color: 'rgba(245, 247, 250, 1)',
showingTrendline: false,
showingTrendline: true,
showingBar: false,
},
{
@ -154,7 +146,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
extraText: 'Average of bytes 17,246',
value: '17,246',
color: 'rgba(245, 247, 250, 1)',
showingTrendline: false,
showingTrendline: true,
showingBar: false,
},
{
@ -163,7 +155,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
extraText: 'Average of bytes 15,687',
value: '15,687',
color: 'rgba(245, 247, 250, 1)',
showingTrendline: false,
showingTrendline: true,
showingBar: false,
},
{
@ -172,7 +164,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
extraText: 'Average of bytes 15,614.333',
value: '15,614.333',
color: 'rgba(245, 247, 250, 1)',
showingTrendline: false,
showingTrendline: true,
showingBar: false,
},
{
@ -181,11 +173,20 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
extraText: 'Average of bytes 5,722.775',
value: '5,722.775',
color: 'rgba(245, 247, 250, 1)',
showingTrendline: false,
showingTrendline: true,
showingBar: false,
},
];
expect(data).to.eql(expectedData);
await PageObjects.lens.openDimensionEditor(
'lnsMetric_primaryMetricDimensionPanel > lns-dimensionTrigger'
);
await testSubjects.click('lnsMetric_supporting_visualization_none');
await PageObjects.lens.closeDimensionEditor();
await PageObjects.lens.waitForVisualization('mtrVis');
});
it('should enable bar with max dimension', async () => {