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

[Cases] Paginate user actions #130227

Closed
cnasikas opened this issue Apr 14, 2022 · 8 comments
Closed

[Cases] Paginate user actions #130227

cnasikas opened this issue Apr 14, 2022 · 8 comments
Assignees
Labels
Feature:Cases Cases feature Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams)

Comments

@cnasikas
Copy link
Member

cnasikas commented Apr 14, 2022

Every user action in Cases is recorded to an immutable appended list. This enables the ability of traceability and non-repudiation of users. Currently, all user actions are being fetched and shown to the UI. This could lead to performance issues if user actions grow significantly when for example multiple alerts will be attached to a case either dynamically (with the use of the case action) or manually (bulk operation).

The UI should fetch and show a portion of user actions. It should show the first 10 user actions and the last 10 user actions. Between the two lists of user actions, top and bottom from now on, a "Show more" button will be shown. The user can click the button to load 10 more user actions. After clicking the button, the top list will grow and contain 20 user actions and the bottom list will show the last 10 user actions.

A user, as she interacts with the case view page, may produce more user actions. In this scenario, the bottom list will grow but it will not hide any of the previously shown user actions. For example, if the bottom list contains 10 user actions and the user produces 2 more user actions then the bottom list will contain 12 user actions.

Mockup

Technical design

User flows

User clicks the show more button

User activity pagination (8)

User creates a user action

User activity pagination (9)

User creates a user action in descending order

User activity pagination (10)

React components

Cases use the EuiCommentList to display the user activity. To support the design requirements we need the following React components: UserActions, UserActionsList, and PaginationButton. The UserActionsList is a component with one purpose, to render a user activity. It does not know anything about pagination or top and bottom lists. It will fetch a portion of the user actions using the useFindUserActions hook and show it using the EuiCommentList component. The parameters of useFindUserActions, which dictate which portion of the user action will be fetched, are passed as props. Two UserActionsList components will be used, one for each list. The UserActions component is the parent component which will keep the state of the pagination. It is responsible to pass the required props to the two UserActionsList components, keep track of the state of the two lists, and change the status accordingly.

User activity pagination@2x (1)

React query

To support a "load more" pagination, React query provides infinitive queries. We should the useInfiniteQuery for the top list of user actions to only fetch the items of the next page and keep the data from previous pages. The bottom user actions can use the normal useQuery hook as we will not paginate them.

Open questions

  • Should we limit the number of total visible user actions in the bottom list?
@cnasikas cnasikas added Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) Feature:Cases Cases feature labels Apr 14, 2022
@elasticmachine
Copy link
Contributor

Pinging @elastic/response-ops-cases (Feature:Cases)

@elasticmachine
Copy link
Contributor

Pinging @elastic/response-ops (Team:ResponseOps)

@cnasikas cnasikas changed the title [Cases] Load more functionality for user actions [Cases] Paginate user actions Jan 19, 2023
@js-jankisalvi js-jankisalvi self-assigned this Feb 23, 2023
@cnasikas cnasikas self-assigned this Feb 23, 2023
@jonathan-buttner
Copy link
Contributor

Should we limit the number of total visible user actions in the bottom list?

I guess in the worst case the user actions at the bottom would not match any of the limiting filtering tabs (meaning that they'd only show up on the All user action list view). If we limit the number shown then it's possible they wouldn't be able to see them at all right?

If a user navigates away from this page and then back to it will we remember the pagination?

Will react query cache the results of the previous pages or will we need to hit the backend with a larger page size to account for the user actions we've already requested previously? Basically my concern is that we'll eventually have to request page 1 with perPage 10k.

@js-jankisalvi
Copy link
Contributor

If a user navigates away from this page and then back to it will we remember the pagination?

Yes navigation from page and then back will remember the pagination, but changing the filter or sort order will reset it as the total number of user actions will be different.

Will react query cache the results of the previous pages or will we need to hit the backend with a larger page size to account for the user actions we've already requested previously? Basically my concern is that we'll eventually have to request page 1 with perPage 10k.

No every time backend will be hit with different page number and constant perPage=10. Since UI already has the data of previous user actions, UI will take care of appending the new data to old data.

@js-jankisalvi
Copy link
Contributor

js-jankisalvi commented Feb 24, 2023

Should we limit the number of total visible user actions in the bottom list?

I guess in the worst case the user actions at the bottom would not match any of the limiting filtering tabs (meaning that they'd only show up on the All user action list view). If we limit the number shown then it's possible they wouldn't be able to see them at all right?

Here filter stats are updated every time new user action is added, so the stats will always tell the truth about total user actions no matter which filter is active.

Regarding how many user actions to show in the bottom list, one proposal:
we can show additional 9 user actions for that filter i.e.

  • if user is on All filter and added 5 comments and 4 other user actions (severity change, tags added etc.) we show them so total 19 user actions visible in bottom list now. And the moment one more user action added, we hide the previous 10 and show this newly added 10 in the bottom list.
  • If Comments filter is active, we show next 9 newly added comments (total 19) and when 10th comment is added, we can hide previous 10 comments of bottom list and show latest 10.

This way we can limit bottom list from exploding with lots of newly added user actions.

(P.S. this scenario only occurs when user hasn't changed the filter and added lots of new user actions for that filter. If user changes the filter or sort order, we reset the pagination to top 10 and last 10 user actions)

@cnasikas
Copy link
Member Author

I guess in the worst case the user actions at the bottom would not match any of the limiting filtering tabs (meaning that they'd only show up on the All user action list view).

If they do not match any of the limiting filtering tabs then it will not show. This could happen though without a limit.

Yes but a portion from the top of the bottom list. For example, assuming descending order, let's say the bottom list contains 10 items and the limit is 15 items. If I do 6 user actions then the first item in the list will be hidden. My last action will be visible because is the last one.

If a user navigates away from this page and then back to it will we remember the pagination?

Yes navigation from page and then back will remember the pagination, but changing the filter or sort order will reset it as the total number of user actions will be different.

No, the pagination will start fresh. When we navigate outside the case view page the component will unmount. When a component unmounts the state (pagination, filtering, etc) will be destroyed. When you navigate again, the component mounts with a clean state (initial one). This article explains the lifecycle of react query very well.

Will react query cache the results of the previous pages or will we need to hit the backend with a larger page size to account for the user actions we've already requested previously? Basically my concern is that we'll eventually have to request page 1 with perPage 10k.

As @js-jankisalvi said we will keep the previous user actions. React query supports this kind of pagination by introducing infinitive queries. React query will keep the results of the previous pages. When the user presses the "Show more button" we will always fetch the next items. For example { page: 2, perPage: 10 }, { page: 3, perPage: 10 }, etc. I will update the issue accordingly.

@cnasikas
Copy link
Member Author

  • if user is on All filter and added 5 comments and 4 other user actions (severity change, tags added etc.) we show them so total 19 user actions visible in bottom list now. And the moment one more user action added, we hide the previous 10 and show this newly added 10 in the bottom list.
  • If Comments filter is active, we show next 9 newly added comments (total 19) and when 10th comment is added, we can hide previous 10 comments of bottom list and show latest 10.

I am wondering if going from 19 visible items to 10 will make the UI "jumping" weirdly. I think showing always 19 and hiding the first item as the user do actions will avoid it. Let's start without a limit and revisit on a second iteration.

js-jankisalvi added a commit that referenced this issue Apr 4, 2023
## Summary

Implements #130227



https://user-images.githubusercontent.com/117571355/224038340-6b1c8cc7-3795-4412-8e67-a026dfe10776.mov



### 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] Any UI touched in this PR is usable by keyboard only (learn more
about [keyboard accessibility](https://webaim.org/techniques/keyboard/))
- [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))
- [x] This was checked for [cross-browser
compatibility](https://www.elastic.co/support/matrix#matrix_browsers)

### Flaky Test Runner
https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/2059

### For maintainers

- [x] 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)

---------

Co-authored-by: kibanamachine <[email protected]>
@cnasikas
Copy link
Member Author

cnasikas commented Apr 4, 2023

Implemented by #152702.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Cases Cases feature Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams)
Projects
None yet
Development

No branches or pull requests

5 participants