-
Notifications
You must be signed in to change notification settings - Fork 8.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Security Solution] Add UI incentivizers to upgrade prebuilt rules #211862
Changes from all commits
8f3555b
7e1de38
e5ed827
07b4d66
8d815a0
94d51a8
a1d7e8c
18e700c
1e11d6b
1cec639
dd2fa00
625290f
9befc79
0aed247
30f99cc
4b7160c
f00f062
b58a1e2
1e76dfc
8133b1a
c47dedd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,6 +11,7 @@ import { | |
EuiCallOut, | ||
EuiFlexGroup, | ||
EuiFlexItem, | ||
EuiLink, | ||
EuiResizableContainer, | ||
EuiSpacer, | ||
EuiTab, | ||
|
@@ -75,6 +76,7 @@ import { usePrebuiltRulesCustomizationStatus } from '../../../rule_management/lo | |
import { PrebuiltRulesCustomizationDisabledReason } from '../../../../../common/detection_engine/prebuilt_rules/prebuilt_rule_customization_status'; | ||
import { ALERT_SUPPRESSION_FIELDS_FIELD_NAME } from '../../../rule_creation/components/alert_suppression_edit'; | ||
import { usePrebuiltRuleCustomizationUpsellingMessage } from '../../../rule_management/logic/prebuilt_rules/use_prebuilt_rule_customization_upselling_message'; | ||
import { useRuleUpdateCallout } from '../../../rule_management/hooks/use_rule_update_callout'; | ||
|
||
const EditRulePageComponent: FC<{ rule: RuleResponse }> = ({ rule }) => { | ||
const { addSuccess } = useAppToasts(); | ||
|
@@ -509,6 +511,16 @@ const EditRulePageComponent: FC<{ rule: RuleResponse }> = ({ rule }) => { | |
[navigateToApp, ruleId] | ||
); | ||
|
||
const upgradeCallout = useRuleUpdateCallout({ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what is a benefit of using hook here, not a component? |
||
rule, | ||
message: ruleI18n.HAS_RULE_UPDATE_EDITING_CALLOUT_MESSAGE, | ||
actionButton: ( | ||
<EuiLink onClick={goToDetailsRule} data-test-subj="ruleEditingUpdateRuleCalloutButton"> | ||
{ruleI18n.HAS_RULE_UPDATE_EDITING_CALLOUT_BUTTON} | ||
</EuiLink> | ||
), | ||
}); | ||
|
||
if ( | ||
redirectToDetections( | ||
isSignalIndexExists, | ||
|
@@ -550,6 +562,7 @@ const EditRulePageComponent: FC<{ rule: RuleResponse }> = ({ rule }) => { | |
setIsRulePreviewVisible={setIsRulePreviewVisible} | ||
togglePanel={togglePanel} | ||
/> | ||
{upgradeCallout} | ||
{invalidSteps.length > 0 && ( | ||
<EuiCallOut title={i18n.SORRY_ERRORS} color="danger" iconType="warning"> | ||
<FormattedMessage | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -148,6 +148,7 @@ import { useManualRuleRunConfirmation } from '../../../rule_gaps/components/manu | |
import { useLegacyUrlRedirect } from './use_redirect_legacy_url'; | ||
import { RuleDetailTabs, useRuleDetailsTabs } from './use_rule_details_tabs'; | ||
import { useIsExperimentalFeatureEnabled } from '../../../../common/hooks/use_experimental_features'; | ||
import { useRuleUpdateCallout } from '../../../rule_management/hooks/use_rule_update_callout'; | ||
|
||
const RULE_EXCEPTION_LIST_TYPES = [ | ||
ExceptionListTypeEnum.DETECTION, | ||
|
@@ -254,7 +255,7 @@ const RuleDetailsPageComponent: React.FC<DetectionEngineComponentProps> = ({ | |
|
||
const { pollForSignalIndex } = useSignalHelpers(); | ||
const [rule, setRule] = useState<RuleResponse | null>(null); | ||
const isLoading = ruleLoading && rule == null; | ||
const isLoading = useMemo(() => ruleLoading && rule == null, [rule, ruleLoading]); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: Calculation is pretty trivial here and resulting in a boolean value. It's not necessary to wrap it into |
||
|
||
const { starting: isStartingJobs, startMlJobs } = useStartMlJobs(); | ||
const startMlJobsIfNeeded = useCallback(async () => { | ||
|
@@ -394,6 +395,12 @@ const RuleDetailsPageComponent: React.FC<DetectionEngineComponentProps> = ({ | |
const lastExecutionDate = lastExecution?.date ?? ''; | ||
const lastExecutionMessage = lastExecution?.message ?? ''; | ||
|
||
const upgradeCallout = useRuleUpdateCallout({ | ||
rule, | ||
message: ruleI18n.HAS_RULE_UPDATE_DETAILS_CALLOUT_MESSAGE, | ||
onUpgrade: refreshRule, | ||
}); | ||
|
||
const ruleStatusInfo = useMemo(() => { | ||
return ( | ||
<> | ||
|
@@ -555,6 +562,7 @@ const RuleDetailsPageComponent: React.FC<DetectionEngineComponentProps> = ({ | |
<> | ||
<NeedAdminForUpdateRulesCallOut /> | ||
<MissingPrivilegesCallOut /> | ||
{upgradeCallout} | ||
{isBulkDuplicateConfirmationVisible && ( | ||
<BulkActionDuplicateExceptionsConfirmation | ||
onCancel={cancelRuleDuplication} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,71 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License | ||
* 2.0; you may not use this file except in compliance with the Elastic License | ||
* 2.0. | ||
*/ | ||
|
||
import { EuiCallOut, EuiSpacer, EuiLink } from '@elastic/eui'; | ||
import React, { useMemo } from 'react'; | ||
import type { RuleResponse } from '../../../../../common/api/detection_engine'; | ||
import * as i18n from './translations'; | ||
import { usePrebuiltRulesUpgrade } from '../../hooks/use_prebuilt_rules_upgrade'; | ||
|
||
interface RuleUpdateCalloutProps { | ||
rule: RuleResponse; | ||
message: string; | ||
actionButton?: JSX.Element; | ||
onUpgrade?: () => void; | ||
} | ||
|
||
const RuleUpdateCalloutComponent = ({ | ||
rule, | ||
message, | ||
actionButton, | ||
onUpgrade, | ||
}: RuleUpdateCalloutProps): JSX.Element | null => { | ||
const { upgradeReviewResponse, rulePreviewFlyout, openRulePreview } = usePrebuiltRulesUpgrade({ | ||
pagination: { | ||
page: 1, // we only want to fetch one result | ||
perPage: 1, | ||
}, | ||
filter: { rule_ids: [rule.id] }, | ||
onUpgrade, | ||
}); | ||
|
||
const isRuleUpgradeable = useMemo( | ||
() => upgradeReviewResponse !== undefined && upgradeReviewResponse.total > 0, | ||
[upgradeReviewResponse] | ||
); | ||
|
||
const updateCallToActionButton = useMemo(() => { | ||
if (actionButton) { | ||
return actionButton; | ||
} | ||
return ( | ||
<EuiLink | ||
onClick={() => openRulePreview(rule.rule_id)} | ||
data-test-subj="ruleDetailsUpdateRuleCalloutButton" | ||
> | ||
{i18n.HAS_RULE_UPDATE_CALLOUT_BUTTON} | ||
</EuiLink> | ||
); | ||
}, [actionButton, openRulePreview, rule.rule_id]); | ||
|
||
if (!isRuleUpgradeable) { | ||
return null; | ||
} | ||
|
||
return ( | ||
<> | ||
<EuiCallOut title={i18n.HAS_RULE_UPDATE_CALLOUT_TITLE} color="primary" iconType="gear"> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Mixing
|
||
<p>{message}</p> | ||
{updateCallToActionButton} | ||
</EuiCallOut> | ||
<EuiSpacer size="l" /> | ||
{rulePreviewFlyout} | ||
</> | ||
); | ||
}; | ||
|
||
export const RuleUpdateCallout = React.memo(RuleUpdateCalloutComponent); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License | ||
* 2.0; you may not use this file except in compliance with the Elastic License | ||
* 2.0. | ||
*/ | ||
|
||
import React from 'react'; | ||
import { EuiCallOut } from '@elastic/eui'; | ||
import * as i18n from './translations'; | ||
|
||
export const RuleHasMissingBaseVersionCallout = () => ( | ||
<EuiCallOut color="warning" size="s"> | ||
<p>{i18n.RULE_BASE_VERSION_IS_MISSING_DESCRIPTION}</p> | ||
</EuiCallOut> | ||
); |
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's roll back changes in this files. It looks like only fragment wrappers left here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We mostly stick to
upgrade
word in the codebase while usingupdate
in texts exposed to users. I recommend to stick toupgrade
in names you used for consistency.