Skip to content

Commit

Permalink
Add configurable defaults to uiSettings (opensearch-project#4344)
Browse files Browse the repository at this point in the history
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 <miki@amazon.com>
  • Loading branch information
AMoo-Miki committed Jun 23, 2023
1 parent 931b915 commit fe1f1d1
Show file tree
Hide file tree
Showing 12 changed files with 250 additions and 8 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down

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

29 changes: 29 additions & 0 deletions src/core/server/rendering/rendering_service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,15 +106,44 @@ 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') || '');

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,
Expand Down
8 changes: 5 additions & 3 deletions src/core/server/rendering/rendering_service.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
6 changes: 5 additions & 1 deletion src/core/server/ui_settings/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,13 @@ export {
*/
export interface IUiSettingsClient {
/**
* Returns registered uiSettings values {@link UiSettingsParams}
* Returns all registered uiSettings values {@link UiSettingsParams}
*/
getRegistered: () => Readonly<Record<string, PublicUiSettingsParams>>;
/**
* 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.
*/
Expand Down
19 changes: 19 additions & 0 deletions src/core/server/ui_settings/ui_settings_client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
4 changes: 4 additions & 0 deletions src/core/server/ui_settings/ui_settings_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<T = any>(key: string): Promise<T> {
const all = await this.getAll();
return all[key];
Expand Down
20 changes: 20 additions & 0 deletions src/core/server/ui_settings/ui_settings_config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<typeof configSchema>;
Expand Down
1 change: 1 addition & 0 deletions src/core/server/ui_settings/ui_settings_service.mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import { UiSettingsService } from './ui_settings_service';
const createClientMock = () => {
const mocked: jest.Mocked<IUiSettingsClient> = {
getRegistered: jest.fn(),
getOverrideOrDefault: jest.fn(),
get: jest.fn(),
getAll: jest.fn(),
getUserProvided: jest.fn(),
Expand Down
42 changes: 41 additions & 1 deletion src/core/server/ui_settings/ui_settings_service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ const overrides = {
overrideBaz: 'baz',
};

const defaults = {
const defaults: Record<string, any> = {
foo: {
name: 'foo',
value: 'bar',
Expand Down Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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<string, any> = {};
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'
);
});
});
});
});
18 changes: 17 additions & 1 deletion src/core/server/ui_settings/ui_settings_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -134,4 +137,17 @@ export class UiSettingsService
}
}
}

private validateAndUpdateConfiguredDefaults(defaults: Record<string, any> = {}) {
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;
}
}
}
2 changes: 1 addition & 1 deletion src/legacy/ui/ui_render/ui_render_mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down

0 comments on commit fe1f1d1

Please sign in to comment.