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][Endpoint] New Event Filters sub-section under Administration area #97903

Merged
merged 15 commits into from
Apr 23, 2021
Merged

[Security solution][Endpoint] New Event Filters sub-section under Administration area #97903

merged 15 commits into from
Apr 23, 2021

Conversation

paul-tavares
Copy link
Contributor

@paul-tavares paul-tavares commented Apr 21, 2021 •
edited
Loading

Summary

  • Add a new sub-tab to the Administration area for Event Filters (WIP and hidden behind the eventFilteringEnabled experimental feature)
  • A new generic component for displaying paginated lists of content. Each item in the content will be rendered using a Component that is also defined as a prop.

olm-954-event-filters-sub-tab

Checklist

Delete any items that are not applicable to this PR.

@paul-tavares paul-tavares added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Team:Defend Workflows “EDR Workflows” sub-team of Security Solution v7.14.0 auto-backport Deprecated - use backport:version if exact versions are needed labels Apr 21, 2021
@paul-tavares paul-tavares self-assigned this Apr 21, 2021
dasansol92
dasansol92 reviewed Apr 22, 2021
View reviewed changes
x-pack/plugins/security_solution/public/management/common/routing.ts Outdated Show resolved Hide resolved
x-pack/plugins/security_solution/public/management/common/routing.ts Outdated Show resolved Hide resolved
...ugins/security_solution/public/management/components/paginated_content/paginated_content.tsx
<div>
<EuiSpacer size="l" />

<EuiTablePagination
Copy link
Contributor

Choose a reason for hiding this comment

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

out of curiosity - Is there an specific reason why you are using this instead of EuiPagination

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just copied it from Trusted apps. I looked at EuiPagination, but it only gives you the pagination portion, not the items per page dropdown. Late yesterday I noticed that we don't have control over the label on the "Pick page Size" dropdown and that the label being used now ("Items per row") is a bit specific to tables and we might want this component to allow control of it.

What do you think? create a custom pagination component for use by this PaginatedContent component?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, I see... What about a custom component with EuiPagination and a dropdown for page size?

Copy link
Contributor

Choose a reason for hiding this comment

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

Something like this: https://elastic.github.io/eui/#/navigation/pagination#customizable-pagination

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I think that will likely be better and will decouple us from the Pagination component used in Tables.
I will include that change in next PR that will follow this one
Thanks for the feedback.

...ugins/security_solution/public/management/components/paginated_content/paginated_content.tsx Outdated
// eslint-disable-next-line @typescript-eslint/no-explicit-any
type ComponentWithAnyProps = ComponentType<any>;

export interface PaginatedContentProps<T, C extends ComponentWithAnyProps> {
Copy link
Contributor

Choose a reason for hiding this comment

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

This component will allow us to reuse a lot of code between trusted apps, event filters and upcoming listings 👍 😃

@dasansol92 dasansol92 mentioned this pull request Apr 23, 2021
Merged
6 tasks
@paul-tavares paul-tavares marked this pull request as ready for review April 23, 2021 13:40
@paul-tavares paul-tavares requested a review from a team as a code owner April 23, 2021 13:40
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-onboarding-and-lifecycle-mgt (Team:Onboarding and Lifecycle Mgt)

@dasansol92
Copy link
Contributor

LGTM!! 🔥 🚢 🥳

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
securitySolution 2167 2172 +5

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.9MB 7.0MB +4.3KB

History

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

cc @paul-tavares

@paul-tavares paul-tavares merged commit 485692d into elastic:master Apr 23, 2021
@paul-tavares paul-tavares deleted the task/olm-954-event-filters-list-view branch April 23, 2021 15:56
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Apr 23, 2021
@paul-tavares @kibanamachine
[Security solution][Endpoint] New Event Filters sub-section under Adm…
da8d24c 
…inistration area (elastic#97903)

* Add Event Filters section to the Admin area (behind feature flag)
* new `PaginatedContent` generic component
* Refactor Trusted Apps grid view to use PaginatedContent
* Refactor usages of `getTestId()` to use new hook
@kibanamachine kibanamachine mentioned this pull request Apr 23, 2021
Merged
@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
✅ 7.x

This backport PR will be merged automatically after passing CI.

kibanamachine added a commit that referenced this pull request Apr 23, 2021
@kibanamachine @paul-tavares
[Security solution][Endpoint] New Event Filters sub-section under Adm…
aaa21ed 
…inistration area (#97903) (#98169)

* Add Event Filters section to the Admin area (behind feature flag)
* new `PaginatedContent` generic component
* Refactor Trusted Apps grid view to use PaginatedContent
* Refactor usages of `getTestId()` to use new hook

Co-authored-by: Paul Tavares <56442535+paul-tavares@users.noreply.github.com>
madirey pushed a commit to madirey/kibana that referenced this pull request May 11, 2021
@paul-tavares @madirey
[Security solution][Endpoint] New Event Filters sub-section under Adm…
5de7357 
…inistration area (elastic#97903)

* Add Event Filters section to the Admin area (behind feature flag)
* new `PaginatedContent` generic component
* Refactor Trusted Apps grid view to use PaginatedContent
* Refactor usages of `getTestId()` to use new hook
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dasansol92 dasansol92 dasansol92 approved these changes

Assignees

@paul-tavares paul-tavares

Labels
auto-backport Deprecated - use backport:version if exact versions are needed OLM Sprint release_note:skip Skip the PR/issue when compiling release notes Team:Defend Workflows “EDR Workflows” sub-team of Security Solution v7.14.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@paul-tavares @elasticmachine @dasansol92 @kibanamachine