Skip to content

[Security Solution][All] Replace old markdown renderer with the new one#80301

Merged
cnasikas merged 6 commits intoelastic:masterfrom
cnasikas:replace_markdown
Oct 21, 2020
Merged

[Security Solution][All] Replace old markdown renderer with the new one#80301
cnasikas merged 6 commits intoelastic:masterfrom
cnasikas:replace_markdown

Conversation

@cnasikas
Copy link
Copy Markdown
Member

@cnasikas cnasikas commented Oct 13, 2020 •
edited
Loading

Summary

This PR introduced the new EuiMarkdownEditor for cases. Some components in the Security Solution app still uses the old markdown editor. This PR removes the old markdown and replace it with the new one.

Note: Markdown URLs are enabled on the case widget on the overview page. Before this PR there were not enabled (rendered).

Screenshot 2020-10-13 at 12 00 13 PM

Screenshot 2020-10-13 at 12 01 31 PM

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@cnasikas cnasikas added Team:SIEM v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.11.0 Team:Detections and Resp Security Detection Response Team Team:Threat Hunting Security Solution Threat Hunting Team labels Oct 13, 2020
@cnasikas cnasikas requested review from a team as code owners October 13, 2020 09:02
@cnasikas cnasikas self-assigned this Oct 13, 2020
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Oct 13, 2020

Pinging @elastic/siem (Team:SIEM)

@cnasikas cnasikas requested a review from stephmilovic October 13, 2020 09:05
yctercero
yctercero reviewed Oct 13, 2020
View reviewed changes
...ugins/security_solution/public/detections/components/rules/step_about_rule_details/index.tsx Outdated
Copy link
Copy Markdown
Contributor

@yctercero yctercero Oct 13, 2020

Choose a reason for hiding this comment

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

👍 thank you!

yctercero
yctercero approved these changes Oct 13, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@yctercero yctercero left a comment

Choose a reason for hiding this comment

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

LGTM (Note - just focused on use in rules, not cases)! Pulled down and tested markdown on rule creation, made sure it shows up in rule details and that a new note is appended to a timeline opened from an alert where the rule has an investigation guide 👍 Thanks for updating this!

stephmilovic
stephmilovic approved these changes Oct 13, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@stephmilovic stephmilovic left a comment

Choose a reason for hiding this comment

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

Code review and manual testing both LGTM. Thanks @cnasikas !!

@cnasikas cnasikas added the Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. label Oct 16, 2020
@cnasikas
Copy link
Copy Markdown
Member Author

cnasikas commented Oct 19, 2020

@elasticmachine merge upstream

@cnasikas cnasikas force-pushed the replace_markdown branch 2 times, most recently from 2ac9079 to 718630e Compare October 20, 2020 08:20
@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented Oct 20, 2020

💚 Build Succeeded

Metrics [docs]

@kbn/optimizer bundle module count

id before after diff
securitySolution 2057 2060 +3

async chunks size

id before after diff
securitySolution 8.1MB 8.1MB -2.7KB

History

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

andrew-goldstein
andrew-goldstein reviewed Oct 20, 2020
View reviewed changes
x-pack/plugins/security_solution/public/common/components/markdown_editor/markdown_link.tsx
<EuiLink
href={disableLinks ? undefined : href}
data-test-subj="markdown-link"
rel={`${REL_NOFOLLOW}`}
Copy link
Copy Markdown
Contributor

@andrew-goldstein andrew-goldstein Oct 20, 2020

Choose a reason for hiding this comment

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

nit: consider dropping the extraneous template, i.e.

rel={REL_NOFOLLOW}

andrew-goldstein
andrew-goldstein approved these changes Oct 20, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@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 migrating the rest of the Security solution to the EUI Markdown formatter and renderer @cnasikas! 🙏

Desk tested:

  • global timeline notes
  • inline timeline notes
  • case notes
  • rendering case descriptions w/links in the Overview
  • external link rendering
  • timeline link rendering
  • timeline Analyze event-link rendering
  • x-browser markdown formatting (the example below is from FF):
    formatting-firefox

LGTM 🚀 📓 ✏️

@cnasikas cnasikas merged commit ad2fe78 into elastic:master Oct 21, 2020
@cnasikas cnasikas deleted the replace_markdown branch October 21, 2020 07:32
cnasikas added a commit to cnasikas/kibana that referenced this pull request Oct 21, 2020
@cnasikas
[Security Solution][All] Replace old markdown renderer with the new o…
07d5719 
…ne (elastic#80301)
@cnasikas cnasikas mentioned this pull request Oct 21, 2020
Merged
cnasikas added a commit that referenced this pull request Oct 21, 2020
@cnasikas
[7.x] [Security Solution][All] Replace old markdown renderer with the…
2a09061 
… new one (#80301) (#81280)
jloleysens added a commit to jloleysens/kibana that referenced this pull request Oct 21, 2020
@jloleysens
Merge branch 'master' of github.com:elastic/kibana into ilm/migrate-w…
805a55d 
…arm-phase-to-formlib

* 'master' of github.com:elastic/kibana: (55 commits)
  [UX] Fix map color variance and apply proper filter for extended stats (elastic#81106)
  [User Experience] Use EuiSelect for percentiles instead of SuperSelect (elastic#81082)
  [DOCS] Add link for monitoring ssl settings (elastic#81057)
  [test] Await loading indicator in monitoring test (elastic#81279)
  [ILM] Minor copy and link additions to cloud CTA for cold phase (elastic#80512)
  [Mappings editor] Add scaled_float and date_range comp integration tests (elastic#81287)
  [Discover] Deangularize context.app (elastic#80851)
  [O11y Overview] Add code to display/hide UX section when appropriate (elastic#80873)
  [Discover] Extend DiscoverNoResults component to show different message on error (elastic#79671)
  Fix tagcloud word overlapping (elastic#81161)
  [Security Solution] Fixes flaky test rules (elastic#81040)
  Changed the code to avoid tech debt with hacky solutions after receiving comments on EUI issue reported about this problem. (elastic#81183)
  [Security Solution][All] Replace old markdown renderer with the new one (elastic#80301)
  Add namespaced version of the API call (elastic#81278)
  [ML] Data Frame Analytics: Fix race condition and support for feature influence legacy format. (elastic#81123)
  [Fleet] Fix POLICY_CHANGE action creation for new policy (elastic#81236)
  [Security Solution][Endpoint][Admin] Malware user notification checkbox (elastic#78084)
  [SecuritySolution][Unit Tests] - fix flakey unit test (elastic#81239)
  skip flaky suite (elastic#81264)
  [Maps] fix top-level Map page is called 'Kibana' (elastic#81238)
  ...

# Conflicts:
#	x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/phases/hot_phase/hot_phase.tsx
#	x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/phases/shared/forcemerge_field.tsx
#	x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/phases/warm_phase.tsx
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

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

@stephmilovic stephmilovic stephmilovic approved these changes

@yctercero yctercero yctercero approved these changes

Assignees

@cnasikas cnasikas

Labels

release_note:skip Skip the PR/issue when compiling release notes Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:SIEM Team:Threat Hunting Security Solution Threat Hunting Team v7.11.0 v8.0.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@cnasikas @elasticmachine @kibanamachine @andrew-goldstein @stephmilovic @yctercero