Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Rebase #6730 and fix tests] Fix/UI settings overrides #7776

Merged
merged 2 commits into from
Aug 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/core/server/ui_settings/settings/theme.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ export const getThemeSettings = (): Record<string, UiSettingsParams> => {
name: i18n.translate('core.ui_settings.params.enableUserControlTitle', {
defaultMessage: 'Enable user control',
}),
value: true,
value: false,
description: i18n.translate('core.ui_settings.params.enableUserControlText', {
defaultMessage: `Enable users to control theming and dark or light mode via "Appearance" control in top navigation. When true, those settings can no longer be set globally by administrators.`,
}),
Expand Down
2 changes: 1 addition & 1 deletion src/core/server/ui_settings/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ export interface IUiSettingsClient {
/**
* Returns the overridden uiSettings value if one exists, or the registered default if one exists {@link UiSettingsParams}
*/
getOverrideOrDefault: (key: string) => unknown;
getOverrideOrDefault: <T = unknown>(key: string) => T;
/**
* Returns the registered default if one exists {@link UiSettingsParams}
*/
Expand Down
2 changes: 1 addition & 1 deletion src/core/server/ui_settings/ui_settings_client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@ describe('ui settings', () => {
const value = chance.word();
const override = chance.word();
const defaults = { key: { value } };
const overrides = { key: { value: override } };
const overrides = { key: override };
const { uiSettings } = setup({ defaults, overrides });
expect(uiSettings.getOverrideOrDefault('key')).toEqual(override);
});
Expand Down
5 changes: 3 additions & 2 deletions src/core/server/ui_settings/ui_settings_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,9 @@ export class UiSettingsClient implements IUiSettingsClient {
return copiedDefaults;
}

getOverrideOrDefault(key: string): unknown {
return this.isOverridden(key) ? this.overrides[key].value : this.defaults[key]?.value;
getOverrideOrDefault<T = unknown>(key: string): T {
// Note: this.overrides is an object with simple values, whereas this.defaults contains UiSettingsParams as the value for each key.
return this.isOverridden(key) ? this.overrides[key] : this.defaults[key]?.value;
}

getDefault(key: string): unknown {
Expand Down
6 changes: 5 additions & 1 deletion src/core/server/ui_settings/ui_settings_config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,11 @@ const configSchema = schema.object({
overrides: schema.object({}, { unknowns: 'allow' }),
defaults: schema.object({
'theme:darkMode': schema.maybe(schema.boolean({ defaultValue: false })),
'theme:version': schema.maybe(schema.string({ defaultValue: DEFAULT_THEME_VERSION })),
'theme:version': schema.maybe(
schema.oneOf([schema.literal('v7'), schema.literal('Next (preview)')], {
defaultValue: 'Next (preview)',
})
),
}),
});

Expand Down
9 changes: 5 additions & 4 deletions src/legacy/ui/ui_render/ui_render_mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -205,15 +205,16 @@ export function uiRenderMixin(osdServer, server, config) {
);
const uiSettings = osdServer.newPlatform.start.core.uiSettings.asScopedToClient(soClient);

// coerce to booleans just in case, to make sure template vars render correctly
const configEnableUserControl =
!authEnabled || request.auth.isAuthenticated
? await uiSettings.get('theme:enableUserControl')
: uiSettings.getOverrideOrDefault('theme:enableUserControl');
? !!(await uiSettings.get('theme:enableUserControl'))
: !!uiSettings.getOverrideOrDefault('theme:enableUserControl');

const configDarkMode =
!authEnabled || request.auth.isAuthenticated
? await uiSettings.get('theme:darkMode')
: uiSettings.getOverrideOrDefault('theme:darkMode');
? !!(await uiSettings.get('theme:darkMode'))
: !!uiSettings.getOverrideOrDefault('theme:darkMode');

const configThemeVersion =
!authEnabled || request.auth.isAuthenticated
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,7 @@ class TableListView extends React.Component<TableListViewProps, TableListViewSta
type: 'icon',
enabled: ({ error }: { error: string }) => !error,
onClick: this.props.editItem,
'data-test-subj': 'edit-dashboard-action',
},
];

Expand Down
45 changes: 40 additions & 5 deletions test/functional/apps/visualize/_custom_branding.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,25 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
expect(actualLabel.toUpperCase()).to.equal(expectedMarkLogo.toUpperCase());
});

it('with customized logo for opensearch overview header in dark mode', async () => {
it('if enable user control, admin customized dark mode logo for opensearch overview header is not applied', async () => {
await PageObjects.common.navigateToApp('management/opensearch-dashboards/settings');
await PageObjects.settings.toggleAdvancedSettingCheckbox('theme:enableUserControl');
const button = await testSubjects.find('advancedSetting-editField-theme:darkMode');
const isDisabled = (await button.getAttribute('disabled')) !== null;
expect(isDisabled).equal(true);
await PageObjects.common.navigateToApp('opensearch_dashboards_overview');
await testSubjects.existOrFail('osdOverviewPageHeaderLogo');
const actualLabel = await testSubjects.getAttribute(
'osdOverviewPageHeaderLogo',
'data-test-logo'
);
expect(actualLabel.toUpperCase()).to.equal(expectedMarkLogo.toUpperCase());
await PageObjects.common.navigateToApp('management/opensearch-dashboards/settings');
await PageObjects.settings.clearAdvancedSettings('theme:enableUserControl');
});

it('admin customized dark mode logo for opensearch overview header is applied', async () => {
await PageObjects.common.navigateToApp('management/opensearch-dashboards/settings');
await PageObjects.settings.toggleAdvancedSettingCheckbox('theme:darkMode');
await PageObjects.common.navigateToApp('opensearch_dashboards_overview');
await testSubjects.existOrFail('osdOverviewPageHeaderLogo');
Expand All @@ -56,6 +72,8 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
'data-test-logo'
);
expect(actualLabel.toUpperCase()).to.equal(expectedMarkLogoDarkMode.toUpperCase());
await PageObjects.common.navigateToApp('management/opensearch-dashboards/settings');
await PageObjects.settings.clearAdvancedSettings('theme:darkMode');
});
});

Expand Down Expand Up @@ -100,9 +118,8 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
expect(actualLabel.toUpperCase()).to.equal(expectedWelcomeMessage.toUpperCase());
});

it('with customized logo in dark mode', async () => {
it('admin customized dark mode logo for home is applied', async () => {
await PageObjects.common.navigateToApp('management/opensearch-dashboards/settings');
await PageObjects.settings.toggleAdvancedSettingCheckbox('theme:enableUserControl');
await PageObjects.settings.toggleAdvancedSettingCheckbox('theme:darkMode');
await PageObjects.common.navigateToApp('home');
await testSubjects.existOrFail('welcomeCustomLogo');
Expand All @@ -111,6 +128,25 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
'data-test-image-url'
);
expect(actualLabel.toUpperCase()).to.equal(expectedMarkLogoDarkMode.toUpperCase());
await PageObjects.common.navigateToApp('management/opensearch-dashboards/settings');
await PageObjects.settings.clearAdvancedSettings('theme:darkMode');
});

it('if enable user control, admin customized dark mode logo for home is not applied', async () => {
await PageObjects.common.navigateToApp('management/opensearch-dashboards/settings');
await PageObjects.settings.toggleAdvancedSettingCheckbox('theme:enableUserControl');
const button = await testSubjects.find('advancedSetting-editField-theme:darkMode');
const isDisabled = (await button.getAttribute('disabled')) !== null;
expect(isDisabled).equal(true);
await PageObjects.common.navigateToApp('home');
await testSubjects.existOrFail('welcomeCustomLogo');
const actualLabel = await testSubjects.getAttribute(
'welcomeCustomLogo',
'data-test-image-url'
);
expect(actualLabel.toUpperCase()).to.equal(expectedMarkLogo.toUpperCase());
await PageObjects.common.navigateToApp('management/opensearch-dashboards/settings');
await PageObjects.settings.clearAdvancedSettings('theme:enableUserControl');
});
});

Expand Down Expand Up @@ -180,10 +216,9 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
});
});

describe('in dark mode', async () => {
describe('OpenSearch Dashboards branding configuration in dark mode', async () => {
before(async function () {
await PageObjects.common.navigateToApp('management/opensearch-dashboards/settings');
await PageObjects.settings.toggleAdvancedSettingCheckbox('theme:enableUserControl');
await PageObjects.settings.toggleAdvancedSettingCheckbox('theme:darkMode');
await PageObjects.common.navigateToApp('home');
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@ export default function ({ getService, getPageObjects }) {

it('should be able to navigate to edit dashboard', async () => {
await listingTable.searchForItemWithName(dashboardName);
const editBttn = await find.allByCssSelector('.euiToolTipAnchor');
await editBttn[3].click();
const editBttn = await testSubjects.find('edit-dashboard-action');
await editBttn.click();
await PageObjects.dashboard.clickCancelOutOfEditMode();
await PageObjects.dashboard.gotoDashboardLandingPage();
});
Expand Down
31 changes: 3 additions & 28 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -15986,7 +15986,7 @@ string-similarity@^4.0.1:
resolved "https://registry.yarnpkg.com/string-similarity/-/string-similarity-4.0.4.tgz#42d01ab0b34660ea8a018da8f56a3309bb8b2a5b"
integrity sha512-/q/8Q4Bl4ZKAPjj8WerIBJWALKkaPRfrvhfF8k/B23i4nzrlRj2/go1m90In7nG/3XDSbOo0+pu6RvCTM9RGMQ==

"string-width-cjs@npm:string-width@^4.2.0":
"string-width-cjs@npm:string-width@^4.2.0", string-width@^4.1.0, string-width@^4.2.0, string-width@^4.2.3:
version "4.2.3"
resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010"
integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g==
Expand Down Expand Up @@ -16021,15 +16021,6 @@ string-width@^3.0.0:
is-fullwidth-code-point "^2.0.0"
strip-ansi "^5.1.0"

string-width@^4.1.0, string-width@^4.2.0, string-width@^4.2.3:
version "4.2.3"
resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010"
integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g==
dependencies:
emoji-regex "^8.0.0"
is-fullwidth-code-point "^3.0.0"
strip-ansi "^6.0.1"

string-width@^5.0.1, string-width@^5.1.2:
version "5.1.2"
resolved "https://registry.yarnpkg.com/string-width/-/string-width-5.1.2.tgz#14f8daec6d81e7221d2a357e668cab73bdbca794"
Expand Down Expand Up @@ -16108,7 +16099,7 @@ stringify-entities@^3.0.1:
character-entities-legacy "^1.0.0"
xtend "^4.0.0"

"strip-ansi-cjs@npm:strip-ansi@^6.0.1":
"strip-ansi-cjs@npm:strip-ansi@^6.0.1", strip-ansi@^6.0.0, strip-ansi@^6.0.1:
version "6.0.1"
resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9"
integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A==
Expand Down Expand Up @@ -16150,13 +16141,6 @@ strip-ansi@^5.1.0, strip-ansi@^5.2.0:
dependencies:
ansi-regex "^4.1.0"

strip-ansi@^6.0.0, strip-ansi@^6.0.1:
version "6.0.1"
resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9"
integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A==
dependencies:
ansi-regex "^5.0.1"

strip-ansi@^7.0.1:
version "7.1.0"
resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-7.1.0.tgz#d5b6568ca689d8561370b0707685d22434faff45"
Expand Down Expand Up @@ -18300,7 +18284,7 @@ [email protected]:
resolved "https://registry.yarnpkg.com/workerpool/-/workerpool-6.2.1.tgz#46fc150c17d826b86a008e5a4508656777e9c343"
integrity sha512-ILEIE97kDZvF9Wb9f6h5aXK4swSlKGUcOEGiIYb2OOu/IrDU9iwj0fD//SsA6E5ibwJxpEvhullJY4Sl4GcpAw==

"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0":
"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0", wrap-ansi@^7.0.0:
version "7.0.0"
resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-7.0.0.tgz#67e145cff510a6a6984bdf1152911d69d2eb9e43"
integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q==
Expand All @@ -18326,15 +18310,6 @@ wrap-ansi@^6.2.0:
string-width "^4.1.0"
strip-ansi "^6.0.0"

wrap-ansi@^7.0.0:
version "7.0.0"
resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-7.0.0.tgz#67e145cff510a6a6984bdf1152911d69d2eb9e43"
integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q==
dependencies:
ansi-styles "^4.0.0"
string-width "^4.1.0"
strip-ansi "^6.0.0"

wrap-ansi@^8.1.0:
version "8.1.0"
resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-8.1.0.tgz#56dc22368ee570face1b49819975d9b9a5ead214"
Expand Down
Loading