Skip to content

Commit

Permalink
[Vis: Default editor] EUIficate order_agg param editor (#36984) (#37330)
Browse files Browse the repository at this point in the history 
* EUIficate order_agg param editor

* Update browser tests

* Add types

* Update enzyme version in x-pack

* Fix functional tests

* Changes due to comments

* Update_terms_helper.tsx

Co-Authored-By: Maryia Lapata <[email protected]>

* Fix code review comments

* Update yarn.lock
  • Loading branch information
@maryia-lapata
maryia-lapata authored May 29, 2019
1 parent e10c351 commit 8791531
Show file tree
Hide file tree
Showing 14 changed files with 394 additions and 247 deletions.
112 changes: 4 additions & 108 deletions src/legacy/ui/public/agg_types/__tests__/buckets/terms.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,84 +47,8 @@ describe('Terms Agg', function () {
});
}

it('defaults to the first metric agg', function () {
init({
responseValueAggs: [
{
id: 'agg1',
type: {
name: 'count'
}
},
{
id: 'agg2',
type: {
name: 'count'
}
}
]
});
expect($rootScope.agg.params.orderBy).to.be('agg1');
});

it('defaults to the first metric agg that is compatible with the terms bucket', function () {
init({
responseValueAggs: [
{
id: 'agg1',
type: {
name: 'top_hits'
}
},
{
id: 'agg2',
type: {
name: 'percentiles'
}
},
{
id: 'agg3',
type: {
name: 'median'
}
},
{
id: 'agg4',
type: {
name: 'std_dev'
}
},
{
id: 'agg5',
type: {
name: 'count'
}
}
]
});
expect($rootScope.agg.params.orderBy).to.be('agg5');
});

it('defaults to the _key metric if no agg is compatible', function () {
init({
responseValueAggs: [
{
id: 'agg1',
type: {
name: 'top_hits'
}
}
]
});
expect($rootScope.agg.params.orderBy).to.be('_key');
});

it('selects _key if there are no metric aggs', function () {
init({});
expect($rootScope.agg.params.orderBy).to.be('_key');
});

it('selects _key if the selected metric becomes incompatible', function () {
// should be rewritten after EUIficate order_agg.html
it.skip('selects _key if the selected metric becomes incompatible', function () {
init({
responseValueAggs: [
{
Expand All @@ -148,36 +72,8 @@ describe('Terms Agg', function () {
expect($rootScope.agg.params.orderBy).to.be('_key');
});

it('selects first metric if it is avg', function () {
init({
responseValueAggs: [
{
id: 'agg1',
type: {
name: 'avg',
field: 'bytes'
}
}
]
});
expect($rootScope.agg.params.orderBy).to.be('agg1');
});

it('selects _key if the first metric is avg_bucket', function () {
$rootScope.responseValueAggs = [
{
id: 'agg1',
type: {
name: 'avg_bucket',
metric: 'custom'
}
}
];
$rootScope.$digest();
expect($rootScope.agg.params.orderBy).to.be('_key');
});

it('selects _key if the selected metric is removed', function () {
// should be rewritten after EUIficate order_agg.html
it.skip('selects _key if the selected metric is removed', function () {
init({
responseValueAggs: [
{
Expand Down
53 changes: 53 additions & 0 deletions src/legacy/ui/public/agg_types/buckets/_terms_helper.tsx
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import { AggConfig } from 'ui/vis';
import { i18n } from '@kbn/i18n';

const aggFilter = [
'!top_hits',
'!percentiles',
'!median',
'!std_dev',
'!derivative',
'!moving_avg',
'!serial_diff',
'!cumulative_sum',
'!avg_bucket',
'!max_bucket',
'!min_bucket',
'!sum_bucket',
];

// Returns true if the agg is compatible with the terms bucket
function isCompatibleAgg(agg: AggConfig) {
return !aggFilter.includes(`!${agg.type.name}`);
}

function safeMakeLabel(agg: AggConfig) {
try {
return agg.makeLabel();
} catch (e) {
return i18n.translate('common.ui.aggTypes.buckets.terms.aggNotValidLabel', {
defaultMessage: '- agg not valid -',
});
}
}

export { aggFilter, isCompatibleAgg, safeMakeLabel };
81 changes: 17 additions & 64 deletions src/legacy/ui/public/agg_types/buckets/terms.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,29 +17,23 @@
* under the License.
*/

import _ from 'lodash';
import chrome from 'ui/chrome';
import { i18n } from '@kbn/i18n';
import { BucketAggType } from './_bucket_agg_type';
import { AggConfig } from '../../vis/agg_config';
import { Schemas } from '../../vis/editors/default/schemas';
import { getRequestInspectorStats, getResponseInspectorStats } from '../../courier/utils/courier_inspector_utils';
import { createFilterTerms } from './create_filter/terms';
import { wrapWithInlineComp } from './_inline_comp_wrapper';
import { buildOtherBucketAgg, mergeOtherBucketAggResponse, updateMissingBucket } from './_terms_other_bucket_helper';
import { isStringType, migrateIncludeExcludeFormat } from './migrate_include_exclude_format';
import { aggFilter } from './_terms_helper';
import orderAggTemplate from '../controls/order_agg.html';
import { OrderParamEditor } from '../controls/order';
import { OrderAggParamEditor } from '../controls/order_agg';
import { SizeParamEditor } from '../controls/size';
import { wrapWithInlineComp } from './_inline_comp_wrapper';
import { i18n } from '@kbn/i18n';

import { getRequestInspectorStats, getResponseInspectorStats } from '../../courier/utils/courier_inspector_utils';
import { buildOtherBucketAgg, mergeOtherBucketAggResponse, updateMissingBucket } from './_terms_other_bucket_helper';
import { MissingBucketParamEditor } from '../controls/missing_bucket';
import { OtherBucketParamEditor } from '../controls/other_bucket';
import { isStringType, migrateIncludeExcludeFormat } from './migrate_include_exclude_format';

const aggFilter = [
'!top_hits', '!percentiles', '!median', '!std_dev',
'!derivative', '!moving_avg', '!serial_diff', '!cumulative_sum',
'!avg_bucket', '!max_bucket', '!min_bucket', '!sum_bucket'
];

const orderAggSchema = (new Schemas([
{
Expand Down Expand Up @@ -120,6 +114,11 @@ export const termsBucketAgg = new BucketAggType({
type: 'field',
filterFieldTypes: ['number', 'boolean', 'date', 'ip', 'string']
},
{
name: 'orderBy',
editorComponent: OrderAggParamEditor,
write: () => {} // prevent default write, it's handled by orderAgg
},
{
name: 'orderAgg',
type: AggConfig,
Expand All @@ -139,33 +138,9 @@ export const termsBucketAgg = new BucketAggType({
return orderAgg;
},
controller: function ($scope) {
$scope.safeMakeLabel = function (agg) {
try {
return agg.makeLabel();
} catch (e) {
return i18n.translate('common.ui.aggTypes.buckets.terms.aggNotValidLabel', {
defaultMessage: '- agg not valid -',
});
}
};

const INIT = {}; // flag to know when prevOrderBy has changed
let prevOrderBy = INIT;

$scope.$watch('responseValueAggs', updateOrderAgg);
$scope.$watch('agg.params.orderBy', updateOrderAgg);

// Returns true if the agg is not compatible with the terms bucket
$scope.rejectAgg = function rejectAgg(agg) {
return aggFilter.includes(`!${agg.type.name}`);
};

$scope.$watch('agg.params.field.type', (type) => {
if (type !== 'string') {
$scope.agg.params.missingBucket = false;
}
});

function updateOrderAgg() {
// abort until we get the responseValueAggs
if (!$scope.responseValueAggs) return;
Expand All @@ -174,27 +149,9 @@ export const termsBucketAgg = new BucketAggType({
const orderBy = params.orderBy;
const paramDef = agg.type.params.byName.orderAgg;

// setup the initial value of orderBy
if (!orderBy && prevOrderBy === INIT) {
let respAgg = _($scope.responseValueAggs).filter((agg) => !$scope.rejectAgg(agg)).first();
if (!respAgg) {
respAgg = { id: '_key' };
}
params.orderBy = respAgg.id;
return;
}

// track the previous value
prevOrderBy = orderBy;

// we aren't creating a custom aggConfig
if (!orderBy || orderBy !== 'custom') {
params.orderAgg = null;
// ensure that orderBy is set to a valid agg
const respAgg = _($scope.responseValueAggs).filter((agg) => !$scope.rejectAgg(agg)).find({ id: orderBy });
if (!respAgg) {
params.orderBy = '_key';
}
return;
}

Expand Down Expand Up @@ -256,22 +213,18 @@ export const termsBucketAgg = new BucketAggType({
value: 'asc'
}
],
write: _.noop // prevent default write, it's handled by orderAgg
write: () => {} // prevent default write, it's handled by orderAgg
},
{
name: 'size',
editorComponent: wrapWithInlineComp(SizeParamEditor),
default: 5
},
{
name: 'orderBy',
write: _.noop // prevent default write, it's handled by orderAgg
},
{
name: 'otherBucket',
default: false,
editorComponent: OtherBucketParamEditor,
write: _.noop,
write: () => {},
},
{
name: 'otherBucketLabel',
Expand All @@ -283,13 +236,13 @@ export const termsBucketAgg = new BucketAggType({
defaultMessage: 'Label for other bucket',
}),
shouldShow: agg => agg.params.otherBucket,
write: _.noop,
write: () => {},
},
{
name: 'missingBucket',
default: false,
editorComponent: MissingBucketParamEditor,
write: _.noop,
write: () => {},
},
{
name: 'missingBucketLabel',
Expand All @@ -303,7 +256,7 @@ export const termsBucketAgg = new BucketAggType({
defaultMessage: 'Label for missing values',
}),
shouldShow: agg => agg.params.missingBucket,
write: _.noop,
write: () => {},
},
{
name: 'exclude',
Expand Down
15 changes: 13 additions & 2 deletions src/legacy/ui/public/agg_types/controls/missing_bucket.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,24 @@
* under the License.
*/

import React from 'react';
import React, { useEffect } from 'react';
import { i18n } from '@kbn/i18n';
import { AggParamEditorProps } from 'ui/vis/editors/default';
import { SwitchParamEditor } from './switch';
import { isStringType } from '../buckets/migrate_include_exclude_format';

function MissingBucketParamEditor(props: AggParamEditorProps<boolean>) {
const fieldTypeIsNotString = !isStringType(props.agg);

useEffect(
() => {
if (fieldTypeIsNotString) {
props.setValue(false);
}
},
[fieldTypeIsNotString]
);

return (
<SwitchParamEditor
dataTestSubj="missingBucketSwitch"
Expand All @@ -37,7 +48,7 @@ function MissingBucketParamEditor(props: AggParamEditorProps<boolean>) {
'If not in the top N, and you enable "Group other values in separate bucket", ' +
'Elasticsearch adds the missing values to the "other" bucket.',
})}
disabled={!isStringType(props.agg)}
disabled={fieldTypeIsNotString}
{...props}
/>
);
Expand Down
Loading

0 comments on commit 8791531

Please sign in to comment.