Skip to content

Commit

Permalink
[8.x] [Advanced Settings] Hide settings that are not applicable to cu…
Browse files Browse the repository at this point in the history
…rrent solution (#209136) (#211835)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Advanced Settings] Hide settings that are not applicable to current
solution (#209136)](#209136)

<!--- Backport version: 9.6.6 -->

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

<!--BACKPORT [{"author":{"name":"Elena
Stoeva","email":"59341489+ElenaStoeva@users.noreply.github.com"},"sourceCommit":{"committedDate":"2025-02-19T19:20:06Z","message":"[Advanced
Settings] Hide settings that are not applicable to current solution
(#209136)\n\nCloses
https://github.com/elastic/kibana/issues/196659\n\n## Summary\n\nThis PR
adds a new setting schema field `solution` which is used in
the\nAdvanced settings UI to decide whether to display the setting,
depending\non the solution of the current space. If the `solution` is
not set in\nthe setting definition, the setting will be displayed in all
solutions.\nOtherwise, the setting will only be displayed in the set
solution.\n\nThe current agreement is that we want to display all
settings in the\n\"Observability\" settings category in the Oblt
solution only and all\nsettings in the \"Security Solution\" settings
category in the Security\nsolution only. Therefore, in this PR we set
the `solution` field\naccordingly in the corresponding setting
definitions. Note: We decided\nto add a new setting definition field
`solution` rather than filtering\nby the already existing `category`
field so that this approach works in\nthe future if we want to hide
other single settings outside of these two\ncategories.\n\n**How to
test:**\nVerify that in the classic solution, you can see all settings,
and that\nthe solution-related settings mentioned above are only
displayed in the\ncorresponding
solution.\n\n\n\nhttps://github.com/user-attachments/assets/398ef3e6-973a-4283-ae20-229bf6139d60\n\n---------\n\nCo-authored-by:
kibanamachine
<42973632+kibanamachine@users.noreply.github.com>","sha":"bc3fae5356c677c983138b26c7bc750266e42d1a","branchLabelMapping":{"^v9.1.0$":"main","^v8.19.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Feature:Kibana
Management","Team:Kibana
Management","release_note:skip","v9.0.0","backport:prev-minor","ci:project-deploy-security","Team:obs-ux-management","v9.1.0","v8.19.0"],"title":"[Advanced
Settings] Hide settings that are not applicable to current
solution","number":209136,"url":"https://github.com/elastic/kibana/pull/209136","mergeCommit":{"message":"[Advanced
Settings] Hide settings that are not applicable to current solution
(#209136)\n\nCloses
https://github.com/elastic/kibana/issues/196659\n\n## Summary\n\nThis PR
adds a new setting schema field `solution` which is used in
the\nAdvanced settings UI to decide whether to display the setting,
depending\non the solution of the current space. If the `solution` is
not set in\nthe setting definition, the setting will be displayed in all
solutions.\nOtherwise, the setting will only be displayed in the set
solution.\n\nThe current agreement is that we want to display all
settings in the\n\"Observability\" settings category in the Oblt
solution only and all\nsettings in the \"Security Solution\" settings
category in the Security\nsolution only. Therefore, in this PR we set
the `solution` field\naccordingly in the corresponding setting
definitions. Note: We decided\nto add a new setting definition field
`solution` rather than filtering\nby the already existing `category`
field so that this approach works in\nthe future if we want to hide
other single settings outside of these two\ncategories.\n\n**How to
test:**\nVerify that in the classic solution, you can see all settings,
and that\nthe solution-related settings mentioned above are only
displayed in the\ncorresponding
solution.\n\n\n\nhttps://github.com/user-attachments/assets/398ef3e6-973a-4283-ae20-229bf6139d60\n\n---------\n\nCo-authored-by:
kibanamachine
<42973632+kibanamachine@users.noreply.github.com>","sha":"bc3fae5356c677c983138b26c7bc750266e42d1a"}},"sourceBranch":"main","suggestedTargetBranches":["9.0","8.x"],"targetPullRequestStates":[{"branch":"9.0","label":"v9.0.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v9.1.0","branchLabelMappingKey":"^v9.1.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/209136","number":209136,"mergeCommit":{"message":"[Advanced
Settings] Hide settings that are not applicable to current solution
(#209136)\n\nCloses
https://github.com/elastic/kibana/issues/196659\n\n## Summary\n\nThis PR
adds a new setting schema field `solution` which is used in
the\nAdvanced settings UI to decide whether to display the setting,
depending\non the solution of the current space. If the `solution` is
not set in\nthe setting definition, the setting will be displayed in all
solutions.\nOtherwise, the setting will only be displayed in the set
solution.\n\nThe current agreement is that we want to display all
settings in the\n\"Observability\" settings category in the Oblt
solution only and all\nsettings in the \"Security Solution\" settings
category in the Security\nsolution only. Therefore, in this PR we set
the `solution` field\naccordingly in the corresponding setting
definitions. Note: We decided\nto add a new setting definition field
`solution` rather than filtering\nby the already existing `category`
field so that this approach works in\nthe future if we want to hide
other single settings outside of these two\ncategories.\n\n**How to
test:**\nVerify that in the classic solution, you can see all settings,
and that\nthe solution-related settings mentioned above are only
displayed in the\ncorresponding
solution.\n\n\n\nhttps://github.com/user-attachments/assets/398ef3e6-973a-4283-ae20-229bf6139d60\n\n---------\n\nCo-authored-by:
kibanamachine
<42973632+kibanamachine@users.noreply.github.com>","sha":"bc3fae5356c677c983138b26c7bc750266e42d1a"}},{"branch":"8.x","label":"v8.19.0","branchLabelMappingKey":"^v8.19.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Elena Stoeva <59341489+ElenaStoeva@users.noreply.github.com>
  • Loading branch information
kibanamachine and ElenaStoeva authored Feb 19, 2025
1 parent 8c44d22 commit 7a729c7
Show file tree
Hide file tree
Showing 22 changed files with 210 additions and 18 deletions.
1 change: 1 addition & 0 deletions src/core/packages/ui-settings/common/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ export type {
UiSettingsType,
ReadonlyModeType,
DeprecationSettings,
UiSettingsSolution,
UiSettingsParams,
UserProvidedValues,
UiSettingsScope,
Expand Down
6 changes: 6 additions & 0 deletions src/core/packages/ui-settings/common/src/ui_settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ export interface GetUiSettingsContext {
request?: KibanaRequest;
}

export type UiSettingsSolution = 'es' | 'oblt' | 'security';

/**
* UiSettings parameters defined by the plugins.
* @public
Expand Down Expand Up @@ -116,6 +118,10 @@ export interface UiSettingsParams<T = unknown> {
* scoped to a namespace. The default value is 'namespace'
*/
scope?: UiSettingsScope;
/** The solution where this setting is applicable.
* This field is used to determine whether the setting should be displayed in the Advanced settings app.
* If undefined, the setting must be displayed in all solutions. */
solution?: UiSettingsSolution;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,34 @@ describe('Settings application', () => {
}
});

it("doesn't render settings that are not applicable in the current solution", async () => {
const services: SettingsApplicationServices = createSettingsApplicationServicesMock(
undefined,
'es',
'security'
);

const { findByTestId } = render(wrap(<SettingsApplication />, services));

// The empty state should be rendered since all settings are for es solution and current space solution is security
expect(findByTestId(DATA_TEST_SUBJ_SETTINGS_EMPTY_STATE)).toBeTruthy();
});

it('renders settings that are applicable in the current solution', async () => {
const services: SettingsApplicationServices = createSettingsApplicationServicesMock(
undefined,
'oblt',
'oblt'
);

const { getByTestId } = render(wrap(<SettingsApplication />, services));

// The form should be rendered
for (const category of spaceCategories) {
expect(getByTestId(`${DATA_TEST_SUBJ_SETTINGS_CATEGORY}-${category}`)).toBeInTheDocument();
}
});

describe('Tabs', () => {
const spaceSettingsTestSubj = `${DATA_TEST_SUBJ_PREFIX_TAB}-${SPACE_SETTINGS_TAB_ID}`;
const globalSettingsTestSubj = `${DATA_TEST_SUBJ_PREFIX_TAB}-${GLOBAL_SETTINGS_TAB_ID}`;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,11 @@
* License v3.0 only", or the "Server Side Public License, v 1".
*/

import { useState } from 'react';
import { useState, useEffect } from 'react';
import useEffectOnce from 'react-use/lib/useEffectOnce';

import { UiSettingsScope } from '@kbn/core-ui-settings-common';
import { SolutionView } from '@kbn/spaces-plugin/common';
import { useServices } from '../services';

/**
Expand All @@ -21,13 +22,31 @@ import { useServices } from '../services';
* @returns An array of settings metadata objects.
*/
export const useSettings = (scope: UiSettingsScope) => {
const { getAllowlistedSettings, subscribeToUpdates } = useServices();
const { getAllowlistedSettings, subscribeToUpdates, getActiveSpace, subscribeToActiveSpace } =
useServices();
const [solutionView, setSolutionView] = useState<SolutionView>();

const [settings, setSettings] = useState(getAllowlistedSettings(scope));
useEffectOnce(() => {
const subscription = subscribeToActiveSpace(() => {
getActiveSpace().then((space) => {
setSolutionView(space.solution);
});
});

return () => {
subscription.unsubscribe();
};
});

const [settings, setSettings] = useState(getAllowlistedSettings(scope, solutionView));

useEffect(() => {
setSettings(getAllowlistedSettings(scope, solutionView));
}, [solutionView, scope, getAllowlistedSettings]); // Update settings when solutionView changes

useEffectOnce(() => {
const subscription = subscribeToUpdates(() => {
setSettings(getAllowlistedSettings(scope));
setSettings(getAllowlistedSettings(scope, solutionView));
}, scope);

return () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ export const KibanaSettingsApplication = ({
sectionRegistry,
application,
chrome,
spaces,
}: SettingsApplicationKibanaDependencies) => (
<SettingsApplicationKibanaProvider
{...{
Expand All @@ -46,6 +47,7 @@ export const KibanaSettingsApplication = ({
sectionRegistry,
application,
chrome,
spaces,
}}
>
<SettingsApplication />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ import {
} from '@kbn/management-settings-utilities/mocks/settings.mock';
import { UiSettingsScope } from '@kbn/core-ui-settings-common';
import { getSettingsCapabilitiesMock } from '@kbn/management-settings-utilities/mocks/capabilities.mock';
import { UiSettingsSolution } from '@kbn/core-ui-settings-common';
import { SolutionView } from '@kbn/spaces-plugin/common';
import { SettingsApplicationProvider, SettingsApplicationServices } from '../services';

const createRootMock = () => {
Expand All @@ -42,11 +44,17 @@ const createRootMock = () => {
};

export const createSettingsApplicationServicesMock = (
hasGlobalSettings?: boolean
hasGlobalSettings?: boolean,
settingsSolution?: UiSettingsSolution,
spaceSolution: SolutionView = 'classic'
): SettingsApplicationServices => ({
...createFormServicesMock(),
getAllowlistedSettings: (scope: UiSettingsScope) =>
scope === 'namespace' ? getSettingsMock() : hasGlobalSettings ? getGlobalSettingsMock() : {},
scope === 'namespace'
? getSettingsMock(undefined, undefined, settingsSolution)
: hasGlobalSettings
? getGlobalSettingsMock(undefined, undefined, settingsSolution)
: {},
getSections: () => [],
getCapabilities: getSettingsCapabilitiesMock,
setBadge: jest.fn(),
Expand All @@ -55,6 +63,11 @@ export const createSettingsApplicationServicesMock = (
subscribeToUpdates: () => new Subscription(),
addUrlToHistory: jest.fn(),
getToastsService: jest.fn(),
getActiveSpace: () =>
Promise.resolve({
solution: spaceSolution,
}),
subscribeToActiveSpace: () => new Subscription(),
});

export const TestWrapper = ({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,15 @@ import { UiSettingsScope } from '@kbn/core-ui-settings-common';
import { RegistryEntry, SectionRegistryStart } from '@kbn/management-settings-section-registry';
import { ToastsStart } from '@kbn/core-notifications-browser';
import { ChromeBadge, ChromeStart } from '@kbn/core-chrome-browser';
import type { SpacesPluginStart } from '@kbn/spaces-plugin/public';
import type { Space } from '@kbn/spaces-plugin/common';
import { SolutionView } from '@kbn/spaces-plugin/common';

export interface Services {
getAllowlistedSettings: (scope: UiSettingsScope) => Record<string, UiSettingMetadata>;
getAllowlistedSettings: (
scope: UiSettingsScope,
solution: SolutionView | undefined
) => Record<string, UiSettingMetadata>;
getSections: (scope: UiSettingsScope) => RegistryEntry[];
getToastsService: () => ToastsStart;
getCapabilities: () => SettingsCapabilities;
Expand All @@ -35,6 +41,8 @@ export interface Services {
isCustomSetting: (key: string, scope: UiSettingsScope) => boolean;
isOverriddenSetting: (key: string, scope: UiSettingsScope) => boolean;
addUrlToHistory: (url: string) => void;
getActiveSpace: () => Promise<Pick<Space, 'solution'>>;
subscribeToActiveSpace: (fn: () => void) => Subscription;
}

export type SettingsApplicationServices = Services & FormServices;
Expand All @@ -57,6 +65,7 @@ export interface KibanaDependencies {
};
application: Pick<ApplicationStart, 'capabilities'>;
chrome: Pick<ChromeStart, 'setBadge'>;
spaces: Pick<SpacesPluginStart, 'getActiveSpace' | 'getActiveSpace$'>;
}

export type SettingsApplicationKibanaDependencies = KibanaDependencies & FormKibanaDependencies;
Expand Down Expand Up @@ -87,6 +96,8 @@ export const SettingsApplicationProvider: FC<PropsWithChildren<SettingsApplicati
isCustomSetting,
isOverriddenSetting,
addUrlToHistory,
getActiveSpace,
subscribeToActiveSpace,
} = services;

return (
Expand All @@ -101,6 +112,8 @@ export const SettingsApplicationProvider: FC<PropsWithChildren<SettingsApplicati
isCustomSetting,
isOverriddenSetting,
addUrlToHistory,
getActiveSpace,
subscribeToActiveSpace,
}}
>
<FormProvider
Expand Down Expand Up @@ -129,18 +142,25 @@ export const SettingsApplicationKibanaProvider: FC<
sectionRegistry,
application,
chrome,
spaces,
} = dependencies;
const { client, globalClient } = settings;

const getScopeClient = (scope: UiSettingsScope) => {
return scope === 'namespace' ? client : globalClient;
};

const getAllowlistedSettings = (scope: UiSettingsScope) => {
const getAllowlistedSettings = (scope: UiSettingsScope, solution: SolutionView | undefined) => {
const scopeClient = getScopeClient(scope);
const rawSettings = Object.fromEntries(
Object.entries(scopeClient.getAll()).filter(
([settingId, settingDef]) => !settingDef.readonly && !client.isCustom(settingId)
([settingId, settingDef]) =>
!settingDef.readonly &&
!client.isCustom(settingId) &&
(!solution ||
solution === 'classic' ||
!settingDef.solution ||
settingDef.solution === solution)
)
);
return normalizeSettings(rawSettings);
Expand Down Expand Up @@ -191,6 +211,10 @@ export const SettingsApplicationKibanaProvider: FC<
isOverriddenSetting,
subscribeToUpdates,
addUrlToHistory: (url: string) => history.push({ pathname: '', search: url }),
getActiveSpace: spaces.getActiveSpace,
subscribeToActiveSpace: (fn: () => void) => {
return spaces.getActiveSpace$().subscribe(fn);
},
};

return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,5 +36,6 @@
"@kbn/core-notifications-browser",
"@kbn/core-chrome-browser",
"@kbn/core-user-profile-browser-mocks",
"@kbn/spaces-plugin",
]
}
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ export const getFieldDefinition = <T extends SettingType>(
type,
userValue: savedValue,
value: defaultValue,
solution,
} = setting;

const { isCustom, isOverridden } = params;
Expand Down Expand Up @@ -144,6 +145,7 @@ export const getFieldDefinition = <T extends SettingType>(
savedValue,
type,
unsavedFieldId: `${id}-unsaved`,
solution,
};

// TODO: clintandrewhall - add validation (e.g. `select` contains non-empty `options`)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
import { ReactElement } from 'react';

import { UiCounterMetricType } from '@kbn/analytics';
import { DeprecationSettings } from '@kbn/core-ui-settings-common';
import { DeprecationSettings, UiSettingsSolution } from '@kbn/core-ui-settings-common';

import { KnownTypeToValue, SettingType } from './setting_type';

Expand Down Expand Up @@ -88,6 +88,11 @@ export interface FieldDefinition<
type: T;
/** An identifier of the field when it has an unsaved change. */
unsavedFieldId: string;
/** The solution where this setting is applicable.
* If undefined, the setting must be displayed in all solutions.
* @see {@link UiSettingsSolution}
*/
solution?: UiSettingsSolution;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
*/

import { KnownTypeToMetadata, SettingType } from '@kbn/management-settings-types';
import { UiSettingsSolution } from '@kbn/core-ui-settings-common';

type Settings = {
[key in Exclude<SettingType, 'json' | 'markdown'>]: KnownTypeToMetadata<key>;
Expand All @@ -20,11 +21,13 @@ type Settings = {
*/
export const getSettingsMock = (
requiresPageReload: boolean = false,
readonly: boolean = false
readonly: boolean = false,
solution?: UiSettingsSolution
): Settings => {
const defaults = {
requiresPageReload,
readonly,
solution,
};

return {
Expand Down Expand Up @@ -135,11 +138,13 @@ export const getSettingsMock = (
*/
export const getGlobalSettingsMock = (
requiresPageReload: boolean = false,
readonly: boolean = false
readonly: boolean = false,
solution?: UiSettingsSolution
) => {
const defaults = {
requiresPageReload,
readonly,
solution,
};
return {
globalString: {
Expand Down
3 changes: 2 additions & 1 deletion src/platform/plugins/private/advanced_settings/kibana.jsonc
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@
],
"optionalPlugins": [
"home",
"usageCollection"
"usageCollection",
"spaces",
],
"requiredBundles": []
}
Expand Down
Loading

0 comments on commit 7a729c7

Please sign in to comment.