-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Index pattern public api => common #68289
Changes from 15 commits
a2da8d3
3a40208
cb9e775
5de1b21
2683b7a
2913835
5cfaee5
f963496
ec2f0c6
ebc56d8
c994a1f
f097b31
6998499
01e6e2f
f75873f
49a06d7
ba4d131
30bf1e0
8e0f3b8
bce0d2e
90a8695
240371a
d8b1999
6c30c28
f96d3c3
5beadc0
7aeb8f2
ace29ae
e3a16b3
0b7f84f
885c8ce
03e849d
985e90a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,5 +7,5 @@ | |
<b>Signature:</b> | ||
|
||
```typescript | ||
indexPattern?: IndexPattern; | ||
indexPattern?: IIndexPattern; | ||
``` |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,17 +18,17 @@ | |
*/ | ||
|
||
import { findIndex } from 'lodash'; | ||
import { ToastsStart } from 'kibana/public'; | ||
import { IndexPattern } from '../index_patterns'; | ||
import { IIndexPattern } from '../../types'; | ||
import { IFieldType } from '../../../common'; | ||
import { Field, FieldSpec } from './field'; | ||
import { FieldFormatsStart } from '../../field_formats'; | ||
import { OnNotification } from '../types'; | ||
import { FieldFormatsStartCommon } from '../../field_formats'; | ||
|
||
type FieldMap = Map<Field['name'], Field>; | ||
|
||
interface FieldListDependencies { | ||
fieldFormats: FieldFormatsStart; | ||
toastNotifications: ToastsStart; | ||
fieldFormats: FieldFormatsStartCommon; | ||
onNotification: OnNotification; | ||
} | ||
|
||
export interface IIndexPatternFieldList extends Array<Field> { | ||
|
@@ -40,19 +40,19 @@ export interface IIndexPatternFieldList extends Array<Field> { | |
} | ||
|
||
export type CreateIndexPatternFieldList = ( | ||
indexPattern: IndexPattern, | ||
indexPattern: IIndexPattern, | ||
specs?: FieldSpec[], | ||
shortDotsEnable?: boolean | ||
) => IIndexPatternFieldList; | ||
|
||
export const getIndexPatternFieldListCreator = ({ | ||
fieldFormats, | ||
toastNotifications, | ||
onNotification, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. on this level we can then remove onNotification and leave catching the exception to the caller. |
||
}: FieldListDependencies): CreateIndexPatternFieldList => (...fieldListParams) => { | ||
class FieldList extends Array<Field> implements IIndexPatternFieldList { | ||
private byName: FieldMap = new Map(); | ||
private groups: Map<Field['type'], FieldMap> = new Map(); | ||
private indexPattern: IndexPattern; | ||
private indexPattern: IIndexPattern; | ||
private shortDotsEnable: boolean; | ||
private setByName = (field: Field) => this.byName.set(field.name, field); | ||
private setByGroup = (field: Field) => { | ||
|
@@ -63,7 +63,7 @@ export const getIndexPatternFieldListCreator = ({ | |
}; | ||
private removeByGroup = (field: IFieldType) => this.groups.get(field.type)!.delete(field.name); | ||
|
||
constructor(indexPattern: IndexPattern, specs: FieldSpec[] = [], shortDotsEnable = false) { | ||
constructor(indexPattern: IIndexPattern, specs: FieldSpec[] = [], shortDotsEnable = false) { | ||
super(); | ||
this.indexPattern = indexPattern; | ||
this.shortDotsEnable = shortDotsEnable; | ||
|
@@ -76,7 +76,7 @@ export const getIndexPatternFieldListCreator = ({ | |
add = (field: FieldSpec) => { | ||
const newField = new Field(this.indexPattern, field, this.shortDotsEnable, { | ||
fieldFormats, | ||
toastNotifications, | ||
onNotification, | ||
}); | ||
this.push(newField); | ||
this.setByName(newField); | ||
|
@@ -94,7 +94,7 @@ export const getIndexPatternFieldListCreator = ({ | |
update = (field: FieldSpec) => { | ||
const newField = new Field(this.indexPattern, field, this.shortDotsEnable, { | ||
fieldFormats, | ||
toastNotifications, | ||
onNotification, | ||
}); | ||
const index = this.findIndex((f) => f.name === newField.name); | ||
this.splice(index, 1, newField); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,57 @@ | ||
/* | ||
* 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 { contains } from 'lodash'; | ||
import { CoreStart } from 'kibana/public'; | ||
import { IndexPatternsContract } from './index_patterns'; | ||
|
||
export type EnsureDefaultIndexPattern = () => Promise<unknown> | undefined; | ||
|
||
export const createEnsureDefaultIndexPattern = ( | ||
core: CoreStart, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. lets not pass in whole core but just uiSettingsClient There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is there a reason this is wrapped in a function ? why not just export ensureDefaultIndexPattern ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I'd prefer to leave further refactoring of this code to future PRs. I'm drawing a rather arbitrary line since I need to do things backwards - provide the service and then clean up the code. |
||
onRedirectNoIndexPattern: () => Promise<unknown> | void | ||
) => { | ||
/** | ||
* Checks whether a default index pattern is set and exists and defines | ||
* one otherwise. | ||
*/ | ||
return async function ensureDefaultIndexPattern(this: IndexPatternsContract) { | ||
const patterns = await this.getIds(); | ||
let defaultId = core.uiSettings.get('defaultIndex'); | ||
let defined = !!defaultId; | ||
const exists = contains(patterns, defaultId); | ||
|
||
if (defined && !exists) { | ||
core.uiSettings.remove('defaultIndex'); | ||
defaultId = defined = false; | ||
} | ||
|
||
if (defined) { | ||
return; | ||
} | ||
|
||
// If there is any index pattern created, set the first as default | ||
if (patterns.length >= 1) { | ||
defaultId = patterns[0]; | ||
core.uiSettings.set('defaultIndex', defaultId); | ||
} else { | ||
return onRedirectNoIndexPattern(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. lets have this method always return and the caller decide what to do (no need to pass in callbacks) |
||
} | ||
}; | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
/* | ||
* 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. | ||
*/ | ||
|
||
export * from './index_patterns_api_client'; | ||
export * from './types'; | ||
export * from './_pattern_cache'; | ||
export * from './flatten_hit'; | ||
export * from './format_hit'; | ||
export * from './index_pattern'; | ||
export * from './index_patterns'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what if we throw here and let the caller handle the exception. instead of passing
onNotification
in they can wrap code in try/cacheThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is good idea but I'd prefer to address it in another ticket as this PR aims to be a minimal set of changes required to provide a common api. I think I'd need to examine all places where index patterns are used to ensure errors are being caught. I've added a ticket to #67920