Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[DOCS] Reason statement shown in alert rendered view #2540

Merged
merged 22 commits into from
Oct 18, 2022
Merged

[DOCS] Reason statement shown in alert rendered view #2540

merged 22 commits into from
Oct 18, 2022

Conversation

nastasha-solomon
Copy link
Contributor

@nastasha-solomon nastasha-solomon commented Oct 4, 2022 •
edited
Loading

Addresses #2479.

Previews:

@nastasha-solomon nastasha-solomon self-assigned this Oct 4, 2022
@github-actions
Copy link

github-actions bot commented Oct 4, 2022

Documentation previews:

@nastasha-solomon nastasha-solomon linked an issue Oct 5, 2022 that may be closed by this pull request
[DOCS] Reason statement shown in alert rendered view #2479
Closed
@nastasha-solomon nastasha-solomon marked this pull request as ready for review October 6, 2022 20:42
benironside
benironside reviewed Oct 10, 2022
View reviewed changes
Copy link
Contributor

@benironside benironside left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, left some suggestions for your consideration, might have gone a bit overboard.

docs/detections/alerts-ui-manage.asciidoc Outdated Show resolved Hide resolved
docs/detections/alerts-view-details.asciidoc Outdated Show resolved Hide resolved
docs/detections/alerts-view-details.asciidoc Outdated Show resolved Hide resolved
andrew-goldstein
andrew-goldstein approved these changes Oct 10, 2022
View reviewed changes
Copy link

@andrew-goldstein andrew-goldstein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for documenting this feature @nastasha-solomon!
LGTM 🚀

nastasha-solomon
nastasha-solomon commented Oct 11, 2022
View reviewed changes
Copy link
Contributor Author

@nastasha-solomon nastasha-solomon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left self a comment to update the alert-details-flyout.png image.

cc: @janmonschke

docs/detections/alerts-view-details.asciidoc Outdated Show resolved Hide resolved
joepeeples
joepeeples reviewed Oct 12, 2022
View reviewed changes
Copy link
Contributor

@joepeeples joepeeples left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! A few comments, and also wondering if we could slightly scale down the main image of the flyout (alert-details-flyout.png); on my machine it takes up nearly the whole screen, which can be disorienting. I couldn't add a comment to the diff, but maybe width of 85% or so would give the page a little more breathing room.

docs/detections/alerts-view-details.asciidoc Outdated Show resolved Hide resolved
docs/detections/alerts-view-details.asciidoc Outdated Show resolved Hide resolved
docs/detections/alerts-view-details.asciidoc Outdated Show resolved Hide resolved
@nastasha-solomon nastasha-solomon mentioned this pull request Oct 13, 2022
Closed
28 tasks
@nastasha-solomon nastasha-solomon added the readyforQA PRs that are ready for QA review. label Oct 17, 2022
@sukhwindersingh-qasource sukhwindersingh-qasource added QA:Validated Issue has been Validated by QA Team and removed readyforQA PRs that are ready for QA review. labels Oct 18, 2022
@sukhwindersingh-qasource
Copy link

Hi @nastasha-solomon ,

We have validated the above Kibana documentation with reference to this PR and found that the doc is correctly updated.

QA

Hence, we are marking this PR as QA:Approved.

Thank you!

jmikell821
jmikell821 approved these changes Oct 18, 2022
View reviewed changes
Copy link
Contributor

@jmikell821 jmikell821 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 👍

@nastasha-solomon nastasha-solomon merged commit 7f1dd07 into main Oct 18, 2022
@nastasha-solomon nastasha-solomon deleted the issue-2479-alert-event-renderer branch October 18, 2022 20:42
@mergify mergify bot mentioned this pull request Oct 18, 2022
Merged
mergify bot pushed a commit that referenced this pull request Oct 18, 2022
@nastasha-solomon @mergify
[DOCS] Reason statement shown in alert rendered view (#2540)
860b2cb 
Co-authored-by: Benjamin Ironside Goldstein <91905639+benironside@users.noreply.github.com>
Co-authored-by: Joe Peeples <joe.peeples@elastic.co>
(cherry picked from commit 7f1dd07)
nastasha-solomon added a commit that referenced this pull request Oct 18, 2022
@mergify @benironside
@joepeeples @nastasha-solomon
[8.5] [DOCS] Reason statement shown in alert rendered view (backport #…
dc78a2c 
…2540) (#2597)

Co-authored-by: Benjamin Ironside Goldstein <91905639+benironside@users.noreply.github.com>
Co-authored-by: Joe Peeples <joe.peeples@elastic.co>
Co-authored-by: nastasha-solomon <79124755+nastasha-solomon@users.noreply.github.com>
@joepeeples joepeeples mentioned this pull request Oct 19, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joepeeples joepeeples joepeeples left review comments

@benironside benironside benironside left review comments

@andrew-goldstein andrew-goldstein andrew-goldstein approved these changes

@jmikell821 jmikell821 jmikell821 approved these changes

Assignees

@nastasha-solomon nastasha-solomon

Labels
QA:Validated Issue has been Validated by QA Team Team: Docs Team: Threat Hunting Formerly Data Visibility v8.5.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[DOCS] Reason statement shown in alert rendered view
6 participants
@nastasha-solomon @sukhwindersingh-qasource @andrew-goldstein @jmikell821 @joepeeples @benironside