Skip to content

Commit

Permalink
[8.x] Sustainable Kibana Architecture: Remove dependencies between pl…
Browse files Browse the repository at this point in the history
…ugins that are related by _App Links_ (#199492) (#200159)

# Backport

This will backport the following commits from `main` to `8.x`:
- [Sustainable Kibana Architecture: Remove dependencies between plugins
that are related by _App Links_
(#199492)](#199492)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Gerard
Soldevila","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-11-14T11:49:21Z","message":"Sustainable
Kibana Architecture: Remove dependencies between plugins that are
related by _App Links_ (#199492)\n\n## Summary\r\n\r\nThis PR introduces
a Core API to check whether a given application has\r\nbeen
registered.\r\nPlugins can use this for their _App Links_, without
having to depend on\r\nthe referenced plugin(s) anymore.\r\n\r\nThis
way, we can get rid of some inter-solution dependencies,
and\r\ncategorise plugins more
appropriately.\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine
<[email protected]>","sha":"ad56ec5f1a838486d9b38c2d5aa92bbf77e127a3","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:Core","release_note:skip","v9.0.0","backport:prev-minor","Team:Obs
AI Assistant","ci:project-deploy-observability"],"title":"Sustainable
Kibana Architecture: Remove dependencies between plugins that are
related by _App
Links_","number":199492,"url":"https://github.com/elastic/kibana/pull/199492","mergeCommit":{"message":"Sustainable
Kibana Architecture: Remove dependencies between plugins that are
related by _App Links_ (#199492)\n\n## Summary\r\n\r\nThis PR introduces
a Core API to check whether a given application has\r\nbeen
registered.\r\nPlugins can use this for their _App Links_, without
having to depend on\r\nthe referenced plugin(s) anymore.\r\n\r\nThis
way, we can get rid of some inter-solution dependencies,
and\r\ncategorise plugins more
appropriately.\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine
<[email protected]>","sha":"ad56ec5f1a838486d9b38c2d5aa92bbf77e127a3"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/199492","number":199492,"mergeCommit":{"message":"Sustainable
Kibana Architecture: Remove dependencies between plugins that are
related by _App Links_ (#199492)\n\n## Summary\r\n\r\nThis PR introduces
a Core API to check whether a given application has\r\nbeen
registered.\r\nPlugins can use this for their _App Links_, without
having to depend on\r\nthe referenced plugin(s) anymore.\r\n\r\nThis
way, we can get rid of some inter-solution dependencies,
and\r\ncategorise plugins more
appropriately.\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine
<[email protected]>","sha":"ad56ec5f1a838486d9b38c2d5aa92bbf77e127a3"}}]}]
BACKPORT-->

Co-authored-by: Gerard Soldevila <[email protected]>
  • Loading branch information
kibanamachine and gsoldevila authored Nov 14, 2024
1 parent 4c15ff4 commit 9047ddb
Show file tree
Hide file tree
Showing 22 changed files with 242 additions and 195 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,14 @@ import { themeServiceMock } from '@kbn/core-theme-browser-mocks';
import { overlayServiceMock } from '@kbn/core-overlays-browser-mocks';
import { customBrandingServiceMock } from '@kbn/core-custom-branding-browser-mocks';
import { analyticsServiceMock } from '@kbn/core-analytics-browser-mocks';
import { MockLifecycle } from './test_helpers/test_types';
import type { MockLifecycle } from './test_helpers/test_types';
import { ApplicationService } from './application_service';
import {
App,
AppDeepLink,
type App,
type AppDeepLink,
AppStatus,
AppUpdater,
PublicAppInfo,
type AppUpdater,
type PublicAppInfo,
} from '@kbn/core-application-browser';
import { act } from 'react-dom/test-utils';
import { DEFAULT_APP_VISIBILITY } from './utils';
Expand Down Expand Up @@ -618,6 +618,26 @@ describe('#start()', () => {
});
});

describe('isAppRegistered', () => {
let isAppRegistered: any;
beforeEach(async () => {
const { register } = service.setup(setupDeps);
register(Symbol(), createApp({ id: 'one_app' }));
register(Symbol(), createApp({ id: 'another_app', appRoute: '/custom/path' }));

const start = await service.start(startDeps);
isAppRegistered = start.isAppRegistered;
});

it('returns false for unregistered apps', () => {
expect(isAppRegistered('oneApp')).toEqual(false);
});

it('returns true for registered apps', () => {
expect(isAppRegistered('another_app')).toEqual(true);
});
});

describe('getUrlForApp', () => {
it('creates URL for unregistered appId', async () => {
service.setup(setupDeps);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,9 @@ export class ApplicationService {
takeUntil(this.stop$)
),
history: this.history!,
isAppRegistered: (appId: string): boolean => {
return applications$.value.get(appId) !== undefined;
},
getUrlForApp: (
appId,
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ const createStartContractMock = (): jest.Mocked<ApplicationStart> => {
navigateToApp: jest.fn(),
navigateToUrl: jest.fn(),
getUrlForApp: jest.fn(),
isAppRegistered: jest.fn(),
};
};

Expand Down Expand Up @@ -92,6 +93,7 @@ const createInternalStartContractMock = (
currentActionMenu$: new BehaviorSubject<MountPoint | undefined>(undefined),
getComponent: jest.fn(),
getUrlForApp: jest.fn(),
isAppRegistered: jest.fn(),
navigateToApp: jest.fn().mockImplementation((appId) => currentAppId$.next(appId)),
navigateToUrl: jest.fn(),
history: createHistoryMock(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,12 @@ export interface ApplicationStart {
applications$: Observable<ReadonlyMap<string, PublicAppInfo>>;

/**
* Navigate to a given app
* Navigate to a given app.
* If a plugin is disabled any applications it registers won't be available either.
* Before rendering a UI element that a user could use to navigate to another application,
* first check if the destination application is actually available using the isAppRegistered API.
*
* @param appId
* @param appId - The identifier of the app to navigate to
* @param options - navigation options
*/
navigateToApp(appId: string, options?: NavigateToAppOptions): Promise<void>;
Expand Down Expand Up @@ -114,6 +117,14 @@ export interface ApplicationStart {
*/
navigateToUrl(url: string, options?: NavigateToUrlOptions): Promise<void>;

/**
* Checks whether a given application is registered.
*
* @param appId - The identifier of the app to check
* @returns true if the given appId is registered in the system, false otherwise.
*/
isAppRegistered(appId: string): boolean;

/**
* Returns the absolute path (or URL) to a given app, including the global base path.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ export function createPluginStartContext<
navigateToApp: deps.application.navigateToApp,
navigateToUrl: deps.application.navigateToUrl,
getUrlForApp: deps.application.getUrlForApp,
isAppRegistered: deps.application.isAppRegistered,
currentLocation$: deps.application.currentLocation$,
},
customBranding: deps.customBranding,
Expand Down
1 change: 1 addition & 0 deletions src/plugins/discover/public/__mocks__/start_contract.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ export const createStartContractMock = (): jest.Mocked<ApplicationStart> => {
capabilities,
navigateToApp: jest.fn(),
navigateToUrl: jest.fn(),
isAppRegistered: jest.fn(),
getUrlForApp: jest.fn(),
};
};

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

8 changes: 5 additions & 3 deletions x-pack/plugins/enterprise_search/kibana.jsonc
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@
"type": "plugin",
"id": "@kbn/enterprise-search-plugin",
"owner": "@elastic/search-kibana",
// Could be categorised as Search in the future, but it currently needs to run in Observability too
"group": "platform",
"visibility": "shared",
// TODO this is currently used from Observability too, must be refactored before solution-specific builds
// see x-pack/plugins/observability_solution/observability_ai_assistant_management/public/routes/components/search_connector_tab.tsx
// cc sphilipse
"group": "search",
"visibility": "private",
"description": "Adds dashboards for discovering and managing Enterprise Search products.",
"plugin": {
"id": "enterpriseSearch",
Expand Down
4 changes: 2 additions & 2 deletions x-pack/plugins/enterprise_search/public/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,8 @@ export type EnterpriseSearchPublicStart = ReturnType<EnterpriseSearchPlugin['sta

interface PluginsSetup {
cloud?: CloudSetup;
licensing: LicensingPluginStart;
home?: HomePublicPluginSetup;
licensing: LicensingPluginStart;
security?: SecurityPluginSetup;
share?: SharePluginSetup;
}
Expand All @@ -98,8 +98,8 @@ export interface PluginsStart {
ml?: MlPluginStart;
navigation: NavigationPublicPluginStart;
searchConnectors?: SearchConnectorsPluginStart;
searchPlayground?: SearchPlaygroundPluginStart;
searchInferenceEndpoints?: SearchInferenceEndpointsPluginStart;
searchPlayground?: SearchPlaygroundPluginStart;
security?: SecurityPluginStart;
share?: SharePluginStart;
}
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/fleet/.storybook/context/application.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export const getApplication = () => {
navigateToApp: async (app: string) => {
action(`Navigate to: ${app}`);
},
isAppRegistered: (appId: string) => true,
getUrlForApp: (url: string) => url,
capabilities: {
catalogue: {},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
"optionalPlugins": [
"home",
"serverless",
"enterpriseSearch"
],
"requiredBundles": [
"kibanaReact",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,10 @@
*/

import { i18n } from '@kbn/i18n';
import { CoreSetup, Plugin, PluginInitializerContext } from '@kbn/core/public';
import { ManagementSetup } from '@kbn/management-plugin/public';
import { HomePublicPluginSetup } from '@kbn/home-plugin/public';
import { ServerlessPluginStart } from '@kbn/serverless/public';
import { EnterpriseSearchPublicStart } from '@kbn/enterprise-search-plugin/public';
import type { CoreSetup, Plugin, PluginInitializerContext } from '@kbn/core/public';
import type { ManagementSetup } from '@kbn/management-plugin/public';
import type { HomePublicPluginSetup } from '@kbn/home-plugin/public';
import type { ServerlessPluginStart } from '@kbn/serverless/public';

import type {
ObservabilityAIAssistantPublicSetup,
Expand All @@ -32,7 +31,6 @@ export interface SetupDependencies {
export interface StartDependencies {
observabilityAIAssistant: ObservabilityAIAssistantPublicStart;
serverless?: ServerlessPluginStart;
enterpriseSearch?: EnterpriseSearchPublicStart;
}

export interface ConfigSchema {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export const SELECTED_CONNECTOR_LOCAL_STORAGE_KEY =

export function SearchConnectorTab() {
const { application } = useKibana().services;
const url = application.getUrlForApp('enterprise_search', { path: '/content/connectors' });
const url = application.getUrlForApp('enterpriseSearch', { path: '/content/connectors' });

return (
<>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,8 @@ export function SettingsPage() {
const { setBreadcrumbs } = useAppContext();
const {
services: {
application: { navigateToApp },
application: { navigateToApp, isAppRegistered },
serverless,
enterpriseSearch,
},
} = useKibana();

Expand Down Expand Up @@ -98,7 +97,7 @@ export function SettingsPage() {
}
),
content: <SearchConnectorTab />,
disabled: enterpriseSearch == null,
disabled: !isAppRegistered('enterpriseSearch'),
},
];

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
"@kbn/core-chrome-browser",
"@kbn/observability-ai-assistant-plugin",
"@kbn/serverless",
"@kbn/enterprise-search-plugin",
"@kbn/management-settings-components-field-row",
"@kbn/observability-shared-plugin",
"@kbn/config-schema",
Expand Down
7 changes: 5 additions & 2 deletions x-pack/plugins/search_inference_endpoints/kibana.jsonc
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,11 @@
"type": "plugin",
"id": "@kbn/search-inference-endpoints",
"owner": "@elastic/search-kibana",
"group": "platform",
"visibility": "shared",
// TODO enterpriseSearch depends on it, and Observability has a menu entry for enterpriseSearch
// must be refactored / fixed before solution-specific builds
// cc sphilipse
"group": "search",
"visibility": "private",
"plugin": {
"id": "searchInferenceEndpoints",
"server": true,
Expand Down
7 changes: 4 additions & 3 deletions x-pack/plugins/search_playground/kibana.jsonc
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@
"type": "plugin",
"id": "@kbn/search-playground",
"owner": "@elastic/search-kibana",
// @kbn/enterprise-search-plugin (platform) and @kbn/serverless-search (search) depend on it
"group": "platform",
"visibility": "shared",
// TODO @kbn/enterprise-search-plugin (platform) and @kbn/serverless-search (search) depend on it
// cc sphilipse
"group": "search",
"visibility": "private",
"plugin": {
"id": "searchPlayground",
"server": true,
Expand Down
Loading

0 comments on commit 9047ddb

Please sign in to comment.