[Security Solution][Alerts] saved query UX changes follow-up (elastic…

…#141747) (elastic#141962) ## 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) Co-authored-by: Vitalii Dmyterko <[email protected]>
kibanamachine · Sep 27, 2022 · 72b5307 · 72b5307
1 parent 9f5113f
commit 72b5307
Show file tree

Hide file tree

Showing 12 changed files with 52 additions and 45 deletions.
diff --git a/x-pack/plugins/security_solution/cypress/e2e/detection_rules/custom_saved_query_rule.cy.ts b/x-pack/plugins/security_solution/cypress/e2e/detection_rules/custom_saved_query_rule.cy.ts
@@ -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,

diff --git a/x-pack/plugins/security_solution/cypress/tasks/api_calls/saved_queries.ts b/x-pack/plugins/security_solution/cypress/tasks/api_calls/saved_queries.ts
@@ -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',
+            },
+          },
+        ],
+      },
+    },
+  });
+};
diff --git a/x-pack/plugins/security_solution/cypress/tasks/common.ts b/x-pack/plugins/security_solution/cypress/tasks/common.ts
@@ -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',

diff --git a/x-pack/plugins/security_solution/public/common/hooks/translations.ts b/x-pack/plugins/security_solution/public/common/hooks/translations.ts
@@ -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',
-  }
-);
diff --git a/x-pack/plugins/security_solution/public/detections/components/rules/query_bar/index.tsx b/x-pack/plugins/security_solution/public/detections/components/rules/query_bar/index.tsx
@@ -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 = ({

diff --git a/...k/plugins/security_solution/public/detections/components/rules/step_define_rule/index.tsx b/...k/plugins/security_solution/public/detections/components/rules/step_define_rule/index.tsx
@@ -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
@@ -750,6 +750,9 @@ const StepDefineRuleComponent: FC<StepDefineRuleProps> = ({
                     'data-test-subj': 'detectionEngineStepDefineRuleShouldLoadQueryDynamically',
                     euiFieldProps: {
                       disabled: isLoading,
+                      label: formQuery?.title
+                        ? i18n.getSavedQueryCheckboxLabel(formQuery.title)
+                        : undefined,
                     },
                   }}
                 />

diff --git a/.../plugins/security_solution/public/detections/components/rules/step_define_rule/schema.tsx b/.../plugins/security_solution/public/detections/components/rules/step_define_rule/schema.tsx
@@ -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,
   },
 };
diff --git a/...ns/security_solution/public/detections/components/rules/step_define_rule/translations.tsx b/...ns/security_solution/public/detections/components/rules/step_define_rule/translations.tsx
@@ -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',
   {

diff --git a/...lugins/security_solution/public/detections/pages/detection_engine/rules/details/index.tsx b/...lugins/security_solution/public/detections/pages/detection_engine/rules/details/index.tsx
@@ -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';
@@ -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);
 

diff --git a/.../security_solution/public/detections/pages/detection_engine/rules/details/translations.ts b/.../security_solution/public/detections/pages/detection_engine/rules/details/translations.ts
@@ -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',
+  }
+);
diff --git a/...ublic/common/hooks/use_get_saved_query.ts → ...gine/rules/details/use_get_saved_query.ts b/...ublic/common/hooks/use_get_saved_query.ts → ...gine/rules/details/use_get_saved_query.ts
@@ -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();

diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/get_filter.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/get_filter.ts
@@ -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;
         }
       }