From eb1d4a2328053c361eea6ab0c301693d6d69a671 Mon Sep 17 00:00:00 2001 From: Angela Chuang <6295984+angorayc@users.noreply.github.com> Date: Mon, 21 Oct 2024 11:56:50 +0100 Subject: [PATCH] [SecuritySolution][Onboarding] Catch completion check failure (#196389) ## Summary Fixes: https://github.com/elastic/kibana/issues/196091 | ~https://github.com/elastic/kibana/issues/196649~ 1. Add the error handling when an error occurred during check complete. 2. Update the AI assistant capabilities to `[['securitySolutionAssistant.ai-assistant', 'actions.show']]` **Steps to verify:** https://p.elstc.co/paste/+6OYqx41#tZxjvqXgQJ2uRlCSqZH8ADMEAvR1+qnXe-5kEbt+bro Login with the user without indices privilege. It should display the tab content without the completion information. When completion check failed - it should display and error `toast`, regard the card as `incomplete` and `show the content`: https://github.com/user-attachments/assets/30b1654e-99cc-4582-8beb-c4a5fb005e6f AI assistant should not show connectors options as `add integration` is regarded as incomplete: Screenshot 2024-10-17 at 15 02 42 Then add the index privilege: Screenshot 2024-10-17 at 15 07 36 When completion check success: it should display the completion results: https://github.com/user-attachments/assets/2a27a042-c634-4f44-bfd0-2ae503f396a2 Set `actions and connectors` to read only: Screenshot 2024-10-17 at 15 04 28 Screenshot 2024-10-17 at 16 40 18 ### Checklist Delete any items that are not applicable to this PR. - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios (cherry picked from commit 10ec204128776930c48376a848fe20b1301569f9) --- .../onboarding_body/cards/assistant/index.ts | 4 +- .../integrations/integrations_card.test.tsx | 11 +++ .../integrations_check_complete.test.ts | 51 ++++++++++++- .../integrations_check_complete.ts | 31 ++++++-- .../hooks/use_completed_cards.test.ts | 72 +++++++++++++++++++ .../hooks/use_completed_cards.ts | 28 ++++++-- 6 files changed, 181 insertions(+), 16 deletions(-) diff --git a/x-pack/plugins/security_solution/public/onboarding/components/onboarding_body/cards/assistant/index.ts b/x-pack/plugins/security_solution/public/onboarding/components/onboarding_body/cards/assistant/index.ts index bf4195b814590..27deda4190f2e 100644 --- a/x-pack/plugins/security_solution/public/onboarding/components/onboarding_body/cards/assistant/index.ts +++ b/x-pack/plugins/security_solution/public/onboarding/components/onboarding_body/cards/assistant/index.ts @@ -25,6 +25,8 @@ export const assistantCardConfig: OnboardingCardConfig = ) ), checkComplete: checkAssistantCardComplete, - capabilities: 'securitySolutionAssistant.ai-assistant', + // Both capabilities are needed for this card, so we should use a double array to create an AND conditional + // (a single array would create an OR conditional between them) + capabilities: [['securitySolutionAssistant.ai-assistant', 'actions.show']], licenseType: 'enterprise', }; diff --git a/x-pack/plugins/security_solution/public/onboarding/components/onboarding_body/cards/integrations/integrations_card.test.tsx b/x-pack/plugins/security_solution/public/onboarding/components/onboarding_body/cards/integrations/integrations_card.test.tsx index 3f79745182c5a..296d5391fd611 100644 --- a/x-pack/plugins/security_solution/public/onboarding/components/onboarding_body/cards/integrations/integrations_card.test.tsx +++ b/x-pack/plugins/security_solution/public/onboarding/components/onboarding_body/cards/integrations/integrations_card.test.tsx @@ -28,4 +28,15 @@ describe('IntegrationsCard', () => { ); expect(getByTestId('loadingInstalledIntegrations')).toBeInTheDocument(); }); + + it('renders the content', () => { + const { queryByTestId } = render( + + ); + expect(queryByTestId('loadingInstalledIntegrations')).not.toBeInTheDocument(); + expect(queryByTestId('integrationsCardGridTabs')).toBeInTheDocument(); + }); }); diff --git a/x-pack/plugins/security_solution/public/onboarding/components/onboarding_body/cards/integrations/integrations_check_complete.test.ts b/x-pack/plugins/security_solution/public/onboarding/components/onboarding_body/cards/integrations/integrations_check_complete.test.ts index 3dd19d8868390..961f1981291b8 100644 --- a/x-pack/plugins/security_solution/public/onboarding/components/onboarding_body/cards/integrations/integrations_check_complete.test.ts +++ b/x-pack/plugins/security_solution/public/onboarding/components/onboarding_body/cards/integrations/integrations_check_complete.test.ts @@ -16,6 +16,7 @@ jest.mock('rxjs', () => ({ })); describe('checkIntegrationsCardComplete', () => { + const mockLastValueFrom = lastValueFrom as jest.Mock; const mockHttpGet: jest.Mock = jest.fn(); const mockSearch: jest.Mock = jest.fn(); const mockService = { @@ -27,6 +28,11 @@ describe('checkIntegrationsCardComplete', () => { search: mockSearch, }, }, + notifications: { + toasts: { + addError: jest.fn(), + }, + }, } as unknown as StartServices; beforeEach(() => { @@ -38,7 +44,7 @@ describe('checkIntegrationsCardComplete', () => { items: [], }); - (lastValueFrom as jest.Mock).mockResolvedValue({ + mockLastValueFrom.mockResolvedValue({ rawResponse: { hits: { total: 0 }, }, @@ -60,7 +66,7 @@ describe('checkIntegrationsCardComplete', () => { items: [{ status: installationStatuses.Installed }], }); - (lastValueFrom as jest.Mock).mockResolvedValue({ + mockLastValueFrom.mockResolvedValue({ rawResponse: { hits: { total: 0 }, }, @@ -86,7 +92,7 @@ describe('checkIntegrationsCardComplete', () => { ], }); - (lastValueFrom as jest.Mock).mockResolvedValue({ + mockLastValueFrom.mockResolvedValue({ rawResponse: { hits: { total: 1 }, }, @@ -103,4 +109,43 @@ describe('checkIntegrationsCardComplete', () => { }, }); }); + + it('renders an error toast when fetching integrations data fails', async () => { + const err = new Error('Failed to fetch integrations data'); + mockHttpGet.mockRejectedValue(err); + + const res = await checkIntegrationsCardComplete(mockService); + + expect(mockService.notifications.toasts.addError).toHaveBeenCalledWith(err, { + title: 'Error fetching integrations data', + }); + expect(res).toEqual({ + isComplete: false, + metadata: { + installedIntegrationsCount: 0, + isAgentRequired: false, + }, + }); + }); + + it('renders an error toast when fetching agents data fails', async () => { + const err = new Error('Failed to fetch agents data'); + mockLastValueFrom.mockRejectedValue(err); + + const res = await checkIntegrationsCardComplete(mockService); + + expect(mockService.notifications.toasts.addError).toHaveBeenCalledWith( + new Error('Failed to fetch agents data'), + { + title: 'Error fetching agents data', + } + ); + expect(res).toEqual({ + isComplete: false, + metadata: { + installedIntegrationsCount: 0, + isAgentRequired: false, + }, + }); + }); }); diff --git a/x-pack/plugins/security_solution/public/onboarding/components/onboarding_body/cards/integrations/integrations_check_complete.ts b/x-pack/plugins/security_solution/public/onboarding/components/onboarding_body/cards/integrations/integrations_check_complete.ts index 912b81bddf3fb..d4193dd8b9ded 100644 --- a/x-pack/plugins/security_solution/public/onboarding/components/onboarding_body/cards/integrations/integrations_check_complete.ts +++ b/x-pack/plugins/security_solution/public/onboarding/components/onboarding_body/cards/integrations/integrations_check_complete.ts @@ -17,18 +17,37 @@ import type { IntegrationCardMetadata } from './types'; export const checkIntegrationsCardComplete: OnboardingCardCheckComplete< IntegrationCardMetadata > = async (services: StartServices) => { - const packageData = await services.http.get( - EPM_API_ROUTES.INSTALL_BY_UPLOAD_PATTERN, - { + const packageData = await services.http + .get(EPM_API_ROUTES.INSTALL_BY_UPLOAD_PATTERN, { version: '2023-10-31', - } - ); + }) + .catch((err: Error) => { + services.notifications.toasts.addError(err, { + title: i18n.translate( + 'xpack.securitySolution.onboarding.integrationsCard.checkComplete.fetchIntegrations.errorTitle', + { + defaultMessage: 'Error fetching integrations data', + } + ), + }); + return { items: [] }; + }); const agentsData = await lastValueFrom( services.data.search.search({ params: { index: AGENT_INDEX, body: { size: 1 } }, }) - ); + ).catch((err: Error) => { + services.notifications.toasts.addError(err, { + title: i18n.translate( + 'xpack.securitySolution.onboarding.integrationsCard.checkComplete.fetchAgents.errorTitle', + { + defaultMessage: 'Error fetching agents data', + } + ), + }); + return { rawResponse: { hits: { total: 0 } } }; + }); const installed = packageData?.items?.filter( (pkg) => diff --git a/x-pack/plugins/security_solution/public/onboarding/components/onboarding_body/hooks/use_completed_cards.test.ts b/x-pack/plugins/security_solution/public/onboarding/components/onboarding_body/hooks/use_completed_cards.test.ts index 31c440e8f1415..2c9fcd573f0d6 100644 --- a/x-pack/plugins/security_solution/public/onboarding/components/onboarding_body/hooks/use_completed_cards.test.ts +++ b/x-pack/plugins/security_solution/public/onboarding/components/onboarding_body/hooks/use_completed_cards.test.ts @@ -11,9 +11,11 @@ import { useCompletedCards } from './use_completed_cards'; import type { OnboardingGroupConfig } from '../../../types'; import type { OnboardingCardId } from '../../../constants'; import { mockReportCardComplete } from '../../__mocks__/onboarding_context_mocks'; +import { useKibana } from '../../../../common/lib/kibana'; const defaultStoredCompletedCardIds: OnboardingCardId[] = []; const mockSetStoredCompletedCardIds = jest.fn(); +const mockUseKibana = useKibana as jest.Mock; const mockUseStoredCompletedCardIds = jest.fn(() => [ defaultStoredCompletedCardIds, mockSetStoredCompletedCardIds, @@ -24,6 +26,15 @@ jest.mock('../../../hooks/use_stored_state', () => ({ })); jest.mock('../../onboarding_context'); +jest.mock('../../../../common/lib/kibana', () => { + const original = jest.requireActual('../../../../common/lib/kibana'); + return { + ...original, + useKibana: jest.fn().mockReturnValue({ + services: { notifications: { toasts: { addError: jest.fn() } } }, + }), + }; +}); const cardComplete = { id: 'card-completed' as OnboardingCardId, @@ -62,6 +73,13 @@ const cardMetadata = { .fn() .mockResolvedValue({ isComplete: true, metadata: { custom: 'metadata' } }), }; +const mockAddError = jest.fn(); +const mockError = new Error('Failed to check complete'); +const cardCheckCompleteFailed = { + id: 'card-failed' as OnboardingCardId, + title: 'card failed', + checkComplete: jest.fn().mockRejectedValue(mockError), +}; const mockCardsGroupConfig = [ { @@ -74,11 +92,65 @@ const mockCardsGroupConfig = [ }, ] as unknown as OnboardingGroupConfig[]; +const mockFailureCardsGroupConfig = [ + { + title: 'Group 1', + cards: [cardCheckCompleteFailed], + }, +] as unknown as OnboardingGroupConfig[]; + describe('useCompletedCards Hook', () => { beforeEach(() => { jest.clearAllMocks(); }); + describe('when checkComplete functions are rejected', () => { + let renderResult: RenderHookResult< + OnboardingGroupConfig[], + ReturnType + >; + beforeEach(async () => { + mockUseKibana.mockReturnValue({ + services: { notifications: { toasts: { addError: mockAddError } } }, + }); + renderResult = renderHook(useCompletedCards, { initialProps: mockFailureCardsGroupConfig }); + await act(async () => { + await waitFor(() => { + expect(mockSetStoredCompletedCardIds).toHaveBeenCalledTimes(0); // number of completed cards + }); + }); + }); + + describe('when a the auto check is called', () => { + beforeEach(async () => { + jest.clearAllMocks(); + await act(async () => { + renderResult.result.current.checkCardComplete(cardCheckCompleteFailed.id); + }); + }); + + it('should not set the completed card ids', async () => { + expect(mockSetStoredCompletedCardIds).not.toHaveBeenCalled(); + }); + + it('should return the correct completed state', () => { + expect(renderResult.result.current.isCardComplete(cardCheckCompleteFailed.id)).toEqual( + false + ); + }); + + it('should show an error toast', () => { + expect(mockAddError).toHaveBeenCalledWith(mockError, { + title: cardCheckCompleteFailed.title, + }); + }); + + it('should not report the completed card', async () => { + expect(mockReportCardComplete).not.toHaveBeenCalled(); + }); + }); + }); + describe('when checkComplete functions are resolved', () => { let renderResult: RenderHookResult< OnboardingGroupConfig[], diff --git a/x-pack/plugins/security_solution/public/onboarding/components/onboarding_body/hooks/use_completed_cards.ts b/x-pack/plugins/security_solution/public/onboarding/components/onboarding_body/hooks/use_completed_cards.ts index 6d8b22c504be9..34092bf2d5eec 100644 --- a/x-pack/plugins/security_solution/public/onboarding/components/onboarding_body/hooks/use_completed_cards.ts +++ b/x-pack/plugins/security_solution/public/onboarding/components/onboarding_body/hooks/use_completed_cards.ts @@ -114,9 +114,17 @@ export const useCompletedCards = (cardsGroupConfig: OnboardingGroupConfig[]) => const cardConfig = cardsWithAutoCheck.find(({ id }) => id === cardId); if (cardConfig) { - cardConfig.checkComplete?.(services).then((checkCompleteResult) => { - processCardCheckCompleteResult(cardId, checkCompleteResult); - }); + cardConfig + .checkComplete?.(services) + .catch((err: Error) => { + services.notifications.toasts.addError(err, { title: cardConfig.title }); + return { + isComplete: false, + }; + }) + .then((checkCompleteResult) => { + processCardCheckCompleteResult(cardId, checkCompleteResult); + }); } }, [cardsWithAutoCheck, processCardCheckCompleteResult, services] @@ -129,9 +137,17 @@ export const useCompletedCards = (cardsGroupConfig: OnboardingGroupConfig[]) => } autoCheckCompletedRef.current = true; cardsWithAutoCheck.map((card) => - card.checkComplete?.(services).then((checkCompleteResult) => { - processCardCheckCompleteResult(card.id, checkCompleteResult); - }) + card + .checkComplete?.(services) + .catch((err: Error) => { + services.notifications.toasts.addError(err, { title: card.title }); + return { + isComplete: false, + }; + }) + .then((checkCompleteResult) => { + processCardCheckCompleteResult(card.id, checkCompleteResult); + }) ); }, [cardsWithAutoCheck, processCardCheckCompleteResult, services]);