Review feedback

elastic · Dec 18, 2024 · 13c18b1 · 13c18b1
1 parent 0be6f58
commit 13c18b1
Show file tree

Hide file tree

Showing 14 changed files with 77 additions and 130 deletions.
diff --git a/x-pack/solutions/security/plugins/security_solution/common/siem_migrations/constants.ts b/x-pack/solutions/security/plugins/security_solution/common/siem_migrations/constants.ts
@@ -11,6 +11,8 @@ export const SIEM_MIGRATIONS_PATH = '/internal/siem_migrations' as const;
 export const SIEM_RULE_MIGRATIONS_PATH = `${SIEM_MIGRATIONS_PATH}/rules` as const;
 
 export const SIEM_RULE_MIGRATIONS_ALL_STATS_PATH = `${SIEM_RULE_MIGRATIONS_PATH}/stats` as const;
+export const SIEM_RULE_MIGRATIONS_INTEGRATIONS_PATH =
+  `${SIEM_RULE_MIGRATIONS_PATH}/integrations` as const;
 export const SIEM_RULE_MIGRATION_CREATE_PATH =
   `${SIEM_RULE_MIGRATIONS_PATH}/{migration_id?}` as const;
 export const SIEM_RULE_MIGRATION_PATH = `${SIEM_RULE_MIGRATIONS_PATH}/{migration_id}` as const;
@@ -25,8 +27,6 @@ export const SIEM_RULE_MIGRATION_INSTALL_TRANSLATED_PATH =
   `${SIEM_RULE_MIGRATION_PATH}/install_translated` as const;
 export const SIEM_RULE_MIGRATIONS_PREBUILT_RULES_PATH =
   `${SIEM_RULE_MIGRATION_PATH}/prebuilt_rules` as const;
-export const SIEM_RULE_MIGRATIONS_INTEGRATIONS_PATH =
-  `${SIEM_RULE_MIGRATION_PATH}/integrations` as const;
 
 export const SIEM_RULE_MIGRATION_RESOURCES_PATH = `${SIEM_RULE_MIGRATION_PATH}/resources` as const;
 export const SIEM_RULE_MIGRATION_RESOURCES_MISSING_PATH =

diff --git a/...ty/plugins/security_solution/common/siem_migrations/model/api/rules/rule_migration.gen.ts b/...ty/plugins/security_solution/common/siem_migrations/model/api/rules/rule_migration.gen.ts
@@ -80,16 +80,6 @@ export const GetRuleMigrationResponse = z.object({
   data: z.array(RuleMigration),
 });
 
-export type GetRuleMigrationIntegrationsRequestParams = z.infer<
-  typeof GetRuleMigrationIntegrationsRequestParams
->;
-export const GetRuleMigrationIntegrationsRequestParams = z.object({
-  migration_id: NonEmptyString,
-});
-export type GetRuleMigrationIntegrationsRequestParamsInput = z.input<
-  typeof GetRuleMigrationIntegrationsRequestParams
->;
-
 /**
  * The map of related integrations, with the integration id as a key
  */

diff --git a/...ugins/security_solution/common/siem_migrations/model/api/rules/rule_migration.schema.yaml b/...ugins/security_solution/common/siem_migrations/model/api/rules/rule_migration.schema.yaml
@@ -405,13 +405,6 @@ paths:
       description: Retrieves all related integrations
       tags:
         - SIEM Rule Migrations
-      parameters:
-        - name: migration_id
-          in: path
-          required: true
-          schema:
-            description: The migration id to retrieve related integrations for
-            $ref: '../../../../../common/api/model/primitives.schema.yaml#/components/schemas/NonEmptyString'
       responses:
         200:
           description: Indicates that related integrations have been retrieved correctly.

diff --git a/...ck/solutions/security/plugins/security_solution/public/siem_migrations/rules/api/index.ts b/...ck/solutions/security/plugins/security_solution/public/siem_migrations/rules/api/index.ts
@@ -281,19 +281,16 @@ export const getRuleMigrationsPrebuiltRules = async ({
   );
 };
 
-export interface GetRelatedIntegrationsParams {
-  /** `id` of the migration to get related integrations for */
-  migrationId: string;
+export interface GetIntegrationsParams {
   /** Optional AbortSignal for cancelling request */
   signal?: AbortSignal;
 }
-/** Retrieves related integrations for a specific migration. */
-export const getRelatedIntegrations = async ({
-  migrationId,
+/** Retrieves existing integrations. */
+export const getIntegrations = async ({
   signal,
-}: GetRelatedIntegrationsParams): Promise<GetRuleMigrationIntegrationsResponse> => {
+}: GetIntegrationsParams): Promise<GetRuleMigrationIntegrationsResponse> => {
   return KibanaServices.get().http.get<GetRuleMigrationIntegrationsResponse>(
-    replaceParams(SIEM_RULE_MIGRATIONS_INTEGRATIONS_PATH, { migration_id: migrationId }),
+    SIEM_RULE_MIGRATIONS_INTEGRATIONS_PATH,
     { version: '1', signal }
   );
 };

diff --git a/...y/plugins/security_solution/public/siem_migrations/rules/components/rules_table/index.tsx b/...y/plugins/security_solution/public/siem_migrations/rules/components/rules_table/index.tsx
@@ -34,7 +34,6 @@ import { BulkActions } from './bulk_actions';
 import { SearchField } from './search_field';
 import { RuleTranslationResult } from '../../../../../common/siem_migrations/constants';
 import * as i18n from './translations';
-import { useGetRelatedIntegrations } from '../../service/hooks/use_get_integrations';
 
 const DEFAULT_PAGE_SIZE = 10;
 const DEFAULT_SORT_FIELD = 'translation_result';
@@ -45,13 +44,23 @@ export interface MigrationRulesTableProps {
    * Selected rule migration id
    */
   migrationId: string;
+
+  /**
+   * Existing integrations.
+   */
+  integrations?: Record<string, RelatedIntegration>;
+
+  /**
+   * Indicates whether the integrations loading is in progress.
+   */
+  isIntegrationsLoading?: boolean;
 }
 
 /**
  * Table Component for displaying SIEM rules migrations
  */
 export const MigrationRulesTable: React.FC<MigrationRulesTableProps> = React.memo(
-  ({ migrationId }) => {
+  ({ migrationId, integrations, isIntegrationsLoading }) => {
     const { addError } = useAppToasts();
 
     const [pageIndex, setPageIndex] = useState(0);
@@ -66,9 +75,6 @@ export const MigrationRulesTable: React.FC<MigrationRulesTableProps> = React.mem
     const { data: prebuiltRules = {}, isLoading: isPrebuiltRulesLoading } =
       useGetMigrationPrebuiltRules(migrationId);
 
-    const { integrations, isLoading: isIntegrationsLoading } =
-      useGetRelatedIntegrations(migrationId);
-
     const {
       data: { ruleMigrations, total } = { ruleMigrations: [], total: 0 },
       isLoading: isDataLoading,
@@ -185,12 +191,7 @@ export const MigrationRulesTable: React.FC<MigrationRulesTableProps> = React.mem
       [addError, installTranslatedMigrationRules]
     );
 
-    const isLoading =
-      isStatsLoading ||
-      isPrebuiltRulesLoading ||
-      isIntegrationsLoading ||
-      isDataLoading ||
-      isTableLoading;
+    const isLoading = isStatsLoading || isPrebuiltRulesLoading || isDataLoading || isTableLoading;
 
     const ruleActionsFactory = useCallback(
       (ruleMigration: RuleMigration, closeRulePreview: () => void) => {
@@ -233,7 +234,7 @@ export const MigrationRulesTable: React.FC<MigrationRulesTableProps> = React.mem
 
     const getMigrationRuleData = useCallback(
       (ruleId: string) => {
-        if (!isLoading && ruleMigrations.length && integrations) {
+        if (!isLoading && ruleMigrations.length) {
           const ruleMigration = ruleMigrations.find((item) => item.id === ruleId);
           let matchedPrebuiltRule: RuleResponse | undefined;
           const relatedIntegrations: RelatedIntegration[] = [];
@@ -245,19 +246,21 @@ export const MigrationRulesTable: React.FC<MigrationRulesTableProps> = React.mem
             matchedPrebuiltRule =
               matchedPrebuiltRuleVersion?.current ?? matchedPrebuiltRuleVersion?.target;
 
-            if (matchedPrebuiltRule?.related_integrations) {
-              relatedIntegrations.push(...matchedPrebuiltRule.related_integrations);
-            } else if (ruleMigration.elastic_rule?.integration_id) {
-              const integration = integrations[ruleMigration.elastic_rule.integration_id];
-              if (integration) {
-                relatedIntegrations.push(integration);
+            if (integrations) {
+              if (matchedPrebuiltRule?.related_integrations) {
+                relatedIntegrations.push(...matchedPrebuiltRule.related_integrations);
+              } else if (ruleMigration.elastic_rule?.integration_id) {
+                const integration = integrations[ruleMigration.elastic_rule.integration_id];
+                if (integration) {
+                  relatedIntegrations.push(integration);
+                }
               }
             }
           }
-          return { ruleMigration, matchedPrebuiltRule, relatedIntegrations };
+          return { ruleMigration, matchedPrebuiltRule, relatedIntegrations, isIntegrationsLoading };
         }
       },
-      [integrations, isLoading, prebuiltRules, ruleMigrations]
+      [integrations, isIntegrationsLoading, isLoading, prebuiltRules, ruleMigrations]
     );
 
     const {

diff --git a/...ity_solution/public/siem_migrations/rules/components/rules_table_columns/integrations.tsx b/...ity_solution/public/siem_migrations/rules/components/rules_table_columns/integrations.tsx
@@ -6,6 +6,7 @@
  */
 
 import React from 'react';
+import { EuiLoadingSpinner } from '@elastic/eui';
 import type { RelatedIntegration } from '../../../../../common/api/detection_engine';
 import { IntegrationsPopover } from '../../../../detections/components/rules/related_integrations/integrations_popover';
 import type { RuleMigration } from '../../../../../common/siem_migrations/model/rule_migration.gen';
@@ -17,13 +18,16 @@ export const createIntegrationsColumn = ({
 }: {
   getMigrationRuleData: (
     ruleId: string
-  ) => { relatedIntegrations?: RelatedIntegration[] } | undefined;
+  ) => { relatedIntegrations?: RelatedIntegration[]; isIntegrationsLoading?: boolean } | undefined;
 }): TableColumn => {
   return {
     field: 'elastic_rule.integration_id',
     name: i18n.COLUMN_INTEGRATIONS,
     render: (_, rule: RuleMigration) => {
       const migrationRuleData = getMigrationRuleData(rule.id);
+      if (migrationRuleData?.isIntegrationsLoading) {
+        return <EuiLoadingSpinner />;
+      }
       const relatedIntegrations = migrationRuleData?.relatedIntegrations;
       if (relatedIntegrations == null || relatedIntegrations.length === 0) {
         return null;

diff --git a/...ecurity_solution/public/siem_migrations/rules/hooks/use_migration_rules_table_columns.tsx b/...ecurity_solution/public/siem_migrations/rules/hooks/use_migration_rules_table_columns.tsx
@@ -31,7 +31,7 @@ export const useMigrationRulesTableColumns = ({
   installMigrationRule: (migrationRule: RuleMigration, enable?: boolean) => void;
   getMigrationRuleData: (
     ruleId: string
-  ) => { relatedIntegrations?: RelatedIntegration[] } | undefined;
+  ) => { relatedIntegrations?: RelatedIntegration[]; isIntegrationsLoading?: boolean } | undefined;
 }): TableColumn[] => {
   return useMemo(
     () => [

diff --git a/...solutions/security/plugins/security_solution/public/siem_migrations/rules/pages/index.tsx b/...solutions/security/plugins/security_solution/public/siem_migrations/rules/pages/index.tsx
@@ -9,6 +9,7 @@ import React, { useEffect, useMemo } from 'react';
 
 import { EuiSkeletonLoading, EuiSkeletonText, EuiSkeletonTitle } from '@elastic/eui';
 import type { RouteComponentProps } from 'react-router-dom';
+import type { RelatedIntegration } from '../../../../common/api/detection_engine';
 import { SiemMigrationTaskStatus } from '../../../../common/siem_migrations/constants';
 import { useNavigation } from '../../../common/lib/kibana';
 import { HeaderPage } from '../../../common/components/header_page';
@@ -22,6 +23,7 @@ import { MissingPrivilegesCallOut } from '../../../detections/components/callout
 import { HeaderButtons } from '../components/header_buttons';
 import { UnknownMigration } from '../components/unknown_migration';
 import { useLatestStats } from '../service/hooks/use_latest_stats';
+import { useGetIntegrations } from '../service/hooks/use_get_integrations';
 
 type MigrationRulesPageProps = RouteComponentProps<{ migrationId?: string }>;
 
@@ -35,6 +37,16 @@ export const MigrationRulesPage: React.FC<MigrationRulesPageProps> = React.memo(
 
     const { data: ruleMigrationsStatsAll, isLoading: isLoadingMigrationsStats } = useLatestStats();
 
+    const [integrations, setIntegrations] = React.useState<
+      Record<string, RelatedIntegration> | undefined
+    >();
+    const { getIntegrations, isLoading: isIntegrationsLoading } =
+      useGetIntegrations(setIntegrations);
+
+    useEffect(() => {
+      getIntegrations();
+    }, [getIntegrations]);
+
     const finishedRuleMigrationsStats = useMemo(() => {
       if (isLoadingMigrationsStats || !ruleMigrationsStatsAll?.length) {
         return [];
@@ -72,8 +84,14 @@ export const MigrationRulesPage: React.FC<MigrationRulesPageProps> = React.memo(
       if (!migrationId || !finishedRuleMigrationsStats.some((stats) => stats.id === migrationId)) {
         return <UnknownMigration />;
       }
-      return <MigrationRulesTable migrationId={migrationId} />;
-    }, [migrationId, finishedRuleMigrationsStats]);
+      return (
+        <MigrationRulesTable
+          migrationId={migrationId}
+          integrations={integrations}
+          isIntegrationsLoading={isIntegrationsLoading}
+        />
+      );
+    }, [migrationId, finishedRuleMigrationsStats, integrations, isIntegrationsLoading]);
 
     return (
       <>

diff --git a/...gins/security_solution/public/siem_migrations/rules/service/hooks/use_get_integrations.ts b/...gins/security_solution/public/siem_migrations/rules/service/hooks/use_get_integrations.ts
@@ -5,44 +5,38 @@
  * 2.0.
  */
 
-import { useCallback, useEffect, useReducer, useState } from 'react';
+import { useCallback, useReducer } from 'react';
 import { i18n } from '@kbn/i18n';
 import type { RelatedIntegration } from '../../../../../common/api/detection_engine';
 import { useKibana } from '../../../../common/lib/kibana/kibana_react';
 import { reducer, initialState } from './common/api_request_reducer';
 
-export const GET_RELATED_INTEGRATIONS_ERROR = i18n.translate(
-  'xpack.securitySolution.siemMigrations.rules.service.getRelatedIntegrationsError',
-  { defaultMessage: 'Failed to fetch related integrations' }
+export const GET_INTEGRATIONS_ERROR = i18n.translate(
+  'xpack.securitySolution.siemMigrations.rules.service.getIntegrationsError',
+  { defaultMessage: 'Failed to fetch integrations' }
 );
 
-export const useGetRelatedIntegrations = (migrationId: string) => {
+export type OnSuccess = (integrations: Record<string, RelatedIntegration>) => void;
+
+export const useGetIntegrations = (onSuccess: OnSuccess) => {
   const { siemMigrations, notifications } = useKibana().services;
   const [state, dispatch] = useReducer(reducer, initialState);
-  const [integrations, setIntegrations] = useState<
-    Record<string, RelatedIntegration> | undefined
-  >();
 
-  const getRelatedIntegrations = useCallback(() => {
+  const getIntegrations = useCallback(() => {
     (async () => {
       try {
         dispatch({ type: 'start' });
-        const results = await siemMigrations.rules.getRelatedIntegrations(migrationId);
+        const integrations = await siemMigrations.rules.getIntegrations();
 
-        setIntegrations(results);
+        onSuccess(integrations);
         dispatch({ type: 'success' });
       } catch (err) {
-        setIntegrations(undefined);
         const apiError = err.body ?? err;
-        notifications.toasts.addError(apiError, { title: GET_RELATED_INTEGRATIONS_ERROR });
+        notifications.toasts.addError(apiError, { title: GET_INTEGRATIONS_ERROR });
         dispatch({ type: 'error', error: apiError });
       }
     })();
-  }, [siemMigrations.rules, migrationId, notifications.toasts]);
-
-  useEffect(() => {
-    getRelatedIntegrations();
-  }, [getRelatedIntegrations]);
+  }, [siemMigrations.rules, notifications.toasts, onSuccess]);
 
-  return { isLoading: state.loading, error: state.error, integrations };
+  return { isLoading: state.loading, error: state.error, getIntegrations };
 };
diff --git a/...plugins/security_solution/public/siem_migrations/rules/service/rule_migrations_service.ts b/...plugins/security_solution/public/siem_migrations/rules/service/rule_migrations_service.ts
@@ -36,7 +36,7 @@ import {
   type GetRuleMigrationsStatsAllParams,
   getMissingResources,
   upsertMigrationResources,
-  getRelatedIntegrations,
+  getIntegrations,
 } from '../api';
 import type { RuleMigrationStats } from '../types';
 import { getSuccessToast } from './success_notification';
@@ -183,10 +183,8 @@ export class SiemRulesMigrationsService {
     });
   }
 
-  public async getRelatedIntegrations(
-    migrationId: string
-  ): Promise<Record<string, RelatedIntegration>> {
-    return getRelatedIntegrations({ migrationId });
+  public async getIntegrations(): Promise<Record<string, RelatedIntegration>> {
+    return getIntegrations({});
   }
 
   private async startTaskStatsPolling(): Promise<void> {