Skip to content

Commit

Permalink
review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@flash1293
flash1293 committed Aug 7, 2019
1 parent 834bb07 commit 1f99d15
Show file tree
Hide file tree
Showing 10 changed files with 292 additions and 91 deletions.
31 changes: 28 additions & 3 deletions x-pack/legacy/plugins/lens/public/indexpattern_plugin/layerpanel.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ describe('Layer Data Panel', () => {
const instance = shallow(<LayerPanel {...defaultProps} />);
act(() => {
instance
.find('[data-test-subj="layerIndexPatternLabel"]')
.find('[data-test-subj="lns_layerIndexPatternLabel"]')
.first()
.simulate('click');
});
Expand All @@ -195,7 +195,7 @@ describe('Layer Data Panel', () => {
const instance = shallow(<LayerPanel {...defaultProps} />);
act(() => {
instance
.find('[data-test-subj="layerIndexPatternLabel"]')
.find('[data-test-subj="lns_layerIndexPatternLabel"]')
.first()
.simulate('click');
});
Expand All @@ -208,11 +208,36 @@ describe('Layer Data Panel', () => {
).toEqual([false, true]);
});

it('should switch data panel to target index pattern', () => {
const instance = shallow(<LayerPanel {...defaultProps} />);
act(() => {
instance
.find('[data-test-subj="lns_layerIndexPatternLabel"]')
.first()
.simulate('click');
});

act(() => {
instance.find(EuiComboBox).prop('onChange')!([
{
label: 'my-compatible-pattern',
value: defaultProps.state.indexPatterns['3'],
},
]);
});

expect(defaultProps.setState).toHaveBeenCalledWith(
expect.objectContaining({
currentIndexPatternId: '3',
})
);
});

it('should switch using updateLayerIndexPattern', () => {
const instance = shallow(<LayerPanel {...defaultProps} />);
act(() => {
instance
.find('[data-test-subj="layerIndexPatternLabel"]')
.find('[data-test-subj="lns_layerIndexPatternLabel"]')
.first()
.simulate('click');
});
Expand Down
138 changes: 74 additions & 64 deletions x-pack/legacy/plugins/lens/public/indexpattern_plugin/layerpanel.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ import {
EuiButtonEmpty,
EuiIcon,
EuiIconTip,
EuiFlexGroup,
EuiFlexItem,
} from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import { I18nProvider } from '@kbn/i18n/react';
Expand Down Expand Up @@ -41,72 +43,80 @@ export function LayerPanel({ state, setState, layerId }: IndexPatternLayerPanelP

return (
<I18nProvider>
{isChooserOpen ? (
<EuiComboBox
data-test-subj="layerIndexPatternSwitcher"
options={indexPatterns.map(indexPattern => ({
label: indexPattern.title,
value: indexPattern,
}))}
inputRef={el => {
if (el) {
el.focus();
}
}}
selectedOptions={[
{
label: state.indexPatterns[currentIndexPatternId].title,
value: state.indexPatterns[currentIndexPatternId].id,
},
]}
singleSelection={{ asPlainText: true }}
isClearable={false}
onBlur={() => {
setChooserOpen(false);
}}
onChange={choices => {
setState({
...state,
layers: {
...state.layers,
[layerId]: updateLayerIndexPattern(state.layers[layerId], choices[0].value!),
},
});
<EuiFlexGroup justifyContent="flexEnd">
{isChooserOpen ? (
<EuiFlexItem>
<EuiComboBox
fullWidth
data-test-subj="lns_layerIndexPatternSwitcher"
options={indexPatterns.map(indexPattern => ({
label: indexPattern.title,
value: indexPattern,
}))}
inputRef={el => {
if (el) {
el.focus();
}
}}
selectedOptions={[
{
label: state.indexPatterns[currentIndexPatternId].title,
value: state.indexPatterns[currentIndexPatternId].id,
},
]}
singleSelection={{ asPlainText: true }}
isClearable={false}
onBlur={() => {
setChooserOpen(false);
}}
onChange={choices => {
setState({
...state,
currentIndexPatternId: choices[0].value!.id,
layers: {
...state.layers,
[layerId]: updateLayerIndexPattern(state.layers[layerId], choices[0].value!),
},
});

setChooserOpen(false);
}}
renderOption={(option, searchValue, contentClassName) => {
const { label, value } = option;
return (
<span className={contentClassName}>
{value && value.isTransferable ? (
<EuiIcon type="empty" />
) : (
<EuiIconTip
type="bolt"
content={i18n.translate(
'xpack.lens.indexPattern.lossyIndexPatternSwitchDescription',
{
defaultMessage:
'Not all operations are compatible with this index pattern and will be removed on switching.',
}
setChooserOpen(false);
}}
renderOption={(option, searchValue, contentClassName) => {
const { label, value } = option;
return (
<span className={contentClassName}>
{value && value.isTransferable ? (
<EuiIcon type="empty" />
) : (
<EuiIconTip
type="bolt"
content={i18n.translate(
'xpack.lens.indexPattern.lossyIndexPatternSwitchDescription',
{
defaultMessage:
'Not all operations are compatible with this index pattern and will be removed on switching.',
}
)}
/>
)}
/>
)}
<EuiHighlight search={searchValue}>{label}</EuiHighlight>
</span>
);
}}
/>
) : (
<EuiButtonEmpty
size="s"
onClick={() => setChooserOpen(true)}
data-test-subj="layerIndexPatternLabel"
>
{state.indexPatterns[state.layers[layerId].indexPatternId].title}
</EuiButtonEmpty>
)}
<EuiHighlight search={searchValue}>{label}</EuiHighlight>
</span>
);
}}
/>
</EuiFlexItem>
) : (
<EuiFlexItem grow={null}>
<EuiButtonEmpty
size="s"
onClick={() => setChooserOpen(true)}
data-test-subj="lns_layerIndexPatternLabel"
>
{state.indexPatterns[state.layers[layerId].indexPatternId].title}
</EuiButtonEmpty>
</EuiFlexItem>
)}
</EuiFlexGroup>
</I18nProvider>
);
}
79 changes: 79 additions & 0 deletions ...acy/plugins/lens/public/indexpattern_plugin/operation_definitions/date_histogram.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,85 @@ describe('date_histogram', () => {
});
});

describe('transfer', () => {
it('should adjust interval and time zone params if that is necessary due to restrictions', () => {
const transferedColumn = dateHistogramOperation.transfer!(
{
dataType: 'date',
isBucketed: true,
label: '',
operationType: 'date_histogram',
sourceField: 'dateField',
params: {
interval: 'd',
},
},
{
title: '',
id: '',
fields: [
{
name: 'dateField',
type: 'date',
aggregatable: true,
searchable: true,
aggregationRestrictions: {
date_histogram: {
agg: 'date_histogram',
time_zone: 'CET',
calendar_interval: 'w',
},
},
},
],
}
);
expect(transferedColumn).toEqual(
expect.objectContaining({
params: {
interval: 'w',
timeZone: 'CET',
},
})
);
});

it('should remove time zone param and normalize interval param', () => {
const transferedColumn = dateHistogramOperation.transfer!(
{
dataType: 'date',
isBucketed: true,
label: '',
operationType: 'date_histogram',
sourceField: 'dateField',
params: {
interval: '20s',
},
},
{
title: '',
id: '',
fields: [
{
name: 'dateField',
type: 'date',
aggregatable: true,
searchable: true,
},
],
}
);
expect(transferedColumn).toEqual(
expect.objectContaining({
params: {
interval: 'M',
timeZone: undefined,
},
})
);
});
});

describe('param editor', () => {
it('should render current value', () => {
const setStateSpy = jest.fn();
Expand Down
67 changes: 47 additions & 20 deletions ...k/legacy/plugins/lens/public/indexpattern_plugin/operation_definitions/date_histogram.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ import { updateColumnParam } from '../state_helpers';

type PropType<C> = C extends React.ComponentType<infer P> ? P : unknown;

const supportedIntervals = ['M', 'w', 'd', 'h'];

// Add ticks to EuiRange component props
const FixedEuiRange = (EuiRange as unknown) as React.ComponentType<
PropType<typeof EuiRange> & {
Expand Down Expand Up @@ -82,22 +84,46 @@ export const dateHistogramOperation: OperationDefinition<DateHistogramIndexPatte
isTransferable: (column, newIndexPattern) => {
const newField = newIndexPattern.fields.find(field => field.name === column.sourceField);

if (!newField || !newField.aggregatable) {
return false;
}

if (!newField.aggregationRestrictions) {
return true;
}

if (newField.aggregationRestrictions.date_histogram) {
return Boolean(
newField &&
newField.type === 'date' &&
newField.aggregatable &&
(!newField.aggregationRestrictions || newField.aggregationRestrictions.date_histogram)
);
},
transfer: (column, newIndexPattern) => {
const newField = newIndexPattern.fields.find(field => field.name === column.sourceField);
if (
newField &&
newField.aggregationRestrictions &&
newField.aggregationRestrictions.date_histogram
) {
const restrictions = newField.aggregationRestrictions.date_histogram;
return (
restrictions.calendar_interval === column.params.interval &&
restrictions.time_zone === column.params.timeZone
);
return {
...column,
params: {
...column.params,
timeZone: restrictions.time_zone,
// TODO this rewrite logic is simplified - if the current interval is a multiple of
// the restricted interval, we could carry it over directly. However as the current
// UI does not allow to select multiples of an interval anyway, this is not included yet.
// If the UI allows to pick more complicated intervals, this should be re-visited.
interval: (newField.aggregationRestrictions.date_histogram.calendar_interval ||
newField.aggregationRestrictions.date_histogram.fixed_interval) as string,
},
};
} else {
return false;
return {
...column,
params: {
...column.params,
// TODO remove this once it's possible to specify free intervals instead of picking from a list
interval: supportedIntervals.includes(column.params.interval)
? column.params.interval
: supportedIntervals[0],
timeZone: undefined,
},
};
}
},
toEsAggsConfig: (column, columnId) => ({
Expand Down Expand Up @@ -131,14 +157,12 @@ export const dateHistogramOperation: OperationDefinition<DateHistogramIndexPatte
const intervalIsRestricted =
field!.aggregationRestrictions && field!.aggregationRestrictions.date_histogram;

const intervals = ['M', 'w', 'd', 'h'];

function intervalToNumeric(interval: string) {
return intervals.indexOf(interval);
return supportedIntervals.indexOf(interval);
}

function numericToInterval(i: number) {
return intervals[i];
return supportedIntervals[i];
}
return (
<EuiForm>
Expand All @@ -158,11 +182,14 @@ export const dateHistogramOperation: OperationDefinition<DateHistogramIndexPatte
) : (
<FixedEuiRange
min={0}
max={intervals.length - 1}
max={supportedIntervals.length - 1}
step={1}
value={intervalToNumeric(column.params.interval)}
showTicks
ticks={intervals.map((interval, index) => ({ label: interval, value: index }))}
ticks={supportedIntervals.map((interval, index) => ({
label: interval,
value: index,
}))}
onChange={(e: React.ChangeEvent<HTMLInputElement>) =>
setState(
updateColumnParam(
Expand Down
Loading

0 comments on commit 1f99d15

Please sign in to comment.