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

[Security Solution] Fix missing title on inspect pop-up #143601

Merged
merged 20 commits into from
Oct 20, 2022
Merged

[Security Solution] Fix missing title on inspect pop-up #143601

merged 20 commits into from
Oct 20, 2022

Conversation

christineweng
Copy link
Contributor

@christineweng christineweng commented Oct 18, 2022 •
edited
Loading

This PR contains fixes for #141617

The bug was first discovered under Sessions, and similar behavior is found on other sections where T-Grid is used. This was fixed by including a translated title when T-Grid is created (createTimeline) or initialized (initializeTGridSettings), and making it persistent for subsequent timeline actions.

After

  • Hosts -> Sessions
    image

  • Hosts -> Events -> Events table
    image

  • Alerts -> Alerts table
    image

  • Rules -> Alerts table
    image

Checklist

@christineweng christineweng added bug Fixes for quality problems that affect the customer experience release_note:skip Skip the PR/issue when compiling release notes Team:Threat Hunting Security Solution Threat Hunting Team Team:Threat Hunting:Investigations Security Solution Investigations Team v8.6.0 labels Oct 18, 2022
@christineweng christineweng self-assigned this Oct 18, 2022
@christineweng
Copy link
Contributor Author

@elasticmachine merge upstream

@christineweng christineweng marked this pull request as ready for review October 19, 2022 22:56
@christineweng christineweng requested a review from a team October 19, 2022 22:56
@christineweng christineweng requested review from a team as code owners October 19, 2022 22:56
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-threat-hunting (Team:Threat Hunting)

@christineweng christineweng enabled auto-merge (squash) October 19, 2022 22:58
michaelolo24
michaelolo24 requested changes Oct 20, 2022
View reviewed changes
Copy link
Contributor

@michaelolo24 michaelolo24 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 making these changes! The fix for the inspect modal looks good, I would just undo the onComponentInitialized removal 😄

michaelolo24
michaelolo24 approved these changes Oct 20, 2022
View reviewed changes
Copy link
Contributor

@michaelolo24 michaelolo24 left a comment

Choose a reason for hiding this comment

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

Awesome, thanks for making the changes!

semd
semd approved these changes Oct 20, 2022
View reviewed changes
Copy link
Contributor

@semd semd left a comment

Choose a reason for hiding this comment

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

LGTM! thanks Christine 🚀

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
securitySolution 10.0MB 10.0MB +351.0B

History

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

cc @christineweng

opauloh
opauloh approved these changes Oct 20, 2022
View reviewed changes
Copy link
Contributor

@opauloh opauloh left a comment

Choose a reason for hiding this comment

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

@awp-viz LGTM - Thanks for this fix!

@christineweng christineweng merged commit 2d6c617 into elastic:main Oct 20, 2022
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Oct 20, 2022
jloleysens added a commit to jloleysens/kibana that referenced this pull request Oct 24, 2022
@jloleysens
Merge branch 'main' into files-upload-design-feedback
da22f2f 
* main: (57 commits)
  [Files] Filepicker (elastic#143111)
  [Infrastructure UI] Replace Lens table with EUI table and own api (elastic#142871)
  [api-docs] Daily api_docs build (elastic#143829)
  [api-docs] Daily api_docs build (elastic#143825)
  [api-docs] Daily api_docs build (elastic#143823)
  [Security Solution] Restructuring folders of Detection Engine + refactoring Rule Management (elastic#142950)
  [Dev tools] Fix performance issue with autocomplete suggestions (elastic#143428)
  [Security Solution] Disable ML rule's edit button link under basic license (elastic#143260)
  [Lens]  Use the language-documentation package for formula (elastic#143649)
  [api-docs] Daily api_docs build (elastic#143811)
  [Security Solution] Fix missing title on inspect pop-up (elastic#143601)
  fix incorrect filters being passed to events table causing duplicate entries in our inpsect tool request tab (elastic#143239)
  [Security Solution][Endpoint] `get-file` response action kibana download file API (elastic#143708)
  Rely on refresh context to update stats independently of overview cards. (elastic#143308)
  [RAM] Rule event log - Fix incorrect results when filtering by message and outcome simultaneously (elastic#143119)
  [ML] Display link to create data view from error cases in data frame analytics results pages (elastic#143596)
  Update links in README :) (elastic#143675)
  Add more tests for ml_inference_logic (elastic#143764)
  skip failing test suite (elastic#143717)
  [DOCS] Add assignees to case APIs (elastic#143610)
  ...
@christineweng christineweng deleted the BUG-Inspect-Modal-Title branch October 27, 2022 14:18
@christineweng christineweng restored the BUG-Inspect-Modal-Title branch October 27, 2022 14:19
@christineweng christineweng mentioned this pull request Nov 28, 2022
Merged
christineweng added a commit that referenced this pull request Nov 29, 2022
@christineweng
[Security Solution] Inspect button title missing (#146473)
32d1dbb 
This PR is a quick bug fix for
#141617

- #143601 fixes the inspect title
issue by passing a 'title' prop to `initializeTGridSettings`.
- It appears subsequent enhancements in `sessions_viewer` also calls
`initializeTGridSettings`, but in a separate manner. This PR combines
the two t-grid initialization calls into one.


![image](https://user-images.githubusercontent.com/18648970/204374253-2c98f7c7-76fd-4b60-92ae-367464e64abf.png)
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Nov 29, 2022
@christineweng
[Security Solution] Inspect button title missing (elastic#146473)
1794f14 
This PR is a quick bug fix for
elastic#141617

- elastic#143601 fixes the inspect title
issue by passing a 'title' prop to `initializeTGridSettings`.
- It appears subsequent enhancements in `sessions_viewer` also calls
`initializeTGridSettings`, but in a separate manner. This PR combines
the two t-grid initialization calls into one.

![image](https://user-images.githubusercontent.com/18648970/204374253-2c98f7c7-76fd-4b60-92ae-367464e64abf.png)

(cherry picked from commit 32d1dbb)
kibanamachine added a commit that referenced this pull request Nov 29, 2022
@kibanamachine @christineweng
[8.6] [Security Solution] Inspect button title missing (#146473) (#14…
f7d8bee 
…6490)

# Backport

This will backport the following commits from `main` to `8.6`:
- [[Security Solution] Inspect button title missing
(#146473)](#146473)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT
[{"author":{"name":"christineweng","email":"[email protected]"},"sourceCommit":{"committedDate":"2022-11-29T00:36:40Z","message":"[Security
Solution] Inspect button title missing (#146473)\n\nThis PR is a quick
bug fix for\r\nhttps://github.com//issues/141617\r\n\r\n-
#143601 fixes the inspect
title\r\nissue by passing a 'title' prop to
`initializeTGridSettings`.\r\n- It appears subsequent enhancements in
`sessions_viewer` also calls\r\n`initializeTGridSettings`, but in a
separate manner. This PR combines\r\nthe two t-grid initialization calls
into
one.\r\n\r\n\r\n![image](https://user-images.githubusercontent.com/18648970/204374253-2c98f7c7-76fd-4b60-92ae-367464e64abf.png)","sha":"32d1dbb1f98053ac34d278d945034363695227c3","branchLabelMapping":{"^v8.7.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","release_note:skip","Team:Threat
Hunting","Team: SecuritySolution","Team:Threat
Hunting:Investigations","v8.6.0","v8.7.0"],"number":146473,"url":"https://github.com/elastic/kibana/pull/146473","mergeCommit":{"message":"[Security
Solution] Inspect button title missing (#146473)\n\nThis PR is a quick
bug fix for\r\nhttps://github.com//issues/141617\r\n\r\n-
#143601 fixes the inspect
title\r\nissue by passing a 'title' prop to
`initializeTGridSettings`.\r\n- It appears subsequent enhancements in
`sessions_viewer` also calls\r\n`initializeTGridSettings`, but in a
separate manner. This PR combines\r\nthe two t-grid initialization calls
into
one.\r\n\r\n\r\n![image](https://user-images.githubusercontent.com/18648970/204374253-2c98f7c7-76fd-4b60-92ae-367464e64abf.png)","sha":"32d1dbb1f98053ac34d278d945034363695227c3"}},"sourceBranch":"main","suggestedTargetBranches":["8.6"],"targetPullRequestStates":[{"branch":"8.6","label":"v8.6.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.7.0","labelRegex":"^v8.7.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/146473","number":146473,"mergeCommit":{"message":"[Security
Solution] Inspect button title missing (#146473)\n\nThis PR is a quick
bug fix for\r\nhttps://github.com//issues/141617\r\n\r\n-
#143601 fixes the inspect
title\r\nissue by passing a 'title' prop to
`initializeTGridSettings`.\r\n- It appears subsequent enhancements in
`sessions_viewer` also calls\r\n`initializeTGridSettings`, but in a
separate manner. This PR combines\r\nthe two t-grid initialization calls
into
one.\r\n\r\n\r\n![image](https://user-images.githubusercontent.com/18648970/204374253-2c98f7c7-76fd-4b60-92ae-367464e64abf.png)","sha":"32d1dbb1f98053ac34d278d945034363695227c3"}}]}]
BACKPORT-->

Co-authored-by: christineweng <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@opauloh opauloh opauloh approved these changes

@semd semd semd approved these changes

@michaelolo24 michaelolo24 michaelolo24 approved these changes

Assignees

@christineweng christineweng

Labels
backport:skip This commit does not require backporting bug Fixes for quality problems that affect the customer experience release_note:skip Skip the PR/issue when compiling release notes Team:Threat Hunting:Investigations Security Solution Investigations Team Team:Threat Hunting Security Solution Threat Hunting Team v8.6.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@christineweng @elasticmachine @kibana-ci @michaelolo24 @semd @opauloh @kibanamachine