[8.x] [Spaces Management] Fix issue with enabled features selection w…

…hen space solution is unselected (#194352) (#194486) # Backport This will backport the following commits from `main` to `8.x`: - [[Spaces Management] Fix issue with enabled features selection when space solution is unselected (#194352)](#194352)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Tim Sullivan <[email protected]>
elastic · Sep 30, 2024 · 84d4a5a · 84d4a5a
1 parent 8618d89
commit 84d4a5a
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');
   }