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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Sep 10, 2024
Merged

Filter Saved Search Based on AppName Supported by Language #7999

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e794db9
Filter Saved Search Based on AppName Supported by Language
LDrago27 Sep 4, 2024
1fdbf88
Fixed test failures
LDrago27 Sep 9, 2024
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
7 changes: 6 additions & 1 deletion src/plugins/dashboard/public/application/utils/create_dashboard_container.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,12 @@ const getEmptyScreenProps = (
getFactory: embeddable.getEmbeddableFactory,
notifications,
overlays,
SavedObjectFinder: getSavedObjectFinder(savedObjects, uiSettings),
SavedObjectFinder: getSavedObjectFinder(
Copy link
Member

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

Copy link
Collaborator Author

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.

savedObjects,
uiSettings,
services.data,
services.application
),
});
}
} else {
Expand Down
7 changes: 6 additions & 1 deletion src/plugins/dashboard/public/application/utils/get_nav_actions.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,12 @@ export const getNavActions = (
getFactory: embeddable.getEmbeddableFactory,
notifications,
overlays,
SavedObjectFinder: getSavedObjectFinder(savedObjects, uiSettings),
SavedObjectFinder: getSavedObjectFinder(
savedObjects,
uiSettings,
services.data,
services.application
),
});
}
};
Expand Down
7 changes: 6 additions & 1 deletion src/plugins/dashboard/public/plugin.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -567,7 +567,12 @@
embeddable,
} = plugins;

const SavedObjectFinder = getSavedObjectFinder(core.savedObjects, core.uiSettings);
const SavedObjectFinder = getSavedObjectFinder(

Check warning on line 570 in src/plugins/dashboard/public/plugin.tsx

View check run for this annotation

Codecov / codecov/patch

src/plugins/dashboard/public/plugin.tsx#L570 

Added line #L570 was not covered by tests
core.savedObjects,
core.uiSettings,
plugins.data,
core.application
);

const changeViewAction = new ReplacePanelAction(
core,
Expand Down
1 change: 1 addition & 0 deletions src/plugins/data/public/query/query_string/language_service/lib/dql_language.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,5 +24,6 @@ export const getDQLLanguageConfig = (
},
showDocLinks: true,
editorSupportedAppNames: ['discover'],
supportedAppNames: ['discover', 'dashboards', 'visualize', 'data-explorer'],
};
};
1 change: 1 addition & 0 deletions src/plugins/data/public/query/query_string/language_service/lib/lucene_language.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,5 +24,6 @@ export const getLuceneLanguageConfig = (
},
showDocLinks: true,
editorSupportedAppNames: ['discover'],
supportedAppNames: ['discover', 'dashboards', 'visualize', 'data-explorer'],
};
};
1 change: 1 addition & 0 deletions src/plugins/data/public/query/query_string/language_service/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,4 +50,5 @@ export interface LanguageConfig {
};
showDocLinks?: boolean;
editorSupportedAppNames?: string[];
supportedAppNames?: string[];
}
3 changes: 3 additions & 0 deletions ...cover/public/application/components/top_nav/__snapshots__/open_search_panel.test.tsx.snap
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions src/plugins/discover/public/application/components/top_nav/open_search_panel.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ export function OpenSearchPanel({ onClose, makeUrl }: Props) {
services: {
core: { uiSettings, savedObjects, application },
addBasePath,
data,
},
} = useOpenSearchDashboards<DiscoverViewServices>();

Expand Down Expand Up @@ -87,6 +88,7 @@ export function OpenSearchPanel({ onClose, makeUrl }: Props) {
name: i18n.translate('discover.savedSearch.savedObjectName', {
defaultMessage: 'Saved search',
}),
includeFields: ['kibanaSavedObjectMeta'],
},
]}
onChoose={(id) => {
Expand All @@ -95,6 +97,8 @@ export function OpenSearchPanel({ onClose, makeUrl }: Props) {
}}
uiSettings={uiSettings}
savedObjects={savedObjects}
application={application}
data={data}
/>
</EuiFlyoutBody>
<EuiFlyoutFooter>
Expand Down
1 change: 1 addition & 0 deletions src/plugins/discover/public/embeddable/search_embeddable_factory.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ export class SearchEmbeddableFactory
}),
type: 'search',
getIconForSavedObject: () => 'search',
includeFields: ['kibanaSavedObjectMeta'],
};

constructor(private getStartServices: () => Promise<StartServices>) {}
Expand Down
7 changes: 6 additions & 1 deletion src/plugins/embeddable/public/plugin.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,12 @@ export class EmbeddablePublicPlugin implements Plugin<EmbeddableSetup, Embeddabl
notifications={core.notifications}
application={core.application}
inspector={inspector}
SavedObjectFinder={getSavedObjectFinder(core.savedObjects, core.uiSettings)}
SavedObjectFinder={getSavedObjectFinder(
core.savedObjects,
core.uiSettings,
data,
core.application
)}
/>
);

Expand Down
2 changes: 2 additions & 0 deletions src/plugins/query_enhancements/public/plugin.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ export class QueryEnhancementsPlugin
showDocLinks: false,
editor: enhancedPPLQueryEditor,
editorSupportedAppNames: ['discover'],
supportedAppNames: ['discover', 'data-explorer'],
};
queryString.getLanguageService().registerLanguage(pplLanguageConfig);

Expand All @@ -98,6 +99,7 @@ export class QueryEnhancementsPlugin
showDocLinks: false,
editor: enhancedSQLQueryEditor,
editorSupportedAppNames: ['discover'],
supportedAppNames: ['discover', 'data-explorer'],
};
queryString.getLanguageService().registerLanguage(sqlLanguageConfig);

Expand Down
55 changes: 53 additions & 2 deletions src/plugins/saved_objects/public/finder/saved_object_finder.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,12 @@
CoreStart,
IUiSettingsClient,
SavedObjectsStart,
ApplicationStart,
} from 'src/core/public';

import { DataSourceAttributes } from 'src/plugins/data_source/common/data_sources';
import { DataPublicPluginStart, LanguageServiceContract } from 'src/plugins/data/public';
import { first } from 'rxjs/operators';
import { getIndexPatternTitle } from '../../../data/common/index_patterns/utils';
import { LISTING_LIMIT_SETTING } from '../../common';

Expand All @@ -76,6 +79,7 @@
interface FinderAttributes {
title?: string;
type: string;
kibanaSavedObjectMeta?: string;
}

interface SavedObjectFinderState {
Expand Down Expand Up @@ -122,6 +126,8 @@
export type SavedObjectFinderUiProps = {
savedObjects: CoreStart['savedObjects'];
uiSettings: CoreStart['uiSettings'];
data?: DataPublicPluginStart;
application?: CoreStart['application'];
} & SavedObjectFinderProps;

class SavedObjectFinderUi extends React.Component<
Expand All @@ -137,8 +143,29 @@
showFilter: PropTypes.bool,
};

public async getCurrentAppId() {
return (
(await this.props?.application?.currentAppId$?.pipe(first()).toPromise()) ??
Promise.resolve(undefined)
);
}

readonly languageService = this.props.data?.query?.queryString?.getLanguageService();
private isComponentMounted: boolean = false;

private isSavedSearchLanguageSupported(
languageId?: string,
currentAppId?: string,
languageService?: LanguageServiceContract
) {
if (!languageId || !currentAppId || !languageService) {
return true;
}
return (

Check warning on line 164 in src/plugins/saved_objects/public/finder/saved_object_finder.tsx

View check run for this annotation

Codecov / codecov/patch

src/plugins/saved_objects/public/finder/saved_object_finder.tsx#L164 

Added line #L164 was not covered by tests
languageService?.getLanguage(languageId)?.supportedAppNames?.includes(currentAppId) ?? true
);
}

private debouncedFetch = _.debounce(async (query: string) => {
const metaDataMap = this.getSavedObjectMetaDataMap();

Expand All @@ -162,6 +189,8 @@
return await client.get<DataSourceAttributes>('data-source', id);
};

const currentAppId = await this.getCurrentAppId();

const savedObjects = await Promise.all(
resp.savedObjects.map(async (obj) => {
if (obj.type === 'index-pattern') {
Expand All @@ -172,13 +201,24 @@
getDataSource
);
return result;
} else if (obj.type === 'search') {
const sourceObject = JSON.parse(
obj.attributes?.kibanaSavedObjectMeta?.searchSourceJSON ?? null
);
const languageId = sourceObject?.query?.language;
if (this.isSavedSearchLanguageSupported(languageId, currentAppId, this.languageService)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i wonder if it makes it a little bit more scalable/safer to just have this as a pass a function

like from the dashboards it could be

 SavedObjectFinder: getSavedObjectFinder(
          savedObjects,
          uiSettings,
          searchFilterFn: (obj) => {
              const sourceObject = JSON.parse(
              obj.attributes?.kibanaSavedObjectMeta?.searchSourceJSON ?? null);
              const languageId = sourceObject?.query?.language;
              services.data.languageService?.getLanguage(languageId)?.supportedAppNames?.includes(currentAppId) ?? true;
          }
      )

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think i only worry ensuring we dont end up adding more required plugins since i know a good amount of community members hit the APIs directly and even though this is the public side the saved objects plugin has been historically didn't need to care about the application which made it reliably abstracted and enforced a pattern for developers.

do we forsee in issues with introducing these concepts of language service to the saved object finder?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The overall change seems safe and the default behavior without the additional parameters remains unchanged for SavedObjectFinder. The saved_object_finder.test.tsx tests all use the default version of SavedObjectFinder without the optional parameters and the behavior remains unchanged.

In the meanwhile I have created the issue #8122 , which we can use to create a more scalable version of the above when required.

return obj;
}
} else {
return obj;
}
})
);

resp.savedObjects = savedObjects.filter((savedObject) => {
if (!savedObject) {
return false;

Check warning on line 220 in src/plugins/saved_objects/public/finder/saved_object_finder.tsx

View check run for this annotation

Codecov / codecov/patch

src/plugins/saved_objects/public/finder/saved_object_finder.tsx#L220 

Added line #L220 was not covered by tests
}
const metaData = metaDataMap[savedObject.type];
if (metaData.showSavedObject) {
return metaData.showSavedObject(savedObject);
Expand Down Expand Up @@ -571,9 +611,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

Check warning on line 621 in src/plugins/saved_objects/public/finder/saved_object_finder.tsx

View check run for this annotation

Codecov / codecov/patch

src/plugins/saved_objects/public/finder/saved_object_finder.tsx#L621 

Added line #L621 was not covered by tests
{...props}
savedObjects={savedObject}
uiSettings={uiSettings}
data={data}
application={application}
/>
);
};

Expand Down
2 changes: 2 additions & 0 deletions src/plugins/visualizations/public/plugin.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@
setEmbeddable,
setNotifications,
setDocLinks,
setDataStart,
} from './services';
import {
VISUALIZE_EMBEDDABLE_TYPE,
Expand Down Expand Up @@ -192,6 +193,7 @@
chrome: core.chrome,
overlays: core.overlays,
});
setDataStart(data);

Check warning on line 196 in src/plugins/visualizations/public/plugin.ts

View check run for this annotation

Codecov / codecov/patch

src/plugins/visualizations/public/plugin.ts#L196 

Added line #L196 was not covered by tests
setSavedAugmentVisLoader(savedAugmentVisLoader);
setI18n(core.i18n);
setTypes(types);
Expand Down
2 changes: 2 additions & 0 deletions src/plugins/visualizations/public/services.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,3 +119,5 @@ export const [getSavedAugmentVisLoader, setSavedAugmentVisLoader] = createGetter
>('savedAugmentVisLoader');

export const [getDocLinks, setDocLinks] = createGetterSetter<DocLinksStart>('docLinks');

export const [getDataStart, setDataStart] = createGetterSetter<DataPublicPluginStart>('DataStart');
4 changes: 4 additions & 0 deletions src/plugins/visualizations/public/wizard/new_vis_modal.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import { EuiModal } from '@elastic/eui';
import { i18n } from '@osd/i18n';

import { METRIC_TYPE, UiStatsMetricType } from '@osd/analytics';
import { DataPublicPluginStart } from 'src/plugins/data/public';
import { ApplicationStart, IUiSettingsClient, SavedObjectsStart } from '../../../../core/public';
import { SearchSelection } from './search_selection';
import { TypeSelection } from './type_selection';
Expand All @@ -55,6 +56,7 @@ interface TypeSelectionProps {
outsideVisualizeApp?: boolean;
stateTransfer?: EmbeddableStateTransfer;
originatingApp?: string;
data: DataPublicPluginStart;
}

interface TypeSelectionState {
Expand Down Expand Up @@ -112,6 +114,8 @@ class NewVisModal extends React.Component<TypeSelectionProps, TypeSelectionState
visType={this.state.visType}
uiSettings={this.props.uiSettings}
savedObjects={this.props.savedObjects}
application={this.props.application}
data={this.props.data}
/>
</EuiModal>
) : (
Expand Down
8 changes: 7 additions & 1 deletion src/plugins/visualizations/public/wizard/search_selection/search_selection.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ import { EuiModalBody, EuiModalHeader, EuiModalHeaderTitle } from '@elastic/eui'
import { i18n } from '@osd/i18n';
import { FormattedMessage } from '@osd/i18n/react';
import React from 'react';
import { IUiSettingsClient, SavedObjectsStart } from '../../../../../core/public';
import { DataPublicPluginStart } from 'src/plugins/data/public';
import { ApplicationStart, IUiSettingsClient, SavedObjectsStart } from '../../../../../core/public';

import { SavedObjectFinderUi } from '../../../../saved_objects/public';
import { VisType } from '../../vis_types';
Expand All @@ -42,6 +43,8 @@ interface SearchSelectionProps {
visType: VisType;
uiSettings: IUiSettingsClient;
savedObjects: SavedObjectsStart;
data: DataPublicPluginStart;
application: ApplicationStart;
}

export class SearchSelection extends React.Component<SearchSelectionProps> {
Expand Down Expand Up @@ -85,6 +88,7 @@ export class SearchSelection extends React.Component<SearchSelectionProps> {
defaultMessage: 'Saved search',
}
),
includeFields: ['kibanaSavedObjectMeta'],
},
{
type: 'index-pattern',
Expand All @@ -100,6 +104,8 @@ export class SearchSelection extends React.Component<SearchSelectionProps> {
fixedPageSize={this.fixedPageSize}
uiSettings={this.props.uiSettings}
savedObjects={this.props.savedObjects}
application={this.props.application}
data={this.props.data}
/>
</EuiModalBody>
</React.Fragment>
Expand Down
2 changes: 2 additions & 0 deletions src/plugins/visualizations/public/wizard/show_new_vis.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import {
getUsageCollector,
getApplication,
getEmbeddable,
getDataStart,
} from '../services';

export interface ShowNewVisModalParams {
Expand Down Expand Up @@ -90,6 +91,7 @@ export function showNewVisModal({
savedObjects={getSavedObjects()}
usageCollection={getUsageCollector()}
application={getApplication()}
data={getDataStart()}
/>
</I18nProvider>
);
Expand Down
Loading