[Spaces Management] Fix issue with enabled features selection when sp…

…ace solution is unselected (#194352) ## Summary Resolves #192812 The feature PR for the new UX in Spaces Management introduced a new editing screen for Spaces. Inadvertently, it introduced a bug where the user's selection of visible features is ignored unless the user has explicitly selected a solution view. This PR fixes that issue as well as adds tests to prevent any future regressions. Also, this PR fixes an issue where the space initials could be left blank, which was another regression. ### Checklist Delete any items that are not applicable to this PR. - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [x] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed --------- Co-authored-by: Elena Shostak <[email protected]>
elastic · Sep 30, 2024 · 0d421e5 · 0d421e5
1 parent 283f6e6
commit 0d421e5
Show file tree

Hide file tree

Showing 5 changed files with 156 additions and 9 deletions.
diff --git a/x-pack/plugins/spaces/public/management/components/enabled_features/feature_table.tsx b/x-pack/plugins/spaces/public/management/components/enabled_features/feature_table.tsx
@@ -130,7 +130,10 @@ export class FeatureTable extends Component<Props, {}> {
       const accordion = (
         <EuiFlexGroup key={category.id} alignItems="baseline" responsive={false} gutterSize="s">
           <EuiFlexItem grow={false}>
-            <EuiCheckbox {...checkboxProps} />
+            <EuiCheckbox
+              {...checkboxProps}
+              data-test-subj={`featureCategoryCheckbox_${category.id}`}
+            />
           </EuiFlexItem>
           <EuiFlexItem grow={1}>
             <EuiAccordion

diff --git a/x-pack/plugins/spaces/public/management/edit_space/edit_space_general_tab.test.tsx b/x-pack/plugins/spaces/public/management/edit_space/edit_space_general_tab.test.tsx
@@ -407,6 +407,76 @@ describe('EditSpaceSettings', () => {
     expect(navigateSpy).toHaveBeenCalledTimes(1);
   });
 
+  it('submits the disabled features list when the solution view is undefined', async () => {
+    const features = [
+      new KibanaFeature({
+        id: 'feature-1',
+        name: 'feature 1',
+        app: [],
+        category: DEFAULT_APP_CATEGORIES.kibana,
+        privileges: null,
+      }),
+    ];
+
+    const spaceToUpdate = {
+      id: 'existing-space',
+      name: 'Existing Space',
+      description: 'hey an existing space',
+      color: '#aabbcc',
+      initials: 'AB',
+      disabledFeatures: [],
+      solution: undefined,
+    };
+
+    render(
+      <TestComponent>
+        <EditSpaceSettingsTab
+          space={spaceToUpdate}
+          history={history}
+          features={features}
+          allowFeatureVisibility={true}
+          allowSolutionVisibility={true}
+          reloadWindow={reloadWindow}
+        />
+      </TestComponent>
+    );
+
+    // update the space visible features
+    const feature1Checkbox = screen.getByTestId('featureCheckbox_feature-1');
+    expect(feature1Checkbox).toBeChecked();
+    await act(async () => {
+      await userEvent.click(feature1Checkbox);
+    });
+    await waitFor(() => {
+      expect(feature1Checkbox).not.toBeChecked();
+    });
+
+    expect(screen.getByTestId('space-edit-page-user-impact-warning')).toBeInTheDocument();
+    expect(screen.queryByTestId('confirmModalTitleText')).not.toBeInTheDocument();
+
+    const updateButton = screen.getByTestId('save-space-button');
+    await act(async () => {
+      await userEvent.click(updateButton);
+    });
+
+    expect(screen.getByTestId('confirmModalTitleText')).toBeInTheDocument();
+
+    const confirmButton = screen.getByTestId('confirmModalConfirmButton');
+    await act(async () => {
+      await userEvent.click(confirmButton);
+    });
+
+    await waitFor(() => {
+      expect(updateSpaceSpy).toHaveBeenCalledWith({
+        ...spaceToUpdate,
+        imageUrl: '',
+        disabledFeatures: ['feature-1'],
+      });
+    });
+
+    expect(navigateSpy).toHaveBeenCalledTimes(1);
+  });
+
   it('empties the disabled features list when the solution view non-classic', async () => {
     const features = [
       new KibanaFeature({

diff --git a/x-pack/plugins/spaces/public/management/edit_space/edit_space_general_tab.tsx b/x-pack/plugins/spaces/public/management/edit_space/edit_space_general_tab.tsx
@@ -6,7 +6,7 @@
  */
 
 import { EuiCallOut, EuiSpacer } from '@elastic/eui';
-import React, { useCallback, useState } from 'react';
+import React, { useCallback, useMemo, useState } from 'react';
 
 import type { ScopedHistory } from '@kbn/core-application-browser';
 import type { KibanaFeature } from '@kbn/features-plugin/common';
@@ -18,6 +18,7 @@ import { EditSpaceTabFooter } from './footer';
 import { useEditSpaceServices } from './provider';
 import type { Space } from '../../../common';
 import { SOLUTION_VIEW_CLASSIC } from '../../../common/constants';
+import { getSpaceInitials } from '../../space_avatar';
 import { ConfirmDeleteModal } from '../components';
 import { ConfirmAlterActiveSpaceModal } from '../components/confirm_alter_active_space_modal';
 import { CustomizeSpace } from '../components/customize_space';
@@ -42,6 +43,11 @@ export const EditSpaceSettingsTab: React.FC<Props> = ({ space, features, history
     imageUrl: imageAvatarSelected ? space.imageUrl : '',
   });
 
+  // space initials are blank by default, they must be calculated
+  const getSpaceFromFormValues = (newFormValues: CustomizeSpaceFormValues) => {
+    return { ...newFormValues, initials: getSpaceInitials(newFormValues) };
+  };
+
   const [isDirty, setIsDirty] = useState(false); // track if unsaved changes have been made
   const [isLoading, setIsLoading] = useState(false); // track if user has just clicked the Update button
   const [showUserImpactWarning, setShowUserImpactWarning] = useState(false);
@@ -137,7 +143,7 @@ export const EditSpaceSettingsTab: React.FC<Props> = ({ space, features, history
       setIsLoading(true);
 
       let disabledFeatures: string[] | undefined;
-      if (spaceClone.solution === SOLUTION_VIEW_CLASSIC) {
+      if (!spaceClone.solution || spaceClone.solution === SOLUTION_VIEW_CLASSIC) {
         disabledFeatures = spaceClone.disabledFeatures;
       }
 
@@ -181,13 +187,27 @@ export const EditSpaceSettingsTab: React.FC<Props> = ({ space, features, history
     [backToSpacesList, notifications.toasts, formValues, spacesManager, logger, props]
   );
 
+  const validator = useMemo(() => new SpaceValidator(), []);
+
   const onClickSubmit = useCallback(() => {
+    validator.enableValidation();
+    const validationResult = validator.validateForSave(
+      formValues,
+      true,
+      props.allowSolutionVisibility
+    );
+
+    if (validationResult.isInvalid) {
+      // invalid form input fields will show the error message
+      return;
+    }
+
     if (showUserImpactWarning) {
       setShowAlteringActiveSpaceDialog(true);
     } else {
       performSave({ requiresReload: false });
     }
-  }, [performSave, showUserImpactWarning]);
+  }, [validator, formValues, props.allowSolutionVisibility, performSave, showUserImpactWarning]);
 
   const doShowAlteringActiveSpaceDialog = () => {
     return (
@@ -245,15 +265,13 @@ export const EditSpaceSettingsTab: React.FC<Props> = ({ space, features, history
     );
   };
 
-  const validator = new SpaceValidator();
-
   return (
     <>
       {doShowAlteringActiveSpaceDialog()}
       {doShowConfirmDeleteSpaceDialog()}
 
       <CustomizeSpace
-        space={formValues}
+        space={getSpaceFromFormValues(formValues)}
         onChange={onChangeSpaceSettings}
         editingExistingSpace={true}
         validator={validator}
@@ -263,7 +281,7 @@ export const EditSpaceSettingsTab: React.FC<Props> = ({ space, features, history
         <>
           <EuiSpacer />
           <SolutionView
-            space={formValues}
+            space={getSpaceFromFormValues(formValues)}
             onChange={onSolutionViewChange}
             validator={validator}
             isEditing={true}
@@ -276,7 +294,7 @@ export const EditSpaceSettingsTab: React.FC<Props> = ({ space, features, history
           <EuiSpacer />
           <EditSpaceEnabledFeatures
             features={features}
-            space={formValues}
+            space={getSpaceFromFormValues(formValues)}
             onChange={onChangeFeatures}
           />
         </>

diff --git a/x-pack/test/functional/apps/spaces/solution_view_flag_enabled/create_edit_space.ts b/x-pack/test/functional/apps/spaces/solution_view_flag_enabled/create_edit_space.ts
@@ -5,12 +5,14 @@
  * 2.0.
  */
 
+import expect from '@kbn/expect';
 import { FtrProviderContext } from '../../../ftr_provider_context';
 
 export default function ({ getPageObjects, getService }: FtrProviderContext) {
   const kibanaServer = getService('kibanaServer');
   const PageObjects = getPageObjects(['common', 'settings', 'security', 'spaceSelector']);
   const testSubjects = getService('testSubjects');
+  const spacesService = getService('spaces');
   const find = getService('find');
 
   describe('edit space', () => {
@@ -69,5 +71,55 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) {
         await testSubjects.missingOrFail('searchSideNav');
       });
     });
+
+    describe('API-created Space', () => {
+      before(async () => {
+        await spacesService.create({
+          id: 'foo-space',
+          name: 'Foo Space',
+          disabledFeatures: [],
+          color: '#AABBCC',
+        });
+      });
+
+      after(async () => {
+        await spacesService.delete('foo-space');
+      });
+
+      it('enabled features can be changed while the solution view remains unselected', async () => {
+        const securityFeatureCheckboxId = 'featureCategoryCheckbox_securitySolution';
+
+        await PageObjects.common.navigateToUrl('management', 'kibana/spaces/edit/foo-space', {
+          shouldUseHashForSubUrl: false,
+        });
+
+        await testSubjects.existOrFail('spaces-view-page');
+
+        // ensure security feature is selected by default
+        expect(await testSubjects.isChecked(securityFeatureCheckboxId)).to.be(true);
+
+        // Do not set a solution view first!
+
+        await PageObjects.spaceSelector.toggleFeatureCategoryCheckbox('securitySolution');
+        //
+        // ensure security feature now unselected
+        expect(await testSubjects.isChecked(securityFeatureCheckboxId)).to.be(false);
+
+        await testSubjects.existOrFail('space-edit-page-user-impact-warning');
+
+        await PageObjects.spaceSelector.clickSaveSpaceCreation();
+
+        await testSubjects.click('confirmModalConfirmButton');
+
+        await testSubjects.existOrFail('spaces-view-page');
+
+        await testSubjects.click('foo-space-hyperlink');
+
+        await testSubjects.existOrFail('spaces-view-page');
+
+        // ensure security feature is still unselected
+        expect(await testSubjects.isChecked(securityFeatureCheckboxId)).to.be(false);
+      });
+    });
   });
 }
diff --git a/x-pack/test/functional/page_objects/space_selector_page.ts b/x-pack/test/functional/page_objects/space_selector_page.ts
@@ -204,6 +204,10 @@ export class SpaceSelectorPageObject extends FtrService {
     await this.testSubjects.click(`featureCategoryButton_${categoryName}`);
   }
 
+  async toggleFeatureCategoryCheckbox(categoryName: string) {
+    await this.testSubjects.click(`featureCategoryCheckbox_${categoryName}`);
+  }
+
   async clickOnDescriptionOfSpace() {
     await this.testSubjects.click('descriptionSpaceText');
   }