[Security Solution] Fix rule snooze description on the rule editing page #155850
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
maximpn
added
enhancement
3 tasks
maximpn
commented
Apr 26, 2023
| @@ -29,17 +33,27 @@ export const NO_ACTIONS_READ_PERMISSIONS = i18n.translate( | |||
| } | |||
| ); | |||
|
|
|||
| export const RULE_SNOOZE_DESCRIPTION = i18n.translate( | |||
| 'xpack.securitySolution.detectionEngine.createRule.stepRuleActions.snoozeDescription', | |||
| const ruleSnoozeDocsLink = `${ | |||
There was a problem hiding this comment.
I'm not 100% sure the link is the right one though I wasn't able to find any rule snooze related docs.
@nastasha-solomon should I use another link here?
There was a problem hiding this comment.
The anchor for this is snooze-rules-ui. I'm adding docs for this feature here.
There was a problem hiding this comment.
Morning/afternoon! Scratch what I said yesterday. If this doc link is appearing at the end of this message, you can use these existing docs for editing rule settings. The anchor for this section is edit-rules-settings.
banderror
added
bug
|
@elasticmachine merge upstream |
nastasha-solomon
added
the
ui-copy
| export const RULE_SNOOZE_DESCRIPTION = ( | ||
| <FormattedMessage | ||
| id="xpack.securitySolution.detectionEngine.createRule.stepRuleActions.snoozeDescription" | ||
| defaultMessage="Select when automated actions should be performed. If a rule is snoozed actions will not be performed. Learn more about actions in our {docs}." |
There was a problem hiding this comment.
Hey, @maximpn! I Just want to verify that the second sentence will appear on a new line, and to the right of the snooze button (similar to what's shown in the original design). Putting it directly after the instruction to choose when actions are performed is a little confusing and contradictory. Putting it next to the snooze button helps form the relationship between the button and the informational text.
For the informational text about the snooze feature, would something like the following work:
Notifications are snoozed for X days. Turn snoozing off to begin sending notifications again. Learn more
Another option:
Notifications are snoozed for X days. They will resume when you unsnooze actions. Learn more
There was a problem hiding this comment.
Hey @nastasha-solomon!
This PR simplifies the approach and just displays the static text on the top.
The problem the text on the right like Notifications are snoozed for X days is managed outside of Security Solution plugin so adding something extra text requires checking if a rule is snoozed. Having it will increase overall complexity which we wanna avoid.
There was a problem hiding this comment.
Thanks for the additional context, @maximpn. I wasn't aware of that limitation but knowing it now, it makes sense why the new UI copy was added under the section title. Here's a few new options for you to choose from:
Option 1: Choose when to perform actions or snooze them. Notifications are not created for snoozed actions. Learn more
Option 2: Select when automated actions should be performed or snooze actions. Learn more
I prefer the first one because I think it flows better and is more descriptive, but the second one works if you can't alter the first part of the sentence.
There was a problem hiding this comment.
I've updated the text with Option 1.
| @@ -29,17 +33,27 @@ export const NO_ACTIONS_READ_PERMISSIONS = i18n.translate( | |||
| } | |||
| ); | |||
|
|
|||
| export const RULE_SNOOZE_DESCRIPTION = i18n.translate( | |||
| 'xpack.securitySolution.detectionEngine.createRule.stepRuleActions.snoozeDescription', | |||
| const ruleSnoozeDocsLink = `${ | |||
There was a problem hiding this comment.
The anchor for this is snooze-rules-ui. I'm adding docs for this feature here.
maximpn
force-pushed
the
rule-snoozing-editing-page-fixes
branch
from
06e0cf0 to
313e7e2
Compare
|
Pinging @elastic/security-detections-response (Team:Detections and Resp) |
|
Pinging @elastic/security-solution (Team: SecuritySolution) |
💚 Build Succeeded
Metrics [docs]Async chunks
Page load bundle
Unknown metric groupsESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: cc @maximpn |
KOTungseth
approved these changes
May 5, 2023
kibanamachine
pushed a commit
to kibanamachine/kibana
that referenced
this pull request
May 5, 2023
…age (elastic#155850) **Addresses:** elastic#147737 **Relates to:** elastic#155612 ## Summary After merging elastic#155612 back there is one issue is left unresolved described in this [comment](elastic#155612 (comment)): > Looks like we show the `Actions will not be preformed until it is unsnoozed` message unconditionally, i.e. regardless of whether the rule is snoozed or not. Since it's in bold it feels like a warning in the case where it doesn't really matter: > Can we hide it when the rule is not snoozed? If it's not trivial, can we make it look less dangerous by making the font regular and playing with the copy a little bit? E.g. `If snoozed actions will not be triggered`. This PR resolves rule snooze description text issue. As snooze settings are resolved outside the security solution plugin having any logic to conditionally display a message will increase the complexity. This way the message was changes to avoid any text to appear conditionally. *Before:* <img width="703" alt="Screenshot 2023-04-24 at 18 36 31" src="https://user-images.githubusercontent.com/7359339/234060523-fe9161a1-0e83-4d39-a193-81c946d95106.png"> *After:*  ### Checklist - [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) (cherry picked from commit 6714d92)
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
maximpn
added a commit
to maximpn/kibana
that referenced
this pull request
May 5, 2023
…age (elastic#155850) **Addresses:** elastic#147737 **Relates to:** elastic#155612 ## Summary After merging elastic#155612 back there is one issue is left unresolved described in this [comment](elastic#155612 (comment)): > Looks like we show the `Actions will not be preformed until it is unsnoozed` message unconditionally, i.e. regardless of whether the rule is snoozed or not. Since it's in bold it feels like a warning in the case where it doesn't really matter: > Can we hide it when the rule is not snoozed? If it's not trivial, can we make it look less dangerous by making the font regular and playing with the copy a little bit? E.g. `If snoozed actions will not be triggered`. This PR resolves rule snooze description text issue. As snooze settings are resolved outside the security solution plugin having any logic to conditionally display a message will increase the complexity. This way the message was changes to avoid any text to appear conditionally. *Before:* <img width="703" alt="Screenshot 2023-04-24 at 18 36 31" src="https://user-images.githubusercontent.com/7359339/234060523-fe9161a1-0e83-4d39-a193-81c946d95106.png"> *After:*  ### Checklist - [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) (cherry picked from commit 6714d92)
kibanamachine
added a commit
that referenced
this pull request
May 5, 2023
…ting page (#155850) (#156888) # Backport This will backport the following commits from `main` to `8.8`: - [[Security Solution] Fix rule snooze description on the rule editing page (#155850)](#155850) <!--- Backport version: 8.9.7 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Maxim Palenov","email":"[email protected]"},"sourceCommit":{"committedDate":"2023-05-05T16:14:41Z","message":"[Security Solution] Fix rule snooze description on the rule editing page (#155850)\n\n**Addresses:** https://github.com/elastic/kibana/issues/147737\r\n**Relates to:** https://github.com/elastic/kibana/pull/155612\r\n\r\n## Summary\r\n\r\nAfter merging #155612 back there is one issue is left unresolved described in this [comment](https://github.com/elastic/kibana/pull/155612#discussion_r1175545697):\r\n\r\n> Looks like we show the `Actions will not be preformed until it is unsnoozed` message unconditionally, i.e. regardless of whether the rule is snoozed or not. Since it's in bold it feels like a warning in the case where it doesn't really matter:\r\n\r\n> Can we hide it when the rule is not snoozed? If it's not trivial, can we make it look less dangerous by making the font regular and playing with the copy a little bit? E.g. `If snoozed actions will not be triggered`.\r\n\r\nThis PR resolves rule snooze description text issue. As snooze settings are resolved outside the security solution plugin having any logic to conditionally display a message will increase the complexity. This way the message was changes to avoid any text to appear conditionally.\r\n\r\n*Before:*\r\n\r\n<img width=\"703\" alt=\"Screenshot 2023-04-24 at 18 36 31\" src=\"https://user-images.githubusercontent.com/7359339/234060523-fe9161a1-0e83-4d39-a193-81c946d95106.png\">\r\n\r\n*After:*\r\n\r\n\r\n\r\n\r\n### Checklist\r\n\r\n- [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)","sha":"6714d926ee041d61c627037189263905918303b3","branchLabelMapping":{"^v8.9.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","release_note:skip","Team:Detections and Resp","Team: SecuritySolution","Feature:Rule Management","Team:Detection Rules","ui-copy","v8.8.0","v8.9.0"],"number":155850,"url":"https://github.com/elastic/kibana/pull/155850","mergeCommit":{"message":"[Security Solution] Fix rule snooze description on the rule editing page (#155850)\n\n**Addresses:** https://github.com/elastic/kibana/issues/147737\r\n**Relates to:** https://github.com/elastic/kibana/pull/155612\r\n\r\n## Summary\r\n\r\nAfter merging #155612 back there is one issue is left unresolved described in this [comment](https://github.com/elastic/kibana/pull/155612#discussion_r1175545697):\r\n\r\n> Looks like we show the `Actions will not be preformed until it is unsnoozed` message unconditionally, i.e. regardless of whether the rule is snoozed or not. Since it's in bold it feels like a warning in the case where it doesn't really matter:\r\n\r\n> Can we hide it when the rule is not snoozed? If it's not trivial, can we make it look less dangerous by making the font regular and playing with the copy a little bit? E.g. `If snoozed actions will not be triggered`.\r\n\r\nThis PR resolves rule snooze description text issue. As snooze settings are resolved outside the security solution plugin having any logic to conditionally display a message will increase the complexity. This way the message was changes to avoid any text to appear conditionally.\r\n\r\n*Before:*\r\n\r\n<img width=\"703\" alt=\"Screenshot 2023-04-24 at 18 36 31\" src=\"https://user-images.githubusercontent.com/7359339/234060523-fe9161a1-0e83-4d39-a193-81c946d95106.png\">\r\n\r\n*After:*\r\n\r\n\r\n\r\n\r\n### Checklist\r\n\r\n- [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)","sha":"6714d926ee041d61c627037189263905918303b3"}},"sourceBranch":"main","suggestedTargetBranches":["8.8"],"targetPullRequestStates":[{"branch":"8.8","label":"v8.8.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.9.0","labelRegex":"^v8.9.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/155850","number":155850,"mergeCommit":{"message":"[Security Solution] Fix rule snooze description on the rule editing page (#155850)\n\n**Addresses:** https://github.com/elastic/kibana/issues/147737\r\n**Relates to:** https://github.com/elastic/kibana/pull/155612\r\n\r\n## Summary\r\n\r\nAfter merging #155612 back there is one issue is left unresolved described in this [comment](https://github.com/elastic/kibana/pull/155612#discussion_r1175545697):\r\n\r\n> Looks like we show the `Actions will not be preformed until it is unsnoozed` message unconditionally, i.e. regardless of whether the rule is snoozed or not. Since it's in bold it feels like a warning in the case where it doesn't really matter:\r\n\r\n> Can we hide it when the rule is not snoozed? If it's not trivial, can we make it look less dangerous by making the font regular and playing with the copy a little bit? E.g. `If snoozed actions will not be triggered`.\r\n\r\nThis PR resolves rule snooze description text issue. As snooze settings are resolved outside the security solution plugin having any logic to conditionally display a message will increase the complexity. This way the message was changes to avoid any text to appear conditionally.\r\n\r\n*Before:*\r\n\r\n<img width=\"703\" alt=\"Screenshot 2023-04-24 at 18 36 31\" src=\"https://user-images.githubusercontent.com/7359339/234060523-fe9161a1-0e83-4d39-a193-81c946d95106.png\">\r\n\r\n*After:*\r\n\r\n\r\n\r\n\r\n### Checklist\r\n\r\n- [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)","sha":"6714d926ee041d61c627037189263905918303b3"}}]}] BACKPORT--> Co-authored-by: Maxim Palenov <[email protected]>
\r\n\r\n\r\n###](https://user-images.githubusercontent.com/7359339/234060523-fe9161a1-0e83-4d39-a193-81c946d95106.png\">\r\n\r\n*After:*\r\n\r\n\r\n\r\n\r\n###){kind=link}
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Addresses: #147737
Relates to: #155612
Summary
After merging #155612 back there is one issue is left unresolved described in this comment:
This PR resolves rule snooze description text issue. As snooze settings are resolved outside the security solution plugin having any logic to conditionally display a message will increase the complexity. This way the message was changes to avoid any text to appear conditionally.
Before:
After:
Checklist