From ee5b3613d2f9fbf592335d0509d23f1f2b909b2e Mon Sep 17 00:00:00 2001 From: Jiawei Wu <74562234+JiaweiWu@users.noreply.github.com> Date: Wed, 16 Nov 2022 01:21:16 -0800 Subject: [PATCH] [RAM] Refactor rule status helper to reduce bundle size (#145317) ## Summary In the PR that updated and consolidated rule status (https://github.com/elastic/kibana/pull/144466). The new helper that was added had references to translations, which increased the `triggers_actions_ui` bundle size by quite a bit. This PR refactors the helper so we're passing in the translations. It seems like a good compromise to allow some logic reuse but without the massive bundle increase. ### Checklist - [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 Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> --- .../sections/rule_details/components/rule.tsx | 12 +++- .../rules_list_table_status_cell.tsx | 12 +++- .../common/lib/rule_status_helper.test.ts | 61 ++++++++++++++++--- .../public/common/lib/rule_status_helpers.ts | 23 ++++--- 4 files changed, 89 insertions(+), 19 deletions(-) diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_details/components/rule.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_details/components/rule.tsx index cffe5e3520ca2..d7540150c90ba 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_details/components/rule.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_details/components/rule.tsx @@ -25,6 +25,11 @@ import { getRuleStatusMessage, } from '../../../../common/lib/rule_status_helpers'; import RuleStatusPanelWithApi from './rule_status_panel'; +import { + ALERT_STATUS_LICENSE_ERROR, + rulesLastRunOutcomeTranslationMapping, + rulesStatusesTranslationsMapping, +} from '../../rules_list/translations'; const RuleEventLogList = lazy(() => import('./rule_event_log_list')); const RuleAlertList = lazy(() => import('./rule_alert_list')); @@ -74,7 +79,12 @@ export function RuleComponent({ }; const healthColor = getRuleHealthColor(rule); - const statusMessage = getRuleStatusMessage(rule); + const statusMessage = getRuleStatusMessage({ + rule, + licenseErrorText: ALERT_STATUS_LICENSE_ERROR, + lastOutcomeTranslations: rulesLastRunOutcomeTranslationMapping, + executionStatusTranslations: rulesStatusesTranslationsMapping, + }); const renderRuleAlertList = () => { return suspendedComponentWithProps( diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/rules_list/components/rules_list_table_status_cell.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/rules_list/components/rules_list_table_status_cell.tsx index e219b3e34bae8..759e16c7f696d 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/rules_list/components/rules_list_table_status_cell.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/rules_list/components/rules_list_table_status_cell.tsx @@ -21,6 +21,11 @@ import { getIsLicenseError, getRuleStatusMessage, } from '../../../../common/lib/rule_status_helpers'; +import { + ALERT_STATUS_LICENSE_ERROR, + rulesLastRunOutcomeTranslationMapping, + rulesStatusesTranslationsMapping, +} from '../translations'; export interface RulesListTableStatusCellProps { rule: RuleTableItem; @@ -33,7 +38,12 @@ export const RulesListTableStatusCell = (props: RulesListTableStatusCellProps) = const isLicenseError = getIsLicenseError(rule); const healthColor = getRuleHealthColor(rule); - const statusMessage = getRuleStatusMessage(rule); + const statusMessage = getRuleStatusMessage({ + rule, + licenseErrorText: ALERT_STATUS_LICENSE_ERROR, + lastOutcomeTranslations: rulesLastRunOutcomeTranslationMapping, + executionStatusTranslations: rulesStatusesTranslationsMapping, + }); const tooltipMessage = lastRun?.outcome === 'failed' ? `Error: ${lastRun?.outcomeMsg}` : null; if (!statusMessage) { diff --git a/x-pack/plugins/triggers_actions_ui/public/common/lib/rule_status_helper.test.ts b/x-pack/plugins/triggers_actions_ui/public/common/lib/rule_status_helper.test.ts index 5cf2e5a20987e..91af1e05ece20 100644 --- a/x-pack/plugins/triggers_actions_ui/public/common/lib/rule_status_helper.test.ts +++ b/x-pack/plugins/triggers_actions_ui/public/common/lib/rule_status_helper.test.ts @@ -9,6 +9,11 @@ import { getRuleHealthColor, getRuleStatusMessage } from './rule_status_helpers' import { RuleTableItem } from '../../types'; import { getIsExperimentalFeatureEnabled } from '../get_experimental_features'; +import { + ALERT_STATUS_LICENSE_ERROR, + rulesLastRunOutcomeTranslationMapping, + rulesStatusesTranslationsMapping, +} from '../../application/sections/rules_list/translations'; jest.mock('../get_experimental_features', () => ({ getIsExperimentalFeatureEnabled: jest.fn(), @@ -97,38 +102,78 @@ describe('getRuleHealthColor', () => { describe('getRuleStatusMessage', () => { it('should get the status message for a successful rule', () => { - let statusMessage = getRuleStatusMessage(mockRule); + let statusMessage = getRuleStatusMessage({ + rule: mockRule, + licenseErrorText: ALERT_STATUS_LICENSE_ERROR, + lastOutcomeTranslations: rulesLastRunOutcomeTranslationMapping, + executionStatusTranslations: rulesStatusesTranslationsMapping, + }); expect(statusMessage).toEqual('Succeeded'); (getIsExperimentalFeatureEnabled as jest.Mock).mockImplementation(() => false); - statusMessage = getRuleStatusMessage(mockRule); + statusMessage = getRuleStatusMessage({ + rule: mockRule, + licenseErrorText: ALERT_STATUS_LICENSE_ERROR, + lastOutcomeTranslations: rulesLastRunOutcomeTranslationMapping, + executionStatusTranslations: rulesStatusesTranslationsMapping, + }); expect(statusMessage).toEqual('Active'); }); it('should get the status message for a warning rule', () => { - let statusMessage = getRuleStatusMessage(warningRule); + let statusMessage = getRuleStatusMessage({ + rule: warningRule, + licenseErrorText: ALERT_STATUS_LICENSE_ERROR, + lastOutcomeTranslations: rulesLastRunOutcomeTranslationMapping, + executionStatusTranslations: rulesStatusesTranslationsMapping, + }); expect(statusMessage).toEqual('Warning'); (getIsExperimentalFeatureEnabled as jest.Mock).mockImplementation(() => false); - statusMessage = getRuleStatusMessage(warningRule); + statusMessage = getRuleStatusMessage({ + rule: warningRule, + licenseErrorText: ALERT_STATUS_LICENSE_ERROR, + lastOutcomeTranslations: rulesLastRunOutcomeTranslationMapping, + executionStatusTranslations: rulesStatusesTranslationsMapping, + }); expect(statusMessage).toEqual('Warning'); }); it('should get the status message for a failed rule', () => { - let statusMessage = getRuleStatusMessage(failedRule); + let statusMessage = getRuleStatusMessage({ + rule: failedRule, + licenseErrorText: ALERT_STATUS_LICENSE_ERROR, + lastOutcomeTranslations: rulesLastRunOutcomeTranslationMapping, + executionStatusTranslations: rulesStatusesTranslationsMapping, + }); expect(statusMessage).toEqual('Failed'); (getIsExperimentalFeatureEnabled as jest.Mock).mockImplementation(() => false); - statusMessage = getRuleStatusMessage(failedRule); + statusMessage = getRuleStatusMessage({ + rule: failedRule, + licenseErrorText: ALERT_STATUS_LICENSE_ERROR, + lastOutcomeTranslations: rulesLastRunOutcomeTranslationMapping, + executionStatusTranslations: rulesStatusesTranslationsMapping, + }); expect(statusMessage).toEqual('Error'); }); it('should get the status message for a license error rule', () => { - let statusMessage = getRuleStatusMessage(licenseErrorRule); + let statusMessage = getRuleStatusMessage({ + rule: licenseErrorRule, + licenseErrorText: ALERT_STATUS_LICENSE_ERROR, + lastOutcomeTranslations: rulesLastRunOutcomeTranslationMapping, + executionStatusTranslations: rulesStatusesTranslationsMapping, + }); expect(statusMessage).toEqual('License Error'); (getIsExperimentalFeatureEnabled as jest.Mock).mockImplementation(() => false); - statusMessage = getRuleStatusMessage(licenseErrorRule); + statusMessage = getRuleStatusMessage({ + rule: licenseErrorRule, + licenseErrorText: ALERT_STATUS_LICENSE_ERROR, + lastOutcomeTranslations: rulesLastRunOutcomeTranslationMapping, + executionStatusTranslations: rulesStatusesTranslationsMapping, + }); expect(statusMessage).toEqual('License Error'); }); }); diff --git a/x-pack/plugins/triggers_actions_ui/public/common/lib/rule_status_helpers.ts b/x-pack/plugins/triggers_actions_ui/public/common/lib/rule_status_helpers.ts index f5552000206f3..f3e6419e4e2fa 100644 --- a/x-pack/plugins/triggers_actions_ui/public/common/lib/rule_status_helpers.ts +++ b/x-pack/plugins/triggers_actions_ui/public/common/lib/rule_status_helpers.ts @@ -11,11 +11,6 @@ import { } from '@kbn/alerting-plugin/common'; import { getIsExperimentalFeatureEnabled } from '../get_experimental_features'; import { Rule } from '../../types'; -import { - rulesLastRunOutcomeTranslationMapping, - rulesStatusesTranslationsMapping, - ALERT_STATUS_LICENSE_ERROR, -} from '../../application/sections/rules_list/translations'; export const getOutcomeHealthColor = (status: RuleLastRunOutcomes) => { switch (status) { @@ -62,15 +57,25 @@ export const getIsLicenseError = (rule: Rule) => { ); }; -export const getRuleStatusMessage = (rule: Rule) => { +export const getRuleStatusMessage = ({ + rule, + licenseErrorText, + lastOutcomeTranslations, + executionStatusTranslations, +}: { + rule: Rule; + licenseErrorText: string; + lastOutcomeTranslations: Record; + executionStatusTranslations: Record; +}) => { const isLicenseError = getIsLicenseError(rule); const isRuleLastRunOutcomeEnabled = getIsExperimentalFeatureEnabled('ruleLastRunOutcome'); if (isLicenseError) { - return ALERT_STATUS_LICENSE_ERROR; + return licenseErrorText; } if (isRuleLastRunOutcomeEnabled) { - return rule.lastRun && rulesLastRunOutcomeTranslationMapping[rule.lastRun.outcome]; + return rule.lastRun && lastOutcomeTranslations[rule.lastRun.outcome]; } - return rulesStatusesTranslationsMapping[rule.executionStatus.status]; + return executionStatusTranslations[rule.executionStatus.status]; };