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

[SIEM][Detections Engine] - Add rule markdown field to rule create, detail, and edit flows #60108

Merged
merged 14 commits into from
Mar 17, 2020
Merged

[SIEM][Detections Engine] - Add rule markdown field to rule create, detail, and edit flows #60108

merged 14 commits into from
Mar 17, 2020

Conversation

yctercero
Copy link
Contributor

@yctercero yctercero commented Mar 13, 2020 •
edited
Loading

Summary

This is part of #59176 - breaking up into backend and frontend PRs.
Backend PR - #59796

Problem to solve/Customer Benefit: Analysts need as much context as possible when investigating signals. If a richer format of information can be provided to them via their UX, then they may become more effective at completing investigations and cases.

Add markdown on rule creation:

rule_markdown_create

Edit rule markdown:

rule_markdown_edit

Rule details page:

rule_markdown_details

Updates to details page:

rule_details

  1. Panels shifted. Toggle will only display if there are investigation notes.
  2. Tabs moved to be below rule detail panels. Panels will now display on both "Detected signals" and "Failure History". Also note that text for tab "Detected signals" changed per designs from the previous text - "Signals (SIEM Detections)".

Checklist

Delete any items that are not applicable to this PR.

@yctercero yctercero self-assigned this Mar 13, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/siem (Team:SIEM)

@yctercero yctercero marked this pull request as ready for review March 13, 2020 14:49
@dhurley14
Copy link
Contributor

Testing locally I saw there was an issue with checkboxes

markdown_checkbox_bug

Just an FYI. Everything else looks good though! Thanks.

dhurley14
dhurley14 approved these changes Mar 13, 2020
View reviewed changes
Copy link
Contributor

@dhurley14 dhurley14 left a comment

Choose a reason for hiding this comment

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

I think the cypress tests need updating to get CI to pass. Once CI passes I think this is good to go!

FrankHassanabad
FrankHassanabad suggested changes Mar 13, 2020
View reviewed changes
Copy link
Contributor

@FrankHassanabad FrankHassanabad left a comment

Choose a reason for hiding this comment

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

I would remove some of the dead tests you have and then I will approve. Probably the CI system will block you there anyways and you will not be able to do it regardless.

...y/plugins/siem/public/pages/detection_engine/rules/components/step_about_rule/index.test.tsx Outdated Show resolved Hide resolved
yctercero
yctercero commented Mar 15, 2020
View reviewed changes
Copy link
Contributor Author

@yctercero yctercero left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Testing locally I saw there was an issue with checkboxes

markdown_checkbox_bug

Just an FYI. Everything else looks good though! Thanks.

@dhurley14 Thanks for pointing that out. I took a look and I think that is the desired behavior. Because it is meant to be readonly, you are allowed to display checkboxes, and checked checkboxes, but I don't believe the user is supposed to be able to check and uncheck as that state would not be saved.

@yctercero
Copy link
Contributor Author

@elasticmachine merge upstream

FrankHassanabad
FrankHassanabad approved these changes Mar 16, 2020
View reviewed changes
Copy link
Contributor

@FrankHassanabad FrankHassanabad 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 the unit tests when there wasn't any. Thank you for the cleanups and the changes from everyone and the extra efforts on this one.

LGTM!

@yctercero
Copy link
Contributor Author

@FrankHassanabad huge thanks for all the feedback and helping debug the circular dep check failure. I've gone ahead and pushed the changes to exclude the test files. I also implemented the changes we spoke about offline.

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@yctercero yctercero merged commit cea277e into elastic:master Mar 17, 2020
@yctercero yctercero mentioned this pull request Mar 17, 2020
Merged
yctercero added a commit to yctercero/kibana that referenced this pull request Mar 17, 2020
@yctercero @MadameSheema @elasticmachine
[SIEM][Detections Engine] - Add rule markdown field to rule create, d…
2dba15e 
…etail, and edit flows (elastic#60108)

* add rule note markdown field to rule creation, rule details, and rule edit flows

Co-authored-by: Gloria Hornero <snootchie.boochies@gmail.com>
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
gmmorris added a commit to gmmorris/kibana that referenced this pull request Mar 17, 2020
@gmmorris
Merge branch 'master' into alerting/tls-warning
08c05b6 
* master:
  [SIEM] Adds 'Closes one signal when more than one opened signals are selected' test again (elastic#60380)
  [SIEM][Detections Engine] - Add rule markdown field to rule create, detail, and edit flows (elastic#60108)
  [Fleet] Add config revision to fleet agents (elastic#60292)
  Allow kbn-config-schema to ignore unknown keys (elastic#59560)
  [ML] Functional tests - disable df analytics clone tests
  skip flaky suite (elastic#58643) (elastic#58991)
  [FTR] Add support for --include and --exclude files via tags (elastic#60123)
  [SIEM] Fix link on overview page (elastic#60348)
  skip flaky test (elastic#60369)
  [Endpoint] Adds take action dropdown and tests to alert details flyout (elastic#59242)
  [Lens] Simplify state management from visualization (elastic#58279)
  Changing default type to start and allowing it to be configured by the event category (elastic#60323)
  [ML] Adds the class_assignment_objective to classification (elastic#60358)
gmmorris added a commit to gmmorris/kibana that referenced this pull request Mar 17, 2020
@gmmorris
Merge branch 'master' into alerting/view-in-app
3c7bea5 
* master: (51 commits)
  do not update cell background if is label cell (elastic#60308)
  FTR configurable test users (elastic#52431)
  [Reporting] Wholesale moves client to newest-platform (elastic#58945)
  [Ingest] Support `show_user` package registry flag (elastic#60338)
  [SIEM] Adds 'Closes one signal when more than one opened signals are selected' test again (elastic#60380)
  [SIEM][Detections Engine] - Add rule markdown field to rule create, detail, and edit flows (elastic#60108)
  [Fleet] Add config revision to fleet agents (elastic#60292)
  Allow kbn-config-schema to ignore unknown keys (elastic#59560)
  [ML] Functional tests - disable df analytics clone tests
  skip flaky suite (elastic#58643) (elastic#58991)
  [FTR] Add support for --include and --exclude files via tags (elastic#60123)
  [SIEM] Fix link on overview page (elastic#60348)
  skip flaky test (elastic#60369)
  [Endpoint] Adds take action dropdown and tests to alert details flyout (elastic#59242)
  [Lens] Simplify state management from visualization (elastic#58279)
  Changing default type to start and allowing it to be configured by the event category (elastic#60323)
  [ML] Adds the class_assignment_objective to classification (elastic#60358)
  [TSVB] fix text color when using custom background color (elastic#60261)
  Fix import to timefilter from in TSVB (elastic#60296)
  [NP] Get rid of usage redirectWhenMissing service (elastic#59777)
  ...
gmmorris added a commit to gmmorris/kibana that referenced this pull request Mar 17, 2020
@gmmorris
Merge branch 'alerting/view-in-app' into alerting/api-constant
1ddeae4 
* alerting/view-in-app: (53 commits)
  fixed typo
  handle optional alerting plugin
  do not update cell background if is label cell (elastic#60308)
  FTR configurable test users (elastic#52431)
  [Reporting] Wholesale moves client to newest-platform (elastic#58945)
  [Ingest] Support `show_user` package registry flag (elastic#60338)
  [SIEM] Adds 'Closes one signal when more than one opened signals are selected' test again (elastic#60380)
  [SIEM][Detections Engine] - Add rule markdown field to rule create, detail, and edit flows (elastic#60108)
  [Fleet] Add config revision to fleet agents (elastic#60292)
  Allow kbn-config-schema to ignore unknown keys (elastic#59560)
  [ML] Functional tests - disable df analytics clone tests
  skip flaky suite (elastic#58643) (elastic#58991)
  [FTR] Add support for --include and --exclude files via tags (elastic#60123)
  [SIEM] Fix link on overview page (elastic#60348)
  skip flaky test (elastic#60369)
  [Endpoint] Adds take action dropdown and tests to alert details flyout (elastic#59242)
  [Lens] Simplify state management from visualization (elastic#58279)
  Changing default type to start and allowing it to be configured by the event category (elastic#60323)
  [ML] Adds the class_assignment_objective to classification (elastic#60358)
  [TSVB] fix text color when using custom background color (elastic#60261)
  ...
yctercero added a commit that referenced this pull request Mar 18, 2020
@yctercero @MadameSheema @elasticmachine
[SIEM][Detections Engine] - Add rule markdown field to rule create, d…
1546a85 
…etail, and edit flows (#60108) (#60416)

* add rule note markdown field to rule creation, rule details, and rule edit flows

Co-authored-by: Gloria Hornero <snootchie.boochies@gmail.com>
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
@yctercero yctercero mentioned this pull request Mar 26, 2020
Merged
5 tasks
@yctercero yctercero deleted the rule_markdown_de_fe branch July 20, 2020 01:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@FrankHassanabad FrankHassanabad FrankHassanabad approved these changes

@dhurley14 dhurley14 dhurley14 approved these changes

Assignees

@yctercero yctercero

Labels
release_note:enhancement Team:SIEM v7.7.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@yctercero @elasticmachine @dhurley14 @kibanamachine @FrankHassanabad @MadameSheema