Fixing User Profiles/Kibana.yml config light mode precedence logic (#…

…158177) ## Summary After changing the UserSettingService to calculate darkmode and return `boolean | undefined` , the Rendering service `darkMode` logic needed to be updated to work when a User chooses 'Light' which provides a 'false' value to the Rendering service. ## Testing For Space Setting: 1. Set Space Adv. Setting to darkMode: true 2. Set User Profile Setting to 'Light' 3. Observe that Light mode takes precedence For Config setting: 1. Set User Profile Setting to 'Dark' 2. In `kibana.yml` set `uiSettings.overrides.theme:darkMode: false` 3. Observe that Light mode takes precedence
elastic · May 22, 2023 · 613b2d5 · 613b2d5
1 parent e1523c1
commit 613b2d5
Show file tree

Hide file tree

Showing 4 changed files with 182 additions and 16 deletions.
diff --git a/packages/core/rendering/core-rendering-server-internal/src/rendering_service.test.ts b/packages/core/rendering/core-rendering-server-internal/src/rendering_service.test.ts
@@ -209,7 +209,7 @@ function renderDarkModeTestCases(
     });
 
     describe('Dark Mode', () => {
-      it('UserSettings value should override the space setting', async () => {
+      it('UserSettings darkMode === true should override the space setting', async () => {
         mockRenderingSetupDeps.userSettings.getUserSettingDarkMode.mockReturnValueOnce(
           Promise.resolve(true)
         );
@@ -235,6 +235,32 @@ function renderDarkModeTestCases(
         });
       });
 
+      it('UserSettings darkMode === false should override the space setting', async () => {
+        mockRenderingSetupDeps.userSettings.getUserSettingDarkMode.mockReturnValueOnce(
+          Promise.resolve(false)
+        );
+
+        getSettingValueMock.mockImplementation((settingName: string) => {
+          if (settingName === 'theme:darkMode') {
+            return true;
+          }
+          return settingName;
+        });
+
+        const settings = { 'theme:darkMode': { userValue: false } };
+        uiSettings.client.getUserProvided.mockResolvedValue(settings);
+
+        const [render] = await getRender();
+        await render(createKibanaRequest(), uiSettings);
+
+        expect(getStylesheetPathsMock).toHaveBeenCalledWith({
+          darkMode: false,
+          themeVersion: 'v8',
+          basePath: '/mock-server-basepath',
+          buildNum: expect.any(Number),
+        });
+      });
+
       it('Space setting value should be used if UsersSettings value is undefined', async () => {
         mockRenderingSetupDeps.userSettings.getUserSettingDarkMode.mockReturnValueOnce(
           Promise.resolve(undefined)
@@ -258,6 +284,102 @@ function renderDarkModeTestCases(
           buildNum: expect.any(Number),
         });
       });
+
+      it('config `theme:darkMode: true` setting should override User Settings theme `darkMode === false', async () => {
+        mockRenderingSetupDeps.userSettings.getUserSettingDarkMode.mockReturnValueOnce(
+          Promise.resolve(false)
+        );
+        getSettingValueMock.mockImplementation((settingName: string) => {
+          if (settingName === 'theme:darkMode') {
+            return true;
+          }
+          return settingName;
+        });
+
+        const settings = { 'theme:darkMode': { userValue: true, isOverridden: true } };
+        uiSettings.client.getUserProvided.mockResolvedValue(settings);
+        const [render] = await getRender();
+        await render(createKibanaRequest(), uiSettings);
+
+        expect(getStylesheetPathsMock).toHaveBeenCalledWith({
+          darkMode: true,
+          themeVersion: 'v8',
+          basePath: '/mock-server-basepath',
+          buildNum: expect.any(Number),
+        });
+      });
+
+      it('config `theme:darkMode: false` setting should override User Settings theme `darkMode === true', async () => {
+        mockRenderingSetupDeps.userSettings.getUserSettingDarkMode.mockReturnValueOnce(
+          Promise.resolve(true)
+        );
+        getSettingValueMock.mockImplementation((settingName: string) => {
+          if (settingName === 'theme:darkMode') {
+            return false;
+          }
+          return settingName;
+        });
+
+        const settings = { 'theme:darkMode': { userValue: false, isOverridden: true } };
+        uiSettings.client.getUserProvided.mockResolvedValue(settings);
+        const [render] = await getRender();
+        await render(createKibanaRequest(), uiSettings);
+
+        expect(getStylesheetPathsMock).toHaveBeenCalledWith({
+          darkMode: false,
+          themeVersion: 'v8',
+          basePath: '/mock-server-basepath',
+          buildNum: expect.any(Number),
+        });
+      });
+
+      it('config `theme:darkMode: false` setting should override User Settings theme `darkMode === undefined', async () => {
+        mockRenderingSetupDeps.userSettings.getUserSettingDarkMode.mockReturnValueOnce(
+          Promise.resolve(undefined)
+        );
+        getSettingValueMock.mockImplementation((settingName: string) => {
+          if (settingName === 'theme:darkMode') {
+            return false;
+          }
+          return settingName;
+        });
+
+        const settings = { 'theme:darkMode': { userValue: false, isOverridden: true } };
+        uiSettings.client.getUserProvided.mockResolvedValue(settings);
+        const [render] = await getRender();
+        await render(createKibanaRequest(), uiSettings);
+
+        expect(getStylesheetPathsMock).toHaveBeenCalledWith({
+          darkMode: false,
+          themeVersion: 'v8',
+          basePath: '/mock-server-basepath',
+          buildNum: expect.any(Number),
+        });
+      });
+
+      it('config `theme:darkMode: true` setting should override User Settings theme `darkMode === undefined', async () => {
+        mockRenderingSetupDeps.userSettings.getUserSettingDarkMode.mockReturnValueOnce(
+          Promise.resolve(undefined)
+        );
+        getSettingValueMock.mockImplementation((settingName: string) => {
+          if (settingName === 'theme:darkMode') {
+            return true;
+          }
+          return settingName;
+        });
+
+        const settings = { 'theme:darkMode': { userValue: true, isOverridden: true } };
+        uiSettings.client.getUserProvided.mockResolvedValue(settings);
+        const [render] = await getRender();
+        await render(createKibanaRequest(), uiSettings);
+
+        expect(getStylesheetPathsMock).toHaveBeenCalledWith({
+          darkMode: true,
+          themeVersion: 'v8',
+          basePath: '/mock-server-basepath',
+          buildNum: expect.any(Number),
+        });
+      });
     });
   });
 }

diff --git a/packages/core/rendering/core-rendering-server-internal/src/rendering_service.tsx b/packages/core/rendering/core-rendering-server-internal/src/rendering_service.tsx
@@ -167,7 +167,9 @@ export class RenderingService {
 
     let darkMode: boolean;
 
-    if (userSettingDarkMode) {
+    const isThemeOverridden = settings.user['theme:darkMode']?.isOverridden ?? false;
+
+    if (userSettingDarkMode !== undefined && !isThemeOverridden) {
       darkMode = userSettingDarkMode;
     } else {
       darkMode = getSettingValue('theme:darkMode', settings, Boolean);

diff --git a/x-pack/plugins/security/public/account_management/user_profile/user_profile.test.tsx b/x-pack/plugins/security/public/account_management/user_profile/user_profile.test.tsx
@@ -310,7 +310,7 @@ describe('useUserProfileForm', () => {
       );
     });
 
-    it('should be disabled if the theme has been set in the config', () => {
+    it('should be disabled if the theme has been set to `darkMode: true` in the config', () => {
       const data: UserProfileData = {};
 
       const nonCloudUser = mockAuthenticatedUser({ elastic_cloud_user: false });
@@ -336,5 +336,32 @@ describe('useUserProfileForm', () => {
       expect(darkModeButton).toBeTruthy();
       expect(darkModeButton.getDOMNode()).toHaveProperty('disabled');
     });
+
+    it('should be disabled if the theme has been set to `darkMode: false` in the config', () => {
+      const data: UserProfileData = {};
+
+      const nonCloudUser = mockAuthenticatedUser({ elastic_cloud_user: false });
+      coreStart.settings.client.get.mockReturnValueOnce(false);
+      coreStart.settings.client.isOverridden.mockReturnValueOnce(true);
+
+      const testWrapper = mount(
+        <Providers
+          services={coreStart}
+          theme$={theme$}
+          history={history}
+          authc={authc}
+          securityApiClients={{
+            userProfiles: new UserProfileAPIClient(coreStart.http),
+            users: new UserAPIClient(coreStart.http),
+          }}
+        >
+          <UserProfile user={nonCloudUser} data={data} />
+        </Providers>
+      );
+
+      const darkModeButton = testWrapper.find('EuiButtonGroup[data-test-subj="darkModeButton"]');
+      expect(darkModeButton).toBeTruthy();
+      expect(darkModeButton.getDOMNode()).toHaveProperty('disabled');
+    });
   });
 });
diff --git a/x-pack/plugins/security/public/account_management/user_profile/user_profile.tsx b/x-pack/plugins/security/public/account_management/user_profile/user_profile.tsx
@@ -144,19 +144,25 @@ function UserDetailsEditor({ user }: { user: AuthenticatedUser }) {
 
 function UserSettingsEditor({
   formik,
-  isDarkModeOverride,
+  isThemeOverridden,
+  isOverriddenThemeDarkMode,
 }: {
   formik: ReturnType<typeof useUserProfileForm>;
-  isDarkModeOverride: boolean;
+  isThemeOverridden: boolean;
+  isOverriddenThemeDarkMode: boolean;
 }) {
   if (!formik.values.data) {
     return null;
   }
 
   let idSelected = formik.values.data.userSettings.darkMode;
 
-  if (isDarkModeOverride) {
-    idSelected = 'dark';
+  if (isThemeOverridden) {
+    if (isOverriddenThemeDarkMode) {
+      idSelected = 'dark';
+    } else {
+      idSelected = 'light';
+    }
   }
 
   return (
@@ -180,7 +186,7 @@ function UserSettingsEditor({
     >
       <FormRow
         name="data.userSettings.darkMode"
-        helpText={renderHelpText(isDarkModeOverride)}
+        helpText={renderHelpText(isThemeOverridden)}
         label={
           <FormLabel for="data.userSettings.darkMode">
             <FormattedMessage
@@ -201,7 +207,7 @@ function UserSettingsEditor({
           buttonSize="m"
           data-test-subj="darkModeButton"
           idSelected={idSelected}
-          isDisabled={isDarkModeOverride}
+          isDisabled={isThemeOverridden}
           options={[
             {
               id: '',
@@ -546,7 +552,9 @@ export const UserProfile: FunctionComponent<UserProfileProps> = ({ user, data })
 
   const isCloudUser = user.elastic_cloud_user;
 
-  const isDarkModeOverride = determineIfDarkModeOverride(services.settings.client);
+  const { isThemeOverridden, isOverriddenThemeDarkMode } = determineIfThemeOverridden(
+    services.settings.client
+  );
 
   const rightSideItems = [
     {
@@ -675,7 +683,11 @@ export const UserProfile: FunctionComponent<UserProfileProps> = ({ user, data })
                   onShowPasswordForm={() => setShowChangePasswordForm(true)}
                 />
                 {isCloudUser ? null : (
-                  <UserSettingsEditor formik={formik} isDarkModeOverride={isDarkModeOverride} />
+                  <UserSettingsEditor
+                    formik={formik}
+                    isThemeOverridden={isThemeOverridden}
+                    isOverriddenThemeDarkMode={isOverriddenThemeDarkMode}
+                  />
                 )}
               </Form>
             </EuiPageTemplate>
@@ -910,9 +922,12 @@ function renderHelpText(isOverridden: boolean) {
   }
 }
 
-function determineIfDarkModeOverride(settingsClient: IUiSettingsClient) {
-  const isThemeOverridden = settingsClient.isOverridden('theme:darkMode');
-  const isOverriddenThemeDarkMode = settingsClient.get<boolean>('theme:darkMode');
-
-  return isThemeOverridden && isOverriddenThemeDarkMode;
+function determineIfThemeOverridden(settingsClient: IUiSettingsClient): {
+  isThemeOverridden: boolean;
+  isOverriddenThemeDarkMode: boolean;
+} {
+  return {
+    isThemeOverridden: settingsClient.isOverridden('theme:darkMode'),
+    isOverriddenThemeDarkMode: settingsClient.get<boolean>('theme:darkMode'),
+  };
 }