Skip to content

Commit

Permalink
Simplify handling of flyout with incomplete columns
Browse files Browse the repository at this point in the history
  • Loading branch information
wylieconlon committed Jun 8, 2021
1 parent 7f5dbea commit 4b81e5d
Show file tree
Hide file tree
Showing 8 changed files with 228 additions and 140 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ describe('LayerPanel', () => {
it('should not update the visualization if the datasource is incomplete', () => {
(generateId as jest.Mock).mockReturnValue(`newid`);
const updateAll = jest.fn();
const updateDatasource = jest.fn();
const updateDatasourceAsync = jest.fn();

mockVisualization.getConfiguration.mockReturnValue({
groups: [
Expand All @@ -276,7 +276,7 @@ describe('LayerPanel', () => {
const component = mountWithIntl(
<LayerPanel
{...getDefaultProps()}
updateDatasource={updateDatasource}
updateDatasourceAsync={updateDatasourceAsync}
updateAll={updateAll}
/>
);
Expand All @@ -295,27 +295,108 @@ describe('LayerPanel', () => {
mockDatasource.renderDimensionEditor.mock.calls.length - 1
][1].setState;

act(() => {
stateFn(
{
indexPatternId: '1',
columns: {},
columnOrder: [],
incompleteColumns: { newId: { operationType: 'count' } },
},
{ isDimensionComplete: false }
);
});
expect(updateAll).not.toHaveBeenCalled();
expect(updateDatasourceAsync).toHaveBeenCalled();

act(() => {
stateFn({
indexPatternId: '1',
columns: {},
columnOrder: [],
incompleteColumns: { newId: { operationType: 'count' } },
});
});
expect(updateAll).not.toHaveBeenCalled();
expect(updateAll).toHaveBeenCalled();
});

it('should remove the dimension when the datasource marks it as removed', () => {
const updateAll = jest.fn();
const updateDatasource = jest.fn();

mockVisualization.getConfiguration.mockReturnValue({
groups: [
{
groupLabel: 'A',
groupId: 'a',
accessors: [{ columnId: 'y' }],
filterOperations: () => true,
supportsMoreColumns: true,
dataTestSubj: 'lnsGroup',
},
],
});

const component = mountWithIntl(
<LayerPanel
{...getDefaultProps()}
datasourceStates={{
ds1: {
isLoading: false,
state: {
layers: [
{
indexPatternId: '1',
columns: {
y: {
operationType: 'moving_average',
references: ['ref'],
},
},
columnOrder: ['y'],
incompleteColumns: {},
},
],
},
},
}}
updateDatasource={updateDatasource}
updateAll={updateAll}
/>
);

act(() => {
component.find('[data-test-subj="lnsLayerPanel-dimensionLink"]').first().simulate('click');
});
component.update();

expect(mockDatasource.renderDimensionEditor).toHaveBeenCalledWith(
expect.any(Element),
expect.objectContaining({ columnId: 'y' })
);
const stateFn =
mockDatasource.renderDimensionEditor.mock.calls[
mockDatasource.renderDimensionEditor.mock.calls.length - 1
][1].setState;

act(() => {
stateFn(
{
indexPatternId: '1',
columns: {},
columnOrder: [],
incompleteColumns: { y: { operationType: 'average' } },
},
{ shouldReplaceDimension: true }
{
isDimensionComplete: false,
}
);
});
expect(updateAll).toHaveBeenCalled();
expect(mockVisualization.removeDimension).toHaveBeenCalledWith(
expect.objectContaining({
columnId: 'y',
})
);
});

it('should keep the DimensionContainer open when configuring a new dimension', () => {
Expand All @@ -334,6 +415,7 @@ describe('LayerPanel', () => {
accessors: [],
filterOperations: () => true,
supportsMoreColumns: true,
enableDimensionEditor: true,
dataTestSubj: 'lnsGroup',
},
],
Expand All @@ -348,6 +430,7 @@ describe('LayerPanel', () => {
accessors: [{ columnId: 'newid' }],
filterOperations: () => true,
supportsMoreColumns: false,
enableDimensionEditor: true,
dataTestSubj: 'lnsGroup',
},
],
Expand All @@ -360,6 +443,20 @@ describe('LayerPanel', () => {
component.update();

expect(component.find('EuiFlyoutHeader').exists()).toBe(true);

const lastArgs =
mockDatasource.renderDimensionEditor.mock.calls[
mockDatasource.renderDimensionEditor.mock.calls.length - 1
][1];

// Simulate what is called by the dimension editor
act(() => {
lastArgs.setState(lastArgs.state, {
isDimensionComplete: true,
});
});

expect(mockVisualization.renderDimensionEditor).toHaveBeenCalled();
});

it('should close the DimensionContainer when the active visualization changes', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -423,10 +423,11 @@ export function LayerPanel(
isFullscreen={isFullscreen}
groupLabel={activeGroup?.groupLabel || ''}
handleClose={() => {
if (layerDatasource.canCloseDimensionEditor) {
if (!layerDatasource.canCloseDimensionEditor(layerDatasourceState)) {
return false;
}
if (
layerDatasource.canCloseDimensionEditor &&
!layerDatasource.canCloseDimensionEditor(layerDatasourceState)
) {
return false;
}
if (layerDatasource.updateStateOnCloseDimension) {
const newState = layerDatasource.updateStateOnCloseDimension({
Expand Down Expand Up @@ -461,36 +462,37 @@ export function LayerPanel(
isFullscreen,
setState: (
newState: unknown,
{
shouldReplaceDimension,
shouldRemoveDimension,
}: {
shouldReplaceDimension?: boolean;
shouldRemoveDimension?: boolean;
} = {}
{ isDimensionComplete = true }: { isDimensionComplete?: boolean } = {}
) => {
if (shouldReplaceDimension || shouldRemoveDimension) {
if (allAccessors.includes(activeId)) {
if (isDimensionComplete) {
props.updateDatasourceAsync(datasourceId, newState);
} else {
// The datasource can indicate that the previously-valid column is no longer
// complete, which clears the visualization. This keeps the flyout open and reuses
// the previous columnId
props.updateAll(
datasourceId,
newState,
activeVisualization.removeDimension({
layerId,
columnId: activeId,
prevState: props.visualizationState,
})
);
}
} else if (isDimensionComplete) {
props.updateAll(
datasourceId,
newState,
shouldRemoveDimension
? activeVisualization.removeDimension({
layerId,
columnId: activeId,
prevState: props.visualizationState,
})
: activeVisualization.setDimension({
layerId,
groupId: activeGroup.groupId,
columnId: activeId,
prevState: props.visualizationState,
})
activeVisualization.setDimension({
layerId,
groupId: activeGroup.groupId,
columnId: activeId,
prevState: props.visualizationState,
})
);
if (shouldRemoveDimension) {
setActiveDimension(initialActiveDimensionState);
} else {
setActiveDimension({ ...activeDimension, isNew: false });
}
setActiveDimension({ ...activeDimension, isNew: false });
} else {
props.updateDatasourceAsync(datasourceId, newState);
}
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/lens/public/editor_frame_service/mocks.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ export function createMockVisualization(): jest.Mocked<Visualization> {
setDimension: jest.fn(),
removeDimension: jest.fn(),
getErrorMessages: jest.fn((_state) => undefined),
renderDimensionEditor: jest.fn(),
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,21 +112,21 @@ export function DimensionEditor(props: DimensionEditorProps) {
const setStateWrapper = (
setter: IndexPatternLayer | ((prevLayer: IndexPatternLayer) => IndexPatternLayer)
) => {
const prevOperationType =
operationDefinitionMap[state.layers[layerId].columns[columnId]?.operationType]?.input;

const hypotheticalLayer = typeof setter === 'function' ? setter(state.layers[layerId]) : setter;
const hasIncompleteColumns = Boolean(hypotheticalLayer.incompleteColumns?.[columnId]);
const prevOperationType =
operationDefinitionMap[hypotheticalLayer.columns[columnId]?.operationType]?.input;
setState(
(prevState) => {
const layer = typeof setter === 'function' ? setter(prevState.layers[layerId]) : setter;
return mergeLayer({ state: prevState, layerId, newLayer: layer });
},
{
shouldReplaceDimension: Boolean(hypotheticalLayer.columns[columnId]),
// clear the dimension if there's an incomplete column pending && previous operation was a fullReference operation
shouldRemoveDimension: Boolean(
hasIncompleteColumns && prevOperationType === 'fullReference'
),
isDimensionComplete:
prevOperationType === 'fullReference'
? hasIncompleteColumns
: Boolean(hypotheticalLayer.columns[columnId]),
}
);
};
Expand Down
Loading

0 comments on commit 4b81e5d

Please sign in to comment.