Skip to content

Commit

Permalink
Andrew's comments addressed
Browse files Browse the repository at this point in the history
  • Loading branch information
mbondyra committed Jun 21, 2022
1 parent 664f6d4 commit 0f79ee0
Show file tree
Hide file tree
Showing 19 changed files with 101 additions and 122 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {
Datasource,
DropType,
FramePublicAPI,
GetDropProps,
GetDropPropsArgs,
isOperation,
Visualization,
DragDropOperation,
Expand Down Expand Up @@ -167,7 +167,7 @@ export function getDropPropsForSameGroup(
}

export const getDropProps = (
dropProps: GetDropProps,
dropProps: GetDropPropsArgs,
sharedDatasource?: Datasource<unknown, unknown>
): { dropTypes: DropType[]; nextLabel?: string } | undefined => {
if (sharedDatasource) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,14 @@ import {
IndexPattern,
IndexPatternField,
DraggedField,
DataViewLensOperation,
DataViewDragDropOperation,
} from '../../types';
import {
getDropPropsForSameGroup,
isOperationFromTheSameGroup,
} from '../../../editor_frame_service/editor_frame/config_panel/buttons/drop_targets_utils';

interface GetDropProps {
interface GetDropPropsArgs {
state: IndexPatternPrivateState;
source?: DragContextState['dragging'];
target: DragDropOperation;
Expand Down Expand Up @@ -69,12 +69,12 @@ export function getField(column: GenericIndexPatternColumn | undefined, dataView
return field;
}

export function getDropProps(props: GetDropProps) {
export function getDropProps(props: GetDropPropsArgs) {
const { state, source, target } = props;
if (!source) {
return;
}
const targetProps: DataViewLensOperation = {
const targetProps: DataViewDragDropOperation = {
...target,
column: state.layers[target.layerId].columns[target.columnId],
dataView: state.indexPatterns[state.layers[target.layerId].indexPatternId],
Expand All @@ -85,7 +85,7 @@ export function getDropProps(props: GetDropProps) {
}

if (isOperation(source)) {
const sourceProps: DataViewLensOperation = {
const sourceProps: DataViewDragDropOperation = {
...source,
column: state.layers[source.layerId]?.columns[source.columnId],
dataView: state.indexPatterns[state.layers[source.layerId]?.indexPatternId],
Expand Down Expand Up @@ -126,7 +126,7 @@ function getDropPropsForField({
state,
source,
target,
}: GetDropProps & { source: DraggedField }): DropProps {
}: GetDropPropsArgs & { source: DraggedField }): DropProps {
const targetColumn = state.layers[target.layerId].columns[target.columnId];
const isTheSameIndexPattern =
state.layers[target.layerId].indexPatternId === source.indexPatternId;
Expand Down Expand Up @@ -158,8 +158,8 @@ function getDropPropsForField({
}

function getDropPropsForCompatibleGroup(
sourceProps: DataViewLensOperation,
targetProps: DataViewLensOperation
sourceProps: DataViewDragDropOperation,
targetProps: DataViewDragDropOperation
): DropProps {
if (!targetProps.column) {
return { dropTypes: ['move_compatible', 'duplicate_compatible'] };
Expand Down Expand Up @@ -187,8 +187,8 @@ function getDropPropsForCompatibleGroup(
}

function getDropPropsFromIncompatibleGroup(
sourceProps: DataViewLensOperation,
targetProps: DataViewLensOperation
sourceProps: DataViewDragDropOperation,
targetProps: DataViewDragDropOperation
): DropProps {
if (!targetProps.dataView || !sourceProps.column) {
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ describe('IndexPatternDimensionEditorPanel: onDrop', () => {
columns: expect.objectContaining({
col1: expect.objectContaining({
dataType: 'number',
sourceField: 'bytes',
sourceField: mockedDraggedField.field.name,
}),
}),
}),
Expand Down Expand Up @@ -149,7 +149,7 @@ describe('IndexPatternDimensionEditorPanel: onDrop', () => {
...state.layers.first.columns,
col2: expect.objectContaining({
dataType: 'number',
sourceField: 'bytes',
sourceField: mockedDraggedField.field.name,
}),
},
},
Expand Down Expand Up @@ -222,7 +222,7 @@ describe('IndexPatternDimensionEditorPanel: onDrop', () => {
...state.layers.first.columns,
col2: expect.objectContaining({
dataType: 'number',
sourceField: 'bytes',
sourceField: mockedDraggedField.field.name,
}),
},
},
Expand Down Expand Up @@ -262,7 +262,7 @@ describe('IndexPatternDimensionEditorPanel: onDrop', () => {
columns: {
newCol: expect.objectContaining({
dataType: 'number',
sourceField: 'bytes',
sourceField: mockedDraggedField.field.name,
}),
col1: testState.layers.first.columns.col1,
col2: testState.layers.first.columns.col2,
Expand Down Expand Up @@ -294,7 +294,9 @@ describe('IndexPatternDimensionEditorPanel: onDrop', () => {
col1: expect.objectContaining({
dataType: 'string',
sourceField: 'dest',
params: expect.objectContaining({ secondaryFields: ['bytes'] }),
params: expect.objectContaining({
secondaryFields: [mockedDraggedField.field.name],
}),
}),
}),
}),
Expand Down Expand Up @@ -1188,7 +1190,7 @@ describe('IndexPatternDimensionEditorPanel: onDrop', () => {
columns: {
newCol: expect.objectContaining({
dataType: 'number',
sourceField: 'bytes',
sourceField: mockedDraggedField.field.name,
}),
col1: testState.layers.first.columns.col1,
col2: testState.layers.first.columns.col2,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,10 @@ import {
import { mergeLayer, mergeLayers } from '../../state_helpers';
import { isDraggedField } from '../../pure_utils';
import { getNewOperation, getField } from './get_drop_props';
import { IndexPatternPrivateState, DraggedField, DataViewLensOperation } from '../../types';
import { IndexPatternPrivateState, DraggedField, DataViewDragDropOperation } from '../../types';
import { trackUiEvent } from '../../../lens_ui_telemetry';

interface DropHandlerProps<T = DataViewLensOperation> {
interface DropHandlerProps<T = DataViewDragDropOperation> {
state: IndexPatternPrivateState;
setState: StateSetter<
IndexPatternPrivateState,
Expand All @@ -41,15 +41,12 @@ interface DropHandlerProps<T = DataViewLensOperation> {
dimensionGroups: VisualizationDimensionGroupConfig[];
dropType?: DropType;
source: T;
target: DataViewLensOperation;
target: DataViewDragDropOperation;
}

export function onDrop(props: DatasourceDimensionDropHandlerProps<IndexPatternPrivateState>) {
const { target, source, dropType, state } = props;

if (!source) {
return false;
}
if (isDraggedField(source) && isFieldDropType(dropType)) {
return onFieldDrop(
{
Expand Down Expand Up @@ -168,7 +165,7 @@ function onFieldDrop(props: DropHandlerProps<DraggedField>, shouldAddField?: boo
}

function onMoveCompatible(
{ setState, state, source, target, dimensionGroups }: DropHandlerProps<DataViewLensOperation>,
{ setState, state, source, target, dimensionGroups }: DropHandlerProps<DataViewDragDropOperation>,
shouldDeleteSource?: boolean
) {
const modifiedLayers = copyColumn({
Expand Down Expand Up @@ -208,7 +205,12 @@ function onMoveCompatible(
}
}

function onReorder({ setState, state, source, target }: DropHandlerProps<DataViewLensOperation>) {
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);
Expand Down Expand Up @@ -236,7 +238,7 @@ function onReorder({ setState, state, source, target }: DropHandlerProps<DataVie
}

function onMoveIncompatible(
{ setState, state, source, dimensionGroups, target }: DropHandlerProps<DataViewLensOperation>,
{ setState, state, source, dimensionGroups, target }: DropHandlerProps<DataViewDragDropOperation>,
shouldDeleteSource?: boolean
) {
const targetLayer = state.layers[target.layerId];
Expand Down Expand Up @@ -400,7 +402,7 @@ function onSwapCompatible({
source,
dimensionGroups,
target,
}: DropHandlerProps<DataViewLensOperation>) {
}: DropHandlerProps<DataViewDragDropOperation>) {
if (target.layerId === source.layerId) {
const layer = state.layers[target.layerId];
const newColumns = {
Expand Down Expand Up @@ -464,7 +466,7 @@ function onCombine({
source,
target,
dimensionGroups,
}: DropHandlerProps<DataViewLensOperation>) {
}: DropHandlerProps<DataViewDragDropOperation>) {
const targetLayer = state.layers[target.layerId];
const targetColumn = targetLayer.columns[target.columnId];
const targetField = getField(targetColumn, target.dataView);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,12 +150,8 @@ export const countOperation: OperationDefinition<CountIndexPatternColumn, 'field
},
];
},
onOtherColumnChanged: (layer, thisColumnId, changedColumnId) =>
adjustTimeScaleOnOtherColumnChange<CountIndexPatternColumn>(
layer,
thisColumnId,
changedColumnId
),
onOtherColumnChanged: (layer, thisColumnId) =>
adjustTimeScaleOnOtherColumnChange<CountIndexPatternColumn>(layer, thisColumnId),
toEsAggsFn: (column, columnId) => {
return buildExpressionFunction<AggFunctionsMapping['aggCount']>('aggCount', {
id: columnId,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ import type {
ReferenceBasedIndexPatternColumn,
} from './column_types';
import {
DataViewLensOperation,
DataViewDragDropOperation,
IndexPattern,
IndexPatternField,
IndexPatternLayer,
Expand Down Expand Up @@ -250,11 +250,7 @@ interface BaseOperationDefinitionProps<C extends BaseIndexPatternColumn, P = {}>
* Based on the current column and the other updated columns, this function has to
* return an updated column. If not implemented, the `id` function is used instead.
*/
onOtherColumnChanged?: (
layer: IndexPatternLayer,
thisColumnId: string,
changedColumnId?: string
) => C;
onOtherColumnChanged?: (layer: IndexPatternLayer, thisColumnId: string) => C;
/**
* React component for operation specific settings shown in the flyout editor
*/
Expand Down Expand Up @@ -614,8 +610,8 @@ interface ManagedReferenceOperationDefinition<C extends BaseIndexPatternColumn>
*/
createCopy: (
layers: Record<string, IndexPatternLayer>,
source: DataViewLensOperation,
target: DataViewLensOperation,
source: DataViewDragDropOperation,
target: DataViewDragDropOperation,
operationDefinitionMap: Record<string, GenericOperationDefinition>
) => Record<string, IndexPatternLayer>;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,9 +108,9 @@ function buildMetricOperation<T extends MetricColumn<string>>({
(!newField.aggregationRestrictions || newField.aggregationRestrictions![type])
);
},
onOtherColumnChanged: (layer, thisColumnId, changedColumnId) =>
onOtherColumnChanged: (layer, thisColumnId) =>
optionalTimeScaling
? (adjustTimeScaleOnOtherColumnChange(layer, thisColumnId, changedColumnId) as T)
? (adjustTimeScaleOnOtherColumnChange(layer, thisColumnId) as T)
: (layer.columns[thisColumnId] as T),
getDefaultLabel: (column, indexPattern, columns) =>
labelLookup(getSafeName(column.sourceField, indexPattern), column),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,7 @@ export const termsOperation: OperationDefinition<TermsIndexPatternColumn, 'field
params: newParams,
};
},
onOtherColumnChanged: (layer, thisColumnId, changedColumnId) => {
onOtherColumnChanged: (layer, thisColumnId) => {
const columns = layer.columns;
const currentColumn = columns[thisColumnId] as TermsIndexPatternColumn;
if (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -755,8 +755,7 @@ describe('terms', () => {
},
},
},
'col2',
'col1'
'col2'
);

expect(updatedColumn).toBe(initialColumn);
Expand Down Expand Up @@ -796,8 +795,7 @@ describe('terms', () => {
columnOrder: [],
indexPatternId: '',
},
'col2',
'col1'
'col2'
);
expect(updatedColumn.params).toEqual(
expect.objectContaining({
Expand Down Expand Up @@ -843,8 +841,7 @@ describe('terms', () => {
columnOrder: [],
indexPatternId: '',
},
'col2',
'col1'
'col2'
);
expect(updatedColumn.params).toEqual(
expect.objectContaining({
Expand Down Expand Up @@ -875,8 +872,7 @@ describe('terms', () => {
columnOrder: [],
indexPatternId: '',
},
'col2',
'col1'
'col2'
);
expect(termsColumn.params).toEqual(
expect.objectContaining({
Expand Down Expand Up @@ -919,8 +915,7 @@ describe('terms', () => {
columnOrder: [],
indexPatternId: '',
},
'col2',
'col1'
'col2'
);
expect(termsColumn.params).toEqual(
expect.objectContaining({
Expand Down Expand Up @@ -951,8 +946,7 @@ describe('terms', () => {
columnOrder: [],
indexPatternId: '',
},
'col2',
'col1'
'col2'
);
expect(termsColumn.params).toEqual(
expect.objectContaining({
Expand Down Expand Up @@ -991,8 +985,7 @@ describe('terms', () => {
},
},
},
'col2',
'col1'
'col2'
);

expect(updatedColumn.params).toEqual(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1369,8 +1369,7 @@ describe('state_helpers', () => {
},
incompleteColumns: {},
},
'col1',
'col2'
'col1'
);
});

Expand Down Expand Up @@ -1436,8 +1435,7 @@ describe('state_helpers', () => {
},
incompleteColumns: {},
}),
'col1',
'willBeReference'
'col1'
);
});

Expand Down Expand Up @@ -2388,8 +2386,7 @@ describe('state_helpers', () => {

expect(operationDefinitionMap.terms.onOtherColumnChanged).toHaveBeenCalledWith(
{ indexPatternId: '1', columnOrder: ['col1', 'col2'], columns: { col1: termsColumn } },
'col1',
'col2'
'col1'
);
});

Expand Down
Loading

0 comments on commit 0f79ee0

Please sign in to comment.