unskip Failing test: Chrome X-Pack UI Functional Tests.x-pack/test/fu…

…nctional/apps/maps/group2/embeddable/add_to_dashboard·js (#167566) Closes #167320 flaky test runner https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/3278 Flakiness caused by #166991. Cause of flakiness is not waiting for dashboard-picker to finish loading options after setting the search value. Flaky test highlights 3 other problems with `SavedObjectSaveModalDashboard` component that are resolved in this PR: 1) Dashboard select not disabled when "New" or "None" is selected. Regression from #166991 2) There is no form validation message giving visual feedback when users click "Save" with "Existing" and no selected dashboard. 3) #166991 switched dashboard selector from `EuiComboBox` to `EuiSelectable`. This changed the behavior of selecting the same item. With the `EuiComboBox` implemenation, selecting the same item retained the selection while selecting the same item with the `EuiSelectable` implementation unselected the item. <img width="300" alt="Screenshot 2023-09-29 at 12 51 29 PM" src="https://github.com/elastic/kibana/assets/373691/edd8d647-b8b6-4ecc-8f91-d1ad9e916a95"> --------- Co-authored-by: Kibana Machine <[email protected]>
elastic · Oct 4, 2023 · 9e30402 · 9e30402
1 parent 8b89c94
commit 9e30402
Show file tree

Hide file tree

Showing 6 changed files with 39 additions and 13 deletions.
diff --git a/src/plugins/presentation_util/public/components/dashboard_picker/dashboard_picker.tsx b/src/plugins/presentation_util/public/components/dashboard_picker/dashboard_picker.tsx
@@ -117,6 +117,7 @@ export function DashboardPicker({ isDisabled, onChange, idsToOmit }: DashboardPi
       panelPaddingSize="none"
       input={
         <ToolbarButton
+          isDisabled={isDisabled}
           fullWidth
           isLoading={isLoading}
           data-test-subj="open-dashboard-picker"
@@ -160,14 +161,9 @@ export function DashboardPicker({ isDisabled, onChange, idsToOmit }: DashboardPi
         onChange={(newOptions, event, selected) => {
           setIsPopoverOpen(false);
 
-          const nextSelectedDashboard: DashboardOption | null =
-            selected.value === selectedDashboard?.value ? null : selected;
-          setSelectedDashboard(nextSelectedDashboard);
-          onChange(
-            nextSelectedDashboard
-              ? { name: nextSelectedDashboard.label, id: nextSelectedDashboard.value }
-              : null
-          );
+          if (!selected || selected.value === selectedDashboard?.value) return;
+          setSelectedDashboard(selected);
+          onChange({ name: selected.label, id: selected.value });
         }}
         renderOption={(option) => <EuiHighlight search={query}>{option.label}</EuiHighlight>}
       >

diff --git a/src/plugins/presentation_util/public/components/saved_object_save_modal_dashboard.tsx b/src/plugins/presentation_util/public/components/saved_object_save_modal_dashboard.tsx
@@ -10,7 +10,11 @@ import React, { useState } from 'react';
 
 import { i18n } from '@kbn/i18n';
 
-import { OnSaveProps, SavedObjectSaveModal } from '@kbn/saved-objects-plugin/public';
+import {
+  OnSaveProps,
+  SavedObjectSaveModal,
+  type SaveModalState,
+} from '@kbn/saved-objects-plugin/public';
 
 import { pluginServices } from '../services';
 import { SaveModalDashboardProps } from './types';
@@ -39,7 +43,7 @@ function SavedObjectSaveModalDashboard(props: SaveModalDashboardProps) {
   const [copyOnSave, setCopyOnSave] = useState<boolean>(initialCopyOnSave);
 
   const rightOptions = !disableDashboardOptions
-    ? () => (
+    ? ({ hasAttemptedSubmit }: SaveModalState) => (
         <SaveModalDashboardSelector
           onSelectDashboard={(dash) => {
             setSelectedDashboard(dash);
@@ -48,7 +52,15 @@ function SavedObjectSaveModalDashboard(props: SaveModalDashboardProps) {
             setDashboardOption(option);
           }}
           canSaveByReference={canSaveByReference}
-          {...{ copyOnSave, documentId, dashboardOption, setAddToLibrary, isAddToLibrarySelected }}
+          {...{
+            copyOnSave,
+            documentId,
+            dashboardOption,
+            setAddToLibrary,
+            isAddToLibrarySelected,
+            hasAttemptedSubmit,
+            hasSelectedDashboard: Boolean(selectedDashboard),
+          }}
         />
       )
     : null;

diff --git a/...resentation_util/public/components/saved_object_save_modal_dashboard_selector.stories.tsx b/...resentation_util/public/components/saved_object_save_modal_dashboard_selector.stories.tsx
@@ -62,6 +62,8 @@ export function Example({
       documentId={hasDocumentId ? 'abc' : undefined}
       isAddToLibrarySelected={isAddToLibrarySelected}
       setAddToLibrary={setAddToLibrary}
+      hasAttemptedSubmit={false}
+      hasSelectedDashboard={false}
     />
   );
 }
diff --git a/...lugins/presentation_util/public/components/saved_object_save_modal_dashboard_selector.tsx b/...lugins/presentation_util/public/components/saved_object_save_modal_dashboard_selector.tsx
@@ -33,6 +33,8 @@ export interface SaveModalDashboardSelectorProps {
   isAddToLibrarySelected: boolean;
   dashboardOption: 'new' | 'existing' | null;
   onChange: (dashboardOption: 'new' | 'existing' | null) => void;
+  hasAttemptedSubmit: boolean;
+  hasSelectedDashboard: boolean;
 }
 
 export function SaveModalDashboardSelector(props: SaveModalDashboardSelectorProps) {
@@ -45,6 +47,8 @@ export function SaveModalDashboardSelector(props: SaveModalDashboardSelectorProp
     dashboardOption,
     onChange,
     copyOnSave,
+    hasAttemptedSubmit,
+    hasSelectedDashboard,
   } = props;
   const isDisabled = !copyOnSave && !!documentId;
 
@@ -81,6 +85,16 @@ export function SaveModalDashboardSelector(props: SaveModalDashboardSelectorProp
                     isDisabled={dashboardOption !== 'existing'}
                     onChange={onSelectDashboard}
                   />
+                  {hasAttemptedSubmit && dashboardOption === 'existing' && !hasSelectedDashboard ? (
+                    <div className="euiFormErrorText euiFormRow__text">
+                      {i18n.translate(
+                        'presentationUtil.saveModalDashboard.existingDashboardRequiredMessage',
+                        {
+                          defaultMessage: 'Dashboard is required',
+                        }
+                      )}
+                    </div>
+                  ) : null}
                 </div>
                 <EuiSpacer size="s" />
               </>

diff --git a/test/functional/page_objects/time_to_visualize_page.ts b/test/functional/page_objects/time_to_visualize_page.ts
@@ -75,8 +75,11 @@ export class TimeToVisualizePageObject extends FtrService {
       await label.click();
 
       if (dashboardId) {
+        await this.testSubjects.waitForEnabled('open-dashboard-picker');
         await this.testSubjects.click('open-dashboard-picker');
         await this.testSubjects.setValue('dashboard-picker-search', dashboardId);
+        await this.common.sleep(150); // wait for input debounce so loading starts
+        await this.testSubjects.waitForEnabled('open-dashboard-picker');
         await this.testSubjects.click(
           `dashboard-picker-option-${dashboardId.replaceAll(' ', '-')}`
         );

diff --git a/x-pack/test/functional/apps/maps/group2/embeddable/add_to_dashboard.js b/x-pack/test/functional/apps/maps/group2/embeddable/add_to_dashboard.js
@@ -21,8 +21,7 @@ export default function ({ getPageObjects, getService }) {
   const testSubjects = getService('testSubjects');
   const security = getService('security');
 
-  // Failing: See https://github.com/elastic/kibana/issues/167320
-  describe.skip('maps add-to-dashboard save flow', () => {
+  describe('maps add-to-dashboard save flow', () => {
     before(async () => {
       await security.testUser.setRoles(
         [