address review feedback

src/plugins/guided_onboarding/public/components/guide_panel.test.tsx

@@ -10,7 +10,7 @@ import { act } from 'react-dom/test-utils';
 import React from 'react';
 
 import { applicationServiceMock } from '@kbn/core-application-browser-mocks';
-import { httpServiceMock, notificationServiceMock } from '@kbn/core/public/mocks';
+import { httpServiceMock } from '@kbn/core/public/mocks';
 import { HttpSetup } from '@kbn/core/public';
 
 import { guidesConfig } from '../constants/guides_config';
@@ -20,7 +20,6 @@ import { GuidePanel } from './guide_panel';
 import { registerTestBed, TestBed } from '@kbn/test-jest-helpers';
 
 const applicationMock = applicationServiceMock.createStartContract();
-const notificationsMock = notificationServiceMock.createSetupContract();
 
 const mockActiveSearchGuideState: GuideState = {
   guideId: 'search',
@@ -43,9 +42,7 @@ const mockActiveSearchGuideState: GuideState = {
 };
 
 const getGuidePanel = () => () => {
-  return (
-    <GuidePanel application={applicationMock} api={apiService} notifications={notificationsMock} />
-  );
+  return <GuidePanel application={applicationMock} api={apiService} />;
 };
 
 describe('Guided setup', () => {
@@ -58,7 +55,6 @@ describe('Guided setup', () => {
     httpClient.get.mockResolvedValue({
       state: [],
     });
-    httpClient.delete.mockResolvedValue({});
     apiService.setup(httpClient);
 
     await act(async () => {

src/plugins/guided_onboarding/public/components/guide_panel.tsx

@@ -29,7 +29,7 @@ import {
 import { ApplicationStart } from '@kbn/core-application-browser';
 import { i18n } from '@kbn/i18n';
 import { FormattedMessage } from '@kbn/i18n-react';
-import type { NotificationsSetup } from '@kbn/core/public';
+
 import { guidesConfig } from '../constants/guides_config';
 import type { GuideState, GuideStepIds } from '../../common/types';
 import type { GuideConfig, StepConfig } from '../types';
@@ -43,7 +43,6 @@ import { getGuidePanelStyles } from './guide_panel.styles';
 interface GuidePanelProps {
   api: ApiService;
   application: ApplicationStart;
-  notifications: NotificationsSetup;
 }
 
 const getConfig = (state?: GuideState): GuideConfig | undefined => {
@@ -84,7 +83,7 @@ const getProgress = (state?: GuideState): number => {
   return 0;
 };
 
-export const GuidePanel = ({ api, application, notifications }: GuidePanelProps) => {
+export const GuidePanel = ({ api, application }: GuidePanelProps) => {
   const { euiTheme } = useEuiTheme();
   const [isGuideOpen, setIsGuideOpen] = useState(false);
   const [isQuitGuideModalOpen, setIsQuitGuideModalOpen] = useState(false);
@@ -339,11 +338,7 @@ export const GuidePanel = ({ api, application, notifications }: GuidePanelProps)
       )}
 
       {isQuitGuideModalOpen && (
-        <QuitGuideModal
-          closeModal={closeQuitGuideModal}
-          currentGuide={guideState!.guideId}
-          notifications={notifications}
-        />
+        <QuitGuideModal closeModal={closeQuitGuideModal} currentGuide={guideState!} />
       )}
     </>
   );

src/plugins/guided_onboarding/public/components/quit_guide_modal.tsx

@@ -9,43 +9,28 @@ import React, { useState } from 'react';
 
 import { EuiText, EuiConfirmModal } from '@elastic/eui';
 import { i18n } from '@kbn/i18n';
-import type { NotificationsSetup } from '@kbn/core-notifications-browser';
-import { GuideId } from '../../common/types';
+import { GuideState } from '../../common/types';
 import { apiService } from '../services/api';
 
 interface QuitGuideModalProps {
   closeModal: () => void;
-  currentGuide: GuideId;
-  notifications: NotificationsSetup;
+  currentGuide: GuideState;
 }
 
-export const QuitGuideModal = ({
-  closeModal,
-  currentGuide,
-  notifications,
-}: QuitGuideModalProps) => {
+export const QuitGuideModal = ({ closeModal, currentGuide }: QuitGuideModalProps) => {
   const [isDeleting, setIsDeleting] = useState<boolean>(false);
 
   const deleteGuide = async () => {
     setIsDeleting(true);
-    const { error } = await apiService.deleteGuide(currentGuide);
-
-    if (error) {
-      setIsDeleting(false);
-      notifications.toasts.addError(error, {
-        title: i18n.translate('guidedOnboarding.quitGuideModal.errorToastTitle', {
-          defaultMessage: 'There was an error quitting the guide. Please try again.',
-        }),
-      });
-    }
+    await apiService.deactivateGuide(currentGuide);
     closeModal();
   };
 
   return (
     <EuiConfirmModal
       maxWidth={448}
       title={i18n.translate('guidedOnboarding.quitGuideModal.modalTitle', {
-        defaultMessage: 'Quit this guide and discard progress?',
+        defaultMessage: 'Quit this guide?',
       })}
       onCancel={closeModal}
       onConfirm={deleteGuide}

src/plugins/guided_onboarding/public/plugin.tsx

@@ -17,7 +17,6 @@ import {
   CoreTheme,
   ApplicationStart,
   PluginInitializerContext,
-  NotificationsSetup,
 } from '@kbn/core/public';
 
 import { KibanaThemeProvider } from '@kbn/kibana-react-plugin/public';
@@ -43,7 +42,7 @@ export class GuidedOnboardingPlugin
       return {};
     }
 
-    const { chrome, http, theme, application, notifications } = core;
+    const { chrome, http, theme, application } = core;
 
     // Initialize services
     apiService.setup(http);
@@ -56,7 +55,6 @@ export class GuidedOnboardingPlugin
           theme$: theme.theme$,
           api: apiService,
           application,
-          notifications,
         }),
     });
 
@@ -73,18 +71,16 @@ export class GuidedOnboardingPlugin
     theme$,
     api,
     application,
-    notifications,
   }: {
     targetDomElement: HTMLElement;
     theme$: Rx.Observable<CoreTheme>;
     api: ApiService;
     application: ApplicationStart;
-    notifications: NotificationsSetup;
   }) {
     ReactDOM.render(
       <KibanaThemeProvider theme$={theme$}>
         <I18nProvider>
-          <GuidePanel api={api} application={application} notifications={notifications} />
+          <GuidePanel api={api} application={application} />
         </I18nProvider>
       </KibanaThemeProvider>,
       targetDomElement

src/plugins/guided_onboarding/public/services/api.test.ts

@@ -72,11 +72,17 @@ describe('GuidedOnboarding ApiService', () => {
     });
   });
 
-  describe('deleteGuide', () => {
-    it('sends a request to the delete API', async () => {
-      await apiService.deleteGuide(searchGuide);
-      expect(httpClient.delete).toHaveBeenCalledTimes(1);
-      expect(httpClient.delete).toHaveBeenCalledWith(`${API_BASE_PATH}/state/${searchGuide}`);
+  describe('deactivateGuide', () => {
+    it('deactivates an existing guide', async () => {
+      await apiService.deactivateGuide(searchAddDataActiveState);
+
+      expect(httpClient.put).toHaveBeenCalledTimes(1);
+      expect(httpClient.put).toHaveBeenCalledWith(`${API_BASE_PATH}/state`, {
+        body: JSON.stringify({
+          ...searchAddDataActiveState,
+          isActive: false,
+        }),
+      });
     });
   });
 

src/plugins/guided_onboarding/public/services/api.ts

@@ -21,7 +21,6 @@ import type { GuideState, GuideId, GuideStep, GuideStepIds } from '../../common/
 
 export class ApiService implements GuidedOnboardingApi {
   private client: HttpSetup | undefined;
-  private isGuideAbandoned: boolean = false;
   private onboardingGuideState$!: BehaviorSubject<GuideState | undefined>;
   public isGuidePanelOpen$: BehaviorSubject<boolean> = new BehaviorSubject<boolean>(false);
 
@@ -39,7 +38,7 @@ export class ApiService implements GuidedOnboardingApi {
     // TODO add error handling if this.client has not been initialized or request fails
     return this.onboardingGuideState$.pipe(
       concatMap((state) =>
-        this.isGuideAbandoned === false && state === undefined
+        state === undefined
           ? from(
               this.client!.get<{ state: GuideState[] }>(`${API_BASE_PATH}/state`, {
                 query: {
@@ -77,34 +76,6 @@ export class ApiService implements GuidedOnboardingApi {
     }
   }
 
-  /**
-   * Async operation to delete a guide
-   * On the server, the SO is deleted for the selected guide ID
-   * This is used for the "Quit guide" functionality on the dropdown panel
-   * @param {GuideId} guideId the id of the guide (one of search, observability, security)
-   * @return {Promise} a promise with the response or error
-   */
-  public async deleteGuide(
-    guideId: GuideId
-  ): Promise<{ response?: { deletedGuide: GuideId }; error?: Error }> {
-    if (!this.client) {
-      throw new Error('ApiService has not be initialized.');
-    }
-
-    try {
-      const response = await this.client.delete<{ deletedGuide: GuideId }>(
-        `${API_BASE_PATH}/state/${guideId}`
-      );
-      // Mark the guide as abandoned
-      this.isGuideAbandoned = true;
-      // Reset the guide state
-      this.onboardingGuideState$.next(undefined);
-      return { response };
-    } catch (error) {
-      return { error };
-    }
-  }
-
   /**
    * Updates the SO with the updated guide state and refreshes the observables
    * This is largely used internally and for tests
@@ -124,7 +95,8 @@ export class ApiService implements GuidedOnboardingApi {
       const response = await this.client.put<{ state: GuideState }>(`${API_BASE_PATH}/state`, {
         body: JSON.stringify(newState),
       });
-      this.onboardingGuideState$.next(newState);
+      // If the guide has been deactivated, we return undefined
+      this.onboardingGuideState$.next(newState.isActive ? newState : undefined);
       this.isGuidePanelOpen$.next(panelState);
       return response;
     } catch (error) {
@@ -181,6 +153,22 @@ export class ApiService implements GuidedOnboardingApi {
     }
   }
 
+  /**
+   * Marks a guide as inactive
+   * This is useful for the dropdown panel, when a user quits a guide
+   * @param {GuideState} guide (optional) the selected guide state, if it exists (i.e., if a user is continuing a guide)
+   * @return {Promise} a promise with the updated guide state
+   */
+  public async deactivateGuide(guide: GuideState): Promise<{ state: GuideState } | undefined> {
+    return await this.updateGuideState(
+      {
+        ...guide,
+        isActive: false,
+      },
+      false
+    );
+  }
+
   /**
    * Completes a guide
    * Updates the overall guide status to 'complete', and marks it as inactive

src/plugins/guided_onboarding/server/routes/index.ts

@@ -161,42 +161,4 @@ export function defineRoutes(router: IRouter) {
       }
     }
   );
-
-  // Delete SO for selected guide
-  router.delete(
-    {
-      path: `${API_BASE_PATH}/state/{guideId}`,
-      validate: {
-        params: schema.object({
-          guideId: schema.string(),
-        }),
-      },
-    },
-    async (context, request, response) => {
-      const coreContext = await context.core;
-      const { guideId } = request.params;
-      const soClient = coreContext.savedObjects.client as SavedObjectsClient;
-
-      const existingGuideSO = await findGuideById(soClient, guideId);
-
-      if (existingGuideSO.total > 0) {
-        const existingGuide = existingGuideSO.saved_objects[0];
-
-        await soClient.delete(guidedSetupSavedObjectsType, existingGuide.id);
-
-        return response.ok({
-          body: {
-            deletedGuide: guideId,
-          },
-        });
-      } else {
-        // In the case that the SO doesn't exist (unlikely), return successful response
-        return response.ok({
-          body: {
-            deletedGuide: guideId,
-          },
-        });
-      }
-    }
-  );
 }