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.

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

[Security Solution] rules count is wrong #121754

Closed
gavinwye opened this issue Dec 21, 2021 · 9 comments · Fixed by #138902 or #139288
Closed

[Security Solution] rules count is wrong #121754

gavinwye opened this issue Dec 21, 2021 · 9 comments · Fixed by #138902 or #139288
Assignees
Labels
8.5 candidate bug Fixes for quality problems that affect the customer experience Feature:Rule Management Security Solution Detection Rule Management area fixed impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. QA:Validated Issue has been validated by QA Team:Detection Rule Management Security Detection Rule Management Team Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v8.5.0

Comments

@gavinwye
Copy link

Describe the bug:
The rules page says we're showing 561 rules but users can only see 20 of 561, because of the way the pagination works.

Kibana/Elasticsearch Stack version:

v 7.16.1

Original install method (e.g. download page, yum, from source, etc.):
Eden

Functional Area (e.g. Endpoint management, timelines, resolver, etc.):
Detections > Rules

Steps to reproduce:
View the page

Current behavior:
The rules page says we're showing 561 rules but users can only see 20 of 561, because of the way the pagination works.

Expected behavior:
We should tell users that we're showing 20 of 561 this element should be linked to the pagination and update as users change the pagination.

We should also question if this is really needed or can it be removed?

Screenshots (if relevant):
image

@gavinwye gavinwye added bug Fixes for quality problems that affect the customer experience triage_needed Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. labels Dec 21, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-solution (Team: SecuritySolution)

@gavinwye gavinwye added the UX: UI/UX Consultation Requires UX lead input/consult before development and UX lead approval on PR before merge. label Dec 21, 2021
@gavinwye gavinwye self-assigned this Dec 22, 2021
@banderror banderror added Feature:Rule Management Security Solution Detection Rule Management area impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. Team:Detection Rule Management Security Detection Rule Management Team Team:Detections and Resp Security Detection Response Team labels Jan 11, 2022
@elasticmachine
Copy link
Contributor

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

@banderror
Copy link
Contributor

This label could also just show Total 561 rules.
A more verbose label could show: Rules: total 561, showing 20, selected 0.

@yiyangliu9286 could you please share your thoughts?

@yiyangliu9286
Copy link

yiyangliu9286 commented Jan 12, 2022

This label could also just show Total 561 rules. A more verbose label could show: Rules: total 561, showing 20, selected 0.

@yiyangliu9286 could you please share your thoughts?

@banderror Thanks for asking this! According to the latest table utility bar component in Figma from EUI. They are suggested to show the format of: Showing 1-10 of {#} Things

So in the sense of Rule's table, for example, here's the design that we could go about based on the EUI table utility bar component:
To be as "Showing 1-10 of 596 rules" for the default state, if we're showing 20 rules per page by default, then it should be "Showing 1-20 of 596 rules" (Figma)
Security Rules

Although this table utility bar component has not yet become a coded component yet. I am suggesting it would be ideal if we could be as close as what EUI component is.

@banderror
Copy link
Contributor

@yiyangliu9286 thank you so much for your quick feedback! This suggestion LGTM 👍

@banderror banderror removed the UX: UI/UX Consultation Requires UX lead input/consult before development and UX lead approval on PR before merge. label Jan 12, 2022
@gavinwye gavinwye assigned gavinwye and unassigned gavinwye Feb 9, 2022
jpdjere added a commit that referenced this issue Aug 23, 2022
…ules table to show pagination info (#138902)

**Fixes:** #121754

## Summary

Changes rules count pagination count to format:

**"Showing 1-10 of 596 rules"** -> when page 1, rules per page is 10 and total rules is 596
**"Showing 6-6 of 6 rules"** -> when page 2, rules per page is 5 and total rules is 6
See other cases in tests.

**Before changes**
![image](https://user-images.githubusercontent.com/5354282/184883106-118f0041-5567-4cf8-bfd5-8383e8146018.png)
![image](https://user-images.githubusercontent.com/5354282/184884145-22d30482-cc2d-4796-8f84-d809a8ca6d8a.png)


**After changes**
![image](https://user-images.githubusercontent.com/5354282/184882149-d6c55dff-39a0-4a72-94a9-217b2e5ca7ac.png)
![image](https://user-images.githubusercontent.com/5354282/184886334-a107a2c5-31dc-4a7b-9e70-54c87c1630e0.png)

## Codebase changes

- Refactors `AllRulesUtilityBar` component to separate usage of the utility bar between the all-rules table use-case and the Exceptions table use-case, by creating a new `ExceptionsTableUtilityBar` component. 


### Checklist

Delete any items that are not applicable to this PR.

- [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios
- [x] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server))

### For maintainers

- [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
kibanamachine pushed a commit to kibanamachine/kibana that referenced this issue Aug 23, 2022
…ules table to show pagination info (elastic#138902)

**Fixes:** elastic#121754

## Summary

Changes rules count pagination count to format:

**"Showing 1-10 of 596 rules"** -> when page 1, rules per page is 10 and total rules is 596
**"Showing 6-6 of 6 rules"** -> when page 2, rules per page is 5 and total rules is 6
See other cases in tests.

**Before changes**
![image](https://user-images.githubusercontent.com/5354282/184883106-118f0041-5567-4cf8-bfd5-8383e8146018.png)
![image](https://user-images.githubusercontent.com/5354282/184884145-22d30482-cc2d-4796-8f84-d809a8ca6d8a.png)

**After changes**
![image](https://user-images.githubusercontent.com/5354282/184882149-d6c55dff-39a0-4a72-94a9-217b2e5ca7ac.png)
![image](https://user-images.githubusercontent.com/5354282/184886334-a107a2c5-31dc-4a7b-9e70-54c87c1630e0.png)

## Codebase changes

- Refactors `AllRulesUtilityBar` component to separate usage of the utility bar between the all-rules table use-case and the Exceptions table use-case, by creating a new `ExceptionsTableUtilityBar` component.

### Checklist

Delete any items that are not applicable to this PR.

- [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios
- [x] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server))

### For maintainers

- [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

(cherry picked from commit 147e414)
@jpdjere
Copy link
Contributor

jpdjere commented Aug 23, 2022

@gavinwye @yiyangliu9286 This issue was addressed in #138902

@jpdjere jpdjere closed this as completed Aug 23, 2022
jpdjere added a commit to jpdjere/kibana that referenced this issue Aug 23, 2022
…ules table to show pagination info (elastic#138902)

**Fixes:** elastic#121754

## Summary

Changes rules count pagination count to format:

**"Showing 1-10 of 596 rules"** -> when page 1, rules per page is 10 and total rules is 596
**"Showing 6-6 of 6 rules"** -> when page 2, rules per page is 5 and total rules is 6
See other cases in tests.

**Before changes**
![image](https://user-images.githubusercontent.com/5354282/184883106-118f0041-5567-4cf8-bfd5-8383e8146018.png)
![image](https://user-images.githubusercontent.com/5354282/184884145-22d30482-cc2d-4796-8f84-d809a8ca6d8a.png)

**After changes**
![image](https://user-images.githubusercontent.com/5354282/184882149-d6c55dff-39a0-4a72-94a9-217b2e5ca7ac.png)
![image](https://user-images.githubusercontent.com/5354282/184886334-a107a2c5-31dc-4a7b-9e70-54c87c1630e0.png)

## Codebase changes

- Refactors `AllRulesUtilityBar` component to separate usage of the utility bar between the all-rules table use-case and the Exceptions table use-case, by creating a new `ExceptionsTableUtilityBar` component.

### Checklist

Delete any items that are not applicable to this PR.

- [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios
- [x] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server))

### For maintainers

- [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

(cherry picked from commit 147e414)
kibanamachine added a commit that referenced this issue Aug 23, 2022
…ules table to show pagination info (#138902) (#139288)

**Fixes:** #121754

## Summary

Changes rules count pagination count to format:

**"Showing 1-10 of 596 rules"** -> when page 1, rules per page is 10 and total rules is 596
**"Showing 6-6 of 6 rules"** -> when page 2, rules per page is 5 and total rules is 6
See other cases in tests.

**Before changes**
![image](https://user-images.githubusercontent.com/5354282/184883106-118f0041-5567-4cf8-bfd5-8383e8146018.png)
![image](https://user-images.githubusercontent.com/5354282/184884145-22d30482-cc2d-4796-8f84-d809a8ca6d8a.png)

**After changes**
![image](https://user-images.githubusercontent.com/5354282/184882149-d6c55dff-39a0-4a72-94a9-217b2e5ca7ac.png)
![image](https://user-images.githubusercontent.com/5354282/184886334-a107a2c5-31dc-4a7b-9e70-54c87c1630e0.png)

## Codebase changes

- Refactors `AllRulesUtilityBar` component to separate usage of the utility bar between the all-rules table use-case and the Exceptions table use-case, by creating a new `ExceptionsTableUtilityBar` component.

### Checklist

Delete any items that are not applicable to this PR.

- [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios
- [x] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server))

### For maintainers

- [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

(cherry picked from commit 147e414)

Co-authored-by: Juan Pablo Djeredjian <[email protected]>
@MadameSheema
Copy link
Member

@deepikakeshav-qasource can you please validate on latest main when you have the chance? Thanks!

@MadameSheema MadameSheema reopened this Sep 5, 2022
@ghost
Copy link

ghost commented Sep 6, 2022

Hi @MadameSheema ,

We have validated this issue on Main branch build and observed that issue is Fixed. ✔️

Please find the below Testing Details:

Build info

Version: Mains
COMMIT: f013788fd6e4971151bbb048dcd368ea2773ff23

Screen-shots
image

image

image

image

Hence, We are closing this issue and marking as QA Validated!!

Thanks!!

@ghost ghost added the QA:Validated Issue has been validated by QA label Sep 6, 2022
@ghost ghost closed this as completed Sep 6, 2022
Mpdreamz pushed a commit to Mpdreamz/kibana that referenced this issue Sep 6, 2022
…ules table to show pagination info (elastic#138902)

**Fixes:** elastic#121754

## Summary

Changes rules count pagination count to format:

**"Showing 1-10 of 596 rules"** -> when page 1, rules per page is 10 and total rules is 596
**"Showing 6-6 of 6 rules"** -> when page 2, rules per page is 5 and total rules is 6
See other cases in tests.

**Before changes**
![image](https://user-images.githubusercontent.com/5354282/184883106-118f0041-5567-4cf8-bfd5-8383e8146018.png)
![image](https://user-images.githubusercontent.com/5354282/184884145-22d30482-cc2d-4796-8f84-d809a8ca6d8a.png)


**After changes**
![image](https://user-images.githubusercontent.com/5354282/184882149-d6c55dff-39a0-4a72-94a9-217b2e5ca7ac.png)
![image](https://user-images.githubusercontent.com/5354282/184886334-a107a2c5-31dc-4a7b-9e70-54c87c1630e0.png)

## Codebase changes

- Refactors `AllRulesUtilityBar` component to separate usage of the utility bar between the all-rules table use-case and the Exceptions table use-case, by creating a new `ExceptionsTableUtilityBar` component. 


### Checklist

Delete any items that are not applicable to this PR.

- [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios
- [x] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server))

### For maintainers

- [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
@ghost
Copy link

ghost commented Sep 23, 2022

Hi Team,

we have validated this issue on 8.5.0 BC1 and found the issue to be still fixed ✔️ .

Build Details:

Version:8.5.0
Commit:0d8de4df69f8084a94cdd9638d7de510813cb5ce
Build:56595

Screen-Shot:

image

Hence we are closing this issue and adding "QA:Validated" tag to it.

thanks !!

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.5 candidate bug Fixes for quality problems that affect the customer experience Feature:Rule Management Security Solution Detection Rule Management area fixed impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. QA:Validated Issue has been validated by QA Team:Detection Rule Management Security Detection Rule Management Team Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v8.5.0
Projects
None yet
6 participants