Skip to content

Commit

Permalink
[ML] Review feedback.
Browse files Browse the repository at this point in the history
  • Loading branch information
walterra committed Feb 21, 2020
1 parent 8a8222a commit c5ae56a
Show file tree
Hide file tree
Showing 7 changed files with 35 additions and 21 deletions.
3 changes: 3 additions & 0 deletions x-pack/legacy/plugins/transform/public/app/common/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,11 +76,14 @@ export {
TermsAgg,
} from './pivot_group_by';
export {
defaultQuery,
getPreviewRequestBody,
getCreateRequestBody,
getPivotQuery,
isDefaultQuery,
isMatchAllQuery,
isSimpleQuery,
matchAllQuery,
PivotQuery,
SimpleQuery,
} from './request';
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,26 @@ import { StepDetailsExposedState } from '../sections/create_transform/components
import { PIVOT_SUPPORTED_GROUP_BY_AGGS } from './pivot_group_by';
import { PivotAggsConfig, PIVOT_SUPPORTED_AGGS } from './pivot_aggs';
import {
defaultQuery,
getPreviewRequestBody,
getCreateRequestBody,
getPivotQuery,
isDefaultQuery,
isMatchAllQuery,
isSimpleQuery,
matchAllQuery,
PivotQuery,
} from './request';

const defaultQuery: PivotQuery = { query_string: { query: '*' } };
const matchAllQuery: PivotQuery = { match_all: {} };
const simpleQuery: PivotQuery = { query_string: { query: 'airline:AAL' } };

describe('Transform: Common', () => {
test('isMatchAllQuery()', () => {
expect(isMatchAllQuery(defaultQuery)).toBe(false);
expect(isMatchAllQuery(matchAllQuery)).toBe(true);
expect(isMatchAllQuery(simpleQuery)).toBe(false);
});

test('isSimpleQuery()', () => {
expect(isSimpleQuery(defaultQuery)).toBe(true);
expect(isSimpleQuery(matchAllQuery)).toBe(false);
Expand Down
6 changes: 6 additions & 0 deletions x-pack/legacy/plugins/transform/public/app/common/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,12 @@ export function isSimpleQuery(arg: any): arg is SimpleQuery {
return arg.query_string !== undefined;
}

export const matchAllQuery = { match_all: {} };
export function isMatchAllQuery(query: any): boolean {
return query.match_all && Object.keys(query.match_all).length === 0;
}

export const defaultQuery: PivotQuery = { query_string: { query: '*' } };
export function isDefaultQuery(query: PivotQuery): boolean {
return isSimpleQuery(query) && query.query_string.query === '*';
}
Expand Down
10 changes: 4 additions & 6 deletions x-pack/legacy/plugins/transform/public/app/lib/kibana/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import {
IndexPatternsContract,
} from '../../../../../../../../src/plugins/data/public';

import { matchAllQuery } from '../../common';

type IndexPatternId = string;
type SavedSearchId = string;

Expand Down Expand Up @@ -98,11 +100,7 @@ export function createSearchItems(

let combinedQuery: CombinedQuery = {
bool: {
must: [
{
match_all: {},
},
],
must: [matchAllQuery],
},
};

Expand All @@ -120,7 +118,7 @@ export function createSearchItems(
}

if (!isIndexPattern(indexPattern)) {
throw new Error('Index Pattern not defined');
throw new Error('Index Pattern is not defined.');
}

return {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@

import React, { createContext, useContext, FC } from 'react';

import { IUiSettingsClient } from 'kibana/public';

import { SavedSearch } from '../../../../../../../../src/legacy/core_plugins/kibana/public/discover/np_ready/types';
import {
IndexPattern,
IndexPatternsContract,
} from '../../../../../../../../src/plugins/data/public';
// eslint-disable-next-line @kbn/eslint/no-restricted-paths
import { IUiSettingsClient } from '../../../../../../../../src/core/public/ui_settings';

interface UninitializedKibanaContextValue {
initialized: false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
getDefaultSelectableFields,
getFlattenedFields,
isDefaultQuery,
matchAllQuery,
EsDoc,
EsDocSource,
EsFieldName,
Expand Down Expand Up @@ -75,7 +76,7 @@ export const useSourceIndexData = (
index: indexPattern.title,
size: SEARCH_SIZE,
// Instead of using the default query (`*`), fall back to a more efficient `match_all` query.
body: { query: isDefaultQuery(query) ? { match_all: {} } : query },
body: { query: isDefaultQuery(query) ? matchAllQuery : query },
});

if (isErrorResponse(resp)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,12 @@ import {
} from '../../../../lib/kibana';

import {
AggName,
DropDownLabel,
getPivotQuery,
getPreviewRequestBody,
isMatchAllQuery,
matchAllQuery,
AggName,
DropDownLabel,
PivotAggDict,
PivotAggsConfig,
PivotAggsConfigDict,
Expand Down Expand Up @@ -104,10 +106,10 @@ export function applyTransformConfigToDefineState(
const aggConfig = transformConfig.pivot.aggregations[aggName] as Dictionary<any>;
const agg = Object.keys(aggConfig)[0];
aggList[aggName] = {
...aggConfig[agg],
agg: agg as PIVOT_SUPPORTED_AGGS,
aggName,
dropDownName: aggName,
...aggConfig[agg],
} as PivotAggsConfig;
return aggList;
}, {} as PivotAggsConfigDict);
Expand All @@ -130,7 +132,7 @@ export function applyTransformConfigToDefineState(

// only apply the query from the transform config to wizard state if it's not the default query
const query = transformConfig.source.query;
if (query !== undefined && !isEqual(query, { match_all: {} })) {
if (query !== undefined && !isEqual(query, matchAllQuery)) {
state.isAdvancedSourceEditorEnabled = true;
state.searchString = '';
state.searchQuery = query;
Expand Down Expand Up @@ -263,10 +265,7 @@ export const StepDefineForm: FC<Props> = React.memo(({ overrides = {}, onChange
const searchHandler = (d: Record<string, any>) => {
const { filterQuery, queryString } = d;
const newSearch = queryString === emptySearch ? defaultSearch : queryString;
const newSearchQuery =
filterQuery.match_all && Object.keys(filterQuery.match_all).length === 0
? defaultSearch
: filterQuery;
const newSearchQuery = isMatchAllQuery(filterQuery) ? defaultSearch : filterQuery;
setSearchString(newSearch);
setSearchQuery(newSearchQuery);
};
Expand Down Expand Up @@ -418,10 +417,10 @@ export const StepDefineForm: FC<Props> = React.memo(({ overrides = {}, onChange
const aggConfigKeys = Object.keys(aggConfig);
const agg = aggConfigKeys[0] as PivotSupportedGroupByAggs;
newGroupByList[aggName] = {
...aggConfig[agg],
agg,
aggName,
dropDownName: '',
...aggConfig[agg],
};
});
}
Expand All @@ -435,10 +434,10 @@ export const StepDefineForm: FC<Props> = React.memo(({ overrides = {}, onChange
const aggConfigKeys = Object.keys(aggConfig);
const agg = aggConfigKeys[0] as PIVOT_SUPPORTED_AGGS;
newAggList[aggName] = {
...aggConfig[agg],
agg,
aggName,
dropDownName: '',
...aggConfig[agg],
};
});
}
Expand Down

0 comments on commit c5ae56a

Please sign in to comment.