From dc79bcea9ea7c93c5ba09d41b3774ace514cbfaa Mon Sep 17 00:00:00 2001 From: Alison Goryachev Date: Mon, 10 Oct 2022 09:31:55 -0400 Subject: [PATCH] address review feedback --- .../public/components/guide_panel.test.tsx | 8 +-- .../public/components/guide_panel.tsx | 11 ++-- .../public/components/quit_guide_modal.tsx | 25 ++-------- .../guided_onboarding/public/plugin.tsx | 8 +-- .../public/services/api.test.ts | 16 ++++-- .../guided_onboarding/public/services/api.ts | 50 +++++++------------ .../guided_onboarding/server/routes/index.ts | 38 -------------- 7 files changed, 42 insertions(+), 114 deletions(-) diff --git a/src/plugins/guided_onboarding/public/components/guide_panel.test.tsx b/src/plugins/guided_onboarding/public/components/guide_panel.test.tsx index 1bee057beb4a..5bd846aebbde 100644 --- a/src/plugins/guided_onboarding/public/components/guide_panel.test.tsx +++ b/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 ( - - ); + return ; }; describe('Guided setup', () => { @@ -58,7 +55,6 @@ describe('Guided setup', () => { httpClient.get.mockResolvedValue({ state: [], }); - httpClient.delete.mockResolvedValue({}); apiService.setup(httpClient); await act(async () => { diff --git a/src/plugins/guided_onboarding/public/components/guide_panel.tsx b/src/plugins/guided_onboarding/public/components/guide_panel.tsx index d59e424cab02..7c122492d84a 100644 --- a/src/plugins/guided_onboarding/public/components/guide_panel.tsx +++ b/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 && ( - + )} ); diff --git a/src/plugins/guided_onboarding/public/components/quit_guide_modal.tsx b/src/plugins/guided_onboarding/public/components/quit_guide_modal.tsx index 4a7beeb04851..a7a7e34c311b 100644 --- a/src/plugins/guided_onboarding/public/components/quit_guide_modal.tsx +++ b/src/plugins/guided_onboarding/public/components/quit_guide_modal.tsx @@ -9,35 +9,20 @@ 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(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(); }; @@ -45,7 +30,7 @@ export const QuitGuideModal = ({ ; api: ApiService; application: ApplicationStart; - notifications: NotificationsSetup; }) { ReactDOM.render( - + , targetDomElement diff --git a/src/plugins/guided_onboarding/public/services/api.test.ts b/src/plugins/guided_onboarding/public/services/api.test.ts index f116a6af153e..5deb3d50987f 100644 --- a/src/plugins/guided_onboarding/public/services/api.test.ts +++ b/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, + }), + }); }); }); diff --git a/src/plugins/guided_onboarding/public/services/api.ts b/src/plugins/guided_onboarding/public/services/api.ts index 09510ffa0b50..7c970717be5f 100644 --- a/src/plugins/guided_onboarding/public/services/api.ts +++ b/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; public isGuidePanelOpen$: BehaviorSubject = new BehaviorSubject(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 diff --git a/src/plugins/guided_onboarding/server/routes/index.ts b/src/plugins/guided_onboarding/server/routes/index.ts index a8b60dc0500e..adc65d0bf686 100755 --- a/src/plugins/guided_onboarding/server/routes/index.ts +++ b/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, - }, - }); - } - } - ); }