From fe1f1d18ea9b26ddb19ae805f676b6e09798b294 Mon Sep 17 00:00:00 2001 From: Miki Date: Fri, 23 Jun 2023 13:20:36 -0700 Subject: [PATCH] Add configurable `defaults` to `uiSettings` (#4344) Also now: * `theme:darkMode` and `theme:version` can be configured via `defaults` * unauthenticated users are no longer forced to light mode Signed-off-by: Miki --- CHANGELOG.md | 1 + .../rendering_service.test.ts.snap | 108 +++++++++++++++++- .../rendering/rendering_service.test.ts | 29 +++++ .../server/rendering/rendering_service.tsx | 8 +- src/core/server/ui_settings/types.ts | 6 +- .../ui_settings/ui_settings_client.test.ts | 19 +++ .../server/ui_settings/ui_settings_client.ts | 4 + .../server/ui_settings/ui_settings_config.ts | 20 ++++ .../ui_settings/ui_settings_service.mock.ts | 1 + .../ui_settings/ui_settings_service.test.ts | 42 ++++++- .../server/ui_settings/ui_settings_service.ts | 18 ++- src/legacy/ui/ui_render/ui_render_mixin.js | 2 +- 12 files changed, 250 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 91481d744d7..3e17268e160 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - [Saved Object Service] Add Repository Factory Provider ([#4149](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4149)) - [@osd/pm] Fix `file:`-linked dependencies' resolution to improve ability to test with local packages ([#4342](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4342)) - [Multiple DataSource] Backend support for adding sample data ([#4268](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4268)) +- Add configurable defaults and overrides to uiSettings ([#4344](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4344)) ### 🐛 Bug Fixes diff --git a/src/core/server/rendering/__snapshots__/rendering_service.test.ts.snap b/src/core/server/rendering/__snapshots__/rendering_service.test.ts.snap index 1913a972ab8..ad92d759a83 100644 --- a/src/core/server/rendering/__snapshots__/rendering_service.test.ts.snap +++ b/src/core/server/rendering/__snapshots__/rendering_service.test.ts.snap @@ -106,7 +106,7 @@ Object { } `; -exports[`RenderingService setup() render() renders "core" page driven by settings 1`] = ` +exports[`RenderingService setup() render() renders "core" page driven by defaults 1`] = ` Object { "anonymousStatusPage": false, "basePath": "/mock-server-basepath", @@ -148,6 +148,59 @@ Object { "name": "title", }, }, + "user": Object {}, + }, + }, + "serverBasePath": "/mock-server-basepath", + "survey": "https://survey.opensearch.org", + "uiPlugins": Array [], + "vars": Object {}, + "version": Any, +} +`; + +exports[`RenderingService setup() render() renders "core" page driven by settings 1`] = ` +Object { + "anonymousStatusPage": false, + "basePath": "/mock-server-basepath", + "branch": Any, + "branding": Object { + "applicationTitle": "OpenSearch Dashboards", + "assetFolderUrl": "/mock-server-basepath/ui/default_branding", + "darkMode": true, + "loadingLogo": Object {}, + "logo": Object {}, + "mark": Object {}, + "useExpandedHeader": true, + }, + "buildNumber": Any, + "csp": Object { + "warnLegacyBrowsers": true, + }, + "env": Object { + "mode": Object { + "dev": Any, + "name": Any, + "prod": Any, + }, + "packageInfo": Object { + "branch": Any, + "buildNum": Any, + "buildSha": Any, + "dist": Any, + "version": Any, + }, + }, + "i18n": Object { + "translationsUrl": "/mock-server-basepath/translations/en.json", + }, + "legacyMetadata": Object { + "uiSettings": Object { + "defaults": Object { + "theme:darkMode": Object { + "value": false, + }, + }, "user": Object { "theme:darkMode": Object { "userValue": true, @@ -216,6 +269,59 @@ Object { } `; +exports[`RenderingService setup() render() renders "core" page with no defaults or overrides 1`] = ` +Object { + "anonymousStatusPage": false, + "basePath": "/mock-server-basepath", + "branch": Any, + "branding": Object { + "applicationTitle": "OpenSearch Dashboards", + "assetFolderUrl": "/mock-server-basepath/ui/default_branding", + "darkMode": false, + "loadingLogo": Object {}, + "logo": Object {}, + "mark": Object {}, + "useExpandedHeader": true, + }, + "buildNumber": Any, + "csp": Object { + "warnLegacyBrowsers": true, + }, + "env": Object { + "mode": Object { + "dev": Any, + "name": Any, + "prod": Any, + }, + "packageInfo": Object { + "branch": Any, + "buildNum": Any, + "buildSha": Any, + "dist": Any, + "version": Any, + }, + }, + "i18n": Object { + "translationsUrl": "/mock-server-basepath/translations/en.json", + }, + "legacyMetadata": Object { + "uiSettings": Object { + "defaults": Object { + "registered": Object { + "name": "title", + }, + }, + "user": Object {}, + }, + }, + "serverBasePath": "/mock-server-basepath", + "survey": "https://survey.opensearch.org", + "uiPlugins": Array [], + "vars": Object {}, + "version": Any, +} +`; + exports[`RenderingService setup() render() renders "core" with excluded user settings 1`] = ` Object { "anonymousStatusPage": false, diff --git a/src/core/server/rendering/rendering_service.test.ts b/src/core/server/rendering/rendering_service.test.ts index 725a02d75d5..56a39915e73 100644 --- a/src/core/server/rendering/rendering_service.test.ts +++ b/src/core/server/rendering/rendering_service.test.ts @@ -106,8 +106,22 @@ describe('RenderingService', () => { expect(data).toMatchSnapshot(INJECTED_METADATA); }); + it('renders "core" page driven by defaults', async () => { + uiSettings.getUserProvided.mockResolvedValue({ 'theme:darkMode': { userValue: false } }); + uiSettings.getOverrideOrDefault.mockImplementation((name) => name === 'theme:darkMode'); + const content = await render(createOpenSearchDashboardsRequest(), uiSettings, { + includeUserSettings: false, + }); + const dom = load(content); + const data = JSON.parse(dom('osd-injected-metadata').attr('data') || ''); + + expect(uiSettings.getUserProvided).not.toHaveBeenCalled(); + expect(data).toMatchSnapshot(INJECTED_METADATA); + }); + it('renders "core" page driven by settings', async () => { uiSettings.getUserProvided.mockResolvedValue({ 'theme:darkMode': { userValue: true } }); + uiSettings.getRegistered.mockReturnValue({ 'theme:darkMode': { value: false } }); const content = await render(createOpenSearchDashboardsRequest(), uiSettings); const dom = load(content); const data = JSON.parse(dom('osd-injected-metadata').attr('data') || ''); @@ -115,6 +129,21 @@ describe('RenderingService', () => { expect(data).toMatchSnapshot(INJECTED_METADATA); }); + it('renders "core" page with no defaults or overrides', async () => { + uiSettings.getUserProvided.mockResolvedValue({}); + uiSettings.getOverrideOrDefault.mockImplementation((name) => + name === 'theme:darkMode' ? undefined : false + ); + const content = await render(createOpenSearchDashboardsRequest(), uiSettings, { + includeUserSettings: false, + }); + const dom = load(content); + const data = JSON.parse(dom('osd-injected-metadata').attr('data') || ''); + + expect(uiSettings.getUserProvided).not.toHaveBeenCalled(); + expect(data).toMatchSnapshot(INJECTED_METADATA); + }); + it('renders "core" with excluded user settings', async () => { const content = await render(createOpenSearchDashboardsRequest(), uiSettings, { includeUserSettings: false, diff --git a/src/core/server/rendering/rendering_service.tsx b/src/core/server/rendering/rendering_service.tsx index 16430739bd0..c62ee08db6b 100644 --- a/src/core/server/rendering/rendering_service.tsx +++ b/src/core/server/rendering/rendering_service.tsx @@ -95,9 +95,11 @@ export class RenderingService { defaults: uiSettings.getRegistered(), user: includeUserSettings ? await uiSettings.getUserProvided() : {}, }; - const darkMode = settings.user?.['theme:darkMode']?.userValue - ? Boolean(settings.user['theme:darkMode'].userValue) - : false; + // Cannot use `uiSettings.get()` since a user might not be authenticated + const darkMode = + (settings.user?.['theme:darkMode']?.userValue ?? + uiSettings.getOverrideOrDefault('theme:darkMode')) || + false; const brandingAssignment = await this.assignBrandingConfig( darkMode, diff --git a/src/core/server/ui_settings/types.ts b/src/core/server/ui_settings/types.ts index 9086fb37697..60510882755 100644 --- a/src/core/server/ui_settings/types.ts +++ b/src/core/server/ui_settings/types.ts @@ -52,9 +52,13 @@ export { */ export interface IUiSettingsClient { /** - * Returns registered uiSettings values {@link UiSettingsParams} + * Returns all registered uiSettings values {@link UiSettingsParams} */ getRegistered: () => Readonly>; + /** + * Returns the overridden uiSettings value if one exists, or the registered default if one exists {@link UiSettingsParams} + */ + getOverrideOrDefault: (key: string) => unknown; /** * Retrieves uiSettings values set by the user with fallbacks to default values if not specified. */ diff --git a/src/core/server/ui_settings/ui_settings_client.test.ts b/src/core/server/ui_settings/ui_settings_client.test.ts index 9108467fd04..f40b7a17358 100644 --- a/src/core/server/ui_settings/ui_settings_client.test.ts +++ b/src/core/server/ui_settings/ui_settings_client.test.ts @@ -333,6 +333,25 @@ describe('ui settings', () => { }); }); + describe('#getOverrideOrDefault()', () => { + it('returns the non-overridden default settings passed within the constructor', () => { + const value = chance.word(); + const defaults = { key: { value } }; + const { uiSettings } = setup({ defaults }); + expect(uiSettings.getOverrideOrDefault('key')).toEqual(value); + expect(uiSettings.getOverrideOrDefault('unknown')).toBeUndefined(); + }); + + it('returns the overridden settings passed within the constructor', () => { + const value = chance.word(); + const override = chance.word(); + const defaults = { key: { value } }; + const overrides = { key: { value: override } }; + const { uiSettings } = setup({ defaults, overrides }); + expect(uiSettings.getOverrideOrDefault('key')).toEqual(override); + }); + }); + describe('#getUserProvided()', () => { it('pulls user configuration from OpenSearch', async () => { const { uiSettings, savedObjectsClient } = setup(); diff --git a/src/core/server/ui_settings/ui_settings_client.ts b/src/core/server/ui_settings/ui_settings_client.ts index e7e5030035b..066efa34caf 100644 --- a/src/core/server/ui_settings/ui_settings_client.ts +++ b/src/core/server/ui_settings/ui_settings_client.ts @@ -91,6 +91,10 @@ export class UiSettingsClient implements IUiSettingsClient { return copiedDefaults; } + getOverrideOrDefault(key: string): unknown { + return this.isOverridden(key) ? this.overrides[key].value : this.defaults[key]?.value; + } + async get(key: string): Promise { const all = await this.getAll(); return all[key]; diff --git a/src/core/server/ui_settings/ui_settings_config.ts b/src/core/server/ui_settings/ui_settings_config.ts index 634fb5c62d5..19e79008845 100644 --- a/src/core/server/ui_settings/ui_settings_config.ts +++ b/src/core/server/ui_settings/ui_settings_config.ts @@ -37,13 +37,33 @@ const deprecations: ConfigDeprecationProvider = ({ unused, renameFromRoot }) => renameFromRoot('server.defaultRoute', 'uiSettings.overrides.defaultRoute'), ]; +/* There are 4 levels of uiSettings: + * 1) defaults hardcoded in code + * 2) defaults provided in the opensearch_dashboards.yml + * 3) values selected by the user and received from savedObjects + * 4) overrides provided in the opensearch_dashboards.yml + * + * Each of these levels override the one above them. + * + * The schema below exposes only a limited set of settings to be set in the config file. + * + * ToDo: Remove overrides; these were added to force the lock down the theme version. + * The schema is temporarily relaxed to allow overriding the `darkMode` and setting + * `defaults`. An upcoming change would relax them further to allow setting them. + */ + const configSchema = schema.object({ overrides: schema.object( { + 'theme:darkMode': schema.maybe(schema.boolean({ defaultValue: true })), 'theme:version': schema.string({ defaultValue: 'v7' }), }, { unknowns: 'allow' } ), + defaults: schema.object({ + 'theme:darkMode': schema.maybe(schema.boolean({ defaultValue: false })), + 'theme:version': schema.maybe(schema.string({ defaultValue: 'v7' })), + }), }); export type UiSettingsConfigType = TypeOf; diff --git a/src/core/server/ui_settings/ui_settings_service.mock.ts b/src/core/server/ui_settings/ui_settings_service.mock.ts index ac52042bb71..bb6e9ec64bd 100644 --- a/src/core/server/ui_settings/ui_settings_service.mock.ts +++ b/src/core/server/ui_settings/ui_settings_service.mock.ts @@ -39,6 +39,7 @@ import { UiSettingsService } from './ui_settings_service'; const createClientMock = () => { const mocked: jest.Mocked = { getRegistered: jest.fn(), + getOverrideOrDefault: jest.fn(), get: jest.fn(), getAll: jest.fn(), getUserProvided: jest.fn(), diff --git a/src/core/server/ui_settings/ui_settings_service.test.ts b/src/core/server/ui_settings/ui_settings_service.test.ts index e32429de7ab..f9316d30695 100644 --- a/src/core/server/ui_settings/ui_settings_service.test.ts +++ b/src/core/server/ui_settings/ui_settings_service.test.ts @@ -46,7 +46,7 @@ const overrides = { overrideBaz: 'baz', }; -const defaults = { +const defaults: Record = { foo: { name: 'foo', value: 'bar', @@ -97,6 +97,21 @@ describe('uiSettings', () => { ); }); }); + + it('fails if configured default was not previously defined', async () => { + const coreContext = mockCoreContext.create(); + coreContext.configService.atPath.mockReturnValueOnce( + new BehaviorSubject({ + defaults: { + foo: 'configured', + }, + }) + ); + const customizedService = new UiSettingsService(coreContext); + await expect(customizedService.setup(setupDeps)).rejects.toMatchInlineSnapshot( + `[Error: [ui settings defaults [foo]: expected key to be have been registered]` + ); + }); }); describe('#start', () => { @@ -168,6 +183,31 @@ describe('uiSettings', () => { expect(MockUiSettingsClientConstructor.mock.calls[0][0].defaults).toEqual(defaults); expect(MockUiSettingsClientConstructor.mock.calls[0][0].defaults).not.toBe(defaults); }); + + it('passes configured defaults to UiSettingsClient', async () => { + const defaultsClone: Record = {}; + Object.keys(defaults).forEach((key: string) => { + defaultsClone[key] = { ...defaults[key] }; + }); + + getCoreSettingsMock.mockReturnValue(defaultsClone); + const coreContext = mockCoreContext.create(); + coreContext.configService.atPath.mockReturnValueOnce( + new BehaviorSubject({ + defaults: { + foo: 'configured', + }, + }) + ); + const customizedService = new UiSettingsService(coreContext); + await customizedService.setup(setupDeps); + const start = await customizedService.start(); + start.asScopedToClient(savedObjectsClient); + expect(MockUiSettingsClientConstructor).toBeCalledTimes(1); + expect(MockUiSettingsClientConstructor.mock.calls[0][0].defaults?.foo?.value).toEqual( + 'configured' + ); + }); }); }); }); diff --git a/src/core/server/ui_settings/ui_settings_service.ts b/src/core/server/ui_settings/ui_settings_service.ts index 5543485b25e..d20b1e964e5 100644 --- a/src/core/server/ui_settings/ui_settings_service.ts +++ b/src/core/server/ui_settings/ui_settings_service.ts @@ -75,7 +75,10 @@ export class UiSettingsService this.register(getCoreSettings()); const config = await this.config$.pipe(first()).toPromise(); - this.overrides = config.overrides; + this.overrides = config.overrides || {}; + + // Use uiSettings.defaults from the config file + this.validateAndUpdateConfiguredDefaults(config.defaults); return { register: this.register.bind(this), @@ -134,4 +137,17 @@ export class UiSettingsService } } } + + private validateAndUpdateConfiguredDefaults(defaults: Record = {}) { + for (const [key, value] of Object.entries(defaults)) { + const definition = this.uiSettingsDefaults.get(key); + if (!definition) + throw new Error(`[ui settings defaults [${key}]: expected key to be have been registered`); + + if (definition.schema) { + definition.schema.validate(value, {}, `ui settings configuration [${key}]`); + } + definition.value = value; + } + } } diff --git a/src/legacy/ui/ui_render/ui_render_mixin.js b/src/legacy/ui/ui_render/ui_render_mixin.js index 673d3483e78..e6591ad72d9 100644 --- a/src/legacy/ui/ui_render/ui_render_mixin.js +++ b/src/legacy/ui/ui_render/ui_render_mixin.js @@ -95,7 +95,7 @@ export function uiRenderMixin(osdServer, server, config) { const darkMode = !authEnabled || request.auth.isAuthenticated ? await uiSettings.get('theme:darkMode') - : false; + : uiSettings.getOverrideOrDefault('theme:darkMode'); const themeVersion = !authEnabled || request.auth.isAuthenticated ? await uiSettings.get('theme:version') : 'v7';