From 84d4a5ab394649fd0bf1096fe83c2c28218755c5 Mon Sep 17 00:00:00 2001 From: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Date: Tue, 1 Oct 2024 07:45:59 +1000 Subject: [PATCH] [8.x] [Spaces Management] Fix issue with enabled features selection when 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)](https://github.com/elastic/kibana/pull/194352) ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) Co-authored-by: Tim Sullivan --- .../enabled_features/feature_table.tsx | 5 +- .../edit_space_general_tab.test.tsx | 70 +++++++++++++++++++ .../edit_space/edit_space_general_tab.tsx | 34 ++++++--- .../create_edit_space.ts | 52 ++++++++++++++ .../page_objects/space_selector_page.ts | 4 ++ 5 files changed, 156 insertions(+), 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 index bc4b586c6cd1d..4fb5786454eed 100644 --- 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 { const accordion = ( - + { 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( + + + + ); + + // 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 index 2b7f04e4d9417..24269528916f8 100644 --- 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 = ({ 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 = ({ 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 = ({ 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 = ({ space, features, history ); }; - const validator = new SpaceValidator(); - return ( <> {doShowAlteringActiveSpaceDialog()} {doShowConfirmDeleteSpaceDialog()} = ({ space, features, history <> = ({ space, features, history 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 index f6f69ada3c0c1..8695077eae74e 100644 --- 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 index e5afbd78fe767..987b03533b480 100644 --- 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'); }