Skip to content

Commit

Permalink
[SecuritySolution][Onboarding] Catch completion check failure (#196389)
Browse files Browse the repository at this point in the history
## Summary

Fixes: #196091 |
~#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:

<img width="1761" alt="Screenshot 2024-10-17 at 15 02 42"
src="https://github.com/user-attachments/assets/07fb317e-57d6-4980-aae3-7eb2d0fce12a">

Then add the index privilege:
<img width="2208" alt="Screenshot 2024-10-17 at 15 07 36"
src="https://github.com/user-attachments/assets/bb879964-e31b-4ee3-8eb3-dff0381be287">

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:
<img width="779" alt="Screenshot 2024-10-17 at 15 04 28"
src="https://github.com/user-attachments/assets/098b0c90-30a9-4e82-ad16-10d2cd64a9cc">

<img width="1250" alt="Screenshot 2024-10-17 at 16 40 18"
src="https://github.com/user-attachments/assets/5e677d5a-32b8-4cea-b240-207a3f055f9c">

### 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 10ec204)
  • Loading branch information
angorayc committed Oct 21, 2024
1 parent 1fcc6c4 commit eb1d4a2
Show file tree
Hide file tree
Showing 6 changed files with 181 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ export const assistantCardConfig: OnboardingCardConfig<AssistantCardMetadata> =
)
),
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',
};
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,15 @@ describe('IntegrationsCard', () => {
);
expect(getByTestId('loadingInstalledIntegrations')).toBeInTheDocument();
});

it('renders the content', () => {
const { queryByTestId } = render(
<IntegrationsCard
{...props}
checkCompleteMetadata={{ installedIntegrationsCount: 1, isAgentRequired: false }}
/>
);
expect(queryByTestId('loadingInstalledIntegrations')).not.toBeInTheDocument();
expect(queryByTestId('integrationsCardGridTabs')).toBeInTheDocument();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand All @@ -27,6 +28,11 @@ describe('checkIntegrationsCardComplete', () => {
search: mockSearch,
},
},
notifications: {
toasts: {
addError: jest.fn(),
},
},
} as unknown as StartServices;

beforeEach(() => {
Expand All @@ -38,7 +44,7 @@ describe('checkIntegrationsCardComplete', () => {
items: [],
});

(lastValueFrom as jest.Mock).mockResolvedValue({
mockLastValueFrom.mockResolvedValue({
rawResponse: {
hits: { total: 0 },
},
Expand All @@ -60,7 +66,7 @@ describe('checkIntegrationsCardComplete', () => {
items: [{ status: installationStatuses.Installed }],
});

(lastValueFrom as jest.Mock).mockResolvedValue({
mockLastValueFrom.mockResolvedValue({
rawResponse: {
hits: { total: 0 },
},
Expand All @@ -86,7 +92,7 @@ describe('checkIntegrationsCardComplete', () => {
],
});

(lastValueFrom as jest.Mock).mockResolvedValue({
mockLastValueFrom.mockResolvedValue({
rawResponse: {
hits: { total: 1 },
},
Expand All @@ -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,
},
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,37 @@ import type { IntegrationCardMetadata } from './types';
export const checkIntegrationsCardComplete: OnboardingCardCheckComplete<
IntegrationCardMetadata
> = async (services: StartServices) => {
const packageData = await services.http.get<GetPackagesResponse>(
EPM_API_ROUTES.INSTALL_BY_UPLOAD_PATTERN,
{
const packageData = await services.http
.get<GetPackagesResponse>(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) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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 = [
{
Expand All @@ -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<typeof useCompletedCards>
>;
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[],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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]);

Expand Down

0 comments on commit eb1d4a2

Please sign in to comment.