-
Notifications
You must be signed in to change notification settings - Fork 867
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
Filter Saved Search Based on AppName Supported by Language #7999
Changes from 1 commit
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 |
---|---|---|
|
@@ -58,9 +58,12 @@ | |
CoreStart, | ||
IUiSettingsClient, | ||
SavedObjectsStart, | ||
ApplicationStart, | ||
} from 'src/core/public'; | ||
|
||
import { DataSourceAttributes } from 'src/plugins/data_source/common/data_sources'; | ||
import { DataPublicPluginStart } from 'src/plugins/data/public'; | ||
import { first } from 'rxjs/operators'; | ||
import { getIndexPatternTitle } from '../../../data/common/index_patterns/utils'; | ||
import { LISTING_LIMIT_SETTING } from '../../common'; | ||
|
||
|
@@ -76,6 +79,7 @@ | |
interface FinderAttributes { | ||
title?: string; | ||
type: string; | ||
kibanaSavedObjectMeta?: string; | ||
} | ||
|
||
interface SavedObjectFinderState { | ||
|
@@ -122,6 +126,8 @@ | |
export type SavedObjectFinderUiProps = { | ||
savedObjects: CoreStart['savedObjects']; | ||
uiSettings: CoreStart['uiSettings']; | ||
data: DataPublicPluginStart; | ||
application: CoreStart['application']; | ||
} & SavedObjectFinderProps; | ||
|
||
class SavedObjectFinderUi extends React.Component< | ||
|
@@ -137,10 +143,17 @@ | |
showFilter: PropTypes.bool, | ||
}; | ||
|
||
public async getCurrentAppId() { | ||
const appId = await this.props.application.currentAppId$.pipe(first()).toPromise(); | ||
return appId; | ||
} | ||
|
||
readonly languageService = this.props.data.query.queryString.getLanguageService(); | ||
private isComponentMounted: boolean = false; | ||
|
||
private debouncedFetch = _.debounce(async (query: string) => { | ||
const metaDataMap = this.getSavedObjectMetaDataMap(); | ||
const currentAppId = await this.getCurrentAppId(); | ||
|
||
const fields = Object.values(metaDataMap) | ||
.map((metaData) => metaData.includeFields || []) | ||
|
@@ -162,7 +175,7 @@ | |
return await client.get<DataSourceAttributes>('data-source', id); | ||
}; | ||
|
||
const savedObjects = await Promise.all( | ||
let savedObjects = await Promise.all( | ||
resp.savedObjects.map(async (obj) => { | ||
if (obj.type === 'index-pattern') { | ||
const result = { ...obj }; | ||
|
@@ -178,6 +191,25 @@ | |
}) | ||
); | ||
|
||
// Filter search objects whose language are not supported by the current Application | ||
savedObjects = savedObjects.filter((obj) => { | ||
LDrago27 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if (obj.type === 'search') { | ||
if (obj.attributes?.kibanaSavedObjectMeta) { | ||
const sourceObject = JSON.parse(obj.attributes?.kibanaSavedObjectMeta?.searchSourceJSON); | ||
const languageId = sourceObject.query.language; | ||
|
||
const languageProperties = this.languageService.getLanguage(languageId); | ||
|
||
if (languageProperties?.supportedAppNames.includes(currentAppId)) { | ||
LDrago27 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return true; | ||
} else { | ||
return false; | ||
} | ||
} | ||
} | ||
return true; | ||
}); | ||
|
||
resp.savedObjects = savedObjects.filter((savedObject) => { | ||
const metaData = metaDataMap[savedObject.type]; | ||
if (metaData.showSavedObject) { | ||
|
@@ -571,9 +603,20 @@ | |
} | ||
} | ||
|
||
const getSavedObjectFinder = (savedObject: SavedObjectsStart, uiSettings: IUiSettingsClient) => { | ||
const getSavedObjectFinder = ( | ||
savedObject: SavedObjectsStart, | ||
uiSettings: IUiSettingsClient, | ||
data: DataPublicPluginStart, | ||
application: ApplicationStart | ||
) => { | ||
return (props: SavedObjectFinderProps) => ( | ||
<SavedObjectFinderUi {...props} savedObjects={savedObject} uiSettings={uiSettings} /> | ||
<SavedObjectFinderUi | ||
{...props} | ||
savedObjects={savedObject} | ||
uiSettings={uiSettings} | ||
data={data} | ||
application={application} | ||
/> | ||
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. @kavilla If we want to do a quick fix for this breaking change, we could make the new parameters optional.
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. Thanks @sejli I have taken your idea and made the new parameters as optional. Therefore if you don't pass in these params they should work in a similar way as before. To highlight the non breaking nature of changes I have reverted back the changes made to the test where we were using it without the new params to ensure it does n't break the functionality elsewhere. |
||
); | ||
}; | ||
|
||
|
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.
oh sorry about. not catching this earlier. im not sure if we want to update the interface like this as it might be a breaking change for any plugin using this.
i see the saved objects plugin gets started up with the services and with it is the data plugin. potentially the same with the services.application? that way we don't need to update the interface. since i can see it already touches a bunch of plugins within OSD. But no positive any plugins outside this repo (non-OpenSearch project and OpenSearch project)
https://github.com/opensearch-project/OpenSearch-Dashboards/blob/main/src/plugins/saved_objects/public/plugin.ts#L57
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.
@kavilla I tried this approach, however I see the downstream applications are directly importing the component without using the pluginStart props. Hence I decided against it.