Skip to content

Commit 850a965

Browse files
committed
Fix linked deps resolution
Signed-off-by: Miki <miki@amazon.com>
1 parent b337bea commit 850a965

12 files changed

+182
-8
lines changed

CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
2222
- Add category option within groups for context menus ([#4144](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4144))
2323
- [Saved Object Service] Add Repository Factory Provider ([#4149](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4149))
2424
- [Multiple DataSource] Backend support for adding sample data ([#4268](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4268))
25+
- Add configurable defaults and overrides to uiSettings ([#4344](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4344))
2526

2627
### 🐛 Bug Fixes
2728

src/core/server/rendering/__snapshots__/rendering_service.test.ts.snap

+54-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/core/server/rendering/rendering_service.test.ts

+14
Original file line numberDiff line numberDiff line change
@@ -106,8 +106,22 @@ describe('RenderingService', () => {
106106
expect(data).toMatchSnapshot(INJECTED_METADATA);
107107
});
108108

109+
it('renders "core" page driven by defaults', async () => {
110+
uiSettings.getUserProvided.mockResolvedValue({ 'theme:darkMode': { userValue: false } });
111+
uiSettings.getOverrideOrDefault.mockImplementation((name) => name === 'theme:darkMode');
112+
const content = await render(createOpenSearchDashboardsRequest(), uiSettings, {
113+
includeUserSettings: false,
114+
});
115+
const dom = load(content);
116+
const data = JSON.parse(dom('osd-injected-metadata').attr('data') || '');
117+
118+
expect(uiSettings.getUserProvided).not.toHaveBeenCalled();
119+
expect(data).toMatchSnapshot(INJECTED_METADATA);
120+
});
121+
109122
it('renders "core" page driven by settings', async () => {
110123
uiSettings.getUserProvided.mockResolvedValue({ 'theme:darkMode': { userValue: true } });
124+
uiSettings.getRegistered.mockReturnValue({ 'theme:darkMode': { value: false } });
111125
const content = await render(createOpenSearchDashboardsRequest(), uiSettings);
112126
const dom = load(content);
113127
const data = JSON.parse(dom('osd-injected-metadata').attr('data') || '');

src/core/server/rendering/rendering_service.tsx

+5-3
Original file line numberDiff line numberDiff line change
@@ -95,9 +95,11 @@ export class RenderingService {
9595
defaults: uiSettings.getRegistered(),
9696
user: includeUserSettings ? await uiSettings.getUserProvided() : {},
9797
};
98-
const darkMode = settings.user?.['theme:darkMode']?.userValue
99-
? Boolean(settings.user['theme:darkMode'].userValue)
100-
: false;
98+
// Cannot use `uiSettings.get()` since a user might not be authenticated
99+
const darkMode =
100+
(settings.user?.['theme:darkMode']?.userValue ??
101+
uiSettings.getOverrideOrDefault('theme:darkMode')) ||
102+
false;
101103

102104
const brandingAssignment = await this.assignBrandingConfig(
103105
darkMode,

src/core/server/ui_settings/types.ts

+5-1
Original file line numberDiff line numberDiff line change
@@ -52,9 +52,13 @@ export {
5252
*/
5353
export interface IUiSettingsClient {
5454
/**
55-
* Returns registered uiSettings values {@link UiSettingsParams}
55+
* Returns all registered uiSettings values {@link UiSettingsParams}
5656
*/
5757
getRegistered: () => Readonly<Record<string, PublicUiSettingsParams>>;
58+
/**
59+
* Returns the overridden uiSettings value if one exists, or the registered default {@link UiSettingsParams}
60+
*/
61+
getOverrideOrDefault: (key: string) => unknown;
5862
/**
5963
* Retrieves uiSettings values set by the user with fallbacks to default values if not specified.
6064
*/

src/core/server/ui_settings/ui_settings_client.test.ts

+19
Original file line numberDiff line numberDiff line change
@@ -333,6 +333,25 @@ describe('ui settings', () => {
333333
});
334334
});
335335

336+
describe('#getOverrideOrDefault()', () => {
337+
it('returns the non-overridden default settings passed within the constructor', () => {
338+
const value = chance.word();
339+
const defaults = { key: { value } };
340+
const { uiSettings } = setup({ defaults });
341+
expect(uiSettings.getOverrideOrDefault('key')).toEqual(value);
342+
expect(uiSettings.getOverrideOrDefault('unknown')).toBeUndefined();
343+
});
344+
345+
it('returns the overridden settings passed within the constructor', () => {
346+
const value = chance.word();
347+
const override = chance.word();
348+
const defaults = { key: { value } };
349+
const overrides = { key: { value: override } };
350+
const { uiSettings } = setup({ defaults, overrides });
351+
expect(uiSettings.getOverrideOrDefault('key')).toEqual(override);
352+
});
353+
});
354+
336355
describe('#getUserProvided()', () => {
337356
it('pulls user configuration from OpenSearch', async () => {
338357
const { uiSettings, savedObjectsClient } = setup();

src/core/server/ui_settings/ui_settings_client.ts

+4
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,10 @@ export class UiSettingsClient implements IUiSettingsClient {
9191
return copiedDefaults;
9292
}
9393

94+
getOverrideOrDefault(key: string): unknown {
95+
return this.isOverridden(key) ? this.overrides[key].value : this.defaults[key]?.value;
96+
}
97+
9498
async get<T = any>(key: string): Promise<T> {
9599
const all = await this.getAll();
96100
return all[key];

src/core/server/ui_settings/ui_settings_config.ts

+20
Original file line numberDiff line numberDiff line change
@@ -37,13 +37,33 @@ const deprecations: ConfigDeprecationProvider = ({ unused, renameFromRoot }) =>
3737
renameFromRoot('server.defaultRoute', 'uiSettings.overrides.defaultRoute'),
3838
];
3939

40+
/* There are 4 levels of uiSettings:
41+
* 1) defaults hardcoded in code
42+
* 2) defaults provided in the opensearch_dashboards.yml
43+
* 3) values selected by the user and received from savedObjects
44+
* 4) overrides provided in the opensearch_dashboards.yml
45+
*
46+
* Each of these levels override the one above them.
47+
*
48+
* The schema below exposes only a limited set of settings to be set in the config file.
49+
*
50+
* ToDo: Remove overrides; these were added to force the lock down the theme version.
51+
* The schema is temporarily relaxed to allow overriding the `darkMode` and setting
52+
* `defaults`. An upcoming change would relax them further to allow setting them.
53+
*/
54+
4055
const configSchema = schema.object({
4156
overrides: schema.object(
4257
{
58+
'theme:darkMode': schema.maybe(schema.boolean({ defaultValue: true })),
4359
'theme:version': schema.string({ defaultValue: 'v7' }),
4460
},
4561
{ unknowns: 'allow' }
4662
),
63+
defaults: schema.object({
64+
'theme:darkMode': schema.maybe(schema.boolean({ defaultValue: false })),
65+
'theme:version': schema.maybe(schema.string({ defaultValue: 'v7' })),
66+
}),
4767
});
4868

4969
export type UiSettingsConfigType = TypeOf<typeof configSchema>;

src/core/server/ui_settings/ui_settings_service.mock.ts

+1
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ import { UiSettingsService } from './ui_settings_service';
3939
const createClientMock = () => {
4040
const mocked: jest.Mocked<IUiSettingsClient> = {
4141
getRegistered: jest.fn(),
42+
getOverrideOrDefault: jest.fn(),
4243
get: jest.fn(),
4344
getAll: jest.fn(),
4445
getUserProvided: jest.fn(),

src/core/server/ui_settings/ui_settings_service.test.ts

+41-1
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ const overrides = {
4646
overrideBaz: 'baz',
4747
};
4848

49-
const defaults = {
49+
const defaults: Record<string, any> = {
5050
foo: {
5151
name: 'foo',
5252
value: 'bar',
@@ -97,6 +97,21 @@ describe('uiSettings', () => {
9797
);
9898
});
9999
});
100+
101+
it('fails if configured default was not previously defined', async () => {
102+
const coreContext = mockCoreContext.create();
103+
coreContext.configService.atPath.mockReturnValueOnce(
104+
new BehaviorSubject({
105+
defaults: {
106+
foo: 'configured',
107+
},
108+
})
109+
);
110+
const customizedService = new UiSettingsService(coreContext);
111+
await expect(customizedService.setup(setupDeps)).rejects.toMatchInlineSnapshot(
112+
`[Error: [ui settings defaults [foo]: expected key to be have been registered]`
113+
);
114+
});
100115
});
101116

102117
describe('#start', () => {
@@ -168,6 +183,31 @@ describe('uiSettings', () => {
168183
expect(MockUiSettingsClientConstructor.mock.calls[0][0].defaults).toEqual(defaults);
169184
expect(MockUiSettingsClientConstructor.mock.calls[0][0].defaults).not.toBe(defaults);
170185
});
186+
187+
it('passes configured defaults to UiSettingsClient', async () => {
188+
const defaultsClone: Record<string, any> = {};
189+
Object.keys(defaults).forEach((key: string) => {
190+
defaultsClone[key] = { ...defaults[key] };
191+
});
192+
193+
getCoreSettingsMock.mockReturnValue(defaultsClone);
194+
const coreContext = mockCoreContext.create();
195+
coreContext.configService.atPath.mockReturnValueOnce(
196+
new BehaviorSubject({
197+
defaults: {
198+
foo: 'configured',
199+
},
200+
})
201+
);
202+
const customizedService = new UiSettingsService(coreContext);
203+
await customizedService.setup(setupDeps);
204+
const start = await customizedService.start();
205+
start.asScopedToClient(savedObjectsClient);
206+
expect(MockUiSettingsClientConstructor).toBeCalledTimes(1);
207+
expect(MockUiSettingsClientConstructor.mock.calls[0][0].defaults?.foo?.value).toEqual(
208+
'configured'
209+
);
210+
});
171211
});
172212
});
173213
});

src/core/server/ui_settings/ui_settings_service.ts

+17-1
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,10 @@ export class UiSettingsService
7575
this.register(getCoreSettings());
7676

7777
const config = await this.config$.pipe(first()).toPromise();
78-
this.overrides = config.overrides;
78+
this.overrides = config.overrides || {};
79+
80+
// Use uiSettings.defaults from the config file
81+
this.validateAndUpdateConfiguredDefaults(config.defaults);
7982

8083
return {
8184
register: this.register.bind(this),
@@ -134,4 +137,17 @@ export class UiSettingsService
134137
}
135138
}
136139
}
140+
141+
private validateAndUpdateConfiguredDefaults(defaults: Record<string, any> = {}) {
142+
for (const [key, value] of Object.entries(defaults)) {
143+
const definition = this.uiSettingsDefaults.get(key);
144+
if (!definition)
145+
throw new Error(`[ui settings defaults [${key}]: expected key to be have been registered`);
146+
147+
if (definition.schema) {
148+
definition.schema.validate(value, {}, `ui settings configuration [${key}]`);
149+
}
150+
definition.value = value;
151+
}
152+
}
137153
}

src/legacy/ui/ui_render/ui_render_mixin.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ export function uiRenderMixin(osdServer, server, config) {
9595
const darkMode =
9696
!authEnabled || request.auth.isAuthenticated
9797
? await uiSettings.get('theme:darkMode')
98-
: false;
98+
: uiSettings.getOverrideOrDefault('theme:darkMode');
9999

100100
const themeVersion =
101101
!authEnabled || request.auth.isAuthenticated ? await uiSettings.get('theme:version') : 'v7';

0 commit comments

Comments
 (0)