Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Input controls crashes if index pattern is not available #79431

Merged
merged 5 commits into from
Oct 8, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/plugins/input_control_vis/public/control/control.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ export abstract class Control<FilterManager extends BaseFilterManager> {
format = (value: any) => {
const indexPattern = this.filterManager.getIndexPattern();
const field = this.filterManager.getField();
if (field) {
if (field && indexPattern) {
return indexPattern.getFormatterForField(field).convert(value);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,11 @@ import expect from '@kbn/expect';

import { FilterManager } from './filter_manager';
import { coreMock } from '../../../../../core/public/mocks';
import { Filter, IndexPattern, FilterManager as QueryFilterManager } from '../../../../data/public';
import {
Filter,
FilterManager as QueryFilterManager,
IndexPatternsContract,
} from '../../../../data/public';

const setupMock = coreMock.createSetup();

Expand All @@ -39,7 +43,6 @@ describe('FilterManager', function () {
const controlId = 'control1';

describe('findFilters', function () {
const indexPatternMock = {} as IndexPattern;
let kbnFilters: Filter[];
const queryFilterMock = new QueryFilterManager(setupMock.uiSettings);
queryFilterMock.getAppFilters = () => kbnFilters;
Expand All @@ -48,7 +51,13 @@ describe('FilterManager', function () {
let filterManager: FilterManagerTest;
beforeEach(() => {
kbnFilters = [];
filterManager = new FilterManagerTest(controlId, 'field1', indexPatternMock, queryFilterMock);
filterManager = new FilterManagerTest(
controlId,
'field1',
'1',
{} as IndexPatternsContract,
queryFilterMock
);
});

test('should not find filters that are not controlled by any visualization', function () {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,22 @@

import _ from 'lodash';

import { FilterManager as QueryFilterManager, IndexPattern, Filter } from '../../../../data/public';
import {
FilterManager as QueryFilterManager,
IndexPattern,
Filter,
IndexPatternsContract,
} from '../../../../data/public';

export abstract class FilterManager {
protected indexPattern: IndexPattern | undefined;

constructor(
public controlId: string,
public fieldName: string,
public indexPattern: IndexPattern,
public queryFilter: QueryFilterManager
private indexPatternId: string,
private indexPatternsService: IndexPatternsContract,
protected queryFilter: QueryFilterManager
) {}

/**
Expand All @@ -41,20 +49,30 @@ export abstract class FilterManager {

abstract getValueFromFilterBar(): any;

getIndexPattern(): IndexPattern {
async init() {
try {
if (!this.indexPattern) {
this.indexPattern = await this.indexPatternsService.get(this.indexPatternId);
}
} catch (e) {
// invalid index pattern id
}
}

getIndexPattern(): IndexPattern | undefined {
return this.indexPattern;
}

getField() {
return this.indexPattern.fields.getByName(this.fieldName);
return this.indexPattern?.fields.getByName(this.fieldName);
}

findFilters(): Filter[] {
const kbnFilters = _.flatten([
this.queryFilter.getAppFilters(),
this.queryFilter.getGlobalFilters(),
]);
return kbnFilters.filter((kbnFilter) => {
return kbnFilters.filter((kbnFilter: Filter) => {
return _.get(kbnFilter, 'meta.controlledBy') === this.controlId;
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,12 @@

import expect from '@kbn/expect';

import { Filter, IndexPattern, FilterManager as QueryFilterManager } from '../../../../data/public';
import {
Filter,
IndexPattern,
FilterManager as QueryFilterManager,
IndexPatternsContract,
} from '../../../../data/public';
import { PhraseFilterManager } from './phrase_filter_manager';

describe('PhraseFilterManager', function () {
Expand All @@ -42,15 +47,20 @@ describe('PhraseFilterManager', function () {
},
},
} as IndexPattern;
const indexPatternsServiceMock = ({
get: jest.fn().mockReturnValue(Promise.resolve(indexPatternMock)),
} as unknown) as jest.Mocked<IndexPatternsContract>;
const queryFilterMock: QueryFilterManager = {} as QueryFilterManager;
let filterManager: PhraseFilterManager;
beforeEach(() => {
beforeEach(async () => {
filterManager = new PhraseFilterManager(
controlId,
'field1',
indexPatternMock,
'1',
indexPatternsServiceMock,
queryFilterMock
);
await filterManager.init();
});

test('should create match phrase filter from single value', function () {
Expand Down Expand Up @@ -89,10 +99,11 @@ describe('PhraseFilterManager', function () {
constructor(
id: string,
fieldName: string,
indexPattern: IndexPattern,
indexPatternId: string,
indexPatternsService: IndexPatternsContract,
queryFilter: QueryFilterManager
) {
super(id, fieldName, indexPattern, queryFilter);
super(id, fieldName, indexPatternId, indexPatternsService, queryFilter);
this.mockFilters = [];
}

Expand All @@ -105,14 +116,15 @@ describe('PhraseFilterManager', function () {
}
}

const indexPatternMock: IndexPattern = {} as IndexPattern;
const indexPatternsServiceMock = {} as IndexPatternsContract;
const queryFilterMock: QueryFilterManager = {} as QueryFilterManager;
let filterManager: MockFindFiltersPhraseFilterManager;
beforeEach(() => {
filterManager = new MockFindFiltersPhraseFilterManager(
controlId,
'field1',
indexPatternMock,
'1',
indexPatternsServiceMock,
queryFilterMock
);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,32 +23,34 @@ import { FilterManager } from './filter_manager';
import {
PhraseFilter,
esFilters,
IndexPattern,
IndexPatternsContract,
FilterManager as QueryFilterManager,
} from '../../../../data/public';

export class PhraseFilterManager extends FilterManager {
constructor(
controlId: string,
fieldName: string,
indexPattern: IndexPattern,
indexPatternId: string,
indexPatternsService: IndexPatternsContract,
queryFilter: QueryFilterManager
) {
super(controlId, fieldName, indexPattern, queryFilter);
super(controlId, fieldName, indexPatternId, indexPatternsService, queryFilter);
}

createFilter(phrases: any): PhraseFilter {
const indexPattern = this.getIndexPattern()!;
let newFilter: PhraseFilter;
const value = this.indexPattern.fields.getByName(this.fieldName);
const value = indexPattern.fields.getByName(this.fieldName);

if (!value) {
throw new Error(`Unable to find field with name: ${this.fieldName} on indexPattern`);
}

if (phrases.length === 1) {
newFilter = esFilters.buildPhraseFilter(value, phrases[0], this.indexPattern);
newFilter = esFilters.buildPhraseFilter(value, phrases[0], indexPattern);
} else {
newFilter = esFilters.buildPhrasesFilter(value, phrases, this.indexPattern);
newFilter = esFilters.buildPhrasesFilter(value, phrases, indexPattern);
}

newFilter.meta.key = this.fieldName;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
RangeFilterMeta,
IndexPattern,
FilterManager as QueryFilterManager,
IndexPatternsContract,
} from '../../../../data/public';

describe('RangeFilterManager', function () {
Expand All @@ -46,15 +47,20 @@ describe('RangeFilterManager', function () {
},
},
} as IndexPattern;
const indexPatternsServiceMock = ({
get: jest.fn().mockReturnValue(Promise.resolve(indexPatternMock)),
} as unknown) as jest.Mocked<IndexPatternsContract>;
const queryFilterMock: QueryFilterManager = {} as QueryFilterManager;
let filterManager: RangeFilterManager;
beforeEach(() => {
beforeEach(async () => {
filterManager = new RangeFilterManager(
controlId,
'field1',
indexPatternMock,
'1',
indexPatternsServiceMock,
queryFilterMock
);
await filterManager.init();
});

test('should create range filter from slider value', function () {
Expand All @@ -75,10 +81,11 @@ describe('RangeFilterManager', function () {
constructor(
id: string,
fieldName: string,
indexPattern: IndexPattern,
indexPatternId: string,
indexPatternsService: IndexPatternsContract,
queryFilter: QueryFilterManager
) {
super(id, fieldName, indexPattern, queryFilter);
super(id, fieldName, indexPatternId, indexPatternsService, queryFilter);
this.mockFilters = [];
}

Expand All @@ -91,14 +98,15 @@ describe('RangeFilterManager', function () {
}
}

const indexPatternMock: IndexPattern = {} as IndexPattern;
const indexPatternsServiceMock = {} as IndexPatternsContract;
const queryFilterMock: QueryFilterManager = {} as QueryFilterManager;
let filterManager: MockFindFiltersRangeFilterManager;
beforeEach(() => {
filterManager = new MockFindFiltersRangeFilterManager(
controlId,
'field1',
indexPatternMock,
'1',
indexPatternsServiceMock,
queryFilterMock
);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,12 @@ export class RangeFilterManager extends FilterManager {
* @return {object} range filter
*/
createFilter(value: SliderValue): RangeFilter {
const indexPattern = this.getIndexPattern()!;
const newFilter = esFilters.buildRangeFilter(
// TODO: Fix type to be required
this.indexPattern.fields.getByName(this.fieldName) as IFieldType,
indexPattern.fields.getByName(this.fieldName) as IFieldType,
toRange(value),
this.indexPattern
indexPattern
);
newFilter.meta.key = this.fieldName;
newFilter.meta.controlledBy = this.controlId;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,17 @@ export class ListControl extends Control<PhraseFilterManager> {
timeout: `${settings.autocompleteTimeout}ms`,
terminate_after: Number(settings.autocompleteTerminateAfter),
};

// dynamic options are only allowed on String fields but the setting defaults to true so it could
// be enabled for non-string fields (since UI input is hidden for non-string fields).
// If field is not string, then disable dynamic options.
const field = indexPattern?.fields
.getAll()
.find(({ name }) => name === this.controlParams.fieldName);
if (field && field.type !== 'string') {
this.options.dynamicOptions = false;
}

const aggs = termsAgg({
field: indexPattern.fields.getByName(fieldName),
size: this.options.dynamicOptions ? null : _.get(this.options, 'size', 5),
Expand Down Expand Up @@ -213,27 +224,20 @@ export async function listControlFactory(
deps: InputControlVisDependencies
) {
const [, { data: dataPluginStart }] = await deps.core.getStartServices();
const indexPattern = await dataPluginStart.indexPatterns.get(controlParams.indexPattern);

// dynamic options are only allowed on String fields but the setting defaults to true so it could
// be enabled for non-string fields (since UI input is hidden for non-string fields).
// If field is not string, then disable dynamic options.
const field = indexPattern.fields.getAll().find(({ name }) => name === controlParams.fieldName);
if (field && field.type !== 'string') {
controlParams.options.dynamicOptions = false;
}

const listControl = new ListControl(
controlParams,
new PhraseFilterManager(
controlParams.id,
controlParams.fieldName,
indexPattern,
controlParams.indexPattern,
dataPluginStart.indexPatterns,
deps.data.query.filterManager
),
useTimeFilter,
dataPluginStart.search.searchSource,
deps
);
await listControl.filterManager.init();
return listControl;
}
Original file line number Diff line number Diff line change
Expand Up @@ -134,18 +134,20 @@ export async function rangeControlFactory(
deps: InputControlVisDependencies
): Promise<RangeControl> {
const [, { data: dataPluginStart }] = await deps.core.getStartServices();
const indexPattern = await dataPluginStart.indexPatterns.get(controlParams.indexPattern);

return new RangeControl(
const rangeControl = new RangeControl(
controlParams,
new RangeFilterManager(
controlParams.id,
controlParams.fieldName,
indexPattern,
controlParams.indexPattern,
dataPluginStart.indexPatterns,
deps.data.query.filterManager
),
useTimeFilter,
dataPluginStart.search.searchSource,
deps
);
await rangeControl.filterManager.init();
return rangeControl;
}