Skip to content

Commit

Permalink
[Security Solution][Alerts] saved query UX changes follow-up (elastic…
Browse files Browse the repository at this point in the history
…#141747)

## Summary
addresses feedback left during code review for elastic#140064

- Saved query checkbox label shows saved query name
- `createSavedQuery` moved to `x-pack/plugins/security_solution/cypress/tasks/api_calls/saved_queries.ts`
- `useGetSavedQuery` moved to `x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/details`
- additional comments and minor code changes

### Before
<img width="762" alt="Screenshot 2022-09-26 at 11 38 05" src="https://user-images.githubusercontent.com/92328789/192256482-2bfc29c9-305a-4ff6-b9cd-b0505e887bd1.png">

### After
<img width="989" alt="Screenshot 2022-09-26 at 11 37 23" src="https://user-images.githubusercontent.com/92328789/192256378-290183b1-44e2-47bb-9d64-c46d0819cff4.png">

(cherry picked from commit f585dd6)
  • Loading branch information
vitaliidm committed Sep 27, 2022
1 parent c0dc453 commit 528dbef
Show file tree
Hide file tree
Showing 12 changed files with 52 additions and 45 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ import {

import { goToRuleDetails, editFirstRule } from '../../tasks/alerts_detection_rules';
import { createTimeline } from '../../tasks/api_calls/timelines';
import { createSavedQuery } from '../../tasks/api_calls/saved_queries';
import { cleanKibana, deleteAlertsAndRules, deleteSavedQueries } from '../../tasks/common';
import { createSavedQuery, deleteSavedQueries } from '../../tasks/api_calls/saved_queries';
import { cleanKibana, deleteAlertsAndRules } from '../../tasks/common';
import {
createAndEnableRule,
fillAboutRuleAndContinue,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,3 +33,20 @@ export const createSavedQuery = (
},
headers: { 'kbn-xsrf': 'cypress-creds' },
});

export const deleteSavedQueries = () => {
const kibanaIndexUrl = `${Cypress.env('ELASTICSEARCH_URL')}/.kibana_\*`;
cy.request('POST', `${kibanaIndexUrl}/_delete_by_query?conflicts=proceed`, {
query: {
bool: {
filter: [
{
match: {
type: 'query',
},
},
],
},
},
});
};
17 changes: 0 additions & 17 deletions x-pack/plugins/security_solution/cypress/tasks/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -179,23 +179,6 @@ export const deleteCases = () => {
});
};

export const deleteSavedQueries = () => {
const kibanaIndexUrl = `${Cypress.env('ELASTICSEARCH_URL')}/.kibana_\*`;
cy.request('POST', `${kibanaIndexUrl}/_delete_by_query?conflicts=proceed`, {
query: {
bool: {
filter: [
{
match: {
type: 'query',
},
},
],
},
},
});
};

export const postDataView = (dataSource: string) => {
cy.request({
method: 'POST',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,3 @@ export const EQL_TIME_INTERVAL_NOT_DEFINED = i18n.translate(
defaultMessage: 'Time intervals are not defined.',
}
);

export const SAVED_QUERY_LOAD_ERROR_TOAST = i18n.translate(
'xpack.securitySolution.hooks.useGetSavedQuery.errorToastMessage',
{
defaultMessage: 'Failed to load the saved query',
}
);
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ const savedQueryToFieldValue = (savedQuery: SavedQuery): FieldValueQueryBar => (
filters: savedQuery.attributes.filters ?? [],
query: savedQuery.attributes.query,
saved_id: savedQuery.id,
title: savedQuery.attributes.title,
});

export const QueryBarDefineRule = ({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -739,8 +739,8 @@ const StepDefineRuleComponent: FC<StepDefineRuleProps> = ({
<>
<EuiSpacer size="s" />
<RuleTypeEuiFormRow
label={i18n.SAVED_QUERY_CHECKBOX_LABEL}
$isVisible={Boolean(formQuery?.saved_id)}
label={i18n.SAVED_QUERY_FORM_ROW_LABEL}
$isVisible={Boolean(formQuery?.saved_id && formQuery?.title)}
fullWidth
>
<CommonUseField
Expand All @@ -750,6 +750,9 @@ const StepDefineRuleComponent: FC<StepDefineRuleProps> = ({
'data-test-subj': 'detectionEngineStepDefineRuleShouldLoadQueryDynamically',
euiFieldProps: {
disabled: isLoading,
label: formQuery?.title
? i18n.getSavedQueryCheckboxLabel(formQuery.title)
: undefined,
},
}}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -629,12 +629,6 @@ export const schema: FormSchema<DefineStepRule> = {
},
shouldLoadQueryDynamically: {
type: FIELD_TYPES.CHECKBOX,
label: i18n.translate(
'xpack.securitySolution.detectionEngine.createRule.stepAboutRule.fieldShouldLoadQueryDynamicallyLabel',
{
defaultMessage: 'Load the saved query dynamically on each rule execution',
}
),
defaultValue: false,
},
};
Original file line number Diff line number Diff line change
Expand Up @@ -78,13 +78,22 @@ export const EQL_QUERY_BAR_LABEL = i18n.translate(
}
);

export const SAVED_QUERY_CHECKBOX_LABEL = i18n.translate(
'xpack.securitySolution.detectionEngine.createRule.stepDefineRule.SavedQueryCheckboxLabel',
export const SAVED_QUERY_FORM_ROW_LABEL = i18n.translate(
'xpack.securitySolution.detectionEngine.createRule.stepDefineRule.SavedQueryFormRowLabel',
{
defaultMessage: 'Saved query',
}
);

export const getSavedQueryCheckboxLabel = (savedQueryName: string) =>
i18n.translate(
'xpack.securitySolution.detectionEngine.createRule.stepDefineRule.fieldShouldLoadQueryDynamicallyLabel',
{
defaultMessage: 'Load saved query "{savedQueryName}" dynamically on each rule execution',
values: { savedQueryName },
}
);

export const THREAT_MATCH_INDEX_HELPER_TEXT = i18n.translate(
'xpack.securitySolution.detectionEngine.createRule.stepDefineRule.threatMatchingIcesHelperDescription',
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ import { ExecutionLogTable } from './execution_log_table/execution_log_table';
import * as detectionI18n from '../../translations';
import * as ruleI18n from '../translations';
import { RuleDetailsContextProvider } from './rule_details_context';
import { useGetSavedQuery } from '../../../../../common/hooks/use_get_saved_query';
import { useGetSavedQuery } from './use_get_saved_query';
import * as i18n from './translations';
import { NeedAdminForUpdateRulesCallOut } from '../../../../components/callouts/need_admin_for_update_callout';
import { MissingPrivilegesCallOut } from '../../../../components/callouts/missing_privileges_callout';
Expand Down Expand Up @@ -307,6 +307,8 @@ const RuleDetailsPageComponent: React.FC<DetectionEngineComponentProps> = ({
const [dataViewOptions, setDataViewOptions] = useState<{ [x: string]: DataViewListItem }>({});

// load saved query only if rule type === 'saved_query', as other rule types still can have saved_id property that is not used
// Rule schema allows to save any rule with saved_id property, but it only used for saved_query rule type
// In future we might look in possibility to restrict rule schema (breaking change!) and remove saved_id from the rest of rules through migration
const savedQueryId = rule?.type === 'saved_query' ? rule?.saved_id : undefined;
const { isSavedQueryLoading, savedQueryBar } = useGetSavedQuery(savedQueryId);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,3 +69,10 @@ export const DELETED_RULE = i18n.translate(
defaultMessage: 'Deleted rule',
}
);

export const SAVED_QUERY_LOAD_ERROR_TOAST = i18n.translate(
'xpack.securitySolution.hooks.useGetSavedQuery.errorToastMessage',
{
defaultMessage: 'Failed to load the saved query',
}
);
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,13 @@

import { useEffect, useMemo } from 'react';

import { useSavedQueryServices } from '../utils/saved_query_services';
import type { DefineStepRule } from '../../detections/pages/detection_engine/rules/types';
import { useSavedQueryServices } from '../../../../../common/utils/saved_query_services';
import type { DefineStepRule } from '../types';

import { useFetch, REQUEST_NAMES } from '../../../../../common/hooks/use_fetch';
import { useAppToasts } from '../../../../../common/hooks/use_app_toasts';

import { useFetch, REQUEST_NAMES } from './use_fetch';
import { SAVED_QUERY_LOAD_ERROR_TOAST } from './translations';
import { useAppToasts } from './use_app_toasts';

export const useGetSavedQuery = (savedQueryId: string | undefined) => {
const savedQueryServices = useSavedQueryServices();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,13 +97,10 @@ export const getFilter = async ({
index,
exceptionFilter,
});
} else if (savedId && index != null) {
// if savedId present and we ending up here, then saved query failed to be fetched
// and we also didn't fall back to saved in rule query
throw Error(`Failed to fetch saved query. "${err.message}"`);
} else {
// user did not give any additional fall back mechanism for generating a rule
// rethrow error for activity monitoring
err.message = `Failed to fetch saved query. "${err.message}"`;
throw err;
}
}
Expand Down

0 comments on commit 528dbef

Please sign in to comment.