Skip to content

[Security Solution][Detections] Moves last updated info inline with status filter#108096

Merged
dplumlee merged 9 commits intoelastic:masterfrom
dplumlee:last-updated-redesign
Aug 14, 2021
Merged

[Security Solution][Detections] Moves last updated info inline with status filter#108096
dplumlee merged 9 commits intoelastic:masterfrom
dplumlee:last-updated-redesign

Conversation

@dplumlee
Copy link
Copy Markdown
Contributor

@dplumlee dplumlee commented Aug 10, 2021

Summary

Related to #107923 and RAC design updates

Moves the last updated information inline with the status filters on both the alerts page and the rule details page

Screenshots

Screen Shot 2021-08-11 at 5 18 06 PM
Screen Shot 2021-08-11 at 5 28 14 PM

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@dplumlee dplumlee added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Team:Detections and Resp Security Detection Response Team Feature:Detection Alerts Security Solution Detection Alerts Feature Theme: rac label obsolete Feature:RAC label obsolete v7.15.0 labels Aug 10, 2021
@dplumlee dplumlee self-assigned this Aug 10, 2021
@dplumlee dplumlee marked this pull request as ready for review August 10, 2021 19:36
@dplumlee dplumlee requested a review from a team as a code owner August 10, 2021 19:36
@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/security-detections-response (Team:Detections and Resp)

@dplumlee
Copy link
Copy Markdown
Contributor Author

@elasticmachine merge upstream

@dplumlee dplumlee changed the title [Security Solution][Detections] Moves last alert info inline with status filter [Security Solution][Detections] Moves last updated info inline with status filter Aug 11, 2021
@dplumlee dplumlee force-pushed the last-updated-redesign branch from cf6ee50 to 6357660 Compare August 11, 2021 21:30
Copy link
Copy Markdown
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.

Hey thanks for doing this! Just a quick update. I think you need to get the updatedAt parameter from the table now instead of the the redux selectors. @kqualters-elastic might be able to confirm.

If you take a look at the file associated with the comment here, you'll see what I mean: https://github.com/elastic/kibana/pull/107727/files#r685628747

I would re-introduce that updatedAt property that was removed and thein either use it to update a property in redux or use context to get the data from useTimelineEvents. For sake of ease, I would just store it in context

I would just do what @kqualters-elastic said below

Copy link
Copy Markdown
Contributor

@kqualters-elastic kqualters-elastic left a comment

Choose a reason for hiding this comment

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

Once the LastUpdatedAt component is used asynchronously, the rest of the changes lgtm

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I know this is by far the easiest way to do this, but the timelines bundle is already much bigger than it should be. Would you mind loading this component in security_solution asynchronously? It's not hard to do, see public/plugin.ts and and public/methods/index.ts for examples. and even just adding this 1 component to the top level public/index.ts increases the bundle size by an almost comical 200kb

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@michaelolo24 's suggestion works as well

@dplumlee dplumlee requested a review from a team as a code owner August 12, 2021 14:18
@dplumlee dplumlee force-pushed the last-updated-redesign branch from bde6c12 to d82f9fd Compare August 12, 2021 14:33
@dplumlee dplumlee force-pushed the last-updated-redesign branch from d82f9fd to 5aeb5e0 Compare August 12, 2021 20:00
@dplumlee dplumlee requested a review from michaelolo24 August 12, 2021 22:11
@dplumlee dplumlee added the auto-backport Deprecated - use backport:version if exact versions are needed label Aug 13, 2021
@dplumlee dplumlee enabled auto-merge (squash) August 13, 2021 01:44
@michaelolo24
Copy link
Copy Markdown
Contributor

Thanks for getting these changes in! @dplumlee and I spoke and we noted the changes that need to be made to update the loading state of the value in an upcoming PR.

Just noting: It loads initially based on logic within security solution for the old table, and then shows 52 years due to that information being undefined when tGridEnabled is true. Then the data from the new table updates from the tGrid side, giving the actual value. Options for fixing this include placing the logic for this behind the tGridEnabled feature flag or removing it when the rest of the old implementation is removed.

@kibanamachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
securitySolution 2362 2360 -2

Async chunks

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

id before after diff
securitySolution 6.5MB 6.5MB -233.0B
timelines 308.7KB 309.0KB +271.0B
total +38.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
securitySolution 211.9KB 212.1KB +170.0B
timelines 309.5KB 309.8KB +338.0B
total +508.0B

History

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

cc @dplumlee

@kibanamachine
Copy link
Copy Markdown
Contributor

💚 Backport successful

Status Branch Result
7.x

This backport PR will be merged automatically after passing CI.

@dplumlee dplumlee deleted the last-updated-redesign branch August 14, 2021 07:09
kibanamachine added a commit that referenced this pull request Aug 14, 2021
…tatus filter (#108096) (#108627)

Co-authored-by: Davis Plumlee <56367316+dplumlee@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-backport Deprecated - use backport:version if exact versions are needed Feature:Detection Alerts Security Solution Detection Alerts Feature Feature:RAC label obsolete release_note:skip Skip the PR/issue when compiling release notes Team:Detections and Resp Security Detection Response Team Theme: rac label obsolete v7.15.0 v8.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants